[Qemu-devel] [PATCH] Implement "info chardev" command.

[Qemu-devel] [PATCH] Implement "info chardev" command.

[Gerd Hoffmann]

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

  Hi,

This patch makes qemu keep track of the character devices in use and
implements a "info chardev" monitor command to print a list.

please apply,
  Gerd

[-- Attachment #2: 0041-Implement-info-chardev-command.patch --]
[-- Type: text/plain, Size: 10526 bytes --]

>From 3bd8020daf1de26cc76e28ae7d38ccf52f1f70bc Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 16 Oct 2008 16:03:49 +0200
Subject: [PATCH] Implement "info chardev" command.

This patch makes qemu keep track of the character devices in use and
implements a "info chardev" monitor command to print a list.

qemu_chr_open() sticks the devices into a linked list now.  It got a new
argument (label), so there is a name for each device.  It also assigns a
filename to each character device.  By default it just copyes the
filename passed in.  Individual drivers can fill in something else
though.  qemu_chr_open_pty() sets the filename to name of the pseudo tty
allocated.

Output looks like this:

  (qemu) info chardev
  monitor: filename=unix:/tmp/run.sh-26827/monitor,server,nowait
  serial0: filename=unix:/tmp/run.sh-26827/console,server
  serial1: filename=pty:/dev/pts/5
  parallel0: filename=vc:640x480

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 gdbstub.c       |    2 +-
 hw/usb-serial.c |    5 ++-
 monitor.c       |    2 +
 qemu-char.h     |    7 +++-
 vl.c            |   98 ++++++++++++++++++++++++++++++++++++------------------
 5 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index ed3f5c6..e95ca67 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1852,7 +1852,7 @@ int gdbserver_start(const char *port)
         port = gdbstub_port_name;
     }
 
-    chr = qemu_chr_open(port);
+    chr = qemu_chr_open("gdb", port);
     if (!chr)
         return -1;
 
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index 40d04cb..a6a756d 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -521,6 +521,8 @@ USBDevice *usb_serial_init(const char *filename)
     USBSerialState *s;
     CharDriverState *cdrv;
     unsigned short vendorid = 0x0403, productid = 0x6001;
+    char label[32];
+    static int index;
 
     while (*filename && *filename != ':') {
         const char *p;
@@ -555,7 +557,8 @@ USBDevice *usb_serial_init(const char *filename)
     if (!s)
         return NULL;
 
-    cdrv = qemu_chr_open(filename);
+    snprintf(label, sizeof(label), "usbserial%d", index++);
+    cdrv = qemu_chr_open(label, filename);
     if (!cdrv)
         goto fail;
     s->cs = cdrv;
diff --git a/monitor.c b/monitor.c
index 233c830..80facf1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1470,6 +1470,8 @@ static const term_cmd_t info_cmds[] = {
       "", "show the version of qemu" },
     { "network", "", do_info_network,
       "", "show the network state" },
+    { "chardev", "", qemu_chr_info,
+      "", "show the character devices" },
     { "block", "", do_info_block,
       "", "show the block devices" },
     { "blockstats", "", do_info_blockstats,
diff --git a/qemu-char.h b/qemu-char.h
index 05d6899..55d81cb 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -1,6 +1,7 @@
 #ifndef QEMU_CHAR_H
 #define QEMU_CHAR_H
 
+#include "sys-queue.h"
 /* character device */
 
 #define CHR_EVENT_BREAK 0 /* serial break char */
@@ -55,9 +56,12 @@ struct CharDriverState {
     void *opaque;
     int focus;
     QEMUBH *bh;
+    char *label;
+    char *filename;
+    TAILQ_ENTRY(CharDriverState) next;
 };
 
-CharDriverState *qemu_chr_open(const char *filename);
+CharDriverState *qemu_chr_open(const char *label, const char *filename);
 void qemu_chr_close(CharDriverState *chr);
 void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
@@ -72,6 +76,7 @@ void qemu_chr_reset(CharDriverState *s);
 int qemu_chr_can_read(CharDriverState *s);
 void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
 void qemu_chr_accept_input(CharDriverState *s);
+void qemu_chr_info(void);
 
 /* async I/O support */
 
diff --git a/vl.c b/vl.c
index e2d610c..065cb06 100644
--- a/vl.c
+++ b/vl.c
@@ -2582,7 +2582,7 @@ static CharDriverState *qemu_chr_open_pty(void)
     CharDriverState *chr;
     PtyCharDriver *s;
     struct termios tty;
-    int slave_fd;
+    int slave_fd, len;
 #if defined(__OpenBSD__)
     char pty_name[PATH_MAX];
 #define q_ptsname(x) pty_name
@@ -2609,6 +2609,9 @@ static CharDriverState *qemu_chr_open_pty(void)
     tcsetattr(slave_fd, TCSAFLUSH, &tty);
     close(slave_fd);
 
+    len = strlen(q_ptsname(s->fd)) + 5;
+    chr->filename = qemu_malloc(len);
+    snprintf(chr->filename, len, "pty:%s", q_ptsname(s->fd));
     fprintf(stderr, "char device redirected to %s\n", q_ptsname(s->fd));
 
     chr->opaque = s;
@@ -3768,90 +3771,115 @@ static CharDriverState *qemu_chr_open_tcp(const char *host_str,
     return NULL;
 }
 
-CharDriverState *qemu_chr_open(const char *filename)
+static TAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs
+= TAILQ_HEAD_INITIALIZER(chardevs);
+
+CharDriverState *qemu_chr_open(const char *label, const char *filename)
 {
     const char *p;
+    CharDriverState *chr;
 
     if (!strcmp(filename, "vc")) {
-        return text_console_init(&display_state, 0);
-    } else if (strstart(filename, "vc:", &p)) {
-        return text_console_init(&display_state, p);
-    } else if (!strcmp(filename, "null")) {
-        return qemu_chr_open_null();
+        chr = text_console_init(&display_state, 0);
+    } else
+    if (strstart(filename, "vc:", &p)) {
+        chr = text_console_init(&display_state, p);
+    } else
+    if (!strcmp(filename, "null")) {
+        chr = qemu_chr_open_null();
     } else
     if (strstart(filename, "tcp:", &p)) {
-        return qemu_chr_open_tcp(p, 0, 0);
+        chr = qemu_chr_open_tcp(p, 0, 0);
     } else
     if (strstart(filename, "telnet:", &p)) {
-        return qemu_chr_open_tcp(p, 1, 0);
+        chr = qemu_chr_open_tcp(p, 1, 0);
     } else
     if (strstart(filename, "udp:", &p)) {
-        return qemu_chr_open_udp(p);
+        chr = qemu_chr_open_udp(p);
     } else
     if (strstart(filename, "mon:", &p)) {
-        CharDriverState *drv = qemu_chr_open(p);
-        if (drv) {
-            drv = qemu_chr_open_mux(drv);
-            monitor_init(drv, !nographic);
-            return drv;
+        chr = qemu_chr_open(label, p);
+        if (chr) {
+            chr = qemu_chr_open_mux(chr);
+            monitor_init(chr, !nographic);
+        } else {
+            printf("Unable to open driver: %s\n", p);
         }
-        printf("Unable to open driver: %s\n", p);
-        return 0;
     } else
 #ifndef _WIN32
     if (strstart(filename, "unix:", &p)) {
-	return qemu_chr_open_tcp(p, 0, 1);
+	chr = qemu_chr_open_tcp(p, 0, 1);
     } else if (strstart(filename, "file:", &p)) {
-        return qemu_chr_open_file_out(p);
+        chr = qemu_chr_open_file_out(p);
     } else if (strstart(filename, "pipe:", &p)) {
-        return qemu_chr_open_pipe(p);
+        chr = qemu_chr_open_pipe(p);
     } else if (!strcmp(filename, "pty")) {
-        return qemu_chr_open_pty();
+        chr = qemu_chr_open_pty();
     } else if (!strcmp(filename, "stdio")) {
-        return qemu_chr_open_stdio();
+        chr = qemu_chr_open_stdio();
     } else
 #if defined(__linux__)
     if (strstart(filename, "/dev/parport", NULL)) {
-        return qemu_chr_open_pp(filename);
+        chr = qemu_chr_open_pp(filename);
     } else
 #endif
 #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
     || defined(__NetBSD__) || defined(__OpenBSD__)
     if (strstart(filename, "/dev/", NULL)) {
-        return qemu_chr_open_tty(filename);
+        chr = qemu_chr_open_tty(filename);
     } else
 #endif
 #else /* !_WIN32 */
     if (strstart(filename, "COM", NULL)) {
-        return qemu_chr_open_win(filename);
+        chr = qemu_chr_open_win(filename);
     } else
     if (strstart(filename, "pipe:", &p)) {
-        return qemu_chr_open_win_pipe(p);
+        chr = qemu_chr_open_win_pipe(p);
     } else
     if (strstart(filename, "con:", NULL)) {
-        return qemu_chr_open_win_con(filename);
+        chr = qemu_chr_open_win_con(filename);
     } else
     if (strstart(filename, "file:", &p)) {
-        return qemu_chr_open_win_file_out(p);
+        chr = qemu_chr_open_win_file_out(p);
     } else
 #endif
 #ifdef CONFIG_BRLAPI
     if (!strcmp(filename, "braille")) {
-        return chr_baum_init();
+        chr = chr_baum_init();
     } else
 #endif
     {
-        return NULL;
+        chr = NULL;
+    }
+
+    if (chr) {
+        if (!chr->filename)
+            chr->filename = qemu_strdup(filename);
+        chr->label = qemu_strdup(label);
+        TAILQ_INSERT_TAIL(&chardevs, chr, next);
     }
+    return chr;
 }
 
 void qemu_chr_close(CharDriverState *chr)
 {
+    TAILQ_REMOVE(&chardevs, chr, next);
     if (chr->chr_close)
         chr->chr_close(chr);
+    qemu_free(chr->filename);
+    qemu_free(chr->label);
     qemu_free(chr);
 }
 
+void qemu_chr_info(void)
+{
+    CharDriverState *chr;
+
+    TAILQ_FOREACH(chr, &chardevs, next) {
+        term_printf("%s: filename=%s\n", chr->label, chr->filename);
+    }
+}
+
 /***********************************************************/
 /* network device redirectors */
 
@@ -9708,7 +9736,7 @@ int main(int argc, char **argv)
         }
     }
     if (monitor_device) {
-        monitor_hd = qemu_chr_open(monitor_device);
+        monitor_hd = qemu_chr_open("monitor", monitor_device);
         if (!monitor_hd) {
             fprintf(stderr, "qemu: could not open monitor device '%s'\n", monitor_device);
             exit(1);
@@ -9719,7 +9747,9 @@ int main(int argc, char **argv)
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         const char *devname = serial_devices[i];
         if (devname && strcmp(devname, "none")) {
-            serial_hds[i] = qemu_chr_open(devname);
+            char label[32];
+            snprintf(label, sizeof(label), "serial%d", i);
+            serial_hds[i] = qemu_chr_open(label, devname);
             if (!serial_hds[i]) {
                 fprintf(stderr, "qemu: could not open serial device '%s'\n",
                         devname);
@@ -9733,7 +9763,9 @@ int main(int argc, char **argv)
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         const char *devname = parallel_devices[i];
         if (devname && strcmp(devname, "none")) {
-            parallel_hds[i] = qemu_chr_open(devname);
+            char label[32];
+            snprintf(label, sizeof(label), "parallel%d", i);
+            parallel_hds[i] = qemu_chr_open(label, devname);
             if (!parallel_hds[i]) {
                 fprintf(stderr, "qemu: could not open parallel device '%s'\n",
                         devname);
-- 
1.5.5.1

Re: [Qemu-devel] [PATCH] Implement "info chardev" command.

[Anthony Liguori]

Gerd Hoffmann wrote:
>   Hi,
>
> This patch makes qemu keep track of the character devices in use and
> implements a "info chardev" monitor command to print a list.
>   

Is it better to enumerate all chardevs or to provide info functions for 
each type of chardev (i.e. 'info serial', 'info parallel').  I don't 
have a strong feeling either way.

The only thing I'd suggest about this patch is that if you're going to 
print pty:/dev/pty/3, then you should also support that syntax for 
specifying a pty.  Otherwise, there's a lack of symmetry.

Regards,

Anthony Liguori

> please apply,
>   Gerd
>   

Re: [Qemu-devel] [PATCH] Implement "info chardev" command.

[Gerd Hoffmann]

Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>>   Hi,
>>
>> This patch makes qemu keep track of the character devices in use and
>> implements a "info chardev" monitor command to print a list.
>>   
> 
> Is it better to enumerate all chardevs or to provide info functions for
> each type of chardev (i.e. 'info serial', 'info parallel').  I don't
> have a strong feeling either way.

I did it this way because
 (a) It is easier to implement
 (b) Covers everything, not just serial and parallel.  There are also
     usb serial dongles, and it also lists gdb & monitor.  And in case
     we get more in the future, it is already covered ;)
 (c) I prefer to keep the number of info commands low.

> The only thing I'd suggest about this patch is that if you're going to
> print pty:/dev/pty/3, then you should also support that syntax for
> specifying a pty.  Otherwise, there's a lack of symmetry.

It's not a bug, it's a feature.

First, when allocating a pts, the kernel hands out the first free one.
There simply is no way to specify one, thus I simply can't support a
pty:/dev/pts/$nr syntax on the command line.

Second, I think even for other types it would be useful to support this
asymmetric syntax.  Allow "unix:" without specifying a path, then have
qemu allocate a random one in $TMPDIR and report the path via monitor.
Likewise for tcp ports.  That allows moving the ressource allocation
from the management application into qemu, closing race windows along
the way.  No patches yet though.

Oh, the same thing for vnc ports is still on the todo list too.
Remember the "-vnc :1,to=10" feature discussed a few weeks (months?) ago
on the list?

cheers,
  Gerd