Re: [Qemu-devel] [PATCH 05/14] block: Set "share-rw" flag in drive-backup when sync=none

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On 31.10.2016 16:38, Fam Zheng wrote:
> In this case we may open the source's backing image chain multiple
> times. Setting share flag means the new open won't try to acquire or
> check any lock, once we implement image locking.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> 
> An alternative is reusing (and bdrv_ref) the existing source's backing
> bs instead of opening another one. If we decide that approach is better,
> it's better to do it in a separate series.

Yes, it is better (there's a reason why we do it for drive-mirror), and
while I somehow agree that we could put it off until later, I'm not sure
we should. Opening an image with both BDRV_O_RDWR and BDRV_O_SHARE_RW at
the same time just because our implementation is lacking is not ideal.

Anyway, the whole issue becomes more complex when involving format
drivers. I'll write more to that in a response to the cover letter.

> ---
>  blockdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index d11a74f..9992c5d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3160,6 +3160,7 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
>      }
>      if (backup->sync == MIRROR_SYNC_MODE_NONE) {
>          source = bs;
> +        flags |= BDRV_O_SHARE_RW;

In any case, there should be a comment explaining the situation here
(having to go through git blame is a bit tedious...), possibly involving
a TODO or FIXME regarding that we really shouldn't be using
BDRV_O_SHARE_RW (or maybe we should? I'm not sure, I'll explore it in
said cover letter response).

Max

>      }
>  
>      size = bdrv_getlength(bs);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]