[PATCH] qemu: Don't report error on successful media eject

[Cole Robinson <crobinso@xxxxxxxxxx>]

If we are just ejecting media, ret == -1 even after the retry loop
determines that the tray is open, as requested. This means media
disconnect always report's error.

Fix it, and fix some other mini issues:

- Don't overwrite the 'eject' error message if the retry loop fails
- Move the retries decrement inside the loop, otherwise the final loop
  might succeed, yet retries == 0 and we will raise error
- Setting ret = -1 in the disk->src check is unneeded
- Fix comment typos

cc: mprivozn@xxxxxxxxxx
---
 src/qemu/qemu_hotplug.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1ed61db..3b277ef 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -98,10 +98,11 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 
     virObjectRef(vm);
     /* we don't want to report errors from media tray_open polling */
-    while (retries--) {
+    while (retries) {
         if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
             break;
 
+        retries--;
         virObjectUnlock(vm);
         VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d", retries);
         usleep(500 * 1000); /* sleep 500ms */
@@ -109,19 +110,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     }
     virObjectUnref(vm);
 
-    if (disk->src) {
-        /* deliberately don't depend on 'ret' as 'eject' may have failed for the
-         * fist time and we are gonna check the drive state anyway */
-        const char *format = NULL;
-
-        /* We haven't succeeded yet */
-        ret = -1;
-
-        if (retries <= 0) {
+    if (retries <= 0) {
+        if (ret == 0) {
+            /* If ret == -1, EjectMedia already set an error message */
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                           _("Unable to eject media before changing it"));
-            goto audit;
+                           _("Unable to eject media"));
         }
+        goto audit;
+    }
+    ret = 0;
+
+    if (disk->src) {
+        /* deliberately don't depend on 'ret' as 'eject' may have failed the
+         * first time and we are gonna check the drive state anyway */
+        const char *format = NULL;
 
         if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) {
             if (disk->format > 0)
-- 
1.8.2.1

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