Re: [PATCH RESEND kvmtool 4/4] Add --loglevel argument for the run command

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Fri, Jun 30, 2023 at 02:31:34PM +0100, Alexandru Elisei wrote:
> Add --loglevel command line argument, with the possible values of 'error',
> 'warning', 'info' or 'debug' to control what messages kvmtool displays. The
> argument functions similarly to the Linux kernel parameter, when lower
> verbosity levels hide all message with a higher verbosity (for example,
> 'warning' hides info and debug messages, allows warning and error
> messsages).
> 
> The default level is 'info', to match the current behaviour. --debug has
> been kept as a legacy option, which might be removed in the future.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  builtin-run.c      | 32 ++++++++++++++++++++++++++++----
>  include/kvm/util.h |  9 +++++++--
>  util/util.c        |  9 +++++++++
>  3 files changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin-run.c b/builtin-run.c
> index 79d031777c26..2e4378819f00 100644
> --- a/builtin-run.c
> +++ b/builtin-run.c
> @@ -58,8 +58,7 @@
>  __thread struct kvm_cpu *current_kvm_cpu;
>  
>  static int  kvm_run_wrapper;
> -
> -bool do_debug_print = false;
> +int loglevel = LOGLEVEL_INFO;
>  
>  static const char * const run_usage[] = {
>  	"lkvm run [<options>] [<kernel image>]",
> @@ -146,6 +145,27 @@ static int mem_parser(const struct option *opt, const char *arg, int unset)
>  	return 0;
>  }
>  
> +static int loglevel_parser(const struct option *opt, const char *arg, int unset)
> +{
> +	if (strcmp(opt->long_name, "debug") == 0) {
> +		loglevel = LOGLEVEL_DEBUG;
> +		return 0;
> +	}
> +
> +	if (strcmp(arg, "debug") == 0)
> +		loglevel = LOGLEVEL_DEBUG;
> +	else if (strcmp(arg, "info") == 0)
> +		loglevel = LOGLEVEL_INFO;
> +	else if (strcmp(arg, "warning") == 0)
> +		loglevel = LOGLEVEL_WARNING;
> +	else if (strcmp(arg, "error") == 0)
> +		loglevel = LOGLEVEL_ERROR;
> +	else
> +		die("Unknown loglevel: %s", arg);
> +
> +	return 0;
> +}
> +
>  #ifndef OPT_ARCH_RUN
>  #define OPT_ARCH_RUN(...)
>  #endif
> @@ -215,6 +235,8 @@ static int mem_parser(const struct option *opt, const char *arg, int unset)
>  		     VIRTIO_TRANS_OPT_HELP_SHORT,		        \
>  		     "Type of virtio transport",			\
>  		     virtio_transport_parser, NULL),			\
> +	OPT_CALLBACK('\0', "loglevel", NULL, "[error|warning|info|debug]",\
> +			"Set the verbosity level", loglevel_parser, NULL),\
>  									\
>  	OPT_GROUP("Kernel options:"),					\
>  	OPT_STRING('k', "kernel", &(cfg)->kernel_filename, "kernel",	\
> @@ -241,8 +263,10 @@ static int mem_parser(const struct option *opt, const char *arg, int unset)
>  		     vfio_device_parser, kvm),				\
>  									\
>  	OPT_GROUP("Debug options:"),					\
> -	OPT_BOOLEAN('\0', "debug", &do_debug_print,			\
> -			"Enable debug messages"),			\
> +	OPT_CALLBACK_NOOPT('\0', "debug", kvm, NULL,			\
> +			"Enable debug messages (deprecated, use "	\
> +			"--loglevel=debug instead)",			\
> +			loglevel_parser, NULL),				\
>  	OPT_BOOLEAN('\0', "debug-single-step", &(cfg)->single_step,	\
>  			"Enable single stepping"),			\
>  	OPT_BOOLEAN('\0', "debug-ioport", &(cfg)->ioport_debug,		\
> diff --git a/include/kvm/util.h b/include/kvm/util.h
> index 6920ce2630ad..e9d63c184752 100644
> --- a/include/kvm/util.h
> +++ b/include/kvm/util.h
> @@ -32,7 +32,10 @@
>  #endif
>  #endif
>  
> -extern bool do_debug_print;
> +#define LOGLEVEL_ERROR		0
> +#define LOGLEVEL_WARNING	1
> +#define LOGLEVEL_INFO		2
> +#define LOGLEVEL_DEBUG		3
>  
>  #define PROT_RW (PROT_READ|PROT_WRITE)
>  #define MAP_ANON_NORESERVE (MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE)
> @@ -45,9 +48,11 @@ extern void pr_info(const char *err, ...) __attribute__((format (printf, 1, 2)))
>  extern void __pr_debug(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
>  
> +extern int loglevel;
> +
>  #define pr_debug(fmt, ...)						\
>  	do {								\
> -		if (do_debug_print)					\
> +		if (loglevel >= LOGLEVEL_DEBUG)				\
>  			__pr_debug("(%s) %s:%d: " fmt, __FILE__,	\
>  				__func__, __LINE__, ##__VA_ARGS__);	\
>  	} while (0)
> diff --git a/util/util.c b/util/util.c
> index e3b36f67f899..962e8d4edb21 100644
> --- a/util/util.c
> +++ b/util/util.c
> @@ -56,6 +56,9 @@ void pr_err(const char *err, ...)
>  {
>  	va_list params;
>  
> +	if (loglevel < LOGLEVEL_ERROR)
> +		return;
> +
>  	va_start(params, err);
>  	error_builtin(err, params);
>  	va_end(params);
> @@ -65,6 +68,9 @@ void pr_warning(const char *warn, ...)
>  {
>  	va_list params;
>  
> +	if (loglevel < LOGLEVEL_WARNING)
> +		return;
> +
>  	va_start(params, warn);
>  	warn_builtin(warn, params);
>  	va_end(params);
> @@ -74,6 +80,9 @@ void pr_info(const char *info, ...)
>  {
>  	va_list params;
>  
> +	if (loglevel < LOGLEVEL_INFO)
> +		return;
> +
>  	va_start(params, info);
>  	info_builtin(info, params);
>  	va_end(params);
> -- 
> 2.41.0
>