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. To that end everything that used
foreach_device_config() and failed was also an immediate exit case so
I mechanically tweaked all the helper functions and simplified some.
Currently an RFC, I suspect we want to rename the foreach function to
something more pithy like foreach_device_config_or_exit().
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
Cc: Ilya Leoshkevich <i...@linux.ibm.com>
---
include/exec/gdbstub.h | 8 +++-
gdbstub/system.c | 22 +++++-----
gdbstub/user.c | 20 +++++----
linux-user/main.c | 6 +--
monitor/hmp-cmds.c | 2 +-
system/vl.c | 98 ++++++++++++++++--------------------------
6 files changed, 70 insertions(+), 86 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/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..fb8f6867ea 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,15 +373,15 @@ 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");
- return -1;
+ error_setg(errp, "Failed to bind socket: %s", strerror(errno));
+ return false;
}
qemu_set_cloexec(fd);
@@ -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 0843b7ab49..1d2ca77b7f 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -811,29 +811,13 @@ static void configure_msg(QemuOpts *opts)
/***********************************************************/
/* USB devices */
-static int usb_device_add(const char *devname)
+static bool usb_parse(const char *devname, Error **errp)
{
- USBDevice *dev = NULL;
-
- if (!machine_usb(current_machine)) {
- return -1;
- }
-
- dev = usbdevice_create(devname);
- if (!dev)
- return -1;
-
- return 0;
-}
-
-static int usb_parse(const char *cmdline)
-{
- int r;
- r = usb_device_add(cmdline);
- if (r < 0) {
- error_report("could not add USB device '%s'", cmdline);
+ if (!usbdevice_create(devname)) {
+ error_setg(errp, "could not add USB device '%s'", devname);
+ return false;
}
- return r;
+ return true;
}
/***********************************************************/
@@ -1307,22 +1291,20 @@ static void add_device_config(int type, const char
*cmdline)
QTAILQ_INSERT_TAIL(&device_configs, conf, next);
}
-static int foreach_device_config(int type, int (*func)(const char *cmdline))
+/*
+ * Run through the special device configs, any error will exit via error_fatal.
+ */
+static void foreach_device_config(int type, 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) {
- return rc;
- }
}
- return 0;
}
static void qemu_disable_default_devices(void)
@@ -1452,7 +1434,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;
@@ -1467,13 +1449,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)
@@ -1485,44 +1467,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)
@@ -2044,12 +2026,10 @@ static void qemu_create_late_backends(void)
qemu_opts_foreach(qemu_find_opts("mon"),
mon_init_func, NULL, &error_fatal);
- if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
- exit(1);
- if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
- exit(1);
- if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
- exit(1);
+ /* failures here will exit via error_fatal */
+ foreach_device_config(DEV_SERIAL, serial_parse);
+ foreach_device_config(DEV_PARALLEL, parallel_parse);
+ foreach_device_config(DEV_DEBUGCON, debugcon_parse);
/* now chardevs have been created we may have semihosting to connect */
qemu_semihosting_chardev_init();
@@ -2668,10 +2648,9 @@ static void qemu_create_cli_devices(void)
qemu_opts_foreach(qemu_find_opts("fw_cfg"),
parse_fw_cfg, fw_cfg_find(), &error_fatal);
- /* init USB devices */
+ /* init USB devices, failure will exit */
if (machine_usb(current_machine)) {
- if (foreach_device_config(DEV_USB, usb_parse) < 0)
- exit(1);
+ foreach_device_config(DEV_USB, usb_parse);
}
/* init generic devices */
@@ -2718,10 +2697,9 @@ static bool qemu_machine_creation_done(Error **errp)
exit(1);
}
- if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
- error_setg(errp, "could not start gdbserver");
- return false;
- }
+ /* failure will exit via error_fatal */
+ foreach_device_config(DEV_GDB, gdbserver_start);
+
if (!vga_interface_created && !default_vga &&
vga_interface_type != VGA_NONE) {
warn_report("A -vga option was passed but this machine "
--
2.39.5
