[Pkg-libvirt-maintainers] Bug#950995: Bug#950995: [arm64] virNetDevGetPhysicalFunction:1391 : internal error: The PF device for VF eth1 has no network device name
Guido Günther
agx at sigxcpu.org
Sun Feb 9 13:55:47 GMT 2020
Hi,
Thanks for digging out the patches. Marking as fixed in newer versions
then. We might want to fold this into a point release at some point.
Cheers,
-- Guido
On Sun, Feb 09, 2020 at 02:46:28PM +0100, Marcin Juszkiewicz wrote:
> Source: libvirt
> Version: 5.0.0-4
> Severity: normal
>
> On AArch64 systems with Cavium ThunderX cpus libvirt refuses to handle
> on board network cards:
>
> 2018-04-30 15:50:09.053+0000: 5069: info : hostname: uk-dc-cavium-01
> 2018-04-30 15:50:09.249+0000: 5069: error : virNetDevGetPhysicalFunction:1391 : internal error: The PF device for VF eth0 has no network device name
>
> https://bugs.linaro.org/show_bug.cgi?id=3778 has more details.
>
> Fixes were merged into 5.1.0 upstream release:
>
> From 6452e2f5e1837bd753ee465e6607ed3c4f62b815 Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:12 -0700
> Subject: [PATCH 1/4] util: fixing wrong assumption that PF has to have netdev assigned
>
>
> From 10bca495e040ef834760a244a31f8b87391c2378 Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:13 -0700
> Subject: [PATCH 2/4] util: Code simplification
>
>
> From 8fac64db5e7941efb820626f0043f5e0a31c79ee Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:14 -0700
> Subject: [PATCH 3/4] util: Fix for NULL dereference
>
>
> From 04983c3c6a821f67994b1c65d4d6175f3ac49d69 Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:15 -0700
> Subject: [PATCH 4/4] util: Fixing invalid error checking from virPCIGetNetname()
>
>
> Those patches apply to 5.0.0-4 Debian package without any problems.
> I have patched version in my repository [1].
>
> 1. http://obs.linaro.org/home:/marcin.juszkiewicz/debian-buster/
>
>
> -- System Information:
> Debian Release: 10.2
> APT prefers stable-updates
> APT policy: (500, 'stable-updates'), (500, 'stable')
> Architecture: arm64 (aarch64)
>
> Kernel: Linux 5.4.0-1-arm64 (SMP w/46 CPU cores)
> Locale: LANG=en_US.UTF-8, LC_CTYPE=pl_PL.UTF-8 (charmap=UTF-8),
> LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
> Shell: /bin/sh linked to /bin/dash
> Init: systemd (via /run/systemd/system)
> LSM: AppArmor: enabled
>
> From 6452e2f5e1837bd753ee465e6607ed3c4f62b815 Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:12 -0700
> Subject: [PATCH 1/4] util: fixing wrong assumption that PF has to have netdev
> assigned
>
> libvirt wrongly assumes that VF netdev has to have the
> netdev assigned to PF. There is no such requirement in SRIOV standard.
> This patch change the virNetDevSwitchdevFeature() function to deal
> with SRIOV devices which does not have netdev on PF. Also corrects
> one comment about PF netdev assumption.
>
> One example of such devices is ThunderX VNIC.
> By applying this change, VF device is used for virNetlinkCommand() as
> it is the only netdev assigned to VNIC.
>
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Signed-off-by: dann frazier <dann.frazier at canonical.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/util/virnetdev.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1355,9 +1355,8 @@
> }
>
> if (!*pfname) {
> - /* this shouldn't be possible. A VF can't exist unless its
> - * PF device is bound to a network driver
> - */
> + /* The SRIOV standard does not require VF netdevs to have
> + * the netdev assigned to a PF. */
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("The PF device for VF %s has no network device name"),
> ifname);
> @@ -3178,8 +3177,12 @@
> if ((is_vf = virNetDevIsVirtualFunction(ifname)) < 0)
> return ret;
>
> - if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
> - goto cleanup;
> + if (is_vf == 1) {
> + /* Ignore error if PF does not have netdev assigned.
> + * In that case pfname == NULL. */
> + if (virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
> + virResetLastError();
> + }
>
> pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
> virNetDevGetPCIDevice(ifname);
>
> From 10bca495e040ef834760a244a31f8b87391c2378 Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:13 -0700
> Subject: [PATCH 2/4] util: Code simplification
>
> Removing redundant sections of the code
>
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Signed-off-by: dann frazier <dann.frazier at canonical.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/util/virnetdev.c | 33 ++++++---------------------------
> 1 file changed, 6 insertions(+), 27 deletions(-)
>
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1443,29 +1443,20 @@
> virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
> int *vf)
> {
> - char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
> int ret = -1;
>
> *pfname = NULL;
>
> if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)
> - return ret;
> -
> - if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0)
> - goto cleanup;
> + return -1;
>
> - if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0)
> + if (virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf) < 0)
> goto cleanup;
>
> - ret = virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf);
> -
> + ret = 0;
> cleanup:
> if (ret < 0)
> VIR_FREE(*pfname);
> -
> - VIR_FREE(vf_sysfs_path);
> - VIR_FREE(pf_sysfs_path);
> -
> return ret;
> }
>
> @@ -1861,13 +1852,9 @@
> * it to PF + VFname
> */
>
> - if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
> + if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
> goto cleanup;
> -
> pfDevName = pfDevOrig;
> -
> - if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
> - goto cleanup;
> }
>
> if (pfDevName) {
> @@ -2019,13 +2006,9 @@
> * it to PF + VFname
> */
>
> - if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
> + if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
> goto cleanup;
> -
> pfDevName = pfDevOrig;
> -
> - if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
> - goto cleanup;
> }
>
> /* if there is a PF, it's now in pfDevName, and linkdev is either
> @@ -2224,13 +2207,9 @@
> * it to PF + VFname
> */
>
> - if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
> + if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf))
> goto cleanup;
> -
> pfDevName = pfDevOrig;
> -
> - if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
> - goto cleanup;
> }
>
>
>
> From 8fac64db5e7941efb820626f0043f5e0a31c79ee Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:14 -0700
> Subject: [PATCH 3/4] util: Fix for NULL dereference
>
> The device xml parser code does not set "model" while parsing the
> following XML:
>
> <interface type='hostdev'>
> <source>
> <address type='pci' domain='0x0002' bus='0x01' slot='0x00' function='0x2'/>
> </source>
> </interface>
>
> The net->model can be NULL and therefore must be compared using
> STREQ_NULLABLE instead of plain STREQ.
>
> Fixes: ac47e4a6225 (qemu: replace "def->nets[i]" with "net" and "def->sounds[i]" with "sound")
> Fixes: c7fc151eec7 (qemu: assign virtio devices to PCIe slot when appropriate)
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Signed-off-by: dann frazier <dann.frazier at canonical.com>
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/qemu/qemu_domain_address.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -230,10 +230,8 @@
> for (i = 0; i < def->nnets; i++) {
> virDomainNetDefPtr net = def->nets[i];
>
> - if (net->model &&
> - STREQ(net->model, "spapr-vlan")) {
> + if (STREQ_NULLABLE(net->model, "spapr-vlan"))
> net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
> - }
>
> if (qemuDomainAssignSpaprVIOAddress(def, &net->info, VIO_ADDR_NET) < 0)
> goto cleanup;
> @@ -323,7 +321,7 @@
> virDomainNetDefPtr net = def->nets[i];
>
> if (net->model &&
> - STREQ(net->model, "virtio") &&
> + STREQ_NULLABLE(net->model, "virtio") &&
> net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> net->info.type = type;
> }
> @@ -691,14 +689,14 @@
> * addresses for other hostdev devices.
> */
> if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
> - STREQ(net->model, "usb-net")) {
> + STREQ_NULLABLE(net->model, "usb-net")) {
> return 0;
> }
>
> - if (STREQ(net->model, "virtio"))
> + if (STREQ_NULLABLE(net->model, "virtio"))
> return virtioFlags;
>
> - if (STREQ(net->model, "e1000e"))
> + if (STREQ_NULLABLE(net->model, "e1000e"))
> return pcieFlags;
>
> return pciFlags;
>
> From 04983c3c6a821f67994b1c65d4d6175f3ac49d69 Mon Sep 17 00:00:00 2001
> From: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Date: Tue, 22 Jan 2019 12:26:15 -0700
> Subject: [PATCH 4/4] util: Fixing invalid error checking from
> virPCIGetNetname()
>
> The @linkdev is In/Out function parameter as second order
> reference pointer so requires first order dereference for
> checking NULL which can be the result of virPCIGetNetName().
>
> Fixes: d6ee56d7237 (util: change virPCIGetNetName() to not return error if device has no net name)
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Signed-off-by: dann frazier <dann.frazier at canonical.com>
> ---
> src/util/virhostdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -314,7 +314,7 @@
> if (virPCIGetNetName(sysfs_path, 0, NULL, linkdev) < 0)
> return -1;
>
> - if (!linkdev) {
> + if (!(*linkdev)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("The device at %s has no network device name"),
> sysfs_path);
>
> _______________________________________________
> Pkg-libvirt-maintainers mailing list
> Pkg-libvirt-maintainers at alioth-lists.debian.net
> https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-libvirt-maintainers
More information about the Pkg-libvirt-maintainers
mailing list