Re: [PATCH v2 07/22] verification/dot2k: Replace is_container() hack with subparsers

[Gabriele Monaco]

On Fri, 2025-04-11 at 09:37 +0200, Nam Cao wrote:
> dot2k is used for both generating deterministic automaton (DA)
> monitor and
> generating container monitor.
> 
> Generating DA monitor and generating container requires different
> parameters. This is implemented by peeking at sys.argv and check
> whether
> "--container" is specified, and use that information to make some
> parameters optional or required.
> 
> This works, but is quite hacky and ugly.
> 
> Replace this hack with Python's built-in subparsers.
> 

Yeah, that's much neater, thanks!

Reviewed-by: Gabriele Monaco <gmonaco@xxxxxxxxxx>

> The old commands:
> 
>   python3 dot2/dot2k -d wip.dot -t per_cpu
>   python3 dot2/dot2k -n sched --container
> 
> are equivalent to the new commands:
> 
>   python3 dot2/dot2k monitor -d wip.dot -t per_cpu
>   python3 dot2/dot2k container -n sched
> 
> Signed-off-by: Nam Cao <namcao@xxxxxxxxxxxxx>
> ---
>  tools/verification/dot2/dot2/dot2k.py |  2 +-
>  tools/verification/dot2/dot2k         | 37 +++++++++++++++----------
> --
>  2 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/verification/dot2/dot2/dot2k.py
> b/tools/verification/dot2/dot2/dot2k.py
> index 0922754454b9..9ec99e297012 100644
> --- a/tools/verification/dot2/dot2/dot2k.py
> +++ b/tools/verification/dot2/dot2/dot2k.py
> @@ -19,7 +19,7 @@ class dot2k(Dot2c):
>      monitor_type = "per_cpu"
>  
>      def __init__(self, file_path, MonitorType, extra_params={}):
> -        self.container = extra_params.get("container")
> +        self.container = extra_params.get("subcmd") == "container"
>          self.parent = extra_params.get("parent")
>          self.__fill_rv_templates_dir()
>  
> diff --git a/tools/verification/dot2/dot2k
> b/tools/verification/dot2/dot2k
> index 767064f415e7..133fb17d9d47 100644
> --- a/tools/verification/dot2/dot2k
> +++ b/tools/verification/dot2/dot2k
> @@ -13,30 +13,33 @@ if __name__ == '__main__':
>      import argparse
>      import sys
>  
> -    def is_container():
> -        """Should work even before parsing the arguments"""
> -        return "-c" in sys.argv or "--container" in sys.argv
> -
>      parser = argparse.ArgumentParser(description='transform .dot
> file into kernel rv monitor')
> -    parser.add_argument('-d', "--dot", dest="dot_file", required=not
> is_container())
> -    parser.add_argument('-t', "--monitor_type", dest="monitor_type",
> required=not is_container(),
> -                        help=f"Available options: {',
> '.join(dot2k.monitor_types.keys())}")
> -    parser.add_argument('-n', "--model_name", dest="model_name",
> required=is_container())
>      parser.add_argument("-D", "--description", dest="description",
> required=False)
>      parser.add_argument("-a", "--auto_patch", dest="auto_patch",
>                          action="store_true", required=False,
>                          help="Patch the kernel in place")
> -    parser.add_argument("-p", "--parent", dest="parent",
> -                        required=False, help="Create a monitor
> nested to parent")
> -    parser.add_argument("-c", "--container", dest="container",
> -                        action="store_true", required=False,
> -                        help="Create an empty monitor to be used as
> a container")
> +
> +    subparsers = parser.add_subparsers(dest="subcmd", required=True)
> +
> +    monitor_parser = subparsers.add_parser("monitor")
> +    monitor_parser.add_argument('-n', "--model_name",
> dest="model_name")
> +    monitor_parser.add_argument("-p", "--parent", dest="parent",
> +                                required=False, help="Create a
> monitor nested to parent")
> +    monitor_parser.add_argument('-d', "--dot", dest="dot_file")
> +    monitor_parser.add_argument('-t', "--monitor_type",
> dest="monitor_type",
> +                                help=f"Available options: {',
> '.join(dot2k.monitor_types.keys())}")
> +
> +    container_parser = subparsers.add_parser("container")
> +    container_parser.add_argument('-n', "--model_name",
> dest="model_name", required=True)
> +
>      params = parser.parse_args()
>  
> -    if not is_container():
> -        print("Opening and parsing the dot file %s" %
> params.dot_file)
>      try:
> -        monitor=dot2k(params.dot_file, params.monitor_type,
> vars(params))
> +        if params.subcmd == "monitor":
> +            print("Opening and parsing the dot file %s" %
> params.dot_file)
> +            monitor = dot2k(params.dot_file, params.monitor_type,
> vars(params))
> +        else:
> +            monitor = dot2k(None, None, vars(params))
>      except Exception as e:
>          print('Error: '+ str(e))
>          print("Sorry : :-(")
> @@ -45,7 +48,7 @@ if __name__ == '__main__':
>      print("Writing the monitor into the directory %s" %
> monitor.name)
>      monitor.print_files()
>      print("Almost done, checklist")
> -    if not is_container():
> +    if params.subcmd == "monitor":
>          print("  - Edit the %s/%s.c to add the instrumentation" %
> (monitor.name, monitor.name))
>          print(monitor.fill_tracepoint_tooltip())
>      print(monitor.fill_makefile_tooltip())