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

[Ján Tomko]

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