Re: [PATCH 01/13] plugins: allow plugin arguments to be passed directly

["Alex Bennée" <alex.bennee@linaro.org>]

Mahmoud Mandour <ma.mandourr@gmail.com> writes:

> Passing arguments to plugins had to be done through "arg=<argname>".
> This is redundant and introduces confusion especially when the argument
> has a name and value (e.g. `-plugin plugin_name,arg="argname=argvalue"`).
>
> This allows passing plugin arguments directly e.g:
>
>     `-plugin plugin_name,argname=argvalue`
>
> For now, passing arguments through "arg=" is still supports but outputs
> a deprecation warning.
>
> Also, this commit makes boolean arguments passed to plugins in the
> `argname=on|off` form instead of the deprecated short-boolean form.
>
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>

Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  linux-user/main.c |  2 +-
>  plugins/loader.c  | 24 ++++++++++++++++++++----
>  qemu-options.hx   |  9 ++++-----
>  3 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 4dfc47ad3b..d47f78132c 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -462,7 +462,7 @@ static const struct qemu_argument arg_table[] = {
>       "",           "[[enable=]<pattern>][,events=<file>][,file=<file>]"},
>  #ifdef CONFIG_PLUGIN
>      {"plugin",     "QEMU_PLUGIN",      true,  handle_arg_plugin,
> -     "",           "[file=]<file>[,arg=<string>]"},
> +     "",           "[file=]<file>[,<argname>=<argvalue>]"},
>  #endif
>      {"version",    "QEMU_VERSION",     false, handle_arg_version,
>       "",           "display version information and exit"},
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 05df40398d..a4ec281692 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -94,6 +94,8 @@ static int plugin_add(void *opaque, const char *name, const char *value,
>  {
>      struct qemu_plugin_parse_arg *arg = opaque;
>      struct qemu_plugin_desc *p;
> +    bool is_on;
> +    char *fullarg;
>  
>      if (strcmp(name, "file") == 0) {
>          if (strcmp(value, "") == 0) {
> @@ -107,18 +109,32 @@ static int plugin_add(void *opaque, const char *name, const char *value,
>              QTAILQ_INSERT_TAIL(arg->head, p, entry);
>          }
>          arg->curr = p;
> -    } else if (strcmp(name, "arg") == 0) {
> +    } else {
>          if (arg->curr == NULL) {
>              error_setg(errp, "missing earlier '-plugin file=' option");
>              return 1;
>          }
> +
> +        if (g_strcmp0(name, "arg") == 0 &&
> +                !qapi_bool_parse(name, value, &is_on, NULL)) {
> +            if (strchr(value, '=') == NULL) {
> +                /* Will treat arg="argname" as "argname=on" */
> +                fullarg = g_strdup_printf("%s=%s", value, "on");
> +            } else {
> +                fullarg = g_strdup_printf("%s", value);
> +            }
> +            warn_report("using 'arg=%s' is deprecated", value);
> +            error_printf("Please use '%s' directly\n", fullarg);
> +        } else {
> +            fullarg = g_strdup_printf("%s=%s", name, value);
> +        }
> +
>          p = arg->curr;
>          p->argc++;
>          p->argv = g_realloc_n(p->argv, p->argc, sizeof(char *));
> -        p->argv[p->argc - 1] = g_strdup(value);
> -    } else {
> -        error_setg(errp, "-plugin: unexpected parameter '%s'; ignored", name);
> +        p->argv[p->argc - 1] = fullarg;
>      }
> +
>      return 0;
>  }
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 14258784b3..36b6cb9a2f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4459,19 +4459,18 @@ SRST
>  
>  ERST
>  DEF("plugin", HAS_ARG, QEMU_OPTION_plugin,
> -    "-plugin [file=]<file>[,arg=<string>]\n"
> +    "-plugin [file=]<file>[,<argname>=<argvalue>]\n"
>      "                load a plugin\n",
>      QEMU_ARCH_ALL)
>  SRST
> -``-plugin file=file[,arg=string]``
> +``-plugin file=file[,argname=argvalue]``
>      Load a plugin.
>  
>      ``file=file``
>          Load the given plugin from a shared library file.
>  
> -    ``arg=string``
> -        Argument string passed to the plugin. (Can be given multiple
> -        times.)
> +    ``argname=argvalue``
> +        Argument passed to the plugin. (Can be given multiple times.)
>  ERST
>  
>  HXCOMM Internal use


-- 
Alex Bennée