On Tue, Jun 09, 2020 at 04:43:47PM -0500, Jonathon Jongsma wrote:
> Test that we run 'mdevctl' with the proper arguments when creating new
> mediated devices with virNodeDeviceCreateXML().
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
...
> +
> + // this function will set a stdin buffer containing the json configuration
> + // of the device. This json value is captured in the mock wrapper above
nitpick: /* ... */ instead of //
> + 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 (virTestCompareToFile(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 testInfo *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->shouldFail, info->virt_type,
> + info->create, mdevxml, cmdlinefile,
> + jsonfile);
> +}
> +
> +static void
> +nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv)
> +{
> + if (!drv)
> + return;
> +
> + 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;
> + char statedir[] = abs_builddir "/nodedevstatedir-XXXXXX";
> + if (VIR_ALLOC(driver) < 0)
> + return -1;
> +
> + if (!g_mkdtemp(statedir)) {
> + fprintf(stderr, "Cannot create fake stateDir");
> + goto error;
> + }
> +
> + driver->stateDir = g_strdup(statedir);
> + 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;
> +
> + if (nodedevTestDriverAddTestDevices() < 0) {
> + ret = EXIT_FAILURE;
> + goto done;
> + }
> +
> + // add a mock device to the device list so it can be used as a parent
> + // reference
..same ^here...
> +
> +#define DO_TEST_FULL(shouldFail, virt_type, create, filename) \
> + do { \
> + struct testInfo info = { shouldFail, virt_type, create, filename, }; \
> + if (virTestRun("mdevctl start " filename, \
> + testMdevctlStartHelper, &info) < 0) \
> + ret = -1; \
> + } \
> + while (0);
> +
> +#define DO_TEST(...) \
> + DO_TEST_FULL(false, "QEMU", CREATE_DEVICE, __VA_ARGS__)
> +#define DO_TEST_FAIL(...) \
> + DO_TEST_FULL(true, "QEMU", CREATE_DEVICE, __VA_ARGS__)
> +
> + // Test mdevctl start commands
> + DO_TEST("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
> + DO_TEST("mdev_fedc4916_1ca8_49ac_b176_871d16c13076");
> + DO_TEST("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..36d50630e4
> --- /dev/null
> +++ b/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml
> @@ -0,0 +1,8 @@
> +<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'/>
> + <iommuGroup number='0'/>
Like I said in the response to one of the earlier patches, I'd like to see
iommuGroup gone from the create XML.
Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
