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@gmail.com>
---
 src/qemu/qemu_domain.c           |  4 +-
 src/qemu/qemu_domain.h           | 10 +++++
 src/qemu/qemu_domainjob.c        | 74 ++++++++++++++++++++++----------
 src/qemu/qemu_domainjob.h        | 32 ++++++++------
 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, 119 insertions(+), 56 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 10d2033db1..4ece07d141 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2303,7 +2303,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
     if (priv->lockState)
         virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState);
 
-    if (qemuDomainObjPrivateXMLFormatJob(buf, vm) < 0)
+    if (priv->job.cb.formatJob(buf, vm) < 0)
         return -1;
 
     if (priv->fakeReboot)
@@ -2962,7 +2962,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 
     priv->lockState = virXPathString("string(./lockstate)", ctxt);
 
-    if (qemuDomainObjPrivateXMLParseJob(vm, ctxt) < 0)
+    if (priv->job.cb.parseJob(vm, ctxt) < 0)
         goto error;
 
     priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index e524fd0002..bb9b414a46 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -492,6 +492,16 @@ struct _qemuDomainXmlNsDef {
     char **capsdel;
 };
 
+typedef struct _qemuDomainJobPrivate qemuDomainJobPrivate;
+typedef qemuDomainJobPrivate *qemuDomainJobPrivatePtr;
+struct _qemuDomainJobPrivate {
+    bool spiceMigration;                /* we asked for spice migration and we
+                                         * should wait for it to finish */
+    bool spiceMigrated;                 /* spice migration completed */
+    bool dumpCompleted;                 /* dump completed */
+    qemuMigrationParamsPtr migParams;
+};
+
 int qemuDomainObjStartWorker(virDomainObjPtr dom);
 void qemuDomainObjStopWorker(virDomainObjPtr dom);
 
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
index 454a5a2c23..d79b8d49f6 100644
--- a/src/qemu/qemu_domainjob.c
+++ b/src/qemu/qemu_domainjob.c
@@ -159,23 +159,32 @@ qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver,
     virObjectEventStateQueue(driver->domainEventState, event);
 }
 
-
-int
-qemuDomainObjInitJob(qemuDomainJobObjPtr job)
+static void *
+qemuJobAllocPrivate(void)
 {
-    memset(job, 0, sizeof(*job));
+    qemuDomainJobPrivatePtr priv;
+    if (VIR_ALLOC(priv) < 0)
+        return NULL;
+    return (void *)priv;
+}
 
-    if (virCondInit(&job->cond) < 0)
-        return -1;
 
-    if (virCondInit(&job->asyncCond) < 0) {
-        virCondDestroy(&job->cond);
-        return -1;
-    }
-
-    return 0;
+static void
+qemuJobFreePrivateData(qemuDomainJobPrivatePtr priv)
+{
+    priv->spiceMigration = false;
+    priv->spiceMigrated = false;
+    priv->dumpCompleted = false;
+    qemuMigrationParamsFree(priv->migParams);
+    priv->migParams = NULL;
 }
 
+static void
+qemuJobFreePrivate(void *opaque)
+{
+    qemuDomainJobObjPtr job = (qemuDomainJobObjPtr) opaque;
+    qemuJobFreePrivateData(job->privateData);
+}
 
 static void
 qemuDomainObjResetJob(qemuDomainJobObjPtr job)
@@ -207,13 +216,9 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObjPtr job)
     job->phase = 0;
     job->mask = QEMU_JOB_DEFAULT_MASK;
     job->abortJob = false;
-    job->spiceMigration = false;
-    job->spiceMigrated = false;
-    job->dumpCompleted = false;
     VIR_FREE(job->error);
     g_clear_pointer(&job->current, qemuDomainJobInfoFree);
-    qemuMigrationParamsFree(job->migParams);
-    job->migParams = NULL;
+    job->cb.freeJobPrivate(job);
     job->apiFlags = 0;
 }
 
@@ -229,7 +234,7 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
     job->asyncJob = priv->job.asyncJob;
     job->asyncOwner = priv->job.asyncOwner;
     job->phase = priv->job.phase;
-    job->migParams = g_steal_pointer(&priv->job.migParams);
+    job->privateData = g_steal_pointer(&priv->job.privateData);
     job->apiFlags = priv->job.apiFlags;
 
     qemuDomainObjResetJob(&priv->job);
@@ -1240,12 +1245,13 @@ qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf,
 }
 
 
-int
+static int
 qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf,
                                  virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     qemuDomainJobObjPtr jobObj = &priv->job;
+    qemuDomainJobPrivatePtr jobPriv = jobObj->privateData;
     g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
     g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
     qemuDomainJob job = jobObj->active;
@@ -1274,8 +1280,8 @@ qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf,
         qemuDomainObjPrivateXMLFormatNBDMigration(&childBuf, vm) < 0)
         return -1;
 
-    if (jobObj->migParams)
-        qemuMigrationParamsFormat(&childBuf, jobObj->migParams);
+    if (jobPriv->migParams)
+        qemuMigrationParamsFormat(&childBuf, jobPriv->migParams);
 
     virXMLFormatElement(buf, "job", &attrBuf, &childBuf);
 
@@ -1369,12 +1375,13 @@ qemuDomainObjPrivateXMLParseJobNBD(virDomainObjPtr vm,
     return 0;
 }
 
-int
+static int
 qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm,
                                 xmlXPathContextPtr ctxt)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     qemuDomainJobObjPtr job = &priv->job;
+    qemuDomainJobPrivatePtr jobPriv = job->privateData;
     VIR_XPATH_NODE_AUTORESTORE(ctxt);
     g_autofree char *tmp = NULL;
 
@@ -1423,8 +1430,29 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm,
     if (qemuDomainObjPrivateXMLParseJobNBD(vm, ctxt) < 0)
         return -1;
 
-    if (qemuMigrationParamsParse(ctxt, &job->migParams) < 0)
+    if (qemuMigrationParamsParse(ctxt, &jobPriv->migParams) < 0)
         return -1;
 
     return 0;
 }
+
+int
+qemuDomainObjInitJob(qemuDomainJobObjPtr job)
+{
+    memset(job, 0, sizeof(*job));
+    job->cb.allocJobPrivate = &qemuJobAllocPrivate;
+    job->cb.freeJobPrivate = &qemuJobFreePrivate;
+    job->cb.formatJob = &qemuDomainObjPrivateXMLFormatJob;
+    job->cb.parseJob = &qemuDomainObjPrivateXMLParseJob;
+    job->privateData = job->cb.allocJobPrivate();
+
+    if (virCondInit(&job->cond) < 0)
+        return -1;
+
+    if (virCondInit(&job->asyncCond) < 0) {
+        virCondDestroy(&job->cond);
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h
index 9d2ee14584..04e0a4315e 100644
--- a/src/qemu/qemu_domainjob.h
+++ b/src/qemu/qemu_domainjob.h
@@ -154,6 +154,21 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainJobInfo, qemuDomainJobInfoFree);
 qemuDomainJobInfoPtr
 qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info);
 
+typedef void *(*qemuDomainObjPrivateJobAlloc)(void);
+typedef void (*qemuDomainObjPrivateJobFree)(void *);
+typedef int (*qemuDomainObjPrivateJobFormat)(virBufferPtr,
+                                             virDomainObjPtr);
+typedef int (*qemuDomainObjPrivateJobParse)(virDomainObjPtr,
+                                            xmlXPathContextPtr);
+
+typedef struct _qemuDomainObjPrivateJobCallbacks qemuDomainObjPrivateJobCallbacks;
+struct _qemuDomainObjPrivateJobCallbacks {
+   qemuDomainObjPrivateJobAlloc allocJobPrivate;
+   qemuDomainObjPrivateJobFree freeJobPrivate;
+   qemuDomainObjPrivateJobFormat formatJob;
+   qemuDomainObjPrivateJobParse parseJob;
+};
+
 typedef struct _qemuDomainJobObj qemuDomainJobObj;
 typedef qemuDomainJobObj *qemuDomainJobObjPtr;
 struct _qemuDomainJobObj {
@@ -182,14 +197,11 @@ struct _qemuDomainJobObj {
     qemuDomainJobInfoPtr current;       /* async job progress data */
     qemuDomainJobInfoPtr completed;     /* statistics data of a recently completed job */
     bool abortJob;                      /* abort of the job requested */
-    bool spiceMigration;                /* we asked for spice migration and we
-                                         * should wait for it to finish */
-    bool spiceMigrated;                 /* spice migration completed */
     char *error;                        /* job event completion error */
-    bool dumpCompleted;                 /* dump completed */
-
-    qemuMigrationParamsPtr migParams;
     unsigned long apiFlags; /* flags passed to the API which started the async job */
+
+    void *privateData;                  /* job specific collection of data */
+    qemuDomainObjPrivateJobCallbacks cb;
 };
 
 const char *qemuDomainAsyncJobPhaseToString(qemuDomainAsyncJob job,
@@ -267,11 +279,3 @@ void qemuDomainObjFreeJob(qemuDomainJobObjPtr job);
 int qemuDomainObjInitJob(qemuDomainJobObjPtr job);
 
 bool qemuDomainJobAllowed(qemuDomainJobObjPtr jobs, qemuDomainJob newJob);
-
-int
-qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf,
-                                 virDomainObjPtr vm);
-
-int
-qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm,
-                                xmlXPathContextPtr ctxt);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ee5b9ffe6..7339856caa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3701,9 +3701,10 @@ static int
 qemuDumpWaitForCompletion(virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
 
     VIR_DEBUG("Waiting for dump completion");
-    while (!priv->job.dumpCompleted && !priv->job.abortJob) {
+    while (!jobPriv->dumpCompleted && !priv->job.abortJob) {
         if (virDomainObjWait(vm) < 0)
             return -1;
     }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4ef3245c75..b489b41a67 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1422,12 +1422,13 @@ static int
 qemuMigrationSrcWaitForSpice(virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
 
-    if (!priv->job.spiceMigration)
+    if (!jobPriv->spiceMigration)
         return 0;
 
     VIR_DEBUG("Waiting for SPICE to finish migration");
-    while (!priv->job.spiceMigrated && !priv->job.abortJob) {
+    while (!jobPriv->spiceMigrated && !priv->job.abortJob) {
         if (virDomainObjWait(vm) < 0)
             return -1;
     }
@@ -1856,9 +1857,11 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriverPtr driver,
 
     if (qemuDomainObjEnterMonitorAsync(driver, vm,
                                        QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) {
+        qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
+
         ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress,
                                           port, tlsPort, tlsSubject);
-        priv->job.spiceMigration = !ret;
+        jobPriv->spiceMigration = !ret;
         if (qemuDomainObjExitMonitor(driver, vm) < 0)
             ret = -1;
     }
@@ -1993,6 +1996,7 @@ qemuMigrationSrcCleanup(virDomainObjPtr vm,
 {
     virQEMUDriverPtr driver = opaque;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
 
     VIR_DEBUG("vm=%s, conn=%p, asyncJob=%s, phase=%s",
               vm->def->name, conn,
@@ -2018,7 +2022,7 @@ qemuMigrationSrcCleanup(virDomainObjPtr vm,
                  " domain was successfully started on destination or not",
                  vm->def->name);
         qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
-                                 priv->job.migParams, priv->job.apiFlags);
+                                 jobPriv->migParams, priv->job.apiFlags);
         /* clear the job and let higher levels decide what to do */
         qemuDomainObjDiscardAsyncJob(driver, vm);
         break;
@@ -2393,6 +2397,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
     int dataFD[2] = { -1, -1 };
     qemuDomainObjPrivatePtr priv = NULL;
     qemuMigrationCookiePtr mig = NULL;
+    qemuDomainJobPrivatePtr jobPriv = NULL;
     bool tunnel = !!st;
     char *xmlout = NULL;
     unsigned int cookieFlags;
@@ -2519,6 +2524,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
     *def = NULL;
 
     priv = vm->privateData;
+    jobPriv = priv->job.privateData;
     priv->origname = g_strdup(origname);
 
     if (taint_hook) {
@@ -2726,7 +2732,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
 
  stopjob:
     qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
-                             priv->job.migParams, priv->job.apiFlags);
+                             jobPriv->migParams, priv->job.apiFlags);
 
     if (stopProcess) {
         unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED;
@@ -2999,6 +3005,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
     int rv = -1;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
     qemuDomainJobInfoPtr jobInfo = NULL;
 
     VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
@@ -3084,7 +3091,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
             qemuMigrationSrcRestoreDomainState(driver, vm);
 
         qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
-                                 priv->job.migParams, priv->job.apiFlags);
+                                 jobPriv->migParams, priv->job.apiFlags);
 
         if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
             VIR_WARN("Failed to save status on vm %s", vm->def->name);
@@ -4681,6 +4688,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
     virErrorPtr orig_err = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
 
     if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
                               flags) < 0)
@@ -4738,7 +4746,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
      */
     if (!v3proto && ret < 0)
         qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
-                                 priv->job.migParams, priv->job.apiFlags);
+                                 jobPriv->migParams, priv->job.apiFlags);
 
     qemuMigrationSrcRestoreDomainState(driver, vm);
 
@@ -4780,6 +4788,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
                              unsigned long resource)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
     int ret = -1;
 
     /* If we didn't start the job in the begin phase, start it now. */
@@ -4814,7 +4823,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
  endjob:
     if (ret < 0) {
         qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
-                                 priv->job.migParams, priv->job.apiFlags);
+                                 jobPriv->migParams, priv->job.apiFlags);
         qemuMigrationJobFinish(driver, vm);
     } else {
         qemuMigrationJobContinue(vm);
@@ -5019,6 +5028,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
     virErrorPtr orig_err = NULL;
     int cookie_flags = 0;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     unsigned short port;
     unsigned long long timeReceived = 0;
@@ -5272,7 +5282,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
     }
 
     qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
-                             priv->job.migParams, priv->job.apiFlags);
+                             jobPriv->migParams, priv->job.apiFlags);
 
     qemuMigrationJobFinish(driver, vm);
     if (!virDomainObjIsActive(vm))
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 6953badcfe..ba3eb14831 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -953,6 +953,7 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver,
                              qemuMigrationParamsPtr migParams)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
     virJSONValuePtr tlsProps = NULL;
     virJSONValuePtr secProps = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -965,7 +966,7 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver,
         goto error;
     }
 
-    if (!priv->job.migParams->params[QEMU_MIGRATION_PARAM_TLS_CREDS].set) {
+    if (!jobPriv->migParams->params[QEMU_MIGRATION_PARAM_TLS_CREDS].set) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("TLS migration is not supported with this "
                          "QEMU binary"));
@@ -1038,8 +1039,9 @@ qemuMigrationParamsDisableTLS(virDomainObjPtr vm,
                               qemuMigrationParamsPtr migParams)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
 
-    if (!priv->job.migParams->params[QEMU_MIGRATION_PARAM_TLS_CREDS].set)
+    if (!jobPriv->migParams->params[QEMU_MIGRATION_PARAM_TLS_CREDS].set)
         return 0;
 
     if (qemuMigrationParamsSetString(migParams,
@@ -1168,6 +1170,7 @@ qemuMigrationParamsCheck(virQEMUDriverPtr driver,
                          virBitmapPtr remoteCaps)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainJobPrivatePtr jobPriv = priv->job.privateData;
     qemuMigrationCapability cap;
     qemuMigrationParty party;
     size_t i;
@@ -1221,7 +1224,7 @@ qemuMigrationParamsCheck(virQEMUDriverPtr driver,
      * to ask QEMU for their current settings.
      */
 
-    return qemuMigrationParamsFetch(driver, vm, asyncJob, &priv->job.migParams);
+    return qemuMigrationParamsFetch(driver, vm, asyncJob, &jobPriv->migParams);
 }
 
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d36088ba98..ac0c0eb8e1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1608,6 +1608,7 @@ qemuProcessHandleSpiceMigrated(qemuMonitorPtr mon G_GNUC_UNUSED,
                                void *opaque G_GNUC_UNUSED)
 {
     qemuDomainObjPrivatePtr priv;
+    qemuDomainJobPrivatePtr jobPriv;
 
     virObjectLock(vm);
 
@@ -1615,12 +1616,13 @@ qemuProcessHandleSpiceMigrated(qemuMonitorPtr mon G_GNUC_UNUSED,
               vm, vm->def->name);
 
     priv = vm->privateData;
+    jobPriv = priv->job.privateData;
     if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) {
         VIR_DEBUG("got SPICE_MIGRATE_COMPLETED event without a migration job");
         goto cleanup;
     }
 
-    priv->job.spiceMigrated = true;
+    jobPriv->spiceMigrated = true;
     virDomainObjBroadcast(vm);
 
  cleanup:
@@ -1720,6 +1722,7 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon G_GNUC_UNUSED,
                                void *opaque G_GNUC_UNUSED)
 {
     qemuDomainObjPrivatePtr priv;
+    qemuDomainJobPrivatePtr jobPriv;
 
     virObjectLock(vm);
 
@@ -1727,11 +1730,12 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon G_GNUC_UNUSED,
               vm, vm->def->name, stats, NULLSTR(error));
 
     priv = vm->privateData;
+    jobPriv = priv->job.privateData;
     if (priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) {
         VIR_DEBUG("got DUMP_COMPLETED event without a dump_completed job");
         goto cleanup;
     }
-    priv->job.dumpCompleted = true;
+    jobPriv->dumpCompleted = true;
     priv->job.current->stats.dump = *stats;
     priv->job.error = g_strdup(error);
 
@@ -3411,6 +3415,8 @@ qemuProcessRecoverMigrationIn(virQEMUDriverPtr driver,
                               virDomainState state,
                               int reason)
 {
+
+    qemuDomainJobPrivatePtr jobPriv = job->privateData;
     bool postcopy = (state == VIR_DOMAIN_PAUSED &&
                      reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED) ||
                     (state == VIR_DOMAIN_RUNNING &&
@@ -3459,7 +3465,7 @@ qemuProcessRecoverMigrationIn(virQEMUDriverPtr driver,
     }
 
     qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_NONE,
-                             job->migParams, job->apiFlags);
+                             jobPriv->migParams, job->apiFlags);
     return 0;
 }
 
@@ -3471,6 +3477,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver,
                                int reason,
                                unsigned int *stopFlags)
 {
+    qemuDomainJobPrivatePtr jobPriv = job->privateData;
     bool postcopy = state == VIR_DOMAIN_PAUSED &&
                     (reason == VIR_DOMAIN_PAUSED_POSTCOPY ||
                      reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED);
@@ -3554,7 +3561,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver,
     }
 
     qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_NONE,
-                             job->migParams, job->apiFlags);
+                             jobPriv->migParams, job->apiFlags);
     return 0;
 }
 
-- 
2.17.1

On 7/10/20 9:11 AM, 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@gmail.com>
> ---
>   src/qemu/qemu_domain.c           |  4 +-
>   src/qemu/qemu_domain.h           | 10 +++++
>   src/qemu/qemu_domainjob.c        | 74 ++++++++++++++++++++++----------
>   src/qemu/qemu_domainjob.h        | 32 ++++++++------
>   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, 119 insertions(+), 56 deletions(-)

Almost.

>   
> +typedef void *(*qemuDomainObjPrivateJobAlloc)(void);
> +typedef void (*qemuDomainObjPrivateJobFree)(void *);
> +typedef int (*qemuDomainObjPrivateJobFormat)(virBufferPtr,
> +                                             virDomainObjPtr);
> +typedef int (*qemuDomainObjPrivateJobParse)(virDomainObjPtr,
> +                                            xmlXPathContextPtr);
> +
> +typedef struct _qemuDomainObjPrivateJobCallbacks qemuDomainObjPrivateJobCallbacks;
> +struct _qemuDomainObjPrivateJobCallbacks {
> +   qemuDomainObjPrivateJobAlloc allocJobPrivate;
> +   qemuDomainObjPrivateJobFree freeJobPrivate;
> +   qemuDomainObjPrivateJobFormat formatJob;
> +   qemuDomainObjPrivateJobParse parseJob;
> +};
> +

This looks correct.

>   typedef struct _qemuDomainJobObj qemuDomainJobObj;
>   typedef qemuDomainJobObj *qemuDomainJobObjPtr;
>   struct _qemuDomainJobObj {
> @@ -182,14 +197,11 @@ struct _qemuDomainJobObj {
>       qemuDomainJobInfoPtr current;       /* async job progress data */
>       qemuDomainJobInfoPtr completed;     /* statistics data of a recently completed job */
>       bool abortJob;                      /* abort of the job requested */
> -    bool spiceMigration;                /* we asked for spice migration and we
> -                                         * should wait for it to finish */
> -    bool spiceMigrated;                 /* spice migration completed */
>       char *error;                        /* job event completion error */
> -    bool dumpCompleted;                 /* dump completed */
> -
> -    qemuMigrationParamsPtr migParams;
>       unsigned long apiFlags; /* flags passed to the API which started the async job */
> +
> +    void *privateData;                  /* job specific collection of data */
> +    qemuDomainObjPrivateJobCallbacks cb;

And so does this. But these callbacks should be passed to 
qemuDomainObjInitJob() which will save them into the cb like this:

int
qemuDomainObjInitJob(qemuDomainJobObjPtr job,
                      qemuDomainObjPrivateJobCallbacks cb)
{
     memset(job, 0, sizeof(*job));

     if (virCondInit(&job->cond) < 0)
         return -1;

     if (virCondInit(&job->asyncCond) < 0) {
         virCondDestroy(&job->cond);
         return -1;
     }

     job->cb = cb;

     if (!(job->privateData = job->cb.allocJobPrivate())) {
         qemuDomainObjFreeJob(job);
         return -1;
     }

     return 0;
}


In general, nothing outside qemu_domainjob.c should be calling these 
callbacks. Therefore for instance when formatting private XML it should 
looks something like this:

static int
qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
                               virDomainObjPtr vm)
{
   ...
   if (qemuDomainJobFormat(buf, priv->job, vm) < 0)
       return -1;
   ...
}


This qemuDomainJobFormat() can look something like this:

int
qemuDomainObjJobFormat(virBufferPtr buf,
                        qemuDomainJobObjPtr job,
                        void *opaque)
{
     g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
     g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);

     if (!qemuDomainTrackJob(job))
         job = QEMU_JOB_NONE;

     if (job == QEMU_JOB_NONE &&
         jobObj->asyncJob == QEMU_ASYNC_JOB_NONE)
         return 0;

     virBufferAsprintf(&attrBuf, " type='%s' async='%s'",
                       qemuDomainJobTypeToString(job),
                       qemuDomainAsyncJobTypeToString(jobObj->asyncJob));

     if (jobObj->phase) {
         virBufferAsprintf(&attrBuf, " phase='%s'",
                           qemuDomainAsyncJobPhaseToString(jobObj->asyncJob,
                                                           jobObj->phase));
     }

     if (jobObj->asyncJob != QEMU_ASYNC_JOB_NONE)
         virBufferAsprintf(&attrBuf, " flags='0x%lx'", jobObj->apiFlags);

     if (job->cb.formatJob(&childBuf, vm) < 0)
         return -1;

     virXMLFormatElement(buf, "job", &attrBuf, &childBuf);

     return 0;
}


This looks very close to qemuDomainObjPrivateXMLFormatJob() and that is 
exactly how we want it. Except all "private" data is now formatted using 
callback (migParams for instance). Does this make more sense?

Michal

On Fri, Jul 10, 2020 at 8:06 PM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> On 7/10/20 9:11 AM, 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@gmail.com>
> > ---
> >   src/qemu/qemu_domain.c           |  4 +-
> >   src/qemu/qemu_domain.h           | 10 +++++
> >   src/qemu/qemu_domainjob.c        | 74 ++++++++++++++++++++++----------
> >   src/qemu/qemu_domainjob.h        | 32 ++++++++------
> >   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, 119 insertions(+), 56 deletions(-)
>
> Almost.
>
> >
> > +typedef void *(*qemuDomainObjPrivateJobAlloc)(void);
> > +typedef void (*qemuDomainObjPrivateJobFree)(void *);
> > +typedef int (*qemuDomainObjPrivateJobFormat)(virBufferPtr,
> > +                                             virDomainObjPtr);
> > +typedef int (*qemuDomainObjPrivateJobParse)(virDomainObjPtr,
> > +                                            xmlXPathContextPtr);
> > +
> > +typedef struct _qemuDomainObjPrivateJobCallbacks qemuDomainObjPrivateJobCallbacks;
> > +struct _qemuDomainObjPrivateJobCallbacks {
> > +   qemuDomainObjPrivateJobAlloc allocJobPrivate;
> > +   qemuDomainObjPrivateJobFree freeJobPrivate;
> > +   qemuDomainObjPrivateJobFormat formatJob;
> > +   qemuDomainObjPrivateJobParse parseJob;
> > +};
> > +
>
> This looks correct.
>
> >   typedef struct _qemuDomainJobObj qemuDomainJobObj;
> >   typedef qemuDomainJobObj *qemuDomainJobObjPtr;
> >   struct _qemuDomainJobObj {
> > @@ -182,14 +197,11 @@ struct _qemuDomainJobObj {
> >       qemuDomainJobInfoPtr current;       /* async job progress data */
> >       qemuDomainJobInfoPtr completed;     /* statistics data of a recently completed job */
> >       bool abortJob;                      /* abort of the job requested */
> > -    bool spiceMigration;                /* we asked for spice migration and we
> > -                                         * should wait for it to finish */
> > -    bool spiceMigrated;                 /* spice migration completed */
> >       char *error;                        /* job event completion error */
> > -    bool dumpCompleted;                 /* dump completed */
> > -
> > -    qemuMigrationParamsPtr migParams;
> >       unsigned long apiFlags; /* flags passed to the API which started the async job */
> > +
> > +    void *privateData;                  /* job specific collection of data */
> > +    qemuDomainObjPrivateJobCallbacks cb;
>
> And so does this. But these callbacks should be passed to
> qemuDomainObjInitJob() which will save them into the cb like this:
>
> int
> qemuDomainObjInitJob(qemuDomainJobObjPtr job,
>                       qemuDomainObjPrivateJobCallbacks cb)
> {
>      memset(job, 0, sizeof(*job));
>
>      if (virCondInit(&job->cond) < 0)
>          return -1;
>
>      if (virCondInit(&job->asyncCond) < 0) {
>          virCondDestroy(&job->cond);
>          return -1;
>      }
>
>      job->cb = cb;
>
>      if (!(job->privateData = job->cb.allocJobPrivate())) {
>          qemuDomainObjFreeJob(job);
>          return -1;
>      }
>
>      return 0;
> }
>
>
> In general, nothing outside qemu_domainjob.c should be calling these
> callbacks. Therefore for instance when formatting private XML it should
> looks something like this:
>
> static int
> qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
>                                virDomainObjPtr vm)
> {
>    ...
>    if (qemuDomainJobFormat(buf, priv->job, vm) < 0)
>        return -1;
>    ...
> }
>
>
> This qemuDomainJobFormat() can look something like this:
>
> int
> qemuDomainObjJobFormat(virBufferPtr buf,
>                         qemuDomainJobObjPtr job,
>                         void *opaque)
> {
>      g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
>      g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
>
>      if (!qemuDomainTrackJob(job))
>          job = QEMU_JOB_NONE;
>
>      if (job == QEMU_JOB_NONE &&
>          jobObj->asyncJob == QEMU_ASYNC_JOB_NONE)
>          return 0;
>
>      virBufferAsprintf(&attrBuf, " type='%s' async='%s'",
>                        qemuDomainJobTypeToString(job),
>                        qemuDomainAsyncJobTypeToString(jobObj->asyncJob));
>
>      if (jobObj->phase) {
>          virBufferAsprintf(&attrBuf, " phase='%s'",
>                            qemuDomainAsyncJobPhaseToString(jobObj->asyncJob,
>                                                            jobObj->phase));
>      }
>
>      if (jobObj->asyncJob != QEMU_ASYNC_JOB_NONE)
>          virBufferAsprintf(&attrBuf, " flags='0x%lx'", jobObj->apiFlags);
>
>      if (job->cb.formatJob(&childBuf, vm) < 0)
>          return -1;
>
>      virXMLFormatElement(buf, "job", &attrBuf, &childBuf);
>
>      return 0;
> }
>
>
> This looks very close to qemuDomainObjPrivateXMLFormatJob() and that is
> exactly how we want it. Except all "private" data is now formatted using
> callback (migParams for instance). Does this make more sense?
>

Yes, it does. I can see how this will be helpful when we'll be moving
the domainjob functions into a common file.

But I still think that, since the function outside domain job should
be allowed to access this callback function, even the function:
qemuDomainFormatJob() needs to be moved to qemu_domainjob from
qemu_domain.
So it makes this series have 4 patches:
1. Changing the params of qemuDomainObjPrivateXMLFormatJob and ParseJob.
2. Moving these functions to qemu_domainjob
3. Creating privateData and callback functions structure for qemuDomainJob
4. Creating privateData and callback functions structure for qemuDomainJobInfo

Thanks,
Prathamesh Chavan