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@xxxxxxxxxx> >>>>> --- >>>>> 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
