On 14.11.18 20:52, Max Reitz wrote:
> On 12.11.18 23:17, Eric Blake wrote:
>> On 8/9/18 5:31 PM, Max Reitz wrote:

[...]

>>> +++ b/block/io.c
>>> @@ -120,6 +120,7 @@ static void bdrv_merge_limits(BlockLimits *dst,
>>> const BlockLimits *src)
>>>   void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>>>   {
>>>       BlockDriver *drv = bs->drv;
>>> +    BlockDriverState *cow_bs = bdrv_filtered_cow_bs(bs);
>>>       Error *local_err = NULL;
>>>         memset(&bs->bl, 0, sizeof(bs->bl));
>>> @@ -148,13 +149,13 @@ void bdrv_refresh_limits(BlockDriverState *bs,
>>> Error **errp)
>>>           bs->bl.max_iov = IOV_MAX;
>>>       }
>>>   -    if (bs->backing) {
>>> -        bdrv_refresh_limits(bs->backing->bs, &local_err);
>>> +    if (cow_bs) {
>>> +        bdrv_refresh_limits(cow_bs, &local_err);
>>>           if (local_err) {
>>>               error_propagate(errp, local_err);
>>>               return;
>>>           }
>>> -        bdrv_merge_limits(&bs->bl, &bs->backing->bs->bl);
>>> +        bdrv_merge_limits(&bs->bl, &cow_bs->bl);
>>
>> Is this doing the right things with blkdebug?
> 
> First, blkdebug doesn't have a COW child, does it?
> 
> Second, we still always invoke the driver's implementation (if there is
> one).  All of the code at the beginning of the function just chooses
> some defaults.  So blkdebug can still override everything.
> 
> But there is indeed something wrong here.  And that is: What is with R/W
> filter drivers that use bs->backing?  After this patch, they won't get
> any defaults.

Hm, yeah, but after the next one they're alright because
bdrv_storage_bs() returns filtered children.  So the issue is just the
span in between...

I suppose I can solve this by assigning bs->file to storage_bs, or
bs->backing if both bs->file and cow_bs are NULL.  And then put a FIXME
behind it that the next patch will solve.

Max