[PATCH 2/3] qemu: make separate function for setting statsType of privateData

[Jiri Denemark]

On Thu, Jan 20, 2022 at 17:59:49 +0100, Kristina Hanicova wrote:
> We only need to set statsType in almost every case of setting
> something from private data, so it seems unnecessary to pull
> privateData out of current / completed job for just this one
> thing every time. I think this patch keeps the code cleaner
> without variables used just once.
> 
> Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
> ---
>  src/qemu/qemu_backup.c    |  4 ++--
>  src/qemu/qemu_domain.c    |  8 ++++++++
>  src/qemu/qemu_domain.h    |  3 +++
>  src/qemu/qemu_driver.c    | 14 ++++++--------
>  src/qemu/qemu_migration.c |  9 ++++-----
>  src/qemu/qemu_process.c   |  5 ++---
>  src/qemu/qemu_snapshot.c  |  5 ++---
>  7 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
> index 081c4d023f..23d3f44dd8 100644
> --- a/src/qemu/qemu_backup.c
> +++ b/src/qemu/qemu_backup.c
> @@ -746,7 +746,6 @@ qemuBackupBegin(virDomainObj *vm,
>                  unsigned int flags)
>  {
>      qemuDomainObjPrivate *priv = vm->privateData;
> -    qemuDomainJobDataPrivate *privData = priv->job.current->privateData;
>      g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
>      g_autoptr(virDomainBackupDef) def = NULL;
>      g_autofree char *suffix = NULL;
> @@ -800,7 +799,8 @@ qemuBackupBegin(virDomainObj *vm,
>      qemuDomainObjSetAsyncJobMask(vm, (QEMU_JOB_DEFAULT_MASK |
>                                        JOB_MASK(QEMU_JOB_SUSPEND) |
>                                        JOB_MASK(QEMU_JOB_MODIFY)));
> -    privData->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP;
> +    qemuDomainJobPrivateSetStatsType(priv->job.current->privateData,
> +                                     QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP);
>  
>      if (!virDomainObjIsActive(vm)) {
>          virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index a8401bac30..62e67b438f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c

The new helper would fit slightly better in qemu_domainjob.c

> @@ -81,6 +81,14 @@
>  VIR_LOG_INIT("qemu.qemu_domain");
>  
>  
> +void
> +qemuDomainJobPrivateSetStatsType(qemuDomainJobDataPrivate *privData,
> +                                 qemuDomainJobStatsType type)

Since we're doing this to minimize the repeated code pattern, I would go
a little bit further and change the prototype to

   qemuDomainJobSetStatsType(virDomainJobData *jobData,
                             qemuDomainJobStatsType type)

> +{

Having an extra

       qemuDomainJobDataPrivate *privData = jobData->privateData;

here is better than repeating the dereference for every single caller.

> +    privData->statsType = type;
> +}
> +
> +
>  static void *
>  qemuJobAllocPrivate(void)
>  {
...

Looks OK otherwise.

Jirka