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

Michal Privoznik mprivozn at redhat.com
Tue Jun 16 18:41:38 UTC 2020


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 at redhat.com>
> ---
>   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




More information about the libvir-list mailing list