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

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(a)redhat.com&gt; > --- > 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.