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@redhat.com> --- docs/formatnode.html.in | 5 +++-- docs/schemas/nodedev.rng | 12 +++++++----- src/conf/node_device_conf.c | 12 ++++++------ 3 files changed, 16 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..4b3b04b7b8 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1775,13 +1775,13 @@ 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

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@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 b1dfe81653..06e5924bef 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2214,6 +2214,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

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@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 | 199 +++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 6 + 7 files changed, 248 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 262e66f3cc..2b3a4d2e71 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 97d790aced..9092d2b7ae 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1178,6 +1178,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 629d4bcf91..dd00b4ddf7 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,25 @@ 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 { @@ -534,6 +579,158 @@ 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, + bool persist, + char **uuid_out) +{ + virCommandPtr cmd; + const char *subcommand; + 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; + } + + if (persist) + subcommand = "define"; + else + subcommand = "start"; + + cmd = virCommandNewArgList(MDEVCTL, subcommand, + "-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, false, + 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, @@ -578,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..576f75375f 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,8 @@ nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, int nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, int callbackID); + +virCommandPtr +nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def, + bool persist, + char **uuid_out); -- 2.21.3

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@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

Test that we run 'mdevctl' with the proper arguments when creating new mediated devices with virNodeDeviceCreateXML(). Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- build-aux/syntax-check.mk | 2 +- tests/Makefile.am | 14 + ...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, 307 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..13cbdbb31e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -388,6 +388,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 +974,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..8d226e012b --- /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, false, &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