9:56 a.m.
v2 of:
https://www.redhat.com/archives/libvir-list/2019-August/msg01418.html
Patches 1/3 and 2/3 were ACKed but I did not pushed them yet - I'd like
to push these all at once.
diff to v1:
- Instead of restoring seclabels on the backing chain, remove the
metadata as we can't know if somebody else is not using some parts of
the chain.
Michal Prívozník (3):
qemu_blockjob: Move active commit failed state handling into a
function
qemu_blockjob: Print image path on failed security metadata move too
qemu_blockjob: Remove secdriver metadata more frequently
src/qemu/qemu_blockjob.c | 78 ++++++++++++++++++++++++++++++++++++----
1 file changed, 72 insertions(+), 6 deletions(-)
--
2.21.0
9:56 a.m.
New subject: [libvirt] [PATCH for 5.7.0 v2 1/3] qemu_blockjob: Move active commit failed state handling into a function
Currently, there are only a few lines of code so a separate
function was not necessary, but this will change. So instead of
putting all the new code under 'case
QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT' create a separate function.
Just like every other case has one.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
ACKed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 3003e9c518..c77a129bfc 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1121,6 +1121,20 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr
driver,
}
+static void
+qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+ qemuBlockJobDataPtr job)
+{
+ VIR_DEBUG("active commit job '%s' on VM '%s' failed",
job->name, vm->def->name);
+
+ if (!job->disk)
+ return;
+
+ virObjectUnref(job->disk->mirror);
+ job->disk->mirror = NULL;
+}
+
+
static void
qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -1211,10 +1225,7 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr
job,
break;
case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
- if (job->disk) {
- virObjectUnref(job->disk->mirror);
- job->disk->mirror = NULL;
- }
+ qemuBlockJobProcessEventFailedActiveCommit(vm, job);
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
--
2.21.0
9:56 a.m.
New subject: [libvirt] [PATCH for 5.7.0 v2 2/3] qemu_blockjob: Print image path on failed security metadata move too
When a block job is completed, the security image metadata are
moved to the new image. If this fails an warning is printed, but
the message contains only domain name and lacks image paths. Put
them both into the warning message.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
ACKed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index c77a129bfc..1b22689e0c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -646,8 +646,14 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver,
virDomainLockImageDetach(driver->lockManager, vm, disk->src);
/* Move secret driver metadata */
- if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror) <
0)
- VIR_WARN("Unable to move disk metadata on vm %s",
vm->def->name);
+ if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror) <
0) {
+ VIR_WARN("Unable to move disk metadata on "
+ "vm %s from %s to %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ NULLSTR(disk->mirror->path),
+ disk->dst);
+ }
virObjectUnref(disk->src);
disk->src = disk->mirror;
--
2.21.0
9:56 a.m.
New subject: [libvirt] [PATCH for 5.7.0 v2 3/3] qemu_blockjob: Remove secdriver metadata more frequently
If a block job reaches failed/cancelled state, or is completed
without pivot then we must remove security driver metadata
associated to the backing chain so that we don't leave any
metadata behind.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 59 ++++++++++++++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 1b22689e0c..a991309ee7 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -659,7 +659,23 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver,
disk->src = disk->mirror;
} else {
if (disk->mirror) {
+ virStorageSourcePtr n;
+
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+ /* Ideally, we would restore seclabels on the backing chain here
+ * but we don't know if somebody else is not using parts of it.
+ * Remove security driver metadata so that they are not leaked. */
+ for (n = disk->mirror; virStorageSourceIsBacking(n); n =
n->backingStore) {
+ if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+ VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+ }
+ }
+
virObjectUnref(disk->mirror);
}
}
@@ -728,7 +744,23 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
case VIR_DOMAIN_BLOCK_JOB_FAILED:
case VIR_DOMAIN_BLOCK_JOB_CANCELED:
if (disk->mirror) {
+ virStorageSourcePtr n;
+
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+ /* Ideally, we would restore seclabels on the backing chain here
+ * but we don't know if somebody else is not using parts of it.
+ * Remove security driver metadata so that they are not leaked. */
+ for (n = disk->mirror; virStorageSourceIsBacking(n); n =
n->backingStore) {
+ if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+ VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+ }
+ }
+
virObjectUnref(disk->mirror);
disk->mirror = NULL;
}
@@ -1128,16 +1160,33 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr
driver,
static void
-qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
qemuBlockJobDataPtr job)
{
+ virDomainDiskDefPtr disk = job->disk;
+ virStorageSourcePtr n;
+
VIR_DEBUG("active commit job '%s' on VM '%s' failed",
job->name, vm->def->name);
- if (!job->disk)
+ if (!disk)
return;
- virObjectUnref(job->disk->mirror);
- job->disk->mirror = NULL;
+ /* Ideally, we would make the backing chain read only again (yes, SELinux
+ * can do that using different labels). But that is not implemented yet and
+ * not leaking security driver metadata is more important. */
+ for (n = disk->mirror; virStorageSourceIsBacking(n); n = n->backingStore) {
+ if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+ VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+ }
+ }
+
+ virObjectUnref(disk->mirror);
+ disk->mirror = NULL;
}
@@ -1231,7 +1280,7 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr
job,
break;
case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
- qemuBlockJobProcessEventFailedActiveCommit(vm, job);
+ qemuBlockJobProcessEventFailedActiveCommit(driver, vm, job);
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
--
2.21.0
10:18 a.m.
New subject: [libvirt] [PATCH for 5.7.0 v2 3/3] qemu_blockjob: Remove secdriver metadata more frequently
On Fri, Aug 30, 2019 at 16:56:10 +0200, Michal Privoznik wrote:
If a block job reaches failed/cancelled state, or is completed
without pivot then we must remove security driver metadata
associated to the backing chain so that we don't leave any
metadata behind.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 59 ++++++++++++++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 5 deletions(-)
ACK
1912
days inactive
1912
days old
4 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa