[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [GSoC][PATCH v2 1/6] qemu_domain: Added `qemuDomainJobInfo` to domainJob's `privateData`



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 gmail com>
> ---
>  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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]