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 | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0fc0084..a220d9f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1514,9 +1514,11 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, { int ret = -1, rc; qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err; virDomainDefPtr vmdef = vm->def; char *devstr = NULL; char *charAlias = NULL; + bool cleanupAttachCharDev = false; bool need_release = false; if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && @@ -1542,10 +1544,11 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) - goto failchardev; + goto monitor_error; + cleanupAttachCharDev = true; if (qemuMonitorAddDevice(priv->mon, devstr) < 0) - goto failadddev; + goto monitor_error; if (qemuDomainObjExitMonitor(driver, vm) < 0) goto audit; @@ -1563,11 +1566,17 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, VIR_FREE(devstr); return ret; - failadddev: + monitor_error: + orig_err = virSaveLastError(); /* detach associated chardev on error */ - qemuMonitorDetachCharDev(priv->mon, charAlias); - failchardev: + if (cleanupAttachCharDev) + qemuMonitorDetachCharDev(priv->mon, charAlias); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + 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