Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list

[Laine Stump]

On 10/26/20 6:53 AM, John Ferlan wrote:

On 10/14/20 1:08 PM, Jonathon Jongsma wrote:
The current udev node device driver ignores all events related to vdpa
devices. Since libvirt now supports vDPA network devices, include these
devices in the device list.

Example output:

virsh # nodedev-list
[...ommitted long list of nodedevs...]
vdpa_vdpa0

virsh # nodedev-dumpxml vdpa_vdpa0
<device>
   <name>vdpa_vdpa0</name>
   <path>/sys/devices/vdpa0</path>
   <parent>computer</parent>
   <driver>
     <name>vhost_vdpa</name>
   </driver>
   <capability type='vdpa'>
     <chardev>/dev/vhost-vdpa-0</chardev>
   </capability>
</device>

NOTE: normally the 'parent' would be a PCI device instead of 'computer',
but this example output is from the vdpa_sim kernel module, so it
doesn't have a normal parent device.

Signed-off-by: Jonathon Jongsma <jjong...@redhat.com>
---
  docs/formatnode.html.in            |  9 +++++
  docs/schemas/nodedev.rng           | 10 ++++++
  include/libvirt/libvirt-nodedev.h  |  1 +
  src/conf/node_device_conf.c        | 14 ++++++++
  src/conf/node_device_conf.h        | 11 ++++++-
  src/conf/virnodedeviceobj.c        |  4 ++-
  src/node_device/node_device_udev.c | 53 ++++++++++++++++++++++++++++++
  tools/virsh-nodedev.c              |  3 ++
  8 files changed, 103 insertions(+), 2 deletions(-)

Coverity notes a RESOURCE_LEAK

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 29a7eaa07c..b1b8427c05 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1142,6 +1142,55 @@ udevProcessCSS(struct udev_device *device,
      return 0;
  }
  
+
+static int
+udevGetVDPACharDev(const char *sysfs_path,
+                   virNodeDevCapDataPtr data)
+{
+    struct dirent *entry;
+    DIR *dir = NULL;
+    int direrr;
+
+    if (virDirOpenIfExists(&dir, sysfs_path) <= 0)
+        return -1;
Any return after this leaks @dir - need a VIR_CLOSE_DIR(dir)


Sigh. I have a nice patch series that converts all DIR*'s to 
g_autoptr(DIR). You'd think I would have seen this one in review :-/


+
+    while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
+        if (g_str_has_prefix(entry->d_name, "vhost-vdpa")) {
+            g_autofree char *chardev = g_strdup_printf("/dev/%s", 
entry->d_name);
+
+            if (!virFileExists(chardev)) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("vDPA chardev path '%s' does not exist"),
+                               chardev);
+                return -1;
+            }
+            VIR_DEBUG("vDPA chardev is at '%s'", chardev);
+
+            data->vdpa.chardev = g_steal_pointer(&chardev);
+            break;
+        }
+    }
+
+    if (direrr < 0)
+        return -1;
+
+    return 0;
+}
+
John

[...]