On Thu, Nov 21, 2019 at 14:02:49 +0100, Michal Privoznik wrote: > On 11/20/19 3:22 PM, Peter Krempa wrote: > > > > Okay, how about this commit message: > > qemuProcessStop: Remove image metadata for running mirror jobs > > If user starts a blockcommit without --pivot or a blockcopy also > without --pivot then we modify access for qemu on both images anda We modify the access perms even if you use --pivot. > leave it like that until pivot is executed. So far so good. Problem > is, if user instead of issuing pivot calls destroy on the domain or > if qemu dies whilst executing the block job. In this case we don't > ever clear the access we granted at the beginning. To fix this, > maybe a bit harsh approach is used, but it works: after all labels > were restored (that is after qemuSecurityRestoreAllLabel() was > called), we iterate over each disk in the domain and remove XATTRs > from the whole backing chain and also from any file the disk is being > mirrored to. > > This would have been done at the time of pivot, but it isn't because > user decided to kill the domain instead. If we don't do this and > leave some XATTRs behind the domain might be unable to start. > > Also, secdriver can't do this because it doesn't know if there is any > job running. It's outside of its scope - the hypervisor driver is > responsible for calling secdriver's APIs. So you correctly explain that this fixes everything up after a block job and you also explain why the secdrivers can't do that. What's missing is explanation why it's okay to call even if no blockjob was ever active on the VM and that this does not have implications e.g. on backing chains shared between two VMs (even if none of them ran a blockjob ever). With the above case explained I'll be willing to ACK the patch. > > https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19 > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > > > > > Michal
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
