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

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

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