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);

Reply via email to