Re: [Qemu-devel] [PATCH v7 1/9] mirror: inherit supported write/zero flags

[Max Reitz]

On 2018-01-30 13:15, Anton Nefedov wrote:
> 
> 
> On 29/1/2018 10:26 PM, Eric Blake wrote:
>> On 01/29/2018 01:21 PM, Max Reitz wrote:
>>> On 2018-01-18 18:48, Anton Nefedov wrote:
>>>> Signed-off-by: Anton Nefedov <address@hidden>
>>>> Reviewed-by: Eric Blake <address@hidden>
>>>> Reviewed-by: Alberto Garcia <address@hidden>
>>>> ---
>>>>   block/mirror.c | 5 +++++
>>>>   1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/block/mirror.c b/block/mirror.c
>>>> index c9badc1..d18ec65 100644
>>>> --- a/block/mirror.c
>>>> +++ b/block/mirror.c
>>>> @@ -1064,6 +1064,11 @@ static void
>>>> bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts)
>>>>       bdrv_refresh_filename(bs->backing->bs);
>>>>       pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
>>>>               bs->backing->bs->filename);
>>>> +    bs->supported_write_flags = BDRV_REQ_FUA &
>>>> +        bs->backing->bs->supported_write_flags;
>>>> +    bs->supported_zero_flags =
>>>> +        (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
>>>> +        bs->backing->bs->supported_zero_flags;
>>>>   }
>>>>     static void bdrv_mirror_top_close(BlockDriverState *bs)
>>>
>>> Fundamentally OK, but why is this in *_refresh_filename()?
>>
>> Indeed, I missed that (or maybe it got moved during a botched rebase?).
>> For comparison, blkdebug sets it during blkdebug_open(), and nbd sets it
>> during nbd_client_init() (called during nbd_open()).
>>
> 
> We need a backing bs here and I believe it's not generally set at the
> time of .bdrv_open().

Well, but *_refresh_filename() is just wrong.

This driver doesn't have .bdrv_open() at all, and we create it fully
manually in mirror_start_job() anyway (as Eric has said).  Therefore, we
can just do this right after bdrv_append() there has succeeded.

Max

signature.asc
Description: OpenPGP digital signature