Re: [GSoC][PATCH v5] qemu_domainjob: introduce `privateData` for `qemuDomainJob`

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 7/16/20 1:48 PM, Prathamesh Chavan wrote:
> To remove dependecy of `qemuDomainJob` on job specific
> paramters, a `privateData` pointer is introduced.
> To handle it, structure of callback functions is
> also introduced.
>
> Signed-off-by: Prathamesh Chavan <pc44800@xxxxxxxxx>
> ---
> Previous version of this patch can be found here[1].
>
> This patch mainly improvises on the changes suggested in the review
> of the previous version by Martin and Michal.
>
> Currently, the structure `qemuDomainJobInfo` is kept untouched.
>
>   src/qemu/qemu_domain.c           | 70 ++++++++++++++++++++++++++++++-
>   src/qemu/qemu_domain.h           | 10 +++++
>   src/qemu/qemu_domainjob.c        | 71 ++++++++++++++++++--------------
>   src/qemu/qemu_domainjob.h        | 38 ++++++++++++-----
>   src/qemu/qemu_driver.c           |  3 +-
>   src/qemu/qemu_migration.c        | 28 +++++++++----
>   src/qemu/qemu_migration_params.c |  9 ++--
>   src/qemu/qemu_process.c          | 15 +++++--
>   8 files changed, 185 insertions(+), 59 deletions(-)

> diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
> index d96d5334a3..7cbfe3801e 100644
> --- a/src/qemu/qemu_domainjob.c
> +++ b/src/qemu/qemu_domainjob.c
> @@ -159,24 +159,6 @@ qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver,
>       virObjectEventStateQueue(driver->domainEventState, event);
>   }
>   
> -
> -int
> -qemuDomainObjInitJob(qemuDomainJobObjPtr job)
> -{
> -    memset(job, 0, sizeof(*job));
> -
> -    if (virCondInit(&job->cond) < 0)
> -        return -1;
> -
> -    if (virCondInit(&job->asyncCond) < 0) {
> -        virCondDestroy(&job->cond);
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -


> +
> +int
> +qemuDomainObjInitJob(qemuDomainJobObjPtr job,
> +                     qemuDomainObjPrivateJobCallbacksPtr cb)
> +{
> +    memset(job, 0, sizeof(*job));
> +    job->cb = cb;
> +
> +    if (!(job->privateData = job->cb->allocJobPrivate()))
> +        return -1;
> +
> +    if (virCondInit(&job->cond) < 0) {
> +        job->cb->freeJobPrivate(job->privateData);
>           return -1;
> +    }
> +
> +    if (virCondInit(&job->asyncCond) < 0) {
> +        job->cb->freeJobPrivate(job->privateData);
> +        virCondDestroy(&job->cond);
> +        return -1;
> +    }
>   
>       return 0;
>   }


There is no need to move this function. It can live where it is.

Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>

As promised on our meeting earlier today, I looked into turning this 
into a virObject/GObject. It's not going to be that easy (read trivial) 
and thus let's go with this patch for now. We can always turn it into an 
object if we want.

Michal