qemu: Add free and copy function for qemuDomainJobInfo and use it

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@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 5d18720f53b70b8e28a73df9f215c088fe851783..03d34c9378b2dcac347bc71f21ff67d9561d1907 100644 (file)
--- 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 3d075bca2685edd143dada7198c31778c55eb1b9..e1e14d2ca82a14b3efa51fb47caa97a46163697e 100644 (file)
--- 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);
 }
@@ -6299,8 +6316,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 cf19f4d101180355d922d5194674fe536a3a20fd..c7f28b04c2b18592aad58d7a6b429102015585fc 100644 (file)
--- 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 00d276061e475efd82e91ddf1f84867154a87cc7..5ff4ee7389a695d3632151bfe7be0f28df039036 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3747,7 +3747,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 bc280e856a096db8abef648c5e0ee07a62719a9d..02e8271e42378e761fb2367c1e6819eda31d736e 100644 (file)
--- 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 1d88ac1d2207466985676cee1de1f340a29ee232..fb8b5bcd92f0038f8cc379e535f4a0dac0cbe5fe 100644 (file)
--- 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;