Re: [Qemu-devel] [PATCH 30/32] qmp: Clean up capability negotiation after commit 02130314d8c

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 07/02/2018 11:22 AM, Markus Armbruster wrote:
>> qmp_greeting() offers capabilities to the client, and
>> qmp_qmp_capabilities() accepts or denies capabilities requested by the
>> client.  The two compute the set of available capabilities
>> independently.  Not nice.
>>
>> Clean this up as follows.  Compute available capabilities just once in
>> monitor_qmp_caps_reset(), and store them in Monitor member
>> qmp.capab_offered[].  Have qmp_greeting() and qmp_qmp_capabilities()
>> use that.  Both are now oblivious of capability details.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>   monitor.c | 93 ++++++++++++++++++++++++-------------------------------
>>   1 file changed, 40 insertions(+), 53 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 876a3a23a7..590e5b5b04 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -172,7 +172,8 @@ typedef struct {
>>        * mode.
>>        */
>>       QmpCommandList *commands;
>> -    bool qmp_caps[QMP_CAPABILITY__MAX];
>> +    bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */
>> +    bool capab[QMP_CAPABILITY__MAX];         /* offered and accepted */
>
> Could also store as uint32_t, using '1U << bit' to compute various
> capabilities as a bitmask (up to 32 capabilities).  But it's not
> frequently executed, so this works.

Yes.  I chose to lessen the churn.  On a green field, I guess I'd go
with bits out of habit.  Might not even be a good habit :)

> Reviewed-by: Eric Blake <address@hidden>

Thanks!