On Wed, 30 Sep 2020 10:02:04 -0500
Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
> Adding Cindy and Jason to cc for input on the path issue below
>
> On Tue, 29 Sep 2020 15:56:13 -0400
> Laine Stump <laine@xxxxxxxxxx> wrote:
>
> > On 9/24/20 5:45 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.
> >
> >
> > Can you provide an example in the commit log of what the output xml
> > looks like for nodedev-list and nodedev-dumpxml?
>
> So, this is what the xml looks like for the vdpa node device:
>
> <device>
> <name>vdpa_vdpa0</name>
> <path>/sys/devices/vdpa0</path>
> <parent>computer</parent>
> <driver>
> <name>vhost_vdpa</name>
> </driver>
> <capability type='vdpa'>
> </capability>
> </device>
>
>
> The /sys/devices/ path is provided by udev, but unfortunately that
> path is not particularly useful for using the device with libvirt.
> Ideally, the node device xml should include the path to the chardev
> (e.g. /dev/vhost-vdpa-0) which is used to actually connect to the
> device. I don't see an obvious way (aside from string manipulation,
> which doesn't seem very reliable) to get from this sysfs path to the
> /dev/ path. Jason or Cindy, do you have any input here?
So after after a little more thinking and poking it looks like it may
not be too hard to connect the sysfs path to the chardev after all. I
don't have any vdpa hardware at the moment, but for vdpa_sim, the sysfs
path /sys/devices/vdpa0 has a subdirectory named vhost-vdpa-0 that
matches the name under /dev/ (/dev/vhost-vdpa-0). So perhaps I can use
that to generate the device path.
Something like this pseudo-code:
for file in syspath {
if file.name begins with "vhost-vdpa" {
return "/dev/" + file.name;
}
}
Are these assumptions safe? Will the file always begin with
"vhost-vdpa"? Will it always match the /dev file? Jason?
In that case, I can adjust the node device XML to be something like:
<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>
(not totally sure about the naming of the xml element).
>
>
> >
> >
> > >
> > > Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > > ---
> > > include/libvirt/libvirt-nodedev.h | 1 +
> > > src/conf/node_device_conf.c | 5 +++++
> > > src/conf/node_device_conf.h | 4 +++-
> > > src/conf/virnodedeviceobj.c | 4 +++-
> > > src/node_device/node_device_udev.c | 16 ++++++++++++++++
> > > tools/virsh-nodedev.c | 3 +++
> > > 6 files changed, 31 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/libvirt/libvirt-nodedev.h
> > > b/include/libvirt/libvirt-nodedev.h index dd2ffd5782..b73b076f14
> > > 100644 --- a/include/libvirt/libvirt-nodedev.h
> > > +++ b/include/libvirt/libvirt-nodedev.h
> > > @@ -82,6 +82,7 @@ typedef enum {
> > > VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV = 1 << 14,
> > > /* Mediated device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV
> > > = 1 << 15, /* CCW device */
> > > VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV = 1 << 16, /* CSS
> > > device */
> > > + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA = 1 << 17, /*
> > > vDPA device */ } virConnectListAllNodeDeviceFlags;
> > >
> > > int virConnectListAllNodeDevices
> > > (virConnectPtr conn, diff --git a/src/conf/node_device_conf.c
> > > b/src/conf/node_device_conf.c index a9a03ad6c2..3eab1cda75 100644
> > > --- a/src/conf/node_device_conf.c
> > > +++ b/src/conf/node_device_conf.c
> > > @@ -66,6 +66,7 @@ VIR_ENUM_IMPL(virNodeDevCap,
> > > "mdev",
> > > "ccw",
> > > "css",
> > > + "vdpa",
> > > );
> > >
> > > VIR_ENUM_IMPL(virNodeDevNetCap,
> > > @@ -614,6 +615,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef
> > > *def) case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > > case VIR_NODE_DEV_CAP_FC_HOST:
> > > case VIR_NODE_DEV_CAP_VPORTS:
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > case VIR_NODE_DEV_CAP_LAST:
> > > break;
> > > }
> > > @@ -1913,6 +1915,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr
> > > ctxt, case VIR_NODE_DEV_CAP_FC_HOST:
> > > case VIR_NODE_DEV_CAP_VPORTS:
> > > case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > case VIR_NODE_DEV_CAP_LAST:
> > > virReportError(VIR_ERR_INTERNAL_ERROR,
> > > _("unknown capability type '%d' for
> > > '%s'"), @@ -2232,6 +2235,7 @@
> > > virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case
> > > VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_CCW_DEV:
> > > case VIR_NODE_DEV_CAP_CSS_DEV:
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > case VIR_NODE_DEV_CAP_LAST:
> > > /* This case is here to shutup the compiler */
> > > break;
> > > @@ -2286,6 +2290,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr
> > > def) case VIR_NODE_DEV_CAP_MDEV:
> > > case VIR_NODE_DEV_CAP_CCW_DEV:
> > > case VIR_NODE_DEV_CAP_CSS_DEV:
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > case VIR_NODE_DEV_CAP_LAST:
> > > break;
> > > }
> > > diff --git a/src/conf/node_device_conf.h
> > > b/src/conf/node_device_conf.h index 5484bc340f..4f8e47a068 100644
> > > --- a/src/conf/node_device_conf.h
> > > +++ b/src/conf/node_device_conf.h
> > > @@ -65,6 +65,7 @@ typedef enum {
> > > VIR_NODE_DEV_CAP_MDEV, /* Mediated device */
> > > VIR_NODE_DEV_CAP_CCW_DEV, /* s390 CCW device */
> > > VIR_NODE_DEV_CAP_CSS_DEV, /* s390 channel
> > > subsystem device */
> > > + VIR_NODE_DEV_CAP_VDPA, /* vDPA device */
> > >
> > > VIR_NODE_DEV_CAP_LAST
> > > } virNodeDevCapType;
> > > @@ -369,7 +370,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr
> > > caps); VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES | \
> > > VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV
> > > | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV | \
> > > - VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV)
> > > + VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV
> > > | \
> > > + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA)
> > >
> > > int
> > > virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr
> > > scsi_host); diff --git a/src/conf/virnodedeviceobj.c
> > > b/src/conf/virnodedeviceobj.c index 8aefd15e94..83c58ebe91 100644
> > > --- a/src/conf/virnodedeviceobj.c
> > > +++ b/src/conf/virnodedeviceobj.c
> > > @@ -711,6 +711,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj
> > > *obj, case VIR_NODE_DEV_CAP_MDEV:
> > > case VIR_NODE_DEV_CAP_CCW_DEV:
> > > case VIR_NODE_DEV_CAP_CSS_DEV:
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > case VIR_NODE_DEV_CAP_LAST:
> > > break;
> > > }
> > > @@ -862,7 +863,8 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
> > > MATCH(MDEV_TYPES) ||
> > > MATCH(MDEV) ||
> > > MATCH(CCW_DEV) ||
> > > - MATCH(CSS_DEV)))
> > > + MATCH(CSS_DEV) ||
> > > + MATCH(VDPA)))
> > > return false;
> > > }
> > >
> > > diff --git a/src/node_device/node_device_udev.c
> > > b/src/node_device/node_device_udev.c index 12e3f30bad..fda72f9071
> > > 100644 --- a/src/node_device/node_device_udev.c
> > > +++ b/src/node_device/node_device_udev.c
> > > @@ -1144,6 +1144,18 @@ udevProcessCSS(struct udev_device *device,
> > > return 0;
> > > }
> > >
> > > +
> > > +static int
> > > +udevProcessVDPA(struct udev_device *device,
> > > + virNodeDeviceDefPtr def)
> > > +{
> > > + if (udevGenerateDeviceName(device, def, NULL) != 0)
> > > + return -1;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +
> > > static int
> > > udevGetDeviceNodes(struct udev_device *device,
> > > virNodeDeviceDefPtr def)
> > > @@ -1224,6 +1236,8 @@ udevGetDeviceType(struct udev_device
> > > *device, *type = VIR_NODE_DEV_CAP_CCW_DEV;
> > > else if (STREQ_NULLABLE(subsystem, "css"))
> > > *type = VIR_NODE_DEV_CAP_CSS_DEV;
> > > + else if (STREQ_NULLABLE(subsystem, "vdpa"))
> > > + *type = VIR_NODE_DEV_CAP_VDPA;
> > >
> > > VIR_FREE(subsystem);
> > > }
> > > @@ -1270,6 +1284,8 @@ udevGetDeviceDetails(struct udev_device
> > > *device, return udevProcessCCW(device, def);
> > > case VIR_NODE_DEV_CAP_CSS_DEV:
> > > return udevProcessCSS(device, def);
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > + return udevProcessVDPA(device, def);
> > > case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > > case VIR_NODE_DEV_CAP_SYSTEM:
> > > case VIR_NODE_DEV_CAP_FC_HOST:
> > > diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
> > > index 2edd403a64..19f0c17b4f 100644
> > > --- a/tools/virsh-nodedev.c
> > > +++ b/tools/virsh-nodedev.c
> > > @@ -464,6 +464,9 @@ cmdNodeListDevices(vshControl *ctl, const
> > > vshCmd *cmd G_GNUC_UNUSED) case VIR_NODE_DEV_CAP_CSS_DEV:
> > > flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV;
> > > break;
> > > + case VIR_NODE_DEV_CAP_VDPA:
> > > + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA;
> > > + break;
> > > case VIR_NODE_DEV_CAP_LAST:
> > > break;
> > > }
> >
> >
>
