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 >>>> Reviewed-by: Eric Blake >>>> Reviewed-by: Alberto Garcia >>>> --- >>>>   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