[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 <jjongsma at 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
>
> [...]
>
>