Re: [PATCH for-5.0 02/31] block: Add BdrvChildRole

[Vladimir Sementsov-Ogievskiy]

28.11.2019 13:52, Max Reitz wrote:
> On 28.11.19 10:31, Vladimir Sementsov-Ogievskiy wrote:
>> 27.11.2019 16:15, Max Reitz wrote:
>>> This enum will supplement BdrvChildClass when it comes to what role (or
>>> combination of roles) a child takes for its parent.
>>>
>>> Because empty enums are not allowed, let us just start with it filled.
>>>
>>> Signed-off-by: Max Reitz <address@hidden>
>>> ---
>>>    include/block/block.h | 38 ++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 38 insertions(+)
>>>
>>> diff --git a/include/block/block.h b/include/block/block.h
>>> index 38963ef203..36817d5689 100644
>>> --- a/include/block/block.h
>>> +++ b/include/block/block.h
>>> @@ -279,6 +279,44 @@ enum {
>>>        DEFAULT_PERM_UNCHANGED      = BLK_PERM_ALL & 
>>> ~DEFAULT_PERM_PASSTHROUGH,
>>>    };
>>>    
>>> +typedef enum BdrvChildRole {
>>
>> Don't you want to call it just BdrvChildFlags ?
>> Benefits:
>>
>> 1. Do not intersect with old BdrvChildRole.
> 
> Well, that doesn’t change the fact that the old BdrvChildRole just
> doesn’t describe a role.
> 
>> 2. I think, BDRV_CHILD_STAY_AT_NODE is not a role, but just a property or 
>> flag..
> 
> I can be convinced to let STAY_AT_NODE stay a property of
> BdrvChildClass. :-)

or BdrvChild if we want it to be property of object, not class.

> 
> What I don’t like about “BdrvChildFlags” is that “flags” doesn’t express
> anything.  The permissions are flags, too.
> 
> Max
> 


-- 
Best regards,
Vladimir