Re: [Qemu-block] [PATCH v4 5/6] block/backup: prohibit backup from using in use bitmaps

[Eric Blake]

On 10/2/18 6:02 PM, John Snow wrote:
> If the bitmap is frozen, we shouldn't touch it.
>
> Signed-off-by: John Snow <address@hidden>
> ---
>   blockdev.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index d0febfca79..098d4c337f 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3512,10 +3512,10 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
> JobTxn *txn,
>               bdrv_unref(target_bs);
>               goto out;
>           }
> -        if (bdrv_dirty_bitmap_qmp_locked(bmap)) {
> +        if (bdrv_dirty_bitmap_user_locked(bmap)) {
>               error_setg(errp,
> -                       "Bitmap '%s' is currently locked and cannot be used for 
> "
> -                       "backup", backup->bitmap);
> +                       "Bitmap '%s' is currently in use by another operation"
> +                       " and cannot be used for backup", backup->bitmap);
>               goto out;

Is this right?  Why can we not have two parallel backups utilizing the 
same unchanging locked bitmap as its source?  Of course, to do that, 
we'd need the condition of being locked to be a ref-counter (increment 
for each backup that reuses the bitmap, decrement when the backup 
finishes, and it is unlocked when the counter is 0) rather than a bool. 
So, without that larger refactoring, this is a conservative approach 
that is a little too strict, but allows for a simpler implementation. 
And the user can always work around the limitation by cloning the locked 
bitmap into another temporary bitmap, and starting the second parallel 
backup with the second backup instead of the original.

Weak Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org