Re: [PATCH] plugins: add plugin API to get args passed to binary

[Pierrick Bouvier]

On 11/1/24 22:10, Demin Han wrote:
> Hi,
>
> Many benchmarks have their own build and run system, such as specint, we 
> don’t want to change their code.

I don't think those benchmarks (such as specint) integrate calling qemu 
with a specific plugin on command line, so I guess you have a wrapper or 
something where you could pass necessary information, or tweak output 
file, without changing the benchmark itself. In case I'm wrong, feel 
free to correct me.

> Actually the log maybe structural data such as in json format and may be 
> output multiple log files with different statistics dimention for one run.
>
> -D can’t satisfy this.

Indeed, it can output only a single file. If your plugin needs something 
more advanced, you can try to output something yourself. However, a 
better and simpler way would be to prefix lines output with a specific 
marker, and post process your plugin trace with a custom script.

Adding command line access to plugins does not solve any of those problems.

I see value in what this series offer, but I don't see how it's related 
to the current need you express.

Regards,
Pierrick

> Regard,
> Denin
>
> 获取 Outlook for iOS <https://aka.ms/o0ukef>
> ------------------------------------------------------------------------
> *发件人:* Pierrick Bouvier <pierrick.bouvier@linaro.org>
> *发送时间:* Saturday, November 2, 2024 2:18:20 AM
> *收件人:* Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org 
> <qemu-devel@nongnu.org>
> *抄送:* alex.bennee@linaro.org <alex.bennee@linaro.org>; 
> erdnaxe@crans.org <erdnaxe@crans.org>; ma.mandourr@gmail.com 
> <ma.mandourr@gmail.com>
> *主题:* Re: [PATCH] plugins: add plugin API to get args passed to binary
> Hi Demin,
>
> thanks for your contribution.
>
> On 11/1/24 02:00, demin.han wrote:
> > Why we need args?
> > When plugin outputs log files, only binary path can't distinguish multiple
> > runs if the binary passed with different args.
> > This is bad for CI using plugin.
>
> Can it be solved simply by encoding this in name of log file from the CI
> run script?
> $ cmd="/usr/bin/echo Hello world"
> $ out_file="$(echo "$cmd" | sed -e 's/\s/_/').log"
> $ qemu -plugin... -d plugin -D "$out_file" $cmd
>
> I can see some good points to add this new API, but for the use case
> presented in commit message, I'm not sure to see what it solves.
>
> > Signed-off-by: demin.han <demin.han@starfivetech.com>
> > ---
> >    include/qemu/qemu-plugin.h   | 11 +++++++++++
> >    plugins/api.c                | 16 ++++++++++++++++
> >    plugins/qemu-plugins.symbols |  1 +
> >    3 files changed, 28 insertions(+)
> >
> > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> > index 622c9a0232..daf75c9f5a 100644
> > --- a/include/qemu/qemu-plugin.h
> > +++ b/include/qemu/qemu-plugin.h
> > @@ -837,6 +837,17 @@ bool qemu_plugin_bool_parse(const char *name, const char 
> > *val, bool *ret);
> >    QEMU_PLUGIN_API
> >    const char *qemu_plugin_path_to_binary(void);
> >   
> > +/**
> > + * qemu_plugin_argv_to_binary() - argv to binary file being executed
> > + *
> > + * Return a string array representing the argv to the binary. For user-mode
> > + * this is the main executable's argv. For system emulation we currently
> > + * return NULL. The user should g_free() the string array once no longer
> > + * needed.
> > + */
> > +QEMU_PLUGIN_API
> > +const char **qemu_plugin_argv_to_binary(void);
> > +
> >    /**
> >     * qemu_plugin_start_code() - returns start of text segment
> >     *
> > diff --git a/plugins/api.c b/plugins/api.c
> > index 24ea64e2de..fa2735db03 100644
> > --- a/plugins/api.c
> > +++ b/plugins/api.c
> > @@ -485,6 +485,22 @@ const char *qemu_plugin_path_to_binary(void)
> >        return path;
> >    }
> >   
> > +const char **qemu_plugin_argv_to_binary(void)
> > +{
> > +    const char **argv = NULL;
> > +#ifdef CONFIG_USER_ONLY
> > +    int i, argc;
> > +    TaskState *ts = get_task_state(current_cpu);
> > +    argc = ts->bprm->argc;
> > +    argv = g_malloc(sizeof(char *) * (argc + 1));
> > +    for (i = 0; i < argc; ++i) {
> > +        argv[i] = g_strdup(ts->bprm->argv[i]);
> > +    }
> > +    argv[argc] = NULL;
> > +#endif
> > +    return argv;
> > +}
> > +
> >    uint64_t qemu_plugin_start_code(void)
> >    {
> >        uint64_t start = 0;
> > diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> > index 032661f9ea..532582effe 100644
> > --- a/plugins/qemu-plugins.symbols
> > +++ b/plugins/qemu-plugins.symbols
> > @@ -1,4 +1,5 @@
> >    {
> > +  qemu_plugin_argv_to_binary;
> >      qemu_plugin_bool_parse;
> >      qemu_plugin_end_code;
> >      qemu_plugin_entry_code;
>
> Regards,
> Pierrick