On Fri, Jul 15, 2016 at 07:50:25AM -0400, John Ferlan wrote:
Based on recent review comment - rather than have a spate of goto failxxxx,
change to a boolean based model. Ensures that the original error can be
preserved and cleanup is a bit more orderly if more objects are added.
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/qemu/qemu_hotplug.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a220d9f..c5a1a91 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1587,12 +1587,17 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
virDomainRNGDefPtr rng)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
char *devstr = NULL;
char *charAlias = NULL;
char *objAlias = NULL;
+ bool releaseaddr = false;
+ bool cleanupCharAlias = false;
could be chardevAdded
+ bool cleanupObjAlias = false;
objAdded
virJSONValuePtr props = NULL;
const char *type;
int ret = -1;
+ int rv;
if (qemuAssignDeviceRNGAlias(vm->def, rng) < 0)
return -1;
@@ -1613,6 +1618,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
rng->source.file))
return -1;
}
+ releaseaddr = true;
if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
@@ -1637,23 +1643,25 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
if (virAsprintf(&charAlias, "char%s", rng->info.alias) < 0)
goto cleanup;
- /* attach the device - up to a 3 stage process */
qemuDomainObjEnterMonitor(driver, vm);
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
qemuMonitorAttachCharDev(priv->mon, charAlias,
rng->source.chardev) < 0)
- goto failchardev;
+ goto monitor_error;
+ cleanupCharAlias = true;
- if (qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0)
- goto failbackend;
- props = NULL;
+ rv = qemuMonitorAddObject(priv->mon, type, objAlias, props);
+ props = NULL; /* qemuMonitorAddObject consumes */
+ if (rv < 0)
+ goto monitor_error;
+ cleanupObjAlias = true;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
- goto failfrontend;
+ goto monitor_error;
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
- vm = NULL;
+ releaseaddr = false;
goto cleanup;
}
@@ -1665,24 +1673,24 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0);
cleanup:
virJSONValueFree(props);
- if (ret < 0 && vm)
+ if (ret < 0 && releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &rng->info, NULL);
VIR_FREE(charAlias);
VIR_FREE(objAlias);
VIR_FREE(devstr);
return ret;
- /* rollback */
- failfrontend:
- ignore_value(qemuMonitorDelObject(priv->mon, objAlias));
- failbackend:
- if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
+ monitor_error:
exit_monitor is nicer IMO.
+ orig_err = virSaveLastError();
+ if (cleanupObjAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, objAlias));
+ if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && cleanupCharAlias)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
- props = NULL; /* qemuMonitorAddObject consumes on failure */
- failchardev:
- if (qemuDomainObjExitMonitor(driver, vm) < 0) {
- vm = NULL;
- goto cleanup;
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
Same comment about the error applies.
Jan
+ releaseaddr = false;
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
}
goto audit;
--
2.5.5
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list