[PATCH v2 2/4] test_driver: Implement virDomainGetJobStats

[Luke Yue]

On Tue, 2021-08-17 at 13:48 +0200, Martin Kletzander wrote:
> On Mon, Aug 16, 2021 at 07:13:35PM +0800, Luke Yue wrote:
> > Signed-off-by: Luke Yue <lukedyue at gmail.com>
> > ---
> > src/test/test_driver.c | 105
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 105 insertions(+)
> > 
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 9306f0e104..93aeec7105 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -2769,6 +2769,110 @@ testDomainGetJobInfo(virDomainPtr dom,
> >     return ret;
> > }
> > 
> > +static int
> > +testDomainJobInfoToParams(testDomainObjPrivate *priv,
> > +                          virDomainJobInfoPtr info,
> > +                          int *type,
> > +                          virTypedParameterPtr *params,
> > +                          int *nparams)
> > +{
> > +    virTypedParameterPtr par = NULL;
> > +    int maxpar = 0;
> > +    int npar = 0;
> > +
> > +    if (virTypedParamsAddInt(&par, &npar, &maxpar,
> > +                             VIR_DOMAIN_JOB_OPERATION,
> > +                             priv->jobOperation) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_DATA_TOTAL,
> > +                                info->dataTotal) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_DATA_PROCESSED,
> > +                                info->dataProcessed) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_DATA_REMAINING,
> > +                                info->dataRemaining) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_TIME_ELAPSED,
> > +                                info->timeElapsed) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_TIME_REMAINING,
> > +                                info->timeRemaining) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_MEMORY_TOTAL,
> > +                                info->memTotal) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_MEMORY_PROCESSED,
> > +                                info->memProcessed) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_MEMORY_REMAINING,
> > +                                info->memRemaining) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_DISK_TOTAL,
> > +                                info->fileTotal) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_DISK_PROCESSED,
> > +                                info->fileProcessed) < 0 ||
> > +        virTypedParamsAddULLong(&par, &npar, &maxpar,
> > +                                VIR_DOMAIN_JOB_DISK_REMAINING,
> > +                                info->fileRemaining) < 0)
> > +        goto error;
> > +
> > +    *type = info->type;
> > +    *params = par;
> > +    *nparams = npar;
> > +    return 0;
> > +
> > + error:
> > +    virTypedParamsFree(par, npar);
> > +    return -1;
> > +}
> > +
> > +static int
> > +testDomainGetJobStats(virDomainPtr domain,
> > +                      int *type,
> > +                      virTypedParameterPtr *params,
> > +                      int *nparams,
> > +                      unsigned int flags)
> > +{
> > +    virDomainJobInfo jobInfo;
> > +    virDomainObj *dom;
> > +    testDomainObjPrivate *priv;
> > +    bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
> 
> Unnecessary cast, just:
> 
> bool completed = flags & VIR_DOMAIN_JOB_STATS_COMPLETED;
> 
> would be fine.
> 
> > +    int ret = -1;
> > +
> > +    virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED |
> > +                  VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED, -1);
> > +
> > +    if (!(dom = testDomObjFromDomain(domain)))
> > +        return -1;
> > +
> > +    priv = dom->privateData;
> > +
> > +    if (testDomainGetJobInfoImpl(dom, &jobInfo) < 0)
> > +        goto cleanup;
> > +
> 
> Unfortunately I do not see how you return different data if you get
> here
> with completed == true.  Either we should return some dummy values or
> get an extra field in priv for some last completed job.
> 
> Otherwise looks fine.
> 

Thanks for the review! I thought we could use priv->jobOperation as
both complete job or running job for testing, though it may not be
pretty reasonable. Will try to add another variable to store the
completed job in v3.

> > +    if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
> > +        *type = VIR_DOMAIN_JOB_NONE;
> > +        *params = NULL;
> > +        *nparams = 0;
> > +        ret = 0;
> > +        goto cleanup;
> > +    }
> > +
> > +    ret = testDomainJobInfoToParams(priv, &jobInfo, type, params,
> > nparams);
> > +
> > +    if (completed && ret == 0 &&
> > +        !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED)) {
> > +        priv->jobState = VIR_DOMAIN_JOB_NONE;
> > +        priv->jobOperation = VIR_DOMAIN_JOB_OPERATION_UNKNOWN;
> > +    }
> > +
> > + cleanup:
> > +    virDomainObjEndAPI(&dom);
> > +
> > +    return ret;
> > +}
> > 
> > static int
> > testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
> > @@ -9672,6 +9776,7 @@ static virHypervisorDriver
> > testHypervisorDriver = {
> >     .domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
> >     .domainSetLifecycleAction = testDomainSetLifecycleAction, /*
> > 5.7.0 */
> >     .domainGetJobInfo = testDomainGetJobInfo, /* 7.7.0 */
> > +    .domainGetJobStats = testDomainGetJobStats, /* 7.7.0 */
> > 
> >     .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
> >     .domainSnapshotListNames = testDomainSnapshotListNames, /*
> > 1.1.4 */
> > -- 
> > 2.32.0
> >