[libvirt] [PATCH v2 2/2] qemuProcessStop: Remove image metadata for running mirror jobs

[Peter Krempa]

On Tue, Nov 19, 2019 at 14:15:12 +0100, Michal Privoznik wrote:
> If user starts a blockcommit without --pivot or a blockcopy also
> without --pivot then we modify access for qemu on both images and
> 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, we need to mimic what block job code does for
> aborting a block job -> remove image metadata from disk->mirror
> and disk->src chains.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> Diff to v2:
> - updated commit message
> - do more remove - for disk->src chain too
> - put a comment about the importance of code placement
> 
>  src/qemu/qemu_process.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 209d07cfe8..b9dd433f54 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7615,6 +7615,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>      for (i = 0; i < vm->def->niothreadids; i++)
>          vm->def->iothreadids[i]->thread_id = 0;
>  
> +    /* Do this explicitly after vm->pid is reset so that security drivers don't
> +     * try to enter the domain's namespace which is non-existent by now as qemu
> +     * is no longer running. */
> +    for (i = 0; i < def->ndisks; i++) {
> +        virDomainDiskDefPtr disk = def->disks[i];
> +
> +        if (disk->mirror)
> +            qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->mirror);
> +
> +        qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src);
> +    }

So aren't we tracking the image metadata also for the shared subsets of
backing chain which might be used by multiple domains? Because this
undoes everything and we do have some code which does this in the
security driver, don't we? So this either is duplicate or it's a
superset of what is done in the security driver.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191120/26bdfc3b/attachment-0001.sig>