Re: [PATCH v3 15/17] tools/xenstore: introduce trace classes

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 17/01/2023 09:11, Juergen Gross wrote:
> @@ -2604,6 +2607,8 @@ static void usage(void)
>   "  -N, --no-fork           to request that the daemon does not fork,\n"
>   "  -P, --output-pid        to request that the pid of the daemon is output,\n"
>   "  -T, --trace-file <file> giving the file for logging, and\n"
> +"      --trace-control=+<switch> activate a specific <switch>\n"
> +"      --trace-control=-<switch> deactivate a specific <switch>\n"
>   "  -E, --entry-nb <nb>     limit the number of entries per domain,\n"
>   "  -S, --entry-size <size> limit the size of entry per domain, and\n"
>   "  -W, --watch-nb <nb>     limit the number of watches per domain,\n"
> @@ -2647,6 +2652,7 @@ static struct option options[] = {
>         { "output-pid", 0, NULL, 'P' },
>         { "entry-size", 1, NULL, 'S' },
>         { "trace-file", 1, NULL, 'T' },
> +       { "trace-control", 1, NULL, 1 },
>         { "transaction", 1, NULL, 't' },
>         { "perm-nb", 1, NULL, 'A' },
>         { "path-max", 1, NULL, 'M' },
> @@ -2721,6 +2727,43 @@ static void set_quota(const char *arg, bool soft)
>                 barf("unknown quota \"%s\"\n", arg);
>   }
>   
> +/* Sorted by bit values of TRACE_* flags. Flag is (1u << index). */
> +const char *trace_switches[] = {
AFAICT, this array is not meant to be modified. So you want a second const.

> +       "obj", "io", "wrl",
> +       NULL
> +};
[...]

> diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
> index 3b96ecd018..c85b15515c 100644
> --- a/tools/xenstore/xenstored_core.h
> +++ b/tools/xenstore/xenstored_core.h
> @@ -287,6 +287,12 @@ extern char **orig_argv;
>   
>   extern char *tracefile;
>   extern int tracefd;
> +extern unsigned int trace_flags;
> +#define TRACE_OBJ      0x00000001
> +#define TRACE_IO       0x00000002
> +#define TRACE_WRL      0x00000004
I would add a comment on top to explain that the value should be kept in 
sync with trace_switches.
Cheers,

--
Julien Grall