[PATCH 0/9] new plugin argument passing scheme

[PATCH 0/9] new plugin argument passing scheme

[Mahmoud Mandour]

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.

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.

Mahmoud Mandour (9):
  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
  docs/deprecated: deprecate passing plugin args through `arg=`

 contrib/plugins/hotblocks.c | 14 +++++++++++--
 contrib/plugins/hotpages.c  | 30 ++++++++++++++++++----------
 contrib/plugins/howvec.c    | 24 +++++++++++++++--------
 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 ++++-----
 12 files changed, 165 insertions(+), 70 deletions(-)

-- 
2.25.1

[PATCH 1/9] plugins: allow plugin arguments to be passed directly

[Mahmoud Mandour]

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

[PATCH 2/9] plugins/api: added a boolean parsing plugin api

[Mahmoud Mandour]

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

[PATCH 3/9] plugins/hotpages: introduce sortby arg and parsed bool args correctly

[Mahmoud Mandour]

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

[PATCH 4/9] plugins/hotblocks: Added correct boolean argument parsing

[Mahmoud Mandour]

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

[PATCH 5/9] plugins/lockstep: make socket path not positional & parse bool arg

[Mahmoud Mandour]

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

[PATCH 6/9] plugins/hwprofile: adapt to the new plugin arguments scheme

[Mahmoud Mandour]

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

[PATCH 7/9] plugins/howvec: Adapting to the new argument passing scheme.

[Mahmoud Mandour]

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   | 24 ++++++++++++++++--------
 docs/devel/tcg-plugins.rst | 10 +++++-----
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c
index 600f7facc1..847baef214 100644
--- a/contrib/plugins/howvec.c
+++ b/contrib/plugins/howvec.c
@@ -333,19 +333,27 @@ 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;
                 }
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

[PATCH 8/9] docs/tcg-plugins: new passing parameters scheme for cache docs

[Mahmoud Mandour]

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

[PATCH 9/9] docs/deprecated: deprecate passing plugin args through `arg=`

[Mahmoud Mandour]

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

Re: [PATCH 0/9] new plugin argument passing scheme

[Mahmoud Mandour]

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

On Fri, Jul 16, 2021 at 10:04 AM Mahmoud Mandour <ma.mandourr@gmail.com>
wrote:

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

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 (9):
>   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
>   docs/deprecated: deprecate passing plugin args through `arg=`
>
>  contrib/plugins/hotblocks.c | 14 +++++++++++--
>  contrib/plugins/hotpages.c  | 30 ++++++++++++++++++----------
>  contrib/plugins/howvec.c    | 24 +++++++++++++++--------
>  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 ++++-----
>  12 files changed, 165 insertions(+), 70 deletions(-)
>
> --
> 2.25.1
>
>

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