Re: [PATCH v2 4/4] qemu_hotplug: try harder to eject media

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/29/2015 11:17 AM, Pavel Hrdina wrote:
> Some guests lock the tray and QEMU eject command will simply fail to
> eject the media.  But the guest OS can handle this attempt to eject the
> media and can unlock the tray and open it. In this case, we should try
> again to actually eject the media.
> 
> If the first attempt fails to detect a tray_open we will fail with
> error, from monitor.  If we receive that event, we know, that the guest
> properly reacted to the eject request, unlocked the tray and opened it.
> In this case, we need to run the command again to actually eject the
> media from the device.  The reason to call it again is, that QEMU don't
> wait for the guest to react and report an error, that the tray is
> locked.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1147471
> 
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
>  src/qemu/qemu_hotplug.c | 73 +++++++++++++++++++++++--------------------------
>  src/qemu/qemu_process.c |  2 ++
>  2 files changed, 36 insertions(+), 39 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 0628964..17595b7 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -59,7 +59,7 @@
>  
>  VIR_LOG_INIT("qemu.qemu_hotplug");
>  
> -#define CHANGE_MEDIA_RETRIES 10
> +#define CHANGE_MEDIA_TIMEOUT 5000
>  
>  /* Wait up to 5 seconds for device removal to finish. */
>  unsigned long long qemuDomainRemoveDeviceWaitTime = 1000ull * 5;
> @@ -166,12 +166,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>                                 virStorageSourcePtr newsrc,
>                                 bool force)
>  {
> -    int ret = -1;
> +    int ret = -1, rc;
>      char *driveAlias = NULL;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
> -    int retries = CHANGE_MEDIA_RETRIES;
>      const char *format = NULL;
>      char *sourcestr = NULL;
> +    bool ejectRetry = false;
> +    unsigned long long now;
>  
>      if (!disk->info.alias) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -193,36 +194,31 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>      if (!(driveAlias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
>          goto error;
>  
> -    qemuDomainObjEnterMonitor(driver, vm);
> -    ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
> -    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> -        ret = -1;
> -        goto cleanup;
> -    }
> -
> -    if (ret < 0)
> -        goto error;
> +    do {
> +        qemuDomainObjEnterMonitor(driver, vm);
> +        rc = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            goto cleanup;
>  
> -    virObjectRef(vm);
> -    /* we don't want to report errors from media tray_open polling */
> -    while (retries) {
> -        if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
> -            break;
> +        if (rc == -2) {
> +            /* we've already tried, error out */
> +            if (ejectRetry)
> +                goto error;
> +            VIR_DEBUG("tray is locked, wait for the guest to unlock "
> +                      "the tray and try to eject it again");
> +            ejectRetry = true;
> +        } else if (rc < 0) {
> +            goto error;
> +        }
>  
> -        retries--;
> -        virObjectUnlock(vm);
> -        VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d", retries);
> -        usleep(500 * 1000); /* sleep 500ms */
> -        virObjectLock(vm);
> -    }
> -    virObjectUnref(vm);
> +        if (virTimeMillisNow(&now) < 0)
> +            goto error;
>  
> -    if (retries <= 0) {
> -        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -                       _("Unable to eject media"));
> -        ret = -1;
> -        goto error;
> -    }
> +        while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) {
> +            if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0)
> +                goto error;
> +        }

Seems this should be

   if (rc == -2) wait for TRAY_MOVED (need a new event)
   else wait for TRAY_OPEN


> +    } while (ejectRetry && rc != 0);
>  
>      if (!virStorageSourceIsEmpty(newsrc)) {
>          if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0)
> @@ -237,19 +233,17 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>              }
>          }
>          qemuDomainObjEnterMonitor(driver, vm);
> -        ret = qemuMonitorChangeMedia(priv->mon,
> -                                     driveAlias,
> -                                     sourcestr,
> -                                     format);
> -        if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> -            ret = -1;
> +        rc = qemuMonitorChangeMedia(priv->mon,
> +                                    driveAlias,
> +                                    sourcestr,
> +                                    format);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
>              goto cleanup;
> -        }
>      }
>  
> -    virDomainAuditDisk(vm, disk->src, newsrc, "update", ret >= 0);
> +    virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);
>  
> -    if (ret < 0)
> +    if (rc < 0)
>          goto error;
>  
>      /* remove the old source from shared device list */
> @@ -259,6 +253,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>      virStorageSourceFree(disk->src);
>      disk->src = newsrc;
>      newsrc = NULL;
> +    ret = 0;
>  
>   cleanup:
>      VIR_FREE(driveAlias);
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a688feb..648ba00 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1152,6 +1152,8 @@ qemuProcessHandleTrayChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>              VIR_WARN("Unable to save status on vm %s after tray moved event",
>                       vm->def->name);
>          }

Seems there should be some sort of DEVICE_TRAY_MOVED event... as well as
OPEN and CLOSE

John

> +
> +        virDomainObjBroadcast(vm);
>      }
>  
>      virObjectUnlock(vm);
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]