<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, May 19, 2016 at 11:37 PM, Laine Stump <<a href="mailto:laine@laine.org" target="_blank">laine@laine.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 05/18/2016 05:31 PM, Shivaprasad G Bhat wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> This patch just introduces the parser function used by the later patches. The parser disallows hostdevices to be used with other virtio devices simultaneously. </blockquote> Why? (and I think you mean "emulated" not "virtio")</blockquote><div> </div><div>Yes. I meant emulated. I am currently disallowing mixing hostdevices with emulated as some drivers might</div><div>expect themselves to be on function 0. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Shivaprasad G Bhat <<a href="mailto:sbhat@linux.vnet.ibm.com" target="_blank">sbhat@linux.vnet.ibm.com</a>> --- 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); +} </blockquote> </div></div> 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.</blockquote><div> </div><div>Fixing it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + /** * 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) +{ </blockquote> 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).</blockquote><div> </div><div>Agree. I am doing it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + 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; + + <a href="http://device.data.net" rel="noreferrer" target="_blank">device.data.net</a> = 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 <a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a> </blockquote> </div></div></blockquote></div> </div></div>