9:08 a.m.
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
9:08 a.m.
New subject: [PATCH v2 1/2] qemu: domainjob: Allow operations if cb is not set in job structure
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(a)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
9:08 a.m.
New subject: [PATCH v2 2/2] qemu: domainjob: Allow InitJob if cb is not set in qemuDomainObjInitJob()
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(a)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
10:28 a.m.
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(a)redhat.com>
Michal
1012
days inactive
1012
days old
3 comments
2 participants
participants (2)
-
Kristina Hanicova -
Michal Prívozník