On 01/30/2018 06:15 AM, Anton Nefedov wrote:

>>>> @@ -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;

>>> 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()

Then is mirror_start_job() a better location, right after we call
bdrv_new_open_driver()?  (Maybe this just goes to show I haven't fully
traced the lifecycle of the mirror driver, and it may all be changing
anyways as we try to fix the BDS graph modifications related with mirrors).

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