On 2013-03-05 17:03, Fabien Chouteau wrote:
> With this patch GDB will issue a "detach" command at the end of a
> debugging session instead of a "kill". This behavior can be inverted
> with the new option -gdb-not-attached.
>
> This patch implements the requirement described in Jan Kiszka's patch:
> "gdbstub: Do not kill target in system emulation mode". The patch can
> therefore be reverted.
>
> Signed-off-by: Fabien Chouteau <chout...@adacore.com>
> ---
> gdbstub.c | 13 ++++++++++++-
> include/exec/gdbstub.h | 2 ++
> qemu-options.hx | 7 +++++++
> vl.c | 3 +++
> 4 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index e414ad9..c0686a9 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,9 @@
> #include "qemu/sockets.h"
> #include "sysemu/kvm.h"
> #include "qemu/bitops.h"
> +#include "exec/gdbstub.h"
> +
> +static bool gdb_attached = true;
>
> #ifndef TARGET_CPU_MEMORY_RW_DEBUG
> static inline int target_memory_rw_debug(CPUArchState *env, target_ulong
> addr,
> @@ -2491,6 +2493,10 @@ static int gdb_handle_packet(GDBState *s, const char
> *line_buf)
> break;
> }
> #endif
> + if (strncmp(p, "Attached", 8) == 0) {
> + put_packet(s, gdb_attached ? "1" : "0");
> + break;
> + }
This works as expected for system mode, but now inverts the behaviour
for user mode - that's unexpected and not ok.
> /* Unrecognised 'q' command. */
> goto unknown_command;
>
> @@ -3055,3 +3061,8 @@ int gdbserver_start(const char *device)
> return 0;
> }
> #endif
> +
> +void gdb_set_attached(bool attached)
> +{
> + gdb_attached = attached;
> +}
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index ba20afa..b2d3b13 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_attached(bool attached);
> +
> #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 6f9334a..026d3eb 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2988,6 +2988,13 @@ property must be set. These objects are placed in the
> '/objects' path.
> ETEXI
>
> +DEF("gdb-not-attached", 0, QEMU_OPTION_gdb_not_attached,
> + "-gdb-not-attached\n"
> + " Do not set Gdb remote server in attached mode.\n"
> + " When exiting debugging session, Gdb will send a
> 'kill'\n"
> + " command instead of a 'detach'.\n",
> + QEMU_ARCH_ALL)
> +
First of all, why do we need this configurable? In which use cases do
you want attached mode for user emulation or kill mode for system
emulation/virtualization?
Then, if you can convince us that such a switch is useful, a new
top-level command line option is still a no-go. We already have -gdb, so
this would have to become an option to it.
In any case, I would suggest to add static attached mode first (enabled
for system, disable for user), revert my patch and then, optionally,
propose an attached mode control.
Jan
signature.asc
Description: OpenPGP digital signature
