On Mon, Sep 16, 2019 at 09:52:28 +0200, Michal Privoznik wrote: > On 9/16/19 9:40 AM, Peter Krempa wrote: > > On Mon, Sep 16, 2019 at 08:49:37 +0200, Michal Privoznik wrote: > > > When doing a blockcommit, the base and possibly top parent are > > > relabelled in qemuDomainBlockCommit(). However, once the block > > > job is finished, we need to remove the secdriver metadata > > > (XATTRs) created at the beginning. > > > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456 > > > > > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > > > --- > > > src/qemu/qemu_blockjob.c | 19 +++++++++++++++++++ > > > 1 file changed, 19 insertions(+) > > > > > > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c > > > index a991309ee7..47715f12f6 100644 > > > --- a/src/qemu/qemu_blockjob.c > > > +++ b/src/qemu/qemu_blockjob.c > > > @@ -677,6 +677,25 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver, > > > } > > > virObjectUnref(disk->mirror); > > > + } else if (job->type == QEMU_BLOCKJOB_TYPE_COMMIT) { > > > + if (qemuSecurityMoveImageMetadata(driver, vm, > > > + job->data.commit.base, NULL) < 0) { > > > + VIR_WARN("Unable to remove disk metadata on " > > > + "vm %s from %s (disk target %s)", > > > + vm->def->name, > > > + NULLSTR(job->data.commit.base->path), > > > + disk->dst); > > > + } > > > + if (job->data.commit.topparent && > > > + job->data.commit.topparent != disk->src && > > > + qemuSecurityMoveImageMetadata(driver, vm, > > > + job->data.commit.topparent, NULL) < 0) { > > > + VIR_WARN("Unable to remove disk metadata on " > > > + "vm %s from %s (disk target %s)", > > > + vm->def->name, > > > + NULLSTR(job->data.commit.topparent->path), > > > + disk->dst); > > > + } > > > > NACK, the legacy block job handler was never designed to touch job->data > > in any way. I don't want to guarantee that the blockjob code bits work > > with the legacy code bits in any way. > > > > Specifically the problem is that any of this data will NOT be available > > after libvirtd restart because all of the job tracking in the status XML > > is only done when QEMU_CAPS_BLOCKDEV is enabled. > > > > So how do you suggest I fix this? Steps to reproduce are here: > > https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c8 Historically the legacy block job handler leaked all permissions it granted to any image during finishing of the block job. In this case since an action must be taken toI suggest discarding all metadata on the backing chain of disk->src. That data is valid because it is refreshed from the disk on VM startup. While I'd prefer if the legacy handler is not touched any more, removing all xattrs here is an acceptable fix. Obviously the second solution is to ensure that the image metadata is handled only when libvirt knows how to manage the backing chain, thus enabling it only if QEMU_CAPS_BLOCKDEV is enabled.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
