Refactor attribute handling code into methods for easier reuse.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
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 */
static bool
virMediatedDeviceAttrsCopy(virMediatedDeviceConfig *dst_config,
virMediatedDeviceConfig *src_config)
--
2.42.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On 1/19/24 10:38 AM, Boris Fiuczynski wrote: > Refactor attribute handling code into methods for easier reuse. > > Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> > --- > 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) I don't see any compelling reason to pass this config struct by value. Just pass a pointer. > { > 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, As far as I can tell, we don't use the abbreviation "attribs" anywhere else in libvirt, but we do use both 'Attributes' and 'Attrs'. For consistency, I'd suggest just using the full word: nodeDeviceParseMdevctlAttributes() > + 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 */ This change doesn't really seem to belong in this commit as there are no changes to this function. Also, I don't understand the comment. I suppose "elminated" is supposed to be "eliminated", but I still can't quite understand what it's trying to say. Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 1/31/24 22:03, Jonathon Jongsma wrote: > On 1/19/24 10:38 AM, Boris Fiuczynski wrote: >> Refactor attribute handling code into methods for easier reuse. >> >> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> >> --- >> 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) > > I don't see any compelling reason to pass this config struct by value. > Just pass a pointer. Changed. > >> { >> 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, > > As far as I can tell, we don't use the abbreviation "attribs" anywhere > else in libvirt, but we do use both 'Attributes' and 'Attrs'. For > consistency, I'd suggest just using the full word: > nodeDeviceParseMdevctlAttributes() Accepted and changed. > >> + 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 */ > > This change doesn't really seem to belong in this commit as there are no > changes to this function. Also, I don't understand the comment. I > suppose "elminated" is supposed to be "eliminated", but I still can't > quite understand what it's trying to say. I will remove this change. What I meant to express was that this method is not a simple copy but a surgical copy which eliminates all differences (above called "any change") in the provided destination config. > > Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> > _______________________________________________ > Devel mailing list -- devel@lists.libvirt.org > To unsubscribe send an email to devel-leave@lists.libvirt.org -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
© 2016 - 2024 Red Hat, Inc.