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

[Michal Privoznik]

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 <pc44...@gmail.com>
---
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 <mpriv...@redhat.com>

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