diff mbox

[V3,3/3] New option -gdb-opts

Message ID 1363109492-1901-4-git-send-email-chouteau@adacore.com
State New
Headers show

Commit Message

Fabien Chouteau March 12, 2013, 5:31 p.m. UTC
We introduce a new command line option. It's a generic option to
customize the gdb server:

-gdb-opts [attached=on|off]

The only parameter for now is "attached".

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 gdbstub.c              |   28 +++++++++++++++++++++++++++-
 include/exec/gdbstub.h |    2 ++
 qemu-options.hx        |   17 +++++++++++++++++
 vl.c                   |    3 +++
 4 files changed, 49 insertions(+), 1 deletion(-)

Comments

Markus Armbruster March 14, 2013, 8:44 a.m. UTC | #1
Fabien Chouteau <chouteau@adacore.com> writes:

> We introduce a new command line option. It's a generic option to
> customize the gdb server:
>
> -gdb-opts [attached=on|off]
>
> The only parameter for now is "attached".
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>

--gdb-opts complements existing --gdb.  You need to use both for full
control.

I figure you do this because you can't extend --gdb, as its argument is
in legacy character device syntax, not QemuOpts.

We had similar cases before, and solved them differently: create a more
general option, then make the old one sugar for the new one.

For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
is in monitor_parse().
Fabien Chouteau March 14, 2013, 10:59 a.m. UTC | #2
On 03/14/2013 09:44 AM, Markus Armbruster wrote:
> Fabien Chouteau <chouteau@adacore.com> writes:
>
>> We introduce a new command line option. It's a generic option to
>> customize the gdb server:
>>
>> -gdb-opts [attached=on|off]
>>
>> The only parameter for now is "attached".
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>
> --gdb-opts complements existing --gdb.  You need to use both for full
> control.
>
> I figure you do this because you can't extend --gdb, as its argument is
> in legacy character device syntax, not QemuOpts.
>

That's right, maybe we can do some string manipulations to handle this case.

-gdb tcp::1234,attached=off

find ',attached={on|off}' and remove it from the string.

> We had similar cases before, and solved them differently: create a more
> general option, then make the old one sugar for the new one.
>
> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
> is in monitor_parse().
>

Something like:

-chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay -gdb-remote chardev=gdb1,attached=off

You still need two options for full control.

Regards,
Markus Armbruster March 14, 2013, 12:34 p.m. UTC | #3
Fabien Chouteau <chouteau@adacore.com> writes:

> On 03/14/2013 09:44 AM, Markus Armbruster wrote:
>> Fabien Chouteau <chouteau@adacore.com> writes:
>>
>>> We introduce a new command line option. It's a generic option to
>>> customize the gdb server:
>>>
>>> -gdb-opts [attached=on|off]
>>>
>>> The only parameter for now is "attached".
>>>
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>
>> --gdb-opts complements existing --gdb.  You need to use both for full
>> control.
>>
>> I figure you do this because you can't extend --gdb, as its argument is
>> in legacy character device syntax, not QemuOpts.
>>
>
> That's right, maybe we can do some string manipulations to handle this case.
>
> -gdb tcp::1234,attached=off
>
> find ',attached={on|off}' and remove it from the string.

That way is madness :)

>> We had similar cases before, and solved them differently: create a more
>> general option, then make the old one sugar for the new one.
>>
>> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
>> is in monitor_parse().
>>
>
> Something like:
>
> -chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay
> -gdb-remote chardev=gdb1,attached=off
>
> You still need two options for full control.

Yes, but following precedence is good.  Our command line is inconsistent
enough as it is.  Just my two cents.
Fabien Chouteau March 14, 2013, 4:33 p.m. UTC | #4
On 03/14/2013 01:34 PM, Markus Armbruster wrote:
> Fabien Chouteau <chouteau@adacore.com> writes:
> 
>> On 03/14/2013 09:44 AM, Markus Armbruster wrote:
>>> Fabien Chouteau <chouteau@adacore.com> writes:
>>>
>>>> We introduce a new command line option. It's a generic option to
>>>> customize the gdb server:
>>>>
>>>> -gdb-opts [attached=on|off]
>>>>
>>>> The only parameter for now is "attached".
>>>>
>>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>>
>>> --gdb-opts complements existing --gdb.  You need to use both for full
>>> control.
>>>
>>> I figure you do this because you can't extend --gdb, as its argument is
>>> in legacy character device syntax, not QemuOpts.
>>>
>>
>> That's right, maybe we can do some string manipulations to handle this case.
>>
>> -gdb tcp::1234,attached=off
>>
>> find ',attached={on|off}' and remove it from the string.
>
> That way is madness :)

Come on, you've seen worse ;)

>>> We had similar cases before, and solved them differently: create a more
>>> general option, then make the old one sugar for the new one.
>>>
>>> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
>>> is in monitor_parse().
>>>
>>
>> Something like:
>>
>> -chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay
>> -gdb-remote chardev=gdb1,attached=off
>>
>> You still need two options for full control.
> 
> Yes, but following precedence is good.  Our command line is inconsistent
> enough as it is.  Just my two cents.
> 

Fair enough, lets forget about the option this is too much work. I'll
just tweak the sources on our branch.

Regards,
Markus Armbruster March 19, 2013, 2:31 p.m. UTC | #5
Fabien Chouteau <chouteau@adacore.com> writes:

> On 03/14/2013 01:34 PM, Markus Armbruster wrote:
>> Fabien Chouteau <chouteau@adacore.com> writes:
>> 
>>> On 03/14/2013 09:44 AM, Markus Armbruster wrote:
>>>> Fabien Chouteau <chouteau@adacore.com> writes:
>>>>
>>>>> We introduce a new command line option. It's a generic option to
>>>>> customize the gdb server:
>>>>>
>>>>> -gdb-opts [attached=on|off]
>>>>>
>>>>> The only parameter for now is "attached".
>>>>>
>>>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>>>
>>>> --gdb-opts complements existing --gdb.  You need to use both for full
>>>> control.
>>>>
>>>> I figure you do this because you can't extend --gdb, as its argument is
>>>> in legacy character device syntax, not QemuOpts.
>>>>
>>>
>>> That's right, maybe we can do some string manipulations to handle this case.
>>>
>>> -gdb tcp::1234,attached=off
>>>
>>> find ',attached={on|off}' and remove it from the string.
>>
>> That way is madness :)
>
> Come on, you've seen worse ;)

Seen?  Perpetrated!  Stared madness in the eye, decided not to come back
for more ;)

>>>> We had similar cases before, and solved them differently: create a more
>>>> general option, then make the old one sugar for the new one.
>>>>
>>>> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
>>>> is in monitor_parse().
>>>>
>>>
>>> Something like:
>>>
>>> -chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay
>>> -gdb-remote chardev=gdb1,attached=off
>>>
>>> You still need two options for full control.
>> 
>> Yes, but following precedence is good.  Our command line is inconsistent
>> enough as it is.  Just my two cents.
>> 
>
> Fair enough, lets forget about the option this is too much work. I'll
> just tweak the sources on our branch.

Pity.
diff mbox

Patch

diff --git a/gdbstub.c b/gdbstub.c
index 0e94311..ee57124 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -32,7 +32,6 @@ 
 #include "monitor/monitor.h"
 #include "char/char.h"
 #include "sysemu/sysemu.h"
-#include "exec/gdbstub.h"
 #endif
 
 #define MAX_PACKET_LENGTH 4096
@@ -41,6 +40,7 @@ 
 #include "qemu/sockets.h"
 #include "sysemu/kvm.h"
 #include "qemu/bitops.h"
+#include "exec/gdbstub.h"
 
 #ifdef CONFIG_USER_ONLY
 static bool gdb_attached; /* false */
@@ -3063,3 +3063,29 @@  int gdbserver_start(const char *device)
     return 0;
 }
 #endif
+
+static QemuOptsList qemu_gdb_opts = {
+    .name = "gdb",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_gdb_opts.head),
+    .desc = {
+        {
+            .name = "attached",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
+void gdb_set_opts(const char *opts_str)
+{
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse(&qemu_gdb_opts, opts_str, 0);
+    if (!opts) {
+        exit(1);
+    }
+
+    gdb_attached = qemu_opt_get_bool(opts, "attached", gdb_attached);
+
+    qemu_opts_del(opts);
+}
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index ba20afa..86fc18b 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -50,4 +50,6 @@  int gdbserver_start(const char *port);
 /* in gdbstub-xml.c, generated by scripts/feature_to_c.sh */
 extern const char *const xml_builtin[][2];
 
+void gdb_set_opts(const char *opts_str);
+
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index cd76f2a..49a480f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2505,6 +2505,23 @@  within gdb and establish the connection via a pipe:
 @end example
 ETEXI
 
+DEF("gdb-opts", HAS_ARG, QEMU_OPTION_gdb_opts, \
+    "-gdb-opts attached=on|off\n"\
+    "                options for the gdb remote server\n", QEMU_ARCH_ALL)
+STEXI
+@item -gdb-opts @var{dev}
+@findex -gdb-opts
+Set options for the gdb remote server:
+@table @option
+@item attached=on|off:
+
+When 'on' (default), gdb sends a 'detach' command at the end of debugging
+session, otherwise gdb sends a 'kill' command.
+
+@end table
+
+ETEXI
+
 DEF("s", 0, QEMU_OPTION_s, \
     "-s              shorthand for -gdb tcp::" DEFAULT_GDBSTUB_PORT "\n",
     QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index 154f7ba..cce96b6 100644
--- a/vl.c
+++ b/vl.c
@@ -3251,6 +3251,9 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_gdb:
                 add_device_config(DEV_GDB, optarg);
                 break;
+            case QEMU_OPTION_gdb_opts:
+                gdb_set_opts(optarg);
+                break;
             case QEMU_OPTION_L:
                 data_dir = optarg;
                 break;