[libvirt] [PATCH v2 4/8] Introduce PCI Multifunction device parser

Laine Stump laine at laine.org
Thu May 19 18:07:00 UTC 2016


On 05/18/2016 05:31 PM, Shivaprasad G Bhat wrote:
> This patch just introduces the parser function used by
> the later patches. The parser disallows hostdevices to be
> used with other virtio devices simultaneously.

Why? (and I think you mean "emulated" not "virtio")


>
> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.vnet.ibm.com>
> ---
>   src/conf/domain_conf.c   |  236 ++++++++++++++++++++++++++++++++++++++++++++++
>   src/conf/domain_conf.h   |   22 ++++
>   src/libvirt_private.syms |    3 +
>   3 files changed, 261 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ed0c471..e946147 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -860,6 +860,36 @@ virDomainXMLOptionClassDispose(void *obj)
>           (xmlopt->config.privFree)(xmlopt->config.priv);
>   }
>   
> +/* virDomainDeviceDefListAddCopy - add a *copy* of the device to this list */
> +int
> +virDomainDeviceDefListAddCopy(virDomainDeviceDefListPtr list,
> +                              virDomainDeviceDefPtr dev,
> +                              const virDomainDef *def,
> +                              virCapsPtr caps,
> +                              virDomainXMLOptionPtr xmlopt)
> +{
> +    virDomainDeviceDefPtr copy = virDomainDeviceDefCopy(dev, def, caps, xmlopt);
> +
> +    if (!copy)
> +        return -1;
> +    if (VIR_APPEND_ELEMENT(list->devs, list->count, copy) < 0) {
> +        virDomainDeviceDefFree(copy);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +void virDomainDeviceDefListDispose(virDomainDeviceDefListPtr list)
> +{
> +    size_t i;
> +
> +    if (!list)
> +        return;
> +    for (i = 0; i < list->count; i++)
> +        virDomainDeviceDefFree(list->devs[i]);
> +    VIR_FREE(list);
> +}


This isn't a vir*Dispose() function, it is a vir*Free() function. the 
Dispose() functions are used for virObject-based objects, and take a 
void *obj as their argument.


> +
>   /**
>    * virDomainKeyWrapCipherDefParseXML:
>    *
> @@ -24365,3 +24395,209 @@ virDomainObjGetShortName(virDomainObjPtr vm)
>   
>       return ret;
>   }
> +
> +static int
> +virDomainPCIMultifunctionDeviceDefParseXML(xmlXPathContextPtr ctxt,
> +                                           const virDomainDef *def,
> +                                           virDomainDeviceDefListPtr devlist,
> +                                           virCapsPtr caps,
> +                                           virDomainXMLOptionPtr xmlopt,
> +                                           unsigned int flags)
> +{

Instead of all these loops for each type of device. I think it would 
make more sense to separate virDomainDeviceDefParse() so that the 
original function was:

      virDomainDeviceDefParse(....)
      {
           ...
           if (!(xml = virXMLParseString(......)))
               return -1;
           node = ctxt->node;

          dev = virDomainDeviceDefParseXML(node, ctxt, def, caps, 
xmlopt, flags)))
          xmlFreeDoc(xml)
          xmlXPathFreeContext(ctxt);
          return dev;
     }

with a new function virDomainDeviceDefParseXML() that contained all the 
rest of the insides of the original function. You could then call this 
new function for each node of the xmlDocPtr that you get after parsing 
the input to your new "parse multiple devices" function (which could 
maybe be called virDomainDeviceDefParseMany()). After all of the devices 
are parsed, then you can validate that they are all PCI devices, and 
each for a different function of the same slot (if an address has been 
assigned).

> +    size_t i, j;
> +    int n;
> +    virDomainDeviceDef device;
> +    xmlNodePtr *nodes = NULL;
> +    char *netprefix = NULL;
> +    int nhostdevs = 0;
> +
> +    device.type = VIR_DOMAIN_DEVICE_DISK;
> +
> +    if ((n = virXPathNodeSet("./disk", ctxt, &nodes)) < 0)
> +        goto error;
> +
> +    for (i = 0; i < n; i++) {
> +        virDomainDiskDefPtr disk = virDomainDiskDefParseXML(xmlopt,
> +                                                            nodes[i],
> +                                                            ctxt,
> +                                                            NULL,
> +                                                            def->seclabels,
> +                                                            def->nseclabels,
> +                                                            flags);
> +        if (!disk)
> +            goto error;
> +
> +        device.data.disk = disk;
> +        if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0)
> +            goto error;
> +        VIR_FREE(disk);
> +    }
> +    VIR_FREE(nodes);
> +
> +    device.type = VIR_DOMAIN_DEVICE_CONTROLLER;
> +    /* analysis of the controller devices */
> +    if ((n = virXPathNodeSet("./controller", ctxt, &nodes)) < 0)
> +        goto error;
> +
> +    for (i = 0; i < n; i++) {
> +        virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i],
> +                                                                              ctxt,
> +                                                                              flags);
> +        if (!controller)
> +            goto error;
> +
> +        device.data.controller = controller;
> +        if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0)
> +            goto error;
> +        VIR_FREE(controller);
> +    }
> +    VIR_FREE(nodes);
> +
> +    device.type = VIR_DOMAIN_DEVICE_NET;
> +    if ((n = virXPathNodeSet("./interface", ctxt, &nodes)) < 0)
> +        goto error;
> +
> +    netprefix = caps->host.netprefix;
> +    for (i = 0; i < n; i++) {
> +        virDomainNetDefPtr net = virDomainNetDefParseXML(xmlopt,
> +                                                         nodes[i],
> +                                                         ctxt,
> +                                                         NULL,
> +                                                         netprefix,
> +                                                         flags);
> +        if (!net)
> +            goto error;
> +
> +        device.data.net = net;
> +        if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0)
> +            goto error;
> +        VIR_FREE(net);
> +    }
> +    VIR_FREE(nodes);
> +
> +    /* analysis of the host devices */
> +    device.type = VIR_DOMAIN_DEVICE_HOSTDEV;
> +    if ((nhostdevs = virXPathNodeSet("./hostdev", ctxt, &nodes)) < 0)
> +        goto error;
> +    if (nhostdevs && devlist->count)
> +        goto misconfig;
> +    for (i = 0; i < nhostdevs; i++) {
> +        virDomainHostdevDefPtr hostdev;
> +
> +        hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt,
> +                                              NULL, flags);
> +        if (!hostdev)
> +            goto error;
> +
> +        if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB ||
> +            hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Can't add host USB device: "
> +                             "USB is disabled in this host"));
> +            virDomainHostdevDefFree(hostdev);
> +            goto error;
> +        }
> +        device.data.hostdev = hostdev;
> +        for (j = 0; j < devlist->count; j++) {
> +            if (virDomainHostdevMatch(hostdev, devlist->devs[j]->data.hostdev)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Duplicate host devices in list"));
> +                goto error;
> +            }
> +        }
> +        if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0)
> +            goto error;
> +        VIR_FREE(hostdev);
> +    }
> +    VIR_FREE(nodes);
> +
> +    /* Parse the RNG devices */
> +    device.type = VIR_DOMAIN_DEVICE_RNG;
> +    if ((n = virXPathNodeSet("./rng", ctxt, &nodes)) < 0)
> +        goto error;
> +    for (i = 0; i < n; i++) {
> +        virDomainRNGDefPtr rng = virDomainRNGDefParseXML(nodes[i],
> +                                                         ctxt,
> +                                                         flags);
> +        if (!rng)
> +            goto error;
> +
> +        device.data.rng = rng;
> +        if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0)
> +            goto error;
> +        VIR_FREE(rng);
> +    }
> +    VIR_FREE(nodes);
> +
> +    device.type = VIR_DOMAIN_DEVICE_CHR;
> +    if ((n = virXPathNodeSet("./serial", ctxt, &nodes)) < 0)
> +        goto error;
> +
> +    for (i = 0; i < n; i++) {
> +        virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
> +                                                         nodes[i],
> +                                                         def->seclabels,
> +                                                         def->nseclabels,
> +                                                         flags);
> +        if (!chr)
> +            goto error;
> +
> +        if (chr->target.port == -1) {
> +            int maxport = -1;
> +            for (j = 0; j < i; j++) {
> +                if (def->serials[j]->target.port > maxport)
> +                    maxport = def->serials[j]->target.port;
> +            }
> +            chr->target.port = maxport + 1;
> +        }
> +        device.data.chr = chr;
> +        if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0)
> +            goto error;
> +        VIR_FREE(chr);
> +    }
> +    VIR_FREE(nodes);
> +
> +    if (nhostdevs  && nhostdevs != devlist->count)
> +        goto misconfig;
> +
> +    return 0;
> + misconfig:
> +    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                   _("Shouldn't mix host devices with other devices"));
> + error:
> +    return -1;
> +}
> +
> +
> +int
> +virDomainPCIMultifunctionDeviceDefParseNode(const char *xml,
> +                        const virDomainDef *def,
> +                        virDomainDeviceDefListPtr devlist,
> +                        virCapsPtr caps,
> +                        virDomainXMLOptionPtr xmlopt,
> +                        unsigned int flags)
> +{
> +    xmlXPathContextPtr ctxt = NULL;
> +    int ret = -1;
> +
> +    xmlDocPtr xmlptr;
> +
> +    if (!(xmlptr = virXMLParse(NULL, xml, _("(device_definition)")))) {
> +        /* We report error anyway later */
> +        return -1;
> +    }
> +
> +    ctxt = xmlXPathNewContext(xmlptr);
> +    if (ctxt == NULL) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    ctxt->node =  xmlDocGetRootElement(xmlptr);
> +    ret = virDomainPCIMultifunctionDeviceDefParseXML(ctxt, def, devlist,
> +                                                     caps, xmlopt, flags);
> +
> + cleanup:
> +    xmlXPathFreeContext(ctxt);
> +    return ret;
> +}
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index b9e696d..9ddfbd1 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2462,6 +2462,20 @@ typedef enum {
>   typedef struct _virDomainXMLOption virDomainXMLOption;
>   typedef virDomainXMLOption *virDomainXMLOptionPtr;
>   
> +struct virDomainDeviceDefList {
> +    virDomainDeviceDefPtr *devs;
> +    size_t count;
> +};
> +typedef struct virDomainDeviceDefList *virDomainDeviceDefListPtr;
> +
> +int
> +virDomainDeviceDefListAddCopy(virDomainDeviceDefListPtr list, virDomainDeviceDefPtr dev,
> +                              const virDomainDef *def,
> +                              virCapsPtr caps,
> +                              virDomainXMLOptionPtr xmlopt);
> +void virDomainDeviceDefListDispose(virDomainDeviceDefListPtr list);
> +
> +
>   /* Called once after everything else has been parsed, for adjusting
>    * overall domain defaults.  */
>   typedef int (*virDomainDefPostParseCallback)(virDomainDefPtr def,
> @@ -3176,6 +3190,14 @@ int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>   
>   bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1);
>   
> +int
> +virDomainPCIMultifunctionDeviceDefParseNode(const char *xml,
> +                        const virDomainDef *def,
> +                        virDomainDeviceDefListPtr devlist,
> +                        virCapsPtr caps,
> +                        virDomainXMLOptionPtr xmlopt,
> +                        unsigned int flags);
> +
>   char *virDomainObjGetShortName(virDomainObjPtr vm);
>   
>   #endif /* __DOMAIN_CONF_H */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e4953b7..d6a60b5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow;
>   virDomainPCIAddressSlotInUse;
>   virDomainPCIAddressValidate;
>   virDomainPCIControllerModelToConnectType;
> +virDomainPCIMultifunctionDeviceDefParseNode;
>   virDomainVirtioSerialAddrAssign;
>   virDomainVirtioSerialAddrAutoAssign;
>   virDomainVirtioSerialAddrIsComplete;
> @@ -249,6 +250,8 @@ virDomainDeviceAddressIsValid;
>   virDomainDeviceAddressTypeToString;
>   virDomainDeviceDefCopy;
>   virDomainDeviceDefFree;
> +virDomainDeviceDefListAddCopy;
> +virDomainDeviceDefListDispose;
>   virDomainDeviceDefParse;
>   virDomainDeviceFindControllerModel;
>   virDomainDeviceGetInfo;
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>




More information about the libvir-list mailing list