[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [RESEND][PATCH] gdbstub: listen on ipv4 address
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [RESEND][PATCH] gdbstub: listen on ipv4 address |
Date: |
Wed, 28 Jan 2009 13:04:21 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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