[GSoC][PATCH v3 0/4] removal of qemu_domainjob dependencies
The following series of patches work on isolating the qemu_domainjob from its dependency on other files such as `qemu_migration_params`, `qemu_monitor`, etc. This is done by the introduction of a `privateData` structure, which is further handled by a structure of callback functions. Previous version of this patch can be found here[1]. As the previous patch was a bulky one, this series of patches are the result of splitting the previous one. [1]: https://www.redhat.com/archives/libvir-list/2020-July/msg00423.html Prathamesh Chavan (4): qemu_domain: moved qemuDomainNamespace to `qemu_domain` qemu_domainjob: job funcitons moved out of `qemu_domain` qemu_domainjob: introduce `privateData` for `qemuDomainJob` qemu_domainjob: introduce `privateData` for `qemuDomainJobInfo` src/qemu/qemu_backup.c | 15 +- src/qemu/qemu_domain.c | 250 +------------------ src/qemu/qemu_domain.h | 28 +++ src/qemu/qemu_domainjob.c | 406 +++++++++++++++++++++++++++---- src/qemu/qemu_domainjob.h | 54 ++-- src/qemu/qemu_driver.c | 21 +- src/qemu/qemu_migration.c | 42 ++-- src/qemu/qemu_migration_cookie.c | 7 +- src/qemu/qemu_migration_params.c | 9 +- src/qemu/qemu_process.c | 26 +- 10 files changed, 510 insertions(+), 348 deletions(-) -- 2.17.1
While moving the code, qemuDomainNamespace also was moved to `qemu_domainjob`. Hence it is moved back to `qemu_domain` where it will be more appropriate. Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- src/qemu/qemu_domain.c | 5 +++++ src/qemu/qemu_domainjob.c | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 248e259634..2d9d8226d6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -83,6 +83,11 @@ VIR_LOG_INIT("qemu.qemu_domain"); +VIR_ENUM_IMPL(qemuDomainNamespace, + QEMU_DOMAIN_NS_LAST, + "mount", +); + /** * qemuDomainObjFromDomain: * @domain: Domain pointer that has to be looked up diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 7111acadda..654f1c4f90 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -63,12 +63,6 @@ VIR_ENUM_IMPL(qemuDomainAsyncJob, "backup", ); -VIR_ENUM_IMPL(qemuDomainNamespace, - QEMU_DOMAIN_NS_LAST, - "mount", -); - - const char * qemuDomainAsyncJobPhaseToString(qemuDomainAsyncJob job, int phase G_GNUC_UNUSED) -- 2.17.1
On 7/10/20 9:11 AM, Prathamesh Chavan wrote:
While moving the code, qemuDomainNamespace also was moved to `qemu_domainjob`. Hence it is moved back to `qemu_domain` where it will be more appropriate.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- src/qemu/qemu_domain.c | 5 +++++ src/qemu/qemu_domainjob.c | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-)
Oops, sorry for not spotting this in the original patchset. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
We move functions `qemuDomainObjPrivateXMLParseJob` and `qemuDomainObjPrivateXMLFormatJob` to `qemu_domainjob`. To make this happen, corresponding changes in their parameters were made, as well as they were exposed to be used in `qemu_domain` file. Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- src/qemu/qemu_domain.c | 245 +------------------------------------- src/qemu/qemu_domainjob.c | 244 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domainjob.h | 8 ++ 3 files changed, 254 insertions(+), 243 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2d9d8226d6..10d2033db1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2168,102 +2168,6 @@ qemuDomainObjPrivateXMLFormatPR(virBufferPtr buf, } -static int -qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf, - virStorageSourcePtr src, - virDomainXMLOptionPtr xmlopt) -{ - g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); - - virBufferAsprintf(&attrBuf, " type='%s' format='%s'", - virStorageTypeToString(src->type), - virStorageFileFormatTypeToString(src->format)); - - if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, false, - VIR_DOMAIN_DEF_FORMAT_STATUS, - false, false, xmlopt) < 0) - return -1; - - virXMLFormatElement(buf, "migrationSource", &attrBuf, &childBuf); - - return 0; -} - - -static int -qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf, - virDomainObjPtr vm) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; - virDomainDiskDefPtr disk; - qemuDomainDiskPrivatePtr diskPriv; - - for (i = 0; i < vm->def->ndisks; i++) { - g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); - disk = vm->def->disks[i]; - diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - - virBufferAsprintf(&attrBuf, " dev='%s' migrating='%s'", - disk->dst, diskPriv->migrating ? "yes" : "no"); - - if (diskPriv->migrSource && - qemuDomainObjPrivateXMLFormatNBDMigrationSource(&childBuf, - diskPriv->migrSource, - priv->driver->xmlopt) < 0) - return -1; - - virXMLFormatElement(buf, "disk", &attrBuf, &childBuf); - } - - return 0; -} - - -static int -qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf, - virDomainObjPtr vm, - qemuDomainObjPrivatePtr priv) -{ - g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); - qemuDomainJob job = priv->job.active; - - if (!qemuDomainTrackJob(job)) - job = QEMU_JOB_NONE; - - if (job == QEMU_JOB_NONE && - priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) - return 0; - - virBufferAsprintf(&attrBuf, " type='%s' async='%s'", - qemuDomainJobTypeToString(job), - qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); - - if (priv->job.phase) { - virBufferAsprintf(&attrBuf, " phase='%s'", - qemuDomainAsyncJobPhaseToString(priv->job.asyncJob, - priv->job.phase)); - } - - if (priv->job.asyncJob != QEMU_ASYNC_JOB_NONE) - virBufferAsprintf(&attrBuf, " flags='0x%lx'", priv->job.apiFlags); - - if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT && - qemuDomainObjPrivateXMLFormatNBDMigration(&childBuf, vm) < 0) - return -1; - - if (priv->job.migParams) - qemuMigrationParamsFormat(&childBuf, priv->job.migParams); - - virXMLFormatElement(buf, "job", &attrBuf, &childBuf); - - return 0; -} - - static bool qemuDomainHasSlirp(virDomainObjPtr vm) { @@ -2399,7 +2303,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, if (priv->lockState) virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState); - if (qemuDomainObjPrivateXMLFormatJob(buf, vm, priv) < 0) + if (qemuDomainObjPrivateXMLFormatJob(buf, vm) < 0) return -1; if (priv->fakeReboot) @@ -2899,151 +2803,6 @@ qemuDomainObjPrivateXMLParsePR(xmlXPathContextPtr ctxt, } -static int -qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node, - xmlXPathContextPtr ctxt, - virDomainDiskDefPtr disk, - virDomainXMLOptionPtr xmlopt) -{ - VIR_XPATH_NODE_AUTORESTORE(ctxt); - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - g_autofree char *format = NULL; - g_autofree char *type = NULL; - g_autoptr(virStorageSource) migrSource = NULL; - xmlNodePtr sourceNode; - - ctxt->node = node; - - if (!(ctxt->node = virXPathNode("./migrationSource", ctxt))) - return 0; - - if (!(type = virXMLPropString(ctxt->node, "type"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing storage source type")); - return -1; - } - - if (!(format = virXMLPropString(ctxt->node, "format"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing storage source format")); - return -1; - } - - if (!(migrSource = virDomainStorageSourceParseBase(type, format, NULL))) - return -1; - - /* newer libvirt uses the <source> subelement instead of formatting the - * source directly into <migrationSource> */ - if ((sourceNode = virXPathNode("./source", ctxt))) - ctxt->node = sourceNode; - - if (virDomainStorageSourceParse(ctxt->node, ctxt, migrSource, - VIR_DOMAIN_DEF_PARSE_STATUS, xmlopt) < 0) - return -1; - - diskPriv->migrSource = g_steal_pointer(&migrSource); - return 0; -} - - -static int -qemuDomainObjPrivateXMLParseJobNBD(virDomainObjPtr vm, - qemuDomainObjPrivatePtr priv, - xmlXPathContextPtr ctxt) -{ - g_autofree xmlNodePtr *nodes = NULL; - size_t i; - int n; - - if ((n = virXPathNodeSet("./disk[@migrating='yes']", ctxt, &nodes)) < 0) - return -1; - - if (n > 0) { - if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { - VIR_WARN("Found disks marked for migration but we were not " - "migrating"); - n = 0; - } - for (i = 0; i < n; i++) { - virDomainDiskDefPtr disk; - g_autofree char *dst = NULL; - - if ((dst = virXMLPropString(nodes[i], "dev")) && - (disk = virDomainDiskByTarget(vm->def, dst))) { - QEMU_DOMAIN_DISK_PRIVATE(disk)->migrating = true; - - if (qemuDomainObjPrivateXMLParseJobNBDSource(nodes[i], ctxt, - disk, - priv->driver->xmlopt) < 0) - return -1; - } - } - } - - return 0; -} - - -static int -qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, - qemuDomainObjPrivatePtr priv, - xmlXPathContextPtr ctxt) -{ - VIR_XPATH_NODE_AUTORESTORE(ctxt); - g_autofree char *tmp = NULL; - - if (!(ctxt->node = virXPathNode("./job[1]", ctxt))) - return 0; - - if ((tmp = virXPathString("string(@type)", ctxt))) { - int type; - - if ((type = qemuDomainJobTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown job type %s"), tmp); - return -1; - } - VIR_FREE(tmp); - priv->job.active = type; - } - - if ((tmp = virXPathString("string(@async)", ctxt))) { - int async; - - if ((async = qemuDomainAsyncJobTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown async job type %s"), tmp); - return -1; - } - VIR_FREE(tmp); - priv->job.asyncJob = async; - - if ((tmp = virXPathString("string(@phase)", ctxt))) { - priv->job.phase = qemuDomainAsyncJobPhaseFromString(async, tmp); - if (priv->job.phase < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown job phase %s"), tmp); - return -1; - } - VIR_FREE(tmp); - } - } - - if (virXPathULongHex("string(@flags)", ctxt, &priv->job.apiFlags) == -2) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid job flags")); - return -1; - } - - if (qemuDomainObjPrivateXMLParseJobNBD(vm, priv, ctxt) < 0) - return -1; - - if (qemuMigrationParamsParse(ctxt, &priv->job.migParams) < 0) - return -1; - - return 0; -} - - static int qemuDomainObjPrivateXMLParseSlirpFeatures(xmlNodePtr featuresNode, xmlXPathContextPtr ctxt, @@ -3203,7 +2962,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, priv->lockState = virXPathString("string(./lockstate)", ctxt); - if (qemuDomainObjPrivateXMLParseJob(vm, priv, ctxt) < 0) + if (qemuDomainObjPrivateXMLParseJob(vm, ctxt) < 0) goto error; priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1; diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 654f1c4f90..454a5a2c23 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -1184,3 +1184,247 @@ qemuDomainObjAbortAsyncJob(virDomainObjPtr obj) priv->job.abortJob = true; virDomainObjBroadcast(obj); } + + +static int +qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + + virBufferAsprintf(&attrBuf, " type='%s' format='%s'", + virStorageTypeToString(src->type), + virStorageFileFormatTypeToString(src->format)); + + if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, false, + VIR_DOMAIN_DEF_FORMAT_STATUS, + false, false, xmlopt) < 0) + return -1; + + virXMLFormatElement(buf, "migrationSource", &attrBuf, &childBuf); + + return 0; +} + + +static int +qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + virDomainDiskDefPtr disk; + qemuDomainDiskPrivatePtr diskPriv; + + for (i = 0; i < vm->def->ndisks; i++) { + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + disk = vm->def->disks[i]; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + virBufferAsprintf(&attrBuf, " dev='%s' migrating='%s'", + disk->dst, diskPriv->migrating ? "yes" : "no"); + + if (diskPriv->migrSource && + qemuDomainObjPrivateXMLFormatNBDMigrationSource(&childBuf, + diskPriv->migrSource, + priv->driver->xmlopt) < 0) + return -1; + + virXMLFormatElement(buf, "disk", &attrBuf, &childBuf); + } + + return 0; +} + + +int +qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainJobObjPtr jobObj = &priv->job; + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + qemuDomainJob job = jobObj->active; + + 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 (jobObj->asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT && + qemuDomainObjPrivateXMLFormatNBDMigration(&childBuf, vm) < 0) + return -1; + + if (jobObj->migParams) + qemuMigrationParamsFormat(&childBuf, jobObj->migParams); + + virXMLFormatElement(buf, "job", &attrBuf, &childBuf); + + return 0; +} + + +static int +qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainDiskDefPtr disk, + virDomainXMLOptionPtr xmlopt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt); + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + g_autofree char *format = NULL; + g_autofree char *type = NULL; + g_autoptr(virStorageSource) migrSource = NULL; + xmlNodePtr sourceNode; + + ctxt->node = node; + + if (!(ctxt->node = virXPathNode("./migrationSource", ctxt))) + return 0; + + if (!(type = virXMLPropString(ctxt->node, "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing storage source type")); + return -1; + } + + if (!(format = virXMLPropString(ctxt->node, "format"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing storage source format")); + return -1; + } + + if (!(migrSource = virDomainStorageSourceParseBase(type, format, NULL))) + return -1; + + /* newer libvirt uses the <source> subelement instead of formatting the + * source directly into <migrationSource> */ + if ((sourceNode = virXPathNode("./source", ctxt))) + ctxt->node = sourceNode; + + if (virDomainStorageSourceParse(ctxt->node, ctxt, migrSource, + VIR_DOMAIN_DEF_PARSE_STATUS, xmlopt) < 0) + return -1; + + diskPriv->migrSource = g_steal_pointer(&migrSource); + return 0; +} + + +static int +qemuDomainObjPrivateXMLParseJobNBD(virDomainObjPtr vm, + xmlXPathContextPtr ctxt) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainJobObjPtr job = &priv->job; + virDomainXMLOptionPtr xmlopt = priv->driver->xmlopt; + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + + if ((n = virXPathNodeSet("./disk[@migrating='yes']", ctxt, &nodes)) < 0) + return -1; + + if (n > 0) { + if (job->asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { + VIR_WARN("Found disks marked for migration but we were not " + "migrating"); + n = 0; + } + for (i = 0; i < n; i++) { + virDomainDiskDefPtr disk; + g_autofree char *dst = NULL; + + if ((dst = virXMLPropString(nodes[i], "dev")) && + (disk = virDomainDiskByTarget(vm->def, dst))) { + QEMU_DOMAIN_DISK_PRIVATE(disk)->migrating = true; + + if (qemuDomainObjPrivateXMLParseJobNBDSource(nodes[i], ctxt, + disk, + xmlopt) < 0) + return -1; + } + } + } + + return 0; +} + +int +qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, + xmlXPathContextPtr ctxt) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainJobObjPtr job = &priv->job; + VIR_XPATH_NODE_AUTORESTORE(ctxt); + g_autofree char *tmp = NULL; + + if (!(ctxt->node = virXPathNode("./job[1]", ctxt))) + return 0; + + if ((tmp = virXPathString("string(@type)", ctxt))) { + int type; + + if ((type = qemuDomainJobTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown job type %s"), tmp); + return -1; + } + VIR_FREE(tmp); + job->active = type; + } + + if ((tmp = virXPathString("string(@async)", ctxt))) { + int async; + + if ((async = qemuDomainAsyncJobTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown async job type %s"), tmp); + return -1; + } + VIR_FREE(tmp); + job->asyncJob = async; + + if ((tmp = virXPathString("string(@phase)", ctxt))) { + job->phase = qemuDomainAsyncJobPhaseFromString(async, tmp); + if (job->phase < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown job phase %s"), tmp); + return -1; + } + VIR_FREE(tmp); + } + } + + if (virXPathULongHex("string(@flags)", ctxt, &job->apiFlags) == -2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid job flags")); + return -1; + } + + if (qemuDomainObjPrivateXMLParseJobNBD(vm, ctxt) < 0) + return -1; + + if (qemuMigrationParamsParse(ctxt, &job->migParams) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h index 124664354d..9d2ee14584 100644 --- a/src/qemu/qemu_domainjob.h +++ b/src/qemu/qemu_domainjob.h @@ -267,3 +267,11 @@ 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); -- 2.17.1
On 7/10/20 9:11 AM, Prathamesh Chavan wrote:
We move functions `qemuDomainObjPrivateXMLParseJob` and `qemuDomainObjPrivateXMLFormatJob` to `qemu_domainjob`. To make this happen, corresponding changes in their parameters were made, as well as they were exposed to be used in `qemu_domain` file.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- src/qemu/qemu_domain.c | 245 +------------------------------------- src/qemu/qemu_domainjob.c | 244 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domainjob.h | 8 ++ 3 files changed, 254 insertions(+), 243 deletions(-)
static bool qemuDomainHasSlirp(virDomainObjPtr vm) { @@ -2399,7 +2303,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, if (priv->lockState) virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState);
- if (qemuDomainObjPrivateXMLFormatJob(buf, vm, priv) < 0) + if (qemuDomainObjPrivateXMLFormatJob(buf, vm) < 0)
This means, that this patch is not a plain code movement. Just for future work, break patches into logical/semantic changes. For instance in this case it could have been: one patch that drops the @priv argument (since the function already gets @vm from which it can obtain @priv), the other patch would be actual code movement. I know it probably doesn't make much sense and I totally get it. I did not make any sense to me when I was starting with libvirt - the end result is the same, isn't it? What's the fuss then? but then I started reviewing patches and realized very quickly how important it is. And then I started backporting patches to RHEL and I realized it again :-) But as you'll see in patch 3/4 we probably don't need to move everything. Michal
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
To remove dependecy of `qemuDomainJobInfo` 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_backup.c | 15 ++++-- src/qemu/qemu_domain.h | 18 +++++++ src/qemu/qemu_domainjob.c | 92 ++++++++++++++++++++++++-------- src/qemu/qemu_domainjob.h | 30 ++++++----- src/qemu/qemu_driver.c | 18 ++++--- src/qemu/qemu_migration.c | 14 +++-- src/qemu/qemu_migration_cookie.c | 7 ++- src/qemu/qemu_process.c | 11 ++-- 8 files changed, 147 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index b711f8f623..a0832b4587 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -529,6 +529,7 @@ qemuBackupJobTerminate(virDomainObjPtr vm, { qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainJobInfoPrivatePtr jobInfoPriv; size_t i; qemuDomainJobInfoUpdateTime(priv->job.current); @@ -536,10 +537,13 @@ qemuBackupJobTerminate(virDomainObjPtr vm, g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); priv->job.completed = qemuDomainJobInfoCopy(priv->job.current); - priv->job.completed->stats.backup.total = priv->backup->push_total; - priv->job.completed->stats.backup.transferred = priv->backup->push_transferred; - priv->job.completed->stats.backup.tmp_used = priv->backup->pull_tmp_used; - priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total; + + jobInfoPriv = priv->job.completed->privateData; + + jobInfoPriv->stats.backup.total = priv->backup->push_total; + jobInfoPriv->stats.backup.transferred = priv->backup->push_transferred; + jobInfoPriv->stats.backup.tmp_used = priv->backup->pull_tmp_used; + jobInfoPriv->stats.backup.tmp_total = priv->backup->pull_tmp_total; priv->job.completed->status = jobstatus; priv->job.completed->errmsg = g_strdup(priv->backup->errmsg); @@ -1069,7 +1073,8 @@ qemuBackupGetJobInfoStats(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainJobInfoPtr jobInfo) { - qemuDomainBackupStats *stats = &jobInfo->stats.backup; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; + qemuDomainBackupStats *stats = &jobInfoPriv->stats.backup; qemuDomainObjPrivatePtr priv = vm->privateData; qemuMonitorJobInfoPtr *blockjobs = NULL; size_t nblockjobs = 0; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index bb9b414a46..7fcbefd0c0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -502,6 +502,24 @@ struct _qemuDomainJobPrivate { qemuMigrationParamsPtr migParams; }; +typedef struct _qemuDomainBackupStats qemuDomainBackupStats; +struct _qemuDomainBackupStats { + unsigned long long transferred; + unsigned long long total; + unsigned long long tmp_used; + unsigned long long tmp_total; +}; + +typedef struct _qemuDomainJobInfoPrivate qemuDomainJobInfoPrivate; +typedef qemuDomainJobInfoPrivate *qemuDomainJobInfoPrivatePtr; +struct _qemuDomainJobInfoPrivate { + union { + qemuMonitorMigrationStats mig; + qemuMonitorDumpStats dump; + qemuDomainBackupStats backup; + } stats; +}; + int qemuDomainObjStartWorker(virDomainObjPtr dom); void qemuDomainObjStopWorker(virDomainObjPtr dom); diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index d79b8d49f6..d8913f1e70 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -115,22 +115,68 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job, return -1; } +static void +qemuJobInfoFreePrivateData(qemuDomainJobInfoPrivatePtr priv) +{ + g_free(&priv->stats); +} + +static void +qemuJobInfoFreePrivate(void *opaque) +{ + qemuDomainJobInfoPtr jobInfo = (qemuDomainJobInfoPtr) opaque; + qemuJobInfoFreePrivateData(jobInfo->privateData); +} void qemuDomainJobInfoFree(qemuDomainJobInfoPtr info) { + info->cb.freeJobInfoPrivate(info); g_free(info->errmsg); g_free(info); } +static void * +qemuDomainJobInfoPrivateAlloc(void) +{ + qemuDomainJobInfoPrivatePtr retPriv = g_new0(qemuDomainJobInfoPrivate, 1); + return (void *)retPriv; +} + +static void +qemuDomainJobInfoPrivateCopy(qemuDomainJobInfoPtr src, + qemuDomainJobInfoPtr dest) +{ + memcpy(dest->privateData, src->privateData, + sizeof(qemuDomainJobInfoPrivate)); +} + +static qemuDomainJobInfoPtr +qemuDomainJobInfoAlloc(void) +{ + qemuDomainJobInfoPtr ret = g_new0(qemuDomainJobInfo, 1); + ret->cb.allocJobInfoPrivate = &qemuDomainJobInfoPrivateAlloc; + ret->cb.freeJobInfoPrivate = &qemuJobInfoFreePrivate; + ret->cb.copyJobInfoPrivate = &qemuDomainJobInfoPrivateCopy; + ret->privateData = ret->cb.allocJobInfoPrivate(); + return ret; +} + +static void +qemuDomainCurrentJobInfoInit(qemuDomainJobObjPtr job) +{ + job->current = qemuDomainJobInfoAlloc(); + job->current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; +} qemuDomainJobInfoPtr qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info) { - qemuDomainJobInfoPtr ret = g_new0(qemuDomainJobInfo, 1); + qemuDomainJobInfoPtr ret = qemuDomainJobInfoAlloc(); memcpy(ret, info, sizeof(*info)); + ret->cb.copyJobInfoPrivate(info, ret); ret->errmsg = g_strdup(info->errmsg); return ret; @@ -284,6 +330,7 @@ int qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo) { unsigned long long now; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; if (!jobInfo->stopped) return 0; @@ -297,8 +344,8 @@ qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo) return 0; } - jobInfo->stats.mig.downtime = now - jobInfo->stopped; - jobInfo->stats.mig.downtime_set = true; + jobInfoPriv->stats.mig.downtime = now - jobInfo->stopped; + jobInfoPriv->stats.mig.downtime_set = true; return 0; } @@ -333,38 +380,39 @@ int qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, virDomainJobInfoPtr info) { + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; info->type = qemuDomainJobStatusToType(jobInfo->status); info->timeElapsed = jobInfo->timeElapsed; switch (jobInfo->statsType) { case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: - info->memTotal = jobInfo->stats.mig.ram_total; - info->memRemaining = jobInfo->stats.mig.ram_remaining; - info->memProcessed = jobInfo->stats.mig.ram_transferred; - info->fileTotal = jobInfo->stats.mig.disk_total + + info->memTotal = jobInfoPriv->stats.mig.ram_total; + info->memRemaining = jobInfoPriv->stats.mig.ram_remaining; + info->memProcessed = jobInfoPriv->stats.mig.ram_transferred; + info->fileTotal = jobInfoPriv->stats.mig.disk_total + jobInfo->mirrorStats.total; - info->fileRemaining = jobInfo->stats.mig.disk_remaining + + info->fileRemaining = jobInfoPriv->stats.mig.disk_remaining + (jobInfo->mirrorStats.total - jobInfo->mirrorStats.transferred); - info->fileProcessed = jobInfo->stats.mig.disk_transferred + + info->fileProcessed = jobInfoPriv->stats.mig.disk_transferred + jobInfo->mirrorStats.transferred; break; case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: - info->memTotal = jobInfo->stats.mig.ram_total; - info->memRemaining = jobInfo->stats.mig.ram_remaining; - info->memProcessed = jobInfo->stats.mig.ram_transferred; + info->memTotal = jobInfoPriv->stats.mig.ram_total; + info->memRemaining = jobInfoPriv->stats.mig.ram_remaining; + info->memProcessed = jobInfoPriv->stats.mig.ram_transferred; break; case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: - info->memTotal = jobInfo->stats.dump.total; - info->memProcessed = jobInfo->stats.dump.completed; + info->memTotal = jobInfoPriv->stats.dump.total; + info->memProcessed = jobInfoPriv->stats.dump.completed; info->memRemaining = info->memTotal - info->memProcessed; break; case QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP: - info->fileTotal = jobInfo->stats.backup.total; - info->fileProcessed = jobInfo->stats.backup.transferred; + info->fileTotal = jobInfoPriv->stats.backup.total; + info->fileProcessed = jobInfoPriv->stats.backup.transferred; info->fileRemaining = info->fileTotal - info->fileProcessed; break; @@ -386,7 +434,8 @@ qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, virTypedParameterPtr *params, int *nparams) { - qemuMonitorMigrationStats *stats = &jobInfo->stats.mig; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; + qemuMonitorMigrationStats *stats = &jobInfoPriv->stats.mig; qemuDomainMirrorStatsPtr mirrorStats = &jobInfo->mirrorStats; virTypedParameterPtr par = NULL; int maxpar = 0; @@ -563,7 +612,8 @@ qemuDomainDumpJobInfoToParams(qemuDomainJobInfoPtr jobInfo, virTypedParameterPtr *params, int *nparams) { - qemuMonitorDumpStats *stats = &jobInfo->stats.dump; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; + qemuMonitorDumpStats *stats = &jobInfoPriv->stats.dump; virTypedParameterPtr par = NULL; int maxpar = 0; int npar = 0; @@ -606,7 +656,8 @@ qemuDomainBackupJobInfoToParams(qemuDomainJobInfoPtr jobInfo, virTypedParameterPtr *params, int *nparams) { - qemuDomainBackupStats *stats = &jobInfo->stats.backup; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; + qemuDomainBackupStats *stats = &jobInfoPriv->stats.backup; g_autoptr(virTypedParamList) par = g_new0(virTypedParamList, 1); if (virTypedParamListAddInt(par, jobInfo->operation, @@ -889,8 +940,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, qemuDomainAsyncJobTypeToString(asyncJob), obj, obj->def->name); qemuDomainObjResetAsyncJob(&priv->job); - priv->job.current = g_new0(qemuDomainJobInfo, 1); - priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; + qemuDomainCurrentJobInfoInit(&priv->job); priv->job.asyncJob = asyncJob; priv->job.asyncOwner = virThreadSelfID(); priv->job.asyncOwnerAPI = virThreadJobGet(); diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h index 04e0a4315e..ba6d5d0d92 100644 --- a/src/qemu/qemu_domainjob.h +++ b/src/qemu/qemu_domainjob.h @@ -107,16 +107,21 @@ struct _qemuDomainMirrorStats { unsigned long long total; }; -typedef struct _qemuDomainBackupStats qemuDomainBackupStats; -struct _qemuDomainBackupStats { - unsigned long long transferred; - unsigned long long total; - unsigned long long tmp_used; - unsigned long long tmp_total; -}; - typedef struct _qemuDomainJobInfo qemuDomainJobInfo; typedef qemuDomainJobInfo *qemuDomainJobInfoPtr; + +typedef void *(*qemuDomainObjJobInfoPrivateAlloc)(void); +typedef void (*qemuDomainObjJobInfoPrivateFree)(void *); +typedef void (*qemuDomainObjJobInfoPrivateCopy)(qemuDomainJobInfoPtr, + qemuDomainJobInfoPtr); + +typedef struct _qemuDomainObjPrivateJobInfoCallbacks qemuDomainObjPrivateJobInfoCallbacks; +struct _qemuDomainObjPrivateJobInfoCallbacks { + qemuDomainObjJobInfoPrivateAlloc allocJobInfoPrivate; + qemuDomainObjJobInfoPrivateFree freeJobInfoPrivate; + qemuDomainObjJobInfoPrivateCopy copyJobInfoPrivate; +}; + struct _qemuDomainJobInfo { qemuDomainJobStatus status; virDomainJobOperation operation; @@ -136,14 +141,11 @@ struct _qemuDomainJobInfo { bool timeDeltaSet; /* Raw values from QEMU */ qemuDomainJobStatsType statsType; - union { - qemuMonitorMigrationStats mig; - qemuMonitorDumpStats dump; - qemuDomainBackupStats backup; - } stats; qemuDomainMirrorStats mirrorStats; - char *errmsg; /* optional error message for failed completed jobs */ + + void *privateData; /* job specific collection of info */ + qemuDomainObjPrivateJobInfoCallbacks cb; }; void diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7339856caa..9f5b93a546 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3702,6 +3702,7 @@ qemuDumpWaitForCompletion(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainJobPrivatePtr jobPriv = priv->job.privateData; + qemuDomainJobInfoPrivatePtr jobInfoPriv = priv->job.current->privateData; VIR_DEBUG("Waiting for dump completion"); while (!jobPriv->dumpCompleted && !priv->job.abortJob) { @@ -3709,7 +3710,7 @@ qemuDumpWaitForCompletion(virDomainObjPtr vm) return -1; } - if (priv->job.current->stats.dump.status == QEMU_MONITOR_DUMP_STATUS_FAILED) { + if (jobInfoPriv->stats.dump.status == QEMU_MONITOR_DUMP_STATUS_FAILED) { if (priv->job.error) virReportError(VIR_ERR_OPERATION_FAILED, _("memory-only dump failed: %s"), @@ -13550,6 +13551,7 @@ qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; qemuMonitorDumpStats stats = { 0 }; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; int rc; if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) @@ -13560,33 +13562,33 @@ qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) return -1; - jobInfo->stats.dump = stats; + jobInfoPriv->stats.dump = stats; if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) return -1; - switch (jobInfo->stats.dump.status) { + switch (jobInfoPriv->stats.dump.status) { case QEMU_MONITOR_DUMP_STATUS_NONE: case QEMU_MONITOR_DUMP_STATUS_FAILED: case QEMU_MONITOR_DUMP_STATUS_LAST: virReportError(VIR_ERR_OPERATION_FAILED, _("dump query failed, status=%d"), - jobInfo->stats.dump.status); + jobInfoPriv->stats.dump.status); return -1; break; case QEMU_MONITOR_DUMP_STATUS_ACTIVE: jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; VIR_DEBUG("dump active, bytes written='%llu' remaining='%llu'", - jobInfo->stats.dump.completed, - jobInfo->stats.dump.total - - jobInfo->stats.dump.completed); + jobInfoPriv->stats.dump.completed, + jobInfoPriv->stats.dump.total - + jobInfoPriv->stats.dump.completed); break; case QEMU_MONITOR_DUMP_STATUS_COMPLETED: jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; VIR_DEBUG("dump completed, bytes written='%llu'", - jobInfo->stats.dump.completed); + jobInfoPriv->stats.dump.completed); break; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b489b41a67..302cbe4c8e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1439,7 +1439,8 @@ qemuMigrationSrcWaitForSpice(virDomainObjPtr vm) static void qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo) { - switch ((qemuMonitorMigrationStatus) jobInfo->stats.mig.status) { + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; + switch ((qemuMonitorMigrationStatus) jobInfoPriv->stats.mig.status) { case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: jobInfo->status = QEMU_DOMAIN_JOB_STATUS_POSTCOPY; break; @@ -1486,6 +1487,7 @@ qemuMigrationAnyFetchStats(virQEMUDriverPtr driver, char **error) { qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; qemuMonitorMigrationStats stats; int rv; @@ -1497,7 +1499,7 @@ qemuMigrationAnyFetchStats(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) return -1; - jobInfo->stats.mig = stats; + jobInfoPriv->stats.mig = stats; return 0; } @@ -1539,12 +1541,13 @@ qemuMigrationJobCheckStatus(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainJobInfoPtr jobInfo = priv->job.current; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; char *error = NULL; bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); int ret = -1; if (!events || - jobInfo->stats.mig.status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { + jobInfoPriv->stats.mig.status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { if (qemuMigrationAnyFetchStats(driver, vm, asyncJob, jobInfo, &error) < 0) return -1; } @@ -3033,6 +3036,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, /* Update times with the values sent by the destination daemon */ if (mig->jobInfo && jobInfo) { int reason; + qemuDomainJobInfoPrivatePtr jobInfoPriv = mig->jobInfo->privateData; /* We need to refresh migration statistics after a completed post-copy * migration since priv->job.completed contains obsolete data from the @@ -3047,8 +3051,8 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, qemuDomainJobInfoUpdateTime(jobInfo); jobInfo->timeDeltaSet = mig->jobInfo->timeDeltaSet; jobInfo->timeDelta = mig->jobInfo->timeDelta; - jobInfo->stats.mig.downtime_set = mig->jobInfo->stats.mig.downtime_set; - jobInfo->stats.mig.downtime = mig->jobInfo->stats.mig.downtime; + jobInfoPriv->stats.mig.downtime_set = jobInfoPriv->stats.mig.downtime_set; + jobInfoPriv->stats.mig.downtime = jobInfoPriv->stats.mig.downtime; } if (flags & VIR_MIGRATE_OFFLINE) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 2e48d1b524..ac1ca7bde4 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -641,7 +641,8 @@ static void qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf, qemuDomainJobInfoPtr jobInfo) { - qemuMonitorMigrationStats *stats = &jobInfo->stats.mig; + qemuDomainJobInfoPrivatePtr jobInfoPriv = jobInfo->privateData; + qemuMonitorMigrationStats *stats = &jobInfoPriv->stats.mig; virBufferAddLit(buf, "<statistics>\n"); virBufferAdjustIndent(buf, 2); @@ -1043,14 +1044,16 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) { qemuDomainJobInfoPtr jobInfo = NULL; qemuMonitorMigrationStats *stats; + qemuDomainJobInfoPrivatePtr jobInfoPriv; VIR_XPATH_NODE_AUTORESTORE(ctxt); if (!(ctxt->node = virXPathNode("./statistics", ctxt))) return NULL; jobInfo = g_new0(qemuDomainJobInfo, 1); + jobInfoPriv = jobInfo->privateData; - stats = &jobInfo->stats.mig; + stats = &jobInfoPriv->stats.mig; jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; virXPathULongLong("string(./started[1])", ctxt, &jobInfo->started); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ac0c0eb8e1..05209c8a85 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1641,6 +1641,7 @@ qemuProcessHandleMigrationStatus(qemuMonitorPtr mon G_GNUC_UNUSED, virQEMUDriverPtr driver = opaque; virObjectEventPtr event = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuDomainJobInfoPrivatePtr jobInfoPriv; int reason; virObjectLock(vm); @@ -1650,12 +1651,14 @@ qemuProcessHandleMigrationStatus(qemuMonitorPtr mon G_GNUC_UNUSED, qemuMonitorMigrationStatusTypeToString(status)); priv = vm->privateData; + jobInfoPriv = priv->job.current->privateData; + if (priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) { VIR_DEBUG("got MIGRATION event without a migration job"); goto cleanup; } - priv->job.current->stats.mig.status = status; + jobInfoPriv->stats.mig.status = status; virDomainObjBroadcast(vm); if (status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY && @@ -1723,6 +1726,7 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon G_GNUC_UNUSED, { qemuDomainObjPrivatePtr priv; qemuDomainJobPrivatePtr jobPriv; + qemuDomainJobInfoPrivatePtr jobInfoPriv; virObjectLock(vm); @@ -1731,18 +1735,19 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon G_GNUC_UNUSED, priv = vm->privateData; jobPriv = priv->job.privateData; + jobInfoPriv = priv->job.current->privateData; if (priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) { VIR_DEBUG("got DUMP_COMPLETED event without a dump_completed job"); goto cleanup; } jobPriv->dumpCompleted = true; - priv->job.current->stats.dump = *stats; + jobInfoPriv->stats.dump = *stats; priv->job.error = g_strdup(error); /* Force error if extracting the DUMP_COMPLETED status failed */ if (!error && status < 0) { priv->job.error = g_strdup(virGetLastErrorMessage()); - priv->job.current->stats.dump.status = QEMU_MONITOR_DUMP_STATUS_FAILED; + jobInfoPriv->stats.dump.status = QEMU_MONITOR_DUMP_STATUS_FAILED; } virDomainObjBroadcast(vm); -- 2.17.1
participants (2)
-
Michal Privoznik -
Prathamesh Chavan