On Wed, Jul 01, 2015 at 05:39:49PM -0400, John Ferlan wrote:
>
>
> 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 <phrd...@redhat.com>
> > ---
> > 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
>
There is no event TRAY_OPEN or TRAY_CLOSE. QEMU has only one event,
DEVICE_TRAY_MOVED. This event is handled by qemuMonitorJSONHandleTrayChange,
which will set the reason to OPEN/CLOSE and emit a qemu monitor event. For this
qemu monitor event there is a handler called qemuProcessHandleTrayChange, where
the tray_status is set to VIR_DOMAIN_DISK_TRAY_OPEN or
VIR_DOMAIN_DISK_TRAY_CLOSED. This handler also wakes up that condition we are
waiting in this code. There is no need for a new event. We are already using
everything, that is available.
Pavel
>
> > + } 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
