2:21 a.m.
Modify the ordering so that the VM definition stays in a state allowing
us to format a valid XML for the virt-aa-helper.
Peter Krempa (2):
qemu: blockjob: Update 'mirror' of a copy job before removing images
qemu: blockjob: Clean out disk mirror data after concluding the job
src/qemu/qemu_blockjob.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
--
2.46.0
2:21 a.m.
New subject: [PATCH 1/2] qemu: blockjob: Update 'mirror' of a copy job before removing images
When concluding a job with a 'mirror' we first unplugged the appropriate
no-longer used images from qemu and then updated the definition.
Normally this wouldn't be a problem because for any other thread this is
done under the VM lock thus atomic. Unfortunately though, the AppArmor
security backend is using a VM XML to pass data to the helper process
and the state of the definition at that point was unsuitable to format a
valid XML thus making 'virt-aa-helper' report parsing failure.
Since we're removing the images the proper state of the VM definition
indeed should not include the mirror element any more at the point when
the images are removed.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/601
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 42856df6d4..9c56d5cb95 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1218,6 +1218,8 @@ qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriver *driver,
virDomainAsyncJob asyncJob)
{
qemuDomainObjPrivate *priv = vm->privateData;
+ g_autoptr(virStorageSource) src = NULL;
+
VIR_DEBUG("copy job '%s' on VM '%s' pivoted", job->name,
vm->def->name);
/* mirror may be NULL for copy job corresponding to migration */
@@ -1241,9 +1243,11 @@ qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriver *driver,
qemuBlockJobRewriteConfigDiskSource(vm, job->disk, job->disk->mirror);
- qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob,
job->disk->src);
- virObjectUnref(job->disk->src);
+ src = g_steal_pointer(&job->disk->src);
+
job->disk->src = g_steal_pointer(&job->disk->mirror);
+
+ qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, src);
}
@@ -1254,6 +1258,7 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriver *driver,
virDomainAsyncJob asyncJob)
{
qemuDomainObjPrivate *priv = vm->privateData;
+ g_autoptr(virStorageSource) mirror = NULL;
VIR_DEBUG("copy job '%s' on VM '%s' aborted", job->name,
vm->def->name);
@@ -1277,9 +1282,10 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriver *driver,
g_clear_pointer(&job->disk->mirror->backingStore,
virObjectUnref);
}
+ mirror = g_steal_pointer(&job->disk->mirror);
+
/* activeWrite bitmap is removed automatically here */
- qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob,
job->disk->mirror);
- g_clear_pointer(&job->disk->mirror, virObjectUnref);
+ qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, mirror);
}
--
2.46.0
2:21 a.m.
New subject: [PATCH 2/2] qemu: blockjob: Clean out disk mirror data after concluding the job
The 'disk->mirrorJob' and 'disk->mirrorState' fields need to be
cleared
after a blockjob, but should be kept around while 'disk->mirror' is
still in place. As 'disk->mirror' is cleared only after conclusion of
the job in 'qemuBlockJobEventProcessConcluded()' we should be resetting
them only afterwards.
Move the code later, but since the job is unregistered from the disk we
need to store the pointer to the disk before concluding the job.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 9c56d5cb95..652b25540a 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1561,12 +1561,18 @@ qemuBlockJobEventProcess(virQEMUDriver *driver,
case QEMU_BLOCKJOB_STATE_COMPLETED:
case QEMU_BLOCKJOB_STATE_FAILED:
case QEMU_BLOCKJOB_STATE_CANCELLED:
- case QEMU_BLOCKJOB_STATE_CONCLUDED:
- if (job->disk) {
- job->disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
- job->disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
- }
+ case QEMU_BLOCKJOB_STATE_CONCLUDED: {
+ virDomainDiskDef *disk = job->disk;
+
qemuBlockJobEventProcessConcluded(job, driver, vm, asyncJob);
+
+ /* Job was unregistered from the disk but we must ensure that the
+ * data is cleared */
+ if (disk) {
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
+ }
+ }
break;
case QEMU_BLOCKJOB_STATE_READY:
--
2.46.0
3:11 a.m.
New subject: [PATCH 0/2] qemu: blockjob: Fix ordering of operations when
concluding a block copy
On Fri, Sep 27, 2024 at 09:21:22AM +0200, Peter Krempa wrote:
Modify the ordering so that the VM definition stays in a state
allowing
us to format a valid XML for the virt-aa-helper.
Peter Krempa (2):
qemu: blockjob: Update 'mirror' of a copy job before removing images
qemu: blockjob: Clean out disk mirror data after concluding the job
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
52
days inactive
56
days old
3 comments
2 participants
participants (2)
-
Pavel Hrdina -
Peter Krempa