All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
@ 2018-06-05 12:26 Dr. David Alan Gilbert (git)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands Dr. David Alan Gilbert (git)
                   ` (7 more replies)
  0 siblings, 8 replies; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Reenable HMP in preconfig mode; it's pretty
easy and anyway I want to do a similar thing for OOB eventually.
We'll want to enable more commands in preconfig mode to make it
useful at some point.

Dave


Dr. David Alan Gilbert (6):
  hmp: Add flag for preconfig commands
  hmp: Allow help on preconfig commands
  hmp: Restrict auto-complete in preconfig
  hmp: Add info commands for preconfig
  hmp: add exit_preconfig
  hmp: Allow HMP in preconfig state again

 hmp-commands-info.hx |  9 +++++++++
 hmp-commands.hx      | 17 +++++++++++++++++
 hmp.c                |  7 +++++++
 hmp.h                |  1 +
 monitor.c            | 42 +++++++++++++++++++++++++++++++++---------
 5 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.17.0

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
@ 2018-06-05 12:26 ` Dr. David Alan Gilbert (git)
  2018-06-07  8:50   ` Peter Xu
  2018-06-07 12:14   ` Markus Armbruster
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 2/6] hmp: Allow help on " Dr. David Alan Gilbert (git)
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add a flag to command definitions to allow them to be used in preconfig
and check it.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/monitor.c b/monitor.c
index 6d0cec552e..50b95f41db 100644
--- a/monitor.c
+++ b/monitor.c
@@ -128,6 +128,7 @@ typedef struct mon_cmd_t {
     const char *args_type;
     const char *params;
     const char *help;
+    const char *flags; /* p=preconfig */
     void (*cmd)(Monitor *mon, const QDict *qdict);
     /* @sub_table is a list of 2nd level of commands. If it does not exist,
      * cmd should be used. If it exists, sub_table[?].cmd should be
@@ -936,6 +937,19 @@ static int parse_cmdline(const char *cmdline,
     return -1;
 }
 
+/*
+ * Returns true if the command can be executed in preconfig mode
+ * i.e. it has the 'p' flag.
+ */
+static bool cmd_can_preconfig(const mon_cmd_t *cmd)
+{
+    if (!cmd->flags) {
+        return false;
+    }
+
+    return strchr(cmd->flags, 'p');
+}
+
 static void help_cmd_dump_one(Monitor *mon,
                               const mon_cmd_t *cmd,
                               char **prefix_args,
@@ -2976,6 +2990,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                        (int)(p - cmdp_start), cmdp_start);
         return NULL;
     }
+    if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
+        monitor_printf(mon, "Command '%.*s' not available in preconfig\n",
+                       (int)(p - cmdp_start), cmdp_start);
+        return NULL;
+    }
 
     /* filter out following useless space */
     while (qemu_isspace(*p)) {
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 2/6] hmp: Allow help on preconfig commands
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands Dr. David Alan Gilbert (git)
@ 2018-06-05 12:26 ` Dr. David Alan Gilbert (git)
  2018-06-07  8:51   ` Peter Xu
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 3/6] hmp: Restrict auto-complete in preconfig Dr. David Alan Gilbert (git)
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Allow the 'help' command in preconfig state but
make it only list the preconfig commands.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands.hx | 1 +
 monitor.c       | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0734fea931..8bf590ae4b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -15,6 +15,7 @@ ETEXI
         .params     = "[cmd]",
         .help       = "show the help",
         .cmd        = do_help_cmd,
+        .flags      = "p",
     },
 
 STEXI
diff --git a/monitor.c b/monitor.c
index 50b95f41db..7d1709c225 100644
--- a/monitor.c
+++ b/monitor.c
@@ -957,6 +957,10 @@ static void help_cmd_dump_one(Monitor *mon,
 {
     int i;
 
+    if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
+        return;
+    }
+
     for (i = 0; i < prefix_args_nb; i++) {
         monitor_printf(mon, "%s ", prefix_args[i]);
     }
@@ -979,7 +983,9 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds,
 
     /* Find one entry to dump */
     for (cmd = cmds; cmd->name != NULL; cmd++) {
-        if (compare_cmd(args[arg_index], cmd->name)) {
+        if (compare_cmd(args[arg_index], cmd->name) &&
+            ((!runstate_check(RUN_STATE_PRECONFIG) ||
+                cmd_can_preconfig(cmd)))) {
             if (cmd->sub_table) {
                 /* continue with next arg */
                 help_cmd_dump(mon, cmd->sub_table,
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 3/6] hmp: Restrict auto-complete in preconfig
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands Dr. David Alan Gilbert (git)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 2/6] hmp: Allow help on " Dr. David Alan Gilbert (git)
@ 2018-06-05 12:26 ` Dr. David Alan Gilbert (git)
  2018-06-07  8:52   ` Peter Xu
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig Dr. David Alan Gilbert (git)
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Don't show the commands that aren't available.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index 7d1709c225..9b29787a52 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3950,12 +3950,17 @@ static void monitor_find_completion_by_table(Monitor *mon,
             cmdname = args[0];
         readline_set_completion_index(mon->rs, strlen(cmdname));
         for (cmd = cmd_table; cmd->name != NULL; cmd++) {
-            cmd_completion(mon, cmdname, cmd->name);
+            if (!runstate_check(RUN_STATE_PRECONFIG) ||
+                 cmd_can_preconfig(cmd)) {
+                cmd_completion(mon, cmdname, cmd->name);
+            }
         }
     } else {
         /* find the command */
         for (cmd = cmd_table; cmd->name != NULL; cmd++) {
-            if (compare_cmd(args[0], cmd->name)) {
+            if (compare_cmd(args[0], cmd->name) &&
+                (!runstate_check(RUN_STATE_PRECONFIG) ||
+                 cmd_can_preconfig(cmd))) {
                 break;
             }
         }
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
                   ` (2 preceding siblings ...)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 3/6] hmp: Restrict auto-complete in preconfig Dr. David Alan Gilbert (git)
@ 2018-06-05 12:26 ` Dr. David Alan Gilbert (git)
  2018-06-07  8:49   ` Peter Xu
  2018-06-07  9:39   ` Igor Mammedov
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 5/6] hmp: add exit_preconfig Dr. David Alan Gilbert (git)
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Allow a bunch of the info commands to be used in preconfig.
Could probably add most of them.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands-info.hx | 9 +++++++++
 hmp-commands.hx      | 1 +
 2 files changed, 10 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ddfcd5adcc..00735e7d1c 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -19,6 +19,7 @@ ETEXI
         .params     = "",
         .help       = "show the version of QEMU",
         .cmd        = hmp_info_version,
+        .flags      = "p",
     },
 
 STEXI
@@ -33,6 +34,7 @@ ETEXI
         .params     = "",
         .help       = "show the network state",
         .cmd        = hmp_info_network,
+        .flags      = "p",
     },
 
 STEXI
@@ -47,6 +49,7 @@ ETEXI
         .params     = "",
         .help       = "show the character devices",
         .cmd        = hmp_info_chardev,
+        .flags      = "p",
     },
 
 STEXI
@@ -62,6 +65,7 @@ ETEXI
         .help       = "show info of one block device or all block devices "
                       "(-n: show named nodes; -v: show details)",
         .cmd        = hmp_info_block,
+        .flags      = "p",
     },
 
 STEXI
@@ -151,6 +155,7 @@ ETEXI
         .params     = "",
         .help       = "show infos for each CPU",
         .cmd        = hmp_info_cpus,
+        .flags      = "p",
     },
 
 STEXI
@@ -165,6 +170,7 @@ ETEXI
         .params     = "",
         .help       = "show the command line history",
         .cmd        = hmp_info_history,
+        .flags      = "p",
     },
 
 STEXI
@@ -255,6 +261,7 @@ ETEXI
         .help       = "show memory tree (-f: dump flat view for address spaces;"
                       "-d: dump dispatch tree, valid with -f only)",
         .cmd        = hmp_info_mtree,
+        .flags      = "p",
     },
 
 STEXI
@@ -399,6 +406,7 @@ ETEXI
         .params     = "",
         .help       = "show the current VM status (running|paused)",
         .cmd        = hmp_info_status,
+        .flags      = "p",
     },
 
 STEXI
@@ -829,6 +837,7 @@ ETEXI
         .params     = "",
         .help       = "Show information about hotpluggable CPUs",
         .cmd        = hmp_hotpluggable_cpus,
+        .flags      = "p",
     },
 
 STEXI
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8bf590ae4b..dc82ed526f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1856,6 +1856,7 @@ ETEXI
         .help       = "show various information about the system state",
         .cmd        = hmp_info_help,
         .sub_table  = info_cmds,
+        .flags      = "p",
     },
 
 STEXI
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 5/6] hmp: add exit_preconfig
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
                   ` (3 preceding siblings ...)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig Dr. David Alan Gilbert (git)
@ 2018-06-05 12:26 ` Dr. David Alan Gilbert (git)
  2018-06-07  8:53   ` Peter Xu
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 6/6] hmp: Allow HMP in preconfig state again Dr. David Alan Gilbert (git)
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add the exit_preconfig command to return to normality.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands.hx | 15 +++++++++++++++
 hmp.c           |  7 +++++++
 hmp.h           |  1 +
 3 files changed, 23 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index dc82ed526f..40e2f6ca08 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -55,6 +55,21 @@ STEXI
 @item q or quit
 @findex quit
 Quit the emulator.
+ETEXI
+
+    {
+        .name       = "exit_preconfig",
+        .args_type  = "",
+        .params     = "",
+        .help       = "exit the preconfig state",
+        .cmd        = hmp_exit_preconfig,
+        .flags      = "p",
+    },
+
+STEXI
+@item exit_preconfig
+@findex exit_preconfig
+Exit the preconfig state
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index ef93f4878b..c7be6ed394 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1065,6 +1065,13 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict)
     qmp_system_powerdown(NULL);
 }
 
+void hmp_exit_preconfig(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    qmp_exit_preconfig(&err);
+    hmp_handle_error(mon, &err);
+}
+
 void hmp_cpu(Monitor *mon, const QDict *qdict)
 {
     int64_t cpu_index;
diff --git a/hmp.h b/hmp.h
index 20f27439d3..33354f1bdd 100644
--- a/hmp.h
+++ b/hmp.h
@@ -44,6 +44,7 @@ void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
 void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
+void hmp_exit_preconfig(Monitor *mon, const QDict *qdict);
 void hmp_cpu(Monitor *mon, const QDict *qdict);
 void hmp_memsave(Monitor *mon, const QDict *qdict);
 void hmp_pmemsave(Monitor *mon, const QDict *qdict);
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 6/6] hmp: Allow HMP in preconfig state again
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
                   ` (4 preceding siblings ...)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 5/6] hmp: add exit_preconfig Dr. David Alan Gilbert (git)
@ 2018-06-05 12:26 ` Dr. David Alan Gilbert (git)
  2018-06-07  8:56   ` Peter Xu
  2018-06-07  8:54 ` [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Peter Xu
  2018-06-07  9:48 ` Igor Mammedov
  7 siblings, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 12:26 UTC (permalink / raw)
  To: qemu-devel, armbru, imammedo

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Now we can cope with preconfig in HMP, reenable by reverting
commit 71dc578e116599ea73c8a2a4e693134702ec0e83.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index 9b29787a52..50bcba18db 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3394,12 +3394,6 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
 
     trace_handle_hmp_command(mon, cmdline);
 
-    if (runstate_check(RUN_STATE_PRECONFIG)) {
-        monitor_printf(mon, "HMP not available in preconfig state, "
-                            "use QMP instead\n");
-        return;
-    }
-
     cmd = monitor_parse_command(mon, cmdline, &cmdline, mon->cmd_table);
     if (!cmd) {
         return;
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig Dr. David Alan Gilbert (git)
@ 2018-06-07  8:49   ` Peter Xu
  2018-06-07 12:22     ` Markus Armbruster
  2018-06-07  9:39   ` Igor Mammedov
  1 sibling, 1 reply; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:49 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Allow a bunch of the info commands to be used in preconfig.
> Could probably add most of them.

I guess some of them may not work yet during preconfig.  E.g.:

$ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
QEMU 2.12.50 monitor - type 'help' for more information
(qemu) info mtree
address-space: memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system

address-space: I/O
  0000000000000000-000000000000ffff (prio 0, i/o): io

But it's fine to enable that I guess.

(Which "info" command would you want to use during preconfig?)

> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands Dr. David Alan Gilbert (git)
@ 2018-06-07  8:50   ` Peter Xu
  2018-06-07 12:14   ` Markus Armbruster
  1 sibling, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:50 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:31PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add a flag to command definitions to allow them to be used in preconfig
> and check it.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 2/6] hmp: Allow help on preconfig commands
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 2/6] hmp: Allow help on " Dr. David Alan Gilbert (git)
@ 2018-06-07  8:51   ` Peter Xu
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:51 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:32PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Allow the 'help' command in preconfig state but
> make it only list the preconfig commands.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hmp-commands.hx | 1 +
>  monitor.c       | 8 +++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 0734fea931..8bf590ae4b 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -15,6 +15,7 @@ ETEXI
>          .params     = "[cmd]",
>          .help       = "show the help",
>          .cmd        = do_help_cmd,
> +        .flags      = "p",
>      },
>  
>  STEXI
> diff --git a/monitor.c b/monitor.c
> index 50b95f41db..7d1709c225 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -957,6 +957,10 @@ static void help_cmd_dump_one(Monitor *mon,
>  {
>      int i;
>  
> +    if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
> +        return;
> +    }
> +
>      for (i = 0; i < prefix_args_nb; i++) {
>          monitor_printf(mon, "%s ", prefix_args[i]);
>      }
> @@ -979,7 +983,9 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds,
>  
>      /* Find one entry to dump */
>      for (cmd = cmds; cmd->name != NULL; cmd++) {
> -        if (compare_cmd(args[arg_index], cmd->name)) {
> +        if (compare_cmd(args[arg_index], cmd->name) &&
> +            ((!runstate_check(RUN_STATE_PRECONFIG) ||
> +                cmd_can_preconfig(cmd)))) {

I saw that this pattern is used for multiple times already.  Maybe
it's time to introduce hmp_cmd_is_available(). :) I'll leave the
decision to you...

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] hmp: Restrict auto-complete in preconfig
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 3/6] hmp: Restrict auto-complete in preconfig Dr. David Alan Gilbert (git)
@ 2018-06-07  8:52   ` Peter Xu
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:52 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:33PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Don't show the commands that aren't available.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  monitor.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 7d1709c225..9b29787a52 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3950,12 +3950,17 @@ static void monitor_find_completion_by_table(Monitor *mon,
>              cmdname = args[0];
>          readline_set_completion_index(mon->rs, strlen(cmdname));
>          for (cmd = cmd_table; cmd->name != NULL; cmd++) {
> -            cmd_completion(mon, cmdname, cmd->name);
> +            if (!runstate_check(RUN_STATE_PRECONFIG) ||
> +                 cmd_can_preconfig(cmd)) {
> +                cmd_completion(mon, cmdname, cmd->name);
> +            }
>          }
>      } else {
>          /* find the command */
>          for (cmd = cmd_table; cmd->name != NULL; cmd++) {
> -            if (compare_cmd(args[0], cmd->name)) {
> +            if (compare_cmd(args[0], cmd->name) &&
> +                (!runstate_check(RUN_STATE_PRECONFIG) ||
> +                 cmd_can_preconfig(cmd))) {

(so if there is a helper we can also use it here)

Reviewed-by: Peter Xu <peterx@redhat.com>

>                  break;
>              }
>          }
> -- 
> 2.17.0
> 
> 

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] hmp: add exit_preconfig
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 5/6] hmp: add exit_preconfig Dr. David Alan Gilbert (git)
@ 2018-06-07  8:53   ` Peter Xu
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:53 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:35PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add the exit_preconfig command to return to normality.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
                   ` (5 preceding siblings ...)
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 6/6] hmp: Allow HMP in preconfig state again Dr. David Alan Gilbert (git)
@ 2018-06-07  8:54 ` Peter Xu
  2018-06-07  9:00   ` Dr. David Alan Gilbert
  2018-06-07  9:45   ` Igor Mammedov
  2018-06-07  9:48 ` Igor Mammedov
  7 siblings, 2 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:54 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Reenable HMP in preconfig mode; it's pretty
> easy and anyway I want to do a similar thing for OOB eventually.
> We'll want to enable more commands in preconfig mode to make it
> useful at some point.
> 
> Dave

Wanna add "quit" into the preconfig-allowed list for HMP as well?

Tested-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 6/6] hmp: Allow HMP in preconfig state again
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 6/6] hmp: Allow HMP in preconfig state again Dr. David Alan Gilbert (git)
@ 2018-06-07  8:56   ` Peter Xu
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  8:56 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru, imammedo

On Tue, Jun 05, 2018 at 01:26:36PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Now we can cope with preconfig in HMP, reenable by reverting
> commit 71dc578e116599ea73c8a2a4e693134702ec0e83.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
  2018-06-07  8:54 ` [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Peter Xu
@ 2018-06-07  9:00   ` Dr. David Alan Gilbert
  2018-06-07  9:05     ` Peter Xu
  2018-06-07  9:45   ` Igor Mammedov
  1 sibling, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-07  9:00 UTC (permalink / raw)
  To: Peter Xu; +Cc: qemu-devel, armbru, imammedo

* Peter Xu (peterx@redhat.com) wrote:
> On Tue, Jun 05, 2018 at 01:26:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Reenable HMP in preconfig mode; it's pretty
> > easy and anyway I want to do a similar thing for OOB eventually.
> > We'll want to enable more commands in preconfig mode to make it
> > useful at some point.
> > 
> > Dave
> 
> Wanna add "quit" into the preconfig-allowed list for HMP as well?

I tried; quit triggers a disallowed state transition!

> Tested-by: Peter Xu <peterx@redhat.com>
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
  2018-06-07  9:00   ` Dr. David Alan Gilbert
@ 2018-06-07  9:05     ` Peter Xu
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07  9:05 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, armbru, imammedo

On Thu, Jun 07, 2018 at 10:00:30AM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Tue, Jun 05, 2018 at 01:26:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > Reenable HMP in preconfig mode; it's pretty
> > > easy and anyway I want to do a similar thing for OOB eventually.
> > > We'll want to enable more commands in preconfig mode to make it
> > > useful at some point.
> > > 
> > > Dave
> > 
> > Wanna add "quit" into the preconfig-allowed list for HMP as well?
> 
> I tried; quit triggers a disallowed state transition!

Would it be understandable if we just avoid calling qmp_quit() when
during preconfig?  An evil idea is, "exit()". :)

After all we have nothing there!

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig Dr. David Alan Gilbert (git)
  2018-06-07  8:49   ` Peter Xu
@ 2018-06-07  9:39   ` Igor Mammedov
  1 sibling, 0 replies; 33+ messages in thread
From: Igor Mammedov @ 2018-06-07  9:39 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru

On Tue,  5 Jun 2018 13:26:34 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Allow a bunch of the info commands to be used in preconfig.
> Could probably add most of them.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hmp-commands-info.hx | 9 +++++++++
>  hmp-commands.hx      | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index ddfcd5adcc..00735e7d1c 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -19,6 +19,7 @@ ETEXI
>          .params     = "",
>          .help       = "show the version of QEMU",
>          .cmd        = hmp_info_version,
> +        .flags      = "p",
>      },
>  
>  STEXI
> @@ -33,6 +34,7 @@ ETEXI
>          .params     = "",
>          .help       = "show the network state",
>          .cmd        = hmp_info_network,
> +        .flags      = "p",
output seems to refer to frontend devices, so we probably should skip this one

>      },
>  
>  STEXI
> @@ -47,6 +49,7 @@ ETEXI
>          .params     = "",
>          .help       = "show the character devices",
>          .cmd        = hmp_info_chardev,
> +        .flags      = "p",
>      },
>  
>  STEXI
> @@ -62,6 +65,7 @@ ETEXI
>          .help       = "show info of one block device or all block devices "
>                        "(-n: show named nodes; -v: show details)",
>          .cmd        = hmp_info_block,
> +        .flags      = "p",
>      },
Devices (frontends) are processed after preconfig point,
so there isn't point in enabling this.
At least until we could instantiate devices at that time.

>  
>  STEXI
> @@ -151,6 +155,7 @@ ETEXI
>          .params     = "",
>          .help       = "show infos for each CPU",
>          .cmd        = hmp_info_cpus,
> +        .flags      = "p",
>      },
this command probably shouldn't be enabled, there can't be any CPUs at this point yet.

>  
>  STEXI
> @@ -165,6 +170,7 @@ ETEXI
>          .params     = "",
>          .help       = "show the command line history",
>          .cmd        = hmp_info_history,
> +        .flags      = "p",
>      },
>  
>  STEXI
> @@ -255,6 +261,7 @@ ETEXI
>          .help       = "show memory tree (-f: dump flat view for address spaces;"
>                        "-d: dump dispatch tree, valid with -f only)",
>          .cmd        = hmp_info_mtree,
> +        .flags      = "p",
>      },
probably we should skip this one as well

>  
>  STEXI
> @@ -399,6 +406,7 @@ ETEXI
>          .params     = "",
>          .help       = "show the current VM status (running|paused)",
>          .cmd        = hmp_info_status,
> +        .flags      = "p",
>      },
>  
>  STEXI
> @@ -829,6 +837,7 @@ ETEXI
>          .params     = "",
>          .help       = "Show information about hotpluggable CPUs",
>          .cmd        = hmp_hotpluggable_cpus,
> +        .flags      = "p",
>      },
>  
>  STEXI
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8bf590ae4b..dc82ed526f 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1856,6 +1856,7 @@ ETEXI
>          .help       = "show various information about the system state",
>          .cmd        = hmp_info_help,
>          .sub_table  = info_cmds,
> +        .flags      = "p",
>      },
>  
>  STEXI

in addition to above we should be able to handle at preconfig time:

info memdev
info name
info numa
info qom-tree
info usbhost
info uuid
info iothreads

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
  2018-06-07  8:54 ` [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Peter Xu
  2018-06-07  9:00   ` Dr. David Alan Gilbert
@ 2018-06-07  9:45   ` Igor Mammedov
  2018-06-07 10:02     ` Peter Xu
  1 sibling, 1 reply; 33+ messages in thread
From: Igor Mammedov @ 2018-06-07  9:45 UTC (permalink / raw)
  To: Peter Xu; +Cc: Dr. David Alan Gilbert (git), qemu-devel, armbru

On Thu, 7 Jun 2018 16:54:44 +0800
Peter Xu <peterx@redhat.com> wrote:

> On Tue, Jun 05, 2018 at 01:26:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Reenable HMP in preconfig mode; it's pretty
> > easy and anyway I want to do a similar thing for OOB eventually.
> > We'll want to enable more commands in preconfig mode to make it
> > useful at some point.
> > 
> > Dave  
> 
> Wanna add "quit" into the preconfig-allowed list for HMP as well?
There is exit-preconfig, but yeah it just forwards us to the next state

But for human users that just want to play with HMP 'quit'
might be useful, but only to the extent of trying out things out.
 
> Tested-by: Peter Xu <peterx@redhat.com>
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
  2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
                   ` (6 preceding siblings ...)
  2018-06-07  8:54 ` [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Peter Xu
@ 2018-06-07  9:48 ` Igor Mammedov
  7 siblings, 0 replies; 33+ messages in thread
From: Igor Mammedov @ 2018-06-07  9:48 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, armbru

On Tue,  5 Jun 2018 13:26:30 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Reenable HMP in preconfig mode; it's pretty
> easy and anyway I want to do a similar thing for OOB eventually.
> We'll want to enable more commands in preconfig mode to make it
> useful at some point.
> 
> Dave
> 
> 
> Dr. David Alan Gilbert (6):
>   hmp: Add flag for preconfig commands
>   hmp: Allow help on preconfig commands
>   hmp: Restrict auto-complete in preconfig
>   hmp: Add info commands for preconfig
>   hmp: add exit_preconfig
>   hmp: Allow HMP in preconfig state again
> 
>  hmp-commands-info.hx |  9 +++++++++
>  hmp-commands.hx      | 17 +++++++++++++++++
>  hmp.c                |  7 +++++++
>  hmp.h                |  1 +
>  monitor.c            | 42 +++++++++++++++++++++++++++++++++---------
>  5 files changed, 67 insertions(+), 9 deletions(-)
> 

I was trying to find out way to use QMP schema info with HMP
without manual commands flagging but this looks so much simpler,
so that if it is acceptable I'm all in for this version.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
except 4/6 which needs a little bit amended.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode
  2018-06-07  9:45   ` Igor Mammedov
@ 2018-06-07 10:02     ` Peter Xu
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Xu @ 2018-06-07 10:02 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: Dr. David Alan Gilbert (git), qemu-devel, armbru

On Thu, Jun 07, 2018 at 11:45:17AM +0200, Igor Mammedov wrote:
> On Thu, 7 Jun 2018 16:54:44 +0800
> Peter Xu <peterx@redhat.com> wrote:
> 
> > On Tue, Jun 05, 2018 at 01:26:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > Reenable HMP in preconfig mode; it's pretty
> > > easy and anyway I want to do a similar thing for OOB eventually.
> > > We'll want to enable more commands in preconfig mode to make it
> > > useful at some point.
> > > 
> > > Dave  
> > 
> > Wanna add "quit" into the preconfig-allowed list for HMP as well?
> There is exit-preconfig, but yeah it just forwards us to the next state
> 
> But for human users that just want to play with HMP 'quit'
> might be useful, but only to the extent of trying out things out.

Yes, actually I don't yet see a good reason to actually use HMP during
preconfig mode even as a whole.  But as Dave mentioned, it's too easy
to achieve that so I don't have a reason to object that too.

Regards,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands
  2018-06-05 12:26 ` [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands Dr. David Alan Gilbert (git)
  2018-06-07  8:50   ` Peter Xu
@ 2018-06-07 12:14   ` Markus Armbruster
  2018-06-07 12:18     ` Dr. David Alan Gilbert
  1 sibling, 1 reply; 33+ messages in thread
From: Markus Armbruster @ 2018-06-07 12:14 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, imammedo

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Add a flag to command definitions to allow them to be used in preconfig
> and check it.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  monitor.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 6d0cec552e..50b95f41db 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -128,6 +128,7 @@ typedef struct mon_cmd_t {
>      const char *args_type;
>      const char *params;
>      const char *help;
> +    const char *flags; /* p=preconfig */
>      void (*cmd)(Monitor *mon, const QDict *qdict);
>      /* @sub_table is a list of 2nd level of commands. If it does not exist,
>       * cmd should be used. If it exists, sub_table[?].cmd should be
> @@ -936,6 +937,19 @@ static int parse_cmdline(const char *cmdline,
>      return -1;
>  }
>  
> +/*
> + * Returns true if the command can be executed in preconfig mode
> + * i.e. it has the 'p' flag.
> + */
> +static bool cmd_can_preconfig(const mon_cmd_t *cmd)
> +{
> +    if (!cmd->flags) {
> +        return false;
> +    }
> +
> +    return strchr(cmd->flags, 'p');
> +}
> +
>  static void help_cmd_dump_one(Monitor *mon,
>                                const mon_cmd_t *cmd,
>                                char **prefix_args,
> @@ -2976,6 +2990,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>                         (int)(p - cmdp_start), cmdp_start);
>          return NULL;
>      }
> +    if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
> +        monitor_printf(mon, "Command '%.*s' not available in preconfig\n",
> +                       (int)(p - cmdp_start), cmdp_start);
> +        return NULL;
> +    }
>  
>      /* filter out following useless space */
>      while (qemu_isspace(*p)) {

This

    (qemu) c
    Command 'c' not available in preconfig

is not a nice user experience.  We can do better (paraphrasing myself):

    Command 'c' not available with -preconfig until you complete
    configuration with QMP

Note that this talks to the user in the user's terms (-preconfig)
instead of internal terms ("in preconfig", whatever that is), and is
more specific on how to ready the monitor.

Obviously, "with QMP" needs an update once HMP acquires a command to
exit preconfig state.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands
  2018-06-07 12:14   ` Markus Armbruster
@ 2018-06-07 12:18     ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 33+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-07 12:18 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, imammedo

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Add a flag to command definitions to allow them to be used in preconfig
> > and check it.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  monitor.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 6d0cec552e..50b95f41db 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -128,6 +128,7 @@ typedef struct mon_cmd_t {
> >      const char *args_type;
> >      const char *params;
> >      const char *help;
> > +    const char *flags; /* p=preconfig */
> >      void (*cmd)(Monitor *mon, const QDict *qdict);
> >      /* @sub_table is a list of 2nd level of commands. If it does not exist,
> >       * cmd should be used. If it exists, sub_table[?].cmd should be
> > @@ -936,6 +937,19 @@ static int parse_cmdline(const char *cmdline,
> >      return -1;
> >  }
> >  
> > +/*
> > + * Returns true if the command can be executed in preconfig mode
> > + * i.e. it has the 'p' flag.
> > + */
> > +static bool cmd_can_preconfig(const mon_cmd_t *cmd)
> > +{
> > +    if (!cmd->flags) {
> > +        return false;
> > +    }
> > +
> > +    return strchr(cmd->flags, 'p');
> > +}
> > +
> >  static void help_cmd_dump_one(Monitor *mon,
> >                                const mon_cmd_t *cmd,
> >                                char **prefix_args,
> > @@ -2976,6 +2990,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >                         (int)(p - cmdp_start), cmdp_start);
> >          return NULL;
> >      }
> > +    if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
> > +        monitor_printf(mon, "Command '%.*s' not available in preconfig\n",
> > +                       (int)(p - cmdp_start), cmdp_start);
> > +        return NULL;
> > +    }
> >  
> >      /* filter out following useless space */
> >      while (qemu_isspace(*p)) {
> 
> This
> 
>     (qemu) c
>     Command 'c' not available in preconfig
> 
> is not a nice user experience.  We can do better (paraphrasing myself):
> 
>     Command 'c' not available with -preconfig until you complete
>     configuration with QMP
> 
> Note that this talks to the user in the user's terms (-preconfig)
> instead of internal terms ("in preconfig", whatever that is), and is
> more specific on how to ready the monitor.
> 
> Obviously, "with QMP" needs an update once HMP acquires a command to
> exit preconfig state.

This series already adds that command.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07  8:49   ` Peter Xu
@ 2018-06-07 12:22     ` Markus Armbruster
  2018-06-07 13:45       ` Dr. David Alan Gilbert
  2018-06-07 15:08       ` Igor Mammedov
  0 siblings, 2 replies; 33+ messages in thread
From: Markus Armbruster @ 2018-06-07 12:22 UTC (permalink / raw)
  To: Peter Xu; +Cc: Dr. David Alan Gilbert (git), imammedo, qemu-devel

Peter Xu <peterx@redhat.com> writes:

> On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> 
>> Allow a bunch of the info commands to be used in preconfig.
>> Could probably add most of them.
>
> I guess some of them may not work yet during preconfig.  E.g.:
>
> $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> QEMU 2.12.50 monitor - type 'help' for more information
> (qemu) info mtree
> address-space: memory
>   0000000000000000-ffffffffffffffff (prio 0, i/o): system
>
> address-space: I/O
>   0000000000000000-000000000000ffff (prio 0, i/o): io
>
> But it's fine to enable that I guess.
>
> (Which "info" command would you want to use during preconfig?)
>
>> 
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>

The reason for having -preconfig is us despairing of making -S do the
right thing.  We'd have to *understand* the tangled mess that is our
startup, and rearrange it so QMP becomes available early enough for
configuring NUMA (and other things), yet late enough for everything to
work.

-preconfig is a cheap hack to avoid this headache, by bypassing almost
all of "everything".

Now you bring back some of "everything".  Dangerous.  You better show it
actually works.  Until you do:

NAK

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07 12:22     ` Markus Armbruster
@ 2018-06-07 13:45       ` Dr. David Alan Gilbert
  2018-06-07 15:12         ` Markus Armbruster
  2018-06-07 15:08       ` Igor Mammedov
  1 sibling, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-07 13:45 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Xu, imammedo, qemu-devel

* Markus Armbruster (armbru@redhat.com) wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> 
> >> Allow a bunch of the info commands to be used in preconfig.
> >> Could probably add most of them.
> >
> > I guess some of them may not work yet during preconfig.  E.g.:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> > QEMU 2.12.50 monitor - type 'help' for more information
> > (qemu) info mtree
> > address-space: memory
> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >
> > address-space: I/O
> >   0000000000000000-000000000000ffff (prio 0, i/o): io
> >
> > But it's fine to enable that I guess.
> >
> > (Which "info" command would you want to use during preconfig?)
> >
> >> 
> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> The reason for having -preconfig is us despairing of making -S do the
> right thing.  We'd have to *understand* the tangled mess that is our
> startup, and rearrange it so QMP becomes available early enough for
> configuring NUMA (and other things), yet late enough for everything to
> work.
> 
> -preconfig is a cheap hack to avoid this headache, by bypassing almost
> all of "everything".
> 
> Now you bring back some of "everything".  Dangerous.  You better show it
> actually works.  Until you do:
> 
> NAK

Well I did test each command in here to make sure it didn't
crash/produce complete junk; but here's the output with the v2 of this
patch that Igor R-b:

[dgilbert@dgilbert-t530 try]$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -preconfig

(qemu) 
(qemu) 
(qemu) help info
info chardev  -- show the character devices
info history  -- show the command line history
info hotpluggable-cpus  -- Show information about hotpluggable CPUs
info iothreads  -- show iothreads
info memdev  -- show memory backends
info name  -- show the current VM name
info numa  -- show NUMA information
info qom-tree [path] -- show QOM composition tree
info status  -- show the current VM status (running|paused)
info usbhost  -- show host USB devices
info uuid  -- show the current VM UUID
info version  -- show the version of QEMU
(qemu) info chardev
serial0: filename=mux
serial0-base: filename=stdio
parallel0: filename=null
(qemu) info history
0: 'help info'
1: 'info chardev'
2: 'info history'
(qemu) info hotpluggable-cpus 
Hotpluggable CPUs:
  type: "qemu64-x86_64-cpu"
  vcpus_count: "1"
  CPUInstance Properties:
    socket-id: "0"
    core-id: "0"
    thread-id: "0"
(qemu) info iothreads
(qemu) info memdev

(qemu) info name
(qemu) info numa
0 nodes
(qemu) info qom-tree 
/machine (pc-i440fx-3.0-machine)
  /peripheral (container)
  /peripheral-anon (container)
  /unattached (container)
    /system[0] (qemu:memory-region)
    /io[0] (qemu:memory-region)
(qemu) info status
VM status: paused (preconfig)
(qemu) info usbhost 
  Bus 2, Addr 3, Port 1.5, Speed 1.5 Mb/s
    Class 00: USB device 093a:2510
  Bus 1, Addr 3, Port 1.6, Speed 480 Mb/s
    Class ef: USB device 5986:02d2
  Bus 3, Addr 2, Port 1, Speed 1.5 Mb/s
    Class 00: USB device 0d3d:0001
(qemu) info uuid
00000000-0000-0000-0000-000000000000
(qemu) info version
2.12.50v2.12.0-1074-gfbb4dcae3a
(qemu) 

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07 12:22     ` Markus Armbruster
  2018-06-07 13:45       ` Dr. David Alan Gilbert
@ 2018-06-07 15:08       ` Igor Mammedov
  2018-06-08  5:41         ` Markus Armbruster
  1 sibling, 1 reply; 33+ messages in thread
From: Igor Mammedov @ 2018-06-07 15:08 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Xu, Dr. David Alan Gilbert (git), qemu-devel

On Thu, 07 Jun 2018 14:22:34 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:  
> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> 
> >> Allow a bunch of the info commands to be used in preconfig.
> >> Could probably add most of them.  
> >
> > I guess some of them may not work yet during preconfig.  E.g.:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> > QEMU 2.12.50 monitor - type 'help' for more information
> > (qemu) info mtree
> > address-space: memory
> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >
> > address-space: I/O
> >   0000000000000000-000000000000ffff (prio 0, i/o): io
> >
> > But it's fine to enable that I guess.
> >
> > (Which "info" command would you want to use during preconfig?)
> >  
> >> 
> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>  
> >
> > Reviewed-by: Peter Xu <peterx@redhat.com>  
> 
> The reason for having -preconfig is us despairing of making -S do the
> right thing.  We'd have to *understand* the tangled mess that is our
> startup, and rearrange it so QMP becomes available early enough for
> configuring NUMA (and other things), yet late enough for everything to
> work.
We solve concrete NUMA problem at -S time as was demonstrated by thread:
"RFC v2 0/4] enable numa configuration before machine  is running from HMP/QMP"
were pros and cons are were summarized. But that won't scale to other things.
 
> -preconfig is a cheap hack to avoid this headache, by bypassing almost
> all of "everything".
hack should allow to move configuration steps to early stage one by one
until the point we could obsolete -S (from configuration point)
without breaking current -S users and clean up start up mess in process.

> Now you bring back some of "everything".  Dangerous.  You better show it
> actually works.  Until you do:
Tested v2, works as expected so far.

> 
> NAK
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07 13:45       ` Dr. David Alan Gilbert
@ 2018-06-07 15:12         ` Markus Armbruster
  2018-06-07 15:28           ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 33+ messages in thread
From: Markus Armbruster @ 2018-06-07 15:12 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: imammedo, qemu-devel, Peter Xu

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
>> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >> 
>> >> Allow a bunch of the info commands to be used in preconfig.
>> >> Could probably add most of them.
>> >
>> > I guess some of them may not work yet during preconfig.  E.g.:
>> >
>> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
>> > QEMU 2.12.50 monitor - type 'help' for more information
>> > (qemu) info mtree
>> > address-space: memory
>> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
>> >
>> > address-space: I/O
>> >   0000000000000000-000000000000ffff (prio 0, i/o): io
>> >
>> > But it's fine to enable that I guess.
>> >
>> > (Which "info" command would you want to use during preconfig?)
>> >
>> >> 
>> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> >
>> > Reviewed-by: Peter Xu <peterx@redhat.com>
>> 
>> The reason for having -preconfig is us despairing of making -S do the
>> right thing.  We'd have to *understand* the tangled mess that is our
>> startup, and rearrange it so QMP becomes available early enough for
>> configuring NUMA (and other things), yet late enough for everything to
>> work.
>> 
>> -preconfig is a cheap hack to avoid this headache, by bypassing almost
>> all of "everything".
>> 
>> Now you bring back some of "everything".  Dangerous.  You better show it
>> actually works.  Until you do:
>> 
>> NAK
>
> Well I did test each command in here to make sure it didn't
> crash/produce complete junk; but here's the output with the v2 of this
> patch that Igor R-b:
[...]

For the sake of the argument, let's assume these commands all work in
preconfig state.  Are their QMP equivalents all available in preconfig
state?

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07 15:12         ` Markus Armbruster
@ 2018-06-07 15:28           ` Dr. David Alan Gilbert
  2018-06-08  5:56             ` Markus Armbruster
  0 siblings, 1 reply; 33+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-07 15:28 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: imammedo, qemu-devel, Peter Xu

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Peter Xu <peterx@redhat.com> writes:
> >> 
> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> >> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> >> 
> >> >> Allow a bunch of the info commands to be used in preconfig.
> >> >> Could probably add most of them.
> >> >
> >> > I guess some of them may not work yet during preconfig.  E.g.:
> >> >
> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> >> > QEMU 2.12.50 monitor - type 'help' for more information
> >> > (qemu) info mtree
> >> > address-space: memory
> >> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >> >
> >> > address-space: I/O
> >> >   0000000000000000-000000000000ffff (prio 0, i/o): io
> >> >
> >> > But it's fine to enable that I guess.
> >> >
> >> > (Which "info" command would you want to use during preconfig?)
> >> >
> >> >> 
> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> >
> >> > Reviewed-by: Peter Xu <peterx@redhat.com>
> >> 
> >> The reason for having -preconfig is us despairing of making -S do the
> >> right thing.  We'd have to *understand* the tangled mess that is our
> >> startup, and rearrange it so QMP becomes available early enough for
> >> configuring NUMA (and other things), yet late enough for everything to
> >> work.
> >> 
> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost
> >> all of "everything".
> >> 
> >> Now you bring back some of "everything".  Dangerous.  You better show it
> >> actually works.  Until you do:
> >> 
> >> NAK
> >
> > Well I did test each command in here to make sure it didn't
> > crash/produce complete junk; but here's the output with the v2 of this
> > patch that Igor R-b:
> [...]
> 
> For the sake of the argument, let's assume these commands all work in
> preconfig state.  Are their QMP equivalents all available in preconfig
> state?

That I don't know; I was happy to fix my list to the ones
Igor recommended.  If you object to some particular entries I'll
be happy to change them.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07 15:08       ` Igor Mammedov
@ 2018-06-08  5:41         ` Markus Armbruster
  0 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2018-06-08  5:41 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: Dr. David Alan Gilbert (git), Peter Xu, qemu-devel

Igor Mammedov <imammedo@redhat.com> writes:

> On Thu, 07 Jun 2018 14:22:34 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:  
>> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >> 
>> >> Allow a bunch of the info commands to be used in preconfig.
>> >> Could probably add most of them.  
>> >
>> > I guess some of them may not work yet during preconfig.  E.g.:
>> >
>> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
>> > QEMU 2.12.50 monitor - type 'help' for more information
>> > (qemu) info mtree
>> > address-space: memory
>> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
>> >
>> > address-space: I/O
>> >   0000000000000000-000000000000ffff (prio 0, i/o): io
>> >
>> > But it's fine to enable that I guess.
>> >
>> > (Which "info" command would you want to use during preconfig?)
>> >  
>> >> 
>> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>  
>> >
>> > Reviewed-by: Peter Xu <peterx@redhat.com>  
>> 
>> The reason for having -preconfig is us despairing of making -S do the
>> right thing.  We'd have to *understand* the tangled mess that is our
>> startup, and rearrange it so QMP becomes available early enough for
>> configuring NUMA (and other things), yet late enough for everything to
>> work.
> We solve concrete NUMA problem at -S time as was demonstrated by thread:
> "RFC v2 0/4] enable numa configuration before machine  is running from HMP/QMP"
> were pros and cons are were summarized. But that won't scale to other things.
>  
>> -preconfig is a cheap hack to avoid this headache, by bypassing almost
>> all of "everything".
> hack should allow to move configuration steps to early stage one by one
> until the point we could obsolete -S (from configuration point)
> without breaking current -S users and clean up start up mess in process.

Sometimes a hack is the only practical way forward.

>> Now you bring back some of "everything".  Dangerous.  You better show it
>> actually works.  Until you do:
> Tested v2, works as expected so far.

See my reply to Dave.

>> 
>> NAK
>> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-07 15:28           ` Dr. David Alan Gilbert
@ 2018-06-08  5:56             ` Markus Armbruster
  2018-06-08  8:18               ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 33+ messages in thread
From: Markus Armbruster @ 2018-06-08  5:56 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: imammedo, qemu-devel, Peter Xu

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> 
>> > * Markus Armbruster (armbru@redhat.com) wrote:
>> >> Peter Xu <peterx@redhat.com> writes:
>> >> 
>> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
>> >> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >> >> 
>> >> >> Allow a bunch of the info commands to be used in preconfig.
>> >> >> Could probably add most of them.
>> >> >
>> >> > I guess some of them may not work yet during preconfig.  E.g.:
>> >> >
>> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
>> >> > QEMU 2.12.50 monitor - type 'help' for more information
>> >> > (qemu) info mtree
>> >> > address-space: memory
>> >> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
>> >> >
>> >> > address-space: I/O
>> >> >   0000000000000000-000000000000ffff (prio 0, i/o): io
>> >> >
>> >> > But it's fine to enable that I guess.
>> >> >
>> >> > (Which "info" command would you want to use during preconfig?)
>> >> >
>> >> >> 
>> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> >> >
>> >> > Reviewed-by: Peter Xu <peterx@redhat.com>
>> >> 
>> >> The reason for having -preconfig is us despairing of making -S do the
>> >> right thing.  We'd have to *understand* the tangled mess that is our
>> >> startup, and rearrange it so QMP becomes available early enough for
>> >> configuring NUMA (and other things), yet late enough for everything to
>> >> work.
>> >> 
>> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost
>> >> all of "everything".
>> >> 
>> >> Now you bring back some of "everything".  Dangerous.  You better show it
>> >> actually works.  Until you do:
>> >> 
>> >> NAK
>> >
>> > Well I did test each command in here to make sure it didn't
>> > crash/produce complete junk; but here's the output with the v2 of this
>> > patch that Igor R-b:
>> [...]
>> 
>> For the sake of the argument, let's assume these commands all work in
>> preconfig state.  Are their QMP equivalents all available in preconfig
>> state?
>
> That I don't know; I was happy to fix my list to the ones
> Igor recommended.  If you object to some particular entries I'll
> be happy to change them.

HMP must not provide more functionality than QMP.  Specifically, we may
provide "info FOO" only when we also provide query-FOO.

There are exceptions to this rule.  I don't think they apply here.  I'm
prepared to discuss them, of course.

I wish there was a way to automate "provide command in HMP when its
buddy is available in QMP", but since the buddies are only connected by
code, that seems infeasible.

Without such automation, the two sets of available commands need to be
kept consistent manually.  The larger they are, the more of a bother.

Bother is fine when it provides commensurate value to users.  Options in
increasing order of value provided:

(1) HMP becomes ready only after we exit preconfig state (what I
    proposed in Message-ID: <87603cxob9.fsf@dusky.pond.sub.org>.

(2) HMP provides help, quit, exit-preconfig.

(3) HMP provides (a subset of) the commands QMP provides.

I figure the maintenance cost of (1) and (2) will be negligible, but (3)
could be a drag.  Are you sure it's worthwhile?

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-08  5:56             ` Markus Armbruster
@ 2018-06-08  8:18               ` Dr. David Alan Gilbert
  2018-06-08  9:30                 ` Igor Mammedov
  2018-06-08 16:51                 ` Markus Armbruster
  0 siblings, 2 replies; 33+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-08  8:18 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: imammedo, qemu-devel, Peter Xu

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> >> 
> >> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> >> Peter Xu <peterx@redhat.com> writes:
> >> >> 
> >> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> >> >> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> >> >> 
> >> >> >> Allow a bunch of the info commands to be used in preconfig.
> >> >> >> Could probably add most of them.
> >> >> >
> >> >> > I guess some of them may not work yet during preconfig.  E.g.:
> >> >> >
> >> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> >> >> > QEMU 2.12.50 monitor - type 'help' for more information
> >> >> > (qemu) info mtree
> >> >> > address-space: memory
> >> >> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >> >> >
> >> >> > address-space: I/O
> >> >> >   0000000000000000-000000000000ffff (prio 0, i/o): io
> >> >> >
> >> >> > But it's fine to enable that I guess.
> >> >> >
> >> >> > (Which "info" command would you want to use during preconfig?)
> >> >> >
> >> >> >> 
> >> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> >> >
> >> >> > Reviewed-by: Peter Xu <peterx@redhat.com>
> >> >> 
> >> >> The reason for having -preconfig is us despairing of making -S do the
> >> >> right thing.  We'd have to *understand* the tangled mess that is our
> >> >> startup, and rearrange it so QMP becomes available early enough for
> >> >> configuring NUMA (and other things), yet late enough for everything to
> >> >> work.
> >> >> 
> >> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost
> >> >> all of "everything".
> >> >> 
> >> >> Now you bring back some of "everything".  Dangerous.  You better show it
> >> >> actually works.  Until you do:
> >> >> 
> >> >> NAK
> >> >
> >> > Well I did test each command in here to make sure it didn't
> >> > crash/produce complete junk; but here's the output with the v2 of this
> >> > patch that Igor R-b:
> >> [...]
> >> 
> >> For the sake of the argument, let's assume these commands all work in
> >> preconfig state.  Are their QMP equivalents all available in preconfig
> >> state?
> >
> > That I don't know; I was happy to fix my list to the ones
> > Igor recommended.  If you object to some particular entries I'll
> > be happy to change them.
> 
> HMP must not provide more functionality than QMP.  Specifically, we may
> provide "info FOO" only when we also provide query-FOO.
>
> There are exceptions to this rule.  I don't think they apply here.  I'm
> prepared to discuss them, of course.

No, that's strictly not true;  HMP can provide anything that helps
a human debug stuff.  The requirement is that if a tool needs it then it
must be provided in QMP.

> I wish there was a way to automate "provide command in HMP when its
> buddy is available in QMP", but since the buddies are only connected by
> code, that seems infeasible.
> 
> Without such automation, the two sets of available commands need to be
> kept consistent manually.  The larger they are, the more of a bother.
> 
> Bother is fine when it provides commensurate value to users.  Options in
> increasing order of value provided:
> 
> (1) HMP becomes ready only after we exit preconfig state (what I
>     proposed in Message-ID: <87603cxob9.fsf@dusky.pond.sub.org>.
>
> (2) HMP provides help, quit, exit-preconfig.
> 
> (3) HMP provides (a subset of) the commands QMP provides.
> 
> I figure the maintenance cost of (1) and (2) will be negligible, but (3)
> could be a drag.  Are you sure it's worthwhile?


I'm not prepared to restrict to (2), and I'm not prepared to restrict
HMP to a subset of QMP;  As I said previously, if there's a command that
you think is incorrect/broken that I've enabled then I'm happy to
remove it.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-08  8:18               ` Dr. David Alan Gilbert
@ 2018-06-08  9:30                 ` Igor Mammedov
  2018-06-08 16:51                 ` Markus Armbruster
  1 sibling, 0 replies; 33+ messages in thread
From: Igor Mammedov @ 2018-06-08  9:30 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Markus Armbruster, qemu-devel, Peter Xu

On Fri, 8 Jun 2018 09:18:46 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Markus Armbruster (armbru@redhat.com) wrote:
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> >   
> > > * Markus Armbruster (armbru@redhat.com) wrote:  
> > >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> > >>   
> > >> > * Markus Armbruster (armbru@redhat.com) wrote:  
> > >> >> Peter Xu <peterx@redhat.com> writes:
> > >> >>   
> > >> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:  
> > >> >> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > >> >> >> 
> > >> >> >> Allow a bunch of the info commands to be used in preconfig.
> > >> >> >> Could probably add most of them.  
> > >> >> >
> > >> >> > I guess some of them may not work yet during preconfig.  E.g.:
> > >> >> >
> > >> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> > >> >> > QEMU 2.12.50 monitor - type 'help' for more information
> > >> >> > (qemu) info mtree
> > >> >> > address-space: memory
> > >> >> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
> > >> >> >
> > >> >> > address-space: I/O
> > >> >> >   0000000000000000-000000000000ffff (prio 0, i/o): io
> > >> >> >
> > >> >> > But it's fine to enable that I guess.
> > >> >> >
> > >> >> > (Which "info" command would you want to use during preconfig?)
> > >> >> >  
> > >> >> >> 
> > >> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>  
> > >> >> >
> > >> >> > Reviewed-by: Peter Xu <peterx@redhat.com>  
> > >> >> 
> > >> >> The reason for having -preconfig is us despairing of making -S do the
> > >> >> right thing.  We'd have to *understand* the tangled mess that is our
> > >> >> startup, and rearrange it so QMP becomes available early enough for
> > >> >> configuring NUMA (and other things), yet late enough for everything to
> > >> >> work.
> > >> >> 
> > >> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost
> > >> >> all of "everything".
> > >> >> 
> > >> >> Now you bring back some of "everything".  Dangerous.  You better show it
> > >> >> actually works.  Until you do:
> > >> >> 
> > >> >> NAK  
> > >> >
> > >> > Well I did test each command in here to make sure it didn't
> > >> > crash/produce complete junk; but here's the output with the v2 of this
> > >> > patch that Igor R-b:  
> > >> [...]
> > >> 
> > >> For the sake of the argument, let's assume these commands all work in
> > >> preconfig state.  Are their QMP equivalents all available in preconfig
> > >> state?  
> > >
> > > That I don't know; I was happy to fix my list to the ones
> > > Igor recommended.  If you object to some particular entries I'll
> > > be happy to change them.  
> > 
> > HMP must not provide more functionality than QMP.  Specifically, we may
> > provide "info FOO" only when we also provide query-FOO.
> >
> > There are exceptions to this rule.  I don't think they apply here.  I'm
> > prepared to discuss them, of course.  
> 
> No, that's strictly not true;  HMP can provide anything that helps
> a human debug stuff.  The requirement is that if a tool needs it then it
> must be provided in QMP.
> 
> > I wish there was a way to automate "provide command in HMP when its
> > buddy is available in QMP", but since the buddies are only connected by
> > code, that seems infeasible.
> > 
> > Without such automation, the two sets of available commands need to be
> > kept consistent manually.  The larger they are, the more of a bother.
> > 
> > Bother is fine when it provides commensurate value to users.  Options in
> > increasing order of value provided:
> > 
> > (1) HMP becomes ready only after we exit preconfig state (what I
> >     proposed in Message-ID: <87603cxob9.fsf@dusky.pond.sub.org>.
> >
> > (2) HMP provides help, quit, exit-preconfig.
> > 
> > (3) HMP provides (a subset of) the commands QMP provides.
> > 
> > I figure the maintenance cost of (1) and (2) will be negligible, but (3)
> > could be a drag.  Are you sure it's worthwhile?  
>
> 
> I'm not prepared to restrict to (2), and I'm not prepared to restrict
> HMP to a subset of QMP;  As I said previously, if there's a command that
> you think is incorrect/broken that I've enabled then I'm happy to
> remove it.
I'd prefer #3 if we going to expose HMP at all or #1.

#3 would allow testing via HMP for those who can't use qmp-shell.
we can trim HMP list to allowed-preconfig commands or audit respective
QMP variants (they should work without changes) and flag them with
allowed-preconfig. 

> 
> Dave
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-08  8:18               ` Dr. David Alan Gilbert
  2018-06-08  9:30                 ` Igor Mammedov
@ 2018-06-08 16:51                 ` Markus Armbruster
  2018-06-08 17:18                   ` Dr. David Alan Gilbert
  1 sibling, 1 reply; 33+ messages in thread
From: Markus Armbruster @ 2018-06-08 16:51 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Markus Armbruster, imammedo, qemu-devel, Peter Xu

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> 
>> > * Markus Armbruster (armbru@redhat.com) wrote:
>> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> >> 
>> >> > * Markus Armbruster (armbru@redhat.com) wrote:
>> >> >> Peter Xu <peterx@redhat.com> writes:
>> >> >> 
>> >> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
>> >> >> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >> >> >> 
>> >> >> >> Allow a bunch of the info commands to be used in preconfig.
>> >> >> >> Could probably add most of them.
>> >> >> >
>> >> >> > I guess some of them may not work yet during preconfig.  E.g.:
>> >> >> >
>> >> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
>> >> >> > QEMU 2.12.50 monitor - type 'help' for more information
>> >> >> > (qemu) info mtree
>> >> >> > address-space: memory
>> >> >> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
>> >> >> >
>> >> >> > address-space: I/O
>> >> >> >   0000000000000000-000000000000ffff (prio 0, i/o): io
>> >> >> >
>> >> >> > But it's fine to enable that I guess.
>> >> >> >
>> >> >> > (Which "info" command would you want to use during preconfig?)
>> >> >> >
>> >> >> >> 
>> >> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> >> >> >
>> >> >> > Reviewed-by: Peter Xu <peterx@redhat.com>
>> >> >> 
>> >> >> The reason for having -preconfig is us despairing of making -S do the
>> >> >> right thing.  We'd have to *understand* the tangled mess that is our
>> >> >> startup, and rearrange it so QMP becomes available early enough for
>> >> >> configuring NUMA (and other things), yet late enough for everything to
>> >> >> work.
>> >> >> 
>> >> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost
>> >> >> all of "everything".
>> >> >> 
>> >> >> Now you bring back some of "everything".  Dangerous.  You better show it
>> >> >> actually works.  Until you do:
>> >> >> 
>> >> >> NAK
>> >> >
>> >> > Well I did test each command in here to make sure it didn't
>> >> > crash/produce complete junk; but here's the output with the v2 of this
>> >> > patch that Igor R-b:
>> >> [...]
>> >> 
>> >> For the sake of the argument, let's assume these commands all work in
>> >> preconfig state.  Are their QMP equivalents all available in preconfig
>> >> state?
>> >
>> > That I don't know; I was happy to fix my list to the ones
>> > Igor recommended.  If you object to some particular entries I'll
>> > be happy to change them.
>> 
>> HMP must not provide more functionality than QMP.  Specifically, we may
>> provide "info FOO" only when we also provide query-FOO.
>>
>> There are exceptions to this rule.  I don't think they apply here.  I'm
>> prepared to discuss them, of course.
>
> No, that's strictly not true;  HMP can provide anything that helps
> a human debug stuff.

Yes, but...

>                       The requirement is that if a tool needs it then it
> must be provided in QMP.

... it must not do that at the expense of QMP's completeness.
Permitting that would be a break from how we worked since QMP became
supported in 0.14 or so.  Seven years, time flies.

The hard requirement for QMP from day one was "provide everything
machine clients need".  To avoid speculation and endless arguments about
what might be needed / not needed, we resolved to approximate this by
"provide everything, except stuff that's *clearly* of no use to
machines".  This has been spelled out on this list several times.
Here are two relatively recent iterations:

Message-ID: <8737cngvq6.fsf@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05309.html

Message-ID: <8760hrxtcn.fsf@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00247.html

Quoting the latter:

In general, functionality available in HMP should also available in QMP.
Exceptions include functionality that makes no sense in QMP, or is of
use only for human users.  If you think your command is an exception,
please explain why in the commit message.

If it isn't, you need to implement it for QMP (including suitable test
cases), then rewrite the HMP version to reuse either the QMP command or
a common core.

Example for "makes no sense in QMP": setting the current CPU[1], because
a QMP monitor doesn't have a current CPU.

Examples for "is of use only for human users": HMP command "help", the
integrated pocket calculator[2].

End quote.

But we're not even arguing about a new HMP command, we're arguing about
making existing commands available in preconfig state.  I assume that if
an HMP command is useful in preconfig state, then its QMP buddy is also
useful there.  I therefore asked you whether their QMP equivalents are
all available in preconfig state.  The answer I expected was a short
table of three columns: HMP command you propose to enable, QMP buddy if
any, whether the QMP buddy is available in preconfig state yes/no.
Estimated effort: minutes.  What I got instead of this data was an
argument about fundamental design decisions.  Fine, reexaminating
fundamentals once in a while isn't a bad idea.  But may I have my data
now?

[...]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
  2018-06-08 16:51                 ` Markus Armbruster
@ 2018-06-08 17:18                   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 33+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-08 17:18 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: imammedo, qemu-devel, Peter Xu

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> >> 
> >> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> >> >> 
> >> >> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> >> >> Peter Xu <peterx@redhat.com> writes:
> >> >> >> 
> >> >> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> >> >> >> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> >> >> >> 
> >> >> >> >> Allow a bunch of the info commands to be used in preconfig.
> >> >> >> >> Could probably add most of them.
> >> >> >> >
> >> >> >> > I guess some of them may not work yet during preconfig.  E.g.:
> >> >> >> >
> >> >> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio
> >> >> >> > QEMU 2.12.50 monitor - type 'help' for more information
> >> >> >> > (qemu) info mtree
> >> >> >> > address-space: memory
> >> >> >> >   0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >> >> >> >
> >> >> >> > address-space: I/O
> >> >> >> >   0000000000000000-000000000000ffff (prio 0, i/o): io
> >> >> >> >
> >> >> >> > But it's fine to enable that I guess.
> >> >> >> >
> >> >> >> > (Which "info" command would you want to use during preconfig?)
> >> >> >> >
> >> >> >> >> 
> >> >> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> >> >> >
> >> >> >> > Reviewed-by: Peter Xu <peterx@redhat.com>
> >> >> >> 
> >> >> >> The reason for having -preconfig is us despairing of making -S do the
> >> >> >> right thing.  We'd have to *understand* the tangled mess that is our
> >> >> >> startup, and rearrange it so QMP becomes available early enough for
> >> >> >> configuring NUMA (and other things), yet late enough for everything to
> >> >> >> work.
> >> >> >> 
> >> >> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost
> >> >> >> all of "everything".
> >> >> >> 
> >> >> >> Now you bring back some of "everything".  Dangerous.  You better show it
> >> >> >> actually works.  Until you do:
> >> >> >> 
> >> >> >> NAK
> >> >> >
> >> >> > Well I did test each command in here to make sure it didn't
> >> >> > crash/produce complete junk; but here's the output with the v2 of this
> >> >> > patch that Igor R-b:
> >> >> [...]
> >> >> 
> >> >> For the sake of the argument, let's assume these commands all work in
> >> >> preconfig state.  Are their QMP equivalents all available in preconfig
> >> >> state?
> >> >
> >> > That I don't know; I was happy to fix my list to the ones
> >> > Igor recommended.  If you object to some particular entries I'll
> >> > be happy to change them.
> >> 
> >> HMP must not provide more functionality than QMP.  Specifically, we may
> >> provide "info FOO" only when we also provide query-FOO.
> >>
> >> There are exceptions to this rule.  I don't think they apply here.  I'm
> >> prepared to discuss them, of course.
> >
> > No, that's strictly not true;  HMP can provide anything that helps
> > a human debug stuff.
> 
> Yes, but...
> 
> >                       The requirement is that if a tool needs it then it
> > must be provided in QMP.
> 
> ... it must not do that at the expense of QMP's completeness.
> Permitting that would be a break from how we worked since QMP became
> supported in 0.14 or so.  Seven years, time flies.
> 
> The hard requirement for QMP from day one was "provide everything
> machine clients need".  To avoid speculation and endless arguments about
> what might be needed / not needed, we resolved to approximate this by
> "provide everything, except stuff that's *clearly* of no use to
> machines".  This has been spelled out on this list several times.
> Here are two relatively recent iterations:
> 
> Message-ID: <8737cngvq6.fsf@dusky.pond.sub.org>
> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05309.html
> 
> Message-ID: <8760hrxtcn.fsf@dusky.pond.sub.org>
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00247.html
> 
> Quoting the latter:
> 
> In general, functionality available in HMP should also available in QMP.
> Exceptions include functionality that makes no sense in QMP, or is of
> use only for human users.  If you think your command is an exception,
> please explain why in the commit message.
> 
> If it isn't, you need to implement it for QMP (including suitable test
> cases), then rewrite the HMP version to reuse either the QMP command or
> a common core.
> 
> Example for "makes no sense in QMP": setting the current CPU[1], because
> a QMP monitor doesn't have a current CPU.
> 
> Examples for "is of use only for human users": HMP command "help", the
> integrated pocket calculator[2].
> 
> End quote.
> 
> But we're not even arguing about a new HMP command, we're arguing about
> making existing commands available in preconfig state.  I assume that if
> an HMP command is useful in preconfig state, then its QMP buddy is also
> useful there.  I therefore asked you whether their QMP equivalents are
> all available in preconfig state.  The answer I expected was a short
> table of three columns: HMP command you propose to enable, QMP buddy if
> any, whether the QMP buddy is available in preconfig state yes/no.
> Estimated effort: minutes.  What I got instead of this data was an
> argument about fundamental design decisions.  Fine, reexaminating
> fundamentals once in a while isn't a bad idea.  But may I have my data
> now?

I believe that table is in the v3 version patch 5.

Dave

> [...]
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2018-06-08 17:19 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-05 12:26 [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Dr. David Alan Gilbert (git)
2018-06-05 12:26 ` [Qemu-devel] [PATCH 1/6] hmp: Add flag for preconfig commands Dr. David Alan Gilbert (git)
2018-06-07  8:50   ` Peter Xu
2018-06-07 12:14   ` Markus Armbruster
2018-06-07 12:18     ` Dr. David Alan Gilbert
2018-06-05 12:26 ` [Qemu-devel] [PATCH 2/6] hmp: Allow help on " Dr. David Alan Gilbert (git)
2018-06-07  8:51   ` Peter Xu
2018-06-05 12:26 ` [Qemu-devel] [PATCH 3/6] hmp: Restrict auto-complete in preconfig Dr. David Alan Gilbert (git)
2018-06-07  8:52   ` Peter Xu
2018-06-05 12:26 ` [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig Dr. David Alan Gilbert (git)
2018-06-07  8:49   ` Peter Xu
2018-06-07 12:22     ` Markus Armbruster
2018-06-07 13:45       ` Dr. David Alan Gilbert
2018-06-07 15:12         ` Markus Armbruster
2018-06-07 15:28           ` Dr. David Alan Gilbert
2018-06-08  5:56             ` Markus Armbruster
2018-06-08  8:18               ` Dr. David Alan Gilbert
2018-06-08  9:30                 ` Igor Mammedov
2018-06-08 16:51                 ` Markus Armbruster
2018-06-08 17:18                   ` Dr. David Alan Gilbert
2018-06-07 15:08       ` Igor Mammedov
2018-06-08  5:41         ` Markus Armbruster
2018-06-07  9:39   ` Igor Mammedov
2018-06-05 12:26 ` [Qemu-devel] [PATCH 5/6] hmp: add exit_preconfig Dr. David Alan Gilbert (git)
2018-06-07  8:53   ` Peter Xu
2018-06-05 12:26 ` [Qemu-devel] [PATCH 6/6] hmp: Allow HMP in preconfig state again Dr. David Alan Gilbert (git)
2018-06-07  8:56   ` Peter Xu
2018-06-07  8:54 ` [Qemu-devel] [PATCH 0/6] Reenable hmp for preconfig mode Peter Xu
2018-06-07  9:00   ` Dr. David Alan Gilbert
2018-06-07  9:05     ` Peter Xu
2018-06-07  9:45   ` Igor Mammedov
2018-06-07 10:02     ` Peter Xu
2018-06-07  9:48 ` Igor Mammedov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.