[libvirt] [PATCH] qemu: hotplug: fix changeable media ejection

[John Ferlan]

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 at linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy at 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*/
>