On 07/08/2016 06:30 AM, Bjoern Walk wrote:
Since return code is checked globally at the end of the function, let's make sure that we set it correctly at any point.
This fixes a regression introduced in commit 0aa19f35 where the first command to eject changeable media would fail unconditionally.
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c322543..789f18c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -240,7 +240,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, /* If the tray is present and tray change event is supported wait for it to open. */ if (diskPriv->tray && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) { - if (qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias, force) < 0) + rc = qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias, force); + if (rc < 0) goto error;
But rc isn't checked in the error and cleanup labels in qemuDomainChangeEjectableMedia, so not sure what the purpose of setting rc is. What am I missing? The existing "else" in the code is for (diskPriv->tray && virQEMU...) condition as I see it. If perhaps you had just set rc, not done the goto, and remove the else condition, e.g.: if (diskPriv... && virQEMUCaps...) rc = qemuHotplugWait... if (rc < 0) goto error; that's just doing the same thing in a different way. John
} else { /* otherwise report possible errors from the attempt to eject the media*/