This started as a clean-up to properly pass a Error handler to the
gdbserver_start so we could do the right thing for command line and
HMP invocations.
Now that we have cleaned up foreach_device_config_or_exit() in earlier
patches we can further simplify by it by passing &error_fatal instead
of checking the return value. Having a return value is still useful
for HMP though so tweak the return to use a simple bool instead.
Message-Id: <20250109170619.2271193-11-alex.ben...@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
Acked-by: Ilya Leoshkevich <i...@linux.ibm.com>
---
v2
- split some work into pre-cursor patches
v3
- fix bsd build
- don't return false from gdbserver_open_port
- use error_setg_errno instead of perror
- split long line
---
include/exec/gdbstub.h | 8 +++++-
bsd-user/main.c | 2 +-
gdbstub/system.c | 22 +++++++++-------
gdbstub/user.c | 22 +++++++++-------
linux-user/main.c | 6 +----
monitor/hmp-cmds.c | 2 +-
system/vl.c | 59 ++++++++++++++++++++----------------------
7 files changed, 62 insertions(+), 59 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index d73f424f56..0675b0b646 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -49,12 +49,18 @@ void gdb_unregister_coprocessor_all(CPUState *cpu);
/**
* gdbserver_start: start the gdb server
* @port_or_device: connection spec for gdb
+ * @errp: error handle
*
* For CONFIG_USER this is either a tcp port or a path to a fifo. For
* system emulation you can use a full chardev spec for your gdbserver
* port.
+ *
+ * The error handle should be either &error_fatal (for start-up) or
+ * &error_warn (for QMP/HMP initiated sessions).
+ *
+ * Returns true when server successfully started.
*/
-int gdbserver_start(const char *port_or_device);
+bool gdbserver_start(const char *port_or_device, Error **errp);
/**
* gdb_feature_builder_init() - Initialize GDBFeatureBuilder.
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0a5bc57836..b2f6a9be2f 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -628,7 +628,7 @@ int main(int argc, char **argv)
target_cpu_init(env, regs);
if (gdbstub) {
- gdbserver_start(gdbstub);
+ gdbserver_start(gdbstub, &error_fatal);
gdb_handlesig(cpu, 0, NULL, NULL, 0);
}
cpu_loop(env);
diff --git a/gdbstub/system.c b/gdbstub/system.c
index 2d9fdff2fe..8ce79fa88c 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -330,26 +330,27 @@ static void create_processes(GDBState *s)
gdb_create_default_process(s);
}
-int gdbserver_start(const char *device)
+bool gdbserver_start(const char *device, Error **errp)
{
Chardev *chr = NULL;
Chardev *mon_chr;
g_autoptr(GString) cs = g_string_new(device);
if (!first_cpu) {
- error_report("gdbstub: meaningless to attach gdb to a "
- "machine without any CPU.");
- return -1;
+ error_setg(errp, "gdbstub: meaningless to attach gdb to a "
+ "machine without any CPU.");
+ return false;
}
if (!gdb_supports_guest_debug()) {
- error_report("gdbstub: current accelerator doesn't "
- "support guest debugging");
- return -1;
+ error_setg(errp, "gdbstub: current accelerator doesn't "
+ "support guest debugging");
+ return false;
}
if (cs->len == 0) {
- return -1;
+ error_setg(errp, "gdbstub: missing connection string");
+ return false;
}
trace_gdbstub_op_start(cs->str);
@@ -374,7 +375,8 @@ int gdbserver_start(const char *device)
*/
chr = qemu_chr_new_noreplay("gdb", cs->str, true, NULL);
if (!chr) {
- return -1;
+ error_setg(errp, "gdbstub: couldn't create chardev");
+ return false;
}
}
@@ -406,7 +408,7 @@ int gdbserver_start(const char *device)
gdbserver_system_state.mon_chr = mon_chr;
gdb_syscall_reset();
- return 0;
+ return true;
}
static void register_types(void)
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 0b4bfa9c48..c2bdfc3d49 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -13,6 +13,7 @@
#include "qemu/bitops.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
+#include "qapi/error.h"
#include "exec/hwaddr.h"
#include "exec/tb-flush.h"
#include "exec/gdbstub.h"
@@ -372,14 +373,14 @@ static bool gdb_accept_tcp(int gdb_fd)
return true;
}
-static int gdbserver_open_port(int port)
+static int gdbserver_open_port(int port, Error **errp)
{
struct sockaddr_in sockaddr;
int fd, ret;
fd = socket(PF_INET, SOCK_STREAM, 0);
if (fd < 0) {
- perror("socket");
+ error_setg_errno(errp, errno, "Failed to create socket");
return -1;
}
qemu_set_cloexec(fd);
@@ -391,13 +392,13 @@ static int gdbserver_open_port(int port)
sockaddr.sin_addr.s_addr = 0;
ret = bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr));
if (ret < 0) {
- perror("bind");
+ error_setg_errno(errp, errno, "Failed to bind socket");
close(fd);
return -1;
}
ret = listen(fd, 1);
if (ret < 0) {
- perror("listen");
+ error_setg_errno(errp, errno, "Failed to listen to socket");
close(fd);
return -1;
}
@@ -405,31 +406,32 @@ static int gdbserver_open_port(int port)
return fd;
}
-int gdbserver_start(const char *port_or_path)
+bool gdbserver_start(const char *port_or_path, Error **errp)
{
int port = g_ascii_strtoull(port_or_path, NULL, 10);
int gdb_fd;
if (port > 0) {
- gdb_fd = gdbserver_open_port(port);
+ gdb_fd = gdbserver_open_port(port, errp);
} else {
gdb_fd = gdbserver_open_socket(port_or_path);
}
if (gdb_fd < 0) {
- return -1;
+ return false;
}
if (port > 0 && gdb_accept_tcp(gdb_fd)) {
- return 0;
+ return true;
} else if (gdb_accept_socket(gdb_fd)) {
gdbserver_user_state.socket_path = g_strdup(port_or_path);
- return 0;
+ return true;
}
/* gone wrong */
close(gdb_fd);
- return -1;
+ error_setg(errp, "gdbstub: failed to accept connection");
+ return false;
}
void gdbserver_fork_start(void)
diff --git a/linux-user/main.c b/linux-user/main.c
index b97634a32d..7198fa0986 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1023,11 +1023,7 @@ int main(int argc, char **argv, char **envp)
target_cpu_copy_regs(env, regs);
if (gdbstub) {
- if (gdbserver_start(gdbstub) < 0) {
- fprintf(stderr, "qemu: could not open gdbserver on %s\n",
- gdbstub);
- exit(EXIT_FAILURE);
- }
+ gdbserver_start(gdbstub, &error_fatal);
gdb_handlesig(cpu, 0, NULL, NULL, 0);
}
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 80b2e5ff9f..0aa22e1ae2 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -285,7 +285,7 @@ void hmp_gdbserver(Monitor *mon, const QDict *qdict)
device = "tcp::" DEFAULT_GDBSTUB_PORT;
}
- if (gdbserver_start(device) < 0) {
+ if (!gdbserver_start(device, &error_warn)) {
monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
device);
} else if (strcmp(device, "none") == 0) {
diff --git a/system/vl.c b/system/vl.c
index 02795c5135..c567826718 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -811,15 +811,15 @@ static void configure_msg(QemuOpts *opts)
/***********************************************************/
/* USB devices */
-static int usb_parse(const char *cmdline)
+static bool usb_parse(const char *cmdline, Error **errp)
{
g_assert(machine_usb(current_machine));
if (!usbdevice_create(cmdline)) {
- error_report("could not add USB device '%s'", cmdline);
- return -1;
+ error_setg(errp, "could not add USB device '%s'", cmdline);
+ return false;
}
- return 0;
+ return true;
}
/***********************************************************/
@@ -1298,24 +1298,21 @@ static void add_device_config(int type, const char
*cmdline)
* @type: device_config type
* @func: device specific config function, returning pass/fail
*
- * Any failure is fatal and we exit with an error message.
+ * @func is called with the &error_fatal handler so device specific
+ * error messages can be reported on failure.
*/
static void foreach_device_config_or_exit(int type,
- int (*func)(const char *cmdline))
+ bool (*func)(const char *cmdline,
+ Error **errp))
{
struct device_config *conf;
- int rc;
QTAILQ_FOREACH(conf, &device_configs, next) {
if (conf->type != type)
continue;
loc_push_restore(&conf->loc);
- rc = func(conf->cmdline);
+ func(conf->cmdline, &error_fatal);
loc_pop(&conf->loc);
- if (rc) {
- error_setg(&error_fatal, "failed to configure: %s", conf->cmdline);
- exit(1);
- }
}
}
@@ -1446,7 +1443,7 @@ static void qemu_create_default_devices(void)
}
}
-static int serial_parse(const char *devname)
+static bool serial_parse(const char *devname, Error **errp)
{
int index = num_serial_hds;
@@ -1461,13 +1458,13 @@ static int serial_parse(const char *devname)
serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL);
if (!serial_hds[index]) {
- error_report("could not connect serial device"
- " to character backend '%s'", devname);
- return -1;
+ error_setg(errp, "could not connect serial device"
+ " to character backend '%s'", devname);
+ return false;
}
}
num_serial_hds++;
- return 0;
+ return true;
}
Chardev *serial_hd(int i)
@@ -1479,44 +1476,44 @@ Chardev *serial_hd(int i)
return NULL;
}
-static int parallel_parse(const char *devname)
+static bool parallel_parse(const char *devname, Error **errp)
{
static int index = 0;
char label[32];
if (strcmp(devname, "none") == 0)
- return 0;
+ return true;
if (index == MAX_PARALLEL_PORTS) {
- error_report("too many parallel ports");
- exit(1);
+ error_setg(errp, "too many parallel ports");
+ return false;
}
snprintf(label, sizeof(label), "parallel%d", index);
parallel_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL);
if (!parallel_hds[index]) {
- error_report("could not connect parallel device"
- " to character backend '%s'", devname);
- return -1;
+ error_setg(errp, "could not connect parallel device"
+ " to character backend '%s'", devname);
+ return false;
}
index++;
- return 0;
+ return true;
}
-static int debugcon_parse(const char *devname)
+static bool debugcon_parse(const char *devname, Error **errp)
{
QemuOpts *opts;
if (!qemu_chr_new_mux_mon("debugcon", devname, NULL)) {
- error_report("invalid character backend '%s'", devname);
- exit(1);
+ error_setg(errp, "invalid character backend '%s'", devname);
+ return false;
}
opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1, NULL);
if (!opts) {
- error_report("already have a debugcon device");
- exit(1);
+ error_setg(errp, "already have a debugcon device");
+ return false;
}
qemu_opt_set(opts, "driver", "isa-debugcon", &error_abort);
qemu_opt_set(opts, "chardev", "debugcon", &error_abort);
- return 0;
+ return true;
}
static gint machine_class_cmp(gconstpointer a, gconstpointer b)
--
2.39.5
