Re: [PATCH 02/11] node_device: refactor mdev attributes handling

Refactor attribute handling code into methods for easier reuse. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- src/conf/node_device_conf.c | 27 ++++--- src/node_device/node_device_driver.c | 108 ++++++++++++++++----------- 2 files changed, 83 insertions(+), 52 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4e1bde503f..68924b3027 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -585,11 +585,22 @@ virNodeDeviceCapStorageDefFormat(virBuffer *buf, } static void -virNodeDeviceCapMdevDefFormat(virBuffer *buf, - const virNodeDevCapData *data) +virNodeDeviceCapMdevAttrFormat(virBuffer *buf, + const virMediatedDeviceConfig *config) { size_t i; + for (i = 0; i < config->nattributes; i++) { + virMediatedDeviceAttr *attr = config->attributes[i]; + virBufferAsprintf(buf, "<attr name='%s' value='%s'/>\n", + attr->name, attr->value); + } +} + +static void +virNodeDeviceCapMdevDefFormat(virBuffer *buf, + const virNodeDevCapData *data) +{ virBufferEscapeString(buf, "<type id='%s'/>\n", data->mdev.dev_config.type); virBufferEscapeString(buf, "<uuid>%s</uuid>\n", data->mdev.uuid); virBufferEscapeString(buf, "<parent_addr>%s</parent_addr>\n", @@ -597,11 +608,7 @@ virNodeDeviceCapMdevDefFormat(virBuffer *buf, virBufferAsprintf(buf, "<iommuGroup number='%u'/>\n", data->mdev.iommuGroupNumber); - for (i = 0; i < data->mdev.dev_config.nattributes; i++) { - virMediatedDeviceAttr *attr = data->mdev.dev_config.attributes[i]; - virBufferAsprintf(buf, "<attr name='%s' value='%s'/>\n", - attr->name, attr->value); - } + virNodeDeviceCapMdevAttrFormat(buf, &data->mdev.dev_config); } static void @@ -2188,7 +2195,7 @@ virNodeDevCapSystemParseXML(xmlXPathContextPtr ctxt, static int virNodeDevCapMdevAttributeParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, - virNodeDevCapMdev *mdev) + virMediatedDeviceConfig *config) { VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autoptr(virMediatedDeviceAttr) attr = virMediatedDeviceAttrNew(); @@ -2202,7 +2209,7 @@ virNodeDevCapMdevAttributeParseXML(xmlXPathContextPtr ctxt, return -1; } - VIR_APPEND_ELEMENT(mdev->dev_config.attributes, mdev->dev_config.nattributes, attr); + VIR_APPEND_ELEMENT(config->attributes, config->nattributes, attr); return 0; } @@ -2253,7 +2260,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, return -1; for (i = 0; i < nattrs; i++) - virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i], mdev); + virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i], &mdev->dev_config); return 0; } diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 1ee59d710b..0c8612eb71 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -599,27 +599,16 @@ nodeDeviceHasCapability(virNodeDeviceDef *def, virNodeDevCapType type) } -/* format a json string that provides configuration information about this mdev - * to the mdevctl utility */ static int -nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf) +nodeDeviceAttributesToJSON(virJSONValue *json, + const virMediatedDeviceConfig config)

{ size_t i; - virNodeDevCapMdev *mdev = &def->caps->data.mdev; - g_autoptr(virJSONValue) json = virJSONValueNewObject(); - const char *startval = mdev->autostart ? "auto" : "manual"; - - if (virJSONValueObjectAppendString(json, "mdev_type", mdev->dev_config.type) < 0) - return -1; - - if (virJSONValueObjectAppendString(json, "start", startval) < 0) - return -1; - - if (mdev->dev_config.attributes) { + if (config.attributes) { g_autoptr(virJSONValue) attributes = virJSONValueNewArray(); - for (i = 0; i < mdev->dev_config.nattributes; i++) { - virMediatedDeviceAttr *attr = mdev->dev_config.attributes[i]; + for (i = 0; i < config.nattributes; i++) { + virMediatedDeviceAttr *attr = config.attributes[i]; g_autoptr(virJSONValue) jsonattr = virJSONValueNewObject(); if (virJSONValueObjectAppendString(jsonattr, attr->name, attr->value) < 0) @@ -633,6 +622,28 @@ nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf) return -1; } + return 0; +} + + +/* format a json string that provides configuration information about this mdev + * to the mdevctl utility */ +static int +nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf) +{ + virNodeDevCapMdev *mdev = &def->caps->data.mdev; + g_autoptr(virJSONValue) json = virJSONValueNewObject(); + const char *startval = mdev->autostart ? "auto" : "manual"; + + if (virJSONValueObjectAppendString(json, "mdev_type", mdev->dev_config.type) < 0) + return -1; + + if (virJSONValueObjectAppendString(json, "start", startval) < 0) + return -1; + + if (nodeDeviceAttributesToJSON(json, mdev->dev_config) < 0) + return -1; + *buf = virJSONValueToString(json, false); if (!*buf) return -1; @@ -1092,6 +1103,39 @@ matchDeviceAddress(virNodeDeviceObj *obj, } +static int +nodeDeviceParseMdevctlAttribs(virMediatedDeviceConfig *config,

+ virJSONValue *attrs) +{ + size_t i; + + if (attrs && virJSONValueIsArray(attrs)) { + int nattrs = virJSONValueArraySize(attrs); + + config->attributes = g_new0(virMediatedDeviceAttr*, nattrs); + config->nattributes = nattrs; + + for (i = 0; i < nattrs; i++) { + virJSONValue *attr = virJSONValueArrayGet(attrs, i); + virMediatedDeviceAttr *attribute; + virJSONValue *value; + + if (!virJSONValueIsObject(attr) || + virJSONValueObjectKeysNumber(attr) != 1) + return -1; + + attribute = g_new0(virMediatedDeviceAttr, 1); + attribute->name = g_strdup(virJSONValueObjectGetKey(attr, 0)); + value = virJSONValueObjectGetValue(attr, 0); + attribute->value = g_strdup(virJSONValueGetString(value)); + config->attributes[i] = attribute; + } + } + + return 0; +} + + static virNodeDeviceDef* nodeDeviceParseMdevctlChildDevice(const char *parent, virJSONValue *json) @@ -1099,7 +1143,6 @@ nodeDeviceParseMdevctlChildDevice(const char *parent, virNodeDevCapMdev *mdev; const char *uuid; virJSONValue *props; - virJSONValue *attrs; g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1); virNodeDeviceObj *parent_obj; const char *start = NULL; @@ -1134,31 +1177,10 @@ nodeDeviceParseMdevctlChildDevice(const char *parent, start = virJSONValueObjectGetString(props, "start"); mdev->autostart = STREQ_NULLABLE(start, "auto"); - attrs = virJSONValueObjectGet(props, "attrs"); - - if (attrs && virJSONValueIsArray(attrs)) { - size_t i; - int nattrs = virJSONValueArraySize(attrs); - - mdev->dev_config.attributes = g_new0(virMediatedDeviceAttr*, nattrs); - mdev->dev_config.nattributes = nattrs; - - for (i = 0; i < nattrs; i++) { - virJSONValue *attr = virJSONValueArrayGet(attrs, i); - virMediatedDeviceAttr *attribute; - virJSONValue *value; - - if (!virJSONValueIsObject(attr) || - virJSONValueObjectKeysNumber(attr) != 1) - return NULL; + if (nodeDeviceParseMdevctlAttribs(&mdev->dev_config, + virJSONValueObjectGet(props, "attrs")) < 0) + return NULL; - attribute = g_new0(virMediatedDeviceAttr, 1); - attribute->name = g_strdup(virJSONValueObjectGetKey(attr, 0)); - value = virJSONValueObjectGetValue(attr, 0); - attribute->value = g_strdup(virJSONValueGetString(value)); - mdev->dev_config.attributes[i] = attribute; - } - } mdevGenerateDeviceName(child); return g_steal_pointer(&child); @@ -1760,7 +1782,9 @@ nodeDeviceUpdateMediatedDevices(void) } -/* returns true if any attributes were copied, else returns false */ +/* Transfer mediated device attributes to the new definition. Any change in + * the attributes is elminated but causes the return value to be true. + * Returns true if any attribute is copied, else returns false */