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@xxxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list