On 6/16/20 4:27 PM, Jonathon Jongsma wrote:
When parsing a nodedev xml file, the iommuGroup element should be
optional. This element should be read-only and is determined by the
device driver. While this is a change to existing behavior, it doesn't
break backwards-compatibility because it makes the parser less strict.
Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
docs/formatnode.html.in | 5 +++--
docs/schemas/nodedev.rng | 12 +++++++-----
src/conf/node_device_conf.c | 12 ++++++------
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 76eae928de..4ed43ec0cb 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -390,8 +390,9 @@
<dt><code>iommuGroup</code></dt>
<dd>
This element supports a single attribute <code>number</code>
- which holds the IOMMU group number the mediated device belongs
- to.
+ which holds the IOMMU group number to which the mediated device
+ belongs. This is a read-only field that is reported by the
+ device driver.
</dd>
</dl>
</dd>
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index fe6ffa0b53..ca3a79db87 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -629,11 +629,13 @@
<data type='string'/>
</attribute>
</element>
- <element name='iommuGroup'>
- <attribute name='number'>
- <ref name='unsignedInt'/>
- </attribute>
- </element>
+ <optional>
+ <element name='iommuGroup'>
+ <attribute name='number'>
+ <ref name='unsignedInt'/>
+ </attribute>
+ </element>
+ </optional>
</define>
<define name='capccwdev'>
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index bccdbd0af8..4b3b04b7b8 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1775,13 +1775,13 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
goto out;
}
- if (virNodeDevCapsDefParseULong("number(./iommuGroup[1]/@number)", ctxt,
- &mdev->iommuGroupNumber, def,
- _("missing iommuGroup number attribute for "
- "'%s'"),
- _("invalid iommuGroup number attribute for "
- "'%s'")) < 0)
+ /* 'iommuGroup' is optional, only report an error if the supplied value is
+ * invalid (-2), not if it's missing (-1) */
+ if (virXPathUInt("number(./iommuGroup[1]/@number)", ctxt, &mdev->iommuGroupNumber) < -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid iommuGroup number attribute for '%s'"), def->name);
goto out;
+ }
I'd prefer these long lines to be split. For instance:
iff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4b3b04b7b8..2ef4514f05 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1777,9 +1777,11 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
/* 'iommuGroup' is optional, only report an error if the supplied
value is
* invalid (-2), not if it's missing (-1) */
- if (virXPathUInt("number(./iommuGroup[1]/@number)", ctxt,
&mdev->iommuGroupNumber) < -1) {
+ if (virXPathUInt("number(./iommuGroup[1]/@number)",
+ ctxt, &mdev->iommuGroupNumber) < -1) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("invalid iommuGroup number attribute for
'%s'"), def->name);
+ _("invalid iommuGroup number attribute for '%s'"),
+ def->name);
goto out;
}
Michal
