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.bierna...@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.bierna...@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.bierna...@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.bierna...@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.bierna...@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.bierna...@linaro.org> Signed-off-by: dann frazier <dann.fraz...@canonical.com> Signed-off-by: Michal Privoznik <mpriv...@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.bierna...@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.bierna...@linaro.org> Signed-off-by: dann frazier <dann.fraz...@canonical.com> Signed-off-by: Michal Privoznik <mpriv...@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.bierna...@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.bierna...@linaro.org> Signed-off-by: dann frazier <dann.fraz...@canonical.com> Reviewed-by: Michal Privoznik <mpriv...@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.bierna...@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.bierna...@linaro.org> Signed-off-by: dann frazier <dann.fraz...@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);