[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 12/47] block: Use bdrv_filter_(bs|child) where obvious
From: |
Max Reitz |
Subject: |
Re: [PATCH v7 12/47] block: Use bdrv_filter_(bs|child) where obvious |
Date: |
Thu, 9 Jul 2020 10:59:13 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
On 08.07.20 20:24, Andrey Shinkevich wrote:
> On 25.06.2020 18:21, Max Reitz wrote:
>> Places that use patterns like
>>
>> if (bs->drv->is_filter && bs->file) {
>> ... something about bs->file->bs ...
>> }
>>
>> should be
>>
>> BlockDriverState *filtered = bdrv_filter_bs(bs);
>> if (filtered) {
>> ... something about @filtered ...
>> }
>>
>> instead.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block.c | 31 ++++++++++++++++++++-----------
>> block/io.c | 7 +++++--
>> migration/block-dirty-bitmap.c | 8 +-------
>> 3 files changed, 26 insertions(+), 20 deletions(-)
>>
> ...
>> diff --git a/block/io.c b/block/io.c
>> index df8f2a98d4..385176b331 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -3307,6 +3307,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild
>> *child, int64_t offset, bool exact,
>> Error **errp)
>> {
>> BlockDriverState *bs = child->bs;
>> + BdrvChild *filtered;
>> BlockDriver *drv = bs->drv;
>> BdrvTrackedRequest req;
>> int64_t old_size, new_bytes;
>> @@ -3358,6 +3359,8 @@ int coroutine_fn bdrv_co_truncate(BdrvChild
>> *child, int64_t offset, bool exact,
>> goto out;
>> }
>> + filtered = bdrv_filter_child(bs);
>> +
>
> Isn't better to have this initialization right before the relevant
> if/else block?
Hm, well, yes. In this case, though, maybe not. Patch 16 will add
another BdrvChild to be initialized here (@backing), and we need to
initialize that one here. So I felt it made sense to group them together.
They got split up when I decided to put @filtered into this patch and
@backing into its own. So now it may look a bit weird, but I feel like
after patch 16 it makes sense.
(I’m indifferent, basically.)
Max
signature.asc
Description: OpenPGP digital signature