Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list
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 <jjongsma(a)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
[...]