Re: [PATCH iproute2 2/2] devlink: add batch command support

[Jiri Pirko <jiri@resnulli.us>]

Fri, Nov 10, 2017 at 08:47:35PM CET, leon@kernel.org wrote:
>On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote:
>> On 10.11.2017 07:57, Leon Romanovsky wrote:
>> > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote:
>> >> The patch adds support to batch devlink commands.
>> >>
>> >> Cc: Jiri Pirko <jiri@mellanox.com>
>> >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com>
>> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> >> ---
>> >>  devlink/devlink.c  | 70 +++++++++++++++++++++++++++++++++++++++++++++++-------
>> >>  man/man8/devlink.8 | 16 +++++++++++++
>> >>  2 files changed, 78 insertions(+), 8 deletions(-)
>> >>
>> >
>> > <..>
>> >
>> >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
>> >> index a480766c..a975ef34 100644
>> >> --- a/man/man8/devlink.8
>> >> +++ b/man/man8/devlink.8
>> >> @@ -12,6 +12,12 @@ devlink \- Devlink tool
>> >>  .sp
>> >>
>> >>  .ti -8
>> >> +.B devlink
>> >> +.RB "[ " -force " ] "
>> >> +.BI "-batch " filename
>> >> +.sp
>> >> +
>> >> +.ti -8
>> >>  .IR OBJECT " := { "
>> >>  .BR dev " | " port " | " monitor " }"
>> >>  .sp
>> >> @@ -32,6 +38,16 @@ Print the version of the
>> >>  utility and exit.
>> >>
>> >>  .TP
>> >> +.BR "\-b", " \-batch " <FILENAME>
>> >> +Read commands from provided file or standard input and invoke them.
>> >> +First failure will cause termination of devlink.
>> >
>> > It is worth to document the expected format of that file.
>> > And IMHO, it is better to have ability to load JSON fie which was
>> > generated by -j, instead of declaring new format/knob.
>> It's just a list of command-lines... like other utils (bridge,ip...)
>
>I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON
>approach, it is more user and script friendly.

Leon, we should really do things in a way they are currently done and
used. Batching is implemented in "ip" for a long time. It makes perfect
sense to have one command line per line of the batch file.

In contrary, json output sounds really odd in this case. With json,
there is no relation to ordinary ip command line params. Or do you want
to extend it to accept json as well?