qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command


From: Wenchao Xia
Subject: Re: [Qemu-devel] [PATCH 2/3] HMP: pass in parameter for info sub command
Date: Thu, 20 Dec 2012 11:02:16 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:16.0) Gecko/20121026 Thunderbird/16.0.2

> On Wed, 19 Dec 2012 18:17:09 +0800
Wenchao Xia <address@hidden> wrote:

   This patch enable sub info command handler getting meaningful
parameter.

Signed-off-by: Wenchao Xia <address@hidden>
---
  hmp-commands.hx |    2 +-
  monitor.c       |   79 +++++++++++++++++++++++++++++++++++++++----------------
  2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..667fab8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1486,7 +1486,7 @@ ETEXI

      {
          .name       = "info",
-        .args_type  = "item:s?",
+        .args_type  = "item:S?",
          .params     = "[subcommand]",
          .help       = "show various information about the system state",
          .mhandler.cmd = do_info,
diff --git a/monitor.c b/monitor.c
index 797680f..ce0e74d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -464,6 +464,11 @@ QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != 
QEVENT_MAX)
  MonitorEventState monitor_event_state[QEVENT_MAX];
  QemuMutex monitor_event_state_lock;

+static const mon_cmd_t *monitor_parse_command(Monitor *mon,
+                                              const char *cmdline,
+                                              const mon_cmd_t *table,
+                                              QDict *qdict);

Please, move the function instead.

  There will be a bit big of code moving then, hope you would be OK with
it.

+
  /*
   * Emits the event to every monitor instance
   */
@@ -809,26 +814,29 @@ static void user_async_cmd_handler(Monitor *mon, const 
mon_cmd_t *cmd,
  static void do_info(Monitor *mon, const QDict *qdict)
  {
      const mon_cmd_t *cmd;
+    QDict *qdict_info;
      const char *item = qdict_get_try_str(qdict, "item");

      if (!item) {
          goto help;
      }

-    for (cmd = info_cmds; cmd->name != NULL; cmd++) {
-        if (compare_cmd(item, cmd->name))
-            break;
-    }
+    qdict_info = qdict_new();

-    if (cmd->name == NULL) {
-        goto help;
+    cmd = monitor_parse_command(mon, item, info_cmds, qdict_info);
+    if (!cmd) {
+        QDECREF(qdict_info);
+        /* don't help here, to avoid error message got ignored */
+        return;
      }

-    cmd->mhandler.info(mon, NULL);
+    cmd->mhandler.info(mon, qdict_info);
+    QDECREF(qdict_info);
      return;

  help:
      help_cmd(mon, "info");
+    return;
  }

  CommandInfoList *qmp_query_commands(Error **errp)
@@ -3534,18 +3542,15 @@ static const mon_cmd_t *search_dispatch_table(const 
mon_cmd_t *disp_table,
      return NULL;
  }

-static const mon_cmd_t *monitor_find_command(const char *cmdname)
+static const mon_cmd_t *monitor_find_command(const char *cmdname,
+                                             const mon_cmd_t *table)
  {
-    return search_dispatch_table(mon_cmds, cmdname);
-}
-
-static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
-{
-    return search_dispatch_table(qmp_cmds, cmdname);
+    return search_dispatch_table(table, cmdname);

Please, keep only search_dispatch_table().

  OK.

Actually, maybe you could change handle_qmp_command() to just loop through
command names and compare them with memcpy() (in a different patch, please)
then you keep monitor_find_command() for handle_hmp_command().

  ah, then monitor_find_command() serve HMP only. How about rename it to
hmp_find_command(table, name)? This can have a more clear meaning. Or
just remove this funtion but use search_dispatch_table() every where?

  }

  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                                                const char *cmdline,
+                                              const mon_cmd_t *table,
                                                QDict *qdict)
  {
      const char *p, *typestr;
@@ -3564,7 +3569,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
      if (!p)
          return NULL;

-    cmd = monitor_find_command(cmdname);
+    cmd = monitor_find_command(cmdname, table);
      if (!cmd) {
          monitor_printf(mon, "unknown command: '%s'\n", cmdname);
          return NULL;
@@ -3872,6 +3877,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
                  }
              }
              break;
+        case 'S':
+            {
+                /* package all remaining string */
+                int len;
+
+                while (qemu_isspace(*p)) {
+                    p++;
+                }
+                if (*typestr == '?') {
+                    typestr++;
+                    if (*p == '\0') {
+                        /* no remaining string: NULL argument */
+                        break;
+                    }
+                }
+                len = strlen(p);
+                if (len <= 0) {
+                    monitor_printf(mon, "%s: string expected\n",
+                                   cmdname);
+                    break;
+                }
+                qdict_put(qdict, key, qstring_from_str(p));
+                p += len;
+            }

This is a true HMP-style hack :)

I don't know how to make this better though (at least not without asking you
to re-write the whole thing). Maybe Markus has a better idea?

  My idea was adding a new tag meaning "package all remaining string
into the key value", what was I can found best before. Hope you have
a better solution.

+            break;
          default:
          bad_type:
              monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
@@ -3925,7 +3955,7 @@ static void handle_user_command(Monitor *mon, const char 
*cmdline)

      qdict = qdict_new();

-    cmd = monitor_parse_command(mon, cmdline, qdict);
+    cmd = monitor_parse_command(mon, cmdline, mon_cmds, qdict);
      if (!cmd)
          goto out;

@@ -4144,12 +4174,7 @@ static void monitor_find_completion(const char *cmdline)
              break;
          case 's':
              /* XXX: more generic ? */
-            if (!strcmp(cmd->name, "info")) {
-                readline_set_completion_index(cur_mon->rs, strlen(str));
-                for(cmd = info_cmds; cmd->name != NULL; cmd++) {
-                    cmd_completion(str, cmd->name);
-                }
-            } else if (!strcmp(cmd->name, "sendkey")) {
+            if (!strcmp(cmd->name, "sendkey")) {
                  char *sep = strrchr(str, '-');
                  if (sep)
                      str = sep + 1;
@@ -4163,6 +4188,14 @@ static void monitor_find_completion(const char *cmdline)
                      cmd_completion(str, cmd->name);
                  }
              }
+        case 'S':
+            /* generic string packaged */
+            if (!strcmp(cmd->name, "info")) {
+                readline_set_completion_index(cur_mon->rs, strlen(str));
+                for (cmd = info_cmds; cmd->name != NULL; cmd++) {
+                    cmd_completion(str, cmd->name);
+                }
+            }
              break;
          default:
              break;
@@ -4483,7 +4516,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
          goto err_out;
      }

-    cmd = qmp_find_cmd(cmd_name);
+    cmd = monitor_find_command(cmd_name, qmp_cmds);
      if (!cmd) {
          qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
          goto err_out;




--
Best Regards

Wenchao Xia




reply via email to

[Prev in Thread] Current Thread [Next in Thread]