[libvirt] [PATCH] virsh: improve waiting for block job readiness

[Michael Chapman]

Wait for a block job event after the job has reached 100% only if
exactly one of the BLOCK_JOB and BLOCK_JOB_2 callbacks were successfully
registered.

If neither callback was registered, then no event will ever be received.
If both were registered, then any user-supplied path is guaranteed to
match one of the events.

Signed-off-by: Michael Chapman <mike at very.puzzling.org>
---

I have found that even a 2.5 second timeout isn't always sufficiently
long for QEMU to flush a disk at the end of a block job.

I hope I've understood the code properly here, because as far as I can
tell the comment I'm removing in this commit isn't right. The path the
user supplies *has* to be either the <source file='name'/> or <target
dev='name'/> exactly in order for the disk to be matched, and these are
precisely the two strings used by the two events.

 tools/virsh-domain.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index edbbc34..60de9ba 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1942,18 +1942,20 @@ virshBlockJobWait(virshBlockJobWaitDataPtr data)
             goto cleanup;
         }
 
-        /* since virsh can't guarantee that the path provided by the user will
-         * later be matched in the event we will need to keep the fallback
-         * approach and claim success if the block job finishes or vanishes. */
-        if (result == 0)
-            break;
+        /* if either event could not be registered we can't guarantee that the
+         * path provided by the user will be matched, so keep the fallback
+         * approach and claim success if the block job finishes or vanishes */
+        if (data->cb_id2 < 0 || data->cb_id2 < 0) {
+            if (result == 0)
+                break;
 
-        /* for two-phase jobs we will try to wait in the synchronized phase
-         * for event arrival since 100% completion doesn't necessarily mean that
-         * the block job has finished and can be terminated with success */
-        if (info.end == info.cur && --retries == 0) {
-            ret = VIR_DOMAIN_BLOCK_JOB_READY;
-            goto cleanup;
+            /* only wait in the synchronized phase for event arrival if
+             * either callback was registered */
+            if (info.end == info.cur &&
+                ((data->cb_id2 < 0 && data->cb_id2 < 0) || --retries == 0)) {
+                ret = VIR_DOMAIN_BLOCK_JOB_READY;
+                goto cleanup;
+            }
         }
 
         if (data->verbose)
-- 
2.4.3