[PATCH 1/4] test_driver: Implement virDomainGetJobInfo

[Luke Yue]

On Tue, 2021-08-10 at 17:49 +0200, Martin Kletzander wrote:
> On Thu, Jul 22, 2021 at 03:13:22PM +0800, Luke Yue wrote:
> > As in testDomainGetControlInfo, a background job should be running
> > between 0-10000 seconds, so make the testDomainGetJobInfo
> > consistent
> > with it.
> > 
> > Signed-off-by: Luke Yue <lukedyue at gmail.com>
> > ---
> > src/test/test_driver.c | 51
> > ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> > 
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 892dc978f2..29b19d80f0 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -2604,6 +2604,56 @@ testDomainGetOSType(virDomainPtr dom
> > G_GNUC_UNUSED)
> >     return ret;
> > }
> > 
> > +static int
> > +testDomainGetJobInfoImpl(virDomainObj *dom,
> > +                         virDomainJobInfoPtr info)
> > +{
> > +    testDomainObjPrivate *priv = dom->privateData;
> > +
> > +    memset(info, 0, sizeof(*info));
> > +
> > +    if (priv->seconds < 10000) {
> > +        info->type = VIR_DOMAIN_JOB_BOUNDED;
> > +        info->dataTotal = 10000;
> > +        info->dataProcessed = priv->seconds;
> > +        info->dataRemaining = 10000 - priv->seconds;
> > +        info->timeElapsed = priv->seconds;
> > +        info->timeRemaining = 10000 - priv->seconds;
> > +        info->memTotal = 1048576;
> > +        info->memProcessed = 1048576 - priv->seconds;
> > +        info->memRemaining = priv->seconds;
> > +        info->fileTotal = 2097152;
> > +        info->fileProcessed = 2097152 - priv->seconds;
> > +        info->fileRemaining = priv->seconds;
> > +    } else if (priv->seconds < 30000 && priv->seconds >= 10000) {
> > +        info->type = VIR_DOMAIN_JOB_COMPLETED;
> > +    } else {
> > +        info->type = VIR_DOMAIN_JOB_NONE;
> > +    }
> > +
> 
> So this is an interesting approach.  However because we are not doing
> different setups for different domains (all possible ones use the
> same
> PrivateAlloc function) we cannot test much here.  But then when you
> think about changing the domain time for some domains it (even by
> just
> using virDomainSetTime API) the job output is very much non-
> intuitive.
> So I would rather prefer some extra struct member(s) to keep track of
> the job info.  It can then be nicely used for testing
> domainGetJobInfo
> with flags as well as aborting and starting jobs in a more interested
> and I think useful way.
> 
> Other than that this series looks fine.

Thanks for the review! So if I understand you correctly, we have to add
struct member(s) such as

  unsigned int jobType;

to testDomainObjPrivate to store the job info rather than using time.
We can change it to VIR_DOMAIN_BLOCK_JOB_CANCELED when we use
virDomainAbortJob, and change it to other values when the APIs should
start a job, such as virDomainBackupBegin (not implement yet though).

But I am a little bit puzzling about testing the domainGetJobInfo with
flags, the virDomainGetJobInfo currently don't have flags as parameter,
do we have to add one? Or do I misunderstand something?

Thanks,
Luke

> 
> > +    return 0;
> > +}
> > +
> > +static int
> > +testDomainGetJobInfo(virDomainPtr dom,
> > +                     virDomainJobInfoPtr info)
> > +{
> > +    virDomainObj *vm;
> > +    int ret = -1;
> > +
> > +    if (!(vm = testDomObjFromDomain(dom)))
> > +        goto cleanup;
> > +
> > +    if (virDomainObjCheckActive(vm) < 0)
> > +        goto cleanup;
> > +
> > +    ret = testDomainGetJobInfoImpl(vm, info);
> > +
> > + cleanup:
> > +    virDomainObjEndAPI(&vm);
> > +    return ret;
> > +}
> > +
> > 
> > static int
> > testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
> > @@ -9484,6 +9534,7 @@ static virHypervisorDriver
> > testHypervisorDriver = {
> >     .domainMemoryPeek = testDomainMemoryPeek, /* 5.4.0 */
> >     .domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
> >     .domainSetLifecycleAction = testDomainSetLifecycleAction, /*
> > 5.7.0 */
> > +    .domainGetJobInfo = testDomainGetJobInfo, /* 7.6.0 */
> > 
> >     .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
> >     .domainSnapshotListNames = testDomainSnapshotListNames, /*
> > 1.1.4 */
> > -- 
> > 2.32.0
> >