[Qemu-devel] [PATCH 2/2] gdbstub: Rework configuration via command line and monitor

[Qemu-devel] [PATCH 2/2] gdbstub: Rework configuration via command line and monitor

[Jan Kiszka]

Introduce a more powerful gdbstub configuration (system emulation olny)
via new switch '-gdb dev'. Keep '-s' as shorthand for '-gdb tcp::1234'.
Use the same syntax also for the corresponding monitor command
'gdbserver'. Its default also remains to listen on port 1234.

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

 gdbstub.c |   25 +++++++++++--------------
 monitor.c |   19 ++++++++++---------
 vl.c      |   33 ++++++++++++---------------------
 3 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e8ceaae..b6d41bb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2395,27 +2395,24 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)
     return len;
 }
 
-int gdbserver_start(const char *port)
+int gdbserver_start(const char *device)
 {
     GDBState *s;
-    char gdbstub_port_name[128];
-    int port_num;
-    char *p;
+    char gdbstub_device_name[128];
     CharDriverState *chr = NULL;
     CharDriverState *mon_chr;
 
-    if (!port || !*port)
-      return -1;
-    if (strcmp(port, "none") != 0) {
-        port_num = strtol(port, &p, 10);
-        if (*p == 0) {
-            /* A numeric value is interpreted as a port number.  */
-            snprintf(gdbstub_port_name, sizeof(gdbstub_port_name),
-                     "tcp::%d,nowait,nodelay,server", port_num);
-            port = gdbstub_port_name;
+    if (!device)
+        return -1;
+    if (strcmp(device, "none") != 0) {
+        if (strstart(device, "tcp:", NULL)) {
+            /* enforce required TCP attributes */
+            snprintf(gdbstub_device_name, sizeof(gdbstub_device_name),
+                     "%s,nowait,nodelay,server", device);
+            device = gdbstub_device_name;
         }
 
-        chr = qemu_chr_open("gdb", port, NULL);
+        chr = qemu_chr_open("gdb", device, NULL);
         if (!chr)
             return -1;
 
diff --git a/monitor.c b/monitor.c
index c6fe968..75c8663 100644
--- a/monitor.c
+++ b/monitor.c
@@ -570,17 +570,18 @@ static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
 }
 
 #ifdef CONFIG_GDBSTUB
-static void do_gdbserver(Monitor *mon, const char *port)
-{
-    if (!port)
-        port = DEFAULT_GDBSTUB_PORT;
-    if (gdbserver_start(port) < 0) {
-        monitor_printf(mon, "Could not open gdbserver socket on port '%s'\n",
-                       port);
-    } else if (strcmp(port, "none") == 0) {
+static void do_gdbserver(Monitor *mon, const char *device)
+{
+    if (!device)
+        device = "tcp::" DEFAULT_GDBSTUB_PORT;
+    if (gdbserver_start(device) < 0) {
+        monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
+                       device);
+    } else if (strcmp(device, "none") == 0) {
         monitor_printf(mon, "Disabled gdbserver\n");
     } else {
-        monitor_printf(mon, "Waiting gdb connection on port '%s'\n", port);
+        monitor_printf(mon, "Waiting for gdb connection on device '%s'\n",
+                       device);
     }
 }
 #endif
diff --git a/vl.c b/vl.c
index b62a2d4..d5c9e02 100644
--- a/vl.c
+++ b/vl.c
@@ -4073,8 +4073,8 @@ static void help(int exitcode)
            "-monitor dev    redirect the monitor to char device 'dev'\n"
            "-pidfile file   write PID to 'file'\n"
            "-S              freeze CPU at startup (use 'c' to start execution)\n"
-           "-s              wait gdb connection to port\n"
-           "-p port         set gdb connection port [default=%s]\n"
+           "-gdb dev        wait for gdb connection on 'dev'\n"
+           "-s              shorthand for -gdb tcp::%s\n"
            "-d item1,...    output log to %s (use -d ? for a list of log items)\n"
            "-hdachs c,h,s[,t]\n"
            "                force hard disk 0 physical geometry and the optional BIOS\n"
@@ -4214,7 +4214,7 @@ enum {
     QEMU_OPTION_pidfile,
     QEMU_OPTION_S,
     QEMU_OPTION_s,
-    QEMU_OPTION_p,
+    QEMU_OPTION_gdb,
     QEMU_OPTION_d,
     QEMU_OPTION_hdachs,
     QEMU_OPTION_L,
@@ -4336,7 +4336,7 @@ static const QEMUOption qemu_options[] = {
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "S", 0, QEMU_OPTION_S },
     { "s", 0, QEMU_OPTION_s },
-    { "p", HAS_ARG, QEMU_OPTION_p },
+    { "gdb", HAS_ARG, QEMU_OPTION_gdb },
     { "d", HAS_ARG, QEMU_OPTION_d },
     { "hdachs", HAS_ARG, QEMU_OPTION_hdachs },
     { "L", HAS_ARG, QEMU_OPTION_L },
@@ -4607,8 +4607,7 @@ static void termsig_setup(void)
 int main(int argc, char **argv, char **envp)
 {
 #ifdef CONFIG_GDBSTUB
-    int use_gdbstub;
-    const char *gdbstub_port;
+    const char *gdbstub_dev = NULL;
 #endif
     uint32_t boot_devices_bitmap = 0;
     int i;
@@ -4687,10 +4686,6 @@ int main(int argc, char **argv, char **envp)
     initrd_filename = NULL;
     ram_size = 0;
     vga_ram_size = VGA_RAM_SIZE;
-#ifdef CONFIG_GDBSTUB
-    use_gdbstub = 0;
-    gdbstub_port = DEFAULT_GDBSTUB_PORT;
-#endif
     snapshot = 0;
     nographic = 0;
     curses = 0;
@@ -5023,10 +5018,10 @@ int main(int argc, char **argv, char **envp)
                 break;
 #ifdef CONFIG_GDBSTUB
             case QEMU_OPTION_s:
-                use_gdbstub = 1;
+                gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT;
                 break;
-            case QEMU_OPTION_p:
-                gdbstub_port = optarg;
+            case QEMU_OPTION_gdb:
+                gdbstub_dev = optarg;
                 break;
 #endif
             case QEMU_OPTION_L:
@@ -5731,14 +5726,10 @@ int main(int argc, char **argv, char **envp)
     }
 
 #ifdef CONFIG_GDBSTUB
-    if (use_gdbstub) {
-        /* XXX: use standard host:port notation and modify options
-           accordingly. */
-        if (gdbserver_start(gdbstub_port) < 0) {
-            fprintf(stderr, "qemu: could not open gdbstub device on port '%s'\n",
-                    gdbstub_port);
-            exit(1);
-        }
+    if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
+        fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
+                gdbstub_dev);
+        exit(1);
     }
 #endif
 

[Qemu-devel] [PATCH 2/2 -v2] gdbstub: Rework configuration via command line and monitor

[Jan Kiszka]

Introduce a more powerful gdbstub configuration (system emulation olny)
via new switch '-gdb dev'. Keep '-s' as shorthand for '-gdb tcp::1234'.
Use the same syntax also for the corresponding monitor command
'gdbserver'. Its default also remains to listen on port 1234.

Changes in v2:
 - Support for pipe-based like to gdb (target remote | qemu -gdb stdio)
 - Properly update the qemu-doc

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

 gdbstub.c     |   41 +++++++++++++++++++++++++++--------------
 monitor.c     |   19 ++++++++++---------
 qemu-doc.texi |   12 +++++++++++-
 vl.c          |   33 ++++++++++++---------------------
 4 files changed, 60 insertions(+), 45 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e8ceaae..cea375f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2395,27 +2395,40 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)
     return len;
 }
 
-int gdbserver_start(const char *port)
+#ifndef _WIN32
+static void gdb_sigterm_handler(int signal)
+{
+    if (vm_running)
+        vm_stop(EXCP_INTERRUPT);
+}
+#endif
+
+int gdbserver_start(const char *device)
 {
     GDBState *s;
-    char gdbstub_port_name[128];
-    int port_num;
-    char *p;
+    char gdbstub_device_name[128];
     CharDriverState *chr = NULL;
     CharDriverState *mon_chr;
 
-    if (!port || !*port)
-      return -1;
-    if (strcmp(port, "none") != 0) {
-        port_num = strtol(port, &p, 10);
-        if (*p == 0) {
-            /* A numeric value is interpreted as a port number.  */
-            snprintf(gdbstub_port_name, sizeof(gdbstub_port_name),
-                     "tcp::%d,nowait,nodelay,server", port_num);
-            port = gdbstub_port_name;
+    if (!device)
+        return -1;
+    if (strcmp(device, "none") != 0) {
+        if (strstart(device, "tcp:", NULL)) {
+            /* enforce required TCP attributes */
+            snprintf(gdbstub_device_name, sizeof(gdbstub_device_name),
+                     "%s,nowait,nodelay,server", device);
+            device = gdbstub_device_name;
         }
+#ifndef _WIN32
+        else if (strcmp(device, "stdio") == 0) {
+            struct sigaction act;
 
-        chr = qemu_chr_open("gdb", port, NULL);
+            memset(&act, 0, sizeof(act));
+            act.sa_handler = gdb_sigterm_handler;
+            sigaction(SIGINT, &act, NULL);
+        }
+#endif
+        chr = qemu_chr_open("gdb", device, NULL);
         if (!chr)
             return -1;
 
diff --git a/monitor.c b/monitor.c
index c6fe968..75c8663 100644
--- a/monitor.c
+++ b/monitor.c
@@ -570,17 +570,18 @@ static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
 }
 
 #ifdef CONFIG_GDBSTUB
-static void do_gdbserver(Monitor *mon, const char *port)
-{
-    if (!port)
-        port = DEFAULT_GDBSTUB_PORT;
-    if (gdbserver_start(port) < 0) {
-        monitor_printf(mon, "Could not open gdbserver socket on port '%s'\n",
-                       port);
-    } else if (strcmp(port, "none") == 0) {
+static void do_gdbserver(Monitor *mon, const char *device)
+{
+    if (!device)
+        device = "tcp::" DEFAULT_GDBSTUB_PORT;
+    if (gdbserver_start(device) < 0) {
+        monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
+                       device);
+    } else if (strcmp(device, "none") == 0) {
         monitor_printf(mon, "Disabled gdbserver\n");
     } else {
-        monitor_printf(mon, "Waiting gdb connection on port '%s'\n", port);
+        monitor_printf(mon, "Waiting for gdb connection on device '%s'\n",
+                       device);
     }
 }
 #endif
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 8efc943..d70d7be 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1097,8 +1097,18 @@ from a script.
 @item -S
 Do not start CPU at startup (you must type 'c' in the monitor).
 
+@item -gdb @var{dev}
+
+Wait for gdb connection on device @var{dev} (@pxref{gdb_usage}). Typical
+connections will likely be TCP-based, but also unix sockets or even stdio
+are reasonable use case. The latter is allowing to start qemu from within
+gdb and establish the connection via a pipe:
+
+(gdb) target remote | exec qemu -gdb stdio ...
+
 @item -s
-Wait gdb connection to port 1234 (@pxref{gdb_usage}).
+Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
+(@pxref{gdb_usage}).
 
 @item -p @var{port}
 Change gdb connection port.  @var{port} can be either a decimal number
diff --git a/vl.c b/vl.c
index abc7f5d..6e31a00 100644
--- a/vl.c
+++ b/vl.c
@@ -4073,8 +4073,8 @@ static void help(int exitcode)
            "-monitor dev    redirect the monitor to char device 'dev'\n"
            "-pidfile file   write PID to 'file'\n"
            "-S              freeze CPU at startup (use 'c' to start execution)\n"
-           "-s              wait gdb connection to port\n"
-           "-p port         set gdb connection port [default=%s]\n"
+           "-gdb dev        wait for gdb connection on 'dev'\n"
+           "-s              shorthand for -gdb tcp::%s\n"
            "-d item1,...    output log to %s (use -d ? for a list of log items)\n"
            "-hdachs c,h,s[,t]\n"
            "                force hard disk 0 physical geometry and the optional BIOS\n"
@@ -4214,7 +4214,7 @@ enum {
     QEMU_OPTION_pidfile,
     QEMU_OPTION_S,
     QEMU_OPTION_s,
-    QEMU_OPTION_p,
+    QEMU_OPTION_gdb,
     QEMU_OPTION_d,
     QEMU_OPTION_hdachs,
     QEMU_OPTION_L,
@@ -4336,7 +4336,7 @@ static const QEMUOption qemu_options[] = {
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "S", 0, QEMU_OPTION_S },
     { "s", 0, QEMU_OPTION_s },
-    { "p", HAS_ARG, QEMU_OPTION_p },
+    { "gdb", HAS_ARG, QEMU_OPTION_gdb },
     { "d", HAS_ARG, QEMU_OPTION_d },
     { "hdachs", HAS_ARG, QEMU_OPTION_hdachs },
     { "L", HAS_ARG, QEMU_OPTION_L },
@@ -4607,8 +4607,7 @@ static void termsig_setup(void)
 int main(int argc, char **argv, char **envp)
 {
 #ifdef CONFIG_GDBSTUB
-    int use_gdbstub;
-    const char *gdbstub_port;
+    const char *gdbstub_dev = NULL;
 #endif
     uint32_t boot_devices_bitmap = 0;
     int i;
@@ -4687,10 +4686,6 @@ int main(int argc, char **argv, char **envp)
     initrd_filename = NULL;
     ram_size = 0;
     vga_ram_size = VGA_RAM_SIZE;
-#ifdef CONFIG_GDBSTUB
-    use_gdbstub = 0;
-    gdbstub_port = DEFAULT_GDBSTUB_PORT;
-#endif
     snapshot = 0;
     nographic = 0;
     curses = 0;
@@ -5023,10 +5018,10 @@ int main(int argc, char **argv, char **envp)
                 break;
 #ifdef CONFIG_GDBSTUB
             case QEMU_OPTION_s:
-                use_gdbstub = 1;
+                gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT;
                 break;
-            case QEMU_OPTION_p:
-                gdbstub_port = optarg;
+            case QEMU_OPTION_gdb:
+                gdbstub_dev = optarg;
                 break;
 #endif
             case QEMU_OPTION_L:
@@ -5732,14 +5727,10 @@ int main(int argc, char **argv, char **envp)
     }
 
 #ifdef CONFIG_GDBSTUB
-    if (use_gdbstub) {
-        /* XXX: use standard host:port notation and modify options
-           accordingly. */
-        if (gdbserver_start(gdbstub_port) < 0) {
-            fprintf(stderr, "qemu: could not open gdbstub device on port '%s'\n",
-                    gdbstub_port);
-            exit(1);
-        }
+    if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
+        fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
+                gdbstub_dev);
+        exit(1);
     }
 #endif
 

[Qemu-devel] [PATCH 2/2 -v3] gdbstub: Rework configuration via command line and monitor

[Jan Kiszka]

Jan Kiszka wrote:
> +@item -gdb @var{dev}
> +
> +Wait for gdb connection on device @var{dev} (@pxref{gdb_usage}). Typical
> +connections will likely be TCP-based, but also unix sockets or even stdio
> +are reasonable use case. The latter is allowing to start qemu from within
> +gdb and establish the connection via a pipe:
> +
> +(gdb) target remote | exec qemu -gdb stdio ...
> +

In fact, unix sockets are not supported by gdb, but udp or /dev/pts/n.

-------->

Introduce a more powerful gdbstub configuration (system emulation only)
via the new switch '-gdb dev'. Keep '-s' as shorthand for
'-gdb tcp::1234'. Use the same syntax also for the corresponding monitor
command 'gdbserver'. Its default remains to listen on port 1234.

Changes in v3:
 - Fix documentation

Changes in v2:
 - Support for pipe-based like to gdb (target remote | qemu -gdb stdio)
 - Properly update the qemu-doc

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

 gdbstub.c     |   41 +++++++++++++++++++++++++++--------------
 monitor.c     |   19 ++++++++++---------
 qemu-doc.texi |   14 ++++++++++++--
 vl.c          |   33 ++++++++++++---------------------
 4 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e8ceaae..cea375f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2395,27 +2395,40 @@ static int gdb_monitor_write(CharDriverState
*chr, const uint8_t *buf, int len)
     return len;
 }

-int gdbserver_start(const char *port)
+#ifndef _WIN32
+static void gdb_sigterm_handler(int signal)
+{
+    if (vm_running)
+        vm_stop(EXCP_INTERRUPT);
+}
+#endif
+
+int gdbserver_start(const char *device)
 {
     GDBState *s;
-    char gdbstub_port_name[128];
-    int port_num;
-    char *p;
+    char gdbstub_device_name[128];
     CharDriverState *chr = NULL;
     CharDriverState *mon_chr;

-    if (!port || !*port)
-      return -1;
-    if (strcmp(port, "none") != 0) {
-        port_num = strtol(port, &p, 10);
-        if (*p == 0) {
-            /* A numeric value is interpreted as a port number.  */
-            snprintf(gdbstub_port_name, sizeof(gdbstub_port_name),
-                     "tcp::%d,nowait,nodelay,server", port_num);
-            port = gdbstub_port_name;
+    if (!device)
+        return -1;
+    if (strcmp(device, "none") != 0) {
+        if (strstart(device, "tcp:", NULL)) {
+            /* enforce required TCP attributes */
+            snprintf(gdbstub_device_name, sizeof(gdbstub_device_name),
+                     "%s,nowait,nodelay,server", device);
+            device = gdbstub_device_name;
         }
+#ifndef _WIN32
+        else if (strcmp(device, "stdio") == 0) {
+            struct sigaction act;

-        chr = qemu_chr_open("gdb", port, NULL);
+            memset(&act, 0, sizeof(act));
+            act.sa_handler = gdb_sigterm_handler;
+            sigaction(SIGINT, &act, NULL);
+        }
+#endif
+        chr = qemu_chr_open("gdb", device, NULL);
         if (!chr)
             return -1;

diff --git a/monitor.c b/monitor.c
index c6fe968..75c8663 100644
--- a/monitor.c
+++ b/monitor.c
@@ -570,17 +570,18 @@ static void encrypted_bdrv_it(void *opaque,
BlockDriverState *bs)
 }

 #ifdef CONFIG_GDBSTUB
-static void do_gdbserver(Monitor *mon, const char *port)
-{
-    if (!port)
-        port = DEFAULT_GDBSTUB_PORT;
-    if (gdbserver_start(port) < 0) {
-        monitor_printf(mon, "Could not open gdbserver socket on port
'%s'\n",
-                       port);
-    } else if (strcmp(port, "none") == 0) {
+static void do_gdbserver(Monitor *mon, const char *device)
+{
+    if (!device)
+        device = "tcp::" DEFAULT_GDBSTUB_PORT;
+    if (gdbserver_start(device) < 0) {
+        monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
+                       device);
+    } else if (strcmp(device, "none") == 0) {
         monitor_printf(mon, "Disabled gdbserver\n");
     } else {
-        monitor_printf(mon, "Waiting gdb connection on port '%s'\n", port);
+        monitor_printf(mon, "Waiting for gdb connection on device '%s'\n",
+                       device);
     }
 }
 #endif
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 8efc943..8049a0d 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1097,8 +1097,18 @@ from a script.
 @item -S
 Do not start CPU at startup (you must type 'c' in the monitor).

+@item -gdb @var{dev}
+
+Wait for gdb connection on device @var{dev} (@pxref{gdb_usage}). Typical
+connections will likely be TCP-based, but also UDP, pseudo TTY, or even
+stdio are reasonable use case. The latter is allowing to start qemu from
+within gdb and establish the connection via a pipe:
+
+(gdb) target remote | exec qemu -gdb stdio ...
+
 @item -s
-Wait gdb connection to port 1234 (@pxref{gdb_usage}).
+Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
+(@pxref{gdb_usage}).

 @item -p @var{port}
 Change gdb connection port.  @var{port} can be either a decimal number
@@ -3114,7 +3124,7 @@ MV88W8xx8 Ethernet controller
 @item
 MV88W8618 audio controller, WM8750 CODEC and mixer
 @item
-128??64 display with brightness control
+128x64 display with brightness control
 @item
 2 buttons, 2 navigation wheels with button function
 @end itemize
diff --git a/vl.c b/vl.c
index abc7f5d..6e31a00 100644
--- a/vl.c
+++ b/vl.c
@@ -4073,8 +4073,8 @@ static void help(int exitcode)
            "-monitor dev    redirect the monitor to char device 'dev'\n"
            "-pidfile file   write PID to 'file'\n"
            "-S              freeze CPU at startup (use 'c' to start
execution)\n"
-           "-s              wait gdb connection to port\n"
-           "-p port         set gdb connection port [default=%s]\n"
+           "-gdb dev        wait for gdb connection on 'dev'\n"
+           "-s              shorthand for -gdb tcp::%s\n"
            "-d item1,...    output log to %s (use -d ? for a list of
log items)\n"
            "-hdachs c,h,s[,t]\n"
            "                force hard disk 0 physical geometry and the
optional BIOS\n"
@@ -4214,7 +4214,7 @@ enum {
     QEMU_OPTION_pidfile,
     QEMU_OPTION_S,
     QEMU_OPTION_s,
-    QEMU_OPTION_p,
+    QEMU_OPTION_gdb,
     QEMU_OPTION_d,
     QEMU_OPTION_hdachs,
     QEMU_OPTION_L,
@@ -4336,7 +4336,7 @@ static const QEMUOption qemu_options[] = {
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "S", 0, QEMU_OPTION_S },
     { "s", 0, QEMU_OPTION_s },
-    { "p", HAS_ARG, QEMU_OPTION_p },
+    { "gdb", HAS_ARG, QEMU_OPTION_gdb },
     { "d", HAS_ARG, QEMU_OPTION_d },
     { "hdachs", HAS_ARG, QEMU_OPTION_hdachs },
     { "L", HAS_ARG, QEMU_OPTION_L },
@@ -4607,8 +4607,7 @@ static void termsig_setup(void)
 int main(int argc, char **argv, char **envp)
 {
 #ifdef CONFIG_GDBSTUB
-    int use_gdbstub;
-    const char *gdbstub_port;
+    const char *gdbstub_dev = NULL;
 #endif
     uint32_t boot_devices_bitmap = 0;
     int i;
@@ -4687,10 +4686,6 @@ int main(int argc, char **argv, char **envp)
     initrd_filename = NULL;
     ram_size = 0;
     vga_ram_size = VGA_RAM_SIZE;
-#ifdef CONFIG_GDBSTUB
-    use_gdbstub = 0;
-    gdbstub_port = DEFAULT_GDBSTUB_PORT;
-#endif
     snapshot = 0;
     nographic = 0;
     curses = 0;
@@ -5023,10 +5018,10 @@ int main(int argc, char **argv, char **envp)
                 break;
 #ifdef CONFIG_GDBSTUB
             case QEMU_OPTION_s:
-                use_gdbstub = 1;
+                gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT;
                 break;
-            case QEMU_OPTION_p:
-                gdbstub_port = optarg;
+            case QEMU_OPTION_gdb:
+                gdbstub_dev = optarg;
                 break;
 #endif
             case QEMU_OPTION_L:
@@ -5732,14 +5727,10 @@ int main(int argc, char **argv, char **envp)
     }

 #ifdef CONFIG_GDBSTUB
-    if (use_gdbstub) {
-        /* XXX: use standard host:port notation and modify options
-           accordingly. */
-        if (gdbserver_start(gdbstub_port) < 0) {
-            fprintf(stderr, "qemu: could not open gdbstub device on
port '%s'\n",
-                    gdbstub_port);
-            exit(1);
-        }
+    if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
+        fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
+                gdbstub_dev);
+        exit(1);
     }
 #endif

Re: [Qemu-devel] [PATCH 2/2 -v2] gdbstub: Rework configuration via command line and monitor

[Paul Brook]

On Saturday 21 March 2009, Jan Kiszka wrote:
> Introduce a more powerful gdbstub configuration (system emulation olny)
> via new switch '-gdb dev'. Keep '-s' as shorthand for '-gdb tcp::1234'.
> Use the same syntax also for the corresponding monitor command
> 'gdbserver'. Its default also remains to listen on port 1234.

I don't see how this is an improvement. In fact it provides less functionality 
than the current -p, which is also consistent with other similar qemu 
options.

Paul

Re: [Qemu-devel] [PATCH 2/2 -v2] gdbstub: Rework configuration via command line and monitor

[Jan Kiszka]

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

Paul Brook wrote:
> On Saturday 21 March 2009, Jan Kiszka wrote:
>> Introduce a more powerful gdbstub configuration (system emulation olny)
>> via new switch '-gdb dev'. Keep '-s' as shorthand for '-gdb tcp::1234'.
>> Use the same syntax also for the corresponding monitor command
>> 'gdbserver'. Its default also remains to listen on port 1234.
> 
> I don't see how this is an improvement. In fact it provides less functionality 
> than the current -p, which is also consistent with other similar qemu 
> options.

Hmm, I was not able to find any further examples for this mixture of
standard device syntax and special interpretation of numerical values -
even in the qemu-doc, the only place where the special case of 'port'
was mentioned.

So I still see this as a significant improvements as it
 - aligns -gdb to the standard syntax use in -monitor, -serial or
   -parallel, making the interface more intuitive
 - documents this properly

I will adjust description to clarify that this patch refactors the
existing feature into a canonical form, but does not actually add a new one.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [PATCH 2/2] gdbstub: Rework configuration via command line and monitor

[Anthony Liguori]

Jan Kiszka wrote:
> diff --git a/vl.c b/vl.c
> index b62a2d4..d5c9e02 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4073,8 +4073,8 @@ static void help(int exitcode)
>             "-monitor dev    redirect the monitor to char device 'dev'\n"
>             "-pidfile file   write PID to 'file'\n"
>             "-S              freeze CPU at startup (use 'c' to start execution)\n"
> -           "-s              wait gdb connection to port\n"
> -           "-p port         set gdb connection port [default=%s]\n"
> +           "-gdb dev        wait for gdb connection on 'dev'\n"
> +           "-s              shorthand for -gdb tcp::%s\n"
>             "-d item1,...    output log to %s (use -d ? for a list of log items)\n"
>             "-hdachs c,h,s[,t]\n"
>             "                force hard disk 0 physical geometry and the optional BIOS\n"
>   

Can you rebase?  That will also get you the qemu-doc.texi documentation 
for free which was missing in this version of the patch.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH v4] gdbstub: Rework configuration via command line and monitor

[Jan Kiszka]

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

Introduce a more canonical gdbstub configuration (system emulation only)
via the new switch '-gdb dev'. Keep '-s' as shorthand for
'-gdb tcp::1234'. Use the same syntax also for the corresponding monitor
command 'gdbserver'. Its default remains to listen on TCP port 1234.

Changes in v4:
 - Rebased over new command line switches meta file

Changes in v3:
 - Fix documentation

Changes in v2:
 - Support for pipe-based like to gdb (target remote | qemu -gdb stdio)
 - Properly update the qemu-doc

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

 gdbstub.c       |   41 +++++++++++++++++++++++++++--------------
 monitor.c       |   19 ++++++++++---------
 qemu-options.hx |   26 ++++++++++++++++----------
 vl.c            |   25 ++++++++-----------------
 4 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e8ceaae..cea375f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2395,27 +2395,40 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)
     return len;
 }
 
-int gdbserver_start(const char *port)
+#ifndef _WIN32
+static void gdb_sigterm_handler(int signal)
+{
+    if (vm_running)
+        vm_stop(EXCP_INTERRUPT);
+}
+#endif
+
+int gdbserver_start(const char *device)
 {
     GDBState *s;
-    char gdbstub_port_name[128];
-    int port_num;
-    char *p;
+    char gdbstub_device_name[128];
     CharDriverState *chr = NULL;
     CharDriverState *mon_chr;
 
-    if (!port || !*port)
-      return -1;
-    if (strcmp(port, "none") != 0) {
-        port_num = strtol(port, &p, 10);
-        if (*p == 0) {
-            /* A numeric value is interpreted as a port number.  */
-            snprintf(gdbstub_port_name, sizeof(gdbstub_port_name),
-                     "tcp::%d,nowait,nodelay,server", port_num);
-            port = gdbstub_port_name;
+    if (!device)
+        return -1;
+    if (strcmp(device, "none") != 0) {
+        if (strstart(device, "tcp:", NULL)) {
+            /* enforce required TCP attributes */
+            snprintf(gdbstub_device_name, sizeof(gdbstub_device_name),
+                     "%s,nowait,nodelay,server", device);
+            device = gdbstub_device_name;
         }
+#ifndef _WIN32
+        else if (strcmp(device, "stdio") == 0) {
+            struct sigaction act;
 
-        chr = qemu_chr_open("gdb", port, NULL);
+            memset(&act, 0, sizeof(act));
+            act.sa_handler = gdb_sigterm_handler;
+            sigaction(SIGINT, &act, NULL);
+        }
+#endif
+        chr = qemu_chr_open("gdb", device, NULL);
         if (!chr)
             return -1;
 
diff --git a/monitor.c b/monitor.c
index c6fe968..75c8663 100644
--- a/monitor.c
+++ b/monitor.c
@@ -570,17 +570,18 @@ static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
 }
 
 #ifdef CONFIG_GDBSTUB
-static void do_gdbserver(Monitor *mon, const char *port)
-{
-    if (!port)
-        port = DEFAULT_GDBSTUB_PORT;
-    if (gdbserver_start(port) < 0) {
-        monitor_printf(mon, "Could not open gdbserver socket on port '%s'\n",
-                       port);
-    } else if (strcmp(port, "none") == 0) {
+static void do_gdbserver(Monitor *mon, const char *device)
+{
+    if (!device)
+        device = "tcp::" DEFAULT_GDBSTUB_PORT;
+    if (gdbserver_start(device) < 0) {
+        monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
+                       device);
+    } else if (strcmp(device, "none") == 0) {
         monitor_printf(mon, "Disabled gdbserver\n");
     } else {
-        monitor_printf(mon, "Waiting gdb connection on port '%s'\n", port);
+        monitor_printf(mon, "Waiting for gdb connection on device '%s'\n",
+                       device);
     }
 }
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 6c58e2a..8a8c938 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1216,19 +1216,25 @@ STEXI
 Do not start CPU at startup (you must type 'c' in the monitor).
 ETEXI
 
-DEF("s", 0, QEMU_OPTION_s, \
-    "-s              wait gdb connection to port\n")
-STEXI
-@item -s
-Wait gdb connection to port 1234 (@pxref{gdb_usage}).
+DEF("gdb", HAS_ARG, QEMU_OPTION_gdb, \
+    "-gdb dev        wait for gdb connection on 'dev'\n")
+STEXI
+@item -gdb @var{dev}
+Wait for gdb connection on device @var{dev} (@pxref{gdb_usage}). Typical
+connections will likely be TCP-based, but also UDP, pseudo TTY, or even
+stdio are reasonable use case. The latter is allowing to start qemu from
+within gdb and establish the connection via a pipe:
+@example
+(gdb) target remote | exec qemu -gdb stdio ...
+@end example
 ETEXI
 
-DEF("p", HAS_ARG, QEMU_OPTION_p, \
-    "-p port         set gdb connection port [default=%s]\n")
+DEF("s", 0, QEMU_OPTION_s, \
+    "-s              shorthand for -gdb tcp::%s\n")
 STEXI
-@item -p @var{port}
-Change gdb connection port.  @var{port} can be either a decimal number
-to specify a TCP port, or a host device (same devices as the serial port).
+@item -s
+Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
+(@pxref{gdb_usage}).
 ETEXI
 
 DEF("d", HAS_ARG, QEMU_OPTION_d, \
diff --git a/vl.c b/vl.c
index 5e6c621..8cb358f 100644
--- a/vl.c
+++ b/vl.c
@@ -4230,8 +4230,7 @@ static void termsig_setup(void)
 int main(int argc, char **argv, char **envp)
 {
 #ifdef CONFIG_GDBSTUB
-    int use_gdbstub;
-    const char *gdbstub_port;
+    const char *gdbstub_dev = NULL;
 #endif
     uint32_t boot_devices_bitmap = 0;
     int i;
@@ -4310,10 +4309,6 @@ int main(int argc, char **argv, char **envp)
     initrd_filename = NULL;
     ram_size = 0;
     vga_ram_size = VGA_RAM_SIZE;
-#ifdef CONFIG_GDBSTUB
-    use_gdbstub = 0;
-    gdbstub_port = DEFAULT_GDBSTUB_PORT;
-#endif
     snapshot = 0;
     nographic = 0;
     curses = 0;
@@ -4646,10 +4641,10 @@ int main(int argc, char **argv, char **envp)
                 break;
 #ifdef CONFIG_GDBSTUB
             case QEMU_OPTION_s:
-                use_gdbstub = 1;
+                gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT;
                 break;
-            case QEMU_OPTION_p:
-                gdbstub_port = optarg;
+            case QEMU_OPTION_gdb:
+                gdbstub_dev = optarg;
                 break;
 #endif
             case QEMU_OPTION_L:
@@ -5363,14 +5358,10 @@ int main(int argc, char **argv, char **envp)
     }
 
 #ifdef CONFIG_GDBSTUB
-    if (use_gdbstub) {
-        /* XXX: use standard host:port notation and modify options
-           accordingly. */
-        if (gdbserver_start(gdbstub_port) < 0) {
-            fprintf(stderr, "qemu: could not open gdbstub device on port '%s'\n",
-                    gdbstub_port);
-            exit(1);
-        }
+    if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
+        fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
+                gdbstub_dev);
+        exit(1);
     }
 #endif
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [PATCH v4] gdbstub: Rework configuration via command line and monitor

[Anthony Liguori]

Jan Kiszka wrote:
> Introduce a more canonical gdbstub configuration (system emulation only)
> via the new switch '-gdb dev'. Keep '-s' as shorthand for
> '-gdb tcp::1234'. Use the same syntax also for the corresponding monitor
> command 'gdbserver'. Its default remains to listen on TCP port 1234.
>
> Changes in v4:
>  - Rebased over new command line switches meta file
>
> Changes in v3:
>  - Fix documentation
>
> Changes in v2:
>  - Support for pipe-based like to gdb (target remote | qemu -gdb stdio)
>  - Properly update the qemu-doc
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>
>   
Applied.  Thanks.

-- 
Regards,

Anthony Liguori