The macro should not have a trailing semicolon so that when the macro is used, the user can add a semicolon themselves. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- tests/nodedevmdevctltest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 9780553a3a..d75dfb1d3e 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -276,7 +276,7 @@ mymain(void) struct startTestInfo info = { virt_type, create, filename }; \ DO_TEST_FULL("mdevctl start " filename, testMdevctlStartHelper, info); \ } \ - while (0); + while (0) #define DO_TEST_START(filename) \ DO_TEST_START_FULL("QEMU", CREATE_DEVICE, filename) -- 2.21.3

Add two flag values for virConnectListAllNodeDevices() so that we can list only node devices that are active or inactive. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- include/libvirt/libvirt-nodedev.h | 9 ++++++--- src/conf/node_device_conf.h | 8 ++++++++ src/conf/virnodedeviceobj.c | 10 ++++++++++ src/libvirt-nodedev.c | 2 ++ src/node_device/node_device_driver.c | 2 +- 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index a2ad61ac6d..98a972e60d 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -61,10 +61,9 @@ int virNodeListDevices (virConnectPtr conn, * virConnectListAllNodeDevices: * * Flags used to filter the returned node devices. Flags in each group - * are exclusive. Currently only one group to filter the devices by cap - * type. - */ + * are exclusive. */ typedef enum { + /* filter the devices by cap type */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM = 1 << 0, /* System capability */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV = 1 << 1, /* PCI device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV = 1 << 2, /* USB device */ @@ -81,6 +80,10 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES = 1 << 13, /* Capable of mediated devices */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV = 1 << 14, /* Mediated device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV = 1 << 15, /* CCW device */ + + /* filter the devices by active state */ + VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE = 1 << 16, /* Inactive devices */ + VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE = 1 << 17, /* Active devices */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 9b8c7aadea..353dbebaf0 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -369,6 +369,14 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps); VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV) +#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE \ + VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE |\ + VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE + +#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ALL \ + VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP | \ + VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE + int virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host); diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 88a7746b27..9838513b69 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -865,6 +865,16 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj, return false; } +#undef MATCH +#define MATCH(FLAG) (flags & (FLAG)) + + if (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE) && + !((MATCH(VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE) && + virNodeDeviceObjIsActive(obj)) || + (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE) && + !virNodeDeviceObjIsActive(obj)))) + return false; + return true; } #undef MATCH diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index ca6c2bb057..6dc61304a0 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -101,6 +101,8 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags) * VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES * VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV + * VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE + * VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE * * Returns the number of node devices found or -1 and sets @devices to NULL in * case of error. On success, the array stored into @devices is guaranteed to diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index e89c8b0ee5..f766fd9f32 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -218,7 +218,7 @@ nodeConnectListAllNodeDevices(virConnectPtr conn, virNodeDevicePtr **devices, unsigned int flags) { - virCheckFlags(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP, -1); + virCheckFlags(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ALL, -1); if (virConnectListAllNodeDevicesEnsureACL(conn) < 0) return -1; -- 2.21.3

This adds some internal API to query for persistent mediated devices that are defined by mdevctl. Following commits will make use of this information. This just provides the infrastructure and tests for this feature. One test verifies that we are executing mdevctl with the proper arguments, and other tests verify that we can parse the returned JSON and convert it to our own internal node device representations. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/node_device/node_device_driver.c | 154 ++++++++++++++++++ src/node_device/node_device_driver.h | 13 ++ src/node_device/node_device_udev.c | 19 +-- .../mdevctl-list-defined.argv | 1 + .../mdevctl-list-multiple-parents.json | 59 +++++++ .../mdevctl-list-multiple-parents.out.xml | 39 +++++ .../mdevctl-list-multiple.json | 59 +++++++ .../mdevctl-list-multiple.out.xml | 39 +++++ .../mdevctl-list-single-noattr.json | 11 ++ .../mdevctl-list-single-noattr.out.xml | 8 + .../mdevctl-list-single.json | 31 ++++ .../mdevctl-list-single.out.xml | 14 ++ tests/nodedevmdevctltest.c | 97 +++++++++++ 13 files changed, 528 insertions(+), 16 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple-parents.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple-parents.out.xml create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create mode 100644 tests/nodedevmdevctldata/mdevctl-list-single-noattr.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-single-noattr.out.xml create mode 100644 tests/nodedevmdevctldata/mdevctl-list-single.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-single.out.xml diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index f766fd9f32..cb8965a6d9 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -812,6 +812,135 @@ virMdevctlStop(virNodeDeviceDefPtr def) } +virCommandPtr +nodeDeviceGetMdevctlListCommand(bool defined, + char **output) +{ + virCommandPtr cmd = virCommandNewArgList(MDEVCTL, + "list", + "--dumpjson", + NULL); + + if (defined) + virCommandAddArg(cmd, "--defined"); + + virCommandSetOutputBuffer(cmd, output); + + return cmd; +} + + +static void mdevGenerateDeviceName(virNodeDeviceDefPtr dev) +{ + nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, NULL); +} + +int +nodeDeviceParseMdevctlJSON(const char *jsonstring, + virNodeDeviceDefPtr **devs) +{ + int n; + g_autoptr(virJSONValue) json_devicelist = NULL; + virNodeDeviceDefPtr *outdevs = NULL; + size_t noutdevs = 0; + + json_devicelist = virJSONValueFromString(jsonstring); + + if (!json_devicelist) + goto parsefailure; + + if (!virJSONValueIsArray(json_devicelist)) + goto parsefailure; + + n = virJSONValueArraySize(json_devicelist); + + for (int i = 0; i < n; i++) { + virJSONValuePtr obj = virJSONValueArrayGet(json_devicelist, i); + int nparents; + + if (!virJSONValueIsObject(obj)) + goto parsefailure; + + nparents = virJSONValueObjectKeysNumber(obj); + + for (int j = 0; j < nparents; j++) { + const char *parent = virJSONValueObjectGetKey(obj, j); + virJSONValuePtr child_array = virJSONValueObjectGetValue(obj, j); + int nchildren; + + if (!virJSONValueIsArray(child_array)) + goto parsefailure; + + nchildren = virJSONValueArraySize(child_array); + + for (int k = 0; k < nchildren; k++) { + virNodeDevCapMdevPtr mdev; + const char *uuid; + virJSONValuePtr props; + g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1); + virJSONValuePtr child_obj = virJSONValueArrayGet(child_array, k); + + /* the child object should have a single key equal to its uuid. + * The value is an object describing the properties of the mdev */ + if (virJSONValueObjectKeysNumber(child_obj) != 1) + goto parsefailure; + + uuid = virJSONValueObjectGetKey(child_obj, 0); + props = virJSONValueObjectGetValue(child_obj, 0); + + child->parent = g_strdup(parent); + child->caps = g_new0(virNodeDevCapsDef, 1); + child->caps->data.type = VIR_NODE_DEV_CAP_MDEV; + + mdev = &child->caps->data.mdev; + mdev->uuid = g_strdup(uuid); + mdev->type = + g_strdup(virJSONValueObjectGetString(props, "mdev_type")); + + virJSONValuePtr attrs = virJSONValueObjectGet(props, "attrs"); + + if (attrs && virJSONValueIsArray(attrs)) { + int nattrs = virJSONValueArraySize(attrs); + + mdev->attributes = g_new0(virMediatedDeviceAttrPtr, nattrs); + mdev->nattributes = nattrs; + + for (int m = 0; m < nattrs; m++) { + virJSONValuePtr attr = virJSONValueArrayGet(attrs, m); + virMediatedDeviceAttrPtr attribute; + + if (!virJSONValueIsObject(attr) || + virJSONValueObjectKeysNumber(attr) != 1) + goto parsefailure; + + attribute = g_new0(virMediatedDeviceAttr, 1); + attribute->name = g_strdup(virJSONValueObjectGetKey(attr, 0)); + virJSONValuePtr value = virJSONValueObjectGetValue(attr, 0); + attribute->value = g_strdup(virJSONValueGetString(value)); + mdev->attributes[m] = attribute; + } + } + mdevGenerateDeviceName(child); + + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) + child = NULL; + } + } + } + + *devs = outdevs; + return noutdevs; + + parsefailure: + for (int i = 0; i < noutdevs; i++) + virNodeDeviceDefFree(outdevs[i]); + VIR_FREE(outdevs); + + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "unable to parse JSON response"); + return -1; +} + + int nodeDeviceDestroy(virNodeDevicePtr device) { @@ -931,3 +1060,28 @@ nodedevRegister(void) # endif #endif } + + +void +nodeDeviceGenerateName(virNodeDeviceDefPtr def, + const char *subsystem, + const char *sysname, + const char *s) +{ + size_t i; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&buf, "%s_%s", + subsystem, + sysname); + + if (s != NULL) + virBufferAsprintf(&buf, "_%s", s); + + def->name = virBufferContentAndReset(&buf); + + for (i = 0; i < strlen(def->name); i++) { + if (!(g_ascii_isalnum(*(def->name + i)))) + *(def->name + i) = '_'; + } +} diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index be5d397828..e7317fd67a 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -123,3 +123,16 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def, char **uuid_out); virCommandPtr nodeDeviceGetMdevctlStopCommand(const char *uuid); + +virCommandPtr +nodeDeviceGetMdevctlListCommand(bool defined, char **output); + +int +nodeDeviceParseMdevctlJSON(const char *jsonstring, + virNodeDeviceDefPtr **devs); + +void +nodeDeviceGenerateName(virNodeDeviceDefPtr def, + const char *subsystem, + const char *sysname, + const char *s); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index bce5466131..93b74b1f24 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -293,22 +293,9 @@ udevGenerateDeviceName(struct udev_device *device, virNodeDeviceDefPtr def, const char *s) { - size_t i; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - - virBufferAsprintf(&buf, "%s_%s", - udev_device_get_subsystem(device), - udev_device_get_sysname(device)); - - if (s != NULL) - virBufferAsprintf(&buf, "_%s", s); - - def->name = virBufferContentAndReset(&buf); - - for (i = 0; i < strlen(def->name); i++) { - if (!(g_ascii_isalnum(*(def->name + i)))) - *(def->name + i) = '_'; - } + nodeDeviceGenerateName(def, + udev_device_get_subsystem(device), + udev_device_get_sysname(device), s); return 0; } diff --git a/tests/nodedevmdevctldata/mdevctl-list-defined.argv b/tests/nodedevmdevctldata/mdevctl-list-defined.argv new file mode 100644 index 0000000000..72b5906e9e --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-defined.argv @@ -0,0 +1 @@ +$MDEVCTL_BINARY$ list --dumpjson --defined diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple-parents.json b/tests/nodedevmdevctldata/mdevctl-list-multiple-parents.json new file mode 100644 index 0000000000..eefcd90c62 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple-parents.json @@ -0,0 +1,59 @@ +[ + { + "0000:00:02.0": [ + { + "200f228a-c80a-4d50-bfb7-f5a0e4e34045": { + "mdev_type": "i915-GVTg_V5_4", + "start": "manual" + } + }, + { + "de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": { + "mdev_type": "i915-GVTg_V5_4", + "start": "auto" + } + }, + { + "435722ea-5f43-468a-874f-da34f1217f13": { + "mdev_type": "i915-GVTg_V5_8", + "start": "manual", + "attrs": [ + { + "testattr": "42" + } + ] + } + } + ] + }, + { + "matrix": [ + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": { + "mdev_type": "vfio_ap-passthrough", + "start": "manual", + "attrs": [ + { + "assign_adapter": "5" + }, + { + "assign_adapter": "6" + }, + { + "assign_domain": "0xab" + }, + { + "assign_control_domain": "0xab" + }, + { + "assign_domain": "4" + }, + { + "assign_control_domain": "4" + } + ] + } + } + ] + } +] + diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple-parents.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple-parents.out.xml new file mode 100644 index 0000000000..543ad916b7 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple-parents.out.xml @@ -0,0 +1,39 @@ +<device> + <name>mdev_200f228a_c80a_4d50_bfb7_f5a0e4e34045</name> + <parent>0000:00:02.0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_4'/> + <iommuGroup number='0'/> + </capability> +</device> +<device> + <name>mdev_de807ffc_1923_4d5f_b6c9_b20ecebc6d4b</name> + <parent>0000:00:02.0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_4'/> + <iommuGroup number='0'/> + </capability> +</device> +<device> + <name>mdev_435722ea_5f43_468a_874f_da34f1217f13</name> + <parent>0000:00:02.0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_8'/> + <iommuGroup number='0'/> + <attr name='testattr' value='42'/> + </capability> +</device> +<device> + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name> + <parent>matrix</parent> + <capability type='mdev'> + <type id='vfio_ap-passthrough'/> + <iommuGroup number='0'/> + <attr name='assign_adapter' value='5'/> + <attr name='assign_adapter' value='6'/> + <attr name='assign_domain' value='0xab'/> + <attr name='assign_control_domain' value='0xab'/> + <attr name='assign_domain' value='4'/> + <attr name='assign_control_domain' value='4'/> + </capability> +</device> diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json new file mode 100644 index 0000000000..eefcd90c62 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json @@ -0,0 +1,59 @@ +[ + { + "0000:00:02.0": [ + { + "200f228a-c80a-4d50-bfb7-f5a0e4e34045": { + "mdev_type": "i915-GVTg_V5_4", + "start": "manual" + } + }, + { + "de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": { + "mdev_type": "i915-GVTg_V5_4", + "start": "auto" + } + }, + { + "435722ea-5f43-468a-874f-da34f1217f13": { + "mdev_type": "i915-GVTg_V5_8", + "start": "manual", + "attrs": [ + { + "testattr": "42" + } + ] + } + } + ] + }, + { + "matrix": [ + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": { + "mdev_type": "vfio_ap-passthrough", + "start": "manual", + "attrs": [ + { + "assign_adapter": "5" + }, + { + "assign_adapter": "6" + }, + { + "assign_domain": "0xab" + }, + { + "assign_control_domain": "0xab" + }, + { + "assign_domain": "4" + }, + { + "assign_control_domain": "4" + } + ] + } + } + ] + } +] + diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml new file mode 100644 index 0000000000..543ad916b7 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml @@ -0,0 +1,39 @@ +<device> + <name>mdev_200f228a_c80a_4d50_bfb7_f5a0e4e34045</name> + <parent>0000:00:02.0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_4'/> + <iommuGroup number='0'/> + </capability> +</device> +<device> + <name>mdev_de807ffc_1923_4d5f_b6c9_b20ecebc6d4b</name> + <parent>0000:00:02.0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_4'/> + <iommuGroup number='0'/> + </capability> +</device> +<device> + <name>mdev_435722ea_5f43_468a_874f_da34f1217f13</name> + <parent>0000:00:02.0</parent> + <capability type='mdev'> + <type id='i915-GVTg_V5_8'/> + <iommuGroup number='0'/> + <attr name='testattr' value='42'/> + </capability> +</device> +<device> + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name> + <parent>matrix</parent> + <capability type='mdev'> + <type id='vfio_ap-passthrough'/> + <iommuGroup number='0'/> + <attr name='assign_adapter' value='5'/> + <attr name='assign_adapter' value='6'/> + <attr name='assign_domain' value='0xab'/> + <attr name='assign_control_domain' value='0xab'/> + <attr name='assign_domain' value='4'/> + <attr name='assign_control_domain' value='4'/> + </capability> +</device> diff --git a/tests/nodedevmdevctldata/mdevctl-list-single-noattr.json b/tests/nodedevmdevctldata/mdevctl-list-single-noattr.json new file mode 100644 index 0000000000..acee3eb628 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-single-noattr.json @@ -0,0 +1,11 @@ +[ + { + "matrix": [ + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": { + "mdev_type": "vfio_ap-passthrough", + "start": "manual" + } + } + ] + } +] diff --git a/tests/nodedevmdevctldata/mdevctl-list-single-noattr.out.xml b/tests/nodedevmdevctldata/mdevctl-list-single-noattr.out.xml new file mode 100644 index 0000000000..85958695da --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-single-noattr.out.xml @@ -0,0 +1,8 @@ +<device> + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name> + <parent>matrix</parent> + <capability type='mdev'> + <type id='vfio_ap-passthrough'/> + <iommuGroup number='0'/> + </capability> +</device> diff --git a/tests/nodedevmdevctldata/mdevctl-list-single.json b/tests/nodedevmdevctldata/mdevctl-list-single.json new file mode 100644 index 0000000000..a1843c2c8e --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-single.json @@ -0,0 +1,31 @@ +[ + { + "matrix": [ + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": { + "mdev_type": "vfio_ap-passthrough", + "start": "manual", + "attrs": [ + { + "assign_adapter": "5" + }, + { + "assign_adapter": "6" + }, + { + "assign_domain": "0xab" + }, + { + "assign_control_domain": "0xab" + }, + { + "assign_domain": "4" + }, + { + "assign_control_domain": "4" + } + ] + } + } + ] + } +] diff --git a/tests/nodedevmdevctldata/mdevctl-list-single.out.xml b/tests/nodedevmdevctldata/mdevctl-list-single.out.xml new file mode 100644 index 0000000000..cb443346ef --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-single.out.xml @@ -0,0 +1,14 @@ +<device> + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name> + <parent>matrix</parent> + <capability type='mdev'> + <type id='vfio_ap-passthrough'/> + <iommuGroup number='0'/> + <attr name='assign_adapter' value='5'/> + <attr name='assign_adapter' value='6'/> + <attr name='assign_domain' value='0xab'/> + <attr name='assign_control_domain' value='0xab'/> + <attr name='assign_domain' value='4'/> + <attr name='assign_control_domain' value='4'/> + </capability> +</device> diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index d75dfb1d3e..585d23d72e 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -143,6 +143,87 @@ testMdevctlStop(const void *data) return ret; } +static int +testMdevctlListDefined(const void *data G_GNUC_UNUSED) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *actualCmdline = NULL; + int ret = -1; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *output = NULL; + g_autofree char *cmdlinefile = + g_strdup_printf("%s/nodedevmdevctldata/mdevctl-list-defined.argv", + abs_srcdir); + + cmd = nodeDeviceGetMdevctlListCommand(true, &output); + + 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 int +testMdevctlParse(const void *data) +{ + g_autofree char *buf = NULL; + const char *filename = data; + g_autofree char *jsonfile = g_strdup_printf("%s/nodedevmdevctldata/%s.json", + abs_srcdir, filename); + g_autofree char *xmloutfile = g_strdup_printf("%s/nodedevmdevctldata/%s.out.xml", + abs_srcdir, filename); + g_autofree char *actualxml = NULL; + int ret = -1; + int nmdevs = 0; + virNodeDeviceDefPtr *mdevs = NULL; + virBuffer xmloutbuf = VIR_BUFFER_INITIALIZER; + + if (virFileReadAll(jsonfile, 1024*1024, &buf) < 0) { + VIR_TEST_DEBUG("Unable to read file %s", jsonfile); + return -1; + } + + if ((nmdevs = nodeDeviceParseMdevctlJSON(buf, &mdevs)) < 0) { + VIR_TEST_DEBUG("Unable to parse json for %s", filename); + return -1; + } + + for (int i = 0; i < nmdevs; i++) { + g_autofree char *devxml = virNodeDeviceDefFormat(mdevs[i]); + if (!devxml) + goto cleanup; + virBufferAddStr(&xmloutbuf, devxml); + } + + if (nodedevCompareToFile(virBufferCurrentContent(&xmloutbuf), xmloutfile) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBufferFreeAndReset(&xmloutbuf); + for (int i = 0; i < nmdevs; i++) + virNodeDeviceDefFree(mdevs[i]); + g_free(mdevs); + + return ret; +} + static void nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv) { @@ -284,6 +365,15 @@ mymain(void) #define DO_TEST_STOP(uuid) \ DO_TEST_FULL("mdevctl stop " uuid, testMdevctlStop, uuid) +#define DO_TEST_LIST_DEFINED() \ + do { \ + if (virTestRun("mdevctl list --defined", testMdevctlListDefined, NULL) < 0) \ + ret = -1; \ + } while (0) + +#define DO_TEST_PARSE_JSON(filename) \ + DO_TEST_FULL("parse mdevctl json " filename, testMdevctlParse, filename) + /* Test mdevctl start commands */ DO_TEST_START("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); DO_TEST_START("mdev_fedc4916_1ca8_49ac_b176_871d16c13076"); @@ -292,6 +382,13 @@ mymain(void) /* Test mdevctl stop command, pass an arbitrary uuid */ DO_TEST_STOP("e2451f73-c95b-4124-b900-e008af37c576"); + DO_TEST_LIST_DEFINED(); + + DO_TEST_PARSE_JSON("mdevctl-list-single"); + DO_TEST_PARSE_JSON("mdevctl-list-single-noattr"); + DO_TEST_PARSE_JSON("mdevctl-list-multiple"); + DO_TEST_PARSE_JSON("mdevctl-list-multiple-parents"); + done: nodedevTestDriverFree(driver); -- 2.21.3

At startup, query devices that are defined by 'mdevctl' and add them to the node device list. This adds a complication: we now have two potential sources of information for a node device: - udev for all devices and for activated mediated devices - mdevctl for persistent mediated devices Unfortunately, neither backend returns full information for a mediated device. For example, if a persistent mediated device in the list (with information provided from mdevctl) is 'started', that same device will now be detected by udev. If we simply overwrite the existing device definition with the new one provided by the udev backend, we will lose extra information that was provided by mdevctl (e.g. attributes, etc). To avoid this, make sure to copy the extra information into the new device definition. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/node_device/node_device_driver.c | 75 ++++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 3 ++ src/node_device/node_device_udev.c | 43 ++++++++++++++++ 3 files changed, 121 insertions(+) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index c5092ff657..9d07ff0e38 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1086,3 +1086,78 @@ nodeDeviceGenerateName(virNodeDeviceDefPtr def, *(def->name + i) = '_'; } } + + +static int +virMdevctlListDefined(virNodeDeviceDefPtr **devs) +{ + int status; + g_autofree char *output = NULL; + g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(true, &output); + + if (virCommandRun(cmd, &status) < 0) + return -1; + + if (!output) + return -1; + + return nodeDeviceParseMdevctlJSON(output, devs); +} + + +int +mdevctlEnumerateDevices(void) +{ + g_autofree virNodeDeviceDefPtr *devs = NULL; + int ndevs; + + if ((ndevs = virMdevctlListDefined(&devs)) < 0) { + virReportSystemError(errno, "%s", + _("failed to query mdevs from mdevctl")); + return -1; + } + + for (int i = 0; i < ndevs; i++) { + virNodeDeviceObjPtr obj; + virObjectEventPtr event; + virNodeDeviceDefPtr dev = devs[i]; + + dev->driver = g_strdup("vfio_mdev"); + + /* If a device defined by mdevctl is already in the list, that means + * that it was found via the normal device discovery process and thus + * is already activated. Active devices contain some additional + * information (e.g. sysfs path) that is not provided by mdevctl, so + * preserve that info */ + bool new_device = true; + if ((obj = virNodeDeviceObjListFindByName(driver->devs, dev->name))) { + virNodeDeviceDefPtr olddef = virNodeDeviceObjGetDef(obj); + + /* Copy any data from the existing device */ + dev->sysfs_path = g_strdup(olddef->sysfs_path); + dev->parent_sysfs_path = g_strdup(olddef->parent_sysfs_path); + dev->driver = g_strdup(olddef->driver); + dev->devnode = g_strdup(olddef->devnode); + dev->caps->data.mdev.iommuGroupNumber = olddef->caps->data.mdev.iommuGroupNumber; + + virNodeDeviceObjEndAPI(&obj); + new_device = false; + } + + if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, dev))) { + virNodeDeviceDefFree(dev); + return -1; + } + + if (new_device) + event = virNodeDeviceEventLifecycleNew(dev->name, + VIR_NODE_DEVICE_EVENT_CREATED, + 0); + else + event = virNodeDeviceEventUpdateNew(dev->name); + virNodeDeviceObjEndAPI(&obj); + virObjectEventStateQueue(driver->nodeDeviceEventState, event); + } + + return 0; +} diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index e7317fd67a..b5c1da4ff3 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -131,6 +131,9 @@ int nodeDeviceParseMdevctlJSON(const char *jsonstring, virNodeDeviceDefPtr **devs); +int +mdevctlEnumerateDevices(void); + void nodeDeviceGenerateName(virNodeDeviceDefPtr def, const char *subsystem, diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index ac7986bc70..e06584a3dc 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1306,6 +1306,45 @@ udevSetParent(struct udev_device *device, return 0; } +static virMediatedDeviceAttrPtr * +virMediatedDeviceAttrsCopy(virMediatedDeviceAttrPtr *attrs, + size_t nattrs) +{ + size_t i; + size_t j = 0; + g_autofree virMediatedDeviceAttrPtr *ret = NULL; + + if (nattrs == 0) + return NULL; + + if (VIR_ALLOC_N(ret, nattrs) < 0) + return NULL; + + for (i = 0; i < nattrs; i++) { + virMediatedDeviceAttrPtr attr = virMediatedDeviceAttrNew(); + attr->name = g_strdup(attrs[i]->name); + attr->value = g_strdup(attrs[i]->value); + VIR_APPEND_ELEMENT_INPLACE(ret, j, attr); + } + + return g_steal_pointer(ret); +} + +/* An existing device definition may have additional info from mdevctl that is + * not available from udev. Transfer this data to the new definition */ +static void +nodeDeviceDefCopyExtraData(virNodeDeviceDefPtr dst, + virNodeDeviceDefPtr src) +{ + virNodeDevCapMdevPtr srcmdev = &src->caps->data.mdev; + virNodeDevCapMdevPtr dstmdev = &dst->caps->data.mdev; + + dstmdev->persistent = srcmdev->persistent; + dstmdev->nattributes = srcmdev->nattributes; + dstmdev->attributes = virMediatedDeviceAttrsCopy(srcmdev->attributes, + srcmdev->nattributes); +} + static int udevAddOneDevice(struct udev_device *device) @@ -1341,6 +1380,8 @@ udevAddOneDevice(struct udev_device *device) goto cleanup; if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) { + nodeDeviceDefCopyExtraData(def, virNodeDeviceObjGetDef(obj)); + virNodeDeviceObjEndAPI(&obj); new_device = false; } @@ -1773,6 +1814,8 @@ nodeStateInitializeEnumerate(void *opaque) /* Populate with known devices */ if (udevEnumerateDevices(udev) != 0) goto error; + if (mdevctlEnumerateDevices() != 0) + goto error; nodeDeviceLock(); driver->initialized = true; -- 2.21.3

We need to peridocally query mdevctl for changes to device definitions since an administrator can define new devices with mdevctl outside of libvirt. In order to do this, a new thread is created to handle the querying. In the future, mdevctl may add a way to signal device add / remove via events, but for now we resort to a bit of a workaround: monitoring the mdevctl config directories for changes to files. When a change is detected, we query mdevctl and update our device list. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/node_device/node_device_udev.c | 182 +++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index e06584a3dc..02017b5325 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -41,6 +41,7 @@ #include "virnetdev.h" #include "virmdev.h" #include "virutil.h" +#include <gio/gio.h> #include "configmake.h" @@ -66,6 +67,11 @@ struct _udevEventData { virCond threadCond; bool threadQuit; bool dataReady; + + virThread mdevctlThread; + virCond mdevctlCond; + bool mdevctlReady; + GList *mdevctl_monitors; }; static virClassPtr udevEventDataClass; @@ -85,8 +91,10 @@ udevEventDataDispose(void *obj) udev = udev_monitor_get_udev(priv->udev_monitor); udev_monitor_unref(priv->udev_monitor); udev_unref(udev); + g_list_free_full(priv->mdevctl_monitors, g_object_unref); virCondDestroy(&priv->threadCond); + virCondDestroy(&priv->mdevctlCond); } @@ -117,6 +125,11 @@ udevEventDataNew(void) return NULL; } + if (virCondInit(&ret->mdevctlCond) < 0) { + virObjectUnref(ret); + return NULL; + } + ret->watch = -1; return ret; } @@ -1520,6 +1533,7 @@ nodeStateCleanup(void) virObjectLock(priv); priv->threadQuit = true; virCondSignal(&priv->threadCond); + virCondSignal(&priv->mdevctlCond); virObjectUnlock(priv); virThreadJoin(&priv->th); } @@ -1601,6 +1615,40 @@ udevEventMonitorSanityCheck(udevEventDataPtr priv, } +/* Thread to query mdevctl for the current state of persistent mediated device + * defintions when any changes are detected */ +static +void mdevctlThread(void *opaque G_GNUC_UNUSED) +{ + udevEventDataPtr priv = driver->privateData; + + while (1) { + virObjectLock(priv); + while (!priv->mdevctlReady && !priv->threadQuit) { + if (virCondWait(&priv->mdevctlCond, &priv->parent.lock)) { + virReportSystemError(errno, "%s", + _("handler failed to wait on condition")); + virObjectUnlock(priv); + return; + } + } + + if (priv->threadQuit) { + virObjectUnlock(priv); + return; + } + + virObjectUnlock(priv); + + mdevctlEnumerateDevices(); + + virObjectLock(priv); + priv->mdevctlReady = false; + virObjectUnlock(priv); + } +} + + /** * udevEventHandleThread * @opaque: unused @@ -1708,6 +1756,69 @@ udevEventHandleCallback(int watch G_GNUC_UNUSED, } +static int timeout_id = 0; + +static gboolean +signalMdevctlThread(void *user_data) +{ + udevEventDataPtr priv = user_data; + + if (timeout_id) { + g_source_remove(timeout_id); + timeout_id = 0; + } + + virObjectLock(priv); + priv->mdevctlReady = true; + virCondSignal(&priv->mdevctlCond); + virObjectUnlock(priv); + + return G_SOURCE_REMOVE; +} + + +static void +mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED, + GFile *file, + GFile *other_file G_GNUC_UNUSED, + GFileMonitorEvent event_type, + gpointer user_data) +{ + udevEventDataPtr priv = user_data; + + /* if a new directory appears, monitor that directory for changes */ + if (g_file_query_file_type(file, G_FILE_QUERY_INFO_NONE, NULL) == + G_FILE_TYPE_DIRECTORY) { + GFileMonitor *mon; + + if ((mon = g_file_monitor(file, G_FILE_MONITOR_NONE, NULL, NULL))) { + g_signal_connect(mon, "changed", + G_CALLBACK(mdevctlEventHandleCallback), + user_data); + virObjectLock(priv); + priv->mdevctl_monitors = g_list_append(priv->mdevctl_monitors, mon); + virObjectUnlock(priv); + } + } + + /* Sometimes a single configuration change results in multiple notify + * events (e.g. several CHANGED events, or a CREATED and CHANGED event + * followed by CHANGES_DONE_HINT). To avoid triggering the mdevctl thread + * multiple times for a single configuration change, try to coalesce these + * changes by waiting for the CHANGES_DONE_HINT event. As a fallback, add + * a timeout to trigger the signal if that event never comes */ + if (event_type == G_FILE_MONITOR_EVENT_CREATED || + event_type == G_FILE_MONITOR_EVENT_CHANGED) { + if (timeout_id) + g_source_remove(timeout_id); + timeout_id = g_timeout_add(100, signalMdevctlThread, priv); + return; + } + + signalMdevctlThread(priv); +} + + /* DMI is intel-compatible specific */ #if defined(__x86_64__) || defined(__i386__) || defined(__amd64__) static void @@ -1858,6 +1969,58 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED) } +/* Recursively monitors dir and any subdirectory for file changes and returns a + * GList of GFileMonitor objects */ +static GList* +monitorDirRecursively(GFile *dir, + GCallback changed_cb, + gpointer user_data) +{ + GList *monitors = NULL; + g_autoptr(GFileInfo) dirinfo = NULL; + g_autoptr(GError) error = NULL; + g_autoptr(GFileEnumerator) children = NULL; + GFileMonitor *mon; + + if (!(children = g_file_enumerate_children(dir, "standard::*", + G_FILE_QUERY_INFO_NONE, NULL, &error))) { + if (error->code == G_IO_ERROR_NOT_DIRECTORY) + return NULL; + goto bail; + } + + if (!(mon = g_file_monitor(dir, G_FILE_MONITOR_NONE, NULL, &error))) + goto bail; + + g_signal_connect(mon, "changed", changed_cb, user_data); + monitors = g_list_append(monitors, mon); + + while (true) { + GFileInfo *info; + GFile *child; + GList *child_monitors = NULL; + + if (!g_file_enumerator_iterate(children, &info, &child, NULL, &error)) + goto bail; + if (!info) + break; + + child_monitors = monitorDirRecursively(child, changed_cb, user_data); + if (child_monitors) + monitors = g_list_concat(monitors, child_monitors); + } + + return monitors; + + bail: + if (error) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to monitor directory: %s"), error->message); + + g_list_free_full(monitors, g_object_unref); + return NULL; +} + static int nodeStateInitialize(bool privileged, const char *root, @@ -1867,6 +2030,8 @@ nodeStateInitialize(bool privileged, udevEventDataPtr priv = NULL; struct udev *udev = NULL; virThread enumThread; + g_autoptr(GError) error = NULL; + g_autoptr(GFile) mdevctlConfigDir = g_file_new_for_path("/etc/mdevctl.d"); if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1969,6 +2134,23 @@ nodeStateInitialize(bool privileged, if (priv->watch == -1) goto unlock; + if (virThreadCreateFull(&priv->mdevctlThread, true, mdevctlThread, + "mdevctl-event", false, NULL) < 0) { + virReportSystemError(errno, "%s", + _("failed to create mdevctl handler thread")); + goto unlock; + } + + /* mdevctl may add notification events in the future: + * https://github.com/mdevctl/mdevctl/issues/27. For now, fall back to + * monitoring the mdevctl configuration directory for changes. + * mdevctl configuration is stored in a directory tree within + * /etc/mdevctl.d/. There is a directory for each parent device, which + * contains a file defining each mediated device */ + priv->mdevctl_monitors = monitorDirRecursively(mdevctlConfigDir, + G_CALLBACK(mdevctlEventHandleCallback), + priv); + virObjectUnlock(priv); /* Create a fictional 'computer' device to root the device tree. */ -- 2.21.3

Add a virsh command that maps to virNodeDeviceDefineXML(). Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- tools/virsh-nodedev.c | 58 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 75542530da..c02b79a349 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -1005,6 +1005,58 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd) } +/* + * "nodedev-define" command + */ +static const vshCmdInfo info_node_device_define[] = { + {.name = "help", + .data = N_("Define a device by an xml file on a node") + }, + {.name = "desc", + .data = N_("Defines a persistent device on the node that can be " + "assigned to a domain. The device must be started before " + "it can be assigned to a domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_node_device_define[] = { + VIRSH_COMMON_OPT_FILE(N_("file containing an XML description " + "of the device")), + {.name = NULL} +}; + +static bool +cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) +{ + virNodeDevicePtr dev = NULL; + const char *from = NULL; + bool ret = true; + char *buffer; + virshControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + return false; + + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) + return false; + + dev = virNodeDeviceDefineXML(priv->conn, buffer, 0); + VIR_FREE(buffer); + + if (dev != NULL) { + vshPrintExtra(ctl, _("Node device %s defined from %s\n"), + virNodeDeviceGetName(dev), from); + virNodeDeviceFree(dev); + } else { + vshError(ctl, _("Failed to define node device from %s"), from); + ret = false; + } + + return ret; +} + + const vshCmdDef nodedevCmds[] = { {.name = "nodedev-create", .handler = cmdNodeDeviceCreate, @@ -1058,5 +1110,11 @@ const vshCmdDef nodedevCmds[] = { .info = info_node_device_event, .flags = 0 }, + {.name = "nodedev-define", + .handler = cmdNodeDeviceDefine, + .opts = opts_node_device_define, + .info = info_node_device_define, + .flags = 0 + }, {.name = NULL} }; -- 2.21.3

Several functions accept providing a node device by name or by wwnn,wwpn pair. Extract the logic to do this into a function that can be used by both callers. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- tools/virsh-nodedev.c | 58 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index c02b79a349..57f987a81e 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -112,23 +112,18 @@ static const vshCmdOptDef opts_node_device_destroy[] = { {.name = NULL} }; -static bool -cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) +static virNodeDevicePtr +vshFindNodeDevice(vshControl *ctl, const char *value) { virNodeDevicePtr dev = NULL; - bool ret = false; - const char *device_value = NULL; char **arr = NULL; int narr; virshControlPtr priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) - return false; - - if (strchr(device_value, ',')) { - narr = vshStringToArray(device_value, &arr); + if (strchr(value, ',')) { + narr = vshStringToArray(value, &arr); if (narr != 2) { - vshError(ctl, _("Malformed device value '%s'"), device_value); + vshError(ctl, _("Malformed device value '%s'"), value); goto cleanup; } @@ -137,9 +132,30 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) dev = virNodeDeviceLookupSCSIHostByWWN(priv->conn, arr[0], arr[1], 0); } else { - dev = virNodeDeviceLookupByName(priv->conn, device_value); + dev = virNodeDeviceLookupByName(priv->conn, value); } + if (!dev) { + vshError(ctl, "%s '%s'", _("Could not find matching device"), value); + goto cleanup; + } + +cleanup: + virStringListFree(arr); + return dev; +} + +static bool +cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevicePtr dev = NULL; + bool ret = false; + const char *device_value = NULL; + + if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + return false; + + dev = vshFindNodeDevice(ctl, device_value); if (!dev) { vshError(ctl, "%s '%s'", _("Could not find matching device"), device_value); goto cleanup; @@ -154,7 +170,6 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - virStringListFree(arr); if (dev) virNodeDeviceFree(dev); return ret; @@ -570,28 +585,12 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd) virNodeDevicePtr device = NULL; char *xml = NULL; const char *device_value = NULL; - char **arr = NULL; - int narr; bool ret = false; - virshControlPtr priv = ctl->privData; if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) return false; - if (strchr(device_value, ',')) { - narr = vshStringToArray(device_value, &arr); - if (narr != 2) { - vshError(ctl, _("Malformed device value '%s'"), device_value); - goto cleanup; - } - - if (!virValidateWWN(arr[0]) || !virValidateWWN(arr[1])) - goto cleanup; - - device = virNodeDeviceLookupSCSIHostByWWN(priv->conn, arr[0], arr[1], 0); - } else { - device = virNodeDeviceLookupByName(priv->conn, device_value); - } + device = vshFindNodeDevice(ctl, device_value); if (!device) { vshError(ctl, "%s '%s'", _("Could not find matching device"), device_value); @@ -605,7 +604,6 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - virStringListFree(arr); VIR_FREE(xml); if (device) virNodeDeviceFree(device); -- 2.21.3

This new API function provides a way to start a persistently-defined mediate device that was defined by virNodeDeviceDefineXML() (or one that was defined externally via mdevctl) Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- include/libvirt/libvirt-nodedev.h | 2 + src/driver-nodedev.h | 4 ++ src/libvirt-nodedev.c | 35 +++++++++++ src/libvirt_public.syms | 1 + src/node_device/node_device_driver.c | 64 ++++++++++++++++++++ src/node_device/node_device_driver.h | 6 ++ src/node_device/node_device_udev.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 ++++- src/remote_protocol-structs | 4 ++ tests/nodedevmdevctldata/mdevctl-create.argv | 1 + tests/nodedevmdevctltest.c | 11 +++- 12 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index f7957499ae..cc6d4c8732 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -133,6 +133,8 @@ virNodeDevicePtr virNodeDeviceDefineXML (virConnectPtr conn, int virNodeDeviceUndefine (virNodeDevicePtr dev); +int virNodeDeviceCreate (virNodeDevicePtr dev); + /** * VIR_NODE_DEVICE_EVENT_CALLBACK: * diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h index e18029ab48..d00dd5845c 100644 --- a/src/driver-nodedev.h +++ b/src/driver-nodedev.h @@ -82,6 +82,9 @@ typedef virNodeDevicePtr typedef int (*virDrvNodeDeviceUndefine)(virNodeDevicePtr dev); +typedef int +(*virDrvNodeDeviceCreate)(virNodeDevicePtr def); + typedef int (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, virNodeDevicePtr dev, @@ -123,4 +126,5 @@ struct _virNodeDeviceDriver { virDrvNodeDeviceDestroy nodeDeviceDestroy; virDrvNodeDeviceDefineXML nodeDeviceDefineXML; virDrvNodeDeviceUndefine nodeDeviceUndefine; + virDrvNodeDeviceCreate nodeDeviceCreate; }; diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index eb01ceb8b0..1b0671c92f 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -839,6 +839,41 @@ virNodeDeviceUndefine(virNodeDevicePtr dev) } +/** + * virNodeDeviceCreate: + * @dev: a device object + * + * Start a defined node device. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virNodeDeviceCreate(virNodeDevicePtr dev) +{ + VIR_DEBUG("dev=%p", dev); + + virResetLastError(); + + virCheckNodeDeviceReturn(dev, -1); + virCheckReadOnlyGoto(dev->conn->flags, error); + + if (dev->conn->nodeDeviceDriver && + dev->conn->nodeDeviceDriver->nodeDeviceCreate) { + int retval = dev->conn->nodeDeviceDriver->nodeDeviceCreate(dev); + if (retval < 0) + goto error; + + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dev->conn); + return -1; +} + + /** * virConnectNodeDeviceEventRegisterAny: * @conn: pointer to the connection diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 35c88ec4b7..fa7144cec1 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -877,5 +877,6 @@ LIBVIRT_6.5.0 { global: virNodeDeviceDefineXML; virNodeDeviceUndefine; + virNodeDeviceCreate; } LIBVIRT_6.0.0; # .... define new API here using predicted next version number .... diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index ab15e82660..51b66ee3d0 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -844,6 +844,17 @@ nodeDeviceGetMdevctlUndefineCommand(const char *uuid) } +virCommandPtr +nodeDeviceGetMdevctlCreateCommand(const char *uuid) +{ + return virCommandNewArgList(MDEVCTL, + "start", + "-u", + uuid, + NULL); + +} + static int virMdevctlStop(virNodeDeviceDefPtr def) { @@ -874,6 +885,21 @@ virMdevctlUndefine(virNodeDeviceDefPtr def) } +static int +virMdevctlCreate(virNodeDeviceDefPtr def) +{ + int status; + g_autoptr(virCommand) cmd = NULL; + + cmd = nodeDeviceGetMdevctlCreateCommand(def->caps->data.mdev.uuid); + + if (virCommandRun(cmd, &status) < 0 || status != 0) + return -1; + + return 0; +} + + virCommandPtr nodeDeviceGetMdevctlListCommand(bool defined, char **output) @@ -1160,6 +1186,44 @@ nodeDeviceUndefine(virNodeDevicePtr device) } +int nodeDeviceCreate(virNodeDevicePtr device) +{ + int ret = -1; + virNodeDeviceObjPtr obj = NULL; + virNodeDeviceDefPtr def; + g_autofree char *parent = NULL; + + if (!(obj = nodeDeviceObjFindByName(device->name))) + return -1; + + if (virNodeDeviceObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Device is already active")); + goto cleanup; + } + def = virNodeDeviceObjGetDef(obj); + + if (virNodeDeviceCreateEnsureACL(device->conn, def) < 0) + goto cleanup; + + if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) { + if (virMdevctlCreate(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to create mediated device")); + goto cleanup; + } + ret = 0; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported device type")); + } + + cleanup: + virNodeDeviceObjEndAPI(&obj); + return ret; +} + + int nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, virNodeDevicePtr device, diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 2adb3a8073..939718effb 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -155,3 +155,9 @@ nodeDeviceGenerateName(virNodeDeviceDefPtr def, const char *subsystem, const char *sysname, const char *s); + +virCommandPtr +nodeDeviceGetMdevctlCreateCommand(const char *uuid); + +int +nodeDeviceCreate(virNodeDevicePtr dev); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 0eb3dea1d6..1e021aa913 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -2200,6 +2200,7 @@ static virNodeDeviceDriver udevNodeDeviceDriver = { .nodeDeviceDestroy = nodeDeviceDestroy, /* 0.7.3 */ .nodeDeviceDefineXML = nodeDeviceDefineXML, /* 6.5.0 */ .nodeDeviceUndefine = nodeDeviceUndefine, /* 6.5.0 */ + .nodeDeviceCreate = nodeDeviceCreate, /* 6.5.0 */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index eb102b6060..f2f4a42651 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8814,6 +8814,7 @@ static virNodeDeviceDriver node_device_driver = { .nodeDeviceNumOfCaps = remoteNodeDeviceNumOfCaps, /* 0.5.0 */ .nodeDeviceListCaps = remoteNodeDeviceListCaps, /* 0.5.0 */ .nodeDeviceCreateXML = remoteNodeDeviceCreateXML, /* 0.6.3 */ + .nodeDeviceCreate = remoteNodeDeviceCreate, /* 6.5.0 */ .nodeDeviceDefineXML = remoteNodeDeviceDefineXML, /* 6.5.0 */ .nodeDeviceUndefine = remoteNodeDeviceUndefine, /* 6.5.0 */ .nodeDeviceDestroy = remoteNodeDeviceDestroy /* 0.6.3 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d668074204..b019f38efd 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2152,6 +2152,10 @@ struct remote_node_device_undefine_args { remote_nonnull_string name; }; +struct remote_node_device_create_args { + remote_nonnull_string name; +}; + /* * Events Register/Deregister: @@ -6682,7 +6686,6 @@ enum remote_procedure { /** * @generate: both * @acl: node_device:write - * @acl: node_device:start */ REMOTE_PROC_NODE_DEVICE_DEFINE_XML = 423, @@ -6691,6 +6694,13 @@ enum remote_procedure { * @priority: high * @acl: node_device:stop */ - REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424 + REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424, + + /** + * @generate: both + * @priority: high + * @acl: node_device:start + */ + REMOTE_PROC_NODE_DEVICE_CREATE = 425 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3d450a6de9..a3cfe2572e 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1610,6 +1610,9 @@ struct remote_node_device_define_xml_ret { struct remote_node_device_undefine_args { remote_nonnull_string name; }; +struct remote_node_device_create_args { + remote_nonnull_string name; +}; struct remote_connect_domain_event_register_ret { int cb_registered; }; @@ -3570,4 +3573,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BACKUP_GET_XML_DESC = 422, REMOTE_PROC_NODE_DEVICE_DEFINE_XML = 423, REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424, + REMOTE_PROC_NODE_DEVICE_CREATE = 425, }; diff --git a/tests/nodedevmdevctldata/mdevctl-create.argv b/tests/nodedevmdevctldata/mdevctl-create.argv new file mode 100644 index 0000000000..802109340c --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-create.argv @@ -0,0 +1 @@ +$MDEVCTL_BINARY$ start -u 8a05ad83-3472-497d-8631-8142f31460e8 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index fbc178a4ac..c768c841b4 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -14,7 +14,8 @@ typedef enum { MDEVCTL_CMD_START, MDEVCTL_CMD_STOP, MDEVCTL_CMD_DEFINE, - MDEVCTL_CMD_UNDEFINE + MDEVCTL_CMD_UNDEFINE, + MDEVCTL_CMD_CREATE } MdevctlCmd; struct startTestInfo { @@ -186,6 +187,9 @@ testMdevctlUuidCommandHelper(const void *data) }else if (info->command == MDEVCTL_CMD_UNDEFINE) { cmd = "undefine"; func = nodeDeviceGetMdevctlUndefineCommand; + }else if (info->command == MDEVCTL_CMD_CREATE) { + cmd = "create"; + func = nodeDeviceGetMdevctlCreateCommand; } else { return -1; } @@ -431,6 +435,9 @@ mymain(void) #define DO_TEST_UNDEFINE(uuid) \ DO_TEST_UUID_COMMAND_FULL("mdevctl undefine " uuid, uuid, MDEVCTL_CMD_UNDEFINE) +#define DO_TEST_CREATE(uuid) \ + DO_TEST_UUID_COMMAND_FULL("mdevctl create " uuid, uuid, MDEVCTL_CMD_CREATE) + #define DO_TEST_LIST_DEFINED() \ do { \ if (virTestRun("mdevctl list --defined", testMdevctlListDefined, NULL) < 0) \ @@ -461,6 +468,8 @@ mymain(void) DO_TEST_UNDEFINE("d76a6b78-45ed-4149-a325-005f9abc5281"); + DO_TEST_CREATE("8a05ad83-3472-497d-8631-8142f31460e8"); + done: nodedevTestDriverFree(driver); -- 2.21.3