[PATCH 2/5] qemu: Add free and copy function for qemuDomainJobInfo and use it

Peter Krempa pkrempa at redhat.com
Thu Apr 16 09:55:43 UTC 2020


In order to add a string to qemuDomainJobInfo we must ensure that it's
freed and copied properly. Add helpers to copy and free the structure
and adjust the code to use them properly for the new semantics.

Additionally also allocation is changed to g_new0 as it includes the
type and thus it's very easy to grep for all the allocations of a given
type.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/qemu/qemu_backup.c           |  5 ++---
 src/qemu/qemu_domain.c           | 26 +++++++++++++++++-----
 src/qemu/qemu_domain.h           |  8 +++++++
 src/qemu/qemu_driver.c           | 37 ++++++++++++++++----------------
 src/qemu/qemu_migration.c        | 16 ++++++--------
 src/qemu/qemu_migration_cookie.c | 10 ++++-----
 6 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 5d18720f53..03d34c9378 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -641,9 +641,8 @@ qemuBackupJobTerminate(virDomainObjPtr vm,

     qemuDomainJobInfoUpdateTime(priv->job.current);

-    g_free(priv->job.completed);
-    priv->job.completed = g_new0(qemuDomainJobInfo, 1);
-    *priv->job.completed = *priv->job.current;
+    g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
+    priv->job.completed = qemuDomainJobInfoCopy(priv->job.current);

     priv->job.completed->stats.backup.total = priv->backup->push_total;
     priv->job.completed->stats.backup.transferred = priv->backup->push_transferred;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 91e234d644..13b1c4e402 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -305,6 +305,23 @@ qemuDomainDisableNamespace(virDomainObjPtr vm,
 }


+void
+qemuDomainJobInfoFree(qemuDomainJobInfoPtr info)
+{
+    g_free(info);
+}
+
+
+qemuDomainJobInfoPtr
+qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info)
+{
+    qemuDomainJobInfoPtr ret = g_new0(qemuDomainJobInfo, 1);
+
+    memcpy(ret, info, sizeof(*info));
+
+    return ret;
+}
+
 void
 qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver,
                                 virDomainObjPtr vm)
@@ -385,7 +402,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
     job->spiceMigrated = false;
     job->dumpCompleted = false;
     VIR_FREE(job->error);
-    VIR_FREE(job->current);
+    g_clear_pointer(&job->current, qemuDomainJobInfoFree);
     qemuMigrationParamsFree(job->migParams);
     job->migParams = NULL;
     job->apiFlags = 0;
@@ -415,8 +432,8 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
 {
     qemuDomainObjResetJob(priv);
     qemuDomainObjResetAsyncJob(priv);
-    VIR_FREE(priv->job.current);
-    VIR_FREE(priv->job.completed);
+    g_clear_pointer(&priv->job.current, qemuDomainJobInfoFree);
+    g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
     virCondDestroy(&priv->job.cond);
     virCondDestroy(&priv->job.asyncCond);
 }
@@ -6291,8 +6308,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
                       qemuDomainAsyncJobTypeToString(asyncJob),
                       obj, obj->def->name);
             qemuDomainObjResetAsyncJob(priv);
-            if (VIR_ALLOC(priv->job.current) < 0)
-                goto cleanup;
+            priv->job.current = g_new0(qemuDomainJobInfo, 1);
             priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
             priv->job.asyncJob = asyncJob;
             priv->job.asyncOwner = virThreadSelfID();
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cf19f4d101..c7f28b04c2 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -177,6 +177,14 @@ struct _qemuDomainJobInfo {
     qemuDomainMirrorStats mirrorStats;
 };

+void
+qemuDomainJobInfoFree(qemuDomainJobInfoPtr info);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainJobInfo, qemuDomainJobInfoFree);
+
+qemuDomainJobInfoPtr
+qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info);
+
 typedef struct _qemuDomainJobObj qemuDomainJobObj;
 typedef qemuDomainJobObj *qemuDomainJobObjPtr;
 struct _qemuDomainJobObj {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 31f199fdef..63e92b84c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3746,7 +3746,7 @@ qemuDumpToFd(virQEMUDriverPtr driver,
     if (detach)
         priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP;
     else
-        VIR_FREE(priv->job.current);
+        g_clear_pointer(&priv->job.current, qemuDomainJobInfoFree);

     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         return -1;
@@ -13598,16 +13598,16 @@ static int
 qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
                               virDomainObjPtr vm,
                               bool completed,
-                              qemuDomainJobInfoPtr jobInfo)
+                              qemuDomainJobInfoPtr *jobInfo)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     int ret = -1;

+    *jobInfo = NULL;
+
     if (completed) {
         if (priv->job.completed && !priv->job.current)
-            *jobInfo = *priv->job.completed;
-        else
-            jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
+            *jobInfo = qemuDomainJobInfoCopy(priv->job.completed);

         return 0;
     }
@@ -13626,26 +13626,25 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
         goto cleanup;

     if (!priv->job.current) {
-        jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
         ret = 0;
         goto cleanup;
     }
-    *jobInfo = *priv->job.current;
+    *jobInfo = qemuDomainJobInfoCopy(priv->job.current);

-    switch (jobInfo->statsType) {
+    switch ((*jobInfo)->statsType) {
     case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION:
     case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
-        if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0)
+        if (qemuDomainGetJobInfoMigrationStats(driver, vm, *jobInfo) < 0)
             goto cleanup;
         break;

     case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP:
-        if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0)
+        if (qemuDomainGetJobInfoDumpStats(driver, vm, *jobInfo) < 0)
             goto cleanup;
         break;

     case QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP:
-        if (qemuBackupGetJobInfoStats(driver, vm, jobInfo) < 0)
+        if (qemuBackupGetJobInfoStats(driver, vm, *jobInfo) < 0)
             goto cleanup;
         break;

@@ -13666,7 +13665,7 @@ qemuDomainGetJobInfo(virDomainPtr dom,
                      virDomainJobInfoPtr info)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
-    qemuDomainJobInfo jobInfo;
+    g_autoptr(qemuDomainJobInfo) jobInfo = NULL;
     virDomainObjPtr vm;
     int ret = -1;

@@ -13681,12 +13680,13 @@ qemuDomainGetJobInfo(virDomainPtr dom,
     if (qemuDomainGetJobStatsInternal(driver, vm, false, &jobInfo) < 0)
         goto cleanup;

-    if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) {
+    if (!jobInfo ||
+        jobInfo->status == QEMU_DOMAIN_JOB_STATUS_NONE) {
         ret = 0;
         goto cleanup;
     }

-    ret = qemuDomainJobInfoToInfo(&jobInfo, info);
+    ret = qemuDomainJobInfoToInfo(jobInfo, info);

  cleanup:
     virDomainObjEndAPI(&vm);
@@ -13704,7 +13704,7 @@ qemuDomainGetJobStats(virDomainPtr dom,
     virQEMUDriverPtr driver = dom->conn->privateData;
     virDomainObjPtr vm;
     qemuDomainObjPrivatePtr priv;
-    qemuDomainJobInfo jobInfo;
+    g_autoptr(qemuDomainJobInfo) jobInfo = NULL;
     bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
     int ret = -1;

@@ -13721,7 +13721,8 @@ qemuDomainGetJobStats(virDomainPtr dom,
     if (qemuDomainGetJobStatsInternal(driver, vm, completed, &jobInfo) < 0)
         goto cleanup;

-    if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) {
+    if (!jobInfo ||
+        jobInfo->status == QEMU_DOMAIN_JOB_STATUS_NONE) {
         *type = VIR_DOMAIN_JOB_NONE;
         *params = NULL;
         *nparams = 0;
@@ -13729,10 +13730,10 @@ qemuDomainGetJobStats(virDomainPtr dom,
         goto cleanup;
     }

-    ret = qemuDomainJobInfoToParams(&jobInfo, type, params, nparams);
+    ret = qemuDomainJobInfoToParams(jobInfo, type, params, nparams);

     if (completed && ret == 0 && !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED))
-        VIR_FREE(priv->job.completed);
+        g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);

  cleanup:
     virDomainObjEndAPI(&vm);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index bc280e856a..02e8271e42 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1724,11 +1724,9 @@ qemuMigrationSrcWaitForCompletion(virQEMUDriverPtr driver,

     qemuDomainJobInfoUpdateTime(jobInfo);
     qemuDomainJobInfoUpdateDowntime(jobInfo);
-    VIR_FREE(priv->job.completed);
-    if (VIR_ALLOC(priv->job.completed) == 0) {
-        *priv->job.completed = *jobInfo;
-        priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
-    }
+    g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
+    priv->job.completed = qemuDomainJobInfoCopy(jobInfo);
+    priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;

     if (asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT &&
         jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED)
@@ -3017,7 +3015,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
     if (retcode == 0)
         jobInfo = priv->job.completed;
     else
-        VIR_FREE(priv->job.completed);
+        g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);

     /* Update times with the values sent by the destination daemon */
     if (mig->jobInfo && jobInfo) {
@@ -5036,7 +5034,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
                                        : QEMU_MIGRATION_PHASE_FINISH2);

     qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup);
-    VIR_FREE(priv->job.completed);
+    g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);

     cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK |
                    QEMU_MIGRATION_COOKIE_STATS |
@@ -5257,7 +5255,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
          * is obsolete anyway.
          */
         if (inPostCopy)
-            VIR_FREE(priv->job.completed);
+            g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
     }

     qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
@@ -5268,7 +5266,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
         qemuDomainRemoveInactiveJob(driver, vm);

  cleanup:
-    VIR_FREE(jobInfo);
+    g_clear_pointer(&jobInfo, qemuDomainJobInfoFree);
     virPortAllocatorRelease(port);
     if (priv->mon)
         qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 1d88ac1d22..fb8b5bcd92 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -123,7 +123,7 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
     VIR_FREE(mig->name);
     VIR_FREE(mig->lockState);
     VIR_FREE(mig->lockDriver);
-    VIR_FREE(mig->jobInfo);
+    g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree);
     virCPUDefFree(mig->cpu);
     qemuMigrationCookieCapsFree(mig->caps);
     VIR_FREE(mig);
@@ -513,10 +513,9 @@ qemuMigrationCookieAddStatistics(qemuMigrationCookiePtr mig,
     if (!priv->job.completed)
         return 0;

-    if (!mig->jobInfo && VIR_ALLOC(mig->jobInfo) < 0)
-        return -1;
+    g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree);
+    mig->jobInfo = qemuDomainJobInfoCopy(priv->job.completed);

-    *mig->jobInfo = *priv->job.completed;
     mig->flags |= QEMU_MIGRATION_COOKIE_STATS;

     return 0;
@@ -1051,8 +1050,7 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt)
     if (!(ctxt->node = virXPathNode("./statistics", ctxt)))
         return NULL;

-    if (VIR_ALLOC(jobInfo) < 0)
-        return NULL;
+    jobInfo = g_new0(qemuDomainJobInfo, 1);

     stats = &jobInfo->stats.mig;
     jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
-- 
2.26.0




More information about the libvir-list mailing list