[libvirt] [PATCH RFC 07/51] qemu: Consolidate disk blockjob variables into a structure

Peter Krempa pkrempa at redhat.com
Wed Dec 12 17:08:23 UTC 2018


Struct qemuDomainDiskPrivate was holding multiple variables connected to
a disk block job. Consolidate them into a new struct qemuBlockJobData.

This will also allow simpler extensions to the block job mechanisms.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/qemu/qemu_blockjob.c | 36 +++++++++++++++++++++++-------------
 src/qemu/qemu_blockjob.h | 15 +++++++++++++++
 src/qemu/qemu_domain.c   | 12 +++++++++---
 src/qemu/qemu_domain.h   |  9 ++-------
 src/qemu/qemu_driver.c   | 16 ++++++++--------
 src/qemu/qemu_process.c  | 15 ++++++++-------
 6 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 1b6d16cbb9..b1a30b1edb 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -42,6 +42,17 @@
 VIR_LOG_INIT("qemu.qemu_blockjob");


+void
+qemuBlockJobDataFree(qemuBlockJobDataPtr job)
+{
+    if (!job)
+        return;
+
+    VIR_FREE(job->errmsg);
+    VIR_FREE(job);
+}
+
+
 /**
  * qemuBlockJobEmitEvents:
  *
@@ -160,7 +171,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
         virStorageSourceBackingStoreClear(disk->src);
         ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true));
         ignore_value(qemuBlockNodeNamesDetect(driver, vm, asyncJob));
-        diskPriv->blockjob = false;
+        diskPriv->blockjob->started = false;
         break;

     case VIR_DOMAIN_BLOCK_JOB_READY:
@@ -176,7 +187,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
         }
         disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
         disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
-        diskPriv->blockjob = false;
+        diskPriv->blockjob->started = false;
         break;

     case VIR_DOMAIN_BLOCK_JOB_LAST:
@@ -213,22 +224,21 @@ qemuBlockJobUpdateDisk(virDomainObjPtr vm,
                        virDomainDiskDefPtr disk,
                        char **error)
 {
-    qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+    qemuBlockJobDataPtr job = QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int status = diskPriv->blockJobStatus;
+    int status = job->status;

     if (error)
         *error = NULL;

     if (status != -1) {
         qemuBlockJobEventProcess(priv->driver, vm, disk, asyncJob,
-                                 diskPriv->blockJobType,
-                                 diskPriv->blockJobStatus);
-        diskPriv->blockJobStatus = -1;
+                                 job->type, status);
+        job->status = -1;
         if (error)
-            VIR_STEAL_PTR(*error, diskPriv->blockJobError);
+            VIR_STEAL_PTR(*error, job->errmsg);
         else
-            VIR_FREE(diskPriv->blockJobError);
+            VIR_FREE(job->errmsg);
     }

     return status;
@@ -251,11 +261,11 @@ qemuBlockJobUpdateDisk(virDomainObjPtr vm,
 void
 qemuBlockJobSyncBeginDisk(virDomainDiskDefPtr disk)
 {
-    qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+    qemuBlockJobDataPtr job = QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob;

     VIR_DEBUG("disk=%s", disk->dst);
-    diskPriv->blockJobSync = true;
-    diskPriv->blockJobStatus = -1;
+    job->synchronous = true;
+    job->status = -1;
 }


@@ -274,5 +284,5 @@ qemuBlockJobSyncEndDisk(virDomainObjPtr vm,
 {
     VIR_DEBUG("disk=%s", disk->dst);
     qemuBlockJobUpdateDisk(vm, asyncJob, disk, NULL);
-    QEMU_DOMAIN_DISK_PRIVATE(disk)->blockJobSync = false;
+    QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob->synchronous = false;
 }
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index 0c440757f2..1479c6f720 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -25,6 +25,21 @@
 # include "internal.h"
 # include "qemu_conf.h"

+
+typedef struct _qemuBlockJobData qemuBlockJobData;
+typedef qemuBlockJobData *qemuBlockJobDataPtr;
+
+struct _qemuBlockJobData {
+    bool started;
+    int type;
+    int status;
+    char *errmsg;
+    bool synchronous; /* API call is waiting for this job */
+};
+
+void
+qemuBlockJobDataFree(qemuBlockJobDataPtr job);
+
 int qemuBlockJobUpdateDisk(virDomainObjPtr vm,
                            int asyncJob,
                            virDomainDiskDefPtr disk,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1eb0e31df0..5a64231f95 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1062,6 +1062,11 @@ qemuDomainDiskPrivateNew(void)
     if (!(priv = virObjectNew(qemuDomainDiskPrivateClass)))
         return NULL;

+    if (VIR_ALLOC(priv->blockjob) < 0) {
+        virObjectUnref(priv);
+        priv = NULL;
+    }
+
     return (virObjectPtr) priv;
 }

@@ -1070,10 +1075,10 @@ qemuDomainDiskPrivateDispose(void *obj)
 {
     qemuDomainDiskPrivatePtr priv = obj;

-    VIR_FREE(priv->blockJobError);
     virStorageSourceFree(priv->migrSource);
     VIR_FREE(priv->qomName);
     VIR_FREE(priv->nodeCopyOnRead);
+    qemuBlockJobDataFree(priv->blockjob);
 }

 static virClassPtr qemuDomainStorageSourcePrivateClass;
@@ -9255,7 +9260,8 @@ qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk)
         return true;
     }

-    if (diskPriv->blockjob) {
+    if (diskPriv->blockjob &&
+        diskPriv->blockjob->started) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("disk '%s' already in active block job"),
                        disk->dst);
@@ -9284,7 +9290,7 @@ qemuDomainHasBlockjob(virDomainObjPtr vm,
         virDomainDiskDefPtr disk = vm->def->disks[i];
         qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);

-        if (!copy_only && diskPriv->blockjob)
+        if (!copy_only && diskPriv->blockjob && diskPriv->blockjob->started)
             return true;

         if (disk->mirror && disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 53b5ea1678..a03950e77b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -32,6 +32,7 @@
 # include "snapshot_conf.h"
 # include "qemu_monitor.h"
 # include "qemu_agent.h"
+# include "qemu_blockjob.h"
 # include "qemu_conf.h"
 # include "qemu_capabilities.h"
 # include "qemu_migration_params.h"
@@ -388,13 +389,7 @@ struct _qemuDomainDiskPrivate {
     /* ideally we want a smarter way to interlock block jobs on single qemu disk
      * in the future, but for now we just disallow any concurrent job on a
      * single disk */
-    bool blockjob;
-
-    /* for some synchronous block jobs, we need to notify the owner */
-    int blockJobType;   /* type of the block job from the event */
-    int blockJobStatus; /* status of the finished block job */
-    char *blockJobError; /* block job completed event error */
-    bool blockJobSync; /* the block job needs synchronized termination */
+    qemuBlockJobDataPtr blockjob;

     bool migrating; /* the disk is being migrated */
     virStorageSourcePtr migrSource; /* disk source object used for NBD migration */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b2a2d70ec..d06eabb265 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4716,7 +4716,7 @@ processBlockJobEvent(virQEMUDriverPtr driver,
                      int status)
 {
     virDomainDiskDefPtr disk;
-    qemuDomainDiskPrivatePtr diskPriv;
+    qemuBlockJobDataPtr job;

     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
         return;
@@ -4731,10 +4731,10 @@ processBlockJobEvent(virQEMUDriverPtr driver,
         goto endjob;
     }

-    diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+    job = QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob;

-    diskPriv->blockJobType = type;
-    diskPriv->blockJobStatus = status;
+    job->type = type;
+    job->status = status;

     qemuBlockJobUpdateDisk(vm, QEMU_ASYNC_JOB_NONE, disk, NULL);

@@ -17294,7 +17294,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
     if (ret < 0)
         goto endjob;

-    QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = true;
+    QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob->started = true;

     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
         VIR_WARN("Unable to save status on vm %s after state change",
@@ -17401,7 +17401,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
     if (!async) {
         qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
         qemuBlockJobUpdateDisk(vm, QEMU_ASYNC_JOB_NONE, disk, NULL);
-        while (diskPriv->blockjob) {
+        while (diskPriv->blockjob->started) {
             if (virDomainObjWait(vm) < 0) {
                 ret = -1;
                 goto endjob;
@@ -17825,7 +17825,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
     disk->mirror = mirror;
     mirror = NULL;
     disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
-    QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = true;
+    QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob->started = true;

     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
         VIR_WARN("Unable to save status on vm %s after state change",
@@ -18225,7 +18225,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
     }

     if (ret == 0) {
-        QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = true;
+        QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob->started = true;
         mirror = NULL;
     } else {
         disk->mirror = NULL;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2f8e19d29d..72725fcdf1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -926,7 +926,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     virQEMUDriverPtr driver = opaque;
     struct qemuProcessEvent *processEvent = NULL;
     virDomainDiskDefPtr disk;
-    qemuDomainDiskPrivatePtr diskPriv;
+    qemuBlockJobDataPtr job;
     char *data = NULL;

     virObjectLock(vm);
@@ -936,14 +936,15 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,

     if (!(disk = qemuProcessFindDomainDiskByAliasOrQOM(vm, diskAlias, NULL)))
         goto error;
-    diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);

-    if (diskPriv->blockJobSync) {
+    job = QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob;
+
+    if (job->synchronous) {
         /* We have a SYNC API waiting for this event, dispatch it back */
-        diskPriv->blockJobType = type;
-        diskPriv->blockJobStatus = status;
-        VIR_FREE(diskPriv->blockJobError);
-        ignore_value(VIR_STRDUP_QUIET(diskPriv->blockJobError, error));
+        job->type = type;
+        job->status = status;
+        VIR_FREE(job->errmsg);
+        ignore_value(VIR_STRDUP_QUIET(job->errmsg, error));
         virDomainObjBroadcast(vm);
     } else {
         /* there is no waiting SYNC API, dispatch the update to a thread */
-- 
2.19.2




More information about the libvir-list mailing list