[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 22/33] block: Make backing files child_of_bds children
From: |
Max Reitz |
Subject: |
Re: [PATCH v3 22/33] block: Make backing files child_of_bds children |
Date: |
Thu, 7 May 2020 11:28:50 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
On 06.05.20 18:37, Kevin Wolf wrote:
> Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
>> Make all parents of backing files pass the appropriate BdrvChildRole.
>> By doing so, we can switch their BdrvChildClass over to the generic
>> child_of_bds, which will do the right thing when given a correct
>> BdrvChildRole.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> ---
>> block.c | 26 ++++++++++++++++++++------
>> block/backup-top.c | 2 +-
>> block/vvfat.c | 3 ++-
>> tests/test-bdrv-drain.c | 13 +++++++------
>> 4 files changed, 30 insertions(+), 14 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 43df38ca30..31affcb4ee 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2770,6 +2770,20 @@ static bool
>> bdrv_inherits_from_recursive(BlockDriverState *child,
>> return child != NULL;
>> }
>>
>> +/*
>> + * Return the BdrvChildRole for @bs's backing child. bs->backing is
>> + * mostly used for COW backing children (role = COW), but also for
>> + * filtered children (role = FILTERED | PRIMARY).
>> + */
>> +static BdrvChildRole bdrv_backing_role(BlockDriverState *bs)
>> +{
>> + if (bs->drv && bs->drv->is_filter) {
>> + return BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY;
>> + } else {
>> + return BDRV_CHILD_COW;
>> + }
>> +}
>
> bdrv_mirror_top and bdrv_commit_top don't set .is_filter, so it will
> return the wrong role for them. (They also don't set .supports_backing,
> so grepping for that wouldn't help...)
I’ll pull in “block: Mark commit and mirror as filter drivers” from the
main series then.
>> /*
>> * Sets the backing file link of a BDS. A new reference is created; callers
>> * which don't need their own reference any more must call bdrv_unref().
>> @@ -2797,8 +2811,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
>> BlockDriverState *backing_hd,
>> goto out;
>> }
>>
>> - bs->backing = bdrv_attach_child(bs, backing_hd, "backing",
>> &child_backing,
>> - 0, errp);
>> + bs->backing = bdrv_attach_child(bs, backing_hd, "backing",
>> &child_of_bds,
>> + bdrv_backing_role(bs), errp);
>> /* If backing_hd was already part of bs's backing chain, and
>> * inherits_from pointed recursively to bs then let's update it to
>> * point directly to bs (else it will become NULL). */
>> @@ -2895,7 +2909,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
>> *parent_options,
>> }
>>
>> backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0,
>> bs,
>> - &child_backing, 0, errp);
>> + &child_of_bds, BDRV_CHILD_COW, errp);
>
> Wouldn't it be more consistent to use bdrv_backing_role() here, too?
It’d be indeed.
Max
signature.asc
Description: OpenPGP digital signature