* [PATCH 01/13] plugins: allow plugin arguments to be passed directly
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 13:31 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 02/13] plugins/api: added a boolean parsing plugin api Mahmoud Mandour
` (12 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée, Laurent Vivier
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>
---
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
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 01/13] plugins: allow plugin arguments to be passed directly
2021-07-17 10:09 ` [PATCH 01/13] plugins: allow plugin arguments to be passed directly Mahmoud Mandour
@ 2021-07-19 13:31 ` Alex Bennée
0 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2021-07-19 13:31 UTC (permalink / raw)
To: Mahmoud Mandour; +Cc: cota, qemu-devel, Laurent Vivier
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 02/13] plugins/api: added a boolean parsing plugin api
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
2021-07-17 10:09 ` [PATCH 01/13] plugins: allow plugin arguments to be passed directly Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:11 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 03/13] plugins/hotpages: introduce sortby arg and parsed bool args correctly Mahmoud Mandour
` (11 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
This call will help boolean argument parsing since arguments are now
passed to plugins as a name and value.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
include/qemu/qemu-plugin.h | 13 +++++++++++++
plugins/api.c | 5 +++++
2 files changed, 18 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index dc3496f36c..7d0b23c659 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -564,4 +564,17 @@ int qemu_plugin_n_max_vcpus(void);
*/
void qemu_plugin_outs(const char *string);
+/**
+ * qemu_plugin_bool_parse() - parses a boolean argument in the form of
+ * "<argname>=[on|yes|true|off|no|false]"
+ *
+ * @name: argument name, the part before the equals sign
+ * @val: argument value, what's after the equals sign
+ * @ret: output return value
+ *
+ * returns true if the combination @name=@val parses correctly to a boolean
+ * argument, and false otherwise
+ */
+bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
+
#endif /* QEMU_PLUGIN_API_H */
diff --git a/plugins/api.c b/plugins/api.c
index 332e2c60e2..43e239f377 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -383,3 +383,8 @@ void qemu_plugin_outs(const char *string)
{
qemu_log_mask(CPU_LOG_PLUGIN, "%s", string);
}
+
+bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
+{
+ return qapi_bool_parse(name, value, ret, NULL);
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 02/13] plugins/api: added a boolean parsing plugin api
2021-07-17 10:09 ` [PATCH 02/13] plugins/api: added a boolean parsing plugin api Mahmoud Mandour
@ 2021-07-19 14:11 ` Alex Bennée
0 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2021-07-19 14:11 UTC (permalink / raw)
To: Mahmoud Mandour; +Cc: cota, qemu-devel
Mahmoud Mandour <ma.mandourr@gmail.com> writes:
> This call will help boolean argument parsing since arguments are now
> passed to plugins as a name and value.
>
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> ---
> include/qemu/qemu-plugin.h | 13 +++++++++++++
> plugins/api.c | 5 +++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index dc3496f36c..7d0b23c659 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -564,4 +564,17 @@ int qemu_plugin_n_max_vcpus(void);
> */
> void qemu_plugin_outs(const char *string);
>
> +/**
> + * qemu_plugin_bool_parse() - parses a boolean argument in the form of
> + * "<argname>=[on|yes|true|off|no|false]"
> + *
> + * @name: argument name, the part before the equals sign
> + * @val: argument value, what's after the equals sign
> + * @ret: output return value
> + *
> + * returns true if the combination @name=@val parses correctly to a boolean
> + * argument, and false otherwise
> + */
> +bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
> +
> #endif /* QEMU_PLUGIN_API_H */
> diff --git a/plugins/api.c b/plugins/api.c
> index 332e2c60e2..43e239f377 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -383,3 +383,8 @@ void qemu_plugin_outs(const char *string)
> {
> qemu_log_mask(CPU_LOG_PLUGIN, "%s", string);
> }
> +
> +bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
> +{
> + return qapi_bool_parse(name, value, ret, NULL);
> +}
I'm not sure we want to have such a naive pass through of
qapi_bool_parse here. For one thing I think all the current call sites
guarantee value will be set to something. I think that's still the case
for args we supply to plugin_init because of our pre-processing but the
plugin user could at any point decide to parse a random string pair with
it.
We should check for name/value being non-NULL before the pass through I
think.
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 03/13] plugins/hotpages: introduce sortby arg and parsed bool args correctly
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
2021-07-17 10:09 ` [PATCH 01/13] plugins: allow plugin arguments to be passed directly Mahmoud Mandour
2021-07-17 10:09 ` [PATCH 02/13] plugins/api: added a boolean parsing plugin api Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:23 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 04/13] plugins/hotblocks: Added correct boolean argument parsing Mahmoud Mandour
` (10 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Since plugin arguments now expect boolean arguments, a plugin argument
name "sortby" now expects a value of "read", "write", or "address".
"io" arg is now expected to be passed as a full-form boolean parameter,
i.e. "io=on|true|yes|off|false|no"
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
contrib/plugins/hotpages.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/contrib/plugins/hotpages.c b/contrib/plugins/hotpages.c
index bf53267532..0d12910af6 100644
--- a/contrib/plugins/hotpages.c
+++ b/contrib/plugins/hotpages.c
@@ -169,16 +169,26 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- if (g_strcmp0(opt, "reads") == 0) {
- sort_by = SORT_R;
- } else if (g_strcmp0(opt, "writes") == 0) {
- sort_by = SORT_W;
- } else if (g_strcmp0(opt, "address") == 0) {
- sort_by = SORT_A;
- } else if (g_strcmp0(opt, "io") == 0) {
- track_io = true;
- } else if (g_str_has_prefix(opt, "pagesize=")) {
- page_size = g_ascii_strtoull(opt + 9, NULL, 10);
+ g_autofree char **tokens = g_strsplit(opt, "=", -1);
+
+ if (g_strcmp0(tokens[0], "sortby") == 0) {
+ if (g_strcmp0(tokens[1], "reads") == 0) {
+ sort_by = SORT_R;
+ } else if (g_strcmp0(tokens[1], "writes") == 0) {
+ sort_by = SORT_W;
+ } else if (g_strcmp0(tokens[1], "address") == 0) {
+ sort_by = SORT_A;
+ } else {
+ fprintf(stderr, "invalid value to sortby: %s\n", tokens[1]);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "io") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &track_io)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "pagesize") == 0) {
+ page_size = g_ascii_strtoull(tokens[1], NULL, 10);
} else {
fprintf(stderr, "option parsing failed: %s\n", opt);
return -1;
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/13] plugins/hotblocks: Added correct boolean argument parsing
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (2 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 03/13] plugins/hotpages: introduce sortby arg and parsed bool args correctly Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:24 ` Alex Bennée
2021-07-19 14:26 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 05/13] plugins/lockstep: make socket path not positional & parse bool arg Mahmoud Mandour
` (9 subsequent siblings)
13 siblings, 2 replies; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
contrib/plugins/hotblocks.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 4b08340143..062200a7a4 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -133,8 +133,18 @@ QEMU_PLUGIN_EXPORT
int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
int argc, char **argv)
{
- if (argc && strcmp(argv[0], "inline") == 0) {
- do_inline = true;
+ for (int i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ if (g_strcmp0(tokens[0], "inline") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else {
+ fprintf(stderr, "option parsing failed: %s\n", opt);
+ return -1;
+ }
}
plugin_init();
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 05/13] plugins/lockstep: make socket path not positional & parse bool arg
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (3 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 04/13] plugins/hotblocks: Added correct boolean argument parsing Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:25 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 06/13] plugins/hwprofile: adapt to the new plugin arguments scheme Mahmoud Mandour
` (8 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
contrib/plugins/lockstep.c | 31 ++++++++++++++++++++++---------
docs/devel/tcg-plugins.rst | 2 +-
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 7fd35eb669..a41ffe83fa 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -319,22 +319,35 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
int argc, char **argv)
{
int i;
-
- if (!argc || !argv[0]) {
- qemu_plugin_outs("Need a socket path to talk to other instance.");
- return -1;
- }
+ g_autofree char *sock_path = NULL;
for (i = 0; i < argc; i++) {
char *p = argv[i];
- if (strcmp(p, "verbose") == 0) {
- verbose = true;
- } else if (!setup_unix_socket(argv[0])) {
- qemu_plugin_outs("Failed to setup socket for communications.");
+ g_autofree char **tokens = g_strsplit(p, "=", 2);
+
+ if (g_strcmp0(tokens[0], "verbose") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &verbose)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", p);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "sockpath") == 0) {
+ sock_path = tokens[1];
+ } else {
+ fprintf(stderr, "option parsing failed: %s\n", p);
return -1;
}
}
+ if (sock_path == NULL) {
+ fprintf(stderr, "Need a socket path to talk to other instance.\n");
+ return -1;
+ }
+
+ if (!setup_unix_socket(sock_path)) {
+ fprintf(stderr, "Failed to setup socket for communications.\n");
+ return -1;
+ }
+
our_id = id;
qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 370c11373f..6ddf9c28c0 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -270,7 +270,7 @@ communicate over::
./sparc-softmmu/qemu-system-sparc -monitor none -parallel none \
-net none -M SS-20 -m 256 -kernel day11/zImage.elf \
- -plugin ./contrib/plugins/liblockstep.so,arg=lockstep-sparc.sock \
+ -plugin ./contrib/plugins/liblockstep.so,sockpath=lockstep-sparc.sock \
-d plugin,nochain
which will eventually report::
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 06/13] plugins/hwprofile: adapt to the new plugin arguments scheme
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (4 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 05/13] plugins/lockstep: make socket path not positional & parse bool arg Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-17 10:09 ` [PATCH 07/13] plugins/howvec: Adapting to the new argument passing scheme Mahmoud Mandour
` (7 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Parsing boolean arguments correctly (e.g. pattern=on or source=false).
Introduced a new "track" argument that takes a [read|write] value. This
substitutes passing read or write to "arg=" that is deprecated.
Also, matches are now taken one by one through the "match" argument.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
contrib/plugins/hwprofile.c | 39 +++++++++++++++++++++++++------------
docs/devel/tcg-plugins.rst | 8 ++++----
2 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/contrib/plugins/hwprofile.c b/contrib/plugins/hwprofile.c
index faf216ac00..691d4edb0c 100644
--- a/contrib/plugins/hwprofile.c
+++ b/contrib/plugins/hwprofile.c
@@ -259,27 +259,42 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
int argc, char **argv)
{
int i;
+ g_autoptr(GString) matches_raw = g_string_new("");
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- if (g_strcmp0(opt, "read") == 0) {
- rw = QEMU_PLUGIN_MEM_R;
- } else if (g_strcmp0(opt, "write") == 0) {
- rw = QEMU_PLUGIN_MEM_W;
- } else if (g_strcmp0(opt, "pattern") == 0) {
- pattern = true;
- } else if (g_strcmp0(opt, "source") == 0) {
- source = true;
- } else if (g_str_has_prefix(opt, "match")) {
- gchar **parts = g_strsplit(opt, "=", 2);
+ g_autofree char **tokens = g_strsplit(opt, "=", 2);
+
+ if (g_strcmp0(tokens[0], "track") == 0) {
+ if (g_strcmp0(tokens[1], "read") == 0) {
+ rw = QEMU_PLUGIN_MEM_R;
+ } else if (g_strcmp0(tokens[1], "write") == 0) {
+ rw = QEMU_PLUGIN_MEM_W;
+ } else {
+ fprintf(stderr, "invalid value for track: %s\n", tokens[1]);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "pattern") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &pattern)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "source") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &source)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "match") == 0) {
check_match = true;
- matches = g_strsplit(parts[1], ",", -1);
- g_strfreev(parts);
+ g_string_append_printf(matches_raw, "%s,", tokens[1]);
} else {
fprintf(stderr, "option parsing failed: %s\n", opt);
return -1;
}
}
+ if (check_match) {
+ matches = g_strsplit(matches_raw->str, ",", -1);
+ }
if (source && pattern) {
fprintf(stderr, "can only currently track either source or pattern.\n");
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 6ddf9c28c0..753f56ac42 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -290,22 +290,22 @@ which will eventually report::
The hwprofile tool can only be used with system emulation and allows
the user to see what hardware is accessed how often. It has a number of options:
- * arg=read or arg=write
+ * track=read or track=write
By default the plugin tracks both reads and writes. You can use one
of these options to limit the tracking to just one class of accesses.
- * arg=source
+ * source
Will include a detailed break down of what the guest PC that made the
- access was. Not compatible with arg=pattern. Example output::
+ access was. Not compatible with the pattern option. Example output::
cirrus-low-memory @ 0xfffffd00000a0000
pc:fffffc0000005cdc, 1, 256
pc:fffffc0000005ce8, 1, 256
pc:fffffc0000005cec, 1, 256
- * arg=pattern
+ * pattern
Instead break down the accesses based on the offset into the HW
region. This can be useful for seeing the most used registers of a
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/13] plugins/howvec: Adapting to the new argument passing scheme.
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (5 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 06/13] plugins/hwprofile: adapt to the new plugin arguments scheme Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:57 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 08/13] docs/tcg-plugins: new passing parameters scheme for cache docs Mahmoud Mandour
` (6 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Correctly parsing plugin argument since they now must be provided as
full-form boolean parameters, e.g.:
-plugin ./contrib/plugins/libhowvec.so,verbose=on,inline=on
Also, introduced the argument "count" that accepts one opt to count
individually at a time.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
contrib/plugins/howvec.c | 27 +++++++++++++++++++--------
docs/devel/tcg-plugins.rst | 10 +++++-----
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c
index 600f7facc1..4a5ec3d936 100644
--- a/contrib/plugins/howvec.c
+++ b/contrib/plugins/howvec.c
@@ -333,23 +333,34 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (i = 0; i < argc; i++) {
char *p = argv[i];
- if (strcmp(p, "inline") == 0) {
- do_inline = true;
- } else if (strcmp(p, "verbose") == 0) {
- verbose = true;
- } else {
+ g_autofree char **tokens = g_strsplit(p, "=", -1);
+ if (g_strcmp0(tokens[0], "inline") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", p);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "verbose") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &verbose)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", p);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "count") == 0) {
+ char *value = tokens[1];
int j;
CountType type = COUNT_INDIVIDUAL;
- if (*p == '!') {
+ if (*value == '!') {
type = COUNT_NONE;
- p++;
+ value++;
}
for (j = 0; j < class_table_sz; j++) {
- if (strcmp(p, class_table[j].opt) == 0) {
+ if (strcmp(value, class_table[j].opt) == 0) {
class_table[j].what = type;
break;
}
}
+ } else {
+ fprintf(stderr, "option parsing failed: %s\n", p);
+ return -1;
}
}
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 753f56ac42..4ab9dc4bb1 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -79,7 +79,7 @@ Once built a program can be run with multiple plugins loaded each with
their own arguments::
$QEMU $OTHER_QEMU_ARGS \
- -plugin tests/plugin/libhowvec.so,arg=inline,arg=hint \
+ -plugin tests/plugin/libhowvec.so,inline=on,count=hint \
-plugin tests/plugin/libhotblocks.so
Arguments are plugin specific and can be used to modify their
@@ -196,13 +196,13 @@ Similar to hotblocks but this time tracks memory accesses::
This is an instruction classifier so can be used to count different
types of instructions. It has a number of options to refine which get
-counted. You can give an argument for a class of instructions to break
-it down fully, so for example to see all the system registers
-accesses::
+counted. You can give a value to the `count` argument for a class of
+instructions to break it down fully, so for example to see all the system
+registers accesses::
./aarch64-softmmu/qemu-system-aarch64 $(QEMU_ARGS) \
-append "root=/dev/sda2 systemd.unit=benchmark.service" \
- -smp 4 -plugin ./contrib/plugins/libhowvec.so,arg=sreg -d plugin
+ -smp 4 -plugin ./contrib/plugins/libhowvec.so,count=sreg -d plugin
which will lead to a sorted list after the class breakdown::
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 07/13] plugins/howvec: Adapting to the new argument passing scheme.
2021-07-17 10:09 ` [PATCH 07/13] plugins/howvec: Adapting to the new argument passing scheme Mahmoud Mandour
@ 2021-07-19 14:57 ` Alex Bennée
0 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2021-07-19 14:57 UTC (permalink / raw)
To: Mahmoud Mandour; +Cc: cota, qemu-devel
Mahmoud Mandour <ma.mandourr@gmail.com> writes:
> Correctly parsing plugin argument since they now must be provided as
> full-form boolean parameters, e.g.:
> -plugin ./contrib/plugins/libhowvec.so,verbose=on,inline=on
>
> Also, introduced the argument "count" that accepts one opt to count
> individually at a time.
>
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 08/13] docs/tcg-plugins: new passing parameters scheme for cache docs
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (6 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 07/13] plugins/howvec: Adapting to the new argument passing scheme Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:58 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 09/13] tests/plugins/bb: adapt to the new arg passing scheme Mahmoud Mandour
` (5 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
docs/devel/tcg-plugins.rst | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 4ab9dc4bb1..be1256d50c 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -349,34 +349,34 @@ will report the following::
The plugin has a number of arguments, all of them are optional:
- * arg="limit=N"
+ * limit=N
Print top N icache and dcache thrashing instructions along with their
address, number of misses, and its disassembly. (default: 32)
- * arg="icachesize=N"
- * arg="iblksize=B"
- * arg="iassoc=A"
+ * icachesize=N
+ * iblksize=B
+ * iassoc=A
Instruction cache configuration arguments. They specify the cache size, block
size, and associativity of the instruction cache, respectively.
(default: N = 16384, B = 64, A = 8)
- * arg="dcachesize=N"
- * arg="dblksize=B"
- * arg="dassoc=A"
+ * dcachesize=N
+ * dblksize=B
+ * dassoc=A
Data cache configuration arguments. They specify the cache size, block size,
and associativity of the data cache, respectively.
(default: N = 16384, B = 64, A = 8)
- * arg="evict=POLICY"
+ * evict=POLICY
Sets the eviction policy to POLICY. Available policies are: :code:`lru`,
:code:`fifo`, and :code:`rand`. The plugin will use the specified policy for
both instruction and data caches. (default: POLICY = :code:`lru`)
- * arg="cores=N"
+ * cores=N
Sets the number of cores for which we maintain separate icache and dcache.
(default: for linux-user, N = 1, for full system emulation: N = cores
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/13] tests/plugins/bb: adapt to the new arg passing scheme
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (7 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 08/13] docs/tcg-plugins: new passing parameters scheme for cache docs Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:58 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 10/13] tests/plugins/insn: made arg inline not positional and parse it as bool Mahmoud Mandour
` (4 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
tests/plugin/bb.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index de09bdde4e..7d470a1011 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -104,10 +104,17 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- if (g_strcmp0(opt, "inline") == 0) {
- do_inline = true;
- } else if (g_strcmp0(opt, "idle") == 0) {
- idle_report = true;
+ g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ if (g_strcmp0(tokens[0], "inline") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "idle") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &idle_report)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
} else {
fprintf(stderr, "option parsing failed: %s\n", opt);
return -1;
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/13] tests/plugins/insn: made arg inline not positional and parse it as bool
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (8 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 09/13] tests/plugins/bb: adapt to the new arg passing scheme Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 14:59 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 11/13] tests/plugins/mem: introduce "track" arg and make args not positional Mahmoud Mandour
` (3 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Made argument "inline" not positional, this has two benefits. First is
that we adhere to how QEMU passes args generally, by taking the last
value of an argument and drop the others. And the second is that this
sets up a framework for potentially adding new args easily.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
tests/plugin/insn.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index c253980ec8..0f6a1938c1 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -62,8 +62,18 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
int argc, char **argv)
{
- if (argc && !strcmp(argv[0], "inline")) {
- do_inline = true;
+ for (int i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ if (g_strcmp0(tokens[0], "inline") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else {
+ fprintf(stderr, "option parsing failed: %s\n", opt);
+ return -1;
+ }
}
qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/13] tests/plugins/mem: introduce "track" arg and make args not positional
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (9 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 10/13] tests/plugins/insn: made arg inline not positional and parse it as bool Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 15:31 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 12/13] tests/plugins/syscalls: adhere to new arg-passing scheme Mahmoud Mandour
` (2 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
This commit makes the plugin adhere to the new plugins arg-passing
scheme by expecting full-form boolean args instead of short-form
booleans. This necessitates that we introduce a new argument, here
"track", to accept "r", "w", or "rw".
Also, it makes arguments not positional and we only care about the last
value specified for a certain argument.
callback/inline args are now supplied separately as bool arguments so
that both can be enabled individually.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
tests/plugin/mem.c | 47 ++++++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index afd1d27e5c..3000f847b5 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -80,29 +80,40 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
int argc, char **argv)
{
- if (argc) {
- if (argc >= 3) {
- if (!strcmp(argv[2], "haddr")) {
- do_haddr = true;
- }
- }
- if (argc >= 2) {
- const char *str = argv[1];
- if (!strcmp(str, "r")) {
+ for (int i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_autofree char **tokens = g_strsplit(opt, "=", 2);
+
+ if (g_strcmp0(tokens[0], "hadrr") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_haddr)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "track") == 0) {
+ if (g_strcmp0(tokens[1], "r") == 0) {
rw = QEMU_PLUGIN_MEM_R;
- } else if (!strcmp(str, "w")) {
+ } else if (g_strcmp0(tokens[1], "w") == 0) {
rw = QEMU_PLUGIN_MEM_W;
+ } else if (g_strcmp0(tokens[1], "rw") == 0) {
+ rw = QEMU_PLUGIN_MEM_RW;
+ } else {
+ fprintf(stderr, "invaild value for argument track: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "inline") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "callback") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_callback)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
}
- }
- if (!strcmp(argv[0], "inline")) {
- do_inline = true;
- do_callback = false;
- } else if (!strcmp(argv[0], "both")) {
- do_inline = true;
- do_callback = true;
} else {
- do_callback = true;
+ fprintf(stderr, "option parsing failed: %s\n", opt);
+ return -1;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 11/13] tests/plugins/mem: introduce "track" arg and make args not positional
2021-07-17 10:09 ` [PATCH 11/13] tests/plugins/mem: introduce "track" arg and make args not positional Mahmoud Mandour
@ 2021-07-19 15:31 ` Alex Bennée
0 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2021-07-19 15:31 UTC (permalink / raw)
To: Mahmoud Mandour; +Cc: cota, qemu-devel
Mahmoud Mandour <ma.mandourr@gmail.com> writes:
> This commit makes the plugin adhere to the new plugins arg-passing
> scheme by expecting full-form boolean args instead of short-form
> booleans. This necessitates that we introduce a new argument, here
> "track", to accept "r", "w", or "rw".
>
> Also, it makes arguments not positional and we only care about the last
> value specified for a certain argument.
>
> callback/inline args are now supplied separately as bool arguments so
> that both can be enabled individually.
>
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> ---
> tests/plugin/mem.c | 47 ++++++++++++++++++++++++++++------------------
> 1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
> index afd1d27e5c..3000f847b5 100644
> --- a/tests/plugin/mem.c
> +++ b/tests/plugin/mem.c
> @@ -80,29 +80,40 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> const qemu_info_t *info,
> int argc, char **argv)
> {
> - if (argc) {
> - if (argc >= 3) {
> - if (!strcmp(argv[2], "haddr")) {
> - do_haddr = true;
> - }
> - }
> - if (argc >= 2) {
> - const char *str = argv[1];
>
> - if (!strcmp(str, "r")) {
> + for (int i = 0; i < argc; i++) {
> + char *opt = argv[i];
> + g_autofree char **tokens = g_strsplit(opt, "=", 2);
> +
> + if (g_strcmp0(tokens[0], "hadrr") == 0) {
s/hadrr/haddr/
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_haddr)) {
> + fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "track") == 0) {
> + if (g_strcmp0(tokens[1], "r") == 0) {
> rw = QEMU_PLUGIN_MEM_R;
> - } else if (!strcmp(str, "w")) {
> + } else if (g_strcmp0(tokens[1], "w") == 0) {
> rw = QEMU_PLUGIN_MEM_W;
> + } else if (g_strcmp0(tokens[1], "rw") == 0) {
> + rw = QEMU_PLUGIN_MEM_RW;
> + } else {
> + fprintf(stderr, "invaild value for argument track: %s\n", opt);
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "inline") == 0) {
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
> + fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "callback") == 0) {
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_callback)) {
> + fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
> + return -1;
> }
> - }
> - if (!strcmp(argv[0], "inline")) {
> - do_inline = true;
> - do_callback = false;
> - } else if (!strcmp(argv[0], "both")) {
> - do_inline = true;
> - do_callback = true;
> } else {
> - do_callback = true;
> + fprintf(stderr, "option parsing failed: %s\n", opt);
> + return -1;
> }
> }
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 12/13] tests/plugins/syscalls: adhere to new arg-passing scheme
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (10 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 11/13] tests/plugins/mem: introduce "track" arg and make args not positional Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 15:32 ` Alex Bennée
2021-07-17 10:09 ` [PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=` Mahmoud Mandour
2021-07-17 13:29 ` [PATCH 00/13] new plugin argument passing scheme Alex Bennée
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Mahmoud Mandour, cota, Alex Bennée
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
tests/plugin/syscall.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c
index 6dd71092e1..484b48de49 100644
--- a/tests/plugin/syscall.c
+++ b/tests/plugin/syscall.c
@@ -119,17 +119,26 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
int argc, char **argv)
{
- if (argc == 0) {
- statistics = g_hash_table_new_full(NULL, g_direct_equal, NULL, g_free);
- } else {
- for (int i = 0; i < argc; i++) {
- if (g_strcmp0(argv[i], "print") != 0) {
- fprintf(stderr, "unsupported argument: %s\n", argv[i]);
- return -1;
+ bool do_print = false;
+
+ for (int i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_autofree char **tokens = g_strsplit(opt, "=", 2);
+
+ if (g_strcmp0(tokens[0], "print") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_print)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
}
+ } else {
+ fprintf(stderr, "unsupported argument: %s\n", argv[i]);
+ return -1;
}
}
+ if (!do_print) {
+ statistics = g_hash_table_new_full(NULL, g_direct_equal, NULL, g_free);
+ }
+
qemu_plugin_register_vcpu_syscall_cb(id, vcpu_syscall);
qemu_plugin_register_vcpu_syscall_ret_cb(id, vcpu_syscall_ret);
qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=`
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (11 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 12/13] tests/plugins/syscalls: adhere to new arg-passing scheme Mahmoud Mandour
@ 2021-07-17 10:09 ` Mahmoud Mandour
2021-07-19 12:13 ` Peter Krempa
2021-07-17 13:29 ` [PATCH 00/13] new plugin argument passing scheme Alex Bennée
13 siblings, 1 reply; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: reviewer:Incompatible changes, Mahmoud Mandour, cota
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
docs/system/deprecated.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e2e0090878..aaf0ee5777 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -126,6 +126,12 @@ other options have been processed. This will either have no effect (if
if they were not given. The property is therefore useless and should not be
specified.
+Plugin argument passing through ``arg=<string>`` (since 6.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Passing arguments through ``arg=`` is redundant is makes the command-line less
+readable, especially when the argument itself consist of a name and a value,
+e.g. ``arg="arg_name=arg_value"``. Therefore, the usage of ``arg`` is redundant.
QEMU Machine Protocol (QMP) commands
------------------------------------
--
2.25.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=`
2021-07-17 10:09 ` [PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=` Mahmoud Mandour
@ 2021-07-19 12:13 ` Peter Krempa
0 siblings, 0 replies; 29+ messages in thread
From: Peter Krempa @ 2021-07-19 12:13 UTC (permalink / raw)
To: Mahmoud Mandour; +Cc: reviewer:Incompatible changes, cota, qemu-devel
On Sat, Jul 17, 2021 at 12:09:20 +0200, Mahmoud Mandour wrote:
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> ---
> docs/system/deprecated.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index e2e0090878..aaf0ee5777 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -126,6 +126,12 @@ other options have been processed. This will either have no effect (if
> if they were not given. The property is therefore useless and should not be
> specified.
>
> +Plugin argument passing through ``arg=<string>`` (since 6.1)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Passing arguments through ``arg=`` is redundant is makes the command-line less
> +readable, especially when the argument itself consist of a name and a value,
> +e.g. ``arg="arg_name=arg_value"``. Therefore, the usage of ``arg`` is redundant.
Based on this entry itself it's hard to figure out that this is for TCG
plugings. Preferably reword that entry so it's more clear.
Libvirt isn't setting these so it's okay to deprecate/remove from our
point of view.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 00/13] new plugin argument passing scheme
2021-07-17 10:09 [PATCH 00/13] new plugin argument passing scheme Mahmoud Mandour
` (12 preceding siblings ...)
2021-07-17 10:09 ` [PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=` Mahmoud Mandour
@ 2021-07-17 13:29 ` Alex Bennée
2021-07-17 13:48 ` Mahmoud Mandour
13 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2021-07-17 13:29 UTC (permalink / raw)
To: Mahmoud Mandour; +Cc: cota, qemu-devel
Mahmoud Mandour <ma.mandourr@gmail.com> writes:
> Hello,
>
> This series removes passing arguments to plugins through "arg=" since
> it's redundant and reduces readability especially when the argument
> itself is composed of a name and a value.
When you re-roll a series it's useful to add a version tag. You can use
--subject-prefix in your git format-patch command to do this.
I'll have a look at this on Monday.
>
> Also, passing arguments through "arg=" still works but is marked as
> deprecated and will produce a deprecation warning.
>
> Right now, the code for parsing the argument before passing it to the
> plugin is unfortunately not so clean but that's mainly because "arg=" is
> still supported.
>
> At first, considering boolean parameters, those were not special to
> plugins and QEMU did not complain about passing them in the form
> "arg=bool_arg" even though that's considered a short-form boolean, which
> is deprecated. As "arg" is removed, a deprecation warning is issued.
>
> This is mitigated by making plugins aware of boolean arguments and
> parses them through a newly exposed API, namely the `qapi_bool_parse`
> function through a plugin API function. Now plugins expect boolean
> parameters to be passed in the form that other parts of QEMU expect,
> i.e. "bool_arg=[on|true|yes|off|false|no]".
>
> Since we're still supporting "arg=arg_name", there are some assumptions
> that I made that I think are suitable:
>
> 1. "arg=arg_name" will be passed to the plugin as "arg_name=on".
> 2. "arg=on" and "arg" will not be assumed to be the old way of
> passing args. Instead, it will assume that the argument name is
> "arg" and it's a boolean parameter. (will be passed to plugin
> as "arg=on")
>
> The docs are updated accordingly and a deprecation notice is put in the
> deprecated.rst file.
>
> v1 -> v2:
> 1. Added patches that handle test plugins as well
> 2. Handled unsupported arguements in howvec
>
> Based-on: <20210714172151.8494-1-ma.mandourr@gmail.com>
>
> However, the dependency is so light and it should only be in the patch
>
> docs/tcg-plugins: new passing parameters scheme for cache docs
>
> where it depends on
>
> docs/devel/tcg-plugins: added cores arg to cache plugin
>
> in the aforementioned series (conflict lies in the argument "cores=N" only.)
>
> Mahmoud Mandour (13):
> plugins: allow plugin arguments to be passed directly
> plugins/api: added a boolean parsing plugin api
> plugins/hotpages: introduce sortby arg and parsed bool args correctly
> plugins/hotblocks: Added correct boolean argument parsing
> plugins/lockstep: make socket path not positional & parse bool arg
> plugins/hwprofile: adapt to the new plugin arguments scheme
> plugins/howvec: Adapting to the new argument passing scheme.
> docs/tcg-plugins: new passing parameters scheme for cache docs
> tests/plugins/bb: adapt to the new arg passing scheme
> tests/plugins/insn: made arg inline not positional and parse it as
> bool
> tests/plugins/mem: introduce "track" arg and make args not positional
> tests/plugins/syscalls: adhere to new arg-passing scheme
> docs/deprecated: deprecate passing plugin args through `arg=`
>
> contrib/plugins/hotblocks.c | 14 +++++++++--
> contrib/plugins/hotpages.c | 30 +++++++++++++++--------
> contrib/plugins/howvec.c | 27 ++++++++++++++-------
> contrib/plugins/hwprofile.c | 39 ++++++++++++++++++++----------
> contrib/plugins/lockstep.c | 31 +++++++++++++++++-------
> docs/devel/tcg-plugins.rst | 38 +++++++++++++++---------------
> docs/system/deprecated.rst | 6 +++++
> include/qemu/qemu-plugin.h | 13 ++++++++++
> linux-user/main.c | 2 +-
> plugins/api.c | 5 ++++
> plugins/loader.c | 24 +++++++++++++++----
> qemu-options.hx | 9 ++++---
> tests/plugin/bb.c | 15 ++++++++----
> tests/plugin/insn.c | 14 +++++++++--
> tests/plugin/mem.c | 47 +++++++++++++++++++++++--------------
> tests/plugin/syscall.c | 23 ++++++++++++------
> 16 files changed, 236 insertions(+), 101 deletions(-)
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 00/13] new plugin argument passing scheme
2021-07-17 13:29 ` [PATCH 00/13] new plugin argument passing scheme Alex Bennée
@ 2021-07-17 13:48 ` Mahmoud Mandour
0 siblings, 0 replies; 29+ messages in thread
From: Mahmoud Mandour @ 2021-07-17 13:48 UTC (permalink / raw)
To: Alex Bennée; +Cc: Emilio G. Cota, open list:All patches CC here
[-- Attachment #1: Type: text/plain, Size: 4643 bytes --]
On Sat, Jul 17, 2021, 15:31 Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Mahmoud Mandour <ma.mandourr@gmail.com> writes:
>
> > Hello,
> >
> > This series removes passing arguments to plugins through "arg=" since
> > it's redundant and reduces readability especially when the argument
> > itself is composed of a name and a value.
>
> When you re-roll a series it's useful to add a version tag. You can use
> --subject-prefix in your git format-patch command to do this.
>
> I'll have a look at this on Monday.
>
Thank you so much for the notice. I usually do it but I missed it this
time, hopefully won't happen again.
> >
> > Also, passing arguments through "arg=" still works but is marked as
> > deprecated and will produce a deprecation warning.
> >
> > Right now, the code for parsing the argument before passing it to the
> > plugin is unfortunately not so clean but that's mainly because "arg=" is
> > still supported.
> >
> > At first, considering boolean parameters, those were not special to
> > plugins and QEMU did not complain about passing them in the form
> > "arg=bool_arg" even though that's considered a short-form boolean, which
> > is deprecated. As "arg" is removed, a deprecation warning is issued.
> >
> > This is mitigated by making plugins aware of boolean arguments and
> > parses them through a newly exposed API, namely the `qapi_bool_parse`
> > function through a plugin API function. Now plugins expect boolean
> > parameters to be passed in the form that other parts of QEMU expect,
> > i.e. "bool_arg=[on|true|yes|off|false|no]".
> >
> > Since we're still supporting "arg=arg_name", there are some assumptions
> > that I made that I think are suitable:
> >
> > 1. "arg=arg_name" will be passed to the plugin as "arg_name=on".
> > 2. "arg=on" and "arg" will not be assumed to be the old way of
> > passing args. Instead, it will assume that the argument name is
> > "arg" and it's a boolean parameter. (will be passed to plugin
> > as "arg=on")
> >
> > The docs are updated accordingly and a deprecation notice is put in the
> > deprecated.rst file.
> >
> > v1 -> v2:
> > 1. Added patches that handle test plugins as well
> > 2. Handled unsupported arguements in howvec
> >
> > Based-on: <20210714172151.8494-1-ma.mandourr@gmail.com>
> >
> > However, the dependency is so light and it should only be in the patch
> >
> > docs/tcg-plugins: new passing parameters scheme for cache docs
> >
> > where it depends on
> >
> > docs/devel/tcg-plugins: added cores arg to cache plugin
> >
> > in the aforementioned series (conflict lies in the argument "cores=N"
> only.)
> >
> > Mahmoud Mandour (13):
> > plugins: allow plugin arguments to be passed directly
> > plugins/api: added a boolean parsing plugin api
> > plugins/hotpages: introduce sortby arg and parsed bool args correctly
> > plugins/hotblocks: Added correct boolean argument parsing
> > plugins/lockstep: make socket path not positional & parse bool arg
> > plugins/hwprofile: adapt to the new plugin arguments scheme
> > plugins/howvec: Adapting to the new argument passing scheme.
> > docs/tcg-plugins: new passing parameters scheme for cache docs
> > tests/plugins/bb: adapt to the new arg passing scheme
> > tests/plugins/insn: made arg inline not positional and parse it as
> > bool
> > tests/plugins/mem: introduce "track" arg and make args not positional
> > tests/plugins/syscalls: adhere to new arg-passing scheme
> > docs/deprecated: deprecate passing plugin args through `arg=`
> >
> > contrib/plugins/hotblocks.c | 14 +++++++++--
> > contrib/plugins/hotpages.c | 30 +++++++++++++++--------
> > contrib/plugins/howvec.c | 27 ++++++++++++++-------
> > contrib/plugins/hwprofile.c | 39 ++++++++++++++++++++----------
> > contrib/plugins/lockstep.c | 31 +++++++++++++++++-------
> > docs/devel/tcg-plugins.rst | 38 +++++++++++++++---------------
> > docs/system/deprecated.rst | 6 +++++
> > include/qemu/qemu-plugin.h | 13 ++++++++++
> > linux-user/main.c | 2 +-
> > plugins/api.c | 5 ++++
> > plugins/loader.c | 24 +++++++++++++++----
> > qemu-options.hx | 9 ++++---
> > tests/plugin/bb.c | 15 ++++++++----
> > tests/plugin/insn.c | 14 +++++++++--
> > tests/plugin/mem.c | 47 +++++++++++++++++++++++--------------
> > tests/plugin/syscall.c | 23 ++++++++++++------
> > 16 files changed, 236 insertions(+), 101 deletions(-)
>
>
> --
> Alex Bennée
[-- Attachment #2: Type: text/html, Size: 6152 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread