Re: [PATCH 6/8] virDriverFeatureIsGlobal: Handle VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER

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(a)redhat.com&gt; >>>> --- >>>> 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. */