[libvirt] [PATCH v2 4/8] Introduce PCI Multifunction device parser
Shivaprasad bhat
shivaprasadbhat at gmail.com
Mon May 23 10:24:13 UTC 2016
On Thu, May 19, 2016 at 11:37 PM, Laine Stump <laine at laine.org> wrote:
> 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")
Yes. I meant emulated. I am currently disallowing mixing hostdevices with
emulated as some drivers might
expect themselves to be on function 0.
>
>
>
>
>> 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.
Fixing it.
>
>
>
> +
>> /**
>> * 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).
Agree. I am doing it.
>
>
> + 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160523/651747c6/attachment-0001.htm>
More information about the libvir-list
mailing list