[libvirt PATCH v4 00/12] Add ability to create mediated devices in libvirt

[libvirt-sandbox PATCH] gitlab:...

[libvirt PATCH] nodedev: Makefile:...

Jonathon Jongsma

Thursday, 18 June 2020 Thu, 18 Jun '20

4:05 p.m.

This is the first portion of an effort to support persistent mediated devices with libvirt. This first series simply enables creating and destroying non-persistent mediated devices via the virNodeDeviceCreateXML() and virNodeDeviceDestroy() functions. The 'mdevctl' utility[1] provides the backend implementation. Hopefully these are the last changes and version can simply be pushed upstream. Changes in v4: - coding style / spacing fixes - remove 'persist' arg from start command - fixed distcheck failure by including test data dir in EXTRA_DIST - Add an item to NEWS.rst [1] https://github.com/mdevctl/mdevctl Jonathon Jongsma (12): nodedev: make iommuGroup optional for mdevs nodedev: factor out nodeDeviceHasCapability() nodedev: add support for mdev attributes nodedev: refactor nodeDeviceFindNewDevice() nodedev: store mdev UUID in mdev caps nodedev: add mdev support to virNodeDeviceCreateXML() nodedev: Build a non-loadable driver lib nodedev: Add testing for 'mdevctl start' nodedev: add mdev support to virNodeDeviceDestroy() nodedev: Add testing for 'mdevctl stop' docs: note node device fields that are read-only news: mediated devices can be created NEWS.rst | 7 + build-aux/syntax-check.mk | 2 +- docs/formatnode.html.in | 21 +- docs/schemas/nodedev.rng | 18 +- libvirt.spec.in | 2 + m4/virt-external-programs.m4 | 3 + src/conf/node_device_conf.c | 68 +++- src/conf/node_device_conf.h | 3 + src/conf/virnodedeviceobj.c | 34 ++ src/conf/virnodedeviceobj.h | 3 + src/libvirt_private.syms | 3 + src/node_device/Makefile.inc.am | 23 +- src/node_device/node_device_driver.c | 360 ++++++++++++++++-- src/node_device/node_device_driver.h | 7 + src/node_device/node_device_udev.c | 5 +- src/util/virmdev.c | 12 + src/util/virmdev.h | 11 + tests/Makefile.am | 15 + ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 + ...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.json | 1 + tests/nodedevmdevctldata/mdevctl-stop.argv | 1 + tests/nodedevmdevctltest.c | 303 +++++++++++++++ ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 7 + ...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 9 + ...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 8 + 29 files changed, 863 insertions(+), 68 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json create mode 100644 tests/nodedevmdevctldata/mdevctl-stop.argv create mode 100644 tests/nodedevmdevctltest.c create mode 100644 tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml create mode 100644 tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml create mode 100644 tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml -- 2.21.3

Show replies by date

Jonathon Jongsma

Thursday, 18 June Thu, 18 Jun

4:05 p.m.

New subject: [libvirt PATCH v4 01/12] nodedev: make iommuGroup optional for mdevs

When parsing a nodedev xml file, the iommuGroup element should be optional. This element should be read-only and is determined by the device driver. While this is a change to existing behavior, it doesn't break backwards-compatibility because it makes the parser less strict. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- docs/formatnode.html.in | 5 +++-- docs/schemas/nodedev.rng | 12 +++++++----- src/conf/node_device_conf.c | 14 ++++++++------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 76eae928de..4ed43ec0cb 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -390,8 +390,9 @@ <dt><code>iommuGroup</code></dt> <dd> This element supports a single attribute <code>number</code> - which holds the IOMMU group number the mediated device belongs - to. + which holds the IOMMU group number to which the mediated device + belongs. This is a read-only field that is reported by the + device driver. </dd> </dl> </dd> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index fe6ffa0b53..ca3a79db87 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -629,11 +629,13 @@ <data type='string'/> </attribute> </element> - <element name='iommuGroup'> - <attribute name='number'> - <ref name='unsignedInt'/> - </attribute> - </element> + <optional> + <element name='iommuGroup'> + <attribute name='number'> + <ref name='unsignedInt'/> + </attribute> + </element> + </optional> </define> <define name='capccwdev'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index bccdbd0af8..2ef4514f05 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1775,13 +1775,15 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, goto out; } - if (virNodeDevCapsDefParseULong("number(./iommuGroup[1]/@number)", ctxt, - &mdev->iommuGroupNumber, def, - _("missing iommuGroup number attribute for " - "'%s'"), - _("invalid iommuGroup number attribute for " - "'%s'")) < 0) + /* 'iommuGroup' is optional, only report an error if the supplied value is + * invalid (-2), not if it's missing (-1) */ + if (virXPathUInt("number(./iommuGroup[1]/@number)", + ctxt, &mdev->iommuGroupNumber) < -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid iommuGroup number attribute for '%s'"), + def->name); goto out; + } ret = 0; out: -- 2.21.3

Jonathon Jongsma

4:05 p.m.

New subject: [libvirt PATCH v4 02/12] nodedev: factor out nodeDeviceHasCapability()

Currently nodeDeviceCreateXML() and nodeDeviceDestroy() only support NPIV HBAs, but we want to be able to create mdev devices as well. This is a first step to enabling that support. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/node_device/node_device_driver.c | 91 ++++++++++++++++++---------- 1 file changed, 58 insertions(+), 33 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index ee175e1095..ba7ea50e5b 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -488,6 +488,21 @@ nodeDeviceFindNewDevice(virConnectPtr conn, } +static bool +nodeDeviceHasCapability(virNodeDeviceDefPtr def, virNodeDevCapType type) +{ + virNodeDevCapsDefPtr cap = def->caps; + + while (cap != NULL) { + if (cap->data.type == type) + return true; + cap = cap->next; + } + + return false; +} + + virNodeDevicePtr nodeDeviceCreateXML(virConnectPtr conn, const char *xmlDesc, @@ -513,24 +528,29 @@ nodeDeviceCreateXML(virConnectPtr conn, if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) return NULL; - if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) - return NULL; + if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_SCSI_HOST)) { + if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) + return NULL; - if ((parent_host = virNodeDeviceObjListGetParentHost(driver->devs, def)) < 0) - return NULL; + if ((parent_host = virNodeDeviceObjListGetParentHost(driver->devs, def)) < 0) + return NULL; - if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) - return NULL; + if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) + return NULL; - device = nodeDeviceFindNewDevice(conn, wwnn, wwpn); - /* We don't check the return value, because one way or another, - * we're returning what we get... */ + device = nodeDeviceFindNewDevice(conn, wwnn, wwpn); + /* We don't check the return value, because one way or another, + * we're returning what we get... */ - if (device == NULL) - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device for '%s' with matching " - "wwnn '%s' and wwpn '%s'"), - def->name, wwnn, wwpn); + if (device == NULL) + virReportError(VIR_ERR_NO_NODE_DEVICE, + _("no node device for '%s' with matching " + "wwnn '%s' and wwpn '%s'"), + def->name, wwnn, wwpn); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported device type")); + } return device; } @@ -557,31 +577,36 @@ nodeDeviceDestroy(virNodeDevicePtr device) if (virNodeDeviceDestroyEnsureACL(device->conn, def) < 0) goto cleanup; - if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) - goto cleanup; + if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_SCSI_HOST)) { + if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) + goto cleanup; - /* Because we're about to release the lock and thus run into a race - * possibility (however improbable) with a udevAddOneDevice change - * event which would essentially free the existing @def (obj->def) and - * replace it with something new, we need to grab the parent field - * and then find the parent obj in order to manage the vport */ - parent = g_strdup(def->parent); + /* Because we're about to release the lock and thus run into a race + * possibility (however improbable) with a udevAddOneDevice change + * event which would essentially free the existing @def (obj->def) and + * replace it with something new, we need to grab the parent field + * and then find the parent obj in order to manage the vport */ + parent = g_strdup(def->parent); - virNodeDeviceObjEndAPI(&obj); + virNodeDeviceObjEndAPI(&obj); - if (!(obj = virNodeDeviceObjListFindByName(driver->devs, parent))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find parent '%s' definition"), parent); - goto cleanup; - } + if (!(obj = virNodeDeviceObjListFindByName(driver->devs, parent))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find parent '%s' definition"), parent); + goto cleanup; + } - if (virSCSIHostGetNumber(parent, &parent_host) < 0) - goto cleanup; + if (virSCSIHostGetNumber(parent, &parent_host) < 0) + goto cleanup; - if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) - goto cleanup; + if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) + goto cleanup; - ret = 0; + ret = 0; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported device type")); + } cleanup: virNodeDeviceObjEndAPI(&obj); -- 2.21.3

Jonathon Jongsma

4:05 p.m.

New subject: [libvirt PATCH v4 03/12] nodedev: add support for mdev attributes

Mediated devices support arbitrary vendor-specific attributes that can be attached to a mediated device. These attributes are ordered, and are written to sysfs in order after a device is created. This patch adds support for these attributes to the mdev data types and XML schema. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- docs/formatnode.html.in | 7 +++++ docs/schemas/nodedev.rng | 6 +++++ src/conf/node_device_conf.c | 53 ++++++++++++++++++++++++++++++++++--- src/conf/node_device_conf.h | 2 ++ src/libvirt_private.syms | 2 ++ src/util/virmdev.c | 12 +++++++++ src/util/virmdev.h | 11 ++++++++ 7 files changed, 90 insertions(+), 3 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 4ed43ec0cb..0637d457ee 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -394,6 +394,13 @@ belongs. This is a read-only field that is reported by the device driver. </dd> + <dt><code>attr</code></dt> + <dd> + This optional element can occur multiple times. It represents a + vendor-specific attribute that is used to configure this + mediated device. It has two required attributes: + <code>name</code> and <code>value</code>. + </dd> </dl> </dd> <dt><code>ccw</code></dt> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index ca3a79db87..4b2b350fd8 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -636,6 +636,12 @@ </attribute> </element> </optional> + <zeroOrMore> + <element name="attr"> + <attribute name="name"/> + <attribute name="value"/> + </element> + </zeroOrMore> </define> <define name='capccwdev'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 2ef4514f05..623a2cb79c 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -500,6 +500,22 @@ virNodeDeviceCapStorageDefFormat(virBufferPtr buf, virBufferAddLit(buf, "<capability type='hotpluggable'/>\n"); } +static void +virNodeDeviceCapMdevDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + size_t i; + + virBufferEscapeString(buf, "<type id='%s'/>\n", data->mdev.type); + virBufferAsprintf(buf, "<iommuGroup number='%u'/>\n", + data->mdev.iommuGroupNumber); + + for (i = 0; i < data->mdev.nattributes; i++) { + virMediatedDeviceAttrPtr attr = data->mdev.attributes[i]; + virBufferAsprintf(buf, "<attr name='%s' value='%s'/>\n", + attr->name, attr->value); + } +} char * virNodeDeviceDefFormat(const virNodeDeviceDef *def) @@ -583,9 +599,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferEscapeString(&buf, "<type>%s</type>\n", virNodeDevDRMTypeToString(data->drm.type)); break; case VIR_NODE_DEV_CAP_MDEV: - virBufferEscapeString(&buf, "<type id='%s'/>\n", data->mdev.type); - virBufferAsprintf(&buf, "<iommuGroup number='%u'/>\n", - data->mdev.iommuGroupNumber); + virNodeDeviceCapMdevDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_CCW_DEV: virBufferAsprintf(&buf, "<cssid>0x%x</cssid>\n", @@ -1757,6 +1771,27 @@ virNodeDevCapSystemParseXML(xmlXPathContextPtr ctxt, return ret; } +static int +virNodeDevCapMdevAttributeParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr node, + virNodeDevCapMdevPtr mdev) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt); + g_autoptr(virMediatedDeviceAttr) attr = virMediatedDeviceAttrNew(); + + ctxt->node = node; + attr->name = virXPathString("string(./@name)", ctxt); + attr->value = virXPathString("string(./@value)", ctxt); + if (!attr->name || !attr->value) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("mdev attribute missing name or value")); + return -1; + } + + return VIR_APPEND_ELEMENT(mdev->attributes, + mdev->nattributes, + attr); +} static int virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, @@ -1766,6 +1801,9 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, { VIR_XPATH_NODE_AUTORESTORE(ctxt); int ret = -1; + int nattrs = 0; + g_autofree xmlNodePtr *attrs = NULL; + size_t i; ctxt->node = node; @@ -1785,6 +1823,12 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, goto out; } + if ((nattrs = virXPathNodeSet("./attr", ctxt, &attrs)) < 0) + goto out; + + for (i = 0; i < nattrs; i++) + virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i], mdev); + ret = 0; out: return ret; @@ -2174,6 +2218,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) break; case VIR_NODE_DEV_CAP_MDEV: VIR_FREE(data->mdev.type); + for (i = 0; i < data->mdev.nattributes; i++) + virMediatedDeviceAttrFree(data->mdev.attributes[i]); + VIR_FREE(data->mdev.attributes); break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_DRM: diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 9e4b0847fb..e3e1e788d4 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -141,6 +141,8 @@ typedef virNodeDevCapMdev *virNodeDevCapMdevPtr; struct _virNodeDevCapMdev { char *type; unsigned int iommuGroupNumber; + virMediatedDeviceAttrPtr *attributes; + size_t nattributes; }; typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 284c6c3880..22bd8b9c17 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2487,6 +2487,8 @@ virMacMapRemove; virMacMapWriteFile; # util/virmdev.h +virMediatedDeviceAttrFree; +virMediatedDeviceAttrNew; virMediatedDeviceFree; virMediatedDeviceGetIOMMUGroupDev; virMediatedDeviceGetIOMMUGroupNum; diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 51a88a91d7..b8023dd991 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -512,3 +512,15 @@ virMediatedDeviceTypeReadAttrs(const char *sysfspath, return 0; } + +virMediatedDeviceAttrPtr virMediatedDeviceAttrNew(void) +{ + return g_new0(virMediatedDeviceAttr, 1); +} + +void virMediatedDeviceAttrFree(virMediatedDeviceAttrPtr attr) +{ + g_free(attr->name); + g_free(attr->value); + g_free(attr); +} diff --git a/src/util/virmdev.h b/src/util/virmdev.h index 51f7f608a2..eb167ccb48 100644 --- a/src/util/virmdev.h +++ b/src/util/virmdev.h @@ -37,6 +37,17 @@ typedef struct _virMediatedDevice virMediatedDevice; typedef virMediatedDevice *virMediatedDevicePtr; typedef struct _virMediatedDeviceList virMediatedDeviceList; typedef virMediatedDeviceList *virMediatedDeviceListPtr; +typedef struct _virMediatedDeviceAttr virMediatedDeviceAttr; +typedef virMediatedDeviceAttr *virMediatedDeviceAttrPtr; + +struct _virMediatedDeviceAttr { + char *name; + char *value; +}; + +virMediatedDeviceAttrPtr virMediatedDeviceAttrNew(void); +void virMediatedDeviceAttrFree(virMediatedDeviceAttrPtr attr); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDeviceAttr, virMediatedDeviceAttrFree); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDeviceList, virObjectUnref); -- 2.21.3

Jonathon Jongsma

4:05 p.m.

New subject: [libvirt PATCH v4 04/12] nodedev: refactor nodeDeviceFindNewDevice()

In preparation for creating mediated devices in libvirt, we will need to wait for new mediated devices to be created as well. Refactor nodeDeviceFindNewDevice() so that we can re-use the main logic from this function to wait for different device types by passing a different 'find' function. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/node_device/node_device_driver.c | 41 +++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index ba7ea50e5b..d6255a43c8 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -447,6 +447,10 @@ nodeDeviceGetTime(time_t *t) } +typedef virNodeDevicePtr (*nodeDeviceFindNewDeviceFunc)(virConnectPtr conn, + const void* opaque); + + /* When large numbers of devices are present on the host, it's * possible for udev not to realize that it has work to do before we * get here. We thus keep trying to find the new device we just @@ -462,8 +466,8 @@ nodeDeviceGetTime(time_t *t) */ static virNodeDevicePtr nodeDeviceFindNewDevice(virConnectPtr conn, - const char *wwnn, - const char *wwpn) + nodeDeviceFindNewDeviceFunc func, + const void *opaque) { virNodeDevicePtr device = NULL; time_t start = 0, now = 0; @@ -474,7 +478,7 @@ nodeDeviceFindNewDevice(virConnectPtr conn, virWaitForDevices(); - device = nodeDeviceLookupSCSIHostByWWN(conn, wwnn, wwpn, 0); + device = func(conn, opaque); if (device != NULL) break; @@ -488,6 +492,35 @@ nodeDeviceFindNewDevice(virConnectPtr conn, } +typedef struct _NewSCSIHostFuncData NewSCSIHostFuncData; +struct _NewSCSIHostFuncData +{ + const char *wwnn; + const char *wwpn; +}; + + +static virNodeDevicePtr +nodeDeviceFindNewSCSIHostFunc(virConnectPtr conn, + const void *opaque) +{ + const NewSCSIHostFuncData *data = opaque; + + return nodeDeviceLookupSCSIHostByWWN(conn, data->wwnn, data->wwpn, 0); +} + + +static virNodeDevicePtr +nodeDeviceFindNewSCSIHost(virConnectPtr conn, + const char *wwnn, + const char *wwpn) +{ + NewSCSIHostFuncData data = { .wwnn = wwnn, .wwpn = wwpn}; + + return nodeDeviceFindNewDevice(conn, nodeDeviceFindNewSCSIHostFunc, &data); +} + + static bool nodeDeviceHasCapability(virNodeDeviceDefPtr def, virNodeDevCapType type) { @@ -538,7 +571,7 @@ nodeDeviceCreateXML(virConnectPtr conn, if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) return NULL; - device = nodeDeviceFindNewDevice(conn, wwnn, wwpn); + device = nodeDeviceFindNewSCSIHost(conn, wwnn, wwpn); /* We don't check the return value, because one way or another, * we're returning what we get... */ -- 2.21.3

Jonathon Jongsma

4:05 p.m.

New subject: [libvirt PATCH v4 05/12] nodedev: store mdev UUID in mdev caps

In order to allow libvirt to create and start new mediated devices, we need to be able to verify that the device has been started. In order to do this, we'll need to save the UUID of newly-discovered devices within the virNodeDevCapMdev structure. This allows us to search the device list by UUID and verify whether the expected device has been started. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/conf/node_device_conf.c | 1 + src/conf/node_device_conf.h | 1 + src/node_device/node_device_udev.c | 5 ++--- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 623a2cb79c..78a537d0ea 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2218,6 +2218,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) break; case VIR_NODE_DEV_CAP_MDEV: VIR_FREE(data->mdev.type); + VIR_FREE(data->mdev.uuid); for (i = 0; i < data->mdev.nattributes; i++) virMediatedDeviceAttrFree(data->mdev.attributes[i]); VIR_FREE(data->mdev.attributes); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index e3e1e788d4..9b8c7aadea 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -141,6 +141,7 @@ typedef virNodeDevCapMdev *virNodeDevCapMdevPtr; struct _virNodeDevCapMdev { char *type; unsigned int iommuGroupNumber; + char *uuid; virMediatedDeviceAttrPtr *attributes; size_t nattributes; }; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 386f23ef3a..bdf0b03add 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1013,7 +1013,6 @@ udevProcessMediatedDevice(struct udev_device *dev, virNodeDeviceDefPtr def) { int ret = -1; - const char *uuidstr = NULL; int iommugrp = -1; char *linkpath = NULL; char *canonicalpath = NULL; @@ -1041,8 +1040,8 @@ udevProcessMediatedDevice(struct udev_device *dev, data->type = g_path_get_basename(canonicalpath); - uuidstr = udev_device_get_sysname(dev); - if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(uuidstr)) < 0) + data->uuid = g_strdup(udev_device_get_sysname(dev)); + if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(data->uuid)) < 0) goto cleanup; if (udevGenerateDeviceName(dev, def, NULL) != 0) -- 2.21.3

Jonathon Jongsma

4:05 p.m.

New subject: [libvirt PATCH v4 06/12] nodedev: add mdev support to virNodeDeviceCreateXML()

With recent additions to the node device xml schema, an xml schema can now describe a mdev device sufficiently for libvirt to create and start the device using the mdevctl utility. Note that some of the the configuration for a mediated device must be passed to mdevctl as a JSON-formatted file. In order to avoid creating and cleaning up temporary files, the JSON is instead fed to stdin and we pass the filename /dev/stdin to mdevctl. While this may not be portable, neither are mediated devices, so I don't believe it should cause any problems. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- libvirt.spec.in | 2 + m4/virt-external-programs.m4 | 3 + src/conf/virnodedeviceobj.c | 34 +++++ src/conf/virnodedeviceobj.h | 3 + src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 197 +++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 5 + 7 files changed, 245 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 450c97b46d..cd7c33ab1a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -522,6 +522,8 @@ Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} # needed for device enumeration Requires: systemd >= 185 +# For managing persistent mediated devices +Requires: mdevctl %description daemon-driver-nodedev The nodedev driver plugin for the libvirtd daemon, providing diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4 index 9046e3bf07..bd3cb1f757 100644 --- a/m4/virt-external-programs.m4 +++ b/m4/virt-external-programs.m4 @@ -65,6 +65,7 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [ AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [$LIBVIRT_SBIN_PATH]) AC_PATH_PROG([SCRUB], [scrub], [scrub], [$LIBVIRT_SBIN_PATH]) AC_PATH_PROG([ADDR2LINE], [addr2line], [addr2line], [$LIBVIRT_SBIN_PATH]) + AC_PATH_PROG([MDEVCTL], [mdevctl], [mdevctl], [$LIBVIRT_SBIN_PATH]) AC_DEFINE_UNQUOTED([DMIDECODE], ["$DMIDECODE"], [Location or name of the dmidecode program]) @@ -88,6 +89,8 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [ [Location or name of the scrub program (for wiping algorithms)]) AC_DEFINE_UNQUOTED([ADDR2LINE], ["$ADDR2LINE"], [Location of addr2line program]) + AC_DEFINE_UNQUOTED([MDEVCTL], ["$MDEVCTL"], + [Location or name of the mdevctl program]) AC_PATH_PROG([IP_PATH], [ip], [/sbin/ip], [$LIBVIRT_SBIN_PATH]) AC_DEFINE_UNQUOTED([IP_PATH], ["$IP_PATH"], [path to ip binary]) diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 3a34a324ca..fd20d5f9e2 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -399,6 +399,40 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs, &data); } +static int +virNodeDeviceObjListFindMediatedDeviceByUUIDCallback(const void *payload, + const void *name G_GNUC_UNUSED, + const void *opaque G_GNUC_UNUSED) +{ + virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; + const char *uuid = (const char *) opaque; + virNodeDevCapsDefPtr cap; + int want = 0; + + virObjectLock(obj); + + for (cap = obj->def->caps; cap != NULL; cap = cap->next) { + if (cap->data.type == VIR_NODE_DEV_CAP_MDEV) { + if (STREQ(cap->data.mdev.uuid, uuid)) { + want = 1; + break; + } + } + } + + virObjectUnlock(obj); + return want; +} + + +virNodeDeviceObjPtr +virNodeDeviceObjListFindMediatedDeviceByUUID(virNodeDeviceObjListPtr devs, + const char *uuid) +{ + return virNodeDeviceObjListSearch(devs, + virNodeDeviceObjListFindMediatedDeviceByUUIDCallback, + uuid); +} static void virNodeDeviceObjListDispose(void *obj) diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h index c9df8dedab..6efdb23d36 100644 --- a/src/conf/virnodedeviceobj.h +++ b/src/conf/virnodedeviceobj.h @@ -118,3 +118,6 @@ virNodeDeviceObjListExport(virConnectPtr conn, void virNodeDeviceObjSetSkipUpdateCaps(virNodeDeviceObjPtr obj, bool skipUpdateCaps); +virNodeDeviceObjPtr +virNodeDeviceObjListFindMediatedDeviceByUUID(virNodeDeviceObjListPtr devs, + const char *uuid); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 22bd8b9c17..42f8d7c222 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1179,6 +1179,7 @@ virNodeDeviceObjListAssignDef; virNodeDeviceObjListExport; virNodeDeviceObjListFindByName; virNodeDeviceObjListFindBySysfsPath; +virNodeDeviceObjListFindMediatedDeviceByUUID; virNodeDeviceObjListFindSCSIHostByWWNs; virNodeDeviceObjListFree; virNodeDeviceObjListGetNames; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index d6255a43c8..35016782d2 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -30,6 +30,7 @@ #include "datatypes.h" #include "viralloc.h" #include "virfile.h" +#include "virjson.h" #include "virstring.h" #include "node_device_conf.h" #include "node_device_event.h" @@ -40,6 +41,7 @@ #include "viraccessapicheck.h" #include "virnetdev.h" #include "virutil.h" +#include "vircommand.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -304,6 +306,30 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, return device; } +static virNodeDevicePtr +nodeDeviceLookupMediatedDeviceByUUID(virConnectPtr conn, + const char *uuid, + unsigned int flags) +{ + virNodeDeviceObjPtr obj = NULL; + virNodeDeviceDefPtr def; + virNodeDevicePtr device = NULL; + + virCheckFlags(0, NULL); + + if (!(obj = virNodeDeviceObjListFindMediatedDeviceByUUID(driver->devs, + uuid))) + return NULL; + + def = virNodeDeviceObjGetDef(obj); + + if ((device = virGetNodeDevice(conn, def->name))) + device->parentName = g_strdup(def->parent); + + virNodeDeviceObjEndAPI(&obj); + return device; +} + char * nodeDeviceGetXMLDesc(virNodeDevicePtr device, @@ -492,6 +518,26 @@ nodeDeviceFindNewDevice(virConnectPtr conn, } +static virNodeDevicePtr +nodeDeviceFindNewMediatedDeviceFunc(virConnectPtr conn, + const void *opaque) +{ + const char *uuid = opaque; + + return nodeDeviceLookupMediatedDeviceByUUID(conn, uuid, 0); +} + + +static virNodeDevicePtr +nodeDeviceFindNewMediatedDevice(virConnectPtr conn, + const char *mdev_uuid) +{ + return nodeDeviceFindNewDevice(conn, + nodeDeviceFindNewMediatedDeviceFunc, + mdev_uuid); +} + + typedef struct _NewSCSIHostFuncData NewSCSIHostFuncData; struct _NewSCSIHostFuncData { @@ -536,6 +582,155 @@ nodeDeviceHasCapability(virNodeDeviceDefPtr def, virNodeDevCapType type) } +/* format a json string that provides configuration information about this mdev + * to the mdevctl utility */ +static int +nodeDeviceDefToMdevctlConfig(virNodeDeviceDefPtr def, char **buf) +{ + size_t i; + virNodeDevCapMdevPtr mdev = &def->caps->data.mdev; + g_autoptr(virJSONValue) json = virJSONValueNewObject(); + + if (virJSONValueObjectAppendString(json, "mdev_type", mdev->type) < 0) + return -1; + + if (virJSONValueObjectAppendString(json, "start", "manual") < 0) + return -1; + + if (mdev->attributes) { + g_autoptr(virJSONValue) attributes = virJSONValueNewArray(); + + for (i = 0; i < mdev->nattributes; i++) { + virMediatedDeviceAttrPtr attr = mdev->attributes[i]; + g_autoptr(virJSONValue) jsonattr = virJSONValueNewObject(); + + if (virJSONValueObjectAppendString(jsonattr, attr->name, attr->value) < 0) + return -1; + + if (virJSONValueArrayAppend(attributes, g_steal_pointer(&jsonattr)) < 0) + return -1; + } + + if (virJSONValueObjectAppend(json, "attrs", g_steal_pointer(&attributes)) < 0) + return -1; + } + + *buf = virJSONValueToString(json, false); + if (!*buf) + return -1; + + return 0; +} + + +static char * +nodeDeviceFindAddressByName(const char *name) +{ + virNodeDeviceDefPtr def = NULL; + virNodeDevCapsDefPtr caps = NULL; + char *pci_addr = NULL; + virNodeDeviceObjPtr dev = virNodeDeviceObjListFindByName(driver->devs, name); + + if (!dev) { + virReportError(VIR_ERR_NO_NODE_DEVICE, + _("could not find device '%s'"), name); + return NULL; + } + + def = virNodeDeviceObjGetDef(dev); + for (caps = def->caps; caps != NULL; caps = caps->next) { + if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { + virPCIDeviceAddress addr = { + .domain = caps->data.pci_dev.domain, + .bus = caps->data.pci_dev.bus, + .slot = caps->data.pci_dev.slot, + .function = caps->data.pci_dev.function + }; + + pci_addr = virPCIDeviceAddressAsString(&addr); + break; + } + } + + virNodeDeviceObjEndAPI(&dev); + + return pci_addr; +} + + +virCommandPtr +nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def, + char **uuid_out) +{ + virCommandPtr cmd; + g_autofree char *json = NULL; + g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent); + + if (!parent_pci) { + virReportError(VIR_ERR_NO_NODE_DEVICE, + _("unable to find PCI address for parent device '%s'"), def->parent); + return NULL; + } + + if (nodeDeviceDefToMdevctlConfig(def, &json) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("couldn't convert node device def to mdevctl JSON")); + return NULL; + } + + cmd = virCommandNewArgList(MDEVCTL, "start", + "-p", parent_pci, + "--jsonfile", "/dev/stdin", + NULL); + + virCommandSetInputBuffer(cmd, json); + virCommandSetOutputBuffer(cmd, uuid_out); + + return cmd; +} + +static int +virMdevctlStart(virNodeDeviceDefPtr def, char **uuid) +{ + int status; + g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlStartCommand(def, uuid); + if (!cmd) + return -1; + + /* an auto-generated uuid is returned via stdout if no uuid is specified in + * the mdevctl args */ + if (virCommandRun(cmd, &status) < 0 || status != 0) + return -1; + + /* remove newline */ + *uuid = g_strstrip(*uuid); + + return 0; +} + + +static virNodeDevicePtr +nodeDeviceCreateXMLMdev(virConnectPtr conn, + virNodeDeviceDefPtr def) +{ + g_autofree char *uuid = NULL; + + if (!def->parent) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("cannot create a mediated device without a parent")); + return NULL; + } + + if (virMdevctlStart(def, &uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to start mediated device")); + return NULL; + } + + return nodeDeviceFindNewMediatedDevice(conn, uuid); +} + + virNodeDevicePtr nodeDeviceCreateXML(virConnectPtr conn, const char *xmlDesc, @@ -580,6 +775,8 @@ nodeDeviceCreateXML(virConnectPtr conn, _("no node device for '%s' with matching " "wwnn '%s' and wwpn '%s'"), def->name, wwnn, wwpn); + } else if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) { + device = nodeDeviceCreateXMLMdev(conn, def); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unsupported device type")); diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index eae5e2cb17..e42c14f6c7 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -24,6 +24,7 @@ #include "internal.h" #include "driver.h" #include "virnodedeviceobj.h" +#include "vircommand.h" #define LINUX_NEW_DEVICE_WAIT_TIME 60 @@ -116,3 +117,7 @@ nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, int nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, int callbackID); + +virCommandPtr +nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def, + char **uuid_out); -- 2.21.3

Erik Skultety

Friday, 19 June Fri, 19 Jun

2:21 a.m.

New subject: [libvirt PATCH v4 06/12] nodedev: add mdev support to virNodeDeviceCreateXML()

On Thu, Jun 18, 2020 at 04:05:59PM -0500, Jonathon Jongsma wrote:

...

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 3a34a324ca..fd20d5f9e2 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -399,6 +399,40 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs, &data); } +static int +virNodeDeviceObjListFindMediatedDeviceByUUIDCallback(const void *payload, + const void *name G_GNUC_UNUSED, + const void *opaque G_GNUC_UNUSED)

@opaque is used I'll fix this before merging. Erik

Jonathon Jongsma

Thursday, 18 June Thu, 18 Jun

4:06 p.m.

New subject: [libvirt PATCH v4 07/12] nodedev: Build a non-loadable driver lib

In order to test the nodedev driver, we need to link against a non-loadable module. Similar to other loadable modules already in the repository, create an _impl library that can be linked against the unit tests and then create a loadable module from that. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/node_device/Makefile.inc.am | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/node_device/Makefile.inc.am b/src/node_device/Makefile.inc.am index 788563665f..5993165b56 100644 --- a/src/node_device/Makefile.inc.am +++ b/src/node_device/Makefile.inc.am @@ -34,34 +34,37 @@ EXTRA_DIST += \ if WITH_NODE_DEVICES # Needed to keep automake quiet about conditionals +noinst_LTLIBRARIES += libvirt_driver_nodedev_impl.la +libvirt_driver_nodedev_la_SOURCES = +libvirt_driver_nodedev_la_LIBADD = libvirt_driver_nodedev_impl.la mod_LTLIBRARIES += libvirt_driver_nodedev.la -libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) +libvirt_driver_nodedev_impl_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) -libvirt_driver_nodedev_la_CFLAGS = \ +libvirt_driver_nodedev_impl_la_CFLAGS = \ -I$(srcdir)/access \ -I$(builddir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) \ $(LIBNL_CFLAGS) \ $(NULL) -libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) -libvirt_driver_nodedev_la_LIBADD = \ +libvirt_driver_nodedev_impl_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) +libvirt_driver_nodedev_impl_la_LIBADD = \ libvirt.la \ $(GLIB_LIBS) \ $(NULL) if WITH_HAL -libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) -libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS) -libvirt_driver_nodedev_la_LIBADD += $(HAL_LIBS) +libvirt_driver_nodedev_impl_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) +libvirt_driver_nodedev_impl_la_CFLAGS += $(HAL_CFLAGS) +libvirt_driver_nodedev_impl_la_LIBADD += $(HAL_LIBS) endif WITH_HAL if WITH_UDEV -libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_UDEV_SOURCES) -libvirt_driver_nodedev_la_CFLAGS += \ +libvirt_driver_nodedev_impl_la_SOURCES += $(NODE_DEVICE_DRIVER_UDEV_SOURCES) +libvirt_driver_nodedev_impl_la_CFLAGS += \ $(UDEV_CFLAGS) \ $(PCIACCESS_CFLAGS) \ $(NULL) -libvirt_driver_nodedev_la_LIBADD += \ +libvirt_driver_nodedev_impl_la_LIBADD += \ $(UDEV_LIBS) \ $(PCIACCESS_LIBS) \ $(NULL) -- 2.21.3

Jonathon Jongsma

4:06 p.m.

New subject: [libvirt PATCH v4 08/12] nodedev: Add testing for 'mdevctl start'

Test that we run 'mdevctl' with the proper arguments when creating new mediated devices with virNodeDeviceCreateXML(). Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- build-aux/syntax-check.mk | 2 +- tests/Makefile.am | 15 + ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 + ...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.json | 1 + tests/nodedevmdevctltest.c | 262 ++++++++++++++++++ ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 7 + ...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 9 + ...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 8 + 12 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json create mode 100644 tests/nodedevmdevctltest.c create mode 100644 tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml create mode 100644 tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml create mode 100644 tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index bf8832a2a5..d47a92b530 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -2015,7 +2015,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ - (^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) + (^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$) diff --git a/tests/Makefile.am b/tests/Makefile.am index f5766a7790..3505c40f42 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -103,6 +103,7 @@ EXTRA_DIST = \ networkxml2xmlupdatein \ networkxml2xmlupdateout \ nodedevschemadata \ + nodedevmdevctldata \ virhostcpudata \ nssdata \ nwfilterxml2firewalldata \ @@ -388,6 +389,10 @@ test_programs += storagevolxml2xmltest test_programs += nodedevxml2xmltest +if WITH_NODE_DEVICES +test_programs += nodedevmdevctltest +endif WITH_NODE_DEVICES + test_programs += interfacexml2xmltest test_programs += cputest @@ -970,6 +975,16 @@ nodedevxml2xmltest_SOURCES = \ testutils.c testutils.h nodedevxml2xmltest_LDADD = $(LDADDS) +if WITH_NODE_DEVICES +nodedevmdevctltest_SOURCES = \ + nodedevmdevctltest.c \ + testutils.c testutils.h + +nodedevmdevctltest_LDADD = \ + ../src/libvirt_driver_nodedev_impl.la \ + $(LDADDS) +endif WITH_NODE_DEVICES + interfacexml2xmltest_SOURCES = \ interfacexml2xmltest.c \ testutils.c testutils.h diff --git a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv new file mode 100644 index 0000000000..eb7262035e --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv @@ -0,0 +1 @@ +$MDEVCTL_BINARY$ start -p 0000:00:02.0 --jsonfile /dev/stdin diff --git a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json new file mode 100644 index 0000000000..bfc6dcace3 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json @@ -0,0 +1 @@ +{"mdev_type":"i915-GVTg_V5_8","start":"manual"} \ No newline at end of file diff --git a/tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv b/tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv new file mode 100644 index 0000000000..eb7262035e --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv @@ -0,0 +1 @@ +$MDEVCTL_BINARY$ start -p 0000:00:02.0 --jsonfile /dev/stdin diff --git a/tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json b/tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json new file mode 100644 index 0000000000..e5b22b2c44 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json @@ -0,0 +1 @@ +{"mdev_type":"i915-GVTg_V5_8","start":"manual","attrs":[{"example-attribute-1":"attribute-value-1"},{"example-attribute-2":"attribute-value-2"}]} \ No newline at end of file diff --git a/tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv b/tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv new file mode 100644 index 0000000000..eb7262035e --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv @@ -0,0 +1 @@ +$MDEVCTL_BINARY$ start -p 0000:00:02.0 --jsonfile /dev/stdin diff --git a/tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json b/tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json new file mode 100644 index 0000000000..2e03d0bd8e --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json @@ -0,0 +1 @@ +{"mdev_type":"i915-GVTg_V5_8","start":"manual","attrs":[{"example-attribute":"attribute-value"}]} \ No newline at end of file diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c new file mode 100644 index 0000000000..4b029c7286 --- /dev/null +++ b/tests/nodedevmdevctltest.c @@ -0,0 +1,262 @@ +#include <config.h> + +#include "internal.h" +#include "testutils.h" +#include "datatypes.h" +#include "node_device/node_device_driver.h" +#include "vircommand.h" +#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW +#include "vircommandpriv.h" + +#define VIR_FROM_THIS VIR_FROM_NODEDEV + +struct startTestInfo { + const char *virt_type; + int create; + const char *filename; +}; + +/* capture stdin passed to command */ +static void +testCommandDryRunCallback(const char *const*args G_GNUC_UNUSED, + const char *const*env G_GNUC_UNUSED, + const char *input, + char **output G_GNUC_UNUSED, + char **error G_GNUC_UNUSED, + int *status G_GNUC_UNUSED, + void *opaque G_GNUC_UNUSED) +{ + char **stdinbuf = opaque; + + *stdinbuf = g_strdup(input); +} + +/* We don't want the result of the test to depend on the path to the mdevctl + * binary on the developer's machine, so replace the path to mdevctl with a + * placeholder string before comparing to the expected output */ +static int +nodedevCompareToFile(const char *actual, + const char *filename) +{ + g_autofree char *replacedCmdline = NULL; + + replacedCmdline = virStringReplace(actual, MDEVCTL, "$MDEVCTL_BINARY$"); + + return virTestCompareToFile(replacedCmdline, filename); +} + +static int +testMdevctlStart(const char *virt_type, + int create, + const char *mdevxml, + const char *startcmdfile, + const char *startjsonfile) +{ + g_autoptr(virNodeDeviceDef) def = NULL; + virNodeDeviceObjPtr obj = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *actualCmdline = NULL; + int ret = -1; + g_autofree char *uuid = NULL; + g_autofree char *stdinbuf = NULL; + g_autoptr(virCommand) cmd = NULL; + + if (!(def = virNodeDeviceDefParseFile(mdevxml, create, virt_type))) + goto cleanup; + + /* this function will set a stdin buffer containing the json configuration + * of the device. The json value is captured in the callback above */ + cmd = nodeDeviceGetMdevctlStartCommand(def, &uuid); + + if (!cmd) + goto cleanup; + + virCommandSetDryRun(&buf, testCommandDryRunCallback, &stdinbuf); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (!(actualCmdline = virBufferCurrentContent(&buf))) + goto cleanup; + + if (nodedevCompareToFile(actualCmdline, startcmdfile) < 0) + goto cleanup; + + if (virTestCompareToFile(stdinbuf, startjsonfile) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBufferFreeAndReset(&buf); + virCommandSetDryRun(NULL, NULL, NULL); + virNodeDeviceObjEndAPI(&obj); + return ret; +} + +static int +testMdevctlStartHelper(const void *data) +{ + const struct startTestInfo *info = data; + + g_autofree char *mdevxml = g_strdup_printf("%s/nodedevschemadata/%s.xml", + abs_srcdir, info->filename); + g_autofree char *cmdlinefile = g_strdup_printf("%s/nodedevmdevctldata/%s-start.argv", + abs_srcdir, info->filename); + g_autofree char *jsonfile = g_strdup_printf("%s/nodedevmdevctldata/%s-start.json", + abs_srcdir, info->filename); + + return testMdevctlStart(info->virt_type, + info->create, mdevxml, cmdlinefile, + jsonfile); +} + +static void +nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv) +{ + if (!drv) + return; + + virNodeDeviceObjListFree(drv->devs); + virCondDestroy(&drv->initCond); + virMutexDestroy(&drv->lock); + VIR_FREE(drv->stateDir); + VIR_FREE(drv); +} + +/* Add a fake root 'computer' device */ +static virNodeDeviceDefPtr +fakeRootDevice(void) +{ + virNodeDeviceDefPtr def = NULL; + + if (VIR_ALLOC(def) != 0 || VIR_ALLOC(def->caps) != 0) { + virNodeDeviceDefFree(def); + return NULL; + } + + def->name = g_strdup("computer"); + + return def; +} + +/* Add a fake pci device that can be used as a parent device for mediated + * devices. For our purposes, it only needs to have a name that matches the + * parent of the mdev, and it needs a PCI address + */ +static virNodeDeviceDefPtr +fakeParentDevice(void) +{ + virNodeDeviceDefPtr def = NULL; + virNodeDevCapPCIDevPtr pci_dev; + + if (VIR_ALLOC(def) != 0 || VIR_ALLOC(def->caps) != 0) { + virNodeDeviceDefFree(def); + return NULL; + } + + def->name = g_strdup("pci_0000_00_02_0"); + def->parent = g_strdup("computer"); + + def->caps->data.type = VIR_NODE_DEV_CAP_PCI_DEV; + pci_dev = &def->caps->data.pci_dev; + pci_dev->domain = 0; + pci_dev->bus = 0; + pci_dev->slot = 2; + pci_dev->function = 0; + + return def; +} + +static int +addDevice(virNodeDeviceDefPtr def) +{ + if (!def) + return -1; + + virNodeDeviceObjPtr obj = virNodeDeviceObjListAssignDef(driver->devs, def); + + if (!obj) { + virNodeDeviceDefFree(def); + return -1; + } + + virNodeDeviceObjEndAPI(&obj); + return 0; +} + +static int +nodedevTestDriverAddTestDevices(void) +{ + if (addDevice(fakeRootDevice()) < 0 || + addDevice(fakeParentDevice()) < 0) + return -1; + + return 0; +} + +/* Bare minimum driver init to be able to test nodedev functionality */ +static int +nodedevTestDriverInit(void) +{ + int ret = -1; + if (VIR_ALLOC(driver) < 0) + return -1; + + driver->lockFD = -1; + if (virMutexInit(&driver->lock) < 0 || + virCondInit(&driver->initCond) < 0) { + VIR_TEST_DEBUG("Unable to initialize test nodedev driver"); + goto error; + } + + if (!(driver->devs = virNodeDeviceObjListNew())) + goto error; + + return 0; + + error: + nodedevTestDriverFree(driver); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + if (nodedevTestDriverInit() < 0) + return EXIT_FAILURE; + + /* add a mock device to the device list so it can be used as a parent + * reference */ + if (nodedevTestDriverAddTestDevices() < 0) { + ret = EXIT_FAILURE; + goto done; + } + +#define DO_TEST_FULL(desc, func, info) \ + if (virTestRun(desc, func, &info) < 0) \ + ret = -1; + +#define DO_TEST_START_FULL(virt_type, create, filename) \ + do { \ + struct startTestInfo info = { virt_type, create, filename }; \ + DO_TEST_FULL("mdevctl start " filename, testMdevctlStartHelper, info); \ + } \ + while (0); + +#define DO_TEST_START(filename) \ + DO_TEST_START_FULL("QEMU", CREATE_DEVICE, filename) + + /* Test mdevctl start commands */ + DO_TEST_START("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); + DO_TEST_START("mdev_fedc4916_1ca8_49ac_b176_871d16c13076"); + DO_TEST_START("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9"); + + done: + nodedevTestDriverFree(driver); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) diff --git a/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml b/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml new file mode 100644 index 0000000000..d6a2e99edc --- /dev/null +++ b/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml @@ -0,0 +1,7 @@ +<device> + <name>mdev_d069d019_36ea_4111_8f0a_8c9a70e21366</name> + <parent>pci_0000_00_02_0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_8'/> + </capability> +</device> diff --git a/tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml b/tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml new file mode 100644 index 0000000000..89568d06ce --- /dev/null +++ b/tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml @@ -0,0 +1,9 @@ +<device> + <name>mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9</name> + <parent>pci_0000_00_02_0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_8'/> + <attr name='example-attribute-1' value='attribute-value-1'/> + <attr name='example-attribute-2' value='attribute-value-2'/> + </capability> +</device> diff --git a/tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml b/tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml new file mode 100644 index 0000000000..7cd0a46e3d --- /dev/null +++ b/tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml @@ -0,0 +1,8 @@ +<device> + <name>mdev_fedc4916_1ca8_49ac_b176_871d16c13076</name> + <parent>pci_0000_00_02_0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_8'/> + <attr name='example-attribute' value='attribute-value'/> + </capability> +</device> -- 2.21.3

Jonathon Jongsma

4:06 p.m.

New subject: [libvirt PATCH v4 09/12] nodedev: add mdev support to virNodeDeviceDestroy()

Add the ability to destroy mdev node devices via the mdevctl utility. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/node_device/node_device_driver.c | 33 ++++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 2 ++ 2 files changed, 35 insertions(+) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 35016782d2..e89c8b0ee5 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -786,6 +786,32 @@ nodeDeviceCreateXML(virConnectPtr conn, } +virCommandPtr +nodeDeviceGetMdevctlStopCommand(const char *uuid) +{ + return virCommandNewArgList(MDEVCTL, + "stop", + "-u", + uuid, + NULL); + +} + +static int +virMdevctlStop(virNodeDeviceDefPtr def) +{ + int status; + g_autoptr(virCommand) cmd = NULL; + + cmd = nodeDeviceGetMdevctlStopCommand(def->caps->data.mdev.uuid); + + if (virCommandRun(cmd, &status) < 0 || status != 0) + return -1; + + return 0; +} + + int nodeDeviceDestroy(virNodeDevicePtr device) { @@ -832,6 +858,13 @@ nodeDeviceDestroy(virNodeDevicePtr device) if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) goto cleanup; + ret = 0; + } else if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) { + if (virMdevctlStop(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to stop mediated device")); + goto cleanup; + } ret = 0; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index e42c14f6c7..be5d397828 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -121,3 +121,5 @@ nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, virCommandPtr nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def, char **uuid_out); +virCommandPtr +nodeDeviceGetMdevctlStopCommand(const char *uuid); -- 2.21.3

Jonathon Jongsma

4:06 p.m.

New subject: [libvirt PATCH v4 10/12] nodedev: Add testing for 'mdevctl stop'

Test that we run 'mdevctl' with the proper arguments when we destroy mediated devices with virNodeDeviceDestroy() Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- tests/nodedevmdevctldata/mdevctl-stop.argv | 1 + tests/nodedevmdevctltest.c | 41 ++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/nodedevmdevctldata/mdevctl-stop.argv diff --git a/tests/nodedevmdevctldata/mdevctl-stop.argv b/tests/nodedevmdevctldata/mdevctl-stop.argv new file mode 100644 index 0000000000..3dbaab671b --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-stop.argv @@ -0,0 +1 @@ +$MDEVCTL_BINARY$ stop -u e2451f73-c95b-4124-b900-e008af37c576 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 4b029c7286..f5bcf5227d 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -110,6 +110,41 @@ testMdevctlStartHelper(const void *data) jsonfile); } +static int +testMdevctlStop(const void *data) +{ + const char *uuid = data; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *actualCmdline = NULL; + int ret = -1; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *cmdlinefile = + g_strdup_printf("%s/nodedevmdevctldata/mdevctl-stop.argv", + abs_srcdir); + + cmd = nodeDeviceGetMdevctlStopCommand(uuid); + + if (!cmd) + goto cleanup; + + virCommandSetDryRun(&buf, NULL, NULL); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (!(actualCmdline = virBufferCurrentContent(&buf))) + goto cleanup; + + if (nodedevCompareToFile(actualCmdline, cmdlinefile) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBufferFreeAndReset(&buf); + virCommandSetDryRun(NULL, NULL, NULL); + return ret; +} + static void nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv) { @@ -248,11 +283,17 @@ mymain(void) #define DO_TEST_START(filename) \ DO_TEST_START_FULL("QEMU", CREATE_DEVICE, filename) +#define DO_TEST_STOP(uuid) \ + DO_TEST_FULL("mdevctl stop " uuid, testMdevctlStop, uuid) + /* Test mdevctl start commands */ DO_TEST_START("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); DO_TEST_START("mdev_fedc4916_1ca8_49ac_b176_871d16c13076"); DO_TEST_START("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9"); + /* Test mdevctl stop command, pass an arbitrary uuid */ + DO_TEST_STOP("e2451f73-c95b-4124-b900-e008af37c576"); + done: nodedevTestDriverFree(driver); -- 2.21.3

Jonathon Jongsma

4:06 p.m.

New subject: [libvirt PATCH v4 11/12] docs: note node device fields that are read-only

As noted by Erik Skultety, we use the same XML schema to report existing devices and to define new devices. However, some schema elements are "read-only". In other words, they are used to report information from the node device driver and cannot be used to define a new device. Note these in the documentation. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- docs/formatnode.html.in | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 0637d457ee..e4328fedbe 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -31,11 +31,16 @@ name is just the bus type and address, as in "pci_0000_00_02_1" or "usb_1_5_3", but some devices are able to provide more specific names, such as - "net_eth1_00_27_13_6a_fe_00". + "net_eth1_00_27_13_6a_fe_00". This is a read-only field that is + reported by the device driver. If this element is set when defining a + new device, it will be ignored. + </dd> <dt><code>path</code></dt> <dd> - Fully qualified sysfs path to the device. + Fully qualified sysfs path to the device. This is a read-only field + that is reported by the device driver. If this element is set when + defining a new device, it will be ignored. </dd> <dt><code>parent</code></dt> <dd> -- 2.21.3

Jonathon Jongsma

4:06 p.m.

New subject: [libvirt PATCH v4 12/12] news: mediated devices can be created

Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- NEWS.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 20964b94d7..42d159b233 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,13 @@ v6.5.0 (unreleased) It's possible to either specify new value as a string or provide a filename which contents then serve as the value. + * nodedev: Add ability to create mediated devices + + Mediated devices can now be created with ``virNodeDeviceCreateXML()``. This + functionality requires the ``mdevctl`` utility to be installed. The XML + schema for node devices was expanded to support attributes for mediated + devices. + * **Improvements** * **Bug fixes** -- 2.21.3

Erik Skultety

Friday, 19 June Fri, 19 Jun

2:22 a.m.

On Thu, Jun 18, 2020 at 04:05:53PM -0500, Jonathon Jongsma wrote:

...

I'll give Michal a chance to have a final look before I push this. Good work. Series: Reviewed-by: Erik Skultety <eskultet(a)redhat.com> Regards, Erik

Michal Privoznik

3:35 a.m.

On 6/19/20 9:22 AM, Erik Skultety wrote:

...

On Thu, Jun 18, 2020 at 04:05:53PM -0500, Jonathon Jongsma wrote: > This is the first portion of an effort to support persistent mediated devices > with libvirt. This first series simply enables creating and destroying > non-persistent mediated devices via the virNodeDeviceCreateXML() and > virNodeDeviceDestroy() functions. The 'mdevctl' utility[1] provides the backend > implementation. > > Hopefully these are the last changes and version can simply be pushed upstream. > > Changes in v4: > - coding style / spacing fixes > - remove 'persist' arg from start command > - fixed distcheck failure by including test data dir in EXTRA_DIST > - Add an item to NEWS.rst > > [1] https://github.com/mdevctl/mdevctl > I'll give Michal a chance to have a final look before I push this. Good work. Series: Reviewed-by: Erik Skultety <eskultet(a)redhat.com>

Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com> Michal

Daniel P. Berrangé

6:06 a.m.

On Fri, Jun 19, 2020 at 10:35:12AM +0200, Michal Privoznik wrote:

...

On 6/19/20 9:22 AM, Erik Skultety wrote: > On Thu, Jun 18, 2020 at 04:05:53PM -0500, Jonathon Jongsma wrote: > > This is the first portion of an effort to support persistent mediated devices > > with libvirt. This first series simply enables creating and destroying > > non-persistent mediated devices via the virNodeDeviceCreateXML() and > > virNodeDeviceDestroy() functions. The 'mdevctl' utility[1] provides the backend > > implementation. > > > > Hopefully these are the last changes and version can simply be pushed upstream. > > > > Changes in v4: > > - coding style / spacing fixes > > - remove 'persist' arg from start command > > - fixed distcheck failure by including test data dir in EXTRA_DIST > > - Add an item to NEWS.rst > > > > [1] https://github.com/mdevctl/mdevctl > > > > I'll give Michal a chance to have a final look before I push this. > Good work. > > Series: > Reviewed-by: Erik Skultety <eskultet(a)redhat.com> > Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>

This appears to have broken the build because the nodedev driver so file has gained a bogus version number Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Jonathon Jongsma

9:12 a.m.

On Fri, 2020-06-19 at 12:06 +0100, Daniel P. Berrangé wrote:

...

On Fri, Jun 19, 2020 at 10:35:12AM +0200, Michal Privoznik wrote: > On 6/19/20 9:22 AM, Erik Skultety wrote: > > On Thu, Jun 18, 2020 at 04:05:53PM -0500, Jonathon Jongsma wrote: > > > This is the first portion of an effort to support persistent > > > mediated devices > > > with libvirt. This first series simply enables creating and > > > destroying > > > non-persistent mediated devices via the > > > virNodeDeviceCreateXML() and > > > virNodeDeviceDestroy() functions. The 'mdevctl' utility[1] > > > provides the backend > > > implementation. > > > > > > Hopefully these are the last changes and version can simply be > > > pushed upstream. > > > > > > Changes in v4: > > > - coding style / spacing fixes > > > - remove 'persist' arg from start command > > > - fixed distcheck failure by including test data dir in > > > EXTRA_DIST > > > - Add an item to NEWS.rst > > > > > > [1] https://github.com/mdevctl/mdevctl > > > > > > > I'll give Michal a chance to have a final look before I push > > this. > > Good work. > > > > Series: > > Reviewed-by: Erik Skultety <eskultet(a)redhat.com> > > > > Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com> This appears to have broken the build because the nodedev driver so file has gained a bogus version number Regards, Daniel

Can you provide more information here? I have not seen any build failures. Where are you seeing it? Local builds? CI? Jonathon

Michal Privoznik

10:14 a.m.

On 6/19/20 4:12 PM, Jonathon Jongsma wrote:

...

On Fri, 2020-06-19 at 12:06 +0100, Daniel P. Berrangé wrote: > On Fri, Jun 19, 2020 at 10:35:12AM +0200, Michal Privoznik wrote: >> On 6/19/20 9:22 AM, Erik Skultety wrote: >>> On Thu, Jun 18, 2020 at 04:05:53PM -0500, Jonathon Jongsma wrote: >>>> This is the first portion of an effort to support persistent >>>> mediated devices >>>> with libvirt. This first series simply enables creating and >>>> destroying >>>> non-persistent mediated devices via the >>>> virNodeDeviceCreateXML() and >>>> virNodeDeviceDestroy() functions. The 'mdevctl' utility[1] >>>> provides the backend >>>> implementation. >>>> >>>> Hopefully these are the last changes and version can simply be >>>> pushed upstream. >>>> >>>> Changes in v4: >>>> - coding style / spacing fixes >>>> - remove 'persist' arg from start command >>>> - fixed distcheck failure by including test data dir in >>>> EXTRA_DIST >>>> - Add an item to NEWS.rst >>>> >>>> [1] https://github.com/mdevctl/mdevctl >>>> >>> >>> I'll give Michal a chance to have a final look before I push >>> this. >>> Good work. >>> >>> Series: >>> Reviewed-by: Erik Skultety <eskultet(a)redhat.com> >>> >> >> Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com> > > This appears to have broken the build because the nodedev driver so > file > has gained a bogus version number > > Regards, > Daniel Can you provide more information here? I have not seen any build failures. Where are you seeing it? Local builds? CI? Jonathon

It's fixed here: https://www.redhat.com/archives/libvir-list/2020-June/msg00886.html Michal

1648

days inactive

1649

days old

devel@lists.libvirt.org

Manage subscription

18 comments

4 participants

tags (0)

participants (4)

Daniel P. Berrangé
Erik Skultety
Jonathon Jongsma
Michal Privoznik

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt PATCH v4 00/12] Add ability to create mediated devices in libvirt