Re: [PATCH 1/2] rvgen: validate the monitor type

[Gabriele Monaco <gmonaco@redhat.com>]

On Thu, 2026-11-12 at 18:20 +0800, Chaosheng Qu wrote:
> From ef3ed8fae481c0ed3dd16c881aae455be9e92f2e Mon Sep 17 00:00:00 2001
> From: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
> Date: Thu, 12 Nov 2026 14:26:10 +0800
> Subject: [PATCH 1/2] rvgen: validate the monitor type
> 
> The monitor type is pasted into the generated monitor as RV_MON_<TYPE>.
> Nothing checks it, so a typo yields a monitor that cannot build and the
> tool still exits 0:
> 
>     $ rvgen monitor -c da -s test_da.dot -t bogus_type -n bm
>     $ grep RV_MON_TYPE bm/bm.c
>     #define RV_MON_TYPE RV_MON_BOGUS_TYPE
> 
> include/linux/rv.h defines only RV_MON_GLOBAL, RV_MON_PER_CPU,
> RV_MON_PER_TASK and RV_MON_PER_OBJ, so that macro never resolves.
> 
> The accepted set is already in Monitor.monitor_types, which the help text
> prints.  Pass it to argparse as choices.
> 
> Signed-off-by: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
> Assisted-by: LLM

Looks neat, thanks!

Some minor issues on the submission itself. Whenever you send a multi-patch
series make sure the patches are part of the same mail thread, this happens
automatically if you use git workflow (format-patch, send-email) [1].

All tools expect that so it's much easier for everyone to see and apply the
entire series.

You'd normally also add a cover letter, but for a simple series like yours where
1 adds the feature and 2 tests it, I'd say, you can skip it.

Essentially if you want to do it manually, the In-Reply-To of your 2/2 should be
the Message-ID of 1/2 (or of 0/2, the cover letter), but I strongly advise you
to use git tools.

Also, please use consistent commit tags, so both your patches should have
verification/rvgen: instead of whatever you put. If in doubt, use git log --
oneline <modified files> to see.

Finally capitalise after the tag (e.g. uppercase Validate).

TL;DR please send a V2 where both patches are in the same thread and the commit
messages are:
verification/rvgen: Validate the monitor type
verification/rvgen: Cover an invalid monitor type

Thanks,
Gabriele

[1] - https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> ---
>  tools/verification/rvgen/__main__.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/verification/rvgen/__main__.py
> b/tools/verification/rvgen/__main__.py
> index 246b43fa2..6053db5e5 100644
> --- a/tools/verification/rvgen/__main__.py
> +++ b/tools/verification/rvgen/__main__.py
> @@ -38,6 +38,7 @@ if __name__ == '__main__':
>      monitor_parser.add_argument('-s', "--spec", dest="spec", required=True,
>                                  help="Monitor specification file")
>      monitor_parser.add_argument('-t', "--monitor_type", dest="monitor_type",
> required=True,
> +                                choices=list(Monitor.monitor_types.keys()),
>                                  help=f"Available options: {',
> '.join(Monitor.monitor_types.keys())}")
>  
>      container_parser = subparsers.add_parser("container",
> parents=[parent_parser])