The commit 6fe678c is partly reverted. The code is moved around and cant revert staright. Doing this at function level is necessary because some devices may not be removed by guest or may take time and new hotplug shouldn't assume the slot is free just because one of the function was freed by the guest. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/conf/domain_addr.c | 22 +++++++++------------- src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 2 +- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index acd8ce6..35c7cd4 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -472,21 +472,17 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - /* We do not support hotplug multi-function PCI device now, so we should - * reserve the whole slot. The function of the PCI device must be 0. - */ - if (dev->addr.pci.function != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with function=0" - " are supported")); - goto cleanup; - } + if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)) || + dev->addr.pci.function != 0) { - if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, - addrStr, flags, true)) - goto cleanup; + if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, + addrStr, flags, true)) + goto cleanup; - ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, flags, false, true); + } else { + ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + } } else { ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fb24808..e4953b7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -98,6 +98,7 @@ virDomainPCIAddressBusSetModel; virDomainPCIAddressEnsureAddr; virDomainPCIAddressFlagsCompatible; virDomainPCIAddressGetNextSlot; +virDomainPCIAddressReleaseAddr; virDomainPCIAddressReleaseSlot; virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextSlot; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 9c8c262..1e7d98c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1682,7 +1682,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, NULLSTR(devstr)); else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - virDomainPCIAddressReleaseSlot(priv->pciaddrs, + virDomainPCIAddressReleaseAddr(priv->pciaddrs, &info->addr.pci) < 0) VIR_WARN("Unable to release PCI address on %s", NULLSTR(devstr));

This patch just introduces the parser function used by the later patches. The parser disallows hostdevices to be used with other virtio devices simultaneously. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/conf/domain_conf.c | 123 ++++++++++++++++++++++++++++++++++++++-------- src/conf/domain_conf.h | 19 +++++++ src/libvirt_private.syms | 3 + 3 files changed, 124 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed0c471..873a309 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 virDomainDeviceDefListFree(virDomainDeviceDefListPtr list) +{ + size_t i; + + if (!list) + return; + for (i = 0; i < list->count; i++) + virDomainDeviceDefFree(list->devs[i]); + VIR_FREE(list); +} + /** * virDomainKeyWrapCipherDefParseXML: * @@ -12880,25 +12910,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode, return NULL; } - -virDomainDeviceDefPtr -virDomainDeviceDefParse(const char *xmlStr, - const virDomainDef *def, - virCapsPtr caps, - virDomainXMLOptionPtr xmlopt, - unsigned int flags) +static +virDomainDeviceDefPtr virDomainDeviceDefParseXML(xmlNodePtr node, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt, + unsigned int flags) { - xmlDocPtr xml; - xmlNodePtr node; - xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; char *netprefix; - - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) - goto error; - - node = ctxt->node; - if (VIR_ALLOC(dev) < 0) goto error; @@ -13031,14 +13052,33 @@ virDomainDeviceDefParse(const char *xmlStr, if (virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt) < 0) goto error; - cleanup: + return dev; + error: + return NULL; +} + +virDomainDeviceDefPtr +virDomainDeviceDefParse(const char *xmlStr, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + xmlDocPtr xml; + xmlNodePtr node; + xmlXPathContextPtr ctxt = NULL; + virDomainDeviceDefPtr dev = NULL; + + if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) + return NULL; + + node = ctxt->node; + + dev = virDomainDeviceDefParseXML(node, def, caps, xmlopt, ctxt, flags); + xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return dev; - - error: - VIR_FREE(dev); - goto cleanup; } @@ -24365,3 +24405,44 @@ virDomainObjGetShortName(virDomainObjPtr vm) return ret; } + + +virDomainDeviceDefListPtr +virDomainDeviceDefParseXMLMany(const char *xml, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + xmlXPathContextPtr ctxt = NULL; + xmlDocPtr xmlPtr; + xmlNodePtr node, root; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist; + + if (!(xmlPtr = virXMLParseStringCtxt(xml, _("(device_definition)"), &ctxt))) + return NULL; + + if (VIR_ALLOC(devlist) < 0) + goto exit; + + root = xmlDocGetRootElement(xmlPtr); + node = root->children; + while (node) { + if (node->type == XML_ELEMENT_NODE) { + dev = virDomainDeviceDefParseXML(node, def, caps, xmlopt, ctxt, flags); + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) { + virDomainDeviceDefFree(dev); + virDomainDeviceDefListFree(devlist); + goto exit; + } + dev = NULL; + } + node = node->next; + } + + exit: + xmlFreeDoc(xmlPtr); + xmlXPathFreeContext(ctxt); + return devlist; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9e696d..ed188f1 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 virDomainDeviceDefListFree(virDomainDeviceDefListPtr list); + + /* Called once after everything else has been parsed, for adjusting * overall domain defaults. */ typedef int (*virDomainDefPostParseCallback)(virDomainDefPtr def, @@ -2717,6 +2731,11 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags); +virDomainDeviceDefListPtr virDomainDeviceDefParseXMLMany(const char *xmlStr, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); virStorageSourcePtr virDomainDiskDefSourceParse(const char *xmlStr, const virDomainDef *def, virDomainXMLOptionPtr xmlopt, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4953b7..0e8a887 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -249,7 +249,10 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; virDomainDeviceDefCopy; virDomainDeviceDefFree; +virDomainDeviceDefListAddCopy; +virDomainDeviceDefListFree; virDomainDeviceDefParse; +virDomainDeviceDefParseXMLMany; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo; virDomainDeviceInfoCopy;

This patch assigns multifunction pci addresses to devices in the devlist. The pciaddrs passed in the argument is not altered so that the actual call to reserve the address using virDomainPCIAddressEnsureAddr() passes. The function focuses on user given address validation and also the auto-assign of addresses. The address auto-assignment works well for PPC64 and x86-i440fx machines. The q35 machines needs some level of logic here to get the correct next valid slot so that the hotplug go through fine. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/conf/domain_addr.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 4 ++ src/libvirt_private.syms | 1 3 files changed, 118 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 7ea9e4d..c171067 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -455,6 +455,119 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, } int +virDomainPCIMultifunctionDeviceAddressValidateAssign(virDomainPCIAddressSetPtr addrs, virDomainDeviceDefListPtr devlist) +{ + size_t i; + char *addrStr = NULL; + virPCIDeviceAddress addr; + virDomainPCIAddressBusPtr bus; + uint8_t slot; + virDomainDeviceInfoPtr info = NULL, privinfo = NULL; + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; + + if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0) + return -1; + + bus = &addrs->buses[addr.bus]; + slot = bus->slots[addr.slot]; + + for (i = 0; i < devlist->count; i++) { + virDomainDeviceDefPtr dev = devlist->devs[i]; + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + info = &dev->data.disk->info; + break; + case VIR_DOMAIN_DEVICE_NET: + info = &dev->data.net->info; + break; + case VIR_DOMAIN_DEVICE_RNG: + info = &dev->data.rng->info; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + info = dev->data.hostdev->info; + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + info = &dev->data.controller->info; + break; + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Only PCI devices are allowed")); + break; + } + + if (i == 0 && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* User has given an address in xml use that slot + * and bus instead */ + bus = &addrs->buses[info->addr.pci.bus]; + slot = bus->slots[info->addr.pci.slot]; + if (slot & 0xFF) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Slot not free")); + goto error; + } + } + + if (privinfo) { + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (privinfo->addr.pci.bus != info->addr.pci.bus || + privinfo->addr.pci.slot != info->addr.pci.slot) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Multifunction PCI device " + "cant be split across different guest PCI slots")); + return -1; + } + } + } + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + info->addr.pci = addr; + info->addr.pci.function = devlist->count - i - 1; + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + if (info->addr.pci.function == 0) + info->addr.pci.multi = VIR_TRISTATE_SWITCH_ON; + } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (info->addr.pci.function != devlist->count - i - 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("PCI function numbers to be in decreasing order")); + goto error; + } + } + + if (!(addrStr = virDomainPCIAddressAsString(&info->addr.pci))) + goto error; + + if (!virDomainPCIAddressValidate(addrs, &info->addr.pci, + addrStr, flags, true)) + goto error; + + privinfo = info; + } + + return 0; + error: + return -1; +} + + +int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f3eda89..1e810b0 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -157,6 +157,10 @@ int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virDomainPCIMultifunctionDeviceAddressValidateAssign(virDomainPCIAddressSetPtr addrs, + virDomainDeviceDefListPtr devlist); + struct _virDomainCCWAddressSet { virHashTablePtr defined; virDomainDeviceCCWAddress next; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0e8a887..9262859 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; +virDomainPCIMultifunctionDeviceAddressValidateAssign; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete;

No functional change. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_domain.c | 401 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 17 ++ src/qemu/qemu_driver.c | 401 ------------------------------------------------ 3 files changed, 418 insertions(+), 401 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b0eb3b6..e1d3824 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -30,6 +30,7 @@ #include "qemu_parse_command.h" #include "qemu_capabilities.h" #include "qemu_migration.h" +#include "qemu_hotplug.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -5352,3 +5353,403 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src) return 0; } + +int +qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virConnectPtr conn) +{ + virDomainDiskDefPtr disk; + virDomainNetDefPtr net; + virDomainHostdevDefPtr hostdev; + virDomainLeaseDefPtr lease; + virDomainControllerDefPtr controller; + virDomainFSDefPtr fs; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("target %s already exists"), disk->dst); + return -1; + } + if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + return -1; + if (qemuCheckDiskConfig(disk) < 0) + return -1; + if (virDomainDiskInsert(vmdef, disk)) + return -1; + /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ + dev->data.disk = NULL; + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) + if (virDomainDefAddImplicitDevices(vmdef) < 0) + return -1; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if (virDomainNetInsert(vmdef, net)) + return -1; + dev->data.net = NULL; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + hostdev = dev->data.hostdev; + if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device is already in the domain configuration")); + return -1; + } + if (virDomainHostdevInsert(vmdef, hostdev)) + return -1; + dev->data.hostdev = NULL; + if (virDomainDefAddImplicitDevices(vmdef) < 0) + return -1; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_LEASE: + lease = dev->data.lease; + if (virDomainLeaseIndex(vmdef, lease) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Lease %s in lockspace %s already exists"), + lease->key, NULLSTR(lease->lockspace)); + return -1; + } + if (virDomainLeaseInsert(vmdef, lease) < 0) + return -1; + + /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ + dev->data.lease = NULL; + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + controller = dev->data.controller; + if (virDomainControllerFind(vmdef, controller->type, + controller->idx) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Target already exists")); + return -1; + } + + if (virDomainControllerInsert(vmdef, controller) < 0) + return -1; + dev->data.controller = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_CHR: + if (qemuDomainChrInsert(vmdef, dev->data.chr) < 0) + return -1; + dev->data.chr = NULL; + if (virDomainDefAddImplicitDevices(vmdef) < 0) + return -1; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_FS: + fs = dev->data.fs; + if (virDomainFSIndexByName(vmdef, fs->dst) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Target already exists")); + return -1; + } + + if (virDomainFSInsert(vmdef, fs) < 0) + return -1; + dev->data.fs = NULL; + break; + + case VIR_DOMAIN_DEVICE_RNG: + if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + virDomainDefHasDeviceAddress(vmdef, &dev->data.rng->info)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("a device with the same address already exists ")); + return -1; + } + + if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) + return -1; + dev->data.rng = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + if (vmdef->nmems == vmdef->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no free memory device slot available")); + return -1; + } + + if (vmdef->mem.cur_balloon == virDomainDefGetMemoryActual(vmdef)) + vmdef->mem.cur_balloon += dev->data.memory->size; + + if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) + return -1; + dev->data.memory = NULL; + break; + + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent attach of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + return 0; +} + +int +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk, det_disk; + virDomainNetDefPtr net; + virDomainHostdevDefPtr hostdev, det_hostdev; + virDomainLeaseDefPtr lease, det_lease; + virDomainControllerDefPtr cont, det_cont; + virDomainChrDefPtr chr; + virDomainFSDefPtr fs; + int idx; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { + virReportError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + return -1; + } + virDomainDiskDefFree(det_disk); + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + return -1; + + /* this is guaranteed to succeed */ + virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: { + hostdev = dev->data.hostdev; + if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("device not present in domain configuration")); + return -1; + } + virDomainHostdevRemove(vmdef, idx); + virDomainHostdevDefFree(det_hostdev); + break; + } + + case VIR_DOMAIN_DEVICE_LEASE: + lease = dev->data.lease; + if (!(det_lease = virDomainLeaseRemove(vmdef, lease))) { + virReportError(VIR_ERR_INVALID_ARG, + _("Lease %s in lockspace %s does not exist"), + lease->key, NULLSTR(lease->lockspace)); + return -1; + } + virDomainLeaseDefFree(det_lease); + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + cont = dev->data.controller; + if ((idx = virDomainControllerFind(vmdef, cont->type, + cont->idx)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("device not present in domain configuration")); + return -1; + } + det_cont = virDomainControllerRemove(vmdef, idx); + virDomainControllerDefFree(det_cont); + + break; + + case VIR_DOMAIN_DEVICE_CHR: + if (!(chr = qemuDomainChrRemove(vmdef, dev->data.chr))) + return -1; + + virDomainChrDefFree(chr); + virDomainChrDefFree(dev->data.chr); + dev->data.chr = NULL; + break; + + case VIR_DOMAIN_DEVICE_FS: + fs = dev->data.fs; + idx = virDomainFSIndexByName(vmdef, fs->dst); + if (idx < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("no matching filesystem device was found")); + return -1; + } + + fs = virDomainFSRemove(vmdef, idx); + virDomainFSDefFree(fs); + break; + + case VIR_DOMAIN_DEVICE_RNG: + if ((idx = virDomainRNGFind(vmdef, dev->data.rng)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("no matching RNG device was found")); + return -1; + } + + virDomainRNGDefFree(virDomainRNGRemove(vmdef, idx)); + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + if ((idx = virDomainMemoryFindInactiveByDef(vmdef, + dev->data.memory)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching memory device was not found")); + return -1; + } + + virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx)); + break; + + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent detach of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + return 0; +} + +int +qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr orig, disk; + virDomainGraphicsDefPtr newGraphics; + virDomainNetDefPtr net; + int pos; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (!(orig = virDomainDiskByName(vmdef, disk->dst, false))) { + virReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), disk->dst); + return -1; + } + if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("this disk doesn't support update")); + return -1; + } + /* + * Update 'orig' + * We allow updating src/type//driverType/cachemode/ + */ + orig->cachemode = disk->cachemode; + orig->startupPolicy = disk->startupPolicy; + + virStorageSourceFree(orig->src); + orig->src = disk->src; + disk->src = NULL; + break; + + case VIR_DOMAIN_DEVICE_GRAPHICS: + newGraphics = dev->data.graphics; + pos = qemuDomainFindGraphicsIndex(vmdef, newGraphics); + if (pos < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find existing graphics type '%s' device to modify"), + virDomainGraphicsTypeToString(newGraphics->type)); + return -1; + } + + virDomainGraphicsDefFree(vmdef->graphics[pos]); + + vmdef->graphics[pos] = newGraphics; + dev->data.graphics = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((pos = virDomainNetFindIdx(vmdef, net)) < 0) + return -1; + + virDomainNetDefFree(vmdef->nets[pos]); + + vmdef->nets[pos] = net; + dev->data.net = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent update of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index dd90e67..82e3308 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -669,4 +669,21 @@ int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm) int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) ATTRIBUTE_NONNULL(1); +int +qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virConnectPtr conn); + +int +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev); + + +int +qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev); + + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37d970e..832aa17 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7790,407 +7790,6 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, return ret; } -static int -qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, - virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, - virConnectPtr conn) -{ - virDomainDiskDefPtr disk; - virDomainNetDefPtr net; - virDomainHostdevDefPtr hostdev; - virDomainLeaseDefPtr lease; - virDomainControllerDefPtr controller; - virDomainFSDefPtr fs; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("target %s already exists"), disk->dst); - return -1; - } - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) - return -1; - if (qemuCheckDiskConfig(disk) < 0) - return -1; - if (virDomainDiskInsert(vmdef, disk)) - return -1; - /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ - dev->data.disk = NULL; - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) - if (virDomainDefAddImplicitDevices(vmdef) < 0) - return -1; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if (virDomainNetInsert(vmdef, net)) - return -1; - dev->data.net = NULL; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_HOSTDEV: - hostdev = dev->data.hostdev; - if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device is already in the domain configuration")); - return -1; - } - if (virDomainHostdevInsert(vmdef, hostdev)) - return -1; - dev->data.hostdev = NULL; - if (virDomainDefAddImplicitDevices(vmdef) < 0) - return -1; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_LEASE: - lease = dev->data.lease; - if (virDomainLeaseIndex(vmdef, lease) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("Lease %s in lockspace %s already exists"), - lease->key, NULLSTR(lease->lockspace)); - return -1; - } - if (virDomainLeaseInsert(vmdef, lease) < 0) - return -1; - - /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ - dev->data.lease = NULL; - break; - - case VIR_DOMAIN_DEVICE_CONTROLLER: - controller = dev->data.controller; - if (virDomainControllerFind(vmdef, controller->type, - controller->idx) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Target already exists")); - return -1; - } - - if (virDomainControllerInsert(vmdef, controller) < 0) - return -1; - dev->data.controller = NULL; - - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_CHR: - if (qemuDomainChrInsert(vmdef, dev->data.chr) < 0) - return -1; - dev->data.chr = NULL; - if (virDomainDefAddImplicitDevices(vmdef) < 0) - return -1; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_FS: - fs = dev->data.fs; - if (virDomainFSIndexByName(vmdef, fs->dst) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("Target already exists")); - return -1; - } - - if (virDomainFSInsert(vmdef, fs) < 0) - return -1; - dev->data.fs = NULL; - break; - - case VIR_DOMAIN_DEVICE_RNG: - if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - virDomainDefHasDeviceAddress(vmdef, &dev->data.rng->info)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("a device with the same address already exists ")); - return -1; - } - - if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) - return -1; - dev->data.rng = NULL; - - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_MEMORY: - if (vmdef->nmems == vmdef->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("no free memory device slot available")); - return -1; - } - - if (vmdef->mem.cur_balloon == virDomainDefGetMemoryActual(vmdef)) - vmdef->mem.cur_balloon += dev->data.memory->size; - - if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) - return -1; - dev->data.memory = NULL; - break; - - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("persistent attach of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - return 0; -} - - -static int -qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) -{ - virDomainDiskDefPtr disk, det_disk; - virDomainNetDefPtr net; - virDomainHostdevDefPtr hostdev, det_hostdev; - virDomainLeaseDefPtr lease, det_lease; - virDomainControllerDefPtr cont, det_cont; - virDomainChrDefPtr chr; - virDomainFSDefPtr fs; - int idx; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { - virReportError(VIR_ERR_INVALID_ARG, - _("no target device %s"), disk->dst); - return -1; - } - virDomainDiskDefFree(det_disk); - break; - - case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) - return -1; - - /* this is guaranteed to succeed */ - virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); - break; - - case VIR_DOMAIN_DEVICE_HOSTDEV: { - hostdev = dev->data.hostdev; - if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); - return -1; - } - virDomainHostdevRemove(vmdef, idx); - virDomainHostdevDefFree(det_hostdev); - break; - } - - case VIR_DOMAIN_DEVICE_LEASE: - lease = dev->data.lease; - if (!(det_lease = virDomainLeaseRemove(vmdef, lease))) { - virReportError(VIR_ERR_INVALID_ARG, - _("Lease %s in lockspace %s does not exist"), - lease->key, NULLSTR(lease->lockspace)); - return -1; - } - virDomainLeaseDefFree(det_lease); - break; - - case VIR_DOMAIN_DEVICE_CONTROLLER: - cont = dev->data.controller; - if ((idx = virDomainControllerFind(vmdef, cont->type, - cont->idx)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); - return -1; - } - det_cont = virDomainControllerRemove(vmdef, idx); - virDomainControllerDefFree(det_cont); - - break; - - case VIR_DOMAIN_DEVICE_CHR: - if (!(chr = qemuDomainChrRemove(vmdef, dev->data.chr))) - return -1; - - virDomainChrDefFree(chr); - virDomainChrDefFree(dev->data.chr); - dev->data.chr = NULL; - break; - - case VIR_DOMAIN_DEVICE_FS: - fs = dev->data.fs; - idx = virDomainFSIndexByName(vmdef, fs->dst); - if (idx < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("no matching filesystem device was found")); - return -1; - } - - fs = virDomainFSRemove(vmdef, idx); - virDomainFSDefFree(fs); - break; - - case VIR_DOMAIN_DEVICE_RNG: - if ((idx = virDomainRNGFind(vmdef, dev->data.rng)) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("no matching RNG device was found")); - return -1; - } - - virDomainRNGDefFree(virDomainRNGRemove(vmdef, idx)); - break; - - case VIR_DOMAIN_DEVICE_MEMORY: - if ((idx = virDomainMemoryFindInactiveByDef(vmdef, - dev->data.memory)) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("matching memory device was not found")); - return -1; - } - - virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx)); - break; - - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("persistent detach of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - return 0; -} - -static int -qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, - virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) -{ - virDomainDiskDefPtr orig, disk; - virDomainGraphicsDefPtr newGraphics; - virDomainNetDefPtr net; - int pos; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (!(orig = virDomainDiskByName(vmdef, disk->dst, false))) { - virReportError(VIR_ERR_INVALID_ARG, - _("target %s doesn't exist."), disk->dst); - return -1; - } - if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && - !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("this disk doesn't support update")); - return -1; - } - /* - * Update 'orig' - * We allow updating src/type//driverType/cachemode/ - */ - orig->cachemode = disk->cachemode; - orig->startupPolicy = disk->startupPolicy; - - virStorageSourceFree(orig->src); - orig->src = disk->src; - disk->src = NULL; - break; - - case VIR_DOMAIN_DEVICE_GRAPHICS: - newGraphics = dev->data.graphics; - pos = qemuDomainFindGraphicsIndex(vmdef, newGraphics); - if (pos < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot find existing graphics type '%s' device to modify"), - virDomainGraphicsTypeToString(newGraphics->type)); - return -1; - } - - virDomainGraphicsDefFree(vmdef->graphics[pos]); - - vmdef->graphics[pos] = newGraphics; - dev->data.graphics = NULL; - break; - - case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((pos = virDomainNetFindIdx(vmdef, net)) < 0) - return -1; - - virDomainNetDefFree(vmdef->nets[pos]); - - vmdef->nets[pos] = net; - dev->data.net = NULL; - - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_RNG: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_CHR: - case VIR_DOMAIN_DEVICE_MEMORY: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("persistent update of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - return 0; -} - static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags)

This helps moving some checks into the top function before actually going ahead with the operation. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_hotplug.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c191919..96caef3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4256,8 +4256,9 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, return ret; } -int -qemuDomainAttachDeviceLive(virDomainObjPtr vm, + +static int +qemuDomainAttachDeviceLiveInternal(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { @@ -4380,6 +4381,14 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, return ret; } +int +qemuDomainAttachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) +{ + return qemuDomainAttachDeviceLiveInternal(vm, dev, dom); +} + static int qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4400,10 +4409,10 @@ qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, return ret; } -int -qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainPtr dom) +static int +qemuDomainDetachDeviceLiveInternal(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; @@ -4462,6 +4471,16 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, return ret; } + +int +qemuDomainDetachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) +{ + return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); +} + + static int qemuDomainChangeDiskLive(virConnectPtr conn, virDomainObjPtr vm, @@ -4542,8 +4561,9 @@ qemuDomainChangeDiskLive(virConnectPtr conn, goto cleanup; } -int -qemuDomainUpdateDeviceLive(virConnectPtr conn, + +static int +qemuDomainUpdateDeviceLiveInternal(virConnectPtr conn, virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom, @@ -4592,3 +4612,14 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, return ret; } + +int +qemuDomainUpdateDeviceLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + bool force) +{ + return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, + dom, force); +}

The checks need to be performed per device and its better to do them a level down in stack as we prepare for multifunction hotplug. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_domain.c | 12 ++++++++++++ src/qemu/qemu_driver.c | 24 ------------------------ src/qemu/qemu_hotplug.c | 12 ++++++++++++ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bf91db3..da5f97d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5531,6 +5531,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDeviceDefPtr dev, virConnectPtr conn) { + if (virDomainDefCompatibleDevice(vmdef, dev, + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + return qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, dev, conn); } @@ -5674,6 +5678,10 @@ int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + if (virDomainDefCompatibleDevice(vmdef, dev, + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + return qemuDomainDetachDeviceConfigInternal(vmdef, dev); } @@ -5778,5 +5786,9 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + if (virDomainDefCompatibleDevice(vmdef, dev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + return qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, dev); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 98e0bfd..9484576 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7521,20 +7521,12 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, dom->conn)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) goto endjob; /* @@ -7648,19 +7640,11 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) goto endjob; /* @@ -7768,19 +7752,11 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; - if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) goto endjob; /* diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 96caef3..9a546e2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4386,6 +4386,10 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { + if (virDomainDefCompatibleDevice(vm->def, dev, + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + return qemuDomainAttachDeviceLiveInternal(vm, dev, dom); } @@ -4477,6 +4481,10 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { + if (virDomainDefCompatibleDevice(vm->def, dev, + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); } @@ -4620,6 +4628,10 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainPtr dom, bool force) { + if (virDomainDefCompatibleDevice(vm->def, dev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, dom, force); }

Shivaprasad G Bhat <shivaprasadbhat(a)gmail.com&gt; writes: Unref / free the qemuCaps

Shivaprasad G Bhat <shivaprasadbhat(a)gmail.com&gt; writes: ??? devcopylist = NULL ? > + if (VIR_ALLOC(devcopylist) < 0) > + goto endjob; > + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], > + vm->def, caps, driver->xmlopt) < 0) > goto endjob; > } > > @@ -7521,13 +7529,13 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, > if (!vmdef) > goto endjob; > > - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, > + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, > dom->conn)) < 0) > goto endjob; > } > > if (flags & VIR_DOMAIN_AFFECT_LIVE) { > - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) > + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, dom)) < 0) > goto endjob; > /* > * update domain status forcibly because the domain status may be > @@ -7555,9 +7563,9 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, > cleanup: > virObjectUnref(qemuCaps); > virDomainDefFree(vmdef); > - if (dev != dev_copy) > - virDomainDeviceDefFree(dev_copy); > - virDomainDeviceDefFree(dev); > + if (devlist != devcopylist) > + virDomainDeviceDefListFree(devcopylist); > + virDomainDeviceDefListFree(devlist); > virDomainObjEndAPI(&vm); > virObjectUnref(caps); > virObjectUnref(cfg); > @@ -7579,7 +7587,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, > virQEMUDriverPtr driver = dom->conn->privateData; > virDomainObjPtr vm = NULL; > virDomainDefPtr vmdef = NULL; > - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; > + virDomainDeviceDefPtr dev = NULL; > + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; > bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; > int ret = -1; > virQEMUCapsPtr qemuCaps = NULL; > @@ -7609,12 +7618,17 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, > if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) > goto cleanup; > > - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, > + dev = virDomainDeviceDefParse(xml, vm->def, > caps, driver->xmlopt, > VIR_DOMAIN_DEF_PARSE_INACTIVE); > - if (dev == NULL) > + if (!dev || VIR_ALLOC(devlist) < 0) > + goto endjob; > + > + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) > goto endjob; > > + devcopylist = devlist; > + > if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) > goto endjob; > > @@ -7624,8 +7638,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, > * create a deep copy of device as adding > * to CONFIG takes one instance. > */ > - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); > - if (!dev_copy) > + if (VIR_ALLOC(devcopylist) < 0) > + goto endjob; > + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], > + vm->def, caps, driver->xmlopt) < 0) > goto endjob; > } > > @@ -7640,12 +7656,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, > if (!vmdef) > goto endjob; > > - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) > + if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, devlist)) < 0) > goto endjob; > } > > if (flags & VIR_DOMAIN_AFFECT_LIVE) { > - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) > + if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, devcopylist, dom, force)) < 0) > goto endjob; > /* > * update domain status forcibly because the domain status may be > @@ -7673,9 +7689,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, > cleanup: > virObjectUnref(qemuCaps); > virDomainDefFree(vmdef); > - if (dev != dev_copy) > - virDomainDeviceDefFree(dev_copy); > - virDomainDeviceDefFree(dev); > + if (devlist != devcopylist) > + virDomainDeviceDefListFree(devcopylist); > + virDomainDeviceDefListFree(devlist); > virDomainObjEndAPI(&vm); > virObjectUnref(caps); > virObjectUnref(cfg); > @@ -7690,7 +7706,8 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, > virQEMUDriverPtr driver = dom->conn->privateData; > virDomainObjPtr vm = NULL; > virDomainDefPtr vmdef = NULL; > - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; > + virDomainDeviceDefPtr dev = NULL; > + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; > int ret = -1; > unsigned int parse_flags = 0; > virQEMUCapsPtr qemuCaps = NULL; > @@ -7724,20 +7741,27 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, > !(flags & VIR_DOMAIN_AFFECT_LIVE)) > parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; > > - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, > + dev = virDomainDeviceDefParse(xml, vm->def, > caps, driver->xmlopt, > parse_flags); > - if (dev == NULL) > + if (!dev || VIR_ALLOC(devlist) < 0) > goto endjob; > > + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) > + goto endjob; > + > + devcopylist = devlist; set to null free devlist ?? Can go in the above patch where prepare is introduced. unref of caps? ********* -1 unref of caps?

Shivaprasad G Bhat <shivaprasadbhat(a)gmail.com&gt; writes: Remove this, not supported for PCI isPCIMultifunctionDeviceXML