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.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); > > _______________________________________________ > Pkg-libvirt-maintainers mailing list > pkg-libvirt-maintain...@alioth-lists.debian.net > https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-libvirt-maintainers
