On Thu, May 19, 2016 at 11:37 PM, Laine Stump <la...@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 <sb...@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@redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list >> >> >
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list