qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH  00/17] monitor rework
@ 2009-02-07 18:16 Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 01/17] block: Polish error handling of brdv_open2 Jan Kiszka
                   ` (16 more replies)
  0 siblings, 17 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

It started with a tiny patch to improve the various gdb pass-through
proposals that have been posted here before. But the deeper I dug, the
more issues of the monitor terminal concept and implementation showed
up. So this ended up as a major rework. I'm not claiming its perfect but
I think it's a significant improvement. Major contributions are:

 o complete fix for disk passwords
 o non-VM-blocking password retrieval
 o unlimited and fully decoupled monitor terminals
 o monitor pass-through via gdb
 o improved usability of mux'ed monitor terminals

I know some patches are huge and probably hard to review, and I already
worked hard on splitting things up into reasonable functional chunks.
But if someone finds a patch that could be broken into more useful
pieces, just let me know.

Thanks in advance for any review comments!


Find the patches also at git://git.kiszka.org/qemu.git queues/monitor

Jan Kiszka (17):
      block: Polish error handling of brdv_open2
      block: Improve bdrv_iterate
      block: Introduce bdrv_get_encrypted_filename
      monitor: Report encrypted disks in snapshot mode
      monitor: Don't change VNC server when disabled
      char-mux: Use separate input buffers
      monitor: Introduce monitor.h and readline.h
      monitor: Rework initial disk password retrieval
      monitor: Simplify password input mode
      monitor: Rework terminal management
      monitor: Drop banner hiding
      monitor: Rework modal password input
      monitor: Introduce ReadLineState
      monitor: Decouple terminals
      monitor: Improve mux'ed console experience
      monitor: Introduce MONITOR_USE_READLINE flag
      monitor: Pass-through for gdbstub

 audio/audio.c      |    6 +-
 audio/wavcapture.c |   20 +-
 block.c            |  157 ++++++++------
 block.h            |    8 +-
 block_int.h        |    1 +
 console.h          |   27 ---
 disas.c            |   14 +-
 gdbstub.c          |   54 +++++-
 hw/i8259.c         |   16 +-
 hw/pc.c            |    8 +-
 hw/pci.c           |   22 +-
 hw/slavio_intctl.c |   14 +-
 hw/sun4c_intctl.c  |   12 +-
 hw/usb-msd.c       |   10 +-
 hw/usb.h           |    4 +
 migration-exec.c   |   13 +-
 migration-tcp.c    |   13 +-
 migration.c        |   34 ++-
 migration.h        |    4 +-
 monitor.c          |  624 ++++++++++++++++++++++++++++++----------------------
 monitor.h          |   24 ++
 net.c              |   12 +-
 qemu-char.c        |   40 +++--
 qemu-char.h        |    8 +-
 qemu-tool.c        |    6 +-
 readline.c         |  406 +++++++++++++++++-----------------
 readline.h         |   47 ++++
 savevm.c           |   70 +++---
 slirp/misc.c       |    4 +-
 usb-linux.c        |   28 ++-
 vl.c               |  113 ++++------
 vnc.c              |   21 +-
 32 files changed, 1038 insertions(+), 802 deletions(-)
 create mode 100644 monitor.h
 create mode 100644 readline.h

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

* [Qemu-devel] [PATCH 01/17] block: Polish error handling of brdv_open2
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 05/17] monitor: Don't change VNC server when disabled Jan Kiszka
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Make sure that we always delete temporary disk images on error, remove
obsolete malloc error checks and return proper error codes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 block.c |   46 +++++++++++++++++++++-------------------------
 1 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/block.c b/block.c
index 4f4bf7c..330317a 100644
--- a/block.c
+++ b/block.c
@@ -311,8 +311,6 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
     int ret;
 
     bs = bdrv_new("");
-    if (!bs)
-        return -ENOMEM;
     ret = bdrv_open2(bs, filename, flags | BDRV_O_FILE, NULL);
     if (ret < 0) {
         bdrv_delete(bs);
@@ -348,12 +346,10 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
 
         /* if there is a backing file, use it */
         bs1 = bdrv_new("");
-        if (!bs1) {
-            return -ENOMEM;
-        }
-        if (bdrv_open(bs1, filename, 0) < 0) {
+        ret = bdrv_open(bs1, filename, 0);
+        if (ret < 0) {
             bdrv_delete(bs1);
-            return -1;
+            return ret;
         }
         total_size = bdrv_getlength(bs1) >> SECTOR_BITS;
 
@@ -371,9 +367,10 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
         else
             realpath(filename, backing_filename);
 
-        if (bdrv_create(&bdrv_qcow2, tmp_filename,
-                        total_size, backing_filename, 0) < 0) {
-            return -1;
+        ret = bdrv_create(&bdrv_qcow2, tmp_filename,
+                          total_size, backing_filename, 0);
+        if (ret < 0) {
+            return ret;
         }
         filename = tmp_filename;
         bs->is_temporary = 1;
@@ -382,14 +379,12 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     pstrcpy(bs->filename, sizeof(bs->filename), filename);
     if (flags & BDRV_O_FILE) {
         drv = find_protocol(filename);
-        if (!drv)
-            return -ENOENT;
-    } else {
-        if (!drv) {
-            drv = find_image_format(filename);
-            if (!drv)
-                return -1;
-        }
+    } else if (!drv) {
+        drv = find_image_format(filename);
+    }
+    if (!drv) {
+        ret = -ENOENT;
+        goto unlink_and_fail;
     }
     bs->drv = drv;
     bs->opaque = qemu_mallocz(drv->instance_size);
@@ -408,6 +403,9 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
         qemu_free(bs->opaque);
         bs->opaque = NULL;
         bs->drv = NULL;
+    unlink_and_fail:
+        if (bs->is_temporary)
+            unlink(filename);
         return ret;
     }
     if (drv->bdrv_getlength) {
@@ -421,15 +419,13 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     if (bs->backing_file[0] != '\0') {
         /* if there is a backing file, use it */
         bs->backing_hd = bdrv_new("");
-        if (!bs->backing_hd) {
-        fail:
-            bdrv_close(bs);
-            return -ENOMEM;
-        }
         path_combine(backing_filename, sizeof(backing_filename),
                      filename, bs->backing_file);
-        if (bdrv_open(bs->backing_hd, backing_filename, open_flags) < 0)
-            goto fail;
+        ret = bdrv_open(bs->backing_hd, backing_filename, open_flags);
+        if (ret < 0) {
+            bdrv_close(bs);
+            return ret;
+        }
     }
 
     /* call the change callback */

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

* [Qemu-devel] [PATCH 03/17] block: Introduce bdrv_get_encrypted_filename
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (3 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 06/17] char-mux: Use separate input buffers Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 09/17] monitor: Simplify password input mode Jan Kiszka
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Introduce bdrv_get_encrypted_filename service to allow more informative
password prompting.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 block.c |   10 ++++++++++
 block.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index d12a9aa..78982f4 100644
--- a/block.c
+++ b/block.c
@@ -1089,6 +1089,16 @@ void bdrv_info_stats (void)
     }
 }
 
+const char *bdrv_get_encrypted_filename(BlockDriverState *bs)
+{
+    if (bs->backing_hd && bs->backing_hd->encrypted)
+        return bs->backing_file;
+    else if (bs->encrypted)
+        return bs->filename;
+    else
+        return NULL;
+}
+
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size)
 {
diff --git a/block.h b/block.h
index aa26ef3..a01fa31 100644
--- a/block.h
+++ b/block.h
@@ -152,6 +152,7 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
                           const uint8_t *buf, int nb_sectors);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 
+const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
 int bdrv_snapshot_create(BlockDriverState *bs,

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

* [Qemu-devel] [PATCH 05/17] monitor: Don't change VNC server when disabled
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 01/17] block: Polish error handling of brdv_open2 Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-09 14:48   ` Anthony Liguori
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 02/17] block: Improve bdrv_iterate Jan Kiszka
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Avoid a segfault when the user issues 'change vnc' without having vnc
enabled on startup.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 vnc.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/vnc.c b/vnc.c
index 68df599..bdfc79b 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2333,6 +2333,8 @@ void vnc_display_close(DisplayState *ds)
 {
     VncState *vs = ds ? (VncState *)ds->opaque : vnc_state;
 
+    if (!vs)
+        return;
     if (vs->display) {
 	qemu_free(vs->display);
 	vs->display = NULL;
@@ -2392,6 +2394,8 @@ int vnc_display_open(DisplayState *ds, const char *display)
     int tls = 0, x509 = 0;
 #endif
 
+    if (!vnc_state)
+        return -1;
     vnc_display_close(ds);
     if (strcmp(display, "none") == 0)
 	return 0;

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

* [Qemu-devel] [PATCH 08/17] monitor: Rework initial disk password retrieval
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (6 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 07/17] monitor: Introduce monitor.h and readline.h Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 10/17] monitor: Rework terminal management Jan Kiszka
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Reading the passwords for encrypted hard disks during early startup is
broken (I guess for quiet a while now):
 - No monitor terminal is ready for input at this point
 - Forcing all mux'ed terminals into monitor mode can confuse other
   users of that channels

To overcome these issues and to lay the ground for a clean decoupling of
monitor terminals, this patch changes the initial password retrieval as
follows:
 - Prevent autostart if there is some encrypted disk
 - Once the user tries to resume the VM, prompt for all missing
   passwords
 - Only resume if all passwords were accepted

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 block.c      |   14 ++++++++++-
 block.h      |    4 ++-
 block_int.h  |    1 +
 hw/usb-msd.c |    6 ++---
 hw/usb.h     |    5 +++-
 monitor.c    |   44 ++++++++++++++++++++++++++++++++---
 monitor.h    |    4 +--
 vl.c         |   73 +++++++++++++++++-----------------------------------------
 8 files changed, 85 insertions(+), 66 deletions(-)

diff --git a/block.c b/block.c
index 5d93de1..3481988 100644
--- a/block.c
+++ b/block.c
@@ -335,6 +335,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     bs->read_only = 0;
     bs->is_temporary = 0;
     bs->encrypted = 0;
+    bs->valid_key = 0;
 
     if (flags & BDRV_O_SNAPSHOT) {
         BlockDriverState *bs1;
@@ -921,6 +922,15 @@ int bdrv_is_encrypted(BlockDriverState *bs)
     return bs->encrypted;
 }
 
+int bdrv_key_required(BlockDriverState *bs)
+{
+    BlockDriverState *backing_hd = bs->backing_hd;
+
+    if (backing_hd && backing_hd->encrypted && !backing_hd->valid_key)
+        return 1;
+    return (bs->encrypted && !bs->valid_key);
+}
+
 int bdrv_set_key(BlockDriverState *bs, const char *key)
 {
     int ret;
@@ -933,7 +943,9 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
     }
     if (!bs->encrypted || !bs->drv || !bs->drv->bdrv_set_key)
         return -1;
-    return bs->drv->bdrv_set_key(bs, key);
+    ret = bs->drv->bdrv_set_key(bs, key);
+    bs->valid_key = (ret == 0);
+    return ret;
 }
 
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size)
diff --git a/block.h b/block.h
index a01fa31..5c6eaf9 100644
--- a/block.h
+++ b/block.h
@@ -103,8 +103,6 @@ BlockDriverAIOCB *bdrv_aio_write(BlockDriverState *bs, int64_t sector_num,
                                  BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
 
-int qemu_key_check(BlockDriverState *bs, const char *name);
-
 /* Ensure contents are flushed to disk.  */
 void bdrv_flush(BlockDriverState *bs);
 void bdrv_flush_all(void);
@@ -144,7 +142,9 @@ BlockDriverState *bdrv_find(const char *name);
 void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs),
                   void *opaque);
 int bdrv_is_encrypted(BlockDriverState *bs);
+int bdrv_key_required(BlockDriverState *bs);
 int bdrv_set_key(BlockDriverState *bs, const char *key);
+int bdrv_query_missing_keys(void);
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
                          void *opaque);
 const char *bdrv_get_device_name(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index e83fd2c..0b7c8fb 100644
--- a/block_int.h
+++ b/block_int.h
@@ -96,6 +96,7 @@ struct BlockDriverState {
     int removable; /* if true, the media can be removed */
     int locked;    /* if true, the media cannot temporarily be ejected */
     int encrypted; /* if true, the media is encrypted */
+    int valid_key; /* if true, a valid encryption key has been set */
     int sg;        /* if true, the device is a /dev/sg* */
     /* event callback when inserting/removing */
     void (*change_cb)(void *opaque);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 342b0e8..2b0338d 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -11,6 +11,7 @@
 #include "usb.h"
 #include "block.h"
 #include "scsi-disk.h"
+#include "monitor.h"
 
 //#define DEBUG_MSD
 
@@ -513,7 +514,7 @@ static void usb_msd_handle_destroy(USBDevice *dev)
     qemu_free(s);
 }
 
-USBDevice *usb_msd_init(const char *filename)
+USBDevice *usb_msd_init(const char *filename, BlockDriverState **pbs)
 {
     MSDState *s;
     BlockDriverState *bdrv;
@@ -552,9 +553,8 @@ USBDevice *usb_msd_init(const char *filename)
     bdrv = bdrv_new("usb");
     if (bdrv_open2(bdrv, filename, 0, drv) < 0)
         goto fail;
-    if (qemu_key_check(bdrv, filename))
-        goto fail;
     s->bs = bdrv;
+    *pbs = bdrv;
 
     s->dev.speed = USB_SPEED_FULL;
     s->dev.handle_packet = usb_generic_handle_packet;
diff --git a/hw/usb.h b/hw/usb.h
index 4204808..4cd832d 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -21,6 +21,9 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+#include "block.h"
+
 #define USB_TOKEN_SETUP 0x2d
 #define USB_TOKEN_IN    0x69 /* device -> host */
 #define USB_TOKEN_OUT   0xe1 /* host -> device */
@@ -250,7 +253,7 @@ USBDevice *usb_keyboard_init(void);
 void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *));
 
 /* usb-msd.c */
-USBDevice *usb_msd_init(const char *filename);
+USBDevice *usb_msd_init(const char *filename, BlockDriverState **pbs);
 
 /* usb-net.c */
 USBDevice *usb_net_init(NICInfo *nd);
diff --git a/monitor.c b/monitor.c
index 61cea11..68bc5e9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -78,6 +78,8 @@ static uint8_t term_outbuf[1024];
 static int term_outbuf_index;
 
 static void monitor_start_input(void);
+static void monitor_readline(const char *prompt, int is_password,
+                             char *buf, int buf_size);
 
 static CPUState *mon_cpu = NULL;
 
@@ -438,7 +440,7 @@ static void do_change_block(const char *device, const char *filename, const char
     if (eject_device(bs, 0) < 0)
         return;
     bdrv_open2(bs, filename, 0, drv);
-    qemu_key_check(bs, filename);
+    monitor_read_bdrv_key(bs);
 }
 
 static void do_change_vnc(const char *target, const char *arg)
@@ -499,9 +501,24 @@ static void do_stop(void)
     vm_stop(EXCP_INTERRUPT);
 }
 
+static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
+{
+    int *err = opaque;
+
+    if (bdrv_key_required(bs))
+        *err = monitor_read_bdrv_key(bs);
+    else
+        *err = 0;
+}
+
 static void do_cont(void)
 {
-    vm_start();
+    int err = 0;
+
+    bdrv_iterate(encrypted_bdrv_it, &err);
+    /* only resume the vm if all keys are set and valid */
+    if (!err)
+        vm_start();
 }
 
 #ifdef CONFIG_GDBSTUB
@@ -2856,8 +2873,8 @@ static void monitor_readline_cb(void *opaque, const char *input)
     monitor_readline_started = 0;
 }
 
-void monitor_readline(const char *prompt, int is_password,
-                      char *buf, int buf_size)
+static void monitor_readline(const char *prompt, int is_password,
+                             char *buf, int buf_size)
 {
     int i;
     int old_focus[MAX_MON];
@@ -2887,3 +2904,22 @@ void monitor_readline(const char *prompt, int is_password,
                 monitor_hd[i]->focus = old_focus[i];
     }
 }
+
+int monitor_read_bdrv_key(BlockDriverState *bs)
+{
+    char password[256];
+    int i;
+
+    if (!bdrv_is_encrypted(bs))
+        return 0;
+
+    monitor_printf("%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
+                   bdrv_get_encrypted_filename(bs));
+    for(i = 0; i < 3; i++) {
+        monitor_readline("Password: ", 1, password, sizeof(password));
+        if (bdrv_set_key(bs, password) == 0)
+            return 0;
+        monitor_printf("invalid password\n");
+    }
+    return -EPERM;
+}
diff --git a/monitor.h b/monitor.h
index 973c336..8f993f1 100644
--- a/monitor.h
+++ b/monitor.h
@@ -5,12 +5,10 @@
 #include "block.h"
 
 void monitor_init(CharDriverState *hd, int show_banner);
-void monitor_readline(const char *prompt, int is_password,
-                      char *buf, int buf_size);
 void monitor_suspend(void);
 void monitor_resume(void);
 
-int monitor_read_bdrv_key(BlockDriverState *bs, const char *name);
+int monitor_read_bdrv_key(BlockDriverState *bs);
 
 void monitor_vprintf(const char *fmt, va_list ap);
 void monitor_printf(const char *fmt, ...)
diff --git a/vl.c b/vl.c
index b00d009..f8f049a 100644
--- a/vl.c
+++ b/vl.c
@@ -194,6 +194,7 @@ ram_addr_t ram_size;
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+static int autostart;
 static int rtc_utc = 1;
 static int rtc_date_offset = -1; /* -1 means no change */
 int cirrus_vga_enabled = 1;
@@ -2549,11 +2550,13 @@ static int drive_init(struct drive_opt *arg, int snapshot,
         bdrv_flags |= BDRV_O_CACHE_WB;
     else if (cache == 3) /* not specified */
         bdrv_flags |= BDRV_O_CACHE_DEF;
-    if (bdrv_open2(bdrv, file, bdrv_flags, drv) < 0 || qemu_key_check(bdrv, file)) {
+    if (bdrv_open2(bdrv, file, bdrv_flags, drv) < 0) {
         fprintf(stderr, "qemu: could not open disk image %s\n",
                         file);
         return -1;
     }
+    if (bdrv_key_required(bdrv))
+        autostart = 0;
     return 0;
 }
 
@@ -2600,7 +2603,7 @@ int usb_device_add_dev(USBDevice *dev)
     return 0;
 }
 
-static int usb_device_add(const char *devname)
+static int usb_device_add(const char *devname, int is_hotplug)
 {
     const char *p;
     USBDevice *dev;
@@ -2617,7 +2620,18 @@ static int usb_device_add(const char *devname)
     } else if (!strcmp(devname, "keyboard")) {
         dev = usb_keyboard_init();
     } else if (strstart(devname, "disk:", &p)) {
-        dev = usb_msd_init(p);
+        BlockDriverState *bs;
+
+        dev = usb_msd_init(p, &bs);
+        if (!dev)
+            return -1;
+        if (bdrv_key_required(bs)) {
+            autostart = 0;
+            if (is_hotplug && monitor_read_bdrv_key(bs) < 0) {
+                dev->handle_destroy(dev);
+                return -1;
+            }
+        }
     } else if (!strcmp(devname, "wacom-tablet")) {
         dev = usb_wacom_init();
     } else if (strstart(devname, "serial:", &p)) {
@@ -2698,7 +2712,7 @@ static int usb_device_del(const char *devname)
 
 void do_usb_add(const char *devname)
 {
-    usb_device_add(devname);
+    usb_device_add(devname, 1);
 }
 
 void do_usb_del(const char *devname)
@@ -4263,45 +4277,6 @@ static const QEMUOption qemu_options[] = {
     { NULL },
 };
 
-/* password input */
-
-int qemu_key_check(BlockDriverState *bs, const char *name)
-{
-    char password[256];
-    int i;
-
-    if (!bdrv_is_encrypted(bs))
-        return 0;
-
-    monitor_printf("%s is encrypted.\n", name);
-    for(i = 0; i < 3; i++) {
-        monitor_readline("Password: ", 1, password, sizeof(password));
-        if (bdrv_set_key(bs, password) == 0)
-            return 0;
-        monitor_printf("invalid password\n");
-    }
-    return -EPERM;
-}
-
-static BlockDriverState *get_bdrv(int index)
-{
-    if (index > nb_drives)
-        return NULL;
-    return drives_table[index].bdrv;
-}
-
-static void read_passwords(void)
-{
-    BlockDriverState *bs;
-    int i;
-
-    for(i = 0; i < 6; i++) {
-        bs = get_bdrv(i);
-        if (bs)
-            qemu_key_check(bs, bdrv_get_device_name(bs));
-    }
-}
-
 #ifdef HAS_AUDIO
 struct soundhw soundhw[] = {
 #ifdef HAS_AUDIO_CHOICE
@@ -4568,7 +4543,6 @@ int main(int argc, char **argv, char **envp)
     int fds[2];
     int tb_size;
     const char *pid_file = NULL;
-    int autostart;
     const char *incoming = NULL;
 
     qemu_cache_utils_init(envp);
@@ -5551,7 +5525,7 @@ int main(int argc, char **argv, char **envp)
     /* init USB devices */
     if (usb_enabled) {
         for(i = 0; i < usb_devices_index; i++) {
-            if (usb_device_add(usb_devices[i]) < 0) {
+            if (usb_device_add(usb_devices[i], 0) < 0) {
                 fprintf(stderr, "Warning: could not add USB device %s\n",
                         usb_devices[i]);
             }
@@ -5661,13 +5635,8 @@ int main(int argc, char **argv, char **envp)
         qemu_start_incoming_migration(incoming);
     }
 
-    {
-        /* XXX: simplify init */
-        read_passwords();
-        if (autostart) {
-            vm_start();
-        }
-    }
+    if (autostart)
+        vm_start();
 
     if (daemonize) {
 	uint8_t status = 0;

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

* [Qemu-devel] [PATCH 04/17] monitor: Report encrypted disks in snapshot mode
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (8 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 10/17] monitor: Rework terminal management Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-09 14:47   ` Anthony Liguori
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 17/17] monitor: Pass-through for gdbstub Jan Kiszka
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

If the backing file is encrypted, 'info block' currently does not report
the disk as encrypted. Fix this by used the standard API to check disk
encryption mode.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 block.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 78982f4..82c2016 100644
--- a/block.c
+++ b/block.c
@@ -1056,7 +1056,7 @@ void bdrv_info(void)
 	    }
             term_printf(" ro=%d", bs->read_only);
             term_printf(" drv=%s", bs->drv->format_name);
-            if (bs->encrypted)
+            if (bdrv_is_encrypted(bs))
                 term_printf(" encrypted");
         } else {
             term_printf(" [not inserted]");

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

* [Qemu-devel] [PATCH 02/17] block: Improve bdrv_iterate
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 01/17] block: Polish error handling of brdv_open2 Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 05/17] monitor: Don't change VNC server when disabled Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-09 14:45   ` Anthony Liguori
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 06/17] char-mux: Use separate input buffers Jan Kiszka
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Make bdrv_iterate more useful by passing the BlockDriverState to the
iterator instead of the device name.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 block.c   |    4 ++--
 block.h   |    3 ++-
 monitor.c |    3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 330317a..d12a9aa 100644
--- a/block.c
+++ b/block.c
@@ -966,12 +966,12 @@ BlockDriverState *bdrv_find(const char *name)
     return NULL;
 }
 
-void bdrv_iterate(void (*it)(void *opaque, const char *name), void *opaque)
+void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque)
 {
     BlockDriverState *bs;
 
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
-        it(opaque, bs->device_name);
+        it(opaque, bs);
     }
 }
 
diff --git a/block.h b/block.h
index e1927dd..aa26ef3 100644
--- a/block.h
+++ b/block.h
@@ -141,7 +141,8 @@ void bdrv_set_change_cb(BlockDriverState *bs,
                         void (*change_cb)(void *opaque), void *opaque);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
 BlockDriverState *bdrv_find(const char *name);
-void bdrv_iterate(void (*it)(void *opaque, const char *name), void *opaque);
+void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs),
+                  void *opaque);
 int bdrv_is_encrypted(BlockDriverState *bs);
 int bdrv_set_key(BlockDriverState *bs, const char *key);
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
diff --git a/monitor.c b/monitor.c
index 354a792..9643e44 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2633,8 +2633,9 @@ static void file_completion(const char *input)
     closedir(ffs);
 }
 
-static void block_completion_it(void *opaque, const char *name)
+static void block_completion_it(void *opaque, BlockDriverState *bs)
 {
+    const char *name = bdrv_get_device_name(bs);
     const char *input = opaque;
 
     if (input[0] == '\0' ||

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

* [Qemu-devel] [PATCH 09/17] monitor: Simplify password input mode
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (4 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 03/17] block: Introduce bdrv_get_encrypted_filename Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 07/17] monitor: Introduce monitor.h and readline.h Jan Kiszka
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Drop the hack to query passwords on all monitor terminals now that they
are requested when the user initially enters 'continue'.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 monitor.c |   20 --------------------
 1 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/monitor.c b/monitor.c
index 68bc5e9..cc814d5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2876,20 +2876,6 @@ static void monitor_readline_cb(void *opaque, const char *input)
 static void monitor_readline(const char *prompt, int is_password,
                              char *buf, int buf_size)
 {
-    int i;
-    int old_focus[MAX_MON];
-
-    if (is_password) {
-        for (i = 0; i < MAX_MON; i++) {
-            old_focus[i] = 0;
-            if (monitor_hd[i]) {
-                old_focus[i] = monitor_hd[i]->focus;
-                monitor_hd[i]->focus = 0;
-                qemu_chr_send_event(monitor_hd[i], CHR_EVENT_FOCUS);
-            }
-        }
-    }
-
     readline_start(prompt, is_password, monitor_readline_cb, NULL);
     monitor_readline_buf = buf;
     monitor_readline_buf_size = buf_size;
@@ -2897,12 +2883,6 @@ static void monitor_readline(const char *prompt, int is_password,
     while (monitor_readline_started) {
         main_loop_wait(10);
     }
-    /* restore original focus */
-    if (is_password) {
-        for (i = 0; i < MAX_MON; i++)
-            if (old_focus[i])
-                monitor_hd[i]->focus = old_focus[i];
-    }
 }
 
 int monitor_read_bdrv_key(BlockDriverState *bs)

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

* [Qemu-devel] [PATCH 10/17] monitor: Rework terminal management
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (7 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 08/17] monitor: Rework initial disk password retrieval Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 04/17] monitor: Report encrypted disks in snapshot mode Jan Kiszka
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Remove the static MAX_MON limit by managing monitor terminals in a
linked list.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 monitor.c |   41 +++++++++++++++++++++--------------------
 monitor.h |    2 +-
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/monitor.c b/monitor.c
index cc814d5..c21ea8a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -67,8 +67,12 @@ typedef struct term_cmd_t {
     const char *help;
 } term_cmd_t;
 
-#define MAX_MON 4
-static CharDriverState *monitor_hd[MAX_MON];
+typedef struct MonitorTerm {
+    CharDriverState *chr;
+    LIST_ENTRY(MonitorTerm) entry;
+} MonitorTerm;
+
+static LIST_HEAD(term_list, MonitorTerm) term_list;
 static int hide_banner;
 
 static const term_cmd_t term_cmds[];
@@ -85,11 +89,13 @@ static CPUState *mon_cpu = NULL;
 
 void monitor_flush(void)
 {
-    int i;
+    MonitorTerm *term;
+
     if (term_outbuf_index > 0) {
-        for (i = 0; i < MAX_MON; i++)
-            if (monitor_hd[i] && monitor_hd[i]->focus == 0)
-                qemu_chr_write(monitor_hd[i], term_outbuf, term_outbuf_index);
+        LIST_FOREACH(term, &term_list, entry) {
+            if (term->chr->focus == 0)
+                qemu_chr_write(term->chr, term_outbuf, term_outbuf_index);
+        }
         term_outbuf_index = 0;
     }
 }
@@ -2834,29 +2840,24 @@ static void term_event(void *opaque, int event)
 
 static int is_first_init = 1;
 
-void monitor_init(CharDriverState *hd, int show_banner)
+void monitor_init(CharDriverState *chr, int show_banner)
 {
-    int i;
+    MonitorTerm *term;
 
     if (is_first_init) {
         key_timer = qemu_new_timer(vm_clock, release_keys, NULL);
-        if (!key_timer)
-            return;
-        for (i = 0; i < MAX_MON; i++) {
-            monitor_hd[i] = NULL;
-        }
         is_first_init = 0;
     }
-    for (i = 0; i < MAX_MON; i++) {
-        if (monitor_hd[i] == NULL) {
-            monitor_hd[i] = hd;
-            break;
-        }
-    }
+
+    term = qemu_mallocz(sizeof(*term));
 
     hide_banner = !show_banner;
 
-    qemu_chr_add_handlers(hd, term_can_read, term_read, term_event, NULL);
+    term->chr = chr;
+
+    qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, NULL);
+
+    LIST_INSERT_HEAD(&term_list, term, entry);
 
     readline_start("", 0, monitor_handle_command1, NULL);
 }
diff --git a/monitor.h b/monitor.h
index 8f993f1..226f76c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -4,7 +4,7 @@
 #include "qemu-char.h"
 #include "block.h"
 
-void monitor_init(CharDriverState *hd, int show_banner);
+void monitor_init(CharDriverState *chr, int show_banner);
 void monitor_suspend(void);
 void monitor_resume(void);
 

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

* [Qemu-devel] [PATCH 07/17] monitor: Introduce monitor.h and readline.h
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (5 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 09/17] monitor: Simplify password input mode Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 08/17] monitor: Rework initial disk password retrieval Jan Kiszka
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Push monitor-related prototypes from console.h to a new monitor.h and
update users. Do the same for readline types and function prototypes. To
clarify that term_printf & friends work against the monitor terminal,
change the prefix of those functions to 'monitor_'. This patch comes
without functional changes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 audio/audio.c      |    6 +
 audio/wavcapture.c |   20 ++-
 block.c            |   62 +++++------
 console.h          |   27 -----
 disas.c            |   14 +-
 hw/i8259.c         |   16 +--
 hw/pc.c            |    8 +
 hw/pci.c           |   22 ++--
 hw/slavio_intctl.c |   14 +-
 hw/sun4c_intctl.c  |   12 +-
 migration-exec.c   |    2 
 migration-tcp.c    |    2 
 migration.c        |   18 ++-
 monitor.c          |  300 +++++++++++++++++++++++++++-------------------------
 monitor.h          |   21 ++++
 net.c              |   12 +-
 qemu-char.c        |    3 -
 qemu-tool.c        |    6 +
 readline.c         |   29 +++--
 readline.h         |   14 ++
 savevm.c           |   70 ++++++------
 slirp/misc.c       |    4 -
 usb-linux.c        |   28 +++--
 vl.c               |   33 +++---
 vnc.c              |   17 ++-
 25 files changed, 392 insertions(+), 368 deletions(-)
 create mode 100644 monitor.h
 create mode 100644 readline.h

diff --git a/audio/audio.c b/audio/audio.c
index b0a5f3b..9bd0025 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -23,7 +23,7 @@
  */
 #include "hw/hw.h"
 #include "audio.h"
-#include "console.h"
+#include "monitor.h"
 #include "qemu-timer.h"
 #include "sysemu.h"
 
@@ -328,10 +328,10 @@ void AUD_vlog (const char *cap, const char *fmt, va_list ap)
 {
     if (conf.log_to_monitor) {
         if (cap) {
-            term_printf ("%s: ", cap);
+            monitor_printf ("%s: ", cap);
         }
 
-        term_vprintf (fmt, ap);
+        monitor_vprintf (fmt, ap);
     }
     else {
         if (cap) {
diff --git a/audio/wavcapture.c b/audio/wavcapture.c
index 6d1c441..a0481c5 100644
--- a/audio/wavcapture.c
+++ b/audio/wavcapture.c
@@ -1,5 +1,5 @@
 #include "hw/hw.h"
-#include "console.h"
+#include "monitor.h"
 #include "audio.h"
 
 typedef struct {
@@ -71,9 +71,9 @@ static void wav_capture_info (void *opaque)
     WAVState *wav = opaque;
     char *path = wav->path;
 
-    term_printf ("Capturing audio(%d,%d,%d) to %s: %d bytes\n",
-                 wav->freq, wav->bits, wav->nchannels,
-                 path ? path : "<not available>", wav->bytes);
+    monitor_printf("Capturing audio(%d,%d,%d) to %s: %d bytes\n",
+                   wav->freq, wav->bits, wav->nchannels,
+                   path ? path : "<not available>", wav->bytes);
 }
 
 static struct capture_ops wav_capture_ops = {
@@ -97,13 +97,13 @@ int wav_start_capture (CaptureState *s, const char *path, int freq,
     CaptureVoiceOut *cap;
 
     if (bits != 8 && bits != 16) {
-        term_printf ("incorrect bit count %d, must be 8 or 16\n", bits);
+        monitor_printf("incorrect bit count %d, must be 8 or 16\n", bits);
         return -1;
     }
 
     if (nchannels != 1 && nchannels != 2) {
-        term_printf ("incorrect channel count %d, must be 1 or 2\n",
-                     nchannels);
+        monitor_printf("incorrect channel count %d, must be 1 or 2\n",
+                       nchannels);
         return -1;
     }
 
@@ -131,8 +131,8 @@ int wav_start_capture (CaptureState *s, const char *path, int freq,
 
     wav->f = qemu_fopen (path, "wb");
     if (!wav->f) {
-        term_printf ("Failed to open wave file `%s'\nReason: %s\n",
-                     path, strerror (errno));
+        monitor_printf("Failed to open wave file `%s'\nReason: %s\n",
+                       path, strerror (errno));
         qemu_free (wav);
         return -1;
     }
@@ -146,7 +146,7 @@ int wav_start_capture (CaptureState *s, const char *path, int freq,
 
     cap = AUD_add_capture (NULL, &as, &ops, wav);
     if (!cap) {
-        term_printf ("Failed to add audio capture\n");
+        monitor_printf("Failed to add audio capture\n");
         qemu_free (wav->path);
         qemu_fclose (wav->f);
         qemu_free (wav);
diff --git a/block.c b/block.c
index 82c2016..5d93de1 100644
--- a/block.c
+++ b/block.c
@@ -28,7 +28,7 @@
 #endif
 
 #include "qemu-common.h"
-#include "console.h"
+#include "monitor.h"
 #include "block_int.h"
 
 #ifdef _BSD
@@ -1030,38 +1030,38 @@ void bdrv_info(void)
     BlockDriverState *bs;
 
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
-        term_printf("%s:", bs->device_name);
-        term_printf(" type=");
+        monitor_printf("%s:", bs->device_name);
+        monitor_printf(" type=");
         switch(bs->type) {
         case BDRV_TYPE_HD:
-            term_printf("hd");
+            monitor_printf("hd");
             break;
         case BDRV_TYPE_CDROM:
-            term_printf("cdrom");
+            monitor_printf("cdrom");
             break;
         case BDRV_TYPE_FLOPPY:
-            term_printf("floppy");
+            monitor_printf("floppy");
             break;
         }
-        term_printf(" removable=%d", bs->removable);
+        monitor_printf(" removable=%d", bs->removable);
         if (bs->removable) {
-            term_printf(" locked=%d", bs->locked);
+            monitor_printf(" locked=%d", bs->locked);
         }
         if (bs->drv) {
-            term_printf(" file=");
-	    term_print_filename(bs->filename);
+            monitor_printf(" file=");
+            monitor_print_filename(bs->filename);
             if (bs->backing_file[0] != '\0') {
-                term_printf(" backing_file=");
-		term_print_filename(bs->backing_file);
-	    }
-            term_printf(" ro=%d", bs->read_only);
-            term_printf(" drv=%s", bs->drv->format_name);
+                monitor_printf(" backing_file=");
+                monitor_print_filename(bs->backing_file);
+            }
+            monitor_printf(" ro=%d", bs->read_only);
+            monitor_printf(" drv=%s", bs->drv->format_name);
             if (bdrv_is_encrypted(bs))
-                term_printf(" encrypted");
+                monitor_printf(" encrypted");
         } else {
-            term_printf(" [not inserted]");
+            monitor_printf(" [not inserted]");
         }
-        term_printf("\n");
+        monitor_printf("\n");
     }
 }
 
@@ -1072,20 +1072,20 @@ void bdrv_info_stats (void)
     BlockDriverInfo bdi;
 
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
-	term_printf ("%s:"
-		     " rd_bytes=%" PRIu64
-		     " wr_bytes=%" PRIu64
-		     " rd_operations=%" PRIu64
-		     " wr_operations=%" PRIu64
-                     ,
-		     bs->device_name,
-		     bs->rd_bytes, bs->wr_bytes,
-		     bs->rd_ops, bs->wr_ops);
+        monitor_printf("%s:"
+                       " rd_bytes=%" PRIu64
+                       " wr_bytes=%" PRIu64
+                       " rd_operations=%" PRIu64
+                       " wr_operations=%" PRIu64
+                       ,
+                       bs->device_name,
+                       bs->rd_bytes, bs->wr_bytes,
+                       bs->rd_ops, bs->wr_ops);
         if (bdrv_get_info(bs, &bdi) == 0)
-            term_printf(" high=%" PRId64
-                        " bytes_free=%" PRId64,
-                        bdi.highest_alloc, bdi.num_free_bytes);
-        term_printf("\n");
+            monitor_printf(" high=%" PRId64
+                           " bytes_free=%" PRId64,
+                           bdi.highest_alloc, bdi.num_free_bytes);
+        monitor_printf("\n");
     }
 }
 
diff --git a/console.h b/console.h
index 4a2f06f..3721547 100644
--- a/console.h
+++ b/console.h
@@ -294,31 +294,4 @@ void curses_display_init(DisplayState *ds, int full_screen);
 
 /* x_keymap.c */
 extern uint8_t _translate_keycode(const int key);
-
-/* FIXME: term_printf et al should probably go elsewhere so everything
-   does not need to include console.h  */
-/* monitor.c */
-void monitor_init(CharDriverState *hd, int show_banner);
-void term_puts(const char *str);
-void term_vprintf(const char *fmt, va_list ap);
-void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
-void term_print_filename(const char *filename);
-void term_flush(void);
-void term_print_help(void);
-void monitor_readline(const char *prompt, int is_password,
-                      char *buf, int buf_size);
-void monitor_suspend(void);
-void monitor_resume(void);
-
-/* readline.c */
-typedef void ReadLineFunc(void *opaque, const char *str);
-
-extern int completion_index;
-void add_completion(const char *str);
-void readline_handle_byte(int ch);
-void readline_find_completion(const char *cmdline);
-const char *readline_get_history(unsigned int index);
-void readline_start(const char *prompt, int is_password,
-                    ReadLineFunc *readline_func, void *opaque);
-
 #endif
diff --git a/disas.c b/disas.c
index 83c8826..a296edb 100644
--- a/disas.c
+++ b/disas.c
@@ -308,8 +308,8 @@ const char *lookup_symbol(target_ulong orig_addr)
 
 #if !defined(CONFIG_USER_ONLY)
 
-void term_vprintf(const char *fmt, va_list ap);
-void term_printf(const char *fmt, ...);
+void monitor_vprintf(const char *fmt, va_list ap);
+void monitor_printf(const char *fmt, ...);
 
 static int monitor_disas_is_physical;
 static CPUState *monitor_disas_env;
@@ -330,7 +330,7 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
 {
     va_list ap;
     va_start(ap, fmt);
-    term_vprintf(fmt, ap);
+    monitor_vprintf(fmt, ap);
     va_end(ap);
     return 0;
 }
@@ -388,15 +388,15 @@ void monitor_disas(CPUState *env,
     print_insn = print_insn_little_mips;
 #endif
 #else
-    term_printf("0x" TARGET_FMT_lx
-		": Asm output not supported on this arch\n", pc);
+    monitor_printf("0x" TARGET_FMT_lx
+                   ": Asm output not supported on this arch\n", pc);
     return;
 #endif
 
     for(i = 0; i < nb_insn; i++) {
-	term_printf("0x" TARGET_FMT_lx ":  ", pc);
+	monitor_printf("0x" TARGET_FMT_lx ":  ", pc);
 	count = print_insn(pc, &disasm_info);
-	term_printf("\n");
+	monitor_printf("\n");
 	if (count < 0)
 	    break;
         pc += count;
diff --git a/hw/i8259.c b/hw/i8259.c
index 933289b..0a1cc73 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -24,7 +24,7 @@
 #include "hw.h"
 #include "pc.h"
 #include "isa.h"
-#include "console.h"
+#include "monitor.h"
 
 /* debug PIC */
 //#define DEBUG_PIC
@@ -521,26 +521,26 @@ void pic_info(void)
 
     for(i=0;i<2;i++) {
         s = &isa_pic->pics[i];
-        term_printf("pic%d: irr=%02x imr=%02x isr=%02x hprio=%d irq_base=%02x rr_sel=%d elcr=%02x fnm=%d\n",
-                    i, s->irr, s->imr, s->isr, s->priority_add,
-                    s->irq_base, s->read_reg_select, s->elcr,
-                    s->special_fully_nested_mode);
+        monitor_printf("pic%d: irr=%02x imr=%02x isr=%02x hprio=%d irq_base=%02x rr_sel=%d elcr=%02x fnm=%d\n",
+                       i, s->irr, s->imr, s->isr, s->priority_add,
+                       s->irq_base, s->read_reg_select, s->elcr,
+                       s->special_fully_nested_mode);
     }
 }
 
 void irq_info(void)
 {
 #ifndef DEBUG_IRQ_COUNT
-    term_printf("irq statistic code not compiled.\n");
+    monitor_printf("irq statistic code not compiled.\n");
 #else
     int i;
     int64_t count;
 
-    term_printf("IRQ statistics:\n");
+    monitor_printf("IRQ statistics:\n");
     for (i = 0; i < 16; i++) {
         count = irq_count[i];
         if (count > 0)
-            term_printf("%2d: %" PRId64 "\n", i, count);
+            monitor_printf("%2d: %" PRId64 "\n", i, count);
     }
 #endif
 }
diff --git a/hw/pc.c b/hw/pc.c
index 176730e..0a81594 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -31,7 +31,7 @@
 #include "net.h"
 #include "smbus.h"
 #include "boards.h"
-#include "console.h"
+#include "monitor.h"
 #include "fw_cfg.h"
 #include "virtio-blk.h"
 #include "virtio-balloon.h"
@@ -211,14 +211,14 @@ static int pc_boot_set(void *opaque, const char *boot_device)
 
     nbds = strlen(boot_device);
     if (nbds > PC_MAX_BOOT_DEVICES) {
-        term_printf("Too many boot devices for PC\n");
+        monitor_printf("Too many boot devices for PC\n");
         return(1);
     }
     for (i = 0; i < nbds; i++) {
         bds[i] = boot_device2nibble(boot_device[i]);
         if (bds[i] == 0) {
-            term_printf("Invalid boot device for PC: '%c'\n",
-                    boot_device[i]);
+            monitor_printf("Invalid boot device for PC: '%c'\n",
+                           boot_device[i]);
             return(1);
         }
     }
diff --git a/hw/pci.c b/hw/pci.c
index 0e57d21..061433d 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -23,7 +23,7 @@
  */
 #include "hw.h"
 #include "pci.h"
-#include "console.h"
+#include "monitor.h"
 #include "net.h"
 #include "virtio-net.h"
 
@@ -588,37 +588,37 @@ static void pci_info_device(PCIDevice *d)
     PCIIORegion *r;
     const pci_class_desc *desc;
 
-    term_printf("  Bus %2d, device %3d, function %d:\n",
+    monitor_printf("  Bus %2d, device %3d, function %d:\n",
            d->bus->bus_num, d->devfn >> 3, d->devfn & 7);
     class = le16_to_cpu(*((uint16_t *)(d->config + PCI_CLASS_DEVICE)));
-    term_printf("    ");
+    monitor_printf("    ");
     desc = pci_class_descriptions;
     while (desc->desc && class != desc->class)
         desc++;
     if (desc->desc) {
-        term_printf("%s", desc->desc);
+        monitor_printf("%s", desc->desc);
     } else {
-        term_printf("Class %04x", class);
+        monitor_printf("Class %04x", class);
     }
-    term_printf(": PCI device %04x:%04x\n",
+    monitor_printf(": PCI device %04x:%04x\n",
            le16_to_cpu(*((uint16_t *)(d->config + PCI_VENDOR_ID))),
            le16_to_cpu(*((uint16_t *)(d->config + PCI_DEVICE_ID))));
 
     if (d->config[PCI_INTERRUPT_PIN] != 0) {
-        term_printf("      IRQ %d.\n", d->config[PCI_INTERRUPT_LINE]);
+        monitor_printf("      IRQ %d.\n", d->config[PCI_INTERRUPT_LINE]);
     }
     if (class == 0x0604) {
-        term_printf("      BUS %d.\n", d->config[0x19]);
+        monitor_printf("      BUS %d.\n", d->config[0x19]);
     }
     for(i = 0;i < PCI_NUM_REGIONS; i++) {
         r = &d->io_regions[i];
         if (r->size != 0) {
-            term_printf("      BAR%d: ", i);
+            monitor_printf("      BAR%d: ", i);
             if (r->type & PCI_ADDRESS_SPACE_IO) {
-                term_printf("I/O at 0x%04x [0x%04x].\n",
+                monitor_printf("I/O at 0x%04x [0x%04x].\n",
                        r->addr, r->addr + r->size - 1);
             } else {
-                term_printf("32 bit memory at 0x%08x [0x%08x].\n",
+                monitor_printf("32 bit memory at 0x%08x [0x%08x].\n",
                        r->addr, r->addr + r->size - 1);
             }
         }
diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
index ff8f0c7..9895d13 100644
--- a/hw/slavio_intctl.c
+++ b/hw/slavio_intctl.c
@@ -23,7 +23,7 @@
  */
 #include "hw.h"
 #include "sun4m.h"
-#include "console.h"
+#include "monitor.h"
 
 //#define DEBUG_IRQ_COUNT
 //#define DEBUG_IRQ
@@ -225,27 +225,27 @@ void slavio_pic_info(void *opaque)
     int i;
 
     for (i = 0; i < MAX_CPUS; i++) {
-        term_printf("per-cpu %d: pending 0x%08x\n", i,
+        monitor_printf("per-cpu %d: pending 0x%08x\n", i,
                     s->slaves[i]->intreg_pending);
     }
-    term_printf("master: pending 0x%08x, disabled 0x%08x\n",
-                s->intregm_pending, s->intregm_disabled);
+    monitor_printf("master: pending 0x%08x, disabled 0x%08x\n",
+                   s->intregm_pending, s->intregm_disabled);
 }
 
 void slavio_irq_info(void *opaque)
 {
 #ifndef DEBUG_IRQ_COUNT
-    term_printf("irq statistic code not compiled.\n");
+    monitor_printf("irq statistic code not compiled.\n");
 #else
     SLAVIO_INTCTLState *s = opaque;
     int i;
     int64_t count;
 
-    term_printf("IRQ statistics:\n");
+    monitor_printf("IRQ statistics:\n");
     for (i = 0; i < 32; i++) {
         count = s->irq_count[i];
         if (count > 0)
-            term_printf("%2d: %" PRId64 "\n", i, count);
+            monitor_printf("%2d: %" PRId64 "\n", i, count);
     }
 #endif
 }
diff --git a/hw/sun4c_intctl.c b/hw/sun4c_intctl.c
index 1759157..6d1f18c 100644
--- a/hw/sun4c_intctl.c
+++ b/hw/sun4c_intctl.c
@@ -23,7 +23,7 @@
  */
 #include "hw.h"
 #include "sun4m.h"
-#include "console.h"
+#include "monitor.h"
 //#define DEBUG_IRQ_COUNT
 //#define DEBUG_IRQ
 
@@ -94,22 +94,22 @@ void sun4c_pic_info(void *opaque)
 {
     Sun4c_INTCTLState *s = opaque;
 
-    term_printf("master: pending 0x%2.2x, enabled 0x%2.2x\n", s->pending,
-                s->reg);
+    monitor_printf("master: pending 0x%2.2x, enabled 0x%2.2x\n", s->pending,
+                   s->reg);
 }
 
 void sun4c_irq_info(void *opaque)
 {
 #ifndef DEBUG_IRQ_COUNT
-    term_printf("irq statistic code not compiled.\n");
+    monitor_printf("irq statistic code not compiled.\n");
 #else
     Sun4c_INTCTLState *s = opaque;
     int64_t count;
 
-    term_printf("IRQ statistics:\n");
+    monitor_printf("IRQ statistics:\n");
     count = s->irq_count[i];
     if (count > 0)
-        term_printf("%2d: %" PRId64 "\n", i, count);
+        monitor_printf("%2d: %" PRId64 "\n", i, count);
 #endif
 }
 
diff --git a/migration-exec.c b/migration-exec.c
index 6ed322a..2c92340 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -18,7 +18,7 @@
 #include "migration.h"
 #include "qemu-char.h"
 #include "sysemu.h"
-#include "console.h"
+#include "monitor.h"
 #include "buffered_file.h"
 #include "block.h"
 
diff --git a/migration-tcp.c b/migration-tcp.c
index 3f5b104..c5e102c 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -16,7 +16,7 @@
 #include "migration.h"
 #include "qemu-char.h"
 #include "sysemu.h"
-#include "console.h"
+#include "monitor.h"
 #include "buffered_file.h"
 #include "block.h"
 
diff --git a/migration.c b/migration.c
index 0ef777a..a7f9355 100644
--- a/migration.c
+++ b/migration.c
@@ -13,7 +13,7 @@
 
 #include "qemu-common.h"
 #include "migration.h"
-#include "console.h"
+#include "monitor.h"
 #include "buffered_file.h"
 #include "sysemu.h"
 #include "block.h"
@@ -60,10 +60,10 @@ void do_migrate(int detach, const char *uri)
         s = exec_start_outgoing_migration(p, max_throttle, detach);
 #endif
     else
-        term_printf("unknown migration protocol: %s\n", uri);
+        monitor_printf("unknown migration protocol: %s\n", uri);
 
     if (s == NULL)
-        term_printf("migration failed\n");
+        monitor_printf("migration failed\n");
     else {
         if (current_migration)
             current_migration->release(current_migration);
@@ -103,21 +103,21 @@ void do_migrate_set_speed(const char *value)
 void do_info_migrate(void)
 {
     MigrationState *s = current_migration;
-    
+
     if (s) {
-        term_printf("Migration status: ");
+        monitor_printf("Migration status: ");
         switch (s->get_status(s)) {
         case MIG_STATE_ACTIVE:
-            term_printf("active\n");
+            monitor_printf("active\n");
             break;
         case MIG_STATE_COMPLETED:
-            term_printf("completed\n");
+            monitor_printf("completed\n");
             break;
         case MIG_STATE_ERROR:
-            term_printf("failed\n");
+            monitor_printf("failed\n");
             break;
         case MIG_STATE_CANCELLED:
-            term_printf("cancelled\n");
+            monitor_printf("cancelled\n");
             break;
         }
     }
diff --git a/monitor.c b/monitor.c
index 9643e44..61cea11 100644
--- a/monitor.c
+++ b/monitor.c
@@ -30,6 +30,8 @@
 #include "net.h"
 #include "qemu-char.h"
 #include "sysemu.h"
+#include "monitor.h"
+#include "readline.h"
 #include "console.h"
 #include "block.h"
 #include "audio/audio.h"
@@ -79,7 +81,7 @@ static void monitor_start_input(void);
 
 static CPUState *mon_cpu = NULL;
 
-void term_flush(void)
+void monitor_flush(void)
 {
     int i;
     if (term_outbuf_index > 0) {
@@ -91,7 +93,7 @@ void term_flush(void)
 }
 
 /* flush at every end of line or if the buffer is full */
-void term_puts(const char *str)
+static void monitor_puts(const char *str)
 {
     char c;
     for(;;) {
@@ -103,26 +105,26 @@ void term_puts(const char *str)
         term_outbuf[term_outbuf_index++] = c;
         if (term_outbuf_index >= (sizeof(term_outbuf) - 1) ||
             c == '\n')
-            term_flush();
+            monitor_flush();
     }
 }
 
-void term_vprintf(const char *fmt, va_list ap)
+void monitor_vprintf(const char *fmt, va_list ap)
 {
     char buf[4096];
     vsnprintf(buf, sizeof(buf), fmt, ap);
-    term_puts(buf);
+    monitor_puts(buf);
 }
 
-void term_printf(const char *fmt, ...)
+void monitor_printf(const char *fmt, ...)
 {
     va_list ap;
     va_start(ap, fmt);
-    term_vprintf(fmt, ap);
+    monitor_vprintf(fmt, ap);
     va_end(ap);
 }
 
-void term_print_filename(const char *filename)
+void monitor_print_filename(const char *filename)
 {
     int i;
 
@@ -131,19 +133,19 @@ void term_print_filename(const char *filename)
 	case ' ':
 	case '"':
 	case '\\':
-	    term_printf("\\%c", filename[i]);
+	    monitor_printf("\\%c", filename[i]);
 	    break;
 	case '\t':
-	    term_printf("\\t");
+	    monitor_printf("\\t");
 	    break;
 	case '\r':
-	    term_printf("\\r");
+	    monitor_printf("\\r");
 	    break;
 	case '\n':
-	    term_printf("\\n");
+	    monitor_printf("\\n");
 	    break;
 	default:
-	    term_printf("%c", filename[i]);
+	    monitor_printf("%c", filename[i]);
 	    break;
 	}
     }
@@ -153,7 +155,7 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
 {
     va_list ap;
     va_start(ap, fmt);
-    term_vprintf(fmt, ap);
+    monitor_vprintf(fmt, ap);
     va_end(ap);
     return 0;
 }
@@ -184,7 +186,8 @@ static void help_cmd1(const term_cmd_t *cmds, const char *prefix, const char *na
 
     for(cmd = cmds; cmd->name != NULL; cmd++) {
         if (!name || !strcmp(name, cmd->name))
-            term_printf("%s%s %s -- %s\n", prefix, cmd->name, cmd->params, cmd->help);
+            monitor_printf("%s%s %s -- %s\n", prefix, cmd->name, cmd->params,
+                           cmd->help);
     }
 }
 
@@ -196,10 +199,10 @@ static void help_cmd(const char *name)
         help_cmd1(term_cmds, "", name);
         if (name && !strcmp(name, "log")) {
             const CPULogItem *item;
-            term_printf("Log items (comma separated):\n");
-            term_printf("%-10s %s\n", "none", "remove all logs");
+            monitor_printf("Log items (comma separated):\n");
+            monitor_printf("%-10s %s\n", "none", "remove all logs");
             for(item = cpu_log_items; item->mask != 0; item++) {
-                term_printf("%-10s %s\n", item->name, item->help);
+                monitor_printf("%-10s %s\n", item->name, item->help);
             }
         }
     }
@@ -243,25 +246,25 @@ static void do_info(const char *item)
 
 static void do_info_version(void)
 {
-  term_printf("%s\n", QEMU_VERSION);
+    monitor_printf("%s\n", QEMU_VERSION);
 }
 
 static void do_info_name(void)
 {
     if (qemu_name)
-        term_printf("%s\n", qemu_name);
+        monitor_printf("%s\n", qemu_name);
 }
 
 #if defined(TARGET_I386)
 static void do_info_hpet(void)
 {
-    term_printf("HPET is %s by QEMU\n", (no_hpet) ? "disabled" : "enabled");
+    monitor_printf("HPET is %s by QEMU\n", (no_hpet) ? "disabled" : "enabled");
 }
 #endif
 
 static void do_info_uuid(void)
 {
-    term_printf(UUID_FMT "\n", qemu_uuid[0], qemu_uuid[1], qemu_uuid[2],
+    monitor_printf(UUID_FMT "\n", qemu_uuid[0], qemu_uuid[1], qemu_uuid[2],
             qemu_uuid[3], qemu_uuid[4], qemu_uuid[5], qemu_uuid[6],
             qemu_uuid[7], qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
             qemu_uuid[11], qemu_uuid[12], qemu_uuid[13], qemu_uuid[14],
@@ -323,28 +326,30 @@ static void do_info_cpus(void)
     mon_get_cpu();
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
-        term_printf("%c CPU #%d:",
-                    (env == mon_cpu) ? '*' : ' ',
-                    env->cpu_index);
+        monitor_printf("%c CPU #%d:",
+                       (env == mon_cpu) ? '*' : ' ',
+                       env->cpu_index);
 #if defined(TARGET_I386)
-        term_printf(" pc=0x" TARGET_FMT_lx, env->eip + env->segs[R_CS].base);
+        monitor_printf(" pc=0x" TARGET_FMT_lx,
+                       env->eip + env->segs[R_CS].base);
 #elif defined(TARGET_PPC)
-        term_printf(" nip=0x" TARGET_FMT_lx, env->nip);
+        monitor_printf(" nip=0x" TARGET_FMT_lx, env->nip);
 #elif defined(TARGET_SPARC)
-        term_printf(" pc=0x" TARGET_FMT_lx " npc=0x" TARGET_FMT_lx, env->pc, env->npc);
+        monitor_printf(" pc=0x" TARGET_FMT_lx " npc=0x" TARGET_FMT_lx,
+                       env->pc, env->npc);
 #elif defined(TARGET_MIPS)
-        term_printf(" PC=0x" TARGET_FMT_lx, env->active_tc.PC);
+        monitor_printf(" PC=0x" TARGET_FMT_lx, env->active_tc.PC);
 #endif
         if (env->halted)
-            term_printf(" (halted)");
-        term_printf("\n");
+            monitor_printf(" (halted)");
+        monitor_printf("\n");
     }
 }
 
 static void do_cpu_set(int index)
 {
     if (mon_set_cpu(index) < 0)
-        term_printf("Invalid CPU index\n");
+        monitor_printf("Invalid CPU index\n");
 }
 
 static void do_info_jit(void)
@@ -362,7 +367,7 @@ static void do_info_history (void)
         str = readline_get_history(i);
         if (!str)
             break;
-	term_printf("%d: '%s'\n", i, str);
+	monitor_printf("%d: '%s'\n", i, str);
         i++;
     }
 }
@@ -388,11 +393,11 @@ static int eject_device(BlockDriverState *bs, int force)
     if (bdrv_is_inserted(bs)) {
         if (!force) {
             if (!bdrv_is_removable(bs)) {
-                term_printf("device is not removable\n");
+                monitor_printf("device is not removable\n");
                 return -1;
             }
             if (bdrv_is_locked(bs)) {
-                term_printf("device is locked\n");
+                monitor_printf("device is locked\n");
                 return -1;
             }
         }
@@ -407,7 +412,7 @@ static void do_eject(int force, const char *filename)
 
     bs = bdrv_find(filename);
     if (!bs) {
-        term_printf("device not found\n");
+        monitor_printf("device not found\n");
         return;
     }
     eject_device(bs, force);
@@ -420,13 +425,13 @@ static void do_change_block(const char *device, const char *filename, const char
 
     bs = bdrv_find(device);
     if (!bs) {
-        term_printf("device not found\n");
+        monitor_printf("device not found\n");
         return;
     }
     if (fmt) {
         drv = bdrv_find_format(fmt);
         if (!drv) {
-            term_printf("invalid format %s\n", fmt);
+            monitor_printf("invalid format %s\n", fmt);
             return;
         }
     }
@@ -447,10 +452,10 @@ static void do_change_vnc(const char *target, const char *arg)
 	} else
 	    monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
-	    term_printf("could not set VNC server password\n");
+	    monitor_printf("could not set VNC server password\n");
     } else {
 	if (vnc_display_open(NULL, target) < 0)
-	    term_printf("could not start VNC server on %s\n", target);
+	    monitor_printf("could not start VNC server on %s\n", target);
     }
 }
 
@@ -512,31 +517,31 @@ static void do_gdbserver(const char *port)
 }
 #endif
 
-static void term_printc(int c)
+static void monitor_printc(int c)
 {
-    term_printf("'");
+    monitor_printf("'");
     switch(c) {
     case '\'':
-        term_printf("\\'");
+        monitor_printf("\\'");
         break;
     case '\\':
-        term_printf("\\\\");
+        monitor_printf("\\\\");
         break;
     case '\n':
-        term_printf("\\n");
+        monitor_printf("\\n");
         break;
     case '\r':
-        term_printf("\\r");
+        monitor_printf("\\r");
         break;
     default:
         if (c >= 32 && c <= 126) {
-            term_printf("%c", c);
+            monitor_printf("%c", c);
         } else {
-            term_printf("\\x%02x", c);
+            monitor_printf("\\x%02x", c);
         }
         break;
     }
-    term_printf("'");
+    monitor_printf("'");
 }
 
 static void memory_dump(int count, int format, int wsize,
@@ -604,9 +609,9 @@ static void memory_dump(int count, int format, int wsize,
 
     while (len > 0) {
         if (is_physical)
-            term_printf(TARGET_FMT_plx ":", addr);
+            monitor_printf(TARGET_FMT_plx ":", addr);
         else
-            term_printf(TARGET_FMT_lx ":", (target_ulong)addr);
+            monitor_printf(TARGET_FMT_lx ":", (target_ulong)addr);
         l = len;
         if (l > line_size)
             l = line_size;
@@ -617,7 +622,7 @@ static void memory_dump(int count, int format, int wsize,
             if (!env)
                 break;
             if (cpu_memory_rw_debug(env, addr, buf, l, 0) < 0) {
-                term_printf(" Cannot access memory\n");
+                monitor_printf(" Cannot access memory\n");
                 break;
             }
         }
@@ -638,27 +643,27 @@ static void memory_dump(int count, int format, int wsize,
                 v = ldq_raw(buf + i);
                 break;
             }
-            term_printf(" ");
+            monitor_printf(" ");
             switch(format) {
             case 'o':
-                term_printf("%#*" PRIo64, max_digits, v);
+                monitor_printf("%#*" PRIo64, max_digits, v);
                 break;
             case 'x':
-                term_printf("0x%0*" PRIx64, max_digits, v);
+                monitor_printf("0x%0*" PRIx64, max_digits, v);
                 break;
             case 'u':
-                term_printf("%*" PRIu64, max_digits, v);
+                monitor_printf("%*" PRIu64, max_digits, v);
                 break;
             case 'd':
-                term_printf("%*" PRId64, max_digits, v);
+                monitor_printf("%*" PRId64, max_digits, v);
                 break;
             case 'c':
-                term_printc(v);
+                monitor_printc(v);
                 break;
             }
             i += wsize;
         }
-        term_printf("\n");
+        monitor_printf("\n");
         addr += l;
         len -= l;
     }
@@ -697,43 +702,43 @@ static void do_print(int count, int format, int size, unsigned int valh, unsigne
 #if TARGET_PHYS_ADDR_BITS == 32
     switch(format) {
     case 'o':
-        term_printf("%#o", val);
+        monitor_printf("%#o", val);
         break;
     case 'x':
-        term_printf("%#x", val);
+        monitor_printf("%#x", val);
         break;
     case 'u':
-        term_printf("%u", val);
+        monitor_printf("%u", val);
         break;
     default:
     case 'd':
-        term_printf("%d", val);
+        monitor_printf("%d", val);
         break;
     case 'c':
-        term_printc(val);
+        monitor_printc(val);
         break;
     }
 #else
     switch(format) {
     case 'o':
-        term_printf("%#" PRIo64, val);
+        monitor_printf("%#" PRIo64, val);
         break;
     case 'x':
-        term_printf("%#" PRIx64, val);
+        monitor_printf("%#" PRIx64, val);
         break;
     case 'u':
-        term_printf("%" PRIu64, val);
+        monitor_printf("%" PRIu64, val);
         break;
     default:
     case 'd':
-        term_printf("%" PRId64, val);
+        monitor_printf("%" PRId64, val);
         break;
     case 'c':
-        term_printc(val);
+        monitor_printc(val);
         break;
     }
 #endif
-    term_printf("\n");
+    monitor_printf("\n");
 }
 
 static void do_memory_save(unsigned int valh, unsigned int vall,
@@ -751,7 +756,7 @@ static void do_memory_save(unsigned int valh, unsigned int vall,
 
     f = fopen(filename, "wb");
     if (!f) {
-        term_printf("could not open '%s'\n", filename);
+        monitor_printf("could not open '%s'\n", filename);
         return;
     }
     while (size != 0) {
@@ -776,7 +781,7 @@ static void do_physical_memory_save(unsigned int valh, unsigned int vall,
 
     f = fopen(filename, "wb");
     if (!f) {
-        term_printf("could not open '%s'\n", filename);
+        monitor_printf("could not open '%s'\n", filename);
         return;
     }
     while (size != 0) {
@@ -805,7 +810,7 @@ static void do_sum(uint32_t start, uint32_t size)
         sum = (sum >> 1) | (sum << 15);
         sum += buf[0];
     }
-    term_printf("%05d\n", sum);
+    monitor_printf("%05d\n", sum);
 }
 
 typedef struct {
@@ -1007,17 +1012,17 @@ static void do_sendkey(const char *string, int has_hold_time, int hold_time)
         if (keyname_len > 0) {
             pstrcpy(keyname_buf, sizeof(keyname_buf), string);
             if (keyname_len > sizeof(keyname_buf) - 1) {
-                term_printf("invalid key: '%s...'\n", keyname_buf);
+                monitor_printf("invalid key: '%s...'\n", keyname_buf);
                 return;
             }
             if (i == MAX_KEYCODES) {
-                term_printf("too many keys\n");
+                monitor_printf("too many keys\n");
                 return;
             }
             keyname_buf[keyname_len] = 0;
             keycode = get_keycode(keyname_buf);
             if (keycode < 0) {
-                term_printf("unknown key: '%s'\n", keyname_buf);
+                monitor_printf("unknown key: '%s'\n", keyname_buf);
                 return;
             }
             keycodes[i++] = keycode;
@@ -1085,8 +1090,8 @@ static void do_ioport_read(int count, int format, int size, int addr, int has_in
         suffix = 'l';
         break;
     }
-    term_printf("port%c[0x%04x] = %#0*x\n",
-                suffix, addr, size * 2, val);
+    monitor_printf("port%c[0x%04x] = %#0*x\n",
+                   suffix, addr, size * 2, val);
 }
 
 /* boot_set handler */
@@ -1106,11 +1111,13 @@ static void do_boot_set(const char *bootdevice)
     if (qemu_boot_set_handler)  {
         res = qemu_boot_set_handler(boot_opaque, bootdevice);
         if (res == 0)
-            term_printf("boot device list now set to %s\n", bootdevice);
+            monitor_printf("boot device list now set to %s\n", bootdevice);
         else
-            term_printf("setting boot device list failed with error %i\n", res);
+            monitor_printf("setting boot device list failed with error %i\n",
+                           res);
     } else {
-        term_printf("no function defined to set boot device list for this architecture\n");
+        monitor_printf("no function defined to set boot device list for this "
+                       "architecture\n");
     }
 }
 
@@ -1127,17 +1134,17 @@ static void do_system_powerdown(void)
 #if defined(TARGET_I386)
 static void print_pte(uint32_t addr, uint32_t pte, uint32_t mask)
 {
-    term_printf("%08x: %08x %c%c%c%c%c%c%c%c\n",
-                addr,
-                pte & mask,
-                pte & PG_GLOBAL_MASK ? 'G' : '-',
-                pte & PG_PSE_MASK ? 'P' : '-',
-                pte & PG_DIRTY_MASK ? 'D' : '-',
-                pte & PG_ACCESSED_MASK ? 'A' : '-',
-                pte & PG_PCD_MASK ? 'C' : '-',
-                pte & PG_PWT_MASK ? 'T' : '-',
-                pte & PG_USER_MASK ? 'U' : '-',
-                pte & PG_RW_MASK ? 'W' : '-');
+    monitor_printf("%08x: %08x %c%c%c%c%c%c%c%c\n",
+                   addr,
+                   pte & mask,
+                   pte & PG_GLOBAL_MASK ? 'G' : '-',
+                   pte & PG_PSE_MASK ? 'P' : '-',
+                   pte & PG_DIRTY_MASK ? 'D' : '-',
+                   pte & PG_ACCESSED_MASK ? 'A' : '-',
+                   pte & PG_PCD_MASK ? 'C' : '-',
+                   pte & PG_PWT_MASK ? 'T' : '-',
+                   pte & PG_USER_MASK ? 'U' : '-',
+                   pte & PG_RW_MASK ? 'W' : '-');
 }
 
 static void tlb_info(void)
@@ -1151,7 +1158,7 @@ static void tlb_info(void)
         return;
 
     if (!(env->cr[0] & CR0_PG_MASK)) {
-        term_printf("PG disabled\n");
+        monitor_printf("PG disabled\n");
         return;
     }
     pgd = env->cr[3] & ~0xfff;
@@ -1184,11 +1191,11 @@ static void mem_print(uint32_t *pstart, int *plast_prot,
     prot1 = *plast_prot;
     if (prot != prot1) {
         if (*pstart != -1) {
-            term_printf("%08x-%08x %08x %c%c%c\n",
-                        *pstart, end, end - *pstart,
-                        prot1 & PG_USER_MASK ? 'u' : '-',
-                        'r',
-                        prot1 & PG_RW_MASK ? 'w' : '-');
+            monitor_printf("%08x-%08x %08x %c%c%c\n",
+                           *pstart, end, end - *pstart,
+                           prot1 & PG_USER_MASK ? 'u' : '-',
+                           'r',
+                           prot1 & PG_RW_MASK ? 'w' : '-');
         }
         if (prot != 0)
             *pstart = end;
@@ -1209,7 +1216,7 @@ static void mem_info(void)
         return;
 
     if (!(env->cr[0] & CR0_PG_MASK)) {
-        term_printf("PG disabled\n");
+        monitor_printf("PG disabled\n");
         return;
     }
     pgd = env->cr[3] & ~0xfff;
@@ -1253,38 +1260,38 @@ static void do_info_kqemu(void)
     val = 0;
     env = mon_get_cpu();
     if (!env) {
-        term_printf("No cpu initialized yet");
+        monitor_printf("No cpu initialized yet");
         return;
     }
     val = env->kqemu_enabled;
-    term_printf("kqemu support: ");
+    monitor_printf("kqemu support: ");
     switch(val) {
     default:
     case 0:
-        term_printf("disabled\n");
+        monitor_printf("disabled\n");
         break;
     case 1:
-        term_printf("enabled for user code\n");
+        monitor_printf("enabled for user code\n");
         break;
     case 2:
-        term_printf("enabled for user and kernel code\n");
+        monitor_printf("enabled for user and kernel code\n");
         break;
     }
 #else
-    term_printf("kqemu support: not compiled\n");
+    monitor_printf("kqemu support: not compiled\n");
 #endif
 }
 
 static void do_info_kvm(void)
 {
 #ifdef CONFIG_KVM
-    term_printf("kvm support: ");
+    monitor_printf("kvm support: ");
     if (kvm_enabled())
-	term_printf("enabled\n");
+	monitor_printf("enabled\n");
     else
-	term_printf("disabled\n");
+	monitor_printf("disabled\n");
 #else
-    term_printf("kvm support: not compiled\n");
+    monitor_printf("kvm support: not compiled\n");
 #endif
 }
 
@@ -1304,17 +1311,17 @@ static void do_info_profile(void)
     total = qemu_time;
     if (total == 0)
         total = 1;
-    term_printf("async time  %" PRId64 " (%0.3f)\n",
-                dev_time, dev_time / (double)ticks_per_sec);
-    term_printf("qemu time   %" PRId64 " (%0.3f)\n",
-                qemu_time, qemu_time / (double)ticks_per_sec);
-    term_printf("kqemu time  %" PRId64 " (%0.3f %0.1f%%) count=%" PRId64 " int=%" PRId64 " excp=%" PRId64 " intr=%" PRId64 "\n",
-                kqemu_time, kqemu_time / (double)ticks_per_sec,
-                kqemu_time / (double)total * 100.0,
-                kqemu_exec_count,
-                kqemu_ret_int_count,
-                kqemu_ret_excp_count,
-                kqemu_ret_intr_count);
+    monitor_printf("async time  %" PRId64 " (%0.3f)\n",
+                   dev_time, dev_time / (double)ticks_per_sec);
+    monitor_printf("qemu time   %" PRId64 " (%0.3f)\n",
+                   qemu_time, qemu_time / (double)ticks_per_sec);
+    monitor_printf("kqemu time  %" PRId64 " (%0.3f %0.1f%%) count=%" PRId64 " int=%" PRId64 " excp=%" PRId64 " intr=%" PRId64 "\n",
+                   kqemu_time, kqemu_time / (double)ticks_per_sec,
+                   kqemu_time / (double)total * 100.0,
+                   kqemu_exec_count,
+                   kqemu_ret_int_count,
+                   kqemu_ret_excp_count,
+                   kqemu_ret_intr_count);
     qemu_time = 0;
     kqemu_time = 0;
     kqemu_exec_count = 0;
@@ -1329,7 +1336,7 @@ static void do_info_profile(void)
 #else
 static void do_info_profile(void)
 {
-    term_printf("Internal profiler not compiled\n");
+    monitor_printf("Internal profiler not compiled\n");
 }
 #endif
 
@@ -1342,7 +1349,7 @@ static void do_info_capture (void)
     CaptureState *s;
 
     for (s = capture_head.lh_first, i = 0; s; s = s->entries.le_next, ++i) {
-        term_printf ("[%d]: ", i);
+        monitor_printf("[%d]: ", i);
         s->ops.info (s->opaque);
     }
 }
@@ -1377,7 +1384,7 @@ static void do_wav_capture (const char *path,
     nchannels = has_channels ? nchannels : 2;
 
     if (wav_start_capture (s, path, freq, bits, nchannels)) {
-        term_printf ("Faied to add wave capture\n");
+        monitor_printf("Faied to add wave capture\n");
         qemu_free (s);
     }
     LIST_INSERT_HEAD (&capture_head, s, entries);
@@ -1400,9 +1407,9 @@ static void do_inject_nmi(int cpu_index)
 static void do_info_status(void)
 {
     if (vm_running)
-       term_printf("VM status: running\n");
+       monitor_printf("VM status: running\n");
     else
-       term_printf("VM status: paused\n");
+       monitor_printf("VM status: paused\n");
 }
 
 
@@ -1418,11 +1425,12 @@ static void do_info_balloon(void)
 
     actual = qemu_balloon_status();
     if (kvm_enabled() && !kvm_has_sync_mmu())
-        term_printf("Using KVM without synchronous MMU, ballooning disabled\n");
+        monitor_printf("Using KVM without synchronous MMU, "
+                       "ballooning disabled\n");
     else if (actual == 0)
-        term_printf("Ballooning not activated in VM\n");
+        monitor_printf("Ballooning not activated in VM\n");
     else
-        term_printf("balloon: actual=%d\n", (int)(actual >> 20));
+        monitor_printf("balloon: actual=%d\n", (int)(actual >> 20));
 }
 
 /* Please update qemu-doc.texi when adding or changing commands */
@@ -1937,7 +1945,7 @@ static const MonitorDef monitor_defs[] = {
 
 static void expr_error(const char *msg)
 {
-    term_printf("%s\n", msg);
+    monitor_printf("%s\n", msg);
     longjmp(expr_env, 1);
 }
 
@@ -2256,7 +2264,7 @@ static void monitor_handle_command(const char *cmdline)
                       void *arg4, void *arg5, void *arg6);
 
 #ifdef DEBUG
-    term_printf("command='%s'\n", cmdline);
+    monitor_printf("command='%s'\n", cmdline);
 #endif
 
     /* extract the command name */
@@ -2280,7 +2288,7 @@ static void monitor_handle_command(const char *cmdline)
         if (compare_cmd(cmdname, cmd->name))
             goto found;
     }
-    term_printf("unknown command: '%s'\n", cmdname);
+    monitor_printf("unknown command: '%s'\n", cmdname);
     return;
  found:
 
@@ -2317,13 +2325,14 @@ static void monitor_handle_command(const char *cmdline)
                 if (ret < 0) {
                     switch(c) {
                     case 'F':
-                        term_printf("%s: filename expected\n", cmdname);
+                        monitor_printf("%s: filename expected\n", cmdname);
                         break;
                     case 'B':
-                        term_printf("%s: block device name expected\n", cmdname);
+                        monitor_printf("%s: block device name expected\n",
+                                       cmdname);
                         break;
                     default:
-                        term_printf("%s: string expected\n", cmdname);
+                        monitor_printf("%s: string expected\n", cmdname);
                         break;
                     }
                     goto fail;
@@ -2334,7 +2343,7 @@ static void monitor_handle_command(const char *cmdline)
             add_str:
                 if (nb_args >= MAX_ARGS) {
                 error_args:
-                    term_printf("%s: too many arguments\n", cmdname);
+                    monitor_printf("%s: too many arguments\n", cmdname);
                     goto fail;
                 }
                 args[nb_args++] = str;
@@ -2392,7 +2401,7 @@ static void monitor_handle_command(const char *cmdline)
                     }
                 next:
                     if (*p != '\0' && !qemu_isspace(*p)) {
-                        term_printf("invalid char in format: '%c'\n", *p);
+                        monitor_printf("invalid char in format: '%c'\n", *p);
                         goto fail;
                     }
                     if (format < 0)
@@ -2487,7 +2496,7 @@ static void monitor_handle_command(const char *cmdline)
                 if (*p == '-') {
                     p++;
                     if (*p != c) {
-                        term_printf("%s: unsupported option -%c\n",
+                        monitor_printf("%s: unsupported option -%c\n",
                                     cmdname, *p);
                         goto fail;
                     }
@@ -2501,7 +2510,7 @@ static void monitor_handle_command(const char *cmdline)
             break;
         default:
         bad_type:
-            term_printf("%s: unknown type '%c'\n", cmdname, c);
+            monitor_printf("%s: unknown type '%c'\n", cmdname, c);
             goto fail;
         }
     }
@@ -2509,8 +2518,8 @@ static void monitor_handle_command(const char *cmdline)
     while (qemu_isspace(*p))
         p++;
     if (*p != '\0') {
-        term_printf("%s: extraneous characters at the end of line\n",
-                    cmdname);
+        monitor_printf("%s: extraneous characters at the end of line\n",
+                       cmdname);
         goto fail;
     }
 
@@ -2548,7 +2557,7 @@ static void monitor_handle_command(const char *cmdline)
         handler_7(args[0], args[1], args[2], args[3], args[4], args[5], args[6]);
         break;
     default:
-        term_printf("unsupported number of arguments: %d\n", nb_args);
+        monitor_printf("unsupported number of arguments: %d\n", nb_args);
         goto fail;
     }
  fail:
@@ -2606,7 +2615,8 @@ static void file_completion(const char *input)
         pstrcpy(file_prefix, sizeof(file_prefix), p + 1);
     }
 #ifdef DEBUG_COMPLETION
-    term_printf("input='%s' path='%s' prefix='%s'\n", input, path, file_prefix);
+    monitor_printf("input='%s' path='%s' prefix='%s'\n",
+                   input, path, file_prefix);
 #endif
     ffs = opendir(path);
     if (!ffs)
@@ -2682,7 +2692,7 @@ void readline_find_completion(const char *cmdline)
     parse_cmdline(cmdline, &nb_args, args);
 #ifdef DEBUG_COMPLETION
     for(i = 0; i < nb_args; i++) {
-        term_printf("arg%d = '%s'\n", i, (char *)args[i]);
+        monitor_printf("arg%d = '%s'\n", i, (char *)args[i]);
     }
 #endif
 
@@ -2800,8 +2810,8 @@ static void term_event(void *opaque, int event)
 	return;
 
     if (!hide_banner)
-	    term_printf("QEMU %s monitor - type 'help' for more information\n",
-			QEMU_VERSION);
+        monitor_printf("QEMU %s monitor - type 'help' for more information\n",
+                       QEMU_VERSION);
     monitor_start_input();
 }
 
diff --git a/monitor.h b/monitor.h
new file mode 100644
index 0000000..973c336
--- /dev/null
+++ b/monitor.h
@@ -0,0 +1,21 @@
+#ifndef MONITOR_H
+#define MONITOR_H
+
+#include "qemu-char.h"
+#include "block.h"
+
+void monitor_init(CharDriverState *hd, int show_banner);
+void monitor_readline(const char *prompt, int is_password,
+                      char *buf, int buf_size);
+void monitor_suspend(void);
+void monitor_resume(void);
+
+int monitor_read_bdrv_key(BlockDriverState *bs, const char *name);
+
+void monitor_vprintf(const char *fmt, va_list ap);
+void monitor_printf(const char *fmt, ...)
+    __attribute__ ((__format__ (__printf__, 1, 2)));
+void monitor_print_filename(const char *filename);
+void monitor_flush(void);
+
+#endif /* !MONITOR_H */
diff --git a/net.c b/net.c
index e7c097e..1d1332a 100644
--- a/net.c
+++ b/net.c
@@ -23,7 +23,7 @@
  */
 #include "qemu-common.h"
 #include "net.h"
-#include "console.h"
+#include "monitor.h"
 #include "sysemu.h"
 #include "qemu-timer.h"
 #include "qemu-char.h"
@@ -1727,9 +1727,9 @@ void do_info_network(void)
     VLANClientState *vc;
 
     for(vlan = first_vlan; vlan != NULL; vlan = vlan->next) {
-        term_printf("VLAN %d devices:\n", vlan->id);
+        monitor_printf("VLAN %d devices:\n", vlan->id);
         for(vc = vlan->first_client; vc != NULL; vc = vc->next)
-            term_printf("  %s: %s\n", vc->name, vc->info_str);
+            monitor_printf("  %s: %s\n", vc->name, vc->info_str);
     }
 }
 
@@ -1745,7 +1745,7 @@ int do_set_link(const char *name, const char *up_or_down)
 done:
 
     if (!vc) {
-        term_printf("could not find network device '%s'", name);
+        monitor_printf("could not find network device '%s'", name);
         return 0;
     }
 
@@ -1754,8 +1754,8 @@ done:
     else if (strcmp(up_or_down, "down") == 0)
         vc->link_down = 1;
     else
-        term_printf("invalid link status '%s'; only 'up' or 'down' valid\n",
-                    up_or_down);
+        monitor_printf("invalid link status '%s'; only 'up' or 'down' valid\n",
+                       up_or_down);
 
     if (vc->link_status_changed)
         vc->link_status_changed(vc);
diff --git a/qemu-char.c b/qemu-char.c
index 38bfab8..15cb931 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -23,6 +23,7 @@
  */
 #include "qemu-common.h"
 #include "net.h"
+#include "monitor.h"
 #include "console.h"
 #include "sysemu.h"
 #include "qemu-timer.h"
@@ -2174,6 +2175,6 @@ void qemu_chr_info(void)
     CharDriverState *chr;
 
     TAILQ_FOREACH(chr, &chardevs, next) {
-        term_printf("%s: filename=%s\n", chr->label, chr->filename);
+        monitor_printf("%s: filename=%s\n", chr->label, chr->filename);
     }
 }
diff --git a/qemu-tool.c b/qemu-tool.c
index 4dda5af..d29ee66 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -12,7 +12,7 @@
  */
 
 #include "qemu-common.h"
-#include "console.h"
+#include "monitor.h"
 #include "sysemu.h"
 #include "qemu-timer.h"
 
@@ -30,11 +30,11 @@ void qemu_service_io(void)
 {
 }
 
-void term_printf(const char *fmt, ...)
+void monitor_printf(const char *fmt, ...)
 {
 }
 
-void term_print_filename(const char *filename)
+void monitor_print_filename(const char *filename)
 {
 }
 
diff --git a/readline.c b/readline.c
index 8572841..4d8a40d 100644
--- a/readline.c
+++ b/readline.c
@@ -22,7 +22,8 @@
  * THE SOFTWARE.
  */
 #include "qemu-common.h"
-#include "console.h"
+#include "monitor.h"
+#include "readline.h"
 
 #define TERM_CMD_BUF_SIZE 4095
 #define TERM_MAX_CMDS 64
@@ -59,8 +60,8 @@ static void *term_readline_opaque;
 
 static void term_show_prompt2(void)
 {
-    term_printf("%s", term_prompt);
-    term_flush();
+    monitor_printf("%s", term_prompt);
+    monitor_flush();
     term_last_cmd_buf_index = 0;
     term_last_cmd_buf_size = 0;
     term_esc_state = IS_NORM;
@@ -81,17 +82,17 @@ static void term_update(void)
     if (term_cmd_buf_size != term_last_cmd_buf_size ||
         memcmp(term_cmd_buf, term_last_cmd_buf, term_cmd_buf_size) != 0) {
         for(i = 0; i < term_last_cmd_buf_index; i++) {
-            term_printf("\033[D");
+            monitor_printf("\033[D");
         }
         term_cmd_buf[term_cmd_buf_size] = '\0';
         if (term_is_password) {
             len = strlen(term_cmd_buf);
             for(i = 0; i < len; i++)
-                term_printf("*");
+                monitor_printf("*");
         } else {
-            term_printf("%s", term_cmd_buf);
+            monitor_printf("%s", term_cmd_buf);
         }
-        term_printf("\033[K");
+        monitor_printf("\033[K");
         memcpy(term_last_cmd_buf, term_cmd_buf, term_cmd_buf_size);
         term_last_cmd_buf_size = term_cmd_buf_size;
         term_last_cmd_buf_index = term_cmd_buf_size;
@@ -100,17 +101,17 @@ static void term_update(void)
         delta = term_cmd_buf_index - term_last_cmd_buf_index;
         if (delta > 0) {
             for(i = 0;i < delta; i++) {
-                term_printf("\033[C");
+                monitor_printf("\033[C");
             }
         } else {
             delta = -delta;
             for(i = 0;i < delta; i++) {
-                term_printf("\033[D");
+                monitor_printf("\033[D");
             }
         }
         term_last_cmd_buf_index = term_cmd_buf_index;
     }
-    term_flush();
+    monitor_flush();
 }
 
 static void term_insert_char(int ch)
@@ -324,7 +325,7 @@ static void term_completion(void)
         if (len > 0 && completions[0][len - 1] != '/')
             term_insert_char(' ');
     } else {
-        term_printf("\n");
+        monitor_printf("\n");
         max_width = 0;
         max_prefix = 0;	
         for(i = 0; i < nb_completions; i++) {
@@ -354,9 +355,9 @@ static void term_completion(void)
         nb_cols = 80 / max_width;
         j = 0;
         for(i = 0; i < nb_completions; i++) {
-            term_printf("%-*s", max_width, completions[i]);
+            monitor_printf("%-*s", max_width, completions[i]);
             if (++j == nb_cols || i == (nb_completions - 1)) {
-                term_printf("\n");
+                monitor_printf("\n");
                 j = 0;
             }
         }
@@ -387,7 +388,7 @@ void readline_handle_byte(int ch)
             term_cmd_buf[term_cmd_buf_size] = '\0';
             if (!term_is_password)
                 term_hist_add(term_cmd_buf);
-            term_printf("\n");
+            monitor_printf("\n");
             term_cmd_buf_index = 0;
             term_cmd_buf_size = 0;
             term_last_cmd_buf_index = 0;
diff --git a/readline.h b/readline.h
new file mode 100644
index 0000000..9538550
--- /dev/null
+++ b/readline.h
@@ -0,0 +1,14 @@
+#ifndef READLINE_H
+#define READLINE_H
+
+typedef void ReadLineFunc(void *opaque, const char *str);
+
+extern int completion_index;
+void add_completion(const char *str);
+void readline_handle_byte(int ch);
+void readline_find_completion(const char *cmdline);
+const char *readline_get_history(unsigned int index);
+void readline_start(const char *prompt, int is_password,
+                    ReadLineFunc *readline_func, void *opaque);
+
+#endif /* !READLINE_H */
diff --git a/savevm.c b/savevm.c
index 3eb2000..33b4bd0 100644
--- a/savevm.c
+++ b/savevm.c
@@ -24,7 +24,7 @@
 #include "qemu-common.h"
 #include "hw/hw.h"
 #include "net.h"
-#include "console.h"
+#include "monitor.h"
 #include "sysemu.h"
 #include "qemu-timer.h"
 #include "qemu-char.h"
@@ -1010,7 +1010,7 @@ void do_savevm(const char *name)
 
     bs = get_bs_snapshots();
     if (!bs) {
-        term_printf("No block device can accept snapshots\n");
+        monitor_printf("No block device can accept snapshots\n");
         return;
     }
 
@@ -1049,22 +1049,22 @@ void do_savevm(const char *name)
     sn->vm_clock_nsec = qemu_get_clock(vm_clock);
 
     if (bdrv_get_info(bs, bdi) < 0 || bdi->vm_state_offset <= 0) {
-        term_printf("Device %s does not support VM state snapshots\n",
-                    bdrv_get_device_name(bs));
+        monitor_printf("Device %s does not support VM state snapshots\n",
+                       bdrv_get_device_name(bs));
         goto the_end;
     }
 
     /* save the VM state */
     f = qemu_fopen_bdrv(bs, bdi->vm_state_offset, 1);
     if (!f) {
-        term_printf("Could not open VM state file\n");
+        monitor_printf("Could not open VM state file\n");
         goto the_end;
     }
     ret = qemu_savevm_state(f);
     vm_state_size = qemu_ftell(f);
     qemu_fclose(f);
     if (ret < 0) {
-        term_printf("Error %d while writing VM\n", ret);
+        monitor_printf("Error %d while writing VM\n", ret);
         goto the_end;
     }
 
@@ -1076,16 +1076,16 @@ void do_savevm(const char *name)
             if (must_delete) {
                 ret = bdrv_snapshot_delete(bs1, old_sn->id_str);
                 if (ret < 0) {
-                    term_printf("Error while deleting snapshot on '%s'\n",
-                                bdrv_get_device_name(bs1));
+                    monitor_printf("Error while deleting snapshot on '%s'\n",
+                                   bdrv_get_device_name(bs1));
                 }
             }
             /* Write VM state size only to the image that contains the state */
             sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
             ret = bdrv_snapshot_create(bs1, sn);
             if (ret < 0) {
-                term_printf("Error while creating snapshot on '%s'\n",
-                            bdrv_get_device_name(bs1));
+                monitor_printf("Error while creating snapshot on '%s'\n",
+                               bdrv_get_device_name(bs1));
             }
         }
     }
@@ -1106,7 +1106,7 @@ void do_loadvm(const char *name)
 
     bs = get_bs_snapshots();
     if (!bs) {
-        term_printf("No block device supports snapshots\n");
+        monitor_printf("No block device supports snapshots\n");
         return;
     }
 
@@ -1122,19 +1122,19 @@ void do_loadvm(const char *name)
             ret = bdrv_snapshot_goto(bs1, name);
             if (ret < 0) {
                 if (bs != bs1)
-                    term_printf("Warning: ");
+                    monitor_printf("Warning: ");
                 switch(ret) {
                 case -ENOTSUP:
-                    term_printf("Snapshots not supported on device '%s'\n",
-                                bdrv_get_device_name(bs1));
+                    monitor_printf("Snapshots not supported on device '%s'\n",
+                                   bdrv_get_device_name(bs1));
                     break;
                 case -ENOENT:
-                    term_printf("Could not find snapshot '%s' on device '%s'\n",
-                                name, bdrv_get_device_name(bs1));
+                    monitor_printf("Could not find snapshot '%s' on device '%s'\n",
+                                   name, bdrv_get_device_name(bs1));
                     break;
                 default:
-                    term_printf("Error %d while activating snapshot on '%s'\n",
-                                ret, bdrv_get_device_name(bs1));
+                    monitor_printf("Error %d while activating snapshot on '%s'\n",
+                                   ret, bdrv_get_device_name(bs1));
                     break;
                 }
                 /* fatal on snapshot block device */
@@ -1145,8 +1145,8 @@ void do_loadvm(const char *name)
     }
 
     if (bdrv_get_info(bs, bdi) < 0 || bdi->vm_state_offset <= 0) {
-        term_printf("Device %s does not support VM state snapshots\n",
-                    bdrv_get_device_name(bs));
+        monitor_printf("Device %s does not support VM state snapshots\n",
+                       bdrv_get_device_name(bs));
         return;
     }
 
@@ -1158,13 +1158,13 @@ void do_loadvm(const char *name)
     /* restore the VM state */
     f = qemu_fopen_bdrv(bs, bdi->vm_state_offset, 0);
     if (!f) {
-        term_printf("Could not open VM state file\n");
+        monitor_printf("Could not open VM state file\n");
         goto the_end;
     }
     ret = qemu_loadvm_state(f);
     qemu_fclose(f);
     if (ret < 0) {
-        term_printf("Error %d while loading VM state\n", ret);
+        monitor_printf("Error %d while loading VM state\n", ret);
     }
  the_end:
     if (saved_vm_running)
@@ -1178,7 +1178,7 @@ void do_delvm(const char *name)
 
     bs = get_bs_snapshots();
     if (!bs) {
-        term_printf("No block device supports snapshots\n");
+        monitor_printf("No block device supports snapshots\n");
         return;
     }
 
@@ -1188,11 +1188,11 @@ void do_delvm(const char *name)
             ret = bdrv_snapshot_delete(bs1, name);
             if (ret < 0) {
                 if (ret == -ENOTSUP)
-                    term_printf("Snapshots not supported on device '%s'\n",
-                                bdrv_get_device_name(bs1));
+                    monitor_printf("Snapshots not supported on device '%s'\n",
+                                   bdrv_get_device_name(bs1));
                 else
-                    term_printf("Error %d while deleting snapshot on '%s'\n",
-                                ret, bdrv_get_device_name(bs1));
+                    monitor_printf("Error %d while deleting snapshot on '%s'\n",
+                                   ret, bdrv_get_device_name(bs1));
             }
         }
     }
@@ -1207,29 +1207,29 @@ void do_info_snapshots(void)
 
     bs = get_bs_snapshots();
     if (!bs) {
-        term_printf("No available block device supports snapshots\n");
+        monitor_printf("No available block device supports snapshots\n");
         return;
     }
-    term_printf("Snapshot devices:");
+    monitor_printf("Snapshot devices:");
     for(i = 0; i <= nb_drives; i++) {
         bs1 = drives_table[i].bdrv;
         if (bdrv_has_snapshot(bs1)) {
             if (bs == bs1)
-                term_printf(" %s", bdrv_get_device_name(bs1));
+                monitor_printf(" %s", bdrv_get_device_name(bs1));
         }
     }
-    term_printf("\n");
+    monitor_printf("\n");
 
     nb_sns = bdrv_snapshot_list(bs, &sn_tab);
     if (nb_sns < 0) {
-        term_printf("bdrv_snapshot_list: error %d\n", nb_sns);
+        monitor_printf("bdrv_snapshot_list: error %d\n", nb_sns);
         return;
     }
-    term_printf("Snapshot list (from %s):\n", bdrv_get_device_name(bs));
-    term_printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+    monitor_printf("Snapshot list (from %s):\n", bdrv_get_device_name(bs));
+    monitor_printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
     for(i = 0; i < nb_sns; i++) {
         sn = &sn_tab[i];
-        term_printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+        monitor_printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
     }
     qemu_free(sn_tab);
 }
diff --git a/slirp/misc.c b/slirp/misc.c
index f558b3c..5cd753c 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -557,14 +557,14 @@ relay(s)
 #endif
 
 #ifdef CONFIG_QEMU
-extern void term_vprintf(const char *fmt, va_list ap);
+extern void monitor_vprintf(const char *fmt, va_list ap);
 
 void lprint(const char *format, ...)
 {
     va_list args;
 
     va_start(args, format);
-    term_vprintf(format, args);
+    monitor_vprintf(format, args);
     va_end(args);
 }
 #else
diff --git a/usb-linux.c b/usb-linux.c
index f19f0c4..a1153fc 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -32,7 +32,7 @@
 
 #include "qemu-common.h"
 #include "qemu-timer.h"
-#include "console.h"
+#include "monitor.h"
 
 #include <dirent.h>
 #include <sys/ioctl.h>
@@ -998,7 +998,8 @@ USBDevice *usb_host_device_open(const char *devname)
         return NULL;
 
     if (hostdev_find(bus_num, addr)) {
-       term_printf("husb: host usb device %d.%d is already open\n", bus_num, addr);
+       monitor_printf("husb: host usb device %d.%d is already open\n",
+                      bus_num, addr);
        return NULL;
     }
 
@@ -1161,7 +1162,7 @@ static int usb_host_read_file(char *line, size_t line_size, const char *device_f
         fclose(f);
         ret = 1;
     } else {
-        term_printf("husb: could not open %s\n", filename);
+        monitor_printf("husb: could not open %s\n", filename);
     }
 
     return ret;
@@ -1292,14 +1293,15 @@ static int usb_host_scan(void *opaque, USBScanFunc *func)
         }
     found_devices:
         if (!usb_fs_type) {
-            term_printf("husb: unable to access USB devices\n");
+            monitor_printf("husb: unable to access USB devices\n");
             return -ENOENT;
         }
 
         /* the module setting (used later for opening devices) */
         usb_host_device_path = qemu_mallocz(strlen(devpath)+1);
         strcpy(usb_host_device_path, devpath);
-        term_printf("husb: using %s file-system with %s\n", fs_type[usb_fs_type], usb_host_device_path);
+        monitor_printf("husb: using %s file-system with %s\n",
+                       fs_type[usb_fs_type], usb_host_device_path);
     }
 
     switch (usb_fs_type) {
@@ -1623,17 +1625,17 @@ static void usb_info_device(int bus_num, int addr, int class_id,
         break;
     }
 
-    term_printf("  Device %d.%d, speed %s Mb/s\n",
+    monitor_printf("  Device %d.%d, speed %s Mb/s\n",
                 bus_num, addr, speed_str);
     class_str = usb_class_str(class_id);
     if (class_str)
-        term_printf("    %s:", class_str);
+        monitor_printf("    %s:", class_str);
     else
-        term_printf("    Class %02x:", class_id);
-    term_printf(" USB device %04x:%04x", vendor_id, product_id);
+        monitor_printf("    Class %02x:", class_id);
+    monitor_printf(" USB device %04x:%04x", vendor_id, product_id);
     if (product_name[0] != '\0')
-        term_printf(", %s", product_name);
-    term_printf("\n");
+        monitor_printf(", %s", product_name);
+    monitor_printf("\n");
 }
 
 static int usb_host_info_device(void *opaque, int bus_num, int addr,
@@ -1670,13 +1672,13 @@ void usb_host_info(void)
     usb_host_scan(NULL, usb_host_info_device);
 
     if (usb_auto_filter)
-        term_printf("  Auto filters:\n");
+        monitor_printf("  Auto filters:\n");
     for (f = usb_auto_filter; f; f = f->next) {
         char bus[10], addr[10], vid[10], pid[10];
         dec2str(f->bus_num, bus, sizeof(bus));
         dec2str(f->addr, addr, sizeof(addr));
         hex2str(f->vendor_id, vid, sizeof(vid));
         hex2str(f->product_id, pid, sizeof(pid));
-    	term_printf("    Device %s.%s ID %s:%s\n", bus, addr, vid, pid);
+        monitor_printf("    Device %s.%s ID %s:%s\n", bus, addr, vid, pid);
     }
 }
diff --git a/vl.c b/vl.c
index aff2b2c..b00d009 100644
--- a/vl.c
+++ b/vl.c
@@ -31,6 +31,7 @@
 #include "hw/baum.h"
 #include "hw/bt.h"
 #include "net.h"
+#include "monitor.h"
 #include "console.h"
 #include "sysemu.h"
 #include "gdbstub.h"
@@ -651,16 +652,16 @@ void do_info_mice(void)
     int index = 0;
 
     if (!qemu_put_mouse_event_head) {
-        term_printf("No mouse devices connected\n");
+        monitor_printf("No mouse devices connected\n");
         return;
     }
 
-    term_printf("Mouse devices available:\n");
+    monitor_printf("Mouse devices available:\n");
     cursor = qemu_put_mouse_event_head;
     while (cursor != NULL) {
-        term_printf("%c Mouse #%d: %s\n",
-                    (cursor == qemu_put_mouse_event_current ? '*' : ' '),
-                    index, cursor->qemu_put_mouse_event_name);
+        monitor_printf("%c Mouse #%d: %s\n",
+                       (cursor == qemu_put_mouse_event_current ? '*' : ' '),
+                       index, cursor->qemu_put_mouse_event_name);
         index++;
         cursor = cursor->next;
     }
@@ -672,7 +673,7 @@ void do_mouse_set(int index)
     int i = 0;
 
     if (!qemu_put_mouse_event_head) {
-        term_printf("No mouse devices connected\n");
+        monitor_printf("No mouse devices connected\n");
         return;
     }
 
@@ -685,7 +686,7 @@ void do_mouse_set(int index)
     if (cursor != NULL)
         qemu_put_mouse_event_current = cursor;
     else
-        term_printf("Mouse at given index not found\n");
+        monitor_printf("Mouse at given index not found\n");
 }
 
 /* compute with 96 bit intermediate result: (a*b)/c */
@@ -2712,7 +2713,7 @@ void usb_info(void)
     const char *speed_str;
 
     if (!usb_enabled) {
-        term_printf("USB support not enabled\n");
+        monitor_printf("USB support not enabled\n");
         return;
     }
 
@@ -2734,8 +2735,8 @@ void usb_info(void)
             speed_str = "?";
             break;
         }
-        term_printf("  Device %d.%d, Speed %s Mb/s, Product %s\n",
-                    0, dev->addr, speed_str, dev->devname);
+        monitor_printf("  Device %d.%d, Speed %s Mb/s, Product %s\n",
+                       0, dev->addr, speed_str, dev->devname);
     }
 }
 
@@ -2773,12 +2774,12 @@ void pcmcia_info(void)
 {
     struct pcmcia_socket_entry_s *iter;
     if (!pcmcia_sockets)
-        term_printf("No PCMCIA sockets\n");
+        monitor_printf("No PCMCIA sockets\n");
 
     for (iter = pcmcia_sockets; iter; iter = iter->next)
-        term_printf("%s: %s\n", iter->socket->slot_string,
-                    iter->socket->attached ? iter->socket->card_string :
-                    "Empty");
+        monitor_printf("%s: %s\n", iter->socket->slot_string,
+                       iter->socket->attached ? iter->socket->card_string :
+                       "Empty");
 }
 
 /***********************************************************/
@@ -4272,12 +4273,12 @@ int qemu_key_check(BlockDriverState *bs, const char *name)
     if (!bdrv_is_encrypted(bs))
         return 0;
 
-    term_printf("%s is encrypted.\n", name);
+    monitor_printf("%s is encrypted.\n", name);
     for(i = 0; i < 3; i++) {
         monitor_readline("Password: ", 1, password, sizeof(password));
         if (bdrv_set_key(bs, password) == 0)
             return 0;
-        term_printf("invalid password\n");
+        monitor_printf("invalid password\n");
     }
     return -EPERM;
 }
diff --git a/vnc.c b/vnc.c
index bdfc79b..63252a8 100644
--- a/vnc.c
+++ b/vnc.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu-common.h"
+#include "monitor.h"
 #include "console.h"
 #include "sysemu.h"
 #include "qemu_socket.h"
@@ -157,16 +158,16 @@ static DisplayChangeListener *dcl;
 void do_info_vnc(void)
 {
     if (vnc_state == NULL || vnc_state->display == NULL)
-	term_printf("VNC server disabled\n");
+	monitor_printf("VNC server disabled\n");
     else {
-	term_printf("VNC server active on: ");
-	term_print_filename(vnc_state->display);
-	term_printf("\n");
+	monitor_printf("VNC server active on: ");
+	monitor_print_filename(vnc_state->display);
+	monitor_printf("\n");
 
 	if (vnc_state->csock == -1)
-	    term_printf("No client connected\n");
+	    monitor_printf("No client connected\n");
 	else
-	    term_printf("Client connected\n");
+	    monitor_printf("Client connected\n");
     }
 }
 
@@ -776,7 +777,7 @@ static void audio_add(VncState *vs)
     struct audio_capture_ops ops;
 
     if (vs->audio_cap) {
-        term_printf ("audio already running\n");
+        monitor_printf("audio already running\n");
         return;
     }
 
@@ -786,7 +787,7 @@ static void audio_add(VncState *vs)
 
     vs->audio_cap = AUD_add_capture(NULL, &vs->as, &ops, vs);
     if (!vs->audio_cap) {
-        term_printf ("Failed to add audio capture\n");
+        monitor_printf("Failed to add audio capture\n");
     }
 }
 

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

* [Qemu-devel] [PATCH 06/17] char-mux: Use separate input buffers
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (2 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 02/17] block: Improve bdrv_iterate Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 03/17] block: Introduce bdrv_get_encrypted_filename Jan Kiszka
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Currently, the intermediate input buffer of mux'ed character devices
records data across all sub-devices. This has the side effect that we
easily leak data recorded over one sub-devices to another once we switch
the focus. Avoid data loss and confusion by defining exclusive buffers.

Note: In contrast to the original author's claim, the buffering concept
still breaks down when the fifo of the currently active sub-device is
full. As we cannot accept futher data from this point on without risking
to loose it, we will also miss escape sequences, just like without all
that buffering. In short: There is no reliable escape sequence handling
without infinite buffers or the risk of loosing some data.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 qemu-char.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 6e10c67..38bfab8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -209,12 +209,15 @@ typedef struct {
     IOEventHandler *chr_event[MAX_MUX];
     void *ext_opaque[MAX_MUX];
     CharDriverState *drv;
-    unsigned char buffer[MUX_BUFFER_SIZE];
-    int prod;
-    int cons;
     int mux_cnt;
     int term_got_escape;
     int max_size;
+    /* Intermediate input buffer allows to catch escape sequences even if the
+       currently active device is not accepting any input - but only until it
+       is full as well. */
+    unsigned char buffer[MAX_MUX][MUX_BUFFER_SIZE];
+    int prod[MAX_MUX];
+    int cons[MAX_MUX];
 } MuxDriver;
 
 
@@ -344,11 +347,11 @@ static void mux_chr_accept_input(CharDriverState *chr)
     int m = chr->focus;
     MuxDriver *d = chr->opaque;
 
-    while (d->prod != d->cons &&
+    while (d->prod[m] != d->cons[m] &&
            d->chr_can_read[m] &&
            d->chr_can_read[m](d->ext_opaque[m])) {
         d->chr_read[m](d->ext_opaque[m],
-                       &d->buffer[d->cons++ & MUX_BUFFER_MASK], 1);
+                       &d->buffer[m][d->cons[m]++ & MUX_BUFFER_MASK], 1);
     }
 }
 
@@ -356,11 +359,12 @@ static int mux_chr_can_read(void *opaque)
 {
     CharDriverState *chr = opaque;
     MuxDriver *d = chr->opaque;
+    int m = chr->focus;
 
-    if ((d->prod - d->cons) < MUX_BUFFER_SIZE)
+    if ((d->prod[m] - d->cons[m]) < MUX_BUFFER_SIZE)
         return 1;
-    if (d->chr_can_read[chr->focus])
-        return d->chr_can_read[chr->focus](d->ext_opaque[chr->focus]);
+    if (d->chr_can_read[m])
+        return d->chr_can_read[m](d->ext_opaque[m]);
     return 0;
 }
 
@@ -375,12 +379,12 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
 
     for(i = 0; i < size; i++)
         if (mux_proc_byte(chr, d, buf[i])) {
-            if (d->prod == d->cons &&
+            if (d->prod[m] == d->cons[m] &&
                 d->chr_can_read[m] &&
                 d->chr_can_read[m](d->ext_opaque[m]))
                 d->chr_read[m](d->ext_opaque[m], &buf[i], 1);
             else
-                d->buffer[d->prod++ & MUX_BUFFER_MASK] = buf[i];
+                d->buffer[m][d->prod[m]++ & MUX_BUFFER_MASK] = buf[i];
         }
 }
 

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

* [Qemu-devel] [PATCH 15/17] monitor: Improve mux'ed console experience
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (13 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 13/17] monitor: Introduce ReadLineState Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 12/17] monitor: Rework modal password input Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 11/17] monitor: Drop banner hiding Jan Kiszka
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Up to now, you never really knew if you already switched the console
after pressing CTRL-A C or if you mistyped it again. This patch
clarifies the situation by providing a prompt in a new line and
injecting a linebreak when switching away again. For this purpose, the
two events CHR_EVENT_MUX_IN and CHR_EVENT_MUX_OUT are introduced and
distributed on focus switches.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 monitor.c   |   26 ++++++++++++++++++++------
 qemu-char.c |   11 +++++++++--
 qemu-char.h |    8 +++++---
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/monitor.c b/monitor.c
index 498b223..2306b10 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2868,15 +2868,29 @@ static void term_event(void *opaque, int event)
 {
     MonitorTerm *old_term = cur_term;
 
-    if (event != CHR_EVENT_RESET)
-	return;
-
     cur_term = opaque;
 
-    monitor_printf("QEMU %s monitor - type 'help' for more information\n",
-                   QEMU_VERSION);
-    monitor_resume(cur_term);
+    switch (event) {
+    case CHR_EVENT_MUX_IN:
+        readline_restart(cur_term->rs);
+        monitor_resume(cur_term);
+        monitor_flush();
+        break;
 
+    case CHR_EVENT_MUX_OUT:
+        if (cur_term->suspend_cnt == 0)
+            monitor_printf("\n");
+        monitor_flush();
+        monitor_suspend();
+        break;
+
+    case CHR_EVENT_RESET:
+        monitor_printf("QEMU %s monitor - type 'help' for more information\n",
+                       QEMU_VERSION);
+        if (cur_term->chr->focus == 0)
+            monitor_resume(cur_term);
+        break;
+    }
     cur_term = old_term;
 }
 
diff --git a/qemu-char.c b/qemu-char.c
index db2fb44..d107c53 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -294,6 +294,12 @@ static void mux_print_help(CharDriverState *chr)
     }
 }
 
+static void mux_chr_send_event(MuxDriver *d, int mux_nr, int event)
+{
+    if (d->chr_event[mux_nr])
+        d->chr_event[mux_nr](d->ext_opaque[mux_nr], event);
+}
+
 static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
 {
     if (d->term_got_escape) {
@@ -325,9 +331,11 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
             break;
         case 'c':
             /* Switch to the next registered device */
+            mux_chr_send_event(d, chr->focus, CHR_EVENT_MUX_OUT);
             chr->focus++;
             if (chr->focus >= d->mux_cnt)
                 chr->focus = 0;
+            mux_chr_send_event(d, chr->focus, CHR_EVENT_MUX_IN);
             break;
        case 't':
            term_timestamps = !term_timestamps;
@@ -397,8 +405,7 @@ static void mux_chr_event(void *opaque, int event)
 
     /* Send the event to all registered listeners */
     for (i = 0; i < d->mux_cnt; i++)
-        if (d->chr_event[i])
-            d->chr_event[i](d->ext_opaque[i], event);
+        mux_chr_send_event(d, i, event);
 }
 
 static void mux_chr_update_read_handler(CharDriverState *chr)
diff --git a/qemu-char.h b/qemu-char.h
index bc0fcf3..b936244 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -4,9 +4,11 @@
 #include "sys-queue.h"
 /* character device */
 
-#define CHR_EVENT_BREAK 0 /* serial break char */
-#define CHR_EVENT_FOCUS 1 /* focus to this terminal (modal input needed) */
-#define CHR_EVENT_RESET 2 /* new connection established */
+#define CHR_EVENT_BREAK   0 /* serial break char */
+#define CHR_EVENT_FOCUS   1 /* focus to this terminal (modal input needed) */
+#define CHR_EVENT_RESET   2 /* new connection established */
+#define CHR_EVENT_MUX_IN  3 /* mux-focus was set to this terminal */
+#define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
 
 
 #define CHR_IOCTL_SERIAL_SET_PARAMS   1

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

* [Qemu-devel] [PATCH 16/17] monitor: Introduce MONITOR_USE_READLINE flag
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (10 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 17/17] monitor: Pass-through for gdbstub Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals Jan Kiszka
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

This allows to create monitor terminals that do not make use of the
interactive readline back-end but rather send complete commands. The
pass-through monitor interface of the gdbstub will be an example for
such terminals.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 migration.c |    6 +++++-
 monitor.c   |   50 +++++++++++++++++++++++++++++++++++++++-----------
 monitor.h   |    1 +
 qemu-char.c |    2 +-
 vl.c        |    2 +-
 5 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/migration.c b/migration.c
index 18c1492..c9365d7 100644
--- a/migration.c
+++ b/migration.c
@@ -128,7 +128,11 @@ void do_info_migrate(void)
 void migrate_fd_monitor_suspend(FdMigrationState *s)
 {
     s->mon_resume_handle = monitor_suspend();
-    dprintf("suspending monitor\n");
+    if (s->mon_resume_handle)
+        dprintf("suspending monitor\n");
+    else
+        monitor_printf("terminal does not allow synchronous migration, "
+                       "continuing detached\n");
 }
 
 void migrate_fd_error(FdMigrationState *s)
diff --git a/monitor.c b/monitor.c
index 2306b10..364b13d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -95,10 +95,16 @@ static void monitor_read_command(MonitorTerm *term, int show_prompt)
         readline_show_prompt(term->rs);
 }
 
-static void monitor_read_password(ReadLineFunc *readline_func, void *opaque)
+static int monitor_read_password(ReadLineFunc *readline_func, void *opaque)
 {
-    readline_start(cur_term->rs, "Password: ", 1, readline_func, opaque);
-    /* prompt is printed on return from the command handler */
+    if (cur_term->rs) {
+        readline_start(cur_term->rs, "Password: ", 1, readline_func, opaque);
+        /* prompt is printed on return from the command handler */
+        return 0;
+    } else {
+        monitor_printf("terminal does not support password prompting\n");
+        return -ENOTTY;
+    }
 }
 
 void monitor_flush(void)
@@ -385,6 +391,9 @@ static void do_info_history (void)
     int i;
     const char *str;
 
+    if (!cur_term->rs)
+        return;
+
     i = 0;
     for(;;) {
         str = readline_get_history(cur_term->rs, i);
@@ -2833,8 +2842,15 @@ static void term_read(void *opaque, const uint8_t *buf, int size)
 
     cur_term = opaque;
 
-    for (i = 0; i < size; i++)
-        readline_handle_byte(cur_term->rs, buf[i]);
+    if (cur_term->rs) {
+        for (i = 0; i < size; i++)
+            readline_handle_byte(cur_term->rs, buf[i]);
+    } else {
+        if (size == 0 || buf[size - 1] != 0)
+            monitor_printf("corrupted command\n");
+        else
+            monitor_handle_command((char *)buf);
+    }
 
     cur_term = old_term;
 }
@@ -2848,6 +2864,9 @@ static void monitor_command_cb(void *opaque, const char *cmdline)
 
 void *monitor_suspend(void)
 {
+    if (!cur_term->rs)
+        return NULL;
+
     cur_term->suspend_cnt++;
     return cur_term;
 }
@@ -2858,8 +2877,10 @@ void monitor_resume(void *handle)
 
     cur_term = handle;
 
-    if (--cur_term->suspend_cnt == 0)
-        readline_show_prompt(cur_term->rs);
+    if (cur_term->rs) {
+        if (--cur_term->suspend_cnt == 0)
+            readline_show_prompt(cur_term->rs);
+    }
 
     cur_term = old_term;
 }
@@ -2907,10 +2928,12 @@ void monitor_init(CharDriverState *chr, int flags)
     term = qemu_mallocz(sizeof(*term));
 
     term->chr = chr;
-    term->flags = flags;
+    term->flags = flags & (MONITOR_IS_DEFAULT | MONITOR_USE_READLINE);
     term->suspend_cnt = 1; /* resume on reset */
-    term->rs = readline_init(monitor_find_completion);
-    monitor_read_command(term, 0);
+    if (flags & MONITOR_USE_READLINE) {
+        term->rs = readline_init(monitor_find_completion);
+        monitor_read_command(term, 0);
+    }
 
     qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, term);
 
@@ -2938,6 +2961,8 @@ void monitor_read_bdrv_key_start(BlockDriverState *bs,
                                  BlockDriverCompletionFunc *completion_cb,
                                  void *opaque)
 {
+    int err;
+
     if (!bdrv_key_required(bs)) {
         if (completion_cb)
             completion_cb(opaque, 0);
@@ -2950,5 +2975,8 @@ void monitor_read_bdrv_key_start(BlockDriverState *bs,
     cur_term->password_completion_cb = completion_cb;
     cur_term->password_opaque = opaque;
 
-    monitor_read_password(bdrv_password_cb, bs);
+    err = monitor_read_password(bdrv_password_cb, bs);
+
+    if (err && completion_cb)
+        completion_cb(opaque, err);
 }
diff --git a/monitor.h b/monitor.h
index 7af267d..bb2a06a 100644
--- a/monitor.h
+++ b/monitor.h
@@ -6,6 +6,7 @@
 
 /* flags for monitor_init */
 #define MONITOR_IS_DEFAULT    0x01
+#define MONITOR_USE_READLINE  0x02
 
 void monitor_init(CharDriverState *chr, int flags);
 void *monitor_suspend(void);
diff --git a/qemu-char.c b/qemu-char.c
index d107c53..b7480a0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2102,7 +2102,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, 0);
+            monitor_init(chr, MONITOR_USE_READLINE);
         } else {
             printf("Unable to open driver: %s\n", p);
         }
diff --git a/vl.c b/vl.c
index a80a25b..d569e6b 100644
--- a/vl.c
+++ b/vl.c
@@ -5594,7 +5594,7 @@ int main(int argc, char **argv, char **envp)
     text_consoles_set_display(display_state);
 
     if (monitor_device && monitor_hd)
-        monitor_init(monitor_hd, MONITOR_IS_DEFAULT);
+        monitor_init(monitor_hd, MONITOR_USE_READLINE | MONITOR_IS_DEFAULT);
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         const char *devname = serial_devices[i];

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

* [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (11 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 16/17] monitor: Introduce MONITOR_USE_READLINE flag Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-09 15:16   ` Anthony Liguori
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 13/17] monitor: Introduce ReadLineState Jan Kiszka
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

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 an
additional terminal can be a multiplexed device channel or a gdb
frontend connected to QEMU's stub.

Therefor, this patch decouples the buffers and states of all monitor
terminals, allowing the user to operate them independently. The basic
idea is stolen from Jason Wessel: When starting to handle a monitor
command or some terminal event, the current monitor terminal is set to
the one associated with the underlying char device, letting all
succeeding monitor_printf show up on only this selected terminal.

There are still two asynchronous monitor writers: some error reporting
in VNC's audio_add and the log-to-monitor feature of the audio
subsystem. In order to keep them happy, the concept of a default monitor
terminal is used, which is either the dedicated monitor terminal or the
first registered mux. If we can agree on converting or dropping
asynchronous monitor users, we would also be able to drop the default
terminal concept again.

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

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 migration-exec.c |   11 +---
 migration-tcp.c  |   11 +---
 migration.c      |   12 +++-
 migration.h      |    4 +
 monitor.c        |  159 +++++++++++++++++++++++++++++++-----------------------
 monitor.h        |    8 ++-
 qemu-char.c      |    2 -
 readline.c       |    3 -
 readline.h       |    1 
 vl.c             |    2 -
 10 files changed, 119 insertions(+), 94 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 2c92340..d2c79d6 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -55,7 +55,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 +89,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_handle = NULL;
     s->bandwidth_limit = bandwidth_limit;
 
-    if (s->detach == 1) {
-        dprintf("detaching from monitor\n");
-        monitor_suspend();
-        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 c5e102c..6c414ac 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -79,7 +79,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 +98,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_handle = NULL;
     s->bandwidth_limit = bandwidth_limit;
     s->fd = socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
@@ -108,11 +108,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();
-        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 a7f9355..18c1492 100644
--- a/migration.c
+++ b/migration.c
@@ -125,6 +125,12 @@ void do_info_migrate(void)
 
 /* shared migration helpers */
 
+void migrate_fd_monitor_suspend(FdMigrationState *s)
+{
+    s->mon_resume_handle = monitor_suspend();
+    dprintf("suspending monitor\n");
+}
+
 void migrate_fd_error(FdMigrationState *s)
 {
     dprintf("setting error state\n");
@@ -145,10 +151,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();
-        s->detach = 0;
-    }
+    if (s->mon_resume_handle)
+        monitor_resume(s->mon_resume_handle);
 
     s->fd = -1;
 }
diff --git a/migration.h b/migration.h
index d9771ad..0ed7818 100644
--- a/migration.h
+++ b/migration.h
@@ -37,7 +37,7 @@ struct FdMigrationState
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
-    int detach;
+    void *mon_resume_handle;
     int state;
     int (*get_error)(struct FdMigrationState*);
     int (*close)(struct FdMigrationState*);
@@ -67,6 +67,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 7b844ac..498b223 100644
--- a/monitor.c
+++ b/monitor.c
@@ -69,39 +69,45 @@ typedef struct term_cmd_t {
 
 typedef struct MonitorTerm {
     CharDriverState *chr;
+    int flags;
+    int suspend_cnt;
+    uint8_t term_outbuf[1024];
+    int term_outbuf_index;
+    ReadLineState *rs;
+    CPUState *mon_cpu;
+    BlockDriverCompletionFunc *password_completion_cb;
+    void *password_opaque;
     LIST_ENTRY(MonitorTerm) entry;
 } MonitorTerm;
 
 static LIST_HEAD(term_list, MonitorTerm) term_list;
+static MonitorTerm *cur_term;
 
 static const term_cmd_t term_cmds[];
 static const term_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;
+static void monitor_command_cb(void *opaque, const char *cmdline);
 
-static void monitor_start_input(void);
-
-static CPUState *mon_cpu = NULL;
+static void monitor_read_command(MonitorTerm *term, int show_prompt)
+{
+    readline_start(term->rs, "(qemu) ", 0, monitor_command_cb, NULL);
+    if (show_prompt)
+        readline_show_prompt(term->rs);
+}
 
 static void monitor_read_password(ReadLineFunc *readline_func, void *opaque)
 {
-    readline_start(rs, "Password: ", 1, readline_func, opaque);
+    readline_start(cur_term->rs, "Password: ", 1, readline_func, opaque);
+    /* prompt is printed on return from the command handler */
 }
 
 void monitor_flush(void)
 {
-    MonitorTerm *term;
-
-    if (term_outbuf_index > 0) {
-        LIST_FOREACH(term, &term_list, entry) {
-            if (term->chr->focus == 0)
-                qemu_chr_write(term->chr, term_outbuf, term_outbuf_index);
-        }
-        term_outbuf_index = 0;
+    if (cur_term && cur_term->term_outbuf_index != 0
+        && cur_term->chr->focus == 0) {
+        qemu_chr_write(cur_term->chr, cur_term->term_outbuf,
+                       cur_term->term_outbuf_index);
+        cur_term->term_outbuf_index = 0;
     }
 }
 
@@ -109,15 +115,19 @@ void monitor_flush(void)
 static void monitor_puts(const char *str)
 {
     char c;
+
+    if (!cur_term)
+        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')
+            cur_term->term_outbuf[cur_term->term_outbuf_index++] = '\r';
+        cur_term->term_outbuf[cur_term->term_outbuf_index++] = c;
+        if (cur_term->term_outbuf_index >= (sizeof(cur_term->term_outbuf) - 1)
+            || c == '\n')
             monitor_flush();
     }
 }
@@ -301,7 +311,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_term->mon_cpu = env;
             return 0;
         }
     }
@@ -310,10 +320,10 @@ static int mon_set_cpu(int cpu_index)
 
 static CPUState *mon_get_cpu(void)
 {
-    if (!mon_cpu) {
+    if (!cur_term->mon_cpu) {
         mon_set_cpu(0);
     }
-    return mon_cpu;
+    return cur_term->mon_cpu;
 }
 
 static void do_info_registers(void)
@@ -340,7 +350,7 @@ static void do_info_cpus(void)
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         monitor_printf("%c CPU #%d:",
-                       (env == mon_cpu) ? '*' : ' ',
+                       (env == cur_term->mon_cpu) ? '*' : ' ',
                        env->cpu_index);
 #if defined(TARGET_I386)
         monitor_printf(" pc=0x" TARGET_FMT_lx,
@@ -377,7 +387,7 @@ static void do_info_history (void)
 
     i = 0;
     for(;;) {
-        str = readline_get_history(rs, i);
+        str = readline_get_history(cur_term->rs, i);
         if (!str)
             break;
 	monitor_printf("%d: '%s'\n", i, str);
@@ -459,7 +469,7 @@ static void change_vnc_password_cb(void *opaque, const char *password)
     if (vnc_display_password(NULL, password) < 0)
         monitor_printf("could not set VNC server password\n");
 
-    monitor_start_input();
+    monitor_read_command(cur_term, 1);
 }
 
 static void do_change_vnc(const char *target, const char *arg)
@@ -2629,7 +2639,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_term->rs, cmd);
         }
         if (*p == '\0')
             break;
@@ -2682,7 +2692,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_term->rs, file);
         }
     }
     closedir(ffs);
@@ -2695,7 +2705,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_term->rs, name);
     }
 }
 
@@ -2755,7 +2765,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_term->rs, strlen(cmdname));
         for(cmd = term_cmds; cmd->name != NULL; cmd++) {
             cmd_completion(cmdname, cmd->name);
         }
@@ -2779,23 +2789,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_term->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_term->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_term->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_term->rs, strlen(str));
                 for(key = key_defs; key->name != NULL; key++) {
                     cmd_completion(str, key->name);
                 }
@@ -2811,58 +2821,68 @@ static void monitor_find_completion(const char *cmdline)
 
 static int term_can_read(void *opaque)
 {
-    return 128;
+    MonitorTerm *term = opaque;
+
+    return (term->suspend_cnt == 0) ? 128 : 0;
 }
 
 static void term_read(void *opaque, const uint8_t *buf, int size)
 {
+    MonitorTerm *old_term = cur_term;
     int i;
-    for(i = 0; i < size; i++)
-        readline_handle_byte(rs, buf[i]);
-}
 
-static int monitor_suspended;
+    cur_term = opaque;
 
-static void monitor_handle_command1(void *opaque, const char *cmdline)
-{
-    monitor_handle_command(cmdline);
-    if (!monitor_suspended)
-        monitor_start_input();
-    else
-        monitor_suspended = 2;
+    for (i = 0; i < size; i++)
+        readline_handle_byte(cur_term->rs, buf[i]);
+
+    cur_term = old_term;
 }
 
-void monitor_suspend(void)
+static void monitor_command_cb(void *opaque, const char *cmdline)
 {
-    monitor_suspended = 1;
+    monitor_suspend();
+    monitor_handle_command(cmdline);
+    monitor_resume(cur_term);
 }
 
-void monitor_resume(void)
+void *monitor_suspend(void)
 {
-    if (monitor_suspended == 2)
-        monitor_start_input();
-    monitor_suspended = 0;
+    cur_term->suspend_cnt++;
+    return cur_term;
 }
 
-static void monitor_start_input(void)
+void monitor_resume(void *handle)
 {
-    readline_start(rs, "(qemu) ", 0, monitor_handle_command1, NULL);
+    MonitorTerm *old_term = cur_term;
+
+    cur_term = handle;
+
+    if (--cur_term->suspend_cnt == 0)
+        readline_show_prompt(cur_term->rs);
+
+    cur_term = old_term;
 }
 
 static void term_event(void *opaque, int event)
 {
+    MonitorTerm *old_term = cur_term;
+
     if (event != CHR_EVENT_RESET)
 	return;
 
+    cur_term = opaque;
+
     monitor_printf("QEMU %s monitor - type 'help' for more information\n",
                    QEMU_VERSION);
-    monitor_start_input();
-}
+    monitor_resume(cur_term);
 
-static int is_first_init = 1;
+    cur_term = old_term;
+}
 
-void monitor_init(CharDriverState *chr)
+void monitor_init(CharDriverState *chr, int flags)
 {
+    static int is_first_init = 1;
     MonitorTerm *term;
 
     if (is_first_init) {
@@ -2873,13 +2893,16 @@ void monitor_init(CharDriverState *chr)
     term = qemu_mallocz(sizeof(*term));
 
     term->chr = chr;
-    rs = readline_init(monitor_find_completion);
+    term->flags = flags;
+    term->suspend_cnt = 1; /* resume on reset */
+    term->rs = readline_init(monitor_find_completion);
+    monitor_read_command(term, 0);
 
-    qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, NULL);
+    qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, term);
 
     LIST_INSERT_HEAD(&term_list, term, entry);
-
-    readline_start(rs, "", 0, monitor_handle_command1, NULL);
+    if (!cur_term || (flags & MONITOR_IS_DEFAULT))
+        cur_term = term;
 }
 
 static void bdrv_password_cb(void *opaque, const char *password)
@@ -2891,10 +2914,10 @@ static void bdrv_password_cb(void *opaque, const char *password)
         monitor_printf("invalid password\n");
         ret = -EPERM;
     }
-    if (password_completion_cb)
-        password_completion_cb(password_opaque, ret);
+    if (cur_term->password_completion_cb)
+        cur_term->password_completion_cb(cur_term->password_opaque, ret);
 
-    monitor_start_input();
+    monitor_read_command(cur_term, 1);
 }
 
 void monitor_read_bdrv_key_start(BlockDriverState *bs,
@@ -2910,8 +2933,8 @@ void monitor_read_bdrv_key_start(BlockDriverState *bs,
     monitor_printf("%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
                    bdrv_get_encrypted_filename(bs));
 
-    password_completion_cb = completion_cb;
-    password_opaque = opaque;
+    cur_term->password_completion_cb = completion_cb;
+    cur_term->password_opaque = opaque;
 
     monitor_read_password(bdrv_password_cb, bs);
 }
diff --git a/monitor.h b/monitor.h
index 4b35a25..7af267d 100644
--- a/monitor.h
+++ b/monitor.h
@@ -4,10 +4,12 @@
 #include "qemu-char.h"
 #include "block.h"
 
-void monitor_init(CharDriverState *chr);
-void monitor_suspend(void);
-void monitor_resume(void);
+/* flags for monitor_init */
+#define MONITOR_IS_DEFAULT    0x01
 
+void monitor_init(CharDriverState *chr, int flags);
+void *monitor_suspend(void);
+void monitor_resume(void *handle);
 void monitor_read_bdrv_key_start(BlockDriverState *bs,
                                  BlockDriverCompletionFunc *completion_cb,
                                  void *opaque);
diff --git a/qemu-char.c b/qemu-char.c
index 5c2ee4e..db2fb44 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2095,7 +2095,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/readline.c b/readline.c
index 6404674..a0b234d 100644
--- a/readline.c
+++ b/readline.c
@@ -31,7 +31,7 @@
 
 #define printf do_not_use_printf
 
-static void readline_show_prompt(ReadLineState *rs)
+void readline_show_prompt(ReadLineState *rs)
 {
     monitor_printf("%s", rs->prompt);
     monitor_flush();
@@ -445,7 +445,6 @@ void readline_start(ReadLineState *rs, const char *prompt, int read_password,
     rs->readline_opaque = opaque;
     rs->read_password = read_password;
     readline_restart(rs);
-    readline_show_prompt(rs);
 }
 
 void readline_restart(ReadLineState *rs)
diff --git a/readline.h b/readline.h
index e385ac4..5247025 100644
--- a/readline.h
+++ b/readline.h
@@ -41,6 +41,7 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index);
 void readline_start(ReadLineState *rs, const char *prompt, int read_password,
                     ReadLineFunc *readline_func, void *opaque);
 void readline_restart(ReadLineState *rs);
+void readline_show_prompt(ReadLineState *rs);
 ReadLineState *readline_init(ReadLineCompletionFunc *completion_finder);
 
 #endif /* !READLINE_H */
diff --git a/vl.c b/vl.c
index f5b6171..a80a25b 100644
--- a/vl.c
+++ b/vl.c
@@ -5594,7 +5594,7 @@ int main(int argc, char **argv, char **envp)
     text_consoles_set_display(display_state);
 
     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];

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

* [Qemu-devel] [PATCH 17/17] monitor: Pass-through for gdbstub
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (9 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 04/17] monitor: Report encrypted disks in snapshot mode Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 16/17] monitor: Introduce MONITOR_USE_READLINE flag Jan Kiszka
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Create a monitor terminal and pass it through the gdbstub. This allows
to use gdb's monitor command to access the QEMU monitor. Works for all
commands except for non-detached migration and password retrieval (user
will receive error messages instead).

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 gdbstub.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 239f2e0..27e887e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -30,6 +30,7 @@
 
 #include "qemu.h"
 #else
+#include "monitor.h"
 #include "qemu-char.h"
 #include "sysemu.h"
 #include "gdbstub.h"
@@ -285,6 +286,7 @@ typedef struct GDBState {
     int running_state;
 #else
     CharDriverState *chr;
+    CharDriverState *mon_chr;
 #endif
 } GDBState;
 
@@ -1819,7 +1821,22 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             put_packet(s, buf);
             break;
         }
-#endif
+#else /* !CONFIG_LINUX_USER */
+        else if (strncmp(p, "Rcmd,", 5) == 0) {
+            int len = strlen(p + 5);
+
+            if ((len % 2) != 0) {
+                put_packet(s, "E01");
+                break;
+            }
+            hextomem(mem_buf, p + 5, len);
+            len = len / 2;
+            mem_buf[len++] = 0;
+            qemu_chr_read(s->mon_chr, mem_buf, len);
+            put_packet(s, "OK");
+            break;
+        }
+#endif /* !CONFIG_LINUX_USER */
         if (strncmp(p, "Supported", 9) == 0) {
             snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH);
 #ifdef GDB_CORE_XML
@@ -2282,6 +2299,35 @@ static void gdb_chr_event(void *opaque, int event)
     }
 }
 
+static void gdb_monitor_output(GDBState *s, const char *msg, int len)
+{
+    char buf[MAX_PACKET_LENGTH];
+
+    buf[0] = 'O';
+    if (len > (MAX_PACKET_LENGTH/2) - 1)
+        len = (MAX_PACKET_LENGTH/2) - 1;
+    memtohex(buf + 1, (uint8_t *)msg, len);
+    put_packet(s, buf);
+}
+
+static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+    const char *p = (const char *)buf;
+    int max_sz;
+
+    max_sz = (sizeof(gdbserver_state->last_packet) - 2) / 2;
+    for (;;) {
+        if (len <= max_sz) {
+            gdb_monitor_output(gdbserver_state, p, len);
+            break;
+        }
+        gdb_monitor_output(gdbserver_state, p, max_sz);
+        p += max_sz;
+        len -= max_sz;
+    }
+    return len;
+}
+
 int gdbserver_start(const char *port)
 {
     GDBState *s;
@@ -2313,6 +2359,12 @@ int gdbserver_start(const char *port)
     qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
                           gdb_chr_event, NULL);
     qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
+
+    /* Initialize a monitor terminal for gdb */
+    s->mon_chr = qemu_mallocz(sizeof(*s->mon_chr));
+    s->mon_chr->chr_write = gdb_monitor_write;
+    monitor_init(s->mon_chr, 0);
+
     return 0;
 }
 #endif

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

* [Qemu-devel] [PATCH 13/17] monitor: Introduce ReadLineState
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (12 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 15/17] monitor: Improve mux'ed console experience Jan Kiszka
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

As another step towards decoupled monitor terminals encapsulate the
state of the readline processor in a separate data structure called
ReadLineState and adapt all interfaces appropriately. For now the
monitor continues to instantiate just a single readline state.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 monitor.c  |   30 +++--
 readline.c |  384 +++++++++++++++++++++++++++++-------------------------------
 readline.h |   44 ++++++-
 3 files changed, 241 insertions(+), 217 deletions(-)

diff --git a/monitor.c b/monitor.c
index 51e6d9f..7b844ac 100644
--- a/monitor.c
+++ b/monitor.c
@@ -81,6 +81,7 @@ static uint8_t term_outbuf[1024];
 static int term_outbuf_index;
 static BlockDriverCompletionFunc *password_completion_cb;
 static void *password_opaque;
+ReadLineState *rs;
 
 static void monitor_start_input(void);
 
@@ -88,7 +89,7 @@ static CPUState *mon_cpu = NULL;
 
 static void monitor_read_password(ReadLineFunc *readline_func, void *opaque)
 {
-    readline_start("Password: ", 1, readline_func, opaque);
+    readline_start(rs, "Password: ", 1, readline_func, opaque);
 }
 
 void monitor_flush(void)
@@ -376,7 +377,7 @@ static void do_info_history (void)
 
     i = 0;
     for(;;) {
-        str = readline_get_history(i);
+        str = readline_get_history(rs, i);
         if (!str)
             break;
 	monitor_printf("%d: '%s'\n", i, str);
@@ -2628,7 +2629,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))) {
-            add_completion(cmd);
+            readline_add_completion(rs, cmd);
         }
         if (*p == '\0')
             break;
@@ -2681,7 +2682,7 @@ static void file_completion(const char *input)
             stat(file, &sb);
             if(S_ISDIR(sb.st_mode))
                 pstrcat(file, sizeof(file), "/");
-            add_completion(file);
+            readline_add_completion(rs, file);
         }
     }
     closedir(ffs);
@@ -2694,7 +2695,7 @@ static void block_completion_it(void *opaque, BlockDriverState *bs)
 
     if (input[0] == '\0' ||
         !strncmp(name, (char *)input, strlen(input))) {
-        add_completion(name);
+        readline_add_completion(rs, name);
     }
 }
 
@@ -2724,7 +2725,7 @@ static void parse_cmdline(const char *cmdline,
     *pnb_args = nb_args;
 }
 
-void readline_find_completion(const char *cmdline)
+static void monitor_find_completion(const char *cmdline)
 {
     const char *cmdname;
     char *args[MAX_ARGS];
@@ -2754,7 +2755,7 @@ void readline_find_completion(const char *cmdline)
             cmdname = "";
         else
             cmdname = args[0];
-        completion_index = strlen(cmdname);
+        readline_set_completion_index(rs, strlen(cmdname));
         for(cmd = term_cmds; cmd->name != NULL; cmd++) {
             cmd_completion(cmdname, cmd->name);
         }
@@ -2778,23 +2779,23 @@ void readline_find_completion(const char *cmdline)
         switch(*ptype) {
         case 'F':
             /* file completion */
-            completion_index = strlen(str);
+            readline_set_completion_index(rs, strlen(str));
             file_completion(str);
             break;
         case 'B':
             /* block device name completion */
-            completion_index = strlen(str);
+            readline_set_completion_index(rs, strlen(str));
             bdrv_iterate(block_completion_it, (void *)str);
             break;
         case 's':
             /* XXX: more generic ? */
             if (!strcmp(cmd->name, "info")) {
-                completion_index = strlen(str);
+                readline_set_completion_index(rs, strlen(str));
                 for(cmd = info_cmds; cmd->name != NULL; cmd++) {
                     cmd_completion(str, cmd->name);
                 }
             } else if (!strcmp(cmd->name, "sendkey")) {
-                completion_index = strlen(str);
+                readline_set_completion_index(rs, strlen(str));
                 for(key = key_defs; key->name != NULL; key++) {
                     cmd_completion(str, key->name);
                 }
@@ -2817,7 +2818,7 @@ static void term_read(void *opaque, const uint8_t *buf, int size)
 {
     int i;
     for(i = 0; i < size; i++)
-        readline_handle_byte(buf[i]);
+        readline_handle_byte(rs, buf[i]);
 }
 
 static int monitor_suspended;
@@ -2845,7 +2846,7 @@ void monitor_resume(void)
 
 static void monitor_start_input(void)
 {
-    readline_start("(qemu) ", 0, monitor_handle_command1, NULL);
+    readline_start(rs, "(qemu) ", 0, monitor_handle_command1, NULL);
 }
 
 static void term_event(void *opaque, int event)
@@ -2872,12 +2873,13 @@ void monitor_init(CharDriverState *chr)
     term = qemu_mallocz(sizeof(*term));
 
     term->chr = chr;
+    rs = readline_init(monitor_find_completion);
 
     qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, NULL);
 
     LIST_INSERT_HEAD(&term_list, term, entry);
 
-    readline_start("", 0, monitor_handle_command1, NULL);
+    readline_start(rs, "", 0, monitor_handle_command1, NULL);
 }
 
 static void bdrv_password_cb(void *opaque, const char *password)
diff --git a/readline.c b/readline.c
index 4d8a40d..6404674 100644
--- a/readline.c
+++ b/readline.c
@@ -25,80 +25,46 @@
 #include "monitor.h"
 #include "readline.h"
 
-#define TERM_CMD_BUF_SIZE 4095
-#define TERM_MAX_CMDS 64
-#define NB_COMPLETIONS_MAX 256
-
 #define IS_NORM 0
 #define IS_ESC  1
 #define IS_CSI  2
 
 #define printf do_not_use_printf
 
-static char term_cmd_buf[TERM_CMD_BUF_SIZE + 1];
-static int term_cmd_buf_index;
-static int term_cmd_buf_size;
-
-static char term_last_cmd_buf[TERM_CMD_BUF_SIZE + 1];
-static int term_last_cmd_buf_index;
-static int term_last_cmd_buf_size;
-
-static int term_esc_state;
-static int term_esc_param;
-
-static char *term_history[TERM_MAX_CMDS];
-static int term_hist_entry = -1;
-
-static int nb_completions;
-int completion_index;
-static char *completions[NB_COMPLETIONS_MAX];
-
-static ReadLineFunc *term_readline_func;
-static int term_is_password;
-static char term_prompt[256];
-static void *term_readline_opaque;
-
-static void term_show_prompt2(void)
+static void readline_show_prompt(ReadLineState *rs)
 {
-    monitor_printf("%s", term_prompt);
+    monitor_printf("%s", rs->prompt);
     monitor_flush();
-    term_last_cmd_buf_index = 0;
-    term_last_cmd_buf_size = 0;
-    term_esc_state = IS_NORM;
-}
-
-static void term_show_prompt(void)
-{
-    term_show_prompt2();
-    term_cmd_buf_index = 0;
-    term_cmd_buf_size = 0;
+    rs->last_cmd_buf_index = 0;
+    rs->last_cmd_buf_size = 0;
+    rs->esc_state = IS_NORM;
 }
 
 /* update the displayed command line */
-static void term_update(void)
+static void readline_update(ReadLineState *rs)
 {
     int i, delta, len;
 
-    if (term_cmd_buf_size != term_last_cmd_buf_size ||
-        memcmp(term_cmd_buf, term_last_cmd_buf, term_cmd_buf_size) != 0) {
-        for(i = 0; i < term_last_cmd_buf_index; i++) {
+    if (rs->cmd_buf_size != rs->last_cmd_buf_size ||
+        memcmp(rs->cmd_buf, rs->last_cmd_buf, rs->cmd_buf_size) != 0) {
+        for(i = 0; i < rs->last_cmd_buf_index; i++) {
             monitor_printf("\033[D");
         }
-        term_cmd_buf[term_cmd_buf_size] = '\0';
-        if (term_is_password) {
-            len = strlen(term_cmd_buf);
+        rs->cmd_buf[rs->cmd_buf_size] = '\0';
+        if (rs->read_password) {
+            len = strlen(rs->cmd_buf);
             for(i = 0; i < len; i++)
                 monitor_printf("*");
         } else {
-            monitor_printf("%s", term_cmd_buf);
+            monitor_printf("%s", rs->cmd_buf);
         }
         monitor_printf("\033[K");
-        memcpy(term_last_cmd_buf, term_cmd_buf, term_cmd_buf_size);
-        term_last_cmd_buf_size = term_cmd_buf_size;
-        term_last_cmd_buf_index = term_cmd_buf_size;
+        memcpy(rs->last_cmd_buf, rs->cmd_buf, rs->cmd_buf_size);
+        rs->last_cmd_buf_size = rs->cmd_buf_size;
+        rs->last_cmd_buf_index = rs->cmd_buf_size;
     }
-    if (term_cmd_buf_index != term_last_cmd_buf_index) {
-        delta = term_cmd_buf_index - term_last_cmd_buf_index;
+    if (rs->cmd_buf_index != rs->last_cmd_buf_index) {
+        delta = rs->cmd_buf_index - rs->last_cmd_buf_index;
         if (delta > 0) {
             for(i = 0;i < delta; i++) {
                 monitor_printf("\033[C");
@@ -109,68 +75,68 @@ static void term_update(void)
                 monitor_printf("\033[D");
             }
         }
-        term_last_cmd_buf_index = term_cmd_buf_index;
+        rs->last_cmd_buf_index = rs->cmd_buf_index;
     }
     monitor_flush();
 }
 
-static void term_insert_char(int ch)
+static void readline_insert_char(ReadLineState *rs, int ch)
 {
-    if (term_cmd_buf_index < TERM_CMD_BUF_SIZE) {
-        memmove(term_cmd_buf + term_cmd_buf_index + 1,
-                term_cmd_buf + term_cmd_buf_index,
-                term_cmd_buf_size - term_cmd_buf_index);
-        term_cmd_buf[term_cmd_buf_index] = ch;
-        term_cmd_buf_size++;
-        term_cmd_buf_index++;
+    if (rs->cmd_buf_index < READLINE_CMD_BUF_SIZE) {
+        memmove(rs->cmd_buf + rs->cmd_buf_index + 1,
+                rs->cmd_buf + rs->cmd_buf_index,
+                rs->cmd_buf_size - rs->cmd_buf_index);
+        rs->cmd_buf[rs->cmd_buf_index] = ch;
+        rs->cmd_buf_size++;
+        rs->cmd_buf_index++;
     }
 }
 
-static void term_backward_char(void)
+static void readline_backward_char(ReadLineState *rs)
 {
-    if (term_cmd_buf_index > 0) {
-        term_cmd_buf_index--;
+    if (rs->cmd_buf_index > 0) {
+        rs->cmd_buf_index--;
     }
 }
 
-static void term_forward_char(void)
+static void readline_forward_char(ReadLineState *rs)
 {
-    if (term_cmd_buf_index < term_cmd_buf_size) {
-        term_cmd_buf_index++;
+    if (rs->cmd_buf_index < rs->cmd_buf_size) {
+        rs->cmd_buf_index++;
     }
 }
 
-static void term_delete_char(void)
+static void readline_delete_char(ReadLineState *rs)
 {
-    if (term_cmd_buf_index < term_cmd_buf_size) {
-        memmove(term_cmd_buf + term_cmd_buf_index,
-                term_cmd_buf + term_cmd_buf_index + 1,
-                term_cmd_buf_size - term_cmd_buf_index - 1);
-        term_cmd_buf_size--;
+    if (rs->cmd_buf_index < rs->cmd_buf_size) {
+        memmove(rs->cmd_buf + rs->cmd_buf_index,
+                rs->cmd_buf + rs->cmd_buf_index + 1,
+                rs->cmd_buf_size - rs->cmd_buf_index - 1);
+        rs->cmd_buf_size--;
     }
 }
 
-static void term_backspace(void)
+static void readline_backspace(ReadLineState *rs)
 {
-    if (term_cmd_buf_index > 0) {
-        term_backward_char();
-        term_delete_char();
+    if (rs->cmd_buf_index > 0) {
+        readline_backward_char(rs);
+        readline_delete_char(rs);
     }
 }
 
-static void term_backword(void)
+static void readline_backword(ReadLineState *rs)
 {
     int start;
 
-    if (term_cmd_buf_index == 0 || term_cmd_buf_index > term_cmd_buf_size) {
+    if (rs->cmd_buf_index == 0 || rs->cmd_buf_index > rs->cmd_buf_size) {
         return;
     }
 
-    start = term_cmd_buf_index - 1;
+    start = rs->cmd_buf_index - 1;
 
     /* find first word (backwards) */
     while (start > 0) {
-        if (!qemu_isspace(term_cmd_buf[start])) {
+        if (!qemu_isspace(rs->cmd_buf[start])) {
             break;
         }
 
@@ -179,7 +145,7 @@ static void term_backword(void)
 
     /* find first space (backwards) */
     while (start > 0) {
-        if (qemu_isspace(term_cmd_buf[start])) {
+        if (qemu_isspace(rs->cmd_buf[start])) {
             ++start;
             break;
         }
@@ -188,61 +154,61 @@ static void term_backword(void)
     }
 
     /* remove word */
-    if (start < term_cmd_buf_index) {
-        memmove(term_cmd_buf + start,
-                term_cmd_buf + term_cmd_buf_index,
-                term_cmd_buf_size - term_cmd_buf_index);
-        term_cmd_buf_size -= term_cmd_buf_index - start;
-        term_cmd_buf_index = start;
+    if (start < rs->cmd_buf_index) {
+        memmove(rs->cmd_buf + start,
+                rs->cmd_buf + rs->cmd_buf_index,
+                rs->cmd_buf_size - rs->cmd_buf_index);
+        rs->cmd_buf_size -= rs->cmd_buf_index - start;
+        rs->cmd_buf_index = start;
     }
 }
 
-static void term_bol(void)
+static void readline_bol(ReadLineState *rs)
 {
-    term_cmd_buf_index = 0;
+    rs->cmd_buf_index = 0;
 }
 
-static void term_eol(void)
+static void readline_eol(ReadLineState *rs)
 {
-    term_cmd_buf_index = term_cmd_buf_size;
+    rs->cmd_buf_index = rs->cmd_buf_size;
 }
 
-static void term_up_char(void)
+static void readline_up_char(ReadLineState *rs)
 {
     int idx;
 
-    if (term_hist_entry == 0)
+    if (rs->hist_entry == 0)
 	return;
-    if (term_hist_entry == -1) {
+    if (rs->hist_entry == -1) {
 	/* Find latest entry */
-	for (idx = 0; idx < TERM_MAX_CMDS; idx++) {
-	    if (term_history[idx] == NULL)
+	for (idx = 0; idx < READLINE_MAX_CMDS; idx++) {
+	    if (rs->history[idx] == NULL)
 		break;
 	}
-	term_hist_entry = idx;
+	rs->hist_entry = idx;
     }
-    term_hist_entry--;
-    if (term_hist_entry >= 0) {
-	pstrcpy(term_cmd_buf, sizeof(term_cmd_buf),
-                term_history[term_hist_entry]);
-	term_cmd_buf_index = term_cmd_buf_size = strlen(term_cmd_buf);
+    rs->hist_entry--;
+    if (rs->hist_entry >= 0) {
+	pstrcpy(rs->cmd_buf, sizeof(rs->cmd_buf),
+                rs->history[rs->hist_entry]);
+	rs->cmd_buf_index = rs->cmd_buf_size = strlen(rs->cmd_buf);
     }
 }
 
-static void term_down_char(void)
+static void readline_down_char(ReadLineState *rs)
 {
-    if (term_hist_entry == TERM_MAX_CMDS - 1 || term_hist_entry == -1)
+    if (rs->hist_entry == READLINE_MAX_CMDS - 1 || rs->hist_entry == -1)
 	return;
-    if (term_history[++term_hist_entry] != NULL) {
-	pstrcpy(term_cmd_buf, sizeof(term_cmd_buf),
-                term_history[term_hist_entry]);
+    if (rs->history[++rs->hist_entry] != NULL) {
+	pstrcpy(rs->cmd_buf, sizeof(rs->cmd_buf),
+                rs->history[rs->hist_entry]);
     } else {
-	term_hist_entry = -1;
+	rs->hist_entry = -1;
     }
-    term_cmd_buf_index = term_cmd_buf_size = strlen(term_cmd_buf);
+    rs->cmd_buf_index = rs->cmd_buf_size = strlen(rs->cmd_buf);
 }
 
-static void term_hist_add(const char *cmdline)
+static void readline_hist_add(ReadLineState *rs, const char *cmdline)
 {
     char *hist_entry, *new_entry;
     int idx;
@@ -250,93 +216,98 @@ static void term_hist_add(const char *cmdline)
     if (cmdline[0] == '\0')
 	return;
     new_entry = NULL;
-    if (term_hist_entry != -1) {
+    if (rs->hist_entry != -1) {
 	/* We were editing an existing history entry: replace it */
-	hist_entry = term_history[term_hist_entry];
-	idx = term_hist_entry;
+	hist_entry = rs->history[rs->hist_entry];
+	idx = rs->hist_entry;
 	if (strcmp(hist_entry, cmdline) == 0) {
 	    goto same_entry;
 	}
     }
     /* Search cmdline in history buffers */
-    for (idx = 0; idx < TERM_MAX_CMDS; idx++) {
-	hist_entry = term_history[idx];
+    for (idx = 0; idx < READLINE_MAX_CMDS; idx++) {
+	hist_entry = rs->history[idx];
 	if (hist_entry == NULL)
 	    break;
 	if (strcmp(hist_entry, cmdline) == 0) {
 	same_entry:
 	    new_entry = hist_entry;
 	    /* Put this entry at the end of history */
-	    memmove(&term_history[idx], &term_history[idx + 1],
-		    (TERM_MAX_CMDS - idx + 1) * sizeof(char *));
-	    term_history[TERM_MAX_CMDS - 1] = NULL;
-	    for (; idx < TERM_MAX_CMDS; idx++) {
-		if (term_history[idx] == NULL)
+	    memmove(&rs->history[idx], &rs->history[idx + 1],
+		    (READLINE_MAX_CMDS - idx + 1) * sizeof(char *));
+	    rs->history[READLINE_MAX_CMDS - 1] = NULL;
+	    for (; idx < READLINE_MAX_CMDS; idx++) {
+		if (rs->history[idx] == NULL)
 		    break;
 	    }
 	    break;
 	}
     }
-    if (idx == TERM_MAX_CMDS) {
+    if (idx == READLINE_MAX_CMDS) {
 	/* Need to get one free slot */
-	free(term_history[0]);
-	memcpy(term_history, &term_history[1],
-	       (TERM_MAX_CMDS - 1) * sizeof(char *));
-	term_history[TERM_MAX_CMDS - 1] = NULL;
-	idx = TERM_MAX_CMDS - 1;
+	free(rs->history[0]);
+	memcpy(rs->history, &rs->history[1],
+	       (READLINE_MAX_CMDS - 1) * sizeof(char *));
+	rs->history[READLINE_MAX_CMDS - 1] = NULL;
+	idx = READLINE_MAX_CMDS - 1;
     }
     if (new_entry == NULL)
 	new_entry = strdup(cmdline);
-    term_history[idx] = new_entry;
-    term_hist_entry = -1;
+    rs->history[idx] = new_entry;
+    rs->hist_entry = -1;
 }
 
 /* completion support */
 
-void add_completion(const char *str)
+void readline_add_completion(ReadLineState *rs, const char *str)
 {
-    if (nb_completions < NB_COMPLETIONS_MAX) {
-        completions[nb_completions++] = qemu_strdup(str);
+    if (rs->nb_completions < READLINE_MAX_COMPLETIONS) {
+        rs->completions[rs->nb_completions++] = qemu_strdup(str);
     }
 }
 
-static void term_completion(void)
+void readline_set_completion_index(ReadLineState *rs, int completion_index)
+{
+    rs->completion_index = completion_index;
+}
+
+static void readline_completion(ReadLineState *rs)
 {
     int len, i, j, max_width, nb_cols, max_prefix;
     char *cmdline;
 
-    nb_completions = 0;
+    rs->nb_completions = 0;
 
-    cmdline = qemu_malloc(term_cmd_buf_index + 1);
-    memcpy(cmdline, term_cmd_buf, term_cmd_buf_index);
-    cmdline[term_cmd_buf_index] = '\0';
-    readline_find_completion(cmdline);
+    cmdline = qemu_malloc(rs->cmd_buf_index + 1);
+    memcpy(cmdline, rs->cmd_buf, rs->cmd_buf_index);
+    cmdline[rs->cmd_buf_index] = '\0';
+    rs->completion_finder(cmdline);
     qemu_free(cmdline);
 
     /* no completion found */
-    if (nb_completions <= 0)
+    if (rs->nb_completions <= 0)
         return;
-    if (nb_completions == 1) {
-        len = strlen(completions[0]);
-        for(i = completion_index; i < len; i++) {
-            term_insert_char(completions[0][i]);
+    if (rs->nb_completions == 1) {
+        len = strlen(rs->completions[0]);
+        for(i = rs->completion_index; i < len; i++) {
+            readline_insert_char(rs, rs->completions[0][i]);
         }
         /* extra space for next argument. XXX: make it more generic */
-        if (len > 0 && completions[0][len - 1] != '/')
-            term_insert_char(' ');
+        if (len > 0 && rs->completions[0][len - 1] != '/')
+            readline_insert_char(rs, ' ');
     } else {
         monitor_printf("\n");
         max_width = 0;
         max_prefix = 0;	
-        for(i = 0; i < nb_completions; i++) {
-            len = strlen(completions[i]);
+        for(i = 0; i < rs->nb_completions; i++) {
+            len = strlen(rs->completions[i]);
             if (i==0) {
                 max_prefix = len;
             } else {
                 if (len < max_prefix)
                     max_prefix = len;
                 for(j=0; j<max_prefix; j++) {
-                    if (completions[i][j] != completions[0][j])
+                    if (rs->completions[i][j] != rs->completions[0][j])
                         max_prefix = j;
                 }
             }
@@ -344,8 +315,8 @@ static void term_completion(void)
                 max_width = len;
         }
         if (max_prefix > 0) 
-            for(i = completion_index; i < max_prefix; i++) {
-                term_insert_char(completions[0][i]);
+            for(i = rs->completion_index; i < max_prefix; i++) {
+                readline_insert_char(rs, rs->completions[0][i]);
             }
         max_width += 2;
         if (max_width < 10)
@@ -354,132 +325,151 @@ static void term_completion(void)
             max_width = 80;
         nb_cols = 80 / max_width;
         j = 0;
-        for(i = 0; i < nb_completions; i++) {
-            monitor_printf("%-*s", max_width, completions[i]);
-            if (++j == nb_cols || i == (nb_completions - 1)) {
+        for(i = 0; i < rs->nb_completions; i++) {
+            monitor_printf("%-*s", max_width, rs->completions[i]);
+            if (++j == nb_cols || i == (rs->nb_completions - 1)) {
                 monitor_printf("\n");
                 j = 0;
             }
         }
-        term_show_prompt2();
+        readline_show_prompt(rs);
     }
 }
 
 /* return true if command handled */
-void readline_handle_byte(int ch)
+void readline_handle_byte(ReadLineState *rs, int ch)
 {
-    switch(term_esc_state) {
+    switch(rs->esc_state) {
     case IS_NORM:
         switch(ch) {
         case 1:
-            term_bol();
+            readline_bol(rs);
             break;
         case 4:
-            term_delete_char();
+            readline_delete_char(rs);
             break;
         case 5:
-            term_eol();
+            readline_eol(rs);
             break;
         case 9:
-            term_completion();
+            readline_completion(rs);
             break;
         case 10:
         case 13:
-            term_cmd_buf[term_cmd_buf_size] = '\0';
-            if (!term_is_password)
-                term_hist_add(term_cmd_buf);
+            rs->cmd_buf[rs->cmd_buf_size] = '\0';
+            if (!rs->read_password)
+                readline_hist_add(rs, rs->cmd_buf);
             monitor_printf("\n");
-            term_cmd_buf_index = 0;
-            term_cmd_buf_size = 0;
-            term_last_cmd_buf_index = 0;
-            term_last_cmd_buf_size = 0;
-            /* NOTE: readline_start can be called here */
-            term_readline_func(term_readline_opaque, term_cmd_buf);
+            rs->cmd_buf_index = 0;
+            rs->cmd_buf_size = 0;
+            rs->last_cmd_buf_index = 0;
+            rs->last_cmd_buf_size = 0;
+            rs->readline_func(rs->readline_opaque, rs->cmd_buf);
             break;
         case 23:
             /* ^W */
-            term_backword();
+            readline_backword(rs);
             break;
         case 27:
-            term_esc_state = IS_ESC;
+            rs->esc_state = IS_ESC;
             break;
         case 127:
         case 8:
-            term_backspace();
+            readline_backspace(rs);
             break;
 	case 155:
-            term_esc_state = IS_CSI;
+            rs->esc_state = IS_CSI;
 	    break;
         default:
             if (ch >= 32) {
-                term_insert_char(ch);
+                readline_insert_char(rs, ch);
             }
             break;
         }
         break;
     case IS_ESC:
         if (ch == '[') {
-            term_esc_state = IS_CSI;
-            term_esc_param = 0;
+            rs->esc_state = IS_CSI;
+            rs->esc_param = 0;
         } else {
-            term_esc_state = IS_NORM;
+            rs->esc_state = IS_NORM;
         }
         break;
     case IS_CSI:
         switch(ch) {
 	case 'A':
 	case 'F':
-	    term_up_char();
+	    readline_up_char(rs);
 	    break;
 	case 'B':
 	case 'E':
-	    term_down_char();
+	    readline_down_char(rs);
 	    break;
         case 'D':
-            term_backward_char();
+            readline_backward_char(rs);
             break;
         case 'C':
-            term_forward_char();
+            readline_forward_char(rs);
             break;
         case '0' ... '9':
-            term_esc_param = term_esc_param * 10 + (ch - '0');
+            rs->esc_param = rs->esc_param * 10 + (ch - '0');
             goto the_end;
         case '~':
-            switch(term_esc_param) {
+            switch(rs->esc_param) {
             case 1:
-                term_bol();
+                readline_bol(rs);
                 break;
             case 3:
-                term_delete_char();
+                readline_delete_char(rs);
                 break;
             case 4:
-                term_eol();
+                readline_eol(rs);
                 break;
             }
             break;
         default:
             break;
         }
-        term_esc_state = IS_NORM;
+        rs->esc_state = IS_NORM;
     the_end:
         break;
     }
-    term_update();
+    readline_update(rs);
 }
 
-void readline_start(const char *prompt, int is_password,
+void readline_start(ReadLineState *rs, const char *prompt, int read_password,
                     ReadLineFunc *readline_func, void *opaque)
 {
-    pstrcpy(term_prompt, sizeof(term_prompt), prompt);
-    term_readline_func = readline_func;
-    term_readline_opaque = opaque;
-    term_is_password = is_password;
-    term_show_prompt();
+    pstrcpy(rs->prompt, sizeof(rs->prompt), prompt);
+    rs->readline_func = readline_func;
+    rs->readline_opaque = opaque;
+    rs->read_password = read_password;
+    readline_restart(rs);
+    readline_show_prompt(rs);
+}
+
+void readline_restart(ReadLineState *rs)
+{
+    rs->cmd_buf_index = 0;
+    rs->cmd_buf_size = 0;
 }
 
-const char *readline_get_history(unsigned int index)
+const char *readline_get_history(ReadLineState *rs, unsigned int index)
 {
-    if (index >= TERM_MAX_CMDS)
+    if (index >= READLINE_MAX_CMDS)
         return NULL;
-    return term_history[index];
+    return rs->history[index];
+}
+
+ReadLineState *readline_init(ReadLineCompletionFunc *completion_finder)
+{
+    ReadLineState *rs = qemu_mallocz(sizeof(*rs));
+
+    if (!rs)
+        return NULL;
+
+    rs->hist_entry = -1;
+    rs->completion_finder = completion_finder;
+
+    return rs;
 }
diff --git a/readline.h b/readline.h
index 9538550..e385ac4 100644
--- a/readline.h
+++ b/readline.h
@@ -1,14 +1,46 @@
 #ifndef READLINE_H
 #define READLINE_H
 
+#define READLINE_CMD_BUF_SIZE 4095
+#define READLINE_MAX_CMDS 64
+#define READLINE_MAX_COMPLETIONS 256
+
 typedef void ReadLineFunc(void *opaque, const char *str);
+typedef void ReadLineCompletionFunc(const char *cmdline);
+
+typedef struct ReadLineState {
+    char cmd_buf[READLINE_CMD_BUF_SIZE + 1];
+    int cmd_buf_index;
+    int cmd_buf_size;
+
+    char last_cmd_buf[READLINE_CMD_BUF_SIZE + 1];
+    int last_cmd_buf_index;
+    int last_cmd_buf_size;
+
+    int esc_state;
+    int esc_param;
+
+    char *history[READLINE_MAX_CMDS];
+    int hist_entry;
+
+    ReadLineCompletionFunc *completion_finder;
+    char *completions[READLINE_MAX_COMPLETIONS];
+    int nb_completions;
+    int completion_index;
+
+    ReadLineFunc *readline_func;
+    void *readline_opaque;
+    int read_password;
+    char prompt[256];
+} ReadLineState;
 
-extern int completion_index;
-void add_completion(const char *str);
-void readline_handle_byte(int ch);
-void readline_find_completion(const char *cmdline);
-const char *readline_get_history(unsigned int index);
-void readline_start(const char *prompt, int is_password,
+void readline_add_completion(ReadLineState *rs, const char *str);
+void readline_set_completion_index(ReadLineState *rs, int completion_index);
+void readline_handle_byte(ReadLineState *rs, int ch);
+const char *readline_get_history(ReadLineState *rs, unsigned int index);
+void readline_start(ReadLineState *rs, const char *prompt, int read_password,
                     ReadLineFunc *readline_func, void *opaque);
+void readline_restart(ReadLineState *rs);
+ReadLineState *readline_init(ReadLineCompletionFunc *completion_finder);
 
 #endif /* !READLINE_H */

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

* [Qemu-devel] [PATCH 12/17] monitor: Rework modal password input
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (14 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 15/17] monitor: Improve mux'ed console experience Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 11/17] monitor: Drop banner hiding Jan Kiszka
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

Currently, waiting for the user to type in some password blocks the
whole VM because monitor_readline starts its own I/O loop. And this loop
also screws up reading passwords from virtual console.

Patch below fixes the shortcomings by using normal I/O processing also
for waiting on a password. To keep to modal property for the monitor
terminal, the command handler is temporarily replaced by a password
handler and a callback infrastructure is established to process the
result before switching back to command mode.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 block.c      |   21 ++++++++---
 hw/usb-msd.c |   10 ++++-
 hw/usb.h     |    3 +-
 monitor.c    |  111 ++++++++++++++++++++++++++++++++--------------------------
 monitor.h    |    4 ++
 vl.c         |   19 ++++++++--
 6 files changed, 105 insertions(+), 63 deletions(-)

diff --git a/block.c b/block.c
index 3481988..5a455e3 100644
--- a/block.c
+++ b/block.c
@@ -429,11 +429,12 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
         }
     }
 
-    /* call the change callback */
-    bs->media_changed = 1;
-    if (bs->change_cb)
-        bs->change_cb(bs->change_opaque);
-
+    if (!bdrv_key_required(bs)) {
+        /* call the change callback */
+        bs->media_changed = 1;
+        if (bs->change_cb)
+            bs->change_cb(bs->change_opaque);
+    }
     return 0;
 }
 
@@ -944,7 +945,15 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
     if (!bs->encrypted || !bs->drv || !bs->drv->bdrv_set_key)
         return -1;
     ret = bs->drv->bdrv_set_key(bs, key);
-    bs->valid_key = (ret == 0);
+    if (ret < 0) {
+        bs->valid_key = 0;
+    } else if (!bs->valid_key) {
+        bs->valid_key = 1;
+        /* call the change callback now, we skipped it on open */
+        bs->media_changed = 1;
+        if (bs->change_cb)
+            bs->change_cb(bs->change_opaque);
+    }
     return ret;
 }
 
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 2b0338d..1594bb0 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -514,7 +514,7 @@ static void usb_msd_handle_destroy(USBDevice *dev)
     qemu_free(s);
 }
 
-USBDevice *usb_msd_init(const char *filename, BlockDriverState **pbs)
+USBDevice *usb_msd_init(const char *filename)
 {
     MSDState *s;
     BlockDriverState *bdrv;
@@ -554,7 +554,6 @@ USBDevice *usb_msd_init(const char *filename, BlockDriverState **pbs)
     if (bdrv_open2(bdrv, filename, 0, drv) < 0)
         goto fail;
     s->bs = bdrv;
-    *pbs = bdrv;
 
     s->dev.speed = USB_SPEED_FULL;
     s->dev.handle_packet = usb_generic_handle_packet;
@@ -574,3 +573,10 @@ USBDevice *usb_msd_init(const char *filename, BlockDriverState **pbs)
     qemu_free(s);
     return NULL;
 }
+
+BlockDriverState *usb_msd_get_bdrv(USBDevice *dev)
+{
+    MSDState *s = (MSDState *)dev;
+
+    return s->bs;
+}
diff --git a/hw/usb.h b/hw/usb.h
index 4cd832d..a5e0d44 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -253,7 +253,8 @@ USBDevice *usb_keyboard_init(void);
 void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *));
 
 /* usb-msd.c */
-USBDevice *usb_msd_init(const char *filename, BlockDriverState **pbs);
+USBDevice *usb_msd_init(const char *filename);
+BlockDriverState *usb_msd_get_bdrv(USBDevice *dev);
 
 /* usb-net.c */
 USBDevice *usb_net_init(NICInfo *nd);
diff --git a/monitor.c b/monitor.c
index e187412..51e6d9f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -79,13 +79,18 @@ static const term_cmd_t info_cmds[];
 
 static uint8_t term_outbuf[1024];
 static int term_outbuf_index;
+static BlockDriverCompletionFunc *password_completion_cb;
+static void *password_opaque;
 
 static void monitor_start_input(void);
-static void monitor_readline(const char *prompt, int is_password,
-                             char *buf, int buf_size);
 
 static CPUState *mon_cpu = NULL;
 
+static void monitor_read_password(ReadLineFunc *readline_func, void *opaque)
+{
+    readline_start("Password: ", 1, readline_func, opaque);
+}
+
 void monitor_flush(void)
 {
     MonitorTerm *term;
@@ -445,21 +450,29 @@ static void do_change_block(const char *device, const char *filename, const char
     if (eject_device(bs, 0) < 0)
         return;
     bdrv_open2(bs, filename, 0, drv);
-    monitor_read_bdrv_key(bs);
+    monitor_read_bdrv_key_start(bs, NULL, NULL);
+}
+
+static void change_vnc_password_cb(void *opaque, const char *password)
+{
+    if (vnc_display_password(NULL, password) < 0)
+        monitor_printf("could not set VNC server password\n");
+
+    monitor_start_input();
 }
 
 static void do_change_vnc(const char *target, const char *arg)
 {
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
-	char password[9];
 	if (arg) {
+            char password[9];
 	    strncpy(password, arg, sizeof(password));
 	    password[sizeof(password) - 1] = '\0';
-	} else
-	    monitor_readline("Password: ", 1, password, sizeof(password));
-	if (vnc_display_password(NULL, password) < 0)
-	    monitor_printf("could not set VNC server password\n");
+            change_vnc_password_cb(NULL, password);
+        } else {
+            monitor_read_password(change_vnc_password_cb, NULL);
+        }
     } else {
 	if (vnc_display_open(NULL, target) < 0)
 	    monitor_printf("could not start VNC server on %s\n", target);
@@ -506,15 +519,7 @@ static void do_stop(void)
     vm_stop(EXCP_INTERRUPT);
 }
 
-static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
-{
-    int *err = opaque;
-
-    if (bdrv_key_required(bs))
-        *err = monitor_read_bdrv_key(bs);
-    else
-        *err = 0;
-}
+static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs);
 
 static void do_cont(void)
 {
@@ -526,6 +531,23 @@ static void do_cont(void)
         vm_start();
 }
 
+static void bdrv_key_cb(void *opaque, int err)
+{
+    /* another key was set successfully, retry to continue */
+    if (!err)
+        do_cont();
+}
+
+static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
+{
+    int *err = opaque;
+
+    if (!*err && bdrv_key_required(bs)) {
+        *err = -EBUSY;
+        monitor_read_bdrv_key_start(bs, bdrv_key_cb, NULL);
+    }
+}
+
 #ifdef CONFIG_GDBSTUB
 static void do_gdbserver(const char *port)
 {
@@ -2858,45 +2880,36 @@ void monitor_init(CharDriverState *chr)
     readline_start("", 0, monitor_handle_command1, NULL);
 }
 
-/* XXX: use threads ? */
-/* modal monitor readline */
-static int monitor_readline_started;
-static char *monitor_readline_buf;
-static int monitor_readline_buf_size;
-
-static void monitor_readline_cb(void *opaque, const char *input)
+static void bdrv_password_cb(void *opaque, const char *password)
 {
-    pstrcpy(monitor_readline_buf, monitor_readline_buf_size, input);
-    monitor_readline_started = 0;
-}
+    BlockDriverState *bs = opaque;
+    int ret = 0;
 
-static void monitor_readline(const char *prompt, int is_password,
-                             char *buf, int buf_size)
-{
-    readline_start(prompt, is_password, monitor_readline_cb, NULL);
-    monitor_readline_buf = buf;
-    monitor_readline_buf_size = buf_size;
-    monitor_readline_started = 1;
-    while (monitor_readline_started) {
-        main_loop_wait(10);
+    if (bdrv_set_key(bs, password) != 0) {
+        monitor_printf("invalid password\n");
+        ret = -EPERM;
     }
+    if (password_completion_cb)
+        password_completion_cb(password_opaque, ret);
+
+    monitor_start_input();
 }
 
-int monitor_read_bdrv_key(BlockDriverState *bs)
+void monitor_read_bdrv_key_start(BlockDriverState *bs,
+                                 BlockDriverCompletionFunc *completion_cb,
+                                 void *opaque)
 {
-    char password[256];
-    int i;
-
-    if (!bdrv_is_encrypted(bs))
-        return 0;
+    if (!bdrv_key_required(bs)) {
+        if (completion_cb)
+            completion_cb(opaque, 0);
+        return;
+    }
 
     monitor_printf("%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
                    bdrv_get_encrypted_filename(bs));
-    for(i = 0; i < 3; i++) {
-        monitor_readline("Password: ", 1, password, sizeof(password));
-        if (bdrv_set_key(bs, password) == 0)
-            return 0;
-        monitor_printf("invalid password\n");
-    }
-    return -EPERM;
+
+    password_completion_cb = completion_cb;
+    password_opaque = opaque;
+
+    monitor_read_password(bdrv_password_cb, bs);
 }
diff --git a/monitor.h b/monitor.h
index ae79a68..4b35a25 100644
--- a/monitor.h
+++ b/monitor.h
@@ -8,7 +8,9 @@ void monitor_init(CharDriverState *chr);
 void monitor_suspend(void);
 void monitor_resume(void);
 
-int monitor_read_bdrv_key(BlockDriverState *bs);
+void monitor_read_bdrv_key_start(BlockDriverState *bs,
+                                 BlockDriverCompletionFunc *completion_cb,
+                                 void *opaque);
 
 void monitor_vprintf(const char *fmt, va_list ap);
 void monitor_printf(const char *fmt, ...)
diff --git a/vl.c b/vl.c
index 4b25080..f5b6171 100644
--- a/vl.c
+++ b/vl.c
@@ -2603,6 +2603,16 @@ int usb_device_add_dev(USBDevice *dev)
     return 0;
 }
 
+static void usb_msd_password_cb(void *opaque, int err)
+{
+    USBDevice *dev = opaque;
+
+    if (!err)
+        usb_device_add_dev(dev);
+    else
+        dev->handle_destroy(dev);
+}
+
 static int usb_device_add(const char *devname, int is_hotplug)
 {
     const char *p;
@@ -2622,14 +2632,15 @@ static int usb_device_add(const char *devname, int is_hotplug)
     } else if (strstart(devname, "disk:", &p)) {
         BlockDriverState *bs;
 
-        dev = usb_msd_init(p, &bs);
+        dev = usb_msd_init(p);
         if (!dev)
             return -1;
+        bs = usb_msd_get_bdrv(dev);
         if (bdrv_key_required(bs)) {
             autostart = 0;
-            if (is_hotplug && monitor_read_bdrv_key(bs) < 0) {
-                dev->handle_destroy(dev);
-                return -1;
+            if (is_hotplug) {
+                monitor_read_bdrv_key_start(bs, usb_msd_password_cb, dev);
+                return 0;
             }
         }
     } else if (!strcmp(devname, "wacom-tablet")) {

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

* [Qemu-devel] [PATCH 11/17] monitor: Drop banner hiding
  2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
                   ` (15 preceding siblings ...)
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 12/17] monitor: Rework modal password input Jan Kiszka
@ 2009-02-07 18:16 ` Jan Kiszka
  16 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-07 18:16 UTC (permalink / raw)
  To: qemu-devel

There is no use for the hide/show banner option, and it is applied
inconsistently anyway (or what makes the difference between
-serial mon:stdio and -nographic for the monitor?). So drop this mode.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 monitor.c   |   10 +++-------
 monitor.h   |    2 +-
 qemu-char.c |    2 +-
 vl.c        |    2 +-
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/monitor.c b/monitor.c
index c21ea8a..e187412 100644
--- a/monitor.c
+++ b/monitor.c
@@ -73,7 +73,6 @@ typedef struct MonitorTerm {
 } MonitorTerm;
 
 static LIST_HEAD(term_list, MonitorTerm) term_list;
-static int hide_banner;
 
 static const term_cmd_t term_cmds[];
 static const term_cmd_t info_cmds[];
@@ -2832,15 +2831,14 @@ static void term_event(void *opaque, int event)
     if (event != CHR_EVENT_RESET)
 	return;
 
-    if (!hide_banner)
-        monitor_printf("QEMU %s monitor - type 'help' for more information\n",
-                       QEMU_VERSION);
+    monitor_printf("QEMU %s monitor - type 'help' for more information\n",
+                   QEMU_VERSION);
     monitor_start_input();
 }
 
 static int is_first_init = 1;
 
-void monitor_init(CharDriverState *chr, int show_banner)
+void monitor_init(CharDriverState *chr)
 {
     MonitorTerm *term;
 
@@ -2851,8 +2849,6 @@ void monitor_init(CharDriverState *chr, int show_banner)
 
     term = qemu_mallocz(sizeof(*term));
 
-    hide_banner = !show_banner;
-
     term->chr = chr;
 
     qemu_chr_add_handlers(chr, term_can_read, term_read, term_event, NULL);
diff --git a/monitor.h b/monitor.h
index 226f76c..ae79a68 100644
--- a/monitor.h
+++ b/monitor.h
@@ -4,7 +4,7 @@
 #include "qemu-char.h"
 #include "block.h"
 
-void monitor_init(CharDriverState *chr, int show_banner);
+void monitor_init(CharDriverState *chr);
 void monitor_suspend(void);
 void monitor_resume(void);
 
diff --git a/qemu-char.c b/qemu-char.c
index 15cb931..5c2ee4e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2095,7 +2095,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, !nographic);
+            monitor_init(chr);
         } else {
             printf("Unable to open driver: %s\n", p);
         }
diff --git a/vl.c b/vl.c
index f8f049a..4b25080 100644
--- a/vl.c
+++ b/vl.c
@@ -5583,7 +5583,7 @@ int main(int argc, char **argv, char **envp)
     text_consoles_set_display(display_state);
 
     if (monitor_device && monitor_hd)
-        monitor_init(monitor_hd, !nographic);
+        monitor_init(monitor_hd);
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         const char *devname = serial_devices[i];

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

* Re: [Qemu-devel] [PATCH 02/17] block: Improve bdrv_iterate
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 02/17] block: Improve bdrv_iterate Jan Kiszka
@ 2009-02-09 14:45   ` Anthony Liguori
  2009-02-09 15:34     ` [Qemu-devel] " Jan Kiszka
  0 siblings, 1 reply; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 14:45 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> Make bdrv_iterate more useful by passing the BlockDriverState to the
> iterator instead of the device name.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>   

> ---
>
>  block.c   |    4 ++--
>  block.h   |    3 ++-
>  monitor.c |    3 ++-
>   

$ grep -l bdrv_iterate *.c hw/*.c
block.c
monitor.c
qemu-img.c
vl.c

So this patch will break the build.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH 04/17] monitor: Report encrypted disks in snapshot mode
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 04/17] monitor: Report encrypted disks in snapshot mode Jan Kiszka
@ 2009-02-09 14:47   ` Anthony Liguori
  2009-02-09 15:35     ` [Qemu-devel] " Jan Kiszka
  0 siblings, 1 reply; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 14:47 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> If the backing file is encrypted, 'info block' currently does not report
> the disk as encrypted. Fix this by used the standard API to check disk
> encryption mode.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
>  block.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/block.c b/block.c
> index 78982f4..82c2016 100644
> --- a/block.c
> +++ b/block.c
> @@ -1056,7 +1056,7 @@ void bdrv_info(void)
>  	    }
>              term_printf(" ro=%d", bs->read_only);
>              term_printf(" drv=%s", bs->drv->format_name);
> -            if (bs->encrypted)
> +            if (bdrv_is_encrypted(bs))
>                  term_printf(" encrypted");
>   

It's a bit more easily parsable if it's in the format encrypted=[1|0].

Regards,

Anthony Liguori

>          } else {
>              term_printf(" [not inserted]");
>
>
>
>
>   

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

* Re: [Qemu-devel] [PATCH 05/17] monitor: Don't change VNC server when disabled
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 05/17] monitor: Don't change VNC server when disabled Jan Kiszka
@ 2009-02-09 14:48   ` Anthony Liguori
  2009-02-09 15:35     ` [Qemu-devel] " Jan Kiszka
  0 siblings, 1 reply; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 14:48 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> Avoid a segfault when the user issues 'change vnc' without having vnc
> enabled on startup.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>   

It'd be easier on me if you split out these sort of fixes from the rest 
of the series (that clearly has nothing to do with improving the monitor).

Regards,

Anthony Liguori

> ---
>
>  vnc.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/vnc.c b/vnc.c
> index 68df599..bdfc79b 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -2333,6 +2333,8 @@ void vnc_display_close(DisplayState *ds)
>  {
>      VncState *vs = ds ? (VncState *)ds->opaque : vnc_state;
>  
> +    if (!vs)
> +        return;
>      if (vs->display) {
>  	qemu_free(vs->display);
>  	vs->display = NULL;
> @@ -2392,6 +2394,8 @@ int vnc_display_open(DisplayState *ds, const char *display)
>      int tls = 0, x509 = 0;
>  #endif
>  
> +    if (!vnc_state)
> +        return -1;
>      vnc_display_close(ds);
>      if (strcmp(display, "none") == 0)
>  	return 0;
>
>
>
>
>   

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

* Re: [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals
  2009-02-07 18:16 ` [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals Jan Kiszka
@ 2009-02-09 15:16   ` Anthony Liguori
  2009-02-09 15:40     ` Anthony Liguori
  2009-02-09 15:45     ` [Qemu-devel] " Jan Kiszka
  0 siblings, 2 replies; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 15:16 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> 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 an
> additional terminal can be a multiplexed device channel or a gdb
> frontend connected to QEMU's stub.
>
> Therefor, this patch decouples the buffers and states of all monitor
> terminals, allowing the user to operate them independently. The basic
> idea is stolen from Jason Wessel: When starting to handle a monitor
> command or some terminal event, the current monitor terminal is set to
> the one associated with the underlying char device, letting all
> succeeding monitor_printf show up on only this selected terminal.
>
> There are still two asynchronous monitor writers: some error reporting
> in VNC's audio_add and the log-to-monitor feature of the audio
> subsystem.

That concerns me.  Nothing should output to the monitor asychronously.

I'd like to see a few changes to make things a bit closer to the long 
term goals for the monitor (having proper multiple monitors devoid of 
global state).

Here's what I'd suggest:

1) Make monitor_printf() take a monitor state.  The easiest thing to do 
would be to introduce this in your previous rename patch making 
everything use current_monitor.
2) Introduce current_monitor and default_monitor global variables.  They 
map to what you describe above and should be maintained as such.
3) Make all monitor callbacks take a monitor state
4) Convert monitor_printf()s called from monitor callbacks to use the 
passed monitor state
5) Eliminate all uses of current_monitor/default_monitor.

I'd say, 1 and 2 are required for this patchset.  I think 3 and 4 would 
be pretty easy to add to your patchset.  I think 5 is probably tougher 
and could wait for another day.

Regards,

Anthony Liguori

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

* [Qemu-devel] Re: [PATCH 02/17] block: Improve bdrv_iterate
  2009-02-09 14:45   ` Anthony Liguori
@ 2009-02-09 15:34     ` Jan Kiszka
  2009-02-09 15:41       ` Anthony Liguori
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-09 15:34 UTC (permalink / raw)
  To: qemu-devel

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> Make bdrv_iterate more useful by passing the BlockDriverState to the
>> iterator instead of the device name.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>   
> 
>> ---
>>
>>  block.c   |    4 ++--
>>  block.h   |    3 ++-
>>  monitor.c |    3 ++-
>>   
> 
> $ grep -l bdrv_iterate *.c hw/*.c
> block.c
> monitor.c
> qemu-img.c
> vl.c
> 
> So this patch will break the build.

$ grep -l "bdrv_iterate(" *.c hw/*.c
block.c
monitor.c

Unless I'm totally blind, all users have been converted (there's only
one). And it still builds for all targets and tools.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

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

* [Qemu-devel] Re: [PATCH 05/17] monitor: Don't change VNC server when disabled
  2009-02-09 14:48   ` Anthony Liguori
@ 2009-02-09 15:35     ` Jan Kiszka
  2009-02-09 16:01       ` Anthony Liguori
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-09 15:35 UTC (permalink / raw)
  To: qemu-devel

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> Avoid a segfault when the user issues 'change vnc' without having vnc
>> enabled on startup.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>   
> 
> It'd be easier on me if you split out these sort of fixes from the rest
> of the series (that clearly has nothing to do with improving the monitor).

You are free to apply this whenever you want, it has no dependency. I'll
drop it from the series when it's no longer needed.

[ Small fixes unfortunately tend to fall through the cracks, so I chose
this path. ]

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

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

* [Qemu-devel] Re: [PATCH 04/17] monitor: Report encrypted disks in snapshot mode
  2009-02-09 14:47   ` Anthony Liguori
@ 2009-02-09 15:35     ` Jan Kiszka
  0 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2009-02-09 15:35 UTC (permalink / raw)
  To: qemu-devel

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> If the backing file is encrypted, 'info block' currently does not report
>> the disk as encrypted. Fix this by used the standard API to check disk
>> encryption mode.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>>  block.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 78982f4..82c2016 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1056,7 +1056,7 @@ void bdrv_info(void)
>>          }
>>              term_printf(" ro=%d", bs->read_only);
>>              term_printf(" drv=%s", bs->drv->format_name);
>> -            if (bs->encrypted)
>> +            if (bdrv_is_encrypted(bs))
>>                  term_printf(" encrypted");
>>   
> 
> It's a bit more easily parsable if it's in the format encrypted=[1|0].
> 

OK, will change this.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

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

* Re: [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals
  2009-02-09 15:16   ` Anthony Liguori
@ 2009-02-09 15:40     ` Anthony Liguori
  2009-02-09 15:45     ` [Qemu-devel] " Jan Kiszka
  1 sibling, 0 replies; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 15:40 UTC (permalink / raw)
  To: qemu-devel

Anthony Liguori wrote:
> I'd like to see a few changes to make things a bit closer to the long 
> term goals for the monitor (having proper multiple monitors devoid of 
> global state).
>
> Here's what I'd suggest:
>
> 1) Make monitor_printf() take a monitor state.  The easiest thing to 
> do would be to introduce this in your previous rename patch making 
> everything use current_monitor.
> 2) Introduce current_monitor and default_monitor global variables.  
> They map to what you describe above and should be maintained as such.
> 3) Make all monitor callbacks take a monitor state
> 4) Convert monitor_printf()s called from monitor callbacks to use the 
> passed monitor state
> 5) Eliminate all uses of current_monitor/default_monitor.
6) Make monitor callbacks return an error value
7) Convert all monitor callbacks to return error values instead of 
printing error messages
8) Introduce greater structure to monitor_printf()s.  For instance, we 
have a number of monitor commands that essentially print dictionaries 
(set of key value pairs).  We should introduce a higher level function 
like monitor_print_list("hd", "locked=false", "file=foo.img", NULL).
9) Introduce computer mode to monitor that makes monitor very parsable 
to a non-human user.
10) Introduce library that interacts with monitor over any supported 
character device.

Just to fill out more of my long term thinking about the monitor.

Regards,

Anthony Liguori

> I'd say, 1 and 2 are required for this patchset.  I think 3 and 4 
> would be pretty easy to add to your patchset.  I think 5 is probably 
> tougher and could wait for another day.
>
> Regards,
>
> Anthony Liguori
>
>
>

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

* Re: [Qemu-devel] Re: [PATCH 02/17] block: Improve bdrv_iterate
  2009-02-09 15:34     ` [Qemu-devel] " Jan Kiszka
@ 2009-02-09 15:41       ` Anthony Liguori
  0 siblings, 0 replies; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 15:41 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> $ grep -l "bdrv_iterate(" *.c hw/*.c
> block.c
> monitor.c
>
> Unless I'm totally blind, all users have been converted (there's only
> one). And it still builds for all targets and tools.
>   

You're right.  Sorry for the noise.

Regards,

Anthony Liguori

> Jan
>
>   

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

* [Qemu-devel] Re: [PATCH 14/17] monitor: Decouple terminals
  2009-02-09 15:16   ` Anthony Liguori
  2009-02-09 15:40     ` Anthony Liguori
@ 2009-02-09 15:45     ` Jan Kiszka
  2009-02-09 15:55       ` Anthony Liguori
  1 sibling, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2009-02-09 15:45 UTC (permalink / raw)
  To: qemu-devel

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> 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 an
>> additional terminal can be a multiplexed device channel or a gdb
>> frontend connected to QEMU's stub.
>>
>> Therefor, this patch decouples the buffers and states of all monitor
>> terminals, allowing the user to operate them independently. The basic
>> idea is stolen from Jason Wessel: When starting to handle a monitor
>> command or some terminal event, the current monitor terminal is set to
>> the one associated with the underlying char device, letting all
>> succeeding monitor_printf show up on only this selected terminal.
>>
>> There are still two asynchronous monitor writers: some error reporting
>> in VNC's audio_add and the log-to-monitor feature of the audio
>> subsystem.
> 
> That concerns me.  Nothing should output to the monitor asychronously.

Indeed.

> 
> I'd like to see a few changes to make things a bit closer to the long
> term goals for the monitor (having proper multiple monitors devoid of
> global state).
> 
> Here's what I'd suggest:
> 
> 1) Make monitor_printf() take a monitor state.  The easiest thing to do
> would be to introduce this in your previous rename patch making
> everything use current_monitor.

Lazy /me was hoping to get around this...

> 2) Introduce current_monitor and default_monitor global variables.  They
> map to what you describe above and should be maintained as such.
> 3) Make all monitor callbacks take a monitor state
> 4) Convert monitor_printf()s called from monitor callbacks to use the
> passed monitor state
> 5) Eliminate all uses of current_monitor/default_monitor.
> 
> I'd say, 1 and 2 are required for this patchset.  I think 3 and 4 would
> be pretty easy to add to your patchset.  I think 5 is probably tougher
> and could wait for another day.

My feeling is (though I have not sound stats at hand) that a lot of
functions all over the place will have to be extended in order to pass
the target monitor around: From the command callbacks, through all the
subsystems, finally reaching the monitor API. Some use cases only
consist of the command callback itself, OK, but the others worry me a
bit. All doable, for sure, but I just want to make sure that we all
agree on the result before starting this "tough" endeavor. :)

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

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

* Re: [Qemu-devel] Re: [PATCH 14/17] monitor: Decouple terminals
  2009-02-09 15:45     ` [Qemu-devel] " Jan Kiszka
@ 2009-02-09 15:55       ` Anthony Liguori
  0 siblings, 0 replies; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 15:55 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> Anthony Liguori wrote:
>> 1) Make monitor_printf() take a monitor state.  The easiest thing to do
>> would be to introduce this in your previous rename patch making
>> everything use current_monitor.
>>     
>
> Lazy /me was hoping to get around this...
>   

You are already changing every instance of term_printf().  How hard can 
it be to add another parameter that gets completely ignored for now :-)

The reason I want to see this in this changeset is that you're touching 
every term_printf().  Adjusting it to the right interface means we don't 
have to churn again and we don't have the hurdle of touching every 
term_printf() to get started.

>> 2) Introduce current_monitor and default_monitor global variables.  They
>> map to what you describe above and should be maintained as such.
>> 3) Make all monitor callbacks take a monitor state
>> 4) Convert monitor_printf()s called from monitor callbacks to use the
>> passed monitor state
>> 5) Eliminate all uses of current_monitor/default_monitor.
>>
>> I'd say, 1 and 2 are required for this patchset.  I think 3 and 4 would
>> be pretty easy to add to your patchset.  I think 5 is probably tougher
>> and could wait for another day.
>>     
>
> My feeling is (though I have not sound stats at hand) that a lot of
> functions all over the place will have to be extended in order to pass
> the target monitor around: From the command callbacks, through all the
> subsystems, finally reaching the monitor API.

Yes, my feeling is the same.  That's why I suggest an incremental 
approach.  You already have introduced a concept of an active and 
default terminal.  You hide this behind the monitor_printf() function.  
I'm simply suggesting making it an explicit part of the interface.

>  Some use cases only
> consist of the command callback itself, OK,

Yes, these are the low hanging fruit for conversion.

>  but the others worry me a
> bit. All doable, for sure, but I just want to make sure that we all
> agree on the result before starting this "tough" endeavor. :)
>   

It's been discussed a lot and there's a strong desire to make the 
monitor interface better for management tools.  I don't expect you to 
address the whole problem in this one series but, as I said earlier, 
since you're already making changes everywhere, let's make the right 
changes at least :-)

Regards,

Anthony Liguori

> Jan
>
>   

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

* Re: [Qemu-devel] Re: [PATCH 05/17] monitor: Don't change VNC server when disabled
  2009-02-09 15:35     ` [Qemu-devel] " Jan Kiszka
@ 2009-02-09 16:01       ` Anthony Liguori
  0 siblings, 0 replies; 30+ messages in thread
From: Anthony Liguori @ 2009-02-09 16:01 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> Anthony Liguori wrote:
>   
>> Jan Kiszka wrote:
>>     
>>> Avoid a segfault when the user issues 'change vnc' without having vnc
>>> enabled on startup.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>   
>>>       
>> It'd be easier on me if you split out these sort of fixes from the rest
>> of the series (that clearly has nothing to do with improving the monitor).
>>     
>
> You are free to apply this whenever you want, it has no dependency. I'll
> drop it from the series when it's no longer needed.
>
> [ Small fixes unfortunately tend to fall through the cracks, so I chose
> this path. ]
>   

 From a purely pragmatic sense, my patch series scripts verify that all 
patches in the series are present as some folks have issues with mailers 
or SMTP delay.  That means for a patch like this, I'd have to apply it 
by hand if I wanted to apply it in the absence of the rest of the 
series.  That means I'll simply wait until the rest of the series is 
ready to go.

BTW, I've now got something locally tracking patches on the list so 
hopefully we're past the days of stuff slipping through the cracks.  I 
have some old patches in my queue still that I've been waiting for other 
maintainers to ack/commit but rest assured, they haven't been forgotten :-)

Regards,

Anthony Liguori

> Jan
>
>   

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

end of thread, other threads:[~2009-02-09 16:01 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-07 18:16 [Qemu-devel] [PATCH 00/17] monitor rework Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 01/17] block: Polish error handling of brdv_open2 Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 05/17] monitor: Don't change VNC server when disabled Jan Kiszka
2009-02-09 14:48   ` Anthony Liguori
2009-02-09 15:35     ` [Qemu-devel] " Jan Kiszka
2009-02-09 16:01       ` Anthony Liguori
2009-02-07 18:16 ` [Qemu-devel] [PATCH 02/17] block: Improve bdrv_iterate Jan Kiszka
2009-02-09 14:45   ` Anthony Liguori
2009-02-09 15:34     ` [Qemu-devel] " Jan Kiszka
2009-02-09 15:41       ` Anthony Liguori
2009-02-07 18:16 ` [Qemu-devel] [PATCH 06/17] char-mux: Use separate input buffers Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 03/17] block: Introduce bdrv_get_encrypted_filename Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 09/17] monitor: Simplify password input mode Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 07/17] monitor: Introduce monitor.h and readline.h Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 08/17] monitor: Rework initial disk password retrieval Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 10/17] monitor: Rework terminal management Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 04/17] monitor: Report encrypted disks in snapshot mode Jan Kiszka
2009-02-09 14:47   ` Anthony Liguori
2009-02-09 15:35     ` [Qemu-devel] " Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 17/17] monitor: Pass-through for gdbstub Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 16/17] monitor: Introduce MONITOR_USE_READLINE flag Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 14/17] monitor: Decouple terminals Jan Kiszka
2009-02-09 15:16   ` Anthony Liguori
2009-02-09 15:40     ` Anthony Liguori
2009-02-09 15:45     ` [Qemu-devel] " Jan Kiszka
2009-02-09 15:55       ` Anthony Liguori
2009-02-07 18:16 ` [Qemu-devel] [PATCH 13/17] monitor: Introduce ReadLineState Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 15/17] monitor: Improve mux'ed console experience Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 12/17] monitor: Rework modal password input Jan Kiszka
2009-02-07 18:16 ` [Qemu-devel] [PATCH 11/17] monitor: Drop banner hiding Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).