[Qemu-devel] [PATCH 0/2] Make the monitor a bit more program-friendly

[Qemu-devel] [PATCH 0/2] Make the monitor a bit more program-friendly

[Anthony Liguori]

This is a short patch series to make the monitor a bit more friendly to 
external programs.

This series is independent of my previous series but may apply with fuzz 
if the other series isn't applied.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 1/2] Escape filenames in monitor

[Anthony Liguori]

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

info block is impossible to parse reliably because there is no escaping 
done on the filename.  A really unfortunately name like "Ugly 
backing_file=foo" would result in:

hda: type=hd removable=0 file=Ugly backing_file=foo ro=0 drv=qcow

Which is ambiguous when compared to a file named "Ugly" with a backing 
file of "foo".  This patch will escape filenames so that this case will 
be printed as:

hda: type=hd removable=0 file=Ugly\ backing_file=foo ro=0 drv=qcow


Which is now parsable.

Regards,

Anthony Liguori

[-- Attachment #2: qemu-monitor-escape.diff --]
[-- Type: text/x-patch, Size: 2441 bytes --]

diff -r 7d5869c61e0d block.c
--- a/block.c	Sat Dec 16 11:50:51 2006 -0600
+++ b/block.c	Sat Dec 16 14:34:22 2006 -0600
@@ -868,9 +868,12 @@ void bdrv_info(void)
             term_printf(" locked=%d", bs->locked);
         }
         if (bs->drv) {
-            term_printf(" file=%s", bs->filename);
-            if (bs->backing_file[0] != '\0')
-                term_printf(" backing_file=%s", bs->backing_file);
+            term_printf(" file=");
+	    term_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);
             if (bs->encrypted)
diff -r 7d5869c61e0d monitor.c
--- a/monitor.c	Sat Dec 16 11:50:51 2006 -0600
+++ b/monitor.c	Sat Dec 16 14:34:22 2006 -0600
@@ -104,6 +104,32 @@ void term_printf(const char *fmt, ...)
     va_start(ap, fmt);
     term_vprintf(fmt, ap);
     va_end(ap);
+}
+
+void term_print_filename(const char *filename)
+{
+    int i;
+
+    for (i = 0; filename[i]; i++) {
+	switch (filename[i]) {
+	case ' ':
+	case '"':
+	    term_printf("\\%c", filename[i]);
+	    break;
+	case '\t':
+	    term_printf("\\t");
+	    break;
+	case '\r':
+	    term_printf("\\r");
+	    break;
+	case '\n':
+	    term_printf("\\n");
+	    break;
+	default:
+	    term_printf("%c", filename[i]);
+	    break;
+	}
+    }
 }
 
 static int monitor_fprintf(FILE *stream, const char *fmt, ...)
diff -r 7d5869c61e0d qemu-img.c
--- a/qemu-img.c	Sat Dec 16 11:50:51 2006 -0600
+++ b/qemu-img.c	Sat Dec 16 14:34:22 2006 -0600
@@ -111,6 +111,11 @@ void term_printf(const char *fmt, ...)
     va_start(ap, fmt);
     vprintf(fmt, ap);
     va_end(ap);
+}
+
+void term_print_filename(const char *filename)
+{
+    term_printf(filename);
 }
 
 void __attribute__((noreturn)) error(const char *fmt, ...) 
diff -r 7d5869c61e0d vl.h
--- a/vl.h	Sat Dec 16 11:50:51 2006 -0600
+++ b/vl.h	Sat Dec 16 14:34:22 2006 -0600
@@ -1318,6 +1318,7 @@ void term_puts(const char *str);
 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,

[Qemu-devel] [PATCH 2/2] Reinitialize monitor upon reconnect

[Anthony Liguori]

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

If you expose the monitor on a character device that support 
reconnecting, it becomes difficult to deterministically execute 
commands.  For instance, one person may connect and send a partial 
command in which case reconnecting may result in your command not being 
what you expect.

Furthermore, not knowing what to expect when you connect initially is 
annoying.  There may be a prompt, there may be nothing, or there may be 
a banner and prompt.

This patch adds a new event that's signaled whenever a character device 
supporting reconnecting reconnects.  For the monitor, this event causes 
the monitor to be reset.

This means no matter when you connect to the monitor (or reconnect), 
you'll always have deterministic behavior (the banner is printed with a 
new prompt).

Please note, if you don't use the nowait option, you'll see the banner 
twice the first time you connect.  I'm not sure I really see the value 
of not always doing nowait (particularly since there is -S).  Perhaps 
someone can shed light on what the use case is?

Regards,

Anthony Liguori

[-- Attachment #2: qemu-monitor-connect.diff --]
[-- Type: text/x-patch, Size: 2322 bytes --]

diff -r 3ce6d719bfc0 monitor.c
--- a/monitor.c	Sat Dec 16 14:34:25 2006 -0600
+++ b/monitor.c	Sat Dec 16 14:57:03 2006 -0600
@@ -2388,6 +2388,16 @@ static void term_read(void *opaque, cons
         readline_handle_byte(buf[i]);
 }
 
+static void term_event(void *opaque, int event)
+{
+    if (event != CHR_EVENT_RESET)
+	return;
+
+    term_printf("QEMU %s monitor - type 'help' for more information\n",
+		QEMU_VERSION);
+    readline_reset();
+}
+
 static void monitor_start_input(void);
 
 static void monitor_handle_command1(void *opaque, const char *cmdline)
@@ -2410,6 +2420,7 @@ void monitor_init(CharDriverState *hd, i
     }
     qemu_chr_add_read_handler(hd, term_can_read, term_read, NULL);
     monitor_start_input();
+    qemu_chr_add_event_handler(hd, term_event);
 }
 
 /* XXX: use threads ? */
diff -r 3ce6d719bfc0 readline.c
--- a/readline.c	Sat Dec 16 14:34:25 2006 -0600
+++ b/readline.c	Sat Dec 16 14:37:52 2006 -0600
@@ -415,6 +415,11 @@ void readline_start(const char *prompt, 
     term_show_prompt();
 }
 
+void readline_reset(void)
+{
+    term_show_prompt();
+}
+
 const char *readline_get_history(unsigned int index)
 {
     if (index >= TERM_MAX_CMDS)
diff -r 3ce6d719bfc0 vl.c
--- a/vl.c	Sat Dec 16 14:34:25 2006 -0600
+++ b/vl.c	Sat Dec 16 14:57:20 2006 -0600
@@ -2399,6 +2399,8 @@ static void tcp_chr_connect(void *opaque
     s->connected = 1;
     qemu_set_fd_handler2(s->fd, tcp_chr_read_poll,
                          tcp_chr_read, NULL, chr);
+    if (chr->chr_event)
+	    chr->chr_event(chr, CHR_EVENT_RESET);
 }
 
 #define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
diff -r 3ce6d719bfc0 vl.h
--- a/vl.h	Sat Dec 16 14:34:25 2006 -0600
+++ b/vl.h	Sat Dec 16 14:42:55 2006 -0600
@@ -240,7 +240,7 @@ void qemu_del_wait_object(HANDLE handle,
 
 #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_IOCTL_SERIAL_SET_PARAMS   1
@@ -1334,6 +1334,7 @@ const char *readline_get_history(unsigne
 const char *readline_get_history(unsigned int index);
 void readline_start(const char *prompt, int is_password,
                     ReadLineFunc *readline_func, void *opaque);
+void readline_reset(void);
 
 void kqemu_record_dump(void);
 

Re: [Qemu-devel] [PATCH 1/2] Escape filenames in monitor

[Julian Seward]

On Saturday 16 December 2006 21:11, Anthony Liguori wrote:
> info block is impossible to parse reliably because there is no escaping
> done on the filename.

Don't you also need to convert \ to \\ ?  Else any \ which was in
the original string will confuse the parser of the escaped output.

J

Re: [Qemu-devel] [PATCH 1/2] Escape filenames in monitor

[Anthony Liguori]

Julian Seward wrote:
> On Saturday 16 December 2006 21:11, Anthony Liguori wrote:
>   
>> info block is impossible to parse reliably because there is no escaping
>> done on the filename.
>>     
>
> Don't you also need to convert \ to \\ ?  Else any \ which was in
> the original string will confuse the parser of the escaped output.
>   

Yup, you are correct.  Thanks!

Regards,

Anthony Liguori

> J
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>