[PATCH 6/8] virDriverFeatureIsGlobal: Handle VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER

[Peter Krempa]

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.
     */