The fact that we are overwriting @ret multiple times is very confusing to see what is actually happening here. Follow our traditional pattern where @ret is initialized to -1, and set to 0 only in case we know we succeeded. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/qemu/qemu_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 992f140b2b..2347e71cfb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8709,35 +8709,35 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, if (!vmdef) goto cleanup; - if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev, caps, - parse_flags, - driver->xmlopt)) < 0) + if (qemuDomainDetachDeviceConfig(vmdef, dev, caps, + parse_flags, + driver->xmlopt) < 0) goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, driver)) < 0) + if (qemuDomainDetachDeviceLive(vm, dev_copy, driver) < 0) goto cleanup; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, * a new controller may be created. */ - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, caps) < 0) { - ret = -1; + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, caps) < 0) goto cleanup; - } } /* Finally, if no error until here, we can save config. */ if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - ret = virDomainSaveConfig(cfg->configDir, caps, vmdef); - if (!ret) { - virDomainObjAssignDef(vm, vmdef, false, NULL); - vmdef = NULL; - } + if (virDomainSaveConfig(cfg->configDir, caps, vmdef) < 0) + goto cleanup; + + virDomainObjAssignDef(vm, vmdef, false, NULL); + vmdef = NULL; } + ret = 0; + cleanup: virObjectUnref(caps); virObjectUnref(cfg); -- 2.16.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list