[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 32/33] block: Pass BdrvChildRole in remaining cases

From: Max Reitz
Subject: Re: [PATCH v2 32/33] block: Pass BdrvChildRole in remaining cases
Date: Tue, 18 Feb 2020 13:01:54 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 11.02.20 16:53, Eric Blake wrote:
> On 2/4/20 11:08 AM, Max Reitz wrote:
>> These calls have no real use for the child role yet, but it will not
>> harm to give one.
>> Notably, the bdrv_root_attach_child() call in blockjob.c is left
>> unmodified because there is not much the generic BlockJob object wants
>> from its children.
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>   block/block-backend.c | 11 +++++++----
>>   block/vvfat.c         |  2 +-
>>   2 files changed, 8 insertions(+), 5 deletions(-)
> Reviewed-by: Eric Blake <address@hidden>
> Is it worth an assert(role) somewhere now that you've converted all
> callers to pass at least one role?

Well, as the commit message states, block_job_add_bdrv() in blockjob.c
still passes BdrvChildRole=0 to bdrv_root_attach_child().  So it depends
on what function we’re looking at.

I suppose we could add such an assertion to bdrv_attach_child() because
we could expect all BDSs to pass some role for their children.

OTOH, maybe a BDS has a legitimate reason not to: Maybe it just wants to
take some permissions on some BDS without having any real relationship
to it.  Right now, some block jobs do that, well, except they do so
through the back door of adding the child BDS to the block job object
(which then passes no child role).  So maybe I’d actually rather not add
such an assertion anywhere.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]