Re: [libvirt] [PATCH 5/7] qemu: Alter error path cleanup for qemuDomainAttachRNGDevice

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(a)redhat.com&gt; --- 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;

+ bool cleanupObjAlias = false;

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:

+ 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)

+ releaseaddr = false; + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); } goto audit; -- 2.5.5 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list