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(a)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).
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(a)redhat.com
>
https://www.redhat.com/mailman/listinfo/libvir-list
>
>