Re: [libvirt PATCH v3 01/11] nodedev: make iommuGroup optional for mdevs

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(a)redhat.com&gt; --- 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; + }