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

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

[Demin Han]

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]

Hi,

Many benchmarks have their own build and run system, such as specint, we don’t want to change their code.

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.

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

[-- Attachment #2: Type: text/html, Size: 5707 bytes --]

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

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

[Demin Han]

> -----Original Message-----
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Sent: 2024年11月5日 5:22
> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com
> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
> 
> 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.

I have two methods without change test code, but they can't get args passed to binary:
1. for those without hook, such as specint, we can utilize binfmt_misc. 
  We may need a simple wrapper just to load plugin or set some common options and register this wrapper to binfmt_misc
2. for those with hook, such as llvm-test-suite, we can set TEST_SUITE_RUN_UNDER or utilize binfmt_misc

I have no idea to write a wrapper which can get args passed to binary without change code.
If have, please give a example or some hint.

> > 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.

Yes, this is not important and not most concerned.
But if we can directly output json or yaml, it would be convenient for post-processing.
This is a bonus for this added api.

Regards,
Demin

> 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

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

[Pierrick Bouvier]

On 11/4/24 18:29, Demin Han wrote:
> 
>> -----Original Message-----
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Sent: 2024年11月5日 5:22
>> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
>> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com
>> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
>>
>> 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.
> 
> I have two methods without change test code, but they can't get args passed to binary:
> 1. for those without hook, such as specint, we can utilize binfmt_misc.
>    We may need a simple wrapper just to load plugin or set some common options and register this wrapper to binfmt_misc
> 2. for those with hook, such as llvm-test-suite, we can set TEST_SUITE_RUN_UNDER or utilize binfmt_misc
> 
> I have no idea to write a wrapper which can get args passed to binary without change code.
> If have, please give a example or some hint.
> 

If you use binfmt_misc, how are you passing the argument to use a plugin?

In the case of llvm-test-suite, you can set TEST_SUITE_RUN_UNDER to a 
wrapper adding a specific plugin, its options, and generating the log 
filename as expected.

>>> 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.
> 
> Yes, this is not important and not most concerned.
> But if we can directly output json or yaml, it would be convenient for post-processing.
> This is a bonus for this added api.
> 

It's something you can do directly in the plugin, by outputing 
json/yaml, or any format wanted. It's probably not a feature we'll 
provide in the public API as it's a very specific need.

> Regards,
> Demin
> 
>> 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

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

[Demin Han]

> -----Original Message-----
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Sent: 2024年11月5日 10:50
> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com
> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
> 
> On 11/4/24 18:29, Demin Han wrote:
> >
> >> -----Original Message-----
> >> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> >> Sent: 2024年11月5日 5:22
> >> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
> >> Cc: alex.bennee@linaro.org; erdnaxe@crans.org;
> ma.mandourr@gmail.com
> >> Subject: Re: [PATCH] plugins: add plugin API to get args passed to
> >> binary
> >>
> >> 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.
> >
> > I have two methods without change test code, but they can't get args
> passed to binary:
> > 1. for those without hook, such as specint, we can utilize binfmt_misc.
> >    We may need a simple wrapper just to load plugin or set some common
> > options and register this wrapper to binfmt_misc 2. for those with
> > hook, such as llvm-test-suite, we can set TEST_SUITE_RUN_UNDER or
> > utilize binfmt_misc
> >
> > I have no idea to write a wrapper which can get args passed to binary
> without change code.
> > If have, please give a example or some hint.
> >
> 
> If you use binfmt_misc, how are you passing the argument to use a plugin?
 qemu-riscv64.conf:
	:qemu-riscv64:M::\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xf3\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/opt/bin/qemu-riscv64.sh:
 qemu-riscv64.sh:
	#!/bin/bash
    /opt/bin/qemu-riscv64 --plugin /opt/bin/libxxx.so -L xxx $@
 
 We have no information about binary to pass.

> In the case of llvm-test-suite, you can set TEST_SUITE_RUN_UNDER to a
> wrapper adding a specific plugin, its options, and generating the log filename
> as expected.

We cant set TEST_SUITE_RUN_UNDER="qemu-riscv64 --plugin /xxx/libxxx.so -L xxx".
We have no any information about binary under test even binary name.
We can get binary name in plugin using qemu_plugin_path_to_binary, but can't solve the issue mentioned in commit msg.


> >>> 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.
> >
> > Yes, this is not important and not most concerned.
> > But if we can directly output json or yaml, it would be convenient for
> post-processing.
> > This is a bonus for this added api.
> >
> 
> It's something you can do directly in the plugin, by outputing json/yaml, or any
> format wanted. It's probably not a feature we'll provide in the public API as
> it's a very specific need.
> 
> > Regards,
> > Demin
> >
> >> 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

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

[Pierrick Bouvier]

On 11/4/24 19:31, Demin Han wrote:
> 
> 
>> -----Original Message-----
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Sent: 2024年11月5日 10:50
>> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
>> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com
>> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
>>
>> On 11/4/24 18:29, Demin Han wrote:
>>>
>>>> -----Original Message-----
>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> Sent: 2024年11月5日 5:22
>>>> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
>>>> Cc: alex.bennee@linaro.org; erdnaxe@crans.org;
>> ma.mandourr@gmail.com
>>>> Subject: Re: [PATCH] plugins: add plugin API to get args passed to
>>>> binary
>>>>
>>>> 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.
>>>
>>> I have two methods without change test code, but they can't get args
>> passed to binary:
>>> 1. for those without hook, such as specint, we can utilize binfmt_misc.
>>>     We may need a simple wrapper just to load plugin or set some common
>>> options and register this wrapper to binfmt_misc 2. for those with
>>> hook, such as llvm-test-suite, we can set TEST_SUITE_RUN_UNDER or
>>> utilize binfmt_misc
>>>
>>> I have no idea to write a wrapper which can get args passed to binary
>> without change code.
>>> If have, please give a example or some hint.
>>>
>>
>> If you use binfmt_misc, how are you passing the argument to use a plugin?
>   qemu-riscv64.conf:
> 	:qemu-riscv64:M::\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xf3\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/opt/bin/qemu-riscv64.sh:
>   qemu-riscv64.sh:
> 	#!/bin/bash
>      /opt/bin/qemu-riscv64 --plugin /opt/bin/libxxx.so -L xxx $@
>   
>   We have no information about binary to pass.
> 

It seems that you already have a wrapper script (qemu-risv64.sh). You 
can replace it and read command line from $@, which contains the command 
passed to it.

>> In the case of llvm-test-suite, you can set TEST_SUITE_RUN_UNDER to a
>> wrapper adding a specific plugin, its options, and generating the log filename
>> as expected.
> 
> We cant set TEST_SUITE_RUN_UNDER="qemu-riscv64 --plugin /xxx/libxxx.so -L xxx".
> We have no any information about binary under test even binary name.
> We can get binary name in plugin using qemu_plugin_path_to_binary, but can't solve the issue mentioned in commit msg.
> 

If you pass a script instead, calling the same command, you can get 
access to command line by reading the content of $@.

> 
>>>>> 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.
>>>
>>> Yes, this is not important and not most concerned.
>>> But if we can directly output json or yaml, it would be convenient for
>> post-processing.
>>> This is a bonus for this added api.
>>>
>>
>> It's something you can do directly in the plugin, by outputing json/yaml, or any
>> format wanted. It's probably not a feature we'll provide in the public API as
>> it's a very specific need.
>>
>>> Regards,
>>> Demin
>>>
>>>> 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

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

[Demin Han]

> 
> It seems that you already have a wrapper script (qemu-risv64.sh). You can
> replace it and read command line from $@, which contains the command
> passed to it.

> >> In the case of llvm-test-suite, you can set TEST_SUITE_RUN_UNDER to a
> >> wrapper adding a specific plugin, its options, and generating the log
> >> filename as expected.
> >
> > We cant set TEST_SUITE_RUN_UNDER="qemu-riscv64 --plugin
> /xxx/libxxx.so -L xxx".
> > We have no any information about binary under test even binary name.
> > We can get binary name in plugin using qemu_plugin_path_to_binary, but
> can't solve the issue mentioned in commit msg.
> >
> 
> If you pass a script instead, calling the same command, you can get access to
> command line by reading the content of $@.
> 

Thanks. 
I think this can work.
Add a arg to plugin and pass the parsed $@ to it.

Regards,
Demin

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

[Pierrick Bouvier]

On 11/4/24 19:53, Demin Han wrote:
>>
>> It seems that you already have a wrapper script (qemu-risv64.sh). You can
>> replace it and read command line from $@, which contains the command
>> passed to it.
> 
>>>> In the case of llvm-test-suite, you can set TEST_SUITE_RUN_UNDER to a
>>>> wrapper adding a specific plugin, its options, and generating the log
>>>> filename as expected.
>>>
>>> We cant set TEST_SUITE_RUN_UNDER="qemu-riscv64 --plugin
>> /xxx/libxxx.so -L xxx".
>>> We have no any information about binary under test even binary name.
>>> We can get binary name in plugin using qemu_plugin_path_to_binary, but
>> can't solve the issue mentioned in commit msg.
>>>
>>
>> If you pass a script instead, calling the same command, you can get access to
>> command line by reading the content of $@.
>>
> 
> Thanks.
> I think this can work.
> Add a arg to plugin and pass the parsed $@ to it.
> 

It would be the most flexible, as you can do any pre/post transformation 
on input/output. It should provide exactly what you are looking for, 
instead of doing complex output in C in your plugin.

> Regards,
> Demin

Let us know if you found a satisfying solution.

Regards,
Pierrick

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

[demin.han]

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.

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;
-- 
2.46.1

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

[Pierrick Bouvier]

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

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

[Alex Bennée]

"demin.han" <demin.han@starfivetech.com> writes:

> 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.

New APIs should come with an example use case for testing. However for
this use case why isn't the plugin using getpid() or gettid() a suitable
solution?

Some additional comments bellow...

> 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.

Maybe more explicit:

qemu_plugin_get_user_argv() and be clear it is user mode only.

Although I suspect a qemu_plugin_get_user_env() might be a more useful
helper depending on the use case.

> 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));

 argv = g_new0(char *, argc + 1);

> +    for (i = 0; i < argc; ++i) {
> +        argv[i] = g_strdup(ts->bprm->argv[i]);
> +    }
> +    argv[argc] = NULL;

Allows you to drop this as well.

> +#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;

You can drop the symbols now we autogenerate them.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

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

[Demin Han]

Hi,

> -----Original Message-----
> From: Alex Bennée <alex.bennee@linaro.org>
> Sent: 2025年1月9日 19:59
> To: Demin Han <demin.han@starfivetech.com>
> Cc: qemu-devel@nongnu.org; erdnaxe@crans.org; ma.mandourr@gmail.com;
> pierrick.bouvier@linaro.org
> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
> 
> "demin.han" <demin.han@starfivetech.com> writes:
> 
> > 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.
> 
> New APIs should come with an example use case for testing. However for this
> use case why isn't the plugin using getpid() or gettid() a suitable solution?


We want a meaning and unique name for plugin output. For example
./a arg0 arg1 should have a_arg0_arg1.log
./a arg2 arg3 should have a_arg2_arg3.log
I think getpid() and getid() is not unique

This case can be solved by using a shell wrapper, extract args and pass them to plugin suggested by Pierrick Bouvier

Regards,
Demin