From 72186f9c8c63983c55adb5194dda739ddc0f4043 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Thu, 26 Mar 2020 17:55:00 +0100 Subject: [PATCH] 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 Reviewed-by: Eric Blake --- 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 3d075bca26..e1e14d2ca8 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); } @@ -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 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 00d276061e..5ff4ee7389 100644 --- 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 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;