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