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

[Pavel Hrdina]

On Tue, Mar 22, 2016 at 10:18:22AM +0100, Peter Krempa wrote:
> 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.

Sure, I'll remove it.

> 
> >                  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.

Well, that's true, we shouldn't report that error, but we already do and this
patch fixes the eject media bug.  This is for different patch.

Pavel