[PATCH v2 0/2] qemu: domainjob: Allow operations if cb is not set in job structure
This is v2 of: https://listman.redhat.com/archives/libvir-list/2022-March/228905.html Diff to v1: * add checks for cb structure in other functions as well (suggested by Michal) Kristina Hanicova (2): qemu: domainjob: Allow operations if cb is not set in job structure qemu: domainjob: Allow InitJob if cb is not set in qemuDomainObjInitJob() src/qemu/qemu_domainjob.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) -- 2.35.1
We should allow resetting / freeing / restoring / parsing / formatting qemuDomainJobObj even if 'cb' attribute is not set. This is theoretical for now, but the attribute must not be always set in the future. It is sufficient to check if 'cb' exists before dereferencing it. This commit partially reverts af16e754cd4efc3ca1. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_domainjob.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 3e73eba4ed..7b81cd8a2b 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -239,8 +239,10 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObj *job) job->abortJob = false; VIR_FREE(job->error); g_clear_pointer(&job->current, virDomainJobDataFree); - job->cb->resetJobPrivate(job->privateData); job->apiFlags = 0; + + if (job->cb) + job->cb->resetJobPrivate(job->privateData); } int @@ -258,7 +260,8 @@ qemuDomainObjRestoreJob(virDomainObj *obj, job->privateData = g_steal_pointer(&priv->job.privateData); job->apiFlags = priv->job.apiFlags; - if (!(priv->job.privateData = priv->job.cb->allocJobPrivate())) + if (priv->job.cb && + !(priv->job.privateData = priv->job.cb->allocJobPrivate())) return -1; job->cb = priv->job.cb; @@ -270,16 +273,15 @@ qemuDomainObjRestoreJob(virDomainObj *obj, void qemuDomainObjClearJob(qemuDomainJobObj *job) { - if (!job->cb) - return; - qemuDomainObjResetJob(job); qemuDomainObjResetAsyncJob(job); - g_clear_pointer(&job->privateData, job->cb->freeJobPrivate); g_clear_pointer(&job->current, virDomainJobDataFree); g_clear_pointer(&job->completed, virDomainJobDataFree); virCondDestroy(&job->cond); virCondDestroy(&job->asyncCond); + + if (job->cb) + g_clear_pointer(&job->privateData, job->cb->freeJobPrivate); } bool @@ -1228,7 +1230,8 @@ qemuDomainObjPrivateXMLFormatJob(virBuffer *buf, if (priv->job.asyncJob != QEMU_ASYNC_JOB_NONE) virBufferAsprintf(&attrBuf, " flags='0x%lx'", priv->job.apiFlags); - if (priv->job.cb->formatJob(&childBuf, &priv->job, vm) < 0) + if (priv->job.cb && + priv->job.cb->formatJob(&childBuf, &priv->job, vm) < 0) return -1; virXMLFormatElement(buf, "job", &attrBuf, &childBuf); @@ -1288,7 +1291,8 @@ qemuDomainObjPrivateXMLParseJob(virDomainObj *vm, return -1; } - if (priv->job.cb->parseJob(ctxt, job, vm) < 0) + if (priv->job.cb && + priv->job.cb->parseJob(ctxt, job, vm) < 0) return -1; return 0; -- 2.35.1
This allows init job even if cb structure is not set. This patch also includes slight rewriting of the function to make it look cleaner when freeing resources, by allocating privateData at the end. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_domainjob.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 7b81cd8a2b..28c09ae179 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -189,17 +189,18 @@ qemuDomainObjInitJob(qemuDomainJobObj *job, memset(job, 0, sizeof(*job)); job->cb = cb; - if (!(job->privateData = job->cb->allocJobPrivate())) + if (virCondInit(&job->cond) < 0) return -1; - if (virCondInit(&job->cond) < 0) { - job->cb->freeJobPrivate(job->privateData); + if (virCondInit(&job->asyncCond) < 0) { + virCondDestroy(&job->cond); return -1; } - if (virCondInit(&job->asyncCond) < 0) { - job->cb->freeJobPrivate(job->privateData); + if (job->cb && + !(job->privateData = job->cb->allocJobPrivate())) { virCondDestroy(&job->cond); + virCondDestroy(&job->asyncCond); return -1; } -- 2.35.1
On 3/16/22 15:08, Kristina Hanicova wrote:
This is v2 of: https://listman.redhat.com/archives/libvir-list/2022-March/228905.html
Diff to v1: * add checks for cb structure in other functions as well (suggested by Michal)
Kristina Hanicova (2): qemu: domainjob: Allow operations if cb is not set in job structure qemu: domainjob: Allow InitJob if cb is not set in qemuDomainObjInitJob()
src/qemu/qemu_domainjob.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Kristina Hanicova -
Michal Prívozník