[PATCH 6/8] virDriverFeatureIsGlobal: Handle VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER

[Michal Prívozník]

On 2/17/22 12:42, Peter Krempa wrote:
> On Thu, Feb 17, 2022 at 10:56:11 +0100, Michal Prívozník wrote:
>> On 2/17/22 10:45, Peter Krempa wrote:
>>> On Thu, Feb 17, 2022 at 10:39:59 +0100, Michal Prívozník wrote:
>>>> On 2/16/22 16:41, Peter Krempa wrote:
>>>>> The fix was on RPC level so everything should advertise it.
>>>>>
>>>>> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>>>>> ---
>>>>>  src/driver.c | 4 +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> [...]
> 
>> git show -W b0f78d626a18bcecae3a4d165540ab88bfbfc9ee --
>> src/libvirt-network.c
>>
>> You'll see that the public API was given @command and then @section
>> arguments, but the callback was called with @section and @command
>> (reversed). This went unnoticed because both arguments are of the same
>> type (thus RPC serializer didn't report any error) and because the
>> public API is called again on the daemon side (during dispatch) so the
>> order was swapped again and thus network driver saw arguments in correct
>> order. Problem arose only when split daemons were introduced -> suddenly
>> the API was called three times (one time at client, one time at
>> virtqemud and finally in virnetworkd), so there was imbalance of
>> swapping. And yes, if client connected to virtnetworkd directly
>> everything worked, because again - two swapps were done.
>>
>>>
>>>>
>>>> I'm not objecting to this patch, just trying to shed more light into the
>>>> problem.
>>>
>>> I can update the comment. Actually the idea is that the comment captures
>>> the reason for the flag, so it should be acurate.
>>>
>>
>> Yeah, it's just that I'm unable to come with anything better.
> 
> How about:
> 
>     /* Feature flag exposes that the accidental switching of order of
>      * arguments in the public API trampoline virNetworkUpdate is known.
>      * Updated clients thus use the correct ordering with an updated
>      * server. All drivers must signal support for this feature.
>      */
> 

Perfect! Thanks.

Michal