On Mon, Aug 17, 2020 at 10:37:16AM +0530, Prathamesh Chavan wrote:
> As `qemuDomainJobInfo` had attributes specific to qemu hypervisor's
> jobs, we moved the attribute `current` and `completed` from
> `qemuDomainJobObj` to its `privateData` structure.
>
> In this process, two callback functions: `setJobInfoOperation`
> and `currentJobInfoInit` were introduced to qemuDomainJob's
> callback structure.
>
> Signed-off-by: Prathamesh Chavan <pc44800@xxxxxxxxx>
> ---
> src/qemu/qemu_backup.c | 22 +-
> src/qemu/qemu_domain.c | 498 +++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 74 +++++
> src/qemu/qemu_domainjob.c | 483 +-----------------------------
> src/qemu/qemu_domainjob.h | 81 +----
> src/qemu/qemu_driver.c | 49 +--
> src/qemu/qemu_migration.c | 62 ++--
> src/qemu/qemu_migration_cookie.c | 8 +-
> src/qemu/qemu_process.c | 32 +-
> 9 files changed, 680 insertions(+), 629 deletions(-)
>
> diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
> index a402730d38..1822c6f267 100644
> --- a/src/qemu/qemu_backup.c
> +++ b/src/qemu/qemu_backup.c
> @@ -529,20 +529,21 @@ qemuBackupJobTerminate(virDomainObjPtr vm,
>
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> + qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
> size_t i;
>
> - qemuDomainJobInfoUpdateTime(priv->job.current);
> + qemuDomainJobInfoUpdateTime(jobPriv->current);
>
> - g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree);
> - priv->job.completed = qemuDomainJobInfoCopy(priv->job.current);
> + g_clear_pointer(&jobPriv->completed, qemuDomainJobInfoFree);
> + jobPriv->completed = qemuDomainJobInfoCopy(jobPriv->current);
>
> - 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.tmp_used = priv->backup->pull_tmp_used;
> - priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total;
> + jobPriv->completed->stats.backup.total = priv->backup->push_total;
> + jobPriv->completed->stats.backup.transferred = priv->backup->push_transferred;
> + jobPriv->completed->stats.backup.tmp_used = priv->backup->pull_tmp_used;
> + jobPriv->completed->stats.backup.tmp_total = priv->backup->pull_tmp_total;
>
> - priv->job.completed->status = jobstatus;
> - priv->job.completed->errmsg = g_strdup(priv->backup->errmsg);
> + jobPriv->completed->status = jobstatus;
> + jobPriv->completed->errmsg = g_strdup(priv->backup->errmsg);
>
> qemuDomainEventEmitJobCompleted(priv->driver, vm);
>
> @@ -694,6 +695,7 @@ qemuBackupBegin(virDomainObjPtr vm,
> unsigned int flags)
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> + qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
> g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
> g_autoptr(virDomainBackupDef) def = NULL;
> g_autofree char *suffix = NULL;
> @@ -745,7 +747,7 @@ qemuBackupBegin(virDomainObjPtr vm,
> qemuDomainObjSetAsyncJobMask(vm, (QEMU_JOB_DEFAULT_MASK |
> JOB_MASK(QEMU_JOB_SUSPEND) |
> JOB_MASK(QEMU_JOB_MODIFY)));
> - priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP;
> + jobPriv->current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP;
>
> if (!virDomainObjIsActive(vm)) {
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
You could perform changes such as the ones ^above in a separate patch, while...
[snip]
> +
> +
> +int
> +qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
> + virDomainJobInfoPtr info)
> +{
> + info->type = qemuDomainJobStatusToType(jobInfo->status);
> + info->timeElapsed = jobInfo->timeElapsed;
> +
> + switch (jobInfo->statsType) {
> + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION:
> + info->memTotal = jobInfo->stats.mig.ram_total;
> + info->memRemaining = jobInfo->stats.mig.ram_remaining;
> + info->memProcessed = jobInfo->stats.mig.ram_transferred;
> + info->fileTotal = jobInfo->stats.mig.disk_total +
> + jobInfo->mirrorStats.total;
> + info->fileRemaining = jobInfo->stats.mig.disk_remaining +
> + (jobInfo->mirrorStats.total -
> + jobInfo->mirrorStats.transferred);
> + info->fileProcessed = jobInfo->stats.mig.disk_transferred +
> + jobInfo->mirrorStats.transferred;
> + break;
> +
> + case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
> + info->memTotal = jobInfo->stats.mig.ram_total;
> + info->memRemaining = jobInfo->stats.mig.ram_remaining;
> + info->memProcessed = jobInfo->stats.mig.ram_transferred;
> + break;
> +
> + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP:
> + info->memTotal = jobInfo->stats.dump.total;
> + info->memProcessed = jobInfo->stats.dump.completed;
> + info->memRemaining = info->memTotal - info->memProcessed;
> + break;
> +
> + case QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP:
> + info->fileTotal = jobInfo->stats.backup.total;
> + info->fileProcessed = jobInfo->stats.backup.transferred;
> + info->fileRemaining = info->fileTotal - info->fileProcessed;
> + break;
> +
> + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE:
> + break;
> + }
> +
> + info->dataTotal = info->memTotal + info->fileTotal;
> + info->dataRemaining = info->memRemaining + info->fileRemaining;
> + info->dataProcessed = info->memProcessed + info->fileProcessed;
> +
> + return 0;
> +}
...moving helper functions like ^this one in the current patch, if...
[snip]
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 3a1bcbbfa3..386ae17272 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -483,6 +483,52 @@ struct _qemuDomainXmlNsDef {
> char **capsdel;
> };
>
> +typedef struct _qemuDomainMirrorStats qemuDomainMirrorStats;
> +typedef qemuDomainMirrorStats *qemuDomainMirrorStatsPtr;
> +struct _qemuDomainMirrorStats {
> + unsigned long long transferred;
> + unsigned long long total;
> +};
> +
> +typedef struct _qemuDomainBackupStats qemuDomainBackupStats;
> +struct _qemuDomainBackupStats {
> + unsigned long long transferred;
> + unsigned long long total;
> + unsigned long long tmp_used;
> + unsigned long long tmp_total;
> +};
> +
> +typedef struct _qemuDomainJobInfo qemuDomainJobInfo;
> +typedef qemuDomainJobInfo *qemuDomainJobInfoPtr;
> +struct _qemuDomainJobInfo {
> + qemuDomainJobStatus status;
> + virDomainJobOperation operation;
> + unsigned long long started; /* When the async job started */
> + unsigned long long stopped; /* When the domain's CPUs were stopped */
> + unsigned long long sent; /* When the source sent status info to the
> + destination (only for migrations). */
> + unsigned long long received; /* When the destination host received status
> + info from the source (migrations only). */
> + /* Computed values */
> + unsigned long long timeElapsed;
> + long long timeDelta; /* delta = received - sent, i.e., the difference
> + between the source and the destination time plus
> + the time between the end of Perform phase on the
> + source and the beginning of Finish phase on the
> + destination. */
> + bool timeDeltaSet;
> + /* Raw values from QEMU */
> + qemuDomainJobStatsType statsType;
> + union {
> + qemuMonitorMigrationStats mig;
> + qemuMonitorDumpStats dump;
> + qemuDomainBackupStats backup;
> + } stats;
> + qemuDomainMirrorStats mirrorStats;
> +
> + char *errmsg; /* optional error message for failed completed jobs */
> +};
> +
...you keep ^these structures in qemu_domainjob.h in patch 1, as qemu_domain.h
includes qemu_domainjob.h, so all the functions that you'd move would have the
symbol.
Then, in patch 2 you'd move the structures themselves and perform changes like
the ones below for example.
> @@ -3330,7 +3333,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
> goto endjob;
> }
>
> - priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP;
> + jobPriv->current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP;
>
> /* Pause */
> if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
> @@ -3715,7 +3718,7 @@ qemuDumpWaitForCompletion(virDomainObjPtr vm)
> return -1;
> }
>
> - if (priv->job.current->stats.dump.status == QEMU_MONITOR_DUMP_STATUS_FAILED) {
> + if (jobPriv->current->stats.dump.status == QEMU_MONITOR_DUMP_STATUS_FAILED) {
> if (priv->job.error)
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("memory-only dump failed: %s"),
Regards,
Erik
