[Qemu-devel] [PATCH v2 16/19] monitor: Decouple terminals

[Jan Kiszka]

Currently all registered (and activate) monitor terminals work in
broadcast mode: Everyone sees what someone else types on some other
terminal and what the monitor reports back. This model is broken when
you have a management monitor terminal that is automatically operated
and some other terminal used for independent guest inspection. Such
additional terminals can be multiplexed device channels or a gdb
frontend connected to QEMU's stub.

Therefore, this patch decouples the buffers and states of all monitor
terminals, allowing the user to operate them independently. It finally
starts to use the 'mon' parameter that was introduced earlier with the
API rework. It also defines the default monitor: the first instantance
that has the MONITOR_IS_DEFAULT flag set, and that is the monitor
created via the "-monitor" command line switch (or "vc" if none is
given).

As the patch requires to rework the monitor suspension interface, it
also takes the freedom to make it "truely" suspending (so far suspending
meant suppressing the prompt, but inputs were still processed).

Signed-off-by: Jan Kiszka <address@hidden>
---

 migration-exec.c |   12 +---
 migration-tcp.c  |   12 +---
 migration.c      |   13 +++--
 migration.h      |    4 +
 monitor.c        |  146 ++++++++++++++++++++++++++++--------------------------
 monitor.h        |    5 +-
 qemu-char.c      |    2 -
 vl.c             |    2 -
 8 files changed, 101 insertions(+), 95 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index e2e6e89..f0869a1 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -18,7 +18,6 @@
 #include "migration.h"
 #include "qemu-char.h"
 #include "sysemu.h"
-#include "monitor.h"
 #include "buffered_file.h"
 #include "block.h"
 
@@ -55,7 +54,7 @@ static int exec_close(FdMigrationState *s)
 
 MigrationState *exec_start_outgoing_migration(const char *command,
                                              int64_t bandwidth_limit,
-                                             int async)
+                                             int detach)
 {
     FdMigrationState *s;
     FILE *f;
@@ -89,14 +88,11 @@ MigrationState *exec_start_outgoing_migration(const char 
*command,
     s->mig_state.release = migrate_fd_release;
 
     s->state = MIG_STATE_ACTIVE;
-    s->detach = !async;
+    s->mon_resume = NULL;
     s->bandwidth_limit = bandwidth_limit;
 
-    if (s->detach == 1) {
-        dprintf("detaching from monitor\n");
-        monitor_suspend(cur_mon);
-        s->detach = 2;
-    }
+    if (!detach)
+        migrate_fd_monitor_suspend(s);
 
     migrate_fd_connect(s);
     return &s->mig_state;
diff --git a/migration-tcp.c b/migration-tcp.c
index 7f67fd9..d9c4c98 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -16,7 +16,6 @@
 #include "migration.h"
 #include "qemu-char.h"
 #include "sysemu.h"
-#include "monitor.h"
 #include "buffered_file.h"
 #include "block.h"
 
@@ -79,7 +78,7 @@ static void tcp_wait_for_connect(void *opaque)
 
 MigrationState *tcp_start_outgoing_migration(const char *host_port,
                                              int64_t bandwidth_limit,
-                                             int async)
+                                             int detach)
 {
     struct sockaddr_in addr;
     FdMigrationState *s;
@@ -98,7 +97,7 @@ MigrationState *tcp_start_outgoing_migration(const char 
*host_port,
     s->mig_state.release = migrate_fd_release;
 
     s->state = MIG_STATE_ACTIVE;
-    s->detach = !async;
+    s->mon_resume = NULL;
     s->bandwidth_limit = bandwidth_limit;
     s->fd = socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
@@ -108,11 +107,8 @@ MigrationState *tcp_start_outgoing_migration(const char 
*host_port,
 
     socket_set_nonblock(s->fd);
 
-    if (s->detach == 1) {
-        dprintf("detaching from monitor\n");
-        monitor_suspend(cur_mon);
-        s->detach = 2;
-    }
+    if (!detach)
+        migrate_fd_monitor_suspend(s);
 
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
diff --git a/migration.c b/migration.c
index cf69bbe..15639c1 100644
--- a/migration.c
+++ b/migration.c
@@ -125,6 +125,13 @@ void do_info_migrate(Monitor *mon)
 
 /* shared migration helpers */
 
+void migrate_fd_monitor_suspend(FdMigrationState *s)
+{
+    s->mon_resume = cur_mon;
+    monitor_suspend(cur_mon);
+    dprintf("suspending monitor\n");
+}
+
 void migrate_fd_error(FdMigrationState *s)
 {
     dprintf("setting error state\n");
@@ -145,10 +152,8 @@ void migrate_fd_cleanup(FdMigrationState *s)
         close(s->fd);
 
     /* Don't resume monitor until we've flushed all of the buffers */
-    if (s->detach == 2) {
-        monitor_resume(cur_mon);
-        s->detach = 0;
-    }
+    if (s->mon_resume)
+        monitor_resume(s->mon_resume);
 
     s->fd = -1;
 }
diff --git a/migration.h b/migration.h
index 32f5a72..696618d 100644
--- a/migration.h
+++ b/migration.h
@@ -39,7 +39,7 @@ struct FdMigrationState
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
-    int detach;
+    Monitor *mon_resume;
     int state;
     int (*get_error)(struct FdMigrationState*);
     int (*close)(struct FdMigrationState*);
@@ -69,6 +69,8 @@ MigrationState *tcp_start_outgoing_migration(const char 
*host_port,
                                             int64_t bandwidth_limit,
                                             int detach);
 
+void migrate_fd_monitor_suspend(FdMigrationState *s);
+
 void migrate_fd_error(FdMigrationState *s);
 
 void migrate_fd_cleanup(FdMigrationState *s);
diff --git a/monitor.c b/monitor.c
index 6940031..ba0e9a8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -69,6 +69,14 @@ typedef struct mon_cmd_t {
 
 struct Monitor {
     CharDriverState *chr;
+    int flags;
+    int suspend_cnt;
+    uint8_t outbuf[1024];
+    int outbuf_index;
+    ReadLineState *rs;
+    CPUState *mon_cpu;
+    BlockDriverCompletionFunc *password_completion_cb;
+    void *password_opaque;
     LIST_ENTRY(Monitor) entry;
 };
 
@@ -77,34 +85,30 @@ static LIST_HEAD(mon_list, Monitor) mon_list;
 static const mon_cmd_t mon_cmds[];
 static const mon_cmd_t info_cmds[];
 
-static uint8_t term_outbuf[1024];
-static int term_outbuf_index;
-static BlockDriverCompletionFunc *password_completion_cb;
-static void *password_opaque;
-ReadLineState *rs;
-
 Monitor *cur_mon = NULL;
 
-static void monitor_start_input(void);
+static void monitor_command_cb(Monitor *mon, const char *cmdline,
+                               void *opaque);
 
-static CPUState *mon_cpu = NULL;
+static void monitor_read_command(Monitor *mon, int show_prompt)
+{
+    readline_start(mon->rs, "(qemu) ", 0, monitor_command_cb, NULL);
+    if (show_prompt)
+        readline_show_prompt(mon->rs);
+}
 
 static void monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
                                   void *opaque)
 {
-    readline_start(rs, "Password: ", 1, readline_func, opaque);
+    readline_start(mon->rs, "Password: ", 1, readline_func, opaque);
+    /* prompt is printed on return from the command handler */
 }
 
 void monitor_flush(Monitor *mon)
 {
-    Monitor *m;
-
-    if (term_outbuf_index > 0) {
-        LIST_FOREACH(m, &mon_list, entry) {
-            if (m->chr->focus == 0)
-                qemu_chr_write(m->chr, term_outbuf, term_outbuf_index);
-        }
-        term_outbuf_index = 0;
+    if (mon && mon->outbuf_index != 0 && mon->chr->focus == 0) {
+        qemu_chr_write(mon->chr, mon->outbuf, mon->outbuf_index);
+        mon->outbuf_index = 0;
     }
 }
 
@@ -112,15 +116,19 @@ void monitor_flush(Monitor *mon)
 static void monitor_puts(Monitor *mon, const char *str)
 {
     char c;
+
+    if (!mon)
+        return;
+
     for(;;) {
         c = *str++;
         if (c == '\0')
             break;
         if (c == '\n')
-            term_outbuf[term_outbuf_index++] = '\r';
-        term_outbuf[term_outbuf_index++] = c;
-        if (term_outbuf_index >= (sizeof(term_outbuf) - 1) ||
-            c == '\n')
+            mon->outbuf[mon->outbuf_index++] = '\r';
+        mon->outbuf[mon->outbuf_index++] = c;
+        if (mon->outbuf_index >= (sizeof(mon->outbuf) - 1)
+            || c == '\n')
             monitor_flush(mon);
     }
 }
@@ -291,7 +299,7 @@ static int mon_set_cpu(int cpu_index)
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         if (env->cpu_index == cpu_index) {
-            mon_cpu = env;
+            cur_mon->mon_cpu = env;
             return 0;
         }
     }
@@ -300,10 +308,10 @@ static int mon_set_cpu(int cpu_index)
 
 static CPUState *mon_get_cpu(void)
 {
-    if (!mon_cpu) {
+    if (!cur_mon->mon_cpu) {
         mon_set_cpu(0);
     }
-    return mon_cpu;
+    return cur_mon->mon_cpu;
 }
 
 static void do_info_registers(Monitor *mon)
@@ -330,7 +338,7 @@ static void do_info_cpus(Monitor *mon)
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         monitor_printf(mon, "%c CPU #%d:",
-                       (env == mon_cpu) ? '*' : ' ',
+                       (env == mon->mon_cpu) ? '*' : ' ',
                        env->cpu_index);
 #if defined(TARGET_I386)
         monitor_printf(mon, " pc=0x" TARGET_FMT_lx,
@@ -367,7 +375,7 @@ static void do_info_history(Monitor *mon)
 
     i = 0;
     for(;;) {
-        str = readline_get_history(rs, i);
+        str = readline_get_history(mon->rs, i);
         if (!str)
             break;
         monitor_printf(mon, "%d: '%s'\n", i, str);
@@ -451,7 +459,7 @@ static void change_vnc_password_cb(Monitor *mon, const char 
*password,
     if (vnc_display_password(NULL, password) < 0)
         monitor_printf(mon, "could not set VNC server password\n");
 
-    monitor_start_input();
+    monitor_read_command(mon, 1);
 }
 
 static void do_change_vnc(Monitor *mon, const char *target, const char *arg)
@@ -2657,7 +2665,7 @@ static void cmd_completion(const char *name, const char 
*list)
         memcpy(cmd, pstart, len);
         cmd[len] = '\0';
         if (name[0] == '\0' || !strncmp(name, cmd, strlen(name))) {
-            readline_add_completion(rs, cmd);
+            readline_add_completion(cur_mon->rs, cmd);
         }
         if (*p == '\0')
             break;
@@ -2710,7 +2718,7 @@ static void file_completion(const char *input)
             stat(file, &sb);
             if(S_ISDIR(sb.st_mode))
                 pstrcat(file, sizeof(file), "/");
-            readline_add_completion(rs, file);
+            readline_add_completion(cur_mon->rs, file);
         }
     }
     closedir(ffs);
@@ -2723,7 +2731,7 @@ static void block_completion_it(void *opaque, 
BlockDriverState *bs)
 
     if (input[0] == '\0' ||
         !strncmp(name, (char *)input, strlen(input))) {
-        readline_add_completion(rs, name);
+        readline_add_completion(cur_mon->rs, name);
     }
 }
 
@@ -2783,7 +2791,7 @@ static void monitor_find_completion(const char *cmdline)
             cmdname = "";
         else
             cmdname = args[0];
-        readline_set_completion_index(rs, strlen(cmdname));
+        readline_set_completion_index(cur_mon->rs, strlen(cmdname));
         for(cmd = mon_cmds; cmd->name != NULL; cmd++) {
             cmd_completion(cmdname, cmd->name);
         }
@@ -2807,23 +2815,23 @@ static void monitor_find_completion(const char *cmdline)
         switch(*ptype) {
         case 'F':
             /* file completion */
-            readline_set_completion_index(rs, strlen(str));
+            readline_set_completion_index(cur_mon->rs, strlen(str));
             file_completion(str);
             break;
         case 'B':
             /* block device name completion */
-            readline_set_completion_index(rs, strlen(str));
+            readline_set_completion_index(cur_mon->rs, strlen(str));
             bdrv_iterate(block_completion_it, (void *)str);
             break;
         case 's':
             /* XXX: more generic ? */
             if (!strcmp(cmd->name, "info")) {
-                readline_set_completion_index(rs, strlen(str));
+                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")) {
-                readline_set_completion_index(rs, strlen(str));
+                readline_set_completion_index(cur_mon->rs, strlen(str));
                 for(key = key_defs; key->name != NULL; key++) {
                     cmd_completion(str, key->name);
                 }
@@ -2837,49 +2845,45 @@ static void monitor_find_completion(const char *cmdline)
         qemu_free(args[i]);
 }
 
-static int term_can_read(void *opaque)
+static int monitor_can_read(void *opaque)
 {
-    return 128;
+    Monitor *mon = opaque;
+
+    return (mon->suspend_cnt == 0) ? 128 : 0;
 }
 
-static void term_read(void *opaque, const uint8_t *buf, int size)
+static void monitor_read(void *opaque, const uint8_t *buf, int size)
 {
+    Monitor *old_mon = cur_mon;
     int i;
 
-    for(i = 0; i < size; i++)
-        readline_handle_byte(rs, buf[i]);
-}
+    cur_mon = opaque;
+
+    for (i = 0; i < size; i++)
+        readline_handle_byte(cur_mon->rs, buf[i]);
 
-static int monitor_suspended;
+    cur_mon = old_mon;
+}
 
 static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
 {
+    monitor_suspend(mon);
     monitor_handle_command(mon, cmdline);
-    if (!monitor_suspended)
-        readline_show_prompt(rs);
-    else
-        monitor_suspended = 2;
+    monitor_resume(mon);
 }
 
 void monitor_suspend(Monitor *mon)
 {
-    monitor_suspended = 1;
+    mon->suspend_cnt++;
 }
 
 void monitor_resume(Monitor *mon)
 {
-    if (monitor_suspended == 2)
-        monitor_start_input();
-    monitor_suspended = 0;
+    if (--mon->suspend_cnt == 0)
+        readline_show_prompt(mon->rs);
 }
 
-static void monitor_start_input(void)
-{
-    readline_start(rs, "(qemu) ", 0, monitor_command_cb, NULL);
-    readline_show_prompt(rs);
-}
-
-static void term_event(void *opaque, int event)
+static void monitor_event(void *opaque, int event)
 {
     Monitor *mon = opaque;
 
@@ -2888,13 +2892,12 @@ static void term_event(void *opaque, int event)
 
     monitor_printf(mon, "QEMU %s monitor - type 'help' for more information\n",
                    QEMU_VERSION);
-    monitor_start_input();
+    readline_show_prompt(mon->rs);
 }
 
-static int is_first_init = 1;
-
-void monitor_init(CharDriverState *chr)
+void monitor_init(CharDriverState *chr, int flags)
 {
+    static int is_first_init = 1;
     Monitor *mon;
 
     if (is_first_init) {
@@ -2905,15 +2908,16 @@ void monitor_init(CharDriverState *chr)
     mon = qemu_mallocz(sizeof(*mon));
 
     mon->chr = chr;
-    rs = readline_init(mon, monitor_find_completion);
+    mon->flags = flags;
+    mon->rs = readline_init(mon, monitor_find_completion);
+    monitor_read_command(mon, 0);
 
-    qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, mon);
+    qemu_chr_add_handlers(chr, monitor_can_read, monitor_read, monitor_event,
+                          mon);
 
     LIST_INSERT_HEAD(&mon_list, mon, entry);
-    if (!cur_mon)
+    if (!cur_mon || (flags & MONITOR_IS_DEFAULT))
         cur_mon = mon;
-
-    readline_start(rs, "", 0, monitor_command_cb, NULL);
 }
 
 static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)
@@ -2925,10 +2929,10 @@ static void bdrv_password_cb(Monitor *mon, const char 
*password, void *opaque)
         monitor_printf(mon, "invalid password\n");
         ret = -EPERM;
     }
-    if (password_completion_cb)
-        password_completion_cb(password_opaque, ret);
+    if (mon->password_completion_cb)
+        mon->password_completion_cb(mon->password_opaque, ret);
 
-    monitor_start_input();
+    monitor_read_command(mon, 1);
 }
 
 void monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
@@ -2944,8 +2948,8 @@ void monitor_read_bdrv_key_start(Monitor *mon, 
BlockDriverState *bs,
     monitor_printf(mon, "%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
                    bdrv_get_encrypted_filename(bs));
 
-    password_completion_cb = completion_cb;
-    password_opaque = opaque;
+    mon->password_completion_cb = completion_cb;
+    mon->password_opaque = opaque;
 
     monitor_read_password(mon, bdrv_password_cb, bs);
 }
diff --git a/monitor.h b/monitor.h
index abe7193..95a4060 100644
--- a/monitor.h
+++ b/monitor.h
@@ -7,7 +7,10 @@
 
 extern Monitor *cur_mon;
 
-void monitor_init(CharDriverState *chr);
+/* flags for monitor_init */
+#define MONITOR_IS_DEFAULT    0x01
+
+void monitor_init(CharDriverState *chr, int flags);
 
 void monitor_suspend(Monitor *mon);
 void monitor_resume(Monitor *mon);
diff --git a/qemu-char.c b/qemu-char.c
index bb50337..f815646 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2123,7 +2123,7 @@ CharDriverState *qemu_chr_open(const char *label, const 
char *filename, void (*i
         chr = qemu_chr_open(label, p, NULL);
         if (chr) {
             chr = qemu_chr_open_mux(chr);
-            monitor_init(chr);
+            monitor_init(chr, 0);
         } else {
             printf("Unable to open driver: %s\n", p);
         }
diff --git a/vl.c b/vl.c
index bbf96e0..4c2a448 100644
--- a/vl.c
+++ b/vl.c
@@ -5658,7 +5658,7 @@ int main(int argc, char **argv, char **envp)
     qemu_chr_initial_reset();
 
     if (monitor_device && monitor_hd)
-        monitor_init(monitor_hd);
+        monitor_init(monitor_hd, MONITOR_IS_DEFAULT);
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         const char *devname = serial_devices[i];