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

[Max Reitz]

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. :-)

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

Max

signature.asc
Description: OpenPGP digital signature