10:16 a.m.
On 2/1/24 6:17 AM, Boris Fiuczynski wrote:
On 1/31/24 22:30, Jonathon Jongsma wrote:
> On 1/19/24 10:38 AM, Boris Fiuczynski wrote:
>> The configuration of a defined mdev can be modified after the mdev is
>> started. The defined configuration and the active configuration can
>> therefore run out of sync. Handle this by storing the modifiable data
>> which is the mdev type and attributes in two separate active and
>> defined configurations. mdevctl supports with callout scripts to do an
>> attribute retrieval of started mdevs which is already implemented in
>> libvirt.
>>
>> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
>> ---
>> include/libvirt/libvirt-nodedev.h | 11 ++++
>> src/conf/node_device_conf.c | 53 ++++++++++------
>> src/conf/node_device_conf.h | 5 +-
>> src/libvirt-nodedev.c | 2 +-
>> src/node_device/node_device_driver.c | 61 +++++++++++++------
>> src/node_device/node_device_driver.h | 6 +-
>> src/node_device/node_device_udev.c | 4 +-
>> src/test/test_driver.c | 6 +-
>> tests/nodedevmdevctltest.c | 4 +-
>> ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml | 14 +++++
>> ...d_b7f0_4fea_b468_f1da537d301b_inactive.xml | 1 +
>> ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml | 10 +++
>> ...c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml | 9 +++
>> ...9_36ea_4111_8f0a_8c9a70e21366_inactive.xml | 1 +
>> ...9_495e_4243_ad9f_beb3f14c23d9_inactive.xml | 1 +
>> ...4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml | 8 +++
>> ...6_1ca8_49ac_b176_871d16c13076_inactive.xml | 1 +
>> tests/nodedevxml2xmltest.c | 59 +++++++++++++++---
>> 18 files changed, 197 insertions(+), 59 deletions(-)
>> create mode 100644
>> tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>> create mode 120000
>> tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
>> create mode 100644
>> tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>> create mode 100644
>> tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
>> create mode 120000
>> tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
>> create mode 120000
>> tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
>> create mode 100644
>> tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
>> create mode 120000
>> tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
>>
>> diff --git a/include/libvirt/libvirt-nodedev.h
>> b/include/libvirt/libvirt-nodedev.h
>> index 428b0d722f..d18246e2f6 100644
>> --- a/include/libvirt/libvirt-nodedev.h
>> +++ b/include/libvirt/libvirt-nodedev.h
>> @@ -117,6 +117,17 @@ int virNodeDeviceListCaps
>> (virNodeDevicePtr dev,
>> char **const names,
>> int maxnames);
>> +/**
>> + * virNodeDeviceGetXMLDescFlags:
>> + *
>> + * Flags used to provide the state of the returned node device
>> configuration.
>> + *
>> + * Since: 10.1.0
>> + */
>> +typedef enum {
>> + VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE = 1 << 0, /*
>> dump inactive device configuration (Since: 10.1.0) */
>> +} virNodeDeviceGetXMLDescFlags;
>
> In all of the other similar cases, this type is named
> vir$(OBJECT)XMLFlags and the flag itself is named
> VIR_$(OBJECT)_XML_INACTIVE. So for consistency, I'd remove the 'get'
> and the 'desc' from the names.
>
I disagree as all other methods that make use of flags base the flags
names on the method name. Here are the examples:
virNodeDeviceCreateXML
virNodeDeviceCreateXMLFlags
vir Node Device Create XML
VIR_NODE_DEVICE_CREATE_XML_*
virNodeDeviceDefineXML
virNodeDeviceDefineXMLFlags
vir Node Device Define XML
VIR_NODE_DEVICE_DEFINE_XML_*
virConnectListAllNodeDevices
virConnectListAllNodeDeviceFlags
vir Connect List Node Device [All is removed]
VIR_CONNECT_LIST_NODE_DEVICES_*
These are the reasons I chose for consistency:
virNodeDeviceGetXMLDesc
virNodeDeviceGetXMLDescFlags
vir Node Device Get XML Desc
VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE
That's true in general, however for the *GetXMLDesc() functions, this
pattern doesn't hold:
domain:
- virDomainGetXMLDesc()
- virDomainXMLFlags
- VIR_DOMAIN_XML_INACTIVE
storage:
- virStoragePoolGetXMLDesc()
- virStorageXMLFlags
- VIR_STORAGE_XML_INACTIVE
network:
- virNetworkGetXMLDesc()
- virNetworkXMLFlags
- VIR_NETWORK_XML_INACTIVE
interface:
- virInterfaceGetXMLDesc()
- virInterfaceXMLFlags
- VIR_INTERFACE_XML_INACTIVE
There are no types following the *GetXMLDescFlags pattern:
$ git grep XMLDescFlags include/
$
I don't feel personally strongly about this point because there are
consistency arguments for both approaches. But I thought I'd mention it.
Jonathon
>> +
>> char * virNodeDeviceGetXMLDesc (virNodeDevicePtr dev,
>> unsigned int flags);
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index 9a0fc68e67..40e15ef4da 100644
>> --- a/src/conf/node_device_conf.c
>> +++ b/src/conf/node_device_conf.c
>> @@ -599,16 +599,22 @@ virNodeDeviceCapMdevAttrFormat(virBuffer *buf,
>> static void
>> virNodeDeviceCapMdevDefFormat(virBuffer *buf,
>> - const virNodeDevCapData *data)
>> + const virNodeDevCapData *data,
>> + bool defined)
>> {
>> - virBufferEscapeString(buf, "<type id='%s'/>\n",
>> data->mdev.dev_config.type);
>> + if (defined)
>> + virBufferEscapeString(buf, "<type id='%s'/>\n",
>> data->mdev.defined_config.type);
>> + else
>> + virBufferEscapeString(buf, "<type id='%s'/>\n",
>> data->mdev.active_config.type);
>> virBufferEscapeString(buf, "<uuid>%s</uuid>\n",
data->mdev.uuid);
>> virBufferEscapeString(buf,
"<parent_addr>%s</parent_addr>\n",
>> data->mdev.parent_addr);
>> virBufferAsprintf(buf, "<iommuGroup
number='%u'/>\n",
>> data->mdev.iommuGroupNumber);
>> -
>> - virNodeDeviceCapMdevAttrFormat(buf, &data->mdev.dev_config);
>> + if (defined)
>> + virNodeDeviceCapMdevAttrFormat(buf,
>> &data->mdev.defined_config);
>> + else
>> + virNodeDeviceCapMdevAttrFormat(buf, &data->mdev.active_config);
>> }
>> static void
>> @@ -659,25 +665,28 @@ virNodeDeviceCapCSSDefFormat(virBuffer *buf,
>> char *
>> -virNodeDeviceDefFormat(const virNodeDeviceDef *def)
>> +virNodeDeviceDefFormat(const virNodeDeviceDef *def, unsigned int flags)
>> {
>> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>> virNodeDevCapsDef *caps;
>> size_t i = 0;
>> + bool inactive_state = flags &
>> VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE;
>> virBufferAddLit(&buf, "<device>\n");
>> virBufferAdjustIndent(&buf, 2);
>> virBufferEscapeString(&buf, "<name>%s</name>\n",
def->name);
>> - virBufferEscapeString(&buf, "<path>%s</path>\n",
def->sysfs_path);
>> - virBufferEscapeString(&buf, "<devnode
type='dev'>%s</devnode>\n",
>> - def->devnode);
>> - if (def->devlinks) {
>> - for (i = 0; def->devlinks[i]; i++)
>> - virBufferEscapeString(&buf, "<devnode
>> type='link'>%s</devnode>\n",
>> - def->devlinks[i]);
>> + if (!inactive_state) {
>> + virBufferEscapeString(&buf,
"<path>%s</path>\n",
>> def->sysfs_path);
>> + virBufferEscapeString(&buf, "<devnode
>> type='dev'>%s</devnode>\n",
>> + def->devnode);
>> + if (def->devlinks) {
>> + for (i = 0; def->devlinks[i]; i++)
>> + virBufferEscapeString(&buf, "<devnode
>> type='link'>%s</devnode>\n",
>> + def->devlinks[i]);
>> + }
>> }
>> virBufferEscapeString(&buf,
"<parent>%s</parent>\n", def->parent);
>> - if (def->driver) {
>> + if (def->driver && !inactive_state) {
>> virBufferAddLit(&buf, "<driver>\n");
>> virBufferAdjustIndent(&buf, 2);
>> virBufferEscapeString(&buf,
"<name>%s</name>\n", def->driver);
>> @@ -738,7 +747,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
>> virBufferEscapeString(&buf,
"<type>%s</type>\n",
>> virNodeDevDRMTypeToString(data->drm.type));
>> break;
>> case VIR_NODE_DEV_CAP_MDEV:
>> - virNodeDeviceCapMdevDefFormat(&buf, data);
>> + virNodeDeviceCapMdevDefFormat(&buf, data, inactive_state);
>> break;
>> case VIR_NODE_DEV_CAP_CCW_DEV:
>> virNodeDeviceCapCCWDefFormat(&buf, data);
>> @@ -2226,7 +2235,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
>> ctxt->node = node;
>> - if (!(mdev->dev_config.type =
>> virXPathString("string(./type[1]/@id)", ctxt))) {
>> + if (!(mdev->defined_config.type =
>> virXPathString("string(./type[1]/@id)", ctxt))) {
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("missing type id attribute for
'%1$s'"),
>> def->name);
>> return -1;
>> @@ -2258,7 +2267,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
>> return -1;
>> for (i = 0; i < nattrs; i++)
>> - virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i],
>> &mdev->dev_config);
>> + virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i],
>> &mdev->defined_config);
>> return 0;
>> }
>> @@ -2601,11 +2610,15 @@ virNodeDevCapsDefFree(virNodeDevCapsDef *caps)
>> g_free(data->sg.path);
>> break;
>> case VIR_NODE_DEV_CAP_MDEV:
>> - g_free(data->mdev.dev_config.type);
>> + g_free(data->mdev.defined_config.type);
>> + g_free(data->mdev.active_config.type);
>> g_free(data->mdev.uuid);
>> - for (i = 0; i < data->mdev.dev_config.nattributes; i++)
>> - virMediatedDeviceAttrFree(data->mdev.dev_config.attributes[i]);
>> - g_free(data->mdev.dev_config.attributes);
>> + for (i = 0; i < data->mdev.defined_config.nattributes; i++)
>> + virMediatedDeviceAttrFree(data->mdev.defined_config.attributes[i]);
>> + g_free(data->mdev.defined_config.attributes);
>> + for (i = 0; i < data->mdev.active_config.nattributes; i++)
>> + virMediatedDeviceAttrFree(data->mdev.active_config.attributes[i]);
>> + g_free(data->mdev.active_config.attributes);
>> g_free(data->mdev.parent_addr);
>> break;
>> case VIR_NODE_DEV_CAP_CSS_DEV:
>> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
>> index f100496272..f59440dbb9 100644
>> --- a/src/conf/node_device_conf.h
>> +++ b/src/conf/node_device_conf.h
>> @@ -153,7 +153,8 @@ typedef struct _virNodeDevCapMdev virNodeDevCapMdev;
>> struct _virNodeDevCapMdev {
>> unsigned int iommuGroupNumber;
>> char *uuid;
>> - virMediatedDeviceConfig dev_config;
>> + virMediatedDeviceConfig defined_config;
>> + virMediatedDeviceConfig active_config;
>> char *parent_addr;
>> bool autostart;
>> };
>> @@ -360,7 +361,7 @@ struct _virNodeDeviceDef {
>> };
>> char *
>> -virNodeDeviceDefFormat(const virNodeDeviceDef *def);
>> +virNodeDeviceDefFormat(const virNodeDeviceDef *def, unsigned int
>> flags);
>> typedef int (*virNodeDeviceDefPostParseCallback)(virNodeDeviceDef
>> *dev,
>> diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
>> index f0f99bc020..9cc5c6466b 100644
>> --- a/src/libvirt-nodedev.c
>> +++ b/src/libvirt-nodedev.c
>> @@ -264,7 +264,7 @@ virNodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
>> /**
>> * virNodeDeviceGetXMLDesc:
>> * @dev: pointer to the node device
>> - * @flags: extra flags; not used yet, so callers should always pass 0
>> + * @flags: bitwise-OR of virNodeDeviceGetXMLDescFlags
>> *
>> * Fetch an XML document describing all aspects of
>> * the device.
>> diff --git a/src/node_device/node_device_driver.c
>> b/src/node_device/node_device_driver.c
>> index 0c8612eb71..d67c95d68d 100644
>> --- a/src/node_device/node_device_driver.c
>> +++ b/src/node_device/node_device_driver.c
>> @@ -338,7 +338,7 @@ nodeDeviceGetXMLDesc(virNodeDevicePtr device,
>> virNodeDeviceDef *def;
>> char *ret = NULL;
>> - virCheckFlags(0, NULL);
>> + virCheckFlags(VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE, NULL);
>> if (nodeDeviceInitWait() < 0)
>> return NULL;
>> @@ -356,7 +356,19 @@ nodeDeviceGetXMLDesc(virNodeDevicePtr device,
>> if (virNodeDeviceUpdateCaps(def) < 0)
>> goto cleanup;
>> - ret = virNodeDeviceDefFormat(def);
>> + if (flags & VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE) {
>> + if (!virNodeDeviceObjIsPersistent(obj)) {
>> + virReportError(VIR_ERR_OPERATION_INVALID,
>> + _("node device '%1$s' is not
persistent"),
>> + def->name);
>> + goto cleanup;
>> + }
>> + } else {
>> + if (!virNodeDeviceObjIsActive(obj))
>> + flags |= VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE;
>> + }
>> +
>> + ret = virNodeDeviceDefFormat(def, flags);
>> cleanup:
>> virNodeDeviceObjEndAPI(&obj);
>> @@ -629,19 +641,20 @@ nodeDeviceAttributesToJSON(virJSONValue *json,
>> /* format a json string that provides configuration information
>> about this mdev
>> * to the mdevctl utility */
>> static int
>> -nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf)
>> +nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf, bool
>> defined)
>
> As far as I can tell, you never actually call this function with
> 'defined' set to false. It doesn't seem to be used in any of the
> following commits either.
Correct. I did not want to hardcode it in the method.
>
>> {
>> virNodeDevCapMdev *mdev = &def->caps->data.mdev;
>> + virMediatedDeviceConfig mdev_config = defined ?
>> mdev->defined_config : mdev->active_config;
>> g_autoptr(virJSONValue) json = virJSONValueNewObject();
>> const char *startval = mdev->autostart ? "auto" :
"manual";
>> - if (virJSONValueObjectAppendString(json, "mdev_type",
>> mdev->dev_config.type) < 0)
>> + if (virJSONValueObjectAppendString(json, "mdev_type",
>> mdev_config.type) < 0)
>> return -1;
>> if (virJSONValueObjectAppendString(json, "start", startval) <
0)
>> return -1;
>> - if (nodeDeviceAttributesToJSON(json, mdev->dev_config) < 0)
>> + if (nodeDeviceAttributesToJSON(json, mdev_config) < 0)
>> return -1;
>> *buf = virJSONValueToString(json, false);
>> @@ -760,7 +773,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
>> return NULL;
>> }
>> - if (nodeDeviceDefToMdevctlConfig(def, &inbuf) < 0) {
>> + if (nodeDeviceDefToMdevctlConfig(def, &inbuf, true) < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("couldn't convert node device def to
>> mdevctl JSON"));
>> return NULL;
>> @@ -1138,9 +1151,11 @@
>> nodeDeviceParseMdevctlAttribs(virMediatedDeviceConfig *config,
>> static virNodeDeviceDef*
>> nodeDeviceParseMdevctlChildDevice(const char *parent,
>> - virJSONValue *json)
>> + virJSONValue *json,
>> + bool defined)
>> {
>> virNodeDevCapMdev *mdev;
>> + virMediatedDeviceConfig *mdev_config;
>> const char *uuid;
>> virJSONValue *props;
>> g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1);
>> @@ -1170,14 +1185,15 @@ nodeDeviceParseMdevctlChildDevice(const char
>> *parent,
>> child->caps->data.type = VIR_NODE_DEV_CAP_MDEV;
>> mdev = &child->caps->data.mdev;
>> + mdev_config = defined ? &mdev->defined_config :
>> &mdev->active_config;
>> mdev->uuid = g_strdup(uuid);
>> mdev->parent_addr = g_strdup(parent);
>> - mdev->dev_config.type =
>> + mdev_config->type =
>> g_strdup(virJSONValueObjectGetString(props, "mdev_type"));
>> start = virJSONValueObjectGetString(props, "start");
>> mdev->autostart = STREQ_NULLABLE(start, "auto");
>> - if (nodeDeviceParseMdevctlAttribs(&mdev->dev_config,
>> + if (nodeDeviceParseMdevctlAttribs(mdev_config,
>> virJSONValueObjectGet(props,
>> "attrs")) < 0)
>> return NULL;
>> @@ -1189,7 +1205,8 @@ nodeDeviceParseMdevctlChildDevice(const char
>> *parent,
>> int
>> nodeDeviceParseMdevctlJSON(const char *jsonstring,
>> - virNodeDeviceDef ***devs)
>> + virNodeDeviceDef ***devs,
>> + bool defined)
>> {
>> int n;
>> g_autoptr(virJSONValue) json_devicelist = NULL;
>> @@ -1259,7 +1276,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
>> g_autoptr(virNodeDeviceDef) child = NULL;
>> virJSONValue *child_obj =
>> virJSONValueArrayGet(child_array, j);
>> - if (!(child = nodeDeviceParseMdevctlChildDevice(parent,
>> child_obj))) {
>> + if (!(child = nodeDeviceParseMdevctlChildDevice(parent,
>> child_obj, defined))) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Unable to parse child device"));
>> goto error;
>> @@ -1402,7 +1419,7 @@ nodeDeviceUpdateMediatedDevice(virNodeDeviceDef
>> *def,
>> /* Active devices contain some additional information (e.g.
>> sysfs
>> * path) that is not provided by mdevctl, so re-use the
>> existing
>> * definition and copy over new mdev data */
>> - changed = nodeDeviceDefCopyFromMdevctl(olddef, owned);
>> + changed = nodeDeviceDefCopyFromMdevctl(olddef, owned, defined);
>> if (was_defined && !changed) {
>> /* if this device was already defined and the definition
>> @@ -1672,7 +1689,7 @@ virMdevctlList(bool defined,
>> return -1;
>> }
>> - return nodeDeviceParseMdevctlJSON(output, devs);
>> + return nodeDeviceParseMdevctlJSON(output, devs, defined);
>> }
>> @@ -1831,16 +1848,24 @@
>> virMediatedDeviceAttrsCopy(virMediatedDeviceConfig *dst_config,
>> * Returns true if anything was copied, else returns false */
>> bool
>> nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
>> - virNodeDeviceDef *src)
>> + virNodeDeviceDef *src,
>> + bool defined)
>> {
>> bool ret = false;
>> virNodeDevCapMdev *srcmdev = &src->caps->data.mdev;
>> virNodeDevCapMdev *dstmdev = &dst->caps->data.mdev;
>> + virMediatedDeviceConfig *srcmdevconfig =
>> &src->caps->data.mdev.active_config;
>> + virMediatedDeviceConfig *dstmdevconfig =
>> &dst->caps->data.mdev.active_config;
>> +
>> + if (defined) {
>> + srcmdevconfig = &src->caps->data.mdev.defined_config;
>> + dstmdevconfig = &dst->caps->data.mdev.defined_config;
>> + }
>> - if (STRNEQ_NULLABLE(dstmdev->dev_config.type,
>> srcmdev->dev_config.type)) {
>> + if (STRNEQ_NULLABLE(dstmdevconfig->type, srcmdevconfig->type)) {
>> ret = true;
>> - g_free(dstmdev->dev_config.type);
>> - dstmdev->dev_config.type = g_strdup(srcmdev->dev_config.type);
>> + g_free(dstmdevconfig->type);
>> + dstmdevconfig->type = g_strdup(srcmdevconfig->type);
>> }
>> if (STRNEQ_NULLABLE(dstmdev->uuid, srcmdev->uuid)) {
>> @@ -1849,7 +1874,7 @@ nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef
>> *dst,
>> dstmdev->uuid = g_strdup(srcmdev->uuid);
>> }
>> - if (virMediatedDeviceAttrsCopy(&dstmdev->dev_config,
>> &srcmdev->dev_config))
>> + if (virMediatedDeviceAttrsCopy(dstmdevconfig, srcmdevconfig))
>> ret = true;
>> if (dstmdev->autostart != srcmdev->autostart) {
>> diff --git a/src/node_device/node_device_driver.h
>> b/src/node_device/node_device_driver.h
>> index c7d5e22daf..4dce7e6f17 100644
>> --- a/src/node_device/node_device_driver.h
>> +++ b/src/node_device/node_device_driver.h
>> @@ -142,7 +142,8 @@ nodeDeviceGetMdevctlListCommand(bool defined,
>> int
>> nodeDeviceParseMdevctlJSON(const char *jsonstring,
>> - virNodeDeviceDef ***devs);
>> + virNodeDeviceDef ***devs,
>> + bool defined);
>> int
>> nodeDeviceUpdateMediatedDevices(void);
>> @@ -154,7 +155,8 @@ nodeDeviceGenerateName(virNodeDeviceDef *def,
>> const char *s);
>> bool nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
>> - virNodeDeviceDef *src);
>> + virNodeDeviceDef *src,
>> + bool defined);
>> int
>> nodeDeviceCreate(virNodeDevice *dev,
>> diff --git a/src/node_device/node_device_udev.c
>> b/src/node_device/node_device_udev.c
>> index 254e802c50..57368a96c3 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -1069,7 +1069,7 @@ udevProcessMediatedDevice(struct udev_device *dev,
>> return -1;
>> }
>> - data->dev_config.type = g_path_get_basename(canonicalpath);
>> + data->active_config.type = g_path_get_basename(canonicalpath);
>> data->uuid = g_strdup(udev_device_get_sysname(dev));
>> if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(data->uuid))
>> < 0)
>> @@ -1572,7 +1572,7 @@ udevAddOneDevice(struct udev_device *device)
>> objdef = virNodeDeviceObjGetDef(obj);
>> if (is_mdev)
>> - nodeDeviceDefCopyFromMdevctl(def, objdef);
>> + nodeDeviceDefCopyFromMdevctl(def, objdef, false);
>> persistent = virNodeDeviceObjIsPersistent(obj);
>> autostart = virNodeDeviceObjIsAutostart(obj);
>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>> index ed545848af..01863233bc 100644
>> --- a/src/test/test_driver.c
>> +++ b/src/test/test_driver.c
>> @@ -7514,12 +7514,12 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
>> virNodeDeviceObj *obj;
>> char *ret = NULL;
>> - virCheckFlags(0, NULL);
>> + virCheckFlags(VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE, NULL);
>> if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
>> return NULL;
>> - ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj));
>> + ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj), flags);
>> virNodeDeviceObjEndAPI(&obj);
>> return ret;
>> @@ -7619,7 +7619,7 @@ testNodeDeviceMockCreateVport(testDriver *driver,
>> "scsi_host11")))
>> goto cleanup;
>> - xml = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(objcopy));
>> + xml = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(objcopy), 0);
>> virNodeDeviceObjEndAPI(&objcopy);
>> if (!xml)
>> goto cleanup;
>> diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
>> index e403328e5a..852d9ed6e7 100644
>> --- a/tests/nodedevmdevctltest.c
>> +++ b/tests/nodedevmdevctltest.c
>> @@ -229,13 +229,13 @@ testMdevctlParse(const void *data)
>> return -1;
>> }
>> - if ((nmdevs = nodeDeviceParseMdevctlJSON(buf, &mdevs)) < 0) {
>> + if ((nmdevs = nodeDeviceParseMdevctlJSON(buf, &mdevs, true)) < 0) {
>> VIR_TEST_DEBUG("Unable to parse json for %s", filename);
>> return -1;
>> }
>> for (i = 0; i < nmdevs; i++) {
>> - g_autofree char *devxml = virNodeDeviceDefFormat(mdevs[i]);
>> + g_autofree char *devxml = virNodeDeviceDefFormat(mdevs[i],
>> VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE);
>> if (!devxml)
>> goto cleanup;
>> virBufferAddStr(&xmloutbuf, devxml);
>> diff --git
>> a/tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
b/tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>> new file mode 100644
>> index 0000000000..6926559efa
>> --- /dev/null
>> +++
>> b/tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>> @@ -0,0 +1,14 @@
>> +<device>
>> + <name>mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0052</name>
>> +
>>
<path>/sys/devices/css0/0.0.0052/c60cc60c-c60c-c60c-c60c-c60cc60cc60c</path>
>> + <parent>css_0_0_0052</parent>
>> + <driver>
>> + <name>vfio_ccw_mdev</name>
>> + </driver>
>> + <capability type='mdev'>
>> + <type id='vfio_ccw-io'/>
>> + <uuid>c60cc60c-c60c-c60c-c60c-c60cc60cc60c</uuid>
>> + <parent_addr>0.0.0052</parent_addr>
>> + <iommuGroup number='4'/>
>> + </capability>
>> +</device>
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
b/tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
>> new file mode 120000
>> index 0000000000..f8ec7d8a32
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
>> @@ -0,0 +1 @@
>> +mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml
>> \ No newline at end of file
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
b/tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>> new file mode 100644
>> index 0000000000..82c60cc065
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>> @@ -0,0 +1,10 @@
>> +<device>
>> + <name>mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0052</name>
>> +
>>
<path>/sys/devices/css0/0.0.0052/c60cc60c-c60c-c60c-c60c-c60cc60cc60c</path>
>> + <parent>css_0_0_0052</parent>
>> + <capability type='mdev'>
>> + <type id='vfio_ccw-io'/>
>> + <uuid>c60cc60c-c60c-c60c-c60c-c60cc60cc60c</uuid>
>> + <iommuGroup number='4'/>
>> + </capability>
>> +</device>
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
b/tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
>> new file mode 100644
>> index 0000000000..87c5ed1340
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
>> @@ -0,0 +1,9 @@
>> +<device>
>> + <name>mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0052</name>
>> + <parent>css_0_0_0052</parent>
>> + <capability type='mdev'>
>> + <type id='vfio_ccw-io'/>
>> + <uuid>c60cc60c-c60c-c60c-c60c-c60cc60cc60c</uuid>
>> + <iommuGroup number='4'/>
>> + </capability>
>> +</device>
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
b/tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
>> new file mode 120000
>> index 0000000000..f6d5789399
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
>> @@ -0,0 +1 @@
>> +mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml
>> \ No newline at end of file
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
b/tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
>> new file mode 120000
>> index 0000000000..233a7506fb
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
>> @@ -0,0 +1 @@
>> +mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml
>> \ No newline at end of file
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
b/tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
>> new file mode 100644
>> index 0000000000..8c4983f43c
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
>> @@ -0,0 +1,8 @@
>> +<device>
>> + <name>mdev_ee0b88c4-f554-4dc1-809d-b2a01e8e48ad</name>
>> + <parent>ap_matrix</parent>
>> + <capability type='mdev'>
>> + <type id='vfio_ap-passthrough'/>
>> + <iommuGroup number='0'/>
>> + </capability>
>> +</device>
>> diff --git
>> a/tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
b/tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
>> new file mode 120000
>> index 0000000000..e053844177
>> --- /dev/null
>> +++
>> b/tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
>> @@ -0,0 +1 @@
>> +mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml
>> \ No newline at end of file
>> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
>> index 068ec68769..be7a5b4df9 100644
>> --- a/tests/nodedevxml2xmltest.c
>> +++ b/tests/nodedevxml2xmltest.c
>> @@ -11,14 +11,20 @@
>> #define VIR_FROM_THIS VIR_FROM_NONE
>> +struct TestData {
>> + const char *filename;
>> + unsigned int flags;
>> +};
>> +
>> static int
>> -testCompareXMLToXMLFiles(const char *xml, const char *outfile)
>> +testCompareXMLToXMLFiles(const char *xml, const char *outfile,
>> unsigned int flags)
>> {
>> g_autofree char *xmlData = NULL;
>> g_autofree char *actual = NULL;
>> int ret = -1;
>> virNodeDeviceDef *dev = NULL;
>> virNodeDevCapsDef *caps;
>> + size_t i;
>> if (virTestLoadFile(xml, &xmlData) < 0)
>> goto fail;
>> @@ -46,9 +52,22 @@ testCompareXMLToXMLFiles(const char *xml, const
>> char *outfile)
>> data->storage.logical_block_size;
>> }
>> }
>> +
>> + if (caps->data.type == VIR_NODE_DEV_CAP_MDEV &&
>> + !(flags & VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE)) {
>> + data->mdev.active_config.type =
>> g_strdup(data->mdev.defined_config.type);
>> + for (i = 0; i < data->mdev.defined_config.nattributes;
>> i++) {
>> + g_autoptr(virMediatedDeviceAttr) attr =
>> g_new0(virMediatedDeviceAttr, 1);
>> + attr->name =
>> g_strdup(data->mdev.defined_config.attributes[i]->name);
>> + attr->value =
>> g_strdup(data->mdev.defined_config.attributes[i]->value);
>> + VIR_APPEND_ELEMENT(data->mdev.active_config.attributes,
>> +
>> data->mdev.active_config.nattributes,
>> + attr);
>> + }
>> + }
>> }
>> - if (!(actual = virNodeDeviceDefFormat(dev)))
>> + if (!(actual = virNodeDeviceDefFormat(dev, flags)))
>> goto fail;
>> if (virTestCompareToFile(actual, outfile) < 0)
>> @@ -65,16 +84,21 @@ static int
>> testCompareXMLToXMLHelper(const void *data)
>> {
>> int result = -1;
>> + const struct TestData *tdata = data;
>> g_autofree char *xml = NULL;
>> g_autofree char *outfile = NULL;
>> xml = g_strdup_printf("%s/nodedevschemadata/%s.xml", abs_srcdir,
>> - (const char *)data);
>> + tdata->filename);
>> - outfile = g_strdup_printf("%s/nodedevxml2xmlout/%s.xml",
>> abs_srcdir,
>> - (const char *)data);
>> + if (tdata->flags & VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE)
>> + outfile =
>> g_strdup_printf("%s/nodedevxml2xmlout/%s_inactive.xml", abs_srcdir,
>> + tdata->filename);
>> + else
>> + outfile = g_strdup_printf("%s/nodedevxml2xmlout/%s.xml",
>> abs_srcdir,
>> + tdata->filename);
>> - result = testCompareXMLToXMLFiles(xml, outfile);
>> + result = testCompareXMLToXMLFiles(xml, outfile, tdata->flags);
>> return result;
>> }
>> @@ -85,10 +109,20 @@ mymain(void)
>> {
>> int ret = 0;
>> +#define DO_TEST_FLAGS(desc, filename, flags) \
>> + do { \
>> + struct TestData data = { filename, flags }; \
>> + if (virTestRun(desc, testCompareXMLToXMLHelper, &data) < 0) \
>> + ret = -1; \
>> + } \
>> + while (0)
>> +
>> #define DO_TEST(name) \
>> - if (virTestRun("Node device XML-2-XML " name, \
>> - testCompareXMLToXMLHelper, (name)) < 0) \
>> - ret = -1
>> + DO_TEST_FLAGS("Node device XML-2-XML " name, name, 0)
>> +
>> +#define DO_TEST_INACTIVE(name) \
>> + DO_TEST_FLAGS("Node device XML-2-XML INACTIVE " name, \
>> + name, VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE)
>> DO_TEST("computer");
>> DO_TEST("DVD_GCC_4247N");
>> @@ -121,6 +155,7 @@ mymain(void)
>> DO_TEST("pci_0000_02_10_7_mdev_types");
>> DO_TEST("pci_0000_42_00_0_vpd");
>> DO_TEST("mdev_3627463d_b7f0_4fea_b468_f1da537d301b");
>> + DO_TEST_INACTIVE("mdev_3627463d_b7f0_4fea_b468_f1da537d301b");
>> DO_TEST("ccw_0_0_ffff");
>> DO_TEST("css_0_0_ffff");
>> DO_TEST("css_0_0_ffff_channel_dev_addr");
>> @@ -134,7 +169,13 @@ mymain(void)
>> DO_TEST("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
>> DO_TEST("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9");
>> DO_TEST("mdev_fedc4916_1ca8_49ac_b176_871d16c13076");
>> + DO_TEST_INACTIVE("mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad");
>> + DO_TEST_INACTIVE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
>> + DO_TEST_INACTIVE("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9");
>> + DO_TEST_INACTIVE("mdev_fedc4916_1ca8_49ac_b176_871d16c13076");
>> DO_TEST("hba_vport_ops");
>> + DO_TEST("mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c");
>> + DO_TEST_INACTIVE("mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c");
>> return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>> }
>
> Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com>
Thanks
> _______________________________________________
> Devel mailing list -- devel(a)lists.libvirt.org
> To unsubscribe send an email to devel-leave(a)lists.libvirt.org