[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 4/6] nodedev: Introduce the mdev capability to a PCI parent device



On Wed, Apr 26, 2017 at 04:55:31PM +0200, Erik Skultety wrote:
> The parent device needs to report the generic stuff about the supported
> mediated devices types, like device API, available instances, type name,
> etc. Therefore this patch introduces a new nested capability element of
> type 'mdev_types' with the resulting XML of the following format:
> 
> <device>
> ...
>   <capability type='pci'>
>   ...
>     <capability type='mdev_types'>
>       <type id='vendor_supplied_id'>
>         <name>optional_vendor_supplied_codename</name>
>         <deviceAPI>vfio-pci</deviceAPI>
>         <availableInstances>NUM</availableInstances>
>       </type>
>       ...
>       <type>
>       ...

It's a commit message so it doesn't matter but I would indent the
place-holder dots :).

>       </type>
>     </capability>
>   </capability>
> ...
> </device>
> 
> Signed-off-by: Erik Skultety <eskultet redhat com>
> ---
>  docs/schemas/nodedev.rng                           |  26 +++++
>  src/conf/node_device_conf.c                        | 104 ++++++++++++++++++
>  src/conf/node_device_conf.h                        |  15 +++
>  src/conf/virnodedeviceobj.c                        |   7 ++
>  src/libvirt_private.syms                           |   1 +
>  src/node_device/node_device_udev.c                 | 119 +++++++++++++++++++++
>  .../pci_0000_02_10_7_mdev_types.xml                |  32 ++++++
>  tests/nodedevxml2xmltest.c                         |   1 +
>  8 files changed, 305 insertions(+)
>  create mode 100644 tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml
> 
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 0f90a73c8..e0a2c5032 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -205,6 +205,32 @@
>      </optional>
>  
>      <optional>
> +      <element name='capability'>
> +        <attribute name='type'>
> +          <value>mdev_types</value>
> +        </attribute>
> +        <oneOrMore>
> +          <element name='type'>
> +            <attribute name='id'>
> +              <data type='string'/>
> +            </attribute>
> +            <optional>
> +              <element name='name'><text/></element>
> +            </optional>
> +            <element name='deviceAPI'>
> +              <choice>
> +                <value>vfio-pci</value>
> +              </choice>
> +            </element>
> +            <element name='availableInstances'>
> +              <ref name='unsignedInt'/>
> +            </element>
> +          </element>
> +        </oneOrMore>
> +      </element>
> +   </optional>
> +
> +    <optional>
>        <element name='iommuGroup'>
>          <attribute name='number'>
>            <ref name='unsignedInt'/>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 24cb6d66f..b3012821a 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -88,6 +88,26 @@ virNodeDevCapsDefParseString(const char *xpath,
>  }
>  
>  
> +static void
> +virNodeDevCapMdevTypeClear(virNodeDevCapMdevTypePtr type)
> +{
> +    VIR_FREE(type->id);
> +    VIR_FREE(type->name);
> +    VIR_FREE(type->device_api);
> +}

There is no need for the extra Clear function since it's static
and used only by the Free function.

> +
> +
> +void
> +virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type)
> +{
> +    if (!type)
> +        return;
> +
> +    virNodeDevCapMdevTypeClear(type);
> +    VIR_FREE(type);
> +}
> +
> +
>  void
>  virNodeDeviceDefFree(virNodeDeviceDefPtr def)
>  {
> @@ -265,6 +285,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
>          virBufferAsprintf(buf, "<capability type='%s'/>\n",
>                            virPCIHeaderTypeToString(data->pci_dev.hdrType));
>      }
> +    if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
> +        virBufferAddLit(buf, "<capability type='mdev_types'>\n");
> +        virBufferAdjustIndent(buf, 2);
> +        for (i = 0; i < data->pci_dev.nmdev_types; i++) {
> +            virNodeDevCapMdevTypePtr type = data->pci_dev.mdev_types[i];
> +            virBufferEscapeString(buf, "<type id='%s'>\n", type->id);
> +            virBufferAdjustIndent(buf, 2);
> +            if (type->name)
> +                virBufferAsprintf(buf, "<name>%s</name>\n",
> +                                  type->name);
> +            virBufferAsprintf(buf, "<deviceAPI>%s</deviceAPI>\n",
> +                              type->device_api);

We should use virBufferEscapeString for <name> and <deviceAPI> as well,
the data stored in these variables are loaded from sysfs.

> +            virBufferAsprintf(buf,
> +                              "<availableInstances>%u</availableInstances>\n",
> +                              type->available_instances);
> +            virBufferAdjustIndent(buf, -2);
> +            virBufferAddLit(buf, "</type>\n");
> +        }
> +        virBufferAdjustIndent(buf, -2);
> +        virBufferAddLit(buf, "</capability>\n");
> +    }
>      if (data->pci_dev.nIommuGroupDevices) {
>          virBufferAsprintf(buf, "<iommuGroup number='%d'>\n",
>                            data->pci_dev.iommuGroupNumber);
> @@ -1365,6 +1406,63 @@ virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr ctxt,
>  
>  
>  static int
> +virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt,
> +                                  virNodeDevCapPCIDevPtr pci_dev)
> +{
> +    int ret = -1;
> +    xmlNodePtr orignode = NULL;
> +    xmlNodePtr *nodes = NULL;
> +    int nmdev_types = virXPathNodeSet("./type", ctxt, &nodes);
> +    virNodeDevCapMdevTypePtr type = NULL;
> +    size_t i;
> +
> +    orignode = ctxt->node;
> +    for (i = 0; i < nmdev_types; i++) {
> +        ctxt->node = nodes[i];
> +
> +        if (VIR_ALLOC(type) < 0)
> +            goto cleanup;
> +
> +        if (!(type->id = virXPathString("string(./@id[1])", ctxt))) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("missing 'id' attribute for mediated device's "
> +                             "<type> element"));
> +            goto cleanup;
> +        }
> +
> +        if (!(type->device_api = virXPathString("string(./deviceAPI[1])", ctxt))) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("missing device API for mediated device type '%s'"),
> +                           type->id);
> +            goto cleanup;
> +        }
> +
> +        if (virXPathUInt("number(./availableInstances)", ctxt,
> +                         &type->available_instances) < 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("missing number of available instances for "
> +                             "mediated device type '%s'"),
> +                           type->id);
> +            goto cleanup;
> +        }
> +
> +        type->name = virXPathString("string(./name)", ctxt);
> +
> +        if (VIR_APPEND_ELEMENT(pci_dev->mdev_types,
> +                               pci_dev->nmdev_types, type) < 0)
> +            goto cleanup;
> +    }
> +
> +    pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV;
> +    ret = 0;
> + cleanup:
> +    virNodeDevCapMdevTypeFree(type);
> +    ctxt->node = orignode;
> +    return ret;
> +}
> +
> +
> +static int
>  virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt,
>                                  xmlNodePtr node,
>                                  virNodeDevCapPCIDevPtr pci_dev)
> @@ -1386,6 +1484,9 @@ virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt,
>      } else if (STREQ(type, "virt_functions") &&
>                 virNodeDevPCICapSRIOVVirtualParseXML(ctxt, pci_dev) < 0) {
>          goto cleanup;
> +    } if (STREQ(type, "mdev_types") &&

s/if/else if/

> +          virNodeDevPCICapMdevTypesParseXML(ctxt, pci_dev)) {

there should be " < 0 " comparison.

> +          goto cleanup;
>      } else {
>          int hdrType = virPCIHeaderTypeFromString(type);
>  

Pavel

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]