[libvirt] [PATCH] qemu-hotplug: fix eject media

[Peter Krempa]

On Thu, Mar 17, 2016 at 16:50:36 +0100, Pavel Hrdina wrote:
> QEMU changed the error message to:
> 
>         "Tray of device 'drive-sata0-0-1' is not open"
> 
> and they may change the error massage in the future.
> 
> This updates the code to not depend on the text from the error message
> but only on error itself.  One more improvement is that we will store
> the original error in case if it's not about tray is not open.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c      | 17 +++++++++--------
>  src/qemu/qemu_monitor_json.c | 12 +-----------
>  src/qemu/qemu_monitor_text.c |  5 +----
>  3 files changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b580283..8122367 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c

[...]

> @@ -202,15 +202,16 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>          if (qemuDomainObjExitMonitor(driver, vm) < 0)
>              goto cleanup;
>  
> -        if (rc == -2) {
> +        if (rc < 0) {
>              /* we've already tried, error out */
> -            if (ejectRetry)
> +            if (ejectError) {
> +                virSetError(ejectError);
> +                virFreeError(ejectError);

I don't think that it's necessary to store the first error message.
Previously we'd report the last error due to the fact that the JSON code
was setting it all the time.

>                  goto error;
> -            VIR_DEBUG("tray is locked, wait for the guest to unlock "
> +            }
> +            ejectError = virSaveLastError();
> +            VIR_DEBUG("tray may be locked, wait for the guest to unlock "
>                        "the tray and try to eject it again");
> -            ejectRetry = true;
> -        } else if (rc < 0) {
> -            goto error;
>          }
>  
>          if (virTimeMillisNow(&now) < 0)
> @@ -220,7 +221,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>              if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0)
>                  goto error;
>          }
> -    } while (ejectRetry && rc != 0);
> +    } while (rc < 0);
>  
>      if (!virStorageSourceIsEmpty(newsrc)) {
>          if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0)
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 8352e53..66a2922 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c

[...]

> @@ -2294,15 +2293,6 @@ int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon,
>      if (ret == 0)
>          ret = qemuMonitorJSONCheckError(cmd, reply);

Additionally I think we should not even report the error from the first
call to this function. Doing that will probably require us to keep the
-2 return value but you'll need to add a bool argument that will
suppress any virReportError in case when the qemu command failed and
return -2 in that case.

>  
> -    if (ret < 0) {
> -        virJSONValuePtr error = virJSONValueObjectGet(reply, "error");

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160322/988fcdae/attachment-0001.sig>