On 03.01.24 00:57, Ilya Maximets wrote: > On 11/16/23 11:41, [email protected] wrote: >> From: Jakob Meng <[email protected]> >> >> A previous patch had changed the JSON-RPC API in lib/unixctl.* (and >> its Python counterpart) in order to allow transporting the requested >> output format from ovs-xxx tools to ovs-vswitchd across unix sockets: >> >> The meaning of 'method' and 'params' had be changed: 'method' would be >> 'execute/v1' when an output format other than 'text' is requested. In >> that case, the first parameter of the JSON array 'params' would be the >> designated command, the second one the output format and the rest >> will be command args. >> That change allowed to transport the output format in a backward >> compatible way: One could use an updated client (ovs-appctl) with an >> old server (ovs-vswitchd) and vice versa. Of course, JSON output would >> only work when both sides have been updated. >> >> This patch reverts the JSON-RPC API to the old behaviour: The command >> will be copied to JSON-RPC's 'method' parameter while command args will >> be copied to the 'params' parameter. >> The client will inject the output format into the command args and the >> server will parse and remove it before passing the args to the command >> callback. >> >> The advantage of this (old) approach is a simpler JSON-RPC API but at >> the cost of inferior usability: ovs-xxx tools are no longer able to >> detect missing JSON support at the server side. Old servers will simply >> pass the output format as an arg to the command which would then result >> in an error such as 'command takes at most ... arguments' or a non-\ >> uniform error from the command callback. > Hi, Jakob. > > I see you don't like the option of passing a format argument by filtering > it out. :) > > Both solutions look like nasty workarounds at this point, some more, some > less, but in the end they all are just abusing the system. So, I came > to a conclusion that two possible "correct" solutions are: > > 1. Implement JSON-RPC v2: https://www.jsonrpc.org/specification > This will give us an automatic protocol version verification and > we can pass paramaters as object, with separated server and method > parameters, e.g. > "params": { > "server-params": { "format": "json" }, > "method-params": [ arg1, arg2, ... ] > } > This is not a lightweight option to implement, will require proper > validation, support for both versions of the spec, and potentially > support for features that we do not actually need. > > 2. Just make 2 calls. Add a new internal "unixctl_set_options" method > that will change the options for a current connection. E.g. > { > "method": "unixctl_set_options", > "params": [ "format:json" ], > "id": 1 > } > If that method fails - no json format. If the method succeeds, server > will reply with JSON results for this connections. > The current connection options can be directly stored in the struct > unixctl_conn. Implementation of the method can be just a normal > jsonrpc method registered by the unixctl module itself. > > Perfect compatibility with old servers, they will reject the first > request with a clear error. Old clients will not use this method > and have no changes in workflow at all. Only clients that need > JSON replies will need to use this method. > > Should be much easier to implement. SO, I'd vote for this option. > > Thoughts? > > Best regards, Ilya Maximets. >
Well, I agree with all your points 😄 Making 2 calls is a good idea, will update my patch. Thank you! _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
