From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LS99c-0006EL-4z for qemu-devel@nongnu.org; Wed, 28 Jan 2009 07:04:24 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LS99a-0006Dw-AT for qemu-devel@nongnu.org; Wed, 28 Jan 2009 07:04:23 -0500 Received: from [199.232.76.173] (port=48527 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LS99a-0006Do-3h for qemu-devel@nongnu.org; Wed, 28 Jan 2009 07:04:22 -0500 Received: from lizzard.sbs.de ([194.138.37.39]:16029) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LS99Z-0001lk-B5 for qemu-devel@nongnu.org; Wed, 28 Jan 2009 07:04:21 -0500 Received: from mail1.sbs.de (localhost [127.0.0.1]) by lizzard.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id n0SC4Hxi004254 for ; Wed, 28 Jan 2009 13:04:17 +0100 Received: from [139.25.109.167] (mchn012c.mchp.siemens.de [139.25.109.167] (may be forged)) by mail1.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id n0SC4HsN020898 for ; Wed, 28 Jan 2009 13:04:17 +0100 Message-ID: <498049C5.8090802@siemens.com> Date: Wed, 28 Jan 2009 13:04:21 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <6DF473C7837D4A31A3A6AF10F70F0F63@FSCPC> <497E350B.6080003@codemonkey.ws> <60118371B6BE4B52AEAE1C778543354A@FSCPC> <497F9177.6050008@codemonkey.ws> <498021EE.5060801@siemens.com> <20090128103858.GD1213@redhat.com> In-Reply-To: <20090128103858.GD1213@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RESEND][PATCH] gdbstub: listen on ipv4 address Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Daniel P. Berrange wrote: > On Wed, Jan 28, 2009 at 10:14:22AM +0100, Jan Kiszka wrote: >> Anthony Liguori wrote: >>> Sebastian Herbszt wrote: >>>> Anthony Liguori wrote: >>>>> Sebastian Herbszt wrote: >>>>>> Make gdbstub listen on ipv4 address like before r5697. >>>>> Any reason to make it explicit? >>>> There seems to be no support for IPv6 in gdb. >>> Are you unable to connect without the ipv4 option? My understanding was >>> that we shouldn't explicitly need this option. >> inet_listen (which is finally called for this) falls back to PF_UNSPEC >> when no protocol is given. I'm not sure if we can expect (according to >> specs) that all our host OSes pick the protocol we want (normally IPv4). > > There are three scenarios you need to consider wrt command line > args for TCP > > - IP address - the parser determines whether it IPv4 or V6 > address and asets PF_INET / PF_INET6 as needed > > - Hostname - with PF_UNSPEC, and AI_ADDRCONFIG, getaddrinfo() > will resolve the name, and return zero or more IP addresses. > An IPv6 address is only returned if a NIC has IPv6 enabled. > An IPv4 address is only returned if a NIC has IPv4 enabled. > QEMU listens on the first address it gets > > If an IPv6 address is returned, QEMU *disables* IPV6_V6ONLY > socket option, so the kernel also accepts IPv4 connections > on the IPv6 socket. > > - No hostname - with PF_UNSPEC, and AI_ADDRCONFIG, getaddrinf() > will return the 0.0.0.0 or :: depending on whether the host > has IPv6 enabled on any NICs. Again QEMU disables IPV6_V6ONLY > to ensure IPv4 connections can be accepted over the IPv6 socket > > Finally the ipv4/v6 command line flags in QEMU can override > the normal getaddrinfo() config. > > The GDB stub current code should be hitting scenario 3, and thus QEMU > should be listening on both IPv4 and IPv6 ports. That's true. gdb stumbled over optimizations and showed me the state before the getaddrinf call. In fact, AF_INET6 is passed to socket() as IPv6 is enabled for my host. > > If everything is working as I describe above, we should not need any > special case to force disable IPv6 for GDB stub, even if GDB itself is > IPv4 only - we always listen on IPv4 address even if we're also listening > on IPv6, unless the command line explicitly has the ',ipv6' flag provided. > > If this isn't working, then perhaps our call to disable the IPV6_V6ONLY > socket option is not working correctly on some OS ? Independent of that, having a canonical command line switch for the gdbstub has some value of its own. The current -s/-p duo is unfortunate. Find a proposal below. Quick-tested only, breaks user mode (which is hard-coded to IPv4, BTW). Jan ---------> diff --git a/gdbstub.c b/gdbstub.c index c99c080..58e55f5 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2411,26 +2411,20 @@ static void gdb_chr_event(void *opaque, int event) } } -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; - if (!port || !*port) + if (!device) return -1; - 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; - } + /* enforce required attributes */ + snprintf(gdbstub_device_name, sizeof(gdbstub_device_name), + "%s,nowait,nodelay,server", device); - chr = qemu_chr_open("gdb", port, NULL); + chr = qemu_chr_open("gdb", gdbstub_device_name, NULL); if (!chr) return -1; diff --git a/vl.c b/vl.c index 3676537..a2424ed 100644 --- a/vl.c +++ b/vl.c @@ -3985,8 +3985,7 @@ 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" + "-s [dev] wait for gdb connection on 'dev' [default=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" @@ -4050,6 +4049,7 @@ static void help(int exitcode) } #define HAS_ARG 0x0001 +#define OPT_ARG 0x0002 enum { /* Please keep in synch with help, qemu_options[] and @@ -4121,7 +4121,6 @@ enum { QEMU_OPTION_pidfile, QEMU_OPTION_S, QEMU_OPTION_s, - QEMU_OPTION_p, QEMU_OPTION_d, QEMU_OPTION_hdachs, QEMU_OPTION_L, @@ -4239,8 +4238,7 @@ static const QEMUOption qemu_options[] = { { "monitor", HAS_ARG, QEMU_OPTION_monitor }, { "pidfile", HAS_ARG, QEMU_OPTION_pidfile }, { "S", 0, QEMU_OPTION_S }, - { "s", 0, QEMU_OPTION_s }, - { "p", HAS_ARG, QEMU_OPTION_p }, + { "s", OPT_ARG, QEMU_OPTION_s }, { "d", HAS_ARG, QEMU_OPTION_d }, { "hdachs", HAS_ARG, QEMU_OPTION_hdachs }, { "L", HAS_ARG, QEMU_OPTION_L }, @@ -4548,8 +4546,8 @@ static void termsig_setup(void) int main(int argc, char **argv, char **envp) { #ifdef CONFIG_GDBSTUB - int use_gdbstub; - const char *gdbstub_port; + int use_gdbstub = 0; + const char *gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT; #endif uint32_t boot_devices_bitmap = 0; int i; @@ -4625,10 +4623,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; @@ -4698,6 +4692,9 @@ int main(int argc, char **argv, char **envp) exit(1); } optarg = argv[optind++]; + } else if ((popt->flags & OPT_ARG) && optind < argc && + argv[optind][0] != '-') { + optarg = argv[optind++]; } else { optarg = NULL; } @@ -4963,9 +4960,8 @@ int main(int argc, char **argv, char **envp) #ifdef CONFIG_GDBSTUB case QEMU_OPTION_s: use_gdbstub = 1; - break; - case QEMU_OPTION_p: - gdbstub_port = optarg; + if (optarg) + gdbstub_dev = optarg; break; #endif case QEMU_OPTION_L: @@ -5660,9 +5656,9 @@ int main(int argc, char **argv, char **envp) 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); + if (gdbserver_start(gdbstub_dev) < 0) { + fprintf(stderr, "qemu: could not open gdbstub device on '%s'\n", + gdbstub_dev); exit(1); } } -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux