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@xxxxxxxxx>
> > ---
> > 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
> >
