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>
This commit is contained in:
Peter Krempa 2020-03-26 17:55:00 +01:00
parent e77a094141
commit 72186f9c8c
6 changed files with 61 additions and 41 deletions

View File

@ -641,9 +641,8 @@ qemuBackupJobTerminate(virDomainObjPtr vm,
qemuDomainJobInfoUpdateTime(priv->job.current); qemuDomainJobInfoUpdateTime(priv->job.current);
g_free(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
priv->job.completed = g_new0(qemuDomainJobInfo, 1); priv->job.completed = qemuDomainJobInfoCopy(priv->job.current);
*priv->job.completed = *priv->job.current;
priv->job.completed->stats.backup.total = priv->backup->push_total; priv->job.completed->stats.backup.total = priv->backup->push_total;
priv->job.completed->stats.backup.transferred = priv->backup->push_transferred; priv->job.completed->stats.backup.transferred = priv->backup->push_transferred;

View File

@ -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 void
qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver, qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver,
virDomainObjPtr vm) virDomainObjPtr vm)
@ -385,7 +402,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
job->spiceMigrated = false; job->spiceMigrated = false;
job->dumpCompleted = false; job->dumpCompleted = false;
VIR_FREE(job->error); VIR_FREE(job->error);
VIR_FREE(job->current); g_clear_pointer(&job->current, qemuDomainJobInfoFree);
qemuMigrationParamsFree(job->migParams); qemuMigrationParamsFree(job->migParams);
job->migParams = NULL; job->migParams = NULL;
job->apiFlags = 0; job->apiFlags = 0;
@ -415,8 +432,8 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
{ {
qemuDomainObjResetJob(priv); qemuDomainObjResetJob(priv);
qemuDomainObjResetAsyncJob(priv); qemuDomainObjResetAsyncJob(priv);
VIR_FREE(priv->job.current); g_clear_pointer(&priv->job.current, qemuDomainJobInfoFree);
VIR_FREE(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
virCondDestroy(&priv->job.cond); virCondDestroy(&priv->job.cond);
virCondDestroy(&priv->job.asyncCond); virCondDestroy(&priv->job.asyncCond);
} }
@ -6299,8 +6316,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
qemuDomainAsyncJobTypeToString(asyncJob), qemuDomainAsyncJobTypeToString(asyncJob),
obj, obj->def->name); obj, obj->def->name);
qemuDomainObjResetAsyncJob(priv); qemuDomainObjResetAsyncJob(priv);
if (VIR_ALLOC(priv->job.current) < 0) priv->job.current = g_new0(qemuDomainJobInfo, 1);
goto cleanup;
priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
priv->job.asyncJob = asyncJob; priv->job.asyncJob = asyncJob;
priv->job.asyncOwner = virThreadSelfID(); priv->job.asyncOwner = virThreadSelfID();

View File

@ -177,6 +177,14 @@ struct _qemuDomainJobInfo {
qemuDomainMirrorStats mirrorStats; qemuDomainMirrorStats mirrorStats;
}; };
void
qemuDomainJobInfoFree(qemuDomainJobInfoPtr info);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainJobInfo, qemuDomainJobInfoFree);
qemuDomainJobInfoPtr
qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info);
typedef struct _qemuDomainJobObj qemuDomainJobObj; typedef struct _qemuDomainJobObj qemuDomainJobObj;
typedef qemuDomainJobObj *qemuDomainJobObjPtr; typedef qemuDomainJobObj *qemuDomainJobObjPtr;
struct _qemuDomainJobObj { struct _qemuDomainJobObj {

View File

@ -3747,7 +3747,7 @@ qemuDumpToFd(virQEMUDriverPtr driver,
if (detach) if (detach)
priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP; priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP;
else else
VIR_FREE(priv->job.current); g_clear_pointer(&priv->job.current, qemuDomainJobInfoFree);
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1; return -1;
@ -13598,16 +13598,16 @@ static int
qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
virDomainObjPtr vm, virDomainObjPtr vm,
bool completed, bool completed,
qemuDomainJobInfoPtr jobInfo) qemuDomainJobInfoPtr *jobInfo)
{ {
qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjPrivatePtr priv = vm->privateData;
int ret = -1; int ret = -1;
*jobInfo = NULL;
if (completed) { if (completed) {
if (priv->job.completed && !priv->job.current) if (priv->job.completed && !priv->job.current)
*jobInfo = *priv->job.completed; *jobInfo = qemuDomainJobInfoCopy(priv->job.completed);
else
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
return 0; return 0;
} }
@ -13626,26 +13626,25 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
goto cleanup; goto cleanup;
if (!priv->job.current) { if (!priv->job.current) {
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
ret = 0; ret = 0;
goto cleanup; 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_MIGRATION:
case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) if (qemuDomainGetJobInfoMigrationStats(driver, vm, *jobInfo) < 0)
goto cleanup; goto cleanup;
break; break;
case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP:
if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0) if (qemuDomainGetJobInfoDumpStats(driver, vm, *jobInfo) < 0)
goto cleanup; goto cleanup;
break; break;
case QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP: case QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP:
if (qemuBackupGetJobInfoStats(driver, vm, jobInfo) < 0) if (qemuBackupGetJobInfoStats(driver, vm, *jobInfo) < 0)
goto cleanup; goto cleanup;
break; break;
@ -13666,7 +13665,7 @@ qemuDomainGetJobInfo(virDomainPtr dom,
virDomainJobInfoPtr info) virDomainJobInfoPtr info)
{ {
virQEMUDriverPtr driver = dom->conn->privateData; virQEMUDriverPtr driver = dom->conn->privateData;
qemuDomainJobInfo jobInfo; g_autoptr(qemuDomainJobInfo) jobInfo = NULL;
virDomainObjPtr vm; virDomainObjPtr vm;
int ret = -1; int ret = -1;
@ -13681,12 +13680,13 @@ qemuDomainGetJobInfo(virDomainPtr dom,
if (qemuDomainGetJobStatsInternal(driver, vm, false, &jobInfo) < 0) if (qemuDomainGetJobStatsInternal(driver, vm, false, &jobInfo) < 0)
goto cleanup; goto cleanup;
if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) { if (!jobInfo ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_NONE) {
ret = 0; ret = 0;
goto cleanup; goto cleanup;
} }
ret = qemuDomainJobInfoToInfo(&jobInfo, info); ret = qemuDomainJobInfoToInfo(jobInfo, info);
cleanup: cleanup:
virDomainObjEndAPI(&vm); virDomainObjEndAPI(&vm);
@ -13704,7 +13704,7 @@ qemuDomainGetJobStats(virDomainPtr dom,
virQEMUDriverPtr driver = dom->conn->privateData; virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm; virDomainObjPtr vm;
qemuDomainObjPrivatePtr priv; qemuDomainObjPrivatePtr priv;
qemuDomainJobInfo jobInfo; g_autoptr(qemuDomainJobInfo) jobInfo = NULL;
bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED); bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
int ret = -1; int ret = -1;
@ -13721,7 +13721,8 @@ qemuDomainGetJobStats(virDomainPtr dom,
if (qemuDomainGetJobStatsInternal(driver, vm, completed, &jobInfo) < 0) if (qemuDomainGetJobStatsInternal(driver, vm, completed, &jobInfo) < 0)
goto cleanup; goto cleanup;
if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) { if (!jobInfo ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_NONE) {
*type = VIR_DOMAIN_JOB_NONE; *type = VIR_DOMAIN_JOB_NONE;
*params = NULL; *params = NULL;
*nparams = 0; *nparams = 0;
@ -13729,10 +13730,10 @@ qemuDomainGetJobStats(virDomainPtr dom,
goto cleanup; 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)) if (completed && ret == 0 && !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED))
VIR_FREE(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
cleanup: cleanup:
virDomainObjEndAPI(&vm); virDomainObjEndAPI(&vm);

View File

@ -1724,11 +1724,9 @@ qemuMigrationSrcWaitForCompletion(virQEMUDriverPtr driver,
qemuDomainJobInfoUpdateTime(jobInfo); qemuDomainJobInfoUpdateTime(jobInfo);
qemuDomainJobInfoUpdateDowntime(jobInfo); qemuDomainJobInfoUpdateDowntime(jobInfo);
VIR_FREE(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
if (VIR_ALLOC(priv->job.completed) == 0) { priv->job.completed = qemuDomainJobInfoCopy(jobInfo);
*priv->job.completed = *jobInfo; priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
}
if (asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT && if (asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT &&
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED) jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED)
@ -3017,7 +3015,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
if (retcode == 0) if (retcode == 0)
jobInfo = priv->job.completed; jobInfo = priv->job.completed;
else else
VIR_FREE(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
/* Update times with the values sent by the destination daemon */ /* Update times with the values sent by the destination daemon */
if (mig->jobInfo && jobInfo) { if (mig->jobInfo && jobInfo) {
@ -5036,7 +5034,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
: QEMU_MIGRATION_PHASE_FINISH2); : QEMU_MIGRATION_PHASE_FINISH2);
qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup); qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup);
VIR_FREE(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK | cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK |
QEMU_MIGRATION_COOKIE_STATS | QEMU_MIGRATION_COOKIE_STATS |
@ -5257,7 +5255,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
* is obsolete anyway. * is obsolete anyway.
*/ */
if (inPostCopy) if (inPostCopy)
VIR_FREE(priv->job.completed); g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
} }
qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
@ -5268,7 +5266,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
qemuDomainRemoveInactiveJob(driver, vm); qemuDomainRemoveInactiveJob(driver, vm);
cleanup: cleanup:
VIR_FREE(jobInfo); g_clear_pointer(&jobInfo, qemuDomainJobInfoFree);
virPortAllocatorRelease(port); virPortAllocatorRelease(port);
if (priv->mon) if (priv->mon)
qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);

View File

@ -123,7 +123,7 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
VIR_FREE(mig->name); VIR_FREE(mig->name);
VIR_FREE(mig->lockState); VIR_FREE(mig->lockState);
VIR_FREE(mig->lockDriver); VIR_FREE(mig->lockDriver);
VIR_FREE(mig->jobInfo); g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree);
virCPUDefFree(mig->cpu); virCPUDefFree(mig->cpu);
qemuMigrationCookieCapsFree(mig->caps); qemuMigrationCookieCapsFree(mig->caps);
VIR_FREE(mig); VIR_FREE(mig);
@ -513,10 +513,9 @@ qemuMigrationCookieAddStatistics(qemuMigrationCookiePtr mig,
if (!priv->job.completed) if (!priv->job.completed)
return 0; return 0;
if (!mig->jobInfo && VIR_ALLOC(mig->jobInfo) < 0) g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree);
return -1; mig->jobInfo = qemuDomainJobInfoCopy(priv->job.completed);
*mig->jobInfo = *priv->job.completed;
mig->flags |= QEMU_MIGRATION_COOKIE_STATS; mig->flags |= QEMU_MIGRATION_COOKIE_STATS;
return 0; return 0;
@ -1051,8 +1050,7 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt)
if (!(ctxt->node = virXPathNode("./statistics", ctxt))) if (!(ctxt->node = virXPathNode("./statistics", ctxt)))
return NULL; return NULL;
if (VIR_ALLOC(jobInfo) < 0) jobInfo = g_new0(qemuDomainJobInfo, 1);
return NULL;
stats = &jobInfo->stats.mig; stats = &jobInfo->stats.mig;
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;