[PATCH 0/7] Sanitize logic to get live/config device definitions for unplug/update
Convert code *DomainUpdateDeviceFlags/*DomainDetachDeviceLiveAndConfig to avoid use of virDomainDeviceDefCopy. We have the original XML string from the user so it doesn't make sense to parse it and then copy it (whcih involves formatting and parsing back), when we can simply parse it twice, saving the extra formatting step. Peter Krempa (7): qemu: driver: Fix formatting of function headers around qemuDomainAttachDevice qemuDomainUpdateDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy qemuDomainDetachDeviceLiveAndConfig: Parse XML twice rather than use virDomainDeviceDefCopy qemuDomainDetachDeviceLiveAndConfig: Refactor cleanup lxcDomainAttachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy lxcDomainDetachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy conf: domain: Remove virDomainDeviceDefCopy src/conf/domain_conf.c | 121 --------------------------------------- src/conf/domain_conf.h | 4 -- src/libvirt_private.syms | 1 - src/lxc/lxc_driver.c | 75 ++++++++++-------------- src/qemu/qemu_driver.c | 97 +++++++++++++------------------ 5 files changed, 70 insertions(+), 228 deletions(-) -- 2.37.3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff5a743716..70368ffb10 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7757,16 +7757,19 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, return ret; } -static int qemuDomainAttachDevice(virDomainPtr dom, const char *xml) + +static int +qemuDomainAttachDevice(virDomainPtr dom, + const char *xml) { - return qemuDomainAttachDeviceFlags(dom, xml, - VIR_DOMAIN_AFFECT_LIVE); + return qemuDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); } -static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, - const char *xml, - unsigned int flags) +static int +qemuDomainUpdateDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; -- 2.37.3
'virDomainDeviceDefCopy' formats the definition and parses it back. Since we already are parsing the XML here, we're better off parsing it twice and save the formatting step. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 70368ffb10..ef0a2e06fd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7775,8 +7775,8 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, virDomainObj *vm = NULL; qemuDomainObjPrivate *priv; g_autoptr(virDomainDef) vmdef = NULL; - g_autoptr(virDomainDeviceDef) dev = NULL; - virDomainDeviceDef *dev_copy = NULL; + g_autoptr(virDomainDeviceDef) dev_config = NULL; + g_autoptr(virDomainDeviceDef) dev_live = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; g_autoptr(virQEMUDriverConfig) cfg = NULL; @@ -7806,21 +7806,15 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - driver->xmlopt, priv->qemuCaps, - parse_flags); - if (dev == NULL) - goto endjob; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!(dev_config = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + priv->qemuCaps, parse_flags))) + goto endjob; + } - if (flags & VIR_DOMAIN_AFFECT_CONFIG && - flags & VIR_DOMAIN_AFFECT_LIVE) { - /* If we are affecting both CONFIG and LIVE - * create a deep copy of device as adding - * to CONFIG takes one instance. - */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, - driver->xmlopt, priv->qemuCaps); - if (!dev_copy) + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + priv->qemuCaps, parse_flags))) goto endjob; } @@ -7833,7 +7827,7 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, /* virDomainDefCompatibleDevice call is delayed until we know the * device we're going to update. */ - if ((ret = qemuDomainUpdateDeviceConfig(vmdef, dev_copy, priv->qemuCaps, + if ((ret = qemuDomainUpdateDeviceConfig(vmdef, dev_config, priv->qemuCaps, parse_flags, driver->xmlopt)) < 0) goto endjob; @@ -7842,7 +7836,7 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { /* virDomainDefCompatibleDevice call is delayed until we know the * device we're going to update. */ - if ((ret = qemuDomainUpdateDeviceLive(vm, dev, dom, force)) < 0) + if ((ret = qemuDomainUpdateDeviceLive(vm, dev_live, dom, force)) < 0) goto endjob; qemuDomainSaveStatus(vm); @@ -7859,8 +7853,6 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, virDomainObjEndJob(vm); cleanup: - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); virDomainObjEndAPI(&vm); return ret; } -- 2.37.3
'virDomainDeviceDefCopy' formats the definition and parses it back. Since we already are parsing the XML here, we're better off parsing it twice and save the formatting step. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ef0a2e06fd..bb345ef017 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7865,8 +7865,8 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, { qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virQEMUDriverConfig) cfg = NULL; - g_autoptr(virDomainDeviceDef) dev = NULL; - virDomainDeviceDef *dev_copy = NULL; + g_autoptr(virDomainDeviceDef) dev_config = NULL; + g_autoptr(virDomainDeviceDef) dev_live = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; g_autoptr(virDomainDef) vmdef = NULL; int ret = -1; @@ -7880,21 +7880,15 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - driver->xmlopt, priv->qemuCaps, - parse_flags); - if (dev == NULL) - goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!(dev_config = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + priv->qemuCaps, parse_flags))) + goto cleanup; + } - if (flags & VIR_DOMAIN_AFFECT_CONFIG && - flags & VIR_DOMAIN_AFFECT_LIVE) { - /* If we are affecting both CONFIG and LIVE - * create a deep copy of device as adding - * to CONFIG takes one instance. - */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, - driver->xmlopt, priv->qemuCaps); - if (!dev_copy) + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + priv->qemuCaps, parse_flags))) goto cleanup; } @@ -7904,7 +7898,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, if (!vmdef) goto cleanup; - if (qemuDomainDetachDeviceConfig(vmdef, dev_copy, priv->qemuCaps, + if (qemuDomainDetachDeviceConfig(vmdef, dev_config, priv->qemuCaps, parse_flags, driver->xmlopt) < 0) goto cleanup; @@ -7913,7 +7907,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, if (flags & VIR_DOMAIN_AFFECT_LIVE) { int rc; - if ((rc = qemuDomainDetachDeviceLive(vm, dev, driver, false)) < 0) + if ((rc = qemuDomainDetachDeviceLive(vm, dev_live, driver, false)) < 0) goto cleanup; if (rc == 0 && qemuDomainUpdateDeviceList(vm, VIR_ASYNC_JOB_NONE) < 0) @@ -7933,8 +7927,6 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, ret = 0; cleanup: - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); return ret; } -- 2.37.3
Remove the 'cleanup' label and 'ret' variable as we can now directly return form all cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bb345ef017..22e4a42c64 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7869,7 +7869,6 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, g_autoptr(virDomainDeviceDef) dev_live = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; g_autoptr(virDomainDef) vmdef = NULL; - int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7883,35 +7882,35 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (!(dev_config = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, priv->qemuCaps, parse_flags))) - goto cleanup; + return -1; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, priv->qemuCaps, parse_flags))) - goto cleanup; + return -1; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, driver->xmlopt, priv->qemuCaps); if (!vmdef) - goto cleanup; + return -1; if (qemuDomainDetachDeviceConfig(vmdef, dev_config, priv->qemuCaps, parse_flags, driver->xmlopt) < 0) - goto cleanup; + return -1; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { int rc; if ((rc = qemuDomainDetachDeviceLive(vm, dev_live, driver, false)) < 0) - goto cleanup; + return -1; if (rc == 0 && qemuDomainUpdateDeviceList(vm, VIR_ASYNC_JOB_NONE) < 0) - goto cleanup; + return -1; qemuDomainSaveStatus(vm); } @@ -7919,15 +7918,12 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, /* Finally, if no error until here, we can save config. */ if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (virDomainDefSave(vmdef, driver->xmlopt, cfg->configDir) < 0) - goto cleanup; + return -1; virDomainObjAssignDef(vm, &vmdef, false, NULL); } - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.37.3
'virDomainDeviceDefCopy' formats the definition and parses it back. Since we already are parsing the XML here, we're better off parsing it twice and save the formatting step. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_driver.c | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d66c26221c..2801ed3d6d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4255,8 +4255,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, virLXCDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; g_autoptr(virDomainDef) vmdef = NULL; - virDomainDeviceDef *dev = NULL; - virDomainDeviceDef *dev_copy = NULL; + g_autoptr(virDomainDeviceDef) dev_config = NULL; + g_autoptr(virDomainDeviceDef) dev_live = NULL; int ret = -1; g_autoptr(virLXCDriverConfig) cfg = virLXCDriverGetConfig(driver); @@ -4275,21 +4275,17 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - driver->xmlopt, NULL, - VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (dev == NULL) - goto endjob; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!(dev_config = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto endjob; + } - if (flags & VIR_DOMAIN_AFFECT_CONFIG && - flags & VIR_DOMAIN_AFFECT_LIVE) { - /* If we are affecting both CONFIG and LIVE - * create a deep copy of device as adding - * to CONFIG takes one instance. - */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, - driver->xmlopt, NULL); - if (!dev_copy) + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto endjob; } @@ -4299,22 +4295,22 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, NULL, + if (virDomainDefCompatibleDevice(vmdef, dev_config, NULL, VIR_DOMAIN_DEVICE_ACTION_ATTACH, false) < 0) goto endjob; - if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev_copy)) < 0) + if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev_config)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev, NULL, + if (virDomainDefCompatibleDevice(vm->def, dev_live, NULL, VIR_DOMAIN_DEVICE_ACTION_ATTACH, true) < 0) goto endjob; - if ((ret = lxcDomainAttachDeviceLive(driver, vm, dev)) < 0) + if ((ret = lxcDomainAttachDeviceLive(driver, vm, dev_live)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -4338,9 +4334,6 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, virDomainObjEndJob(vm); cleanup: - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); virDomainObjEndAPI(&vm); return ret; } -- 2.37.3
'virDomainDeviceDefCopy' formats the definition and parses it back. Since we already are parsing the XML here, we're better off parsing it twice and save the formatting step. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_driver.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2801ed3d6d..5a16e7375e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4416,8 +4416,10 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, g_autoptr(virCaps) caps = NULL; virDomainObj *vm = NULL; g_autoptr(virDomainDef) vmdef = NULL; - virDomainDeviceDef *dev = NULL; - virDomainDeviceDef *dev_copy = NULL; + g_autoptr(virDomainDeviceDef) dev_config = NULL; + g_autoptr(virDomainDeviceDef) dev_live = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE | + VIR_DOMAIN_DEF_PARSE_INACTIVE; int ret = -1; g_autoptr(virLXCDriverConfig) cfg = virLXCDriverGetConfig(driver); @@ -4439,22 +4441,15 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto endjob; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - driver->xmlopt, NULL, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); - if (dev == NULL) - goto endjob; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!(dev_config = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + NULL, parse_flags))) + goto endjob; + } - if (flags & VIR_DOMAIN_AFFECT_CONFIG && - flags & VIR_DOMAIN_AFFECT_LIVE) { - /* If we are affecting both CONFIG and LIVE - * create a deep copy of device as adding - * to CONFIG takes one instance. - */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, - driver->xmlopt, NULL); - if (!dev_copy) + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + NULL, parse_flags))) goto endjob; } @@ -4464,12 +4459,12 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob; - if ((ret = lxcDomainDetachDeviceConfig(vmdef, dev_copy)) < 0) + if ((ret = lxcDomainDetachDeviceConfig(vmdef, dev_config)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = lxcDomainDetachDeviceLive(driver, vm, dev)) < 0) + if ((ret = lxcDomainDetachDeviceLive(driver, vm, dev_live)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -4493,9 +4488,6 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, virDomainObjEndJob(vm); cleanup: - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); virDomainObjEndAPI(&vm); return ret; } -- 2.37.3
The function is now unused. Remove it to dissuade anybody from trying to use it in the future. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 121 --------------------------------------- src/conf/domain_conf.h | 4 -- src/libvirt_private.syms | 1 - 3 files changed, 126 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3790121cf7..ce16cc414a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28347,127 +28347,6 @@ virDomainNetFindByName(virDomainDef *def, } -/** - * virDomainDeviceDefCopy: - * @caps: Capabilities - * @def: Domain definition to which @src belongs - * @src: source to be copied - * - * virDomainDeviceDefCopy does a deep copy of only the parts of a - * DeviceDef that are valid when just the flag VIR_DOMAIN_DEF_PARSE_INACTIVE is - * set. This means that any part of the device xml that is conditionally - * parsed/formatted based on some other flag being set (or on the INACTIVE - * flag being reset) *will not* be copied to the destination. Caveat emptor. - * - * Returns a pointer to copied @src or NULL in case of error. - */ -virDomainDeviceDef * -virDomainDeviceDefCopy(virDomainDeviceDef *src, - const virDomainDef *def, - virDomainXMLOption *xmlopt, - void *parseOpaque) -{ - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - int flags = VIR_DOMAIN_DEF_FORMAT_INACTIVE | VIR_DOMAIN_DEF_FORMAT_SECURE; - int rc = -1; - g_autofree char *xmlStr = NULL; - - switch ((virDomainDeviceType) src->type) { - case VIR_DOMAIN_DEVICE_DISK: - rc = virDomainDiskDefFormat(&buf, src->data.disk, flags, xmlopt); - break; - case VIR_DOMAIN_DEVICE_LEASE: - virDomainLeaseDefFormat(&buf, src->data.lease); - rc = 0; - break; - case VIR_DOMAIN_DEVICE_FS: - rc = virDomainFSDefFormat(&buf, src->data.fs, flags); - break; - case VIR_DOMAIN_DEVICE_NET: - rc = virDomainNetDefFormat(&buf, src->data.net, xmlopt, flags); - break; - case VIR_DOMAIN_DEVICE_INPUT: - rc = virDomainInputDefFormat(&buf, src->data.input, flags); - break; - case VIR_DOMAIN_DEVICE_SOUND: - rc = virDomainSoundDefFormat(&buf, src->data.sound, flags); - break; - case VIR_DOMAIN_DEVICE_VIDEO: - rc = virDomainVideoDefFormat(&buf, src->data.video, flags); - break; - case VIR_DOMAIN_DEVICE_HOSTDEV: - rc = virDomainHostdevDefFormat(&buf, src->data.hostdev, flags, xmlopt); - break; - case VIR_DOMAIN_DEVICE_WATCHDOG: - rc = virDomainWatchdogDefFormat(&buf, src->data.watchdog, flags); - break; - case VIR_DOMAIN_DEVICE_CONTROLLER: - rc = virDomainControllerDefFormat(&buf, src->data.controller, flags); - break; - case VIR_DOMAIN_DEVICE_GRAPHICS: - rc = virDomainGraphicsDefFormat(&buf, src->data.graphics, flags); - break; - case VIR_DOMAIN_DEVICE_HUB: - rc = virDomainHubDefFormat(&buf, src->data.hub, flags); - break; - case VIR_DOMAIN_DEVICE_REDIRDEV: - rc = virDomainRedirdevDefFormat(&buf, src->data.redirdev, flags); - break; - case VIR_DOMAIN_DEVICE_RNG: - rc = virDomainRNGDefFormat(&buf, src->data.rng, flags); - break; - case VIR_DOMAIN_DEVICE_CHR: - rc = virDomainChrDefFormat(&buf, src->data.chr, flags); - break; - case VIR_DOMAIN_DEVICE_TPM: - virDomainTPMDefFormat(&buf, src->data.tpm, flags, xmlopt); - rc = 0; - break; - case VIR_DOMAIN_DEVICE_PANIC: - virDomainPanicDefFormat(&buf, src->data.panic); - rc = 0; - break; - case VIR_DOMAIN_DEVICE_MEMORY: - rc = virDomainMemoryDefFormat(&buf, src->data.memory, flags); - break; - case VIR_DOMAIN_DEVICE_SHMEM: - virDomainShmemDefFormat(&buf, src->data.shmem, flags); - rc = 0; - break; - case VIR_DOMAIN_DEVICE_IOMMU: - virDomainIOMMUDefFormat(&buf, src->data.iommu); - rc = 0; - break; - case VIR_DOMAIN_DEVICE_VSOCK: - virDomainVsockDefFormat(&buf, src->data.vsock); - rc = 0; - break; - case VIR_DOMAIN_DEVICE_AUDIO: - rc = virDomainAudioDefFormat(&buf, src->data.audio); - break; - - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Copying definition of '%d' type " - "is not implemented yet."), - src->type); - return NULL; - } - - if (rc < 0) - return NULL; - - xmlStr = virBufferContentAndReset(&buf); - return virDomainDeviceDefParse(xmlStr, def, xmlopt, parseOpaque, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); -} - - virSecurityLabelDef * virDomainDefGetSecurityLabelDef(const virDomainDef *def, const char *model) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a9f7e8d977..c19dfc5470 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3471,10 +3471,6 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainShmemDef, virDomainShmemDefFree); void virDomainDeviceDefFree(virDomainDeviceDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree); -virDomainDeviceDef *virDomainDeviceDefCopy(virDomainDeviceDef *src, - const virDomainDef *def, - virDomainXMLOption *xmlopt, - void *parseOpaque); virDomainDeviceInfo *virDomainDeviceGetInfo(const virDomainDeviceDef *device); void virDomainDeviceSetData(virDomainDeviceDef *device, void *devicedata); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ebd7bc61a8..fd79a9d440 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -353,7 +353,6 @@ virDomainDefSetVcpusMax; virDomainDefVcpuOrderClear; virDomainDeleteConfig; virDomainDeviceAliasIsUserAlias; -virDomainDeviceDefCopy; virDomainDeviceDefFree; virDomainDeviceDefParse; virDomainDeviceFindSCSIController; -- 2.37.3
On a Wednesday in 2022, Peter Krempa wrote:
Convert code *DomainUpdateDeviceFlags/*DomainDetachDeviceLiveAndConfig to avoid use of virDomainDeviceDefCopy. We have the original XML string from the user so it doesn't make sense to parse it and then copy it (whcih involves formatting and parsing back), when we can simply parse it twice, saving the extra formatting step.
Peter Krempa (7): qemu: driver: Fix formatting of function headers around qemuDomainAttachDevice qemuDomainUpdateDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy qemuDomainDetachDeviceLiveAndConfig: Parse XML twice rather than use virDomainDeviceDefCopy qemuDomainDetachDeviceLiveAndConfig: Refactor cleanup lxcDomainAttachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy lxcDomainDetachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy conf: domain: Remove virDomainDeviceDefCopy
src/conf/domain_conf.c | 121 --------------------------------------- src/conf/domain_conf.h | 4 -- src/libvirt_private.syms | 1 - src/lxc/lxc_driver.c | 75 ++++++++++-------------- src/qemu/qemu_driver.c | 97 +++++++++++++------------------ 5 files changed, 70 insertions(+), 228 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa