From a0cfb6daa757761c0397dc0ceb211b64891a1983 Mon Sep 17 00:00:00 2001

Message-Id: <a0cfb6daa757761c0397dc0ceb211b64891a1983@dist-git>

From: Peter Krempa <pkrempa@redhat.com>

Date: Thu, 2 Apr 2015 11:27:59 +0200

Subject: [PATCH] qemu: blockjob: Synchronously update backing chain in XML on

 ABORT/PIVOT

https://bugzilla.redhat.com/show_bug.cgi?id=1208021

When the synchronous pivot option is selected, libvirt would not update

the backing chain until the job was exitted. Some applications then

received invalid data as their job serialized first.

This patch removes polling to wait for the ABORT/PIVOT job completion

and replaces it with a condition. If a synchronous operation is

requested the update of the XML is executed in the job of the caller of

the synchronous request. Otherwise the monitor event callback uses a

separate worker to update the backing chain with a new job.

This is a regression since 1a92c719101e5bfa6fe2b78006ad04c7f075ea28

(downstream commit 12fdae1ebb74296a4db3b191f16dfda757024b8f)

When the ABORT job is finished synchronously you get the following call

stack:

 #0  qemuBlockJobEventProcess

 #1  qemuDomainBlockJobImpl

 #2  qemuDomainBlockJobAbort

 #3  virDomainBlockJobAbort

While previously or while using the _ASYNC flag you'd get:

 #0  qemuBlockJobEventProcess

 #1  processBlockJobEvent

 #2  qemuProcessEventHandler

 #3  virThreadPoolWorker

(cherry picked from commit 630ee5ac6cf4e3be3f3e986897a289865dd2604b)

Conflicts:

	src/qemu/qemu_driver.c - context: The deleted hunk that was

        polling for the block job state was not yet converted to the new

        locking scheme downstream.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/conf/domain_conf.c  | 16 +++++++++++++++-

 src/conf/domain_conf.h  |  6 ++++++

 src/qemu/qemu_driver.c  | 45 +++++++++++++++++++--------------------------

 src/qemu/qemu_process.c | 38 +++++++++++++++++++++++++-------------

 4 files changed, 65 insertions(+), 40 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 9bfffd0..c7059d2 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -1238,9 +1238,22 @@ virDomainDiskDefNew(void)

     if (VIR_ALLOC(ret) < 0)

         return NULL;

+

     if (VIR_ALLOC(ret->src) < 0)

-        VIR_FREE(ret);

+        goto error;

+

+    if (virCondInit(&ret->blockJobSyncCond) < 0) {

+        virReportSystemError(errno, "%s", _("Failed to initialize condition"));

+        goto error;

+    }

+

     return ret;

+

+ error:

+    virStorageSourceFree(ret->src);

+    VIR_FREE(ret);

+

+    return NULL;

 }

@@ -1258,6 +1271,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def)

     VIR_FREE(def->vendor);

     VIR_FREE(def->product);

     virDomainDeviceInfoClear(&def->info);

+    virCondDestroy(&def->blockJobSyncCond);

     VIR_FREE(def);

 }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

index 654c27d..d672294 100644

--- a/src/conf/domain_conf.h

+++ b/src/conf/domain_conf.h

@@ -644,6 +644,12 @@ struct _virDomainDiskDef {

     int mirrorState; /* enum virDomainDiskMirrorState */

     int mirrorJob; /* virDomainBlockJobType */

+    /* for some synchronous block jobs, we need to notify the owner */

+    virCond blockJobSyncCond;

+    int blockJobType;   /* type of the block job from the event */

+    int blockJobStatus; /* status of the finished block job */

+    bool blockJobSync; /* the block job needs synchronized termination */

+

     struct {

         unsigned int cylinders;

         unsigned int heads;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index 490650e..5f7fedc 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -15693,6 +15693,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,

         goto endjob;

     if (mode == BLOCK_JOB_ABORT) {

+        if (async && !(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {

+            /* prepare state for event delivery */

+            disk->blockJobStatus = -1;

+            disk->blockJobSync = true;

+        }

+

         if ((flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) &&

             !(async && disk->mirror)) {

             virReportError(VIR_ERR_OPERATION_INVALID,

@@ -15802,36 +15808,23 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,

                                                      status);

             event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,

                                                        status);

-        } else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {

+        } else if (disk->blockJobSync) {

             /* XXX If the event reports failure, we should reflect

              * that back into the return status of this API call.  */

-            while (1) {

-                /* Poll every 50ms */

-                static struct timespec ts = { .tv_sec = 0,

-                                              .tv_nsec = 50 * 1000 * 1000ull };

-                virDomainBlockJobInfo dummy;

-

-                qemuDomainObjEnterMonitor(driver, vm);

-                ret = qemuMonitorBlockJob(priv->mon, device, NULL, NULL, 0,

-                                          &dummy, BLOCK_JOB_INFO, async);

-                qemuDomainObjExitMonitor(driver, vm);

-

-                if (ret <= 0)

-                    break;

-

-                virObjectUnlock(vm);

-

-                nanosleep(&ts, NULL);

-

-                virObjectLock(vm);

-

-                if (!virDomainObjIsActive(vm)) {

-                    virReportError(VIR_ERR_OPERATION_INVALID, "%s",

-                                   _("domain is not running"));

-                    ret = -1;

-                    break;

+            while (disk->blockJobStatus == -1 && disk->blockJobSync) {

+                if (virCondWait(&disk->blockJobSyncCond, &vm->parent.lock) < 0) {

+                    virReportSystemError(errno, "%s",

+                                         _("Unable to wait on block job sync "

+                                           "condition"));

+                    disk->blockJobSync = false;

+                    goto endjob;

                 }

             }

+

+            qemuBlockJobEventProcess(driver, vm, disk,

+                                     disk->blockJobType,

+                                     disk->blockJobStatus);

+            disk->blockJobSync = false;

         }

     }

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c

index 83a59a1..49b5df4 100644

--- a/src/qemu/qemu_process.c

+++ b/src/qemu/qemu_process.c

@@ -1025,28 +1025,40 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,

 {

     virQEMUDriverPtr driver = opaque;

     struct qemuProcessEvent *processEvent = NULL;

-    char *data;

+    virDomainDiskDefPtr disk;

+    char *data = NULL;

     virObjectLock(vm);

     VIR_DEBUG("Block job for device %s (domain: %p,%s) type %d status %d",

               diskAlias, vm, vm->def->name, type, status);

-    if (VIR_ALLOC(processEvent) < 0)

+    if (!(disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias)))

         goto error;

-    processEvent->eventType = QEMU_PROCESS_EVENT_BLOCK_JOB;

-    if (VIR_STRDUP(data, diskAlias) < 0)

-        goto error;

-    processEvent->data = data;

-    processEvent->vm = vm;

-    processEvent->action = type;

-    processEvent->status = status;

+    if (disk->blockJobSync) {

+        disk->blockJobType = type;

+        disk->blockJobStatus = status;

+        /* We have an SYNC API waiting for this event, dispatch it back */

+        virCondSignal(&disk->blockJobSyncCond);

+    } else {

+        /* there is no waiting SYNC API, dispatch the update to a thread */

+        if (VIR_ALLOC(processEvent) < 0)

+            goto error;

-    virObjectRef(vm);

-    if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {

-        ignore_value(virObjectUnref(vm));

-        goto error;

+        processEvent->eventType = QEMU_PROCESS_EVENT_BLOCK_JOB;

+        if (VIR_STRDUP(data, diskAlias) < 0)

+            goto error;

+        processEvent->data = data;

+        processEvent->vm = vm;

+        processEvent->action = type;

+        processEvent->status = status;

+

+        virObjectRef(vm);

+        if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {

+            ignore_value(virObjectUnref(vm));

+            goto error;

+        }

     }

  cleanup:

-- 

2.3.5