* [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected
@ 2014-03-05 0:38 minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 1/7] qemu-char: Allocate CharDriverState in qemu_chr_new_from_opts minyard
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, hwd, afaerber, mst
This patch set was part of the IPMI patches, but people have asked it to be
separated out because it's useful by itself and the IPMI patches are stuck
in legal limbo.
I have left the patch to move allocating CharDriverState in one place
(patch 1). I didn't get much opinion either way, but I think it is
an improvement and a net reduction in code. I also added a patch (patch 6)
that goes another step there, it makes the error handling in
qmp_chardev_add() consistent and is also a net reduction in code.
The main patches to add the feature are patch 2 and patch 3.
The rest are little possible bug fixes that I noticed while working in the
code.
I went through all the comments and I believe I fixed everything.
Thanks to everyone that reviewed this before.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 1/7] qemu-char: Allocate CharDriverState in qemu_chr_new_from_opts
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
@ 2014-03-05 0:38 ` minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected minyard
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
This allocates the CharDriverState structure and passes it in to the
open routine. This allows a coming option to automatically attempt to
reconnect a chardev if the connection fails. The chardev has to be
kept around so a reconnect can be done on it.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
backends/baum.c | 6 +-
backends/msmouse.c | 5 +-
hw/misc/ivshmem.c | 11 ++-
include/sysemu/char.h | 9 ++-
include/ui/console.h | 4 +-
include/ui/qemu-spice.h | 9 ++-
qemu-char.c | 189 ++++++++++++++++++++++--------------------------
spice-qemu-char.c | 15 ++--
ui/console.c | 10 +--
ui/gtk.c | 5 +-
10 files changed, 122 insertions(+), 141 deletions(-)
diff --git a/backends/baum.c b/backends/baum.c
index 1132899..9910a74 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -561,10 +561,9 @@ static void baum_close(struct CharDriverState *chr)
g_free(baum);
}
-CharDriverState *chr_baum_init(void)
+CharDriverState *chr_baum_init(CharDriverState *chr)
{
BaumDriverState *baum;
- CharDriverState *chr;
brlapi_handle_t *handle;
#ifdef CONFIG_SDL
SDL_SysWMinfo info;
@@ -572,7 +571,7 @@ CharDriverState *chr_baum_init(void)
int tty;
baum = g_malloc0(sizeof(BaumDriverState));
- baum->chr = chr = g_malloc0(sizeof(CharDriverState));
+ baum->chr = chr;
chr->opaque = baum;
chr->chr_write = baum_write;
@@ -618,7 +617,6 @@ fail:
brlapi__closeConnection(handle);
fail_handle:
g_free(handle);
- g_free(chr);
g_free(baum);
return NULL;
}
diff --git a/backends/msmouse.c b/backends/msmouse.c
index c0dbfcd..b4e7a23 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -63,11 +63,8 @@ static void msmouse_chr_close (struct CharDriverState *chr)
g_free (chr);
}
-CharDriverState *qemu_chr_open_msmouse(void)
+CharDriverState *qemu_chr_open_msmouse(CharDriverState *chr)
{
- CharDriverState *chr;
-
- chr = g_malloc0(sizeof(CharDriverState));
chr->chr_write = msmouse_chr_write;
chr->chr_close = msmouse_chr_close;
chr->explicit_be_open = true;
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 8d144ba..75e83c3 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -291,12 +291,17 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier *
{
/* create a event character device based on the passed eventfd */
IVShmemState *s = opaque;
- CharDriverState * chr;
+ CharDriverState *chr, *newchr;
int eventfd = event_notifier_get_fd(n);
- chr = qemu_chr_open_eventfd(eventfd);
-
+ newchr = g_malloc0(sizeof(*chr));
+ if (newchr == NULL) {
+ fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd);
+ exit(-1);
+ }
+ chr = qemu_chr_open_eventfd(newchr, eventfd);
if (chr == NULL) {
+ g_free(newchr);
fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd);
exit(-1);
}
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index b81a6ff..f6844dd 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -292,21 +292,22 @@ CharDriverState *qemu_chr_find(const char *name);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
-void register_char_driver(const char *name, CharDriverState *(*open)(QemuOpts *));
+void register_char_driver(const char *name,
+ CharDriverState *(*open)(CharDriverState *, QemuOpts *));
void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
/* add an eventfd to the qemu devices that are polled */
-CharDriverState *qemu_chr_open_eventfd(int eventfd);
+CharDriverState *qemu_chr_open_eventfd(CharDriverState *chr, int eventfd);
extern int term_escape_char;
CharDriverState *qemu_char_get_next_serial(void);
/* msmouse */
-CharDriverState *qemu_chr_open_msmouse(void);
+CharDriverState *qemu_chr_open_msmouse(CharDriverState *chr);
/* baum.c */
-CharDriverState *chr_baum_init(void);
+CharDriverState *chr_baum_init(CharDriverState *chr);
#endif
diff --git a/include/ui/console.h b/include/ui/console.h
index 4156a87..572a1ff 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -299,9 +299,9 @@ void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
DisplaySurface *qemu_console_surface(QemuConsole *con);
DisplayState *qemu_console_displaystate(QemuConsole *console);
-typedef CharDriverState *(VcHandler)(ChardevVC *vc);
+typedef CharDriverState *(VcHandler)(CharDriverState *chr, ChardevVC *vc);
-CharDriverState *vc_init(ChardevVC *vc);
+CharDriverState *vc_init(CharDriverState *chr, ChardevVC *vc);
void register_vc_handler(VcHandler *handler);
/* sdl.c */
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index a93b4b2..15220a1 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -48,12 +48,15 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
void do_info_spice_print(Monitor *mon, const QObject *data);
void do_info_spice(Monitor *mon, QObject **ret_data);
-CharDriverState *qemu_chr_open_spice_vmc(const char *type);
+CharDriverState *qemu_chr_open_spice_vmc(CharDriverState *chr,
+ const char *type);
#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name);
+CharDriverState *qemu_chr_open_spice_port(CharDriverState *chr,
+ const char *name);
void qemu_spice_register_ports(void);
#else
-static inline CharDriverState *qemu_chr_open_spice_port(const char *name)
+static inline CharDriverState *qemu_chr_open_spice_port(CharDriverState *chr,
+ const char *name)
{ return NULL; }
#endif
diff --git a/qemu-char.c b/qemu-char.c
index 4d50838..fc688cf 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -232,11 +232,8 @@ static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
return len;
}
-static CharDriverState *qemu_chr_open_null(void)
+static CharDriverState *qemu_chr_open_null(CharDriverState *chr)
{
- CharDriverState *chr;
-
- chr = g_malloc0(sizeof(CharDriverState));
chr->chr_write = null_chr_write;
chr->explicit_be_open = true;
return chr;
@@ -519,12 +516,11 @@ static Notifier muxes_realize_notify = {
.notify = muxes_realize_done,
};
-static CharDriverState *qemu_chr_open_mux(CharDriverState *drv)
+static CharDriverState *qemu_chr_open_mux(CharDriverState *chr,
+ CharDriverState *drv)
{
- CharDriverState *chr;
MuxDriver *d;
- chr = g_malloc0(sizeof(CharDriverState));
d = g_malloc0(sizeof(MuxDriver));
chr->opaque = d;
@@ -894,12 +890,11 @@ static void fd_chr_close(struct CharDriverState *chr)
}
/* open a character device to a unix fd */
-static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
+static CharDriverState *qemu_chr_open_fd(CharDriverState *chr,
+ int fd_in, int fd_out)
{
- CharDriverState *chr;
FDCharDriver *s;
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(FDCharDriver));
s->fd_in = io_channel_from_fd(fd_in);
s->fd_out = io_channel_from_fd(fd_out);
@@ -914,7 +909,8 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
return chr;
}
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
+ ChardevHostdev *opts)
{
int fd_in, fd_out;
char filename_in[256], filename_out[256];
@@ -939,7 +935,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
return NULL;
}
}
- return qemu_chr_open_fd(fd_in, fd_out);
+ return qemu_chr_open_fd(chr, fd_in, fd_out);
}
/* init terminal so that we can grab keys */
@@ -980,10 +976,9 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr)
fd_chr_close(chr);
}
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(CharDriverState *chr,
+ ChardevStdio *opts)
{
- CharDriverState *chr;
-
if (is_daemonized()) {
error_report("cannot use stdio with -daemonize");
return NULL;
@@ -993,7 +988,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
fcntl(0, F_SETFL, O_NONBLOCK);
atexit(term_exit);
- chr = qemu_chr_open_fd(0, 1);
+ qemu_chr_open_fd(chr, 0, 1);
chr->chr_close = qemu_chr_close_stdio;
chr->chr_set_echo = qemu_chr_set_echo_stdio;
if (opts->has_signal) {
@@ -1160,10 +1155,10 @@ static void pty_chr_close(struct CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pty(const char *id,
+static CharDriverState *qemu_chr_open_pty(CharDriverState *chr,
+ const char *id,
ChardevReturn *ret)
{
- CharDriverState *chr;
PtyCharDriver *s;
int master_fd, slave_fd;
char pty_name[PATH_MAX];
@@ -1175,8 +1170,6 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
close(slave_fd);
- chr = g_malloc0(sizeof(CharDriverState));
-
chr->filename = g_strdup_printf("pty:%s", pty_name);
ret->pty = g_strdup(pty_name);
ret->has_pty = true;
@@ -1398,12 +1391,10 @@ static void qemu_chr_close_tty(CharDriverState *chr)
}
}
-static CharDriverState *qemu_chr_open_tty_fd(int fd)
+static CharDriverState *qemu_chr_open_tty_fd(CharDriverState *chr, int fd)
{
- CharDriverState *chr;
-
tty_serial_init(fd, 115200, 'N', 8, 1);
- chr = qemu_chr_open_fd(fd, fd);
+ qemu_chr_open_fd(chr, fd, fd);
chr->chr_ioctl = tty_serial_ioctl;
chr->chr_close = qemu_chr_close_tty;
return chr;
@@ -1523,9 +1514,8 @@ static void pp_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(CharDriverState *chr, int fd)
{
- CharDriverState *chr;
ParallelCharDriver *drv;
if (ioctl(fd, PPCLAIM) < 0) {
@@ -1537,7 +1527,6 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
drv->fd = fd;
drv->mode = IEEE1284_MODE_COMPAT;
- chr = g_malloc0(sizeof(CharDriverState));
chr->chr_write = null_chr_write;
chr->chr_ioctl = pp_ioctl;
chr->chr_close = pp_close;
@@ -1588,11 +1577,8 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
return 0;
}
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(CharDriverState *chr, int fd)
{
- CharDriverState *chr;
-
- chr = g_malloc0(sizeof(CharDriverState));
chr->opaque = (void *)(intptr_t)fd;
chr->chr_write = null_chr_write;
chr->chr_ioctl = pp_ioctl;
@@ -1811,12 +1797,11 @@ static int win_chr_poll(void *opaque)
return 0;
}
-static CharDriverState *qemu_chr_open_win_path(const char *filename)
+static CharDriverState *qemu_chr_open_win_path(CharDriverState *chr,
+ const char *filename)
{
- CharDriverState *chr;
WinCharState *s;
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(WinCharState));
chr->opaque = s;
chr->chr_write = win_chr_write;
@@ -1824,7 +1809,6 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename)
if (win_chr_init(chr, filename) < 0) {
g_free(s);
- g_free(chr);
return NULL;
}
return chr;
@@ -1909,13 +1893,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
}
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
+ ChardevHostdev *opts)
{
const char *filename = opts->device;
- CharDriverState *chr;
WinCharState *s;
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(WinCharState));
chr->opaque = s;
chr->chr_write = win_chr_write;
@@ -1923,18 +1906,16 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
if (win_chr_pipe_init(chr, filename) < 0) {
g_free(s);
- g_free(chr);
return NULL;
}
return chr;
}
-static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
+static CharDriverState *qemu_chr_open_win_file(CharDriverState *chr,
+ HANDLE fd_out)
{
- CharDriverState *chr;
WinCharState *s;
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(WinCharState));
s->hcom = fd_out;
chr->opaque = s;
@@ -1942,9 +1923,9 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
return chr;
}
-static CharDriverState *qemu_chr_open_win_con(void)
+static CharDriverState *qemu_chr_open_win_con(CharDriverState *chr)
{
- return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
+ return qemu_chr_open_win_file(chr, GetStdHandle(STD_OUTPUT_HANDLE));
}
static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -2083,14 +2064,13 @@ static void win_stdio_close(CharDriverState *chr)
g_free(chr);
}
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(CharDriverState *chr,
+ ChardevStdio *opts)
{
- CharDriverState *chr;
WinStdioCharState *stdio;
DWORD dwMode;
int is_console = 0;
- chr = g_malloc0(sizeof(CharDriverState));
stdio = g_malloc0(sizeof(WinStdioCharState));
stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE);
@@ -2247,12 +2227,10 @@ static void udp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_udp_fd(int fd)
+static CharDriverState *qemu_chr_open_udp_fd(CharDriverState *chr, int fd)
{
- CharDriverState *chr = NULL;
NetCharDriver *s = NULL;
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(NetCharDriver));
s->fd = fd;
@@ -2268,7 +2246,7 @@ static CharDriverState *qemu_chr_open_udp_fd(int fd)
return chr;
}
-static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_udp(CharDriverState *chr, QemuOpts *opts)
{
Error *local_err = NULL;
int fd = -1;
@@ -2279,7 +2257,7 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
error_free(local_err);
return NULL;
}
- return qemu_chr_open_udp_fd(fd);
+ return qemu_chr_open_udp_fd(chr, fd);
}
/***********************************************************/
@@ -2490,9 +2468,9 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
}
#ifndef _WIN32
-CharDriverState *qemu_chr_open_eventfd(int eventfd)
+CharDriverState *qemu_chr_open_eventfd(CharDriverState *chr, int eventfd)
{
- return qemu_chr_open_fd(eventfd, eventfd);
+ return qemu_chr_open_fd(chr, eventfd, eventfd);
}
#endif
@@ -2607,12 +2585,12 @@ static void tcp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
+static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
+ int fd, bool do_nodelay,
bool is_listen, bool is_telnet,
bool is_waitconnect,
Error **errp)
{
- CharDriverState *chr = NULL;
TCPCharDriver *s = NULL;
char host[NI_MAXHOST], serv[NI_MAXSERV];
const char *left = "", *right = "";
@@ -2625,7 +2603,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
return NULL;
}
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(TCPCharDriver));
s->connected = 0;
@@ -2691,9 +2668,9 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
return chr;
}
-static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
+ QemuOpts *opts)
{
- CharDriverState *chr = NULL;
Error *local_err = NULL;
int fd = -1;
@@ -2723,8 +2700,8 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
if (!is_waitconnect)
qemu_set_nonblock(fd);
- chr = qemu_chr_open_socket_fd(fd, do_nodelay, is_listen, is_telnet,
- is_waitconnect, &local_err);
+ qemu_chr_open_socket_fd(chr, fd, do_nodelay, is_listen, is_telnet,
+ is_waitconnect, &local_err);
if (local_err) {
goto fail;
}
@@ -2741,7 +2718,6 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
}
if (chr) {
g_free(chr->opaque);
- g_free(chr);
}
return NULL;
}
@@ -2803,13 +2779,12 @@ static void ringbuf_chr_close(struct CharDriverState *chr)
chr->opaque = NULL;
}
-static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts,
+static CharDriverState *qemu_chr_open_ringbuf(struct CharDriverState *chr,
+ ChardevRingbuf *opts,
Error **errp)
{
- CharDriverState *chr;
RingBufCharDriver *d;
- chr = g_malloc0(sizeof(CharDriverState));
d = g_malloc(sizeof(*d));
d->size = opts->has_size ? opts->size : 65536;
@@ -2832,7 +2807,6 @@ static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts,
fail:
g_free(d);
- g_free(chr);
return NULL;
}
@@ -3155,7 +3129,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
typedef struct CharDriver {
const char *name;
/* old, pre qapi */
- CharDriverState *(*open)(QemuOpts *opts);
+ CharDriverState *(*open)(CharDriverState *chr, QemuOpts *opts);
/* new, qapi-based */
ChardevBackendKind kind;
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
@@ -3163,7 +3137,8 @@ typedef struct CharDriver {
static GSList *backends;
-void register_char_driver(const char *name, CharDriverState *(*open)(QemuOpts *))
+void register_char_driver(const char *name,
+ CharDriverState *(*open)(CharDriverState*,QemuOpts *))
{
CharDriver *s;
@@ -3268,7 +3243,8 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
return chr;
}
- chr = cd->open(opts);
+ chr = g_malloc0(sizeof(CharDriverState));
+ chr = cd->open(chr, opts);
if (!chr) {
error_setg(errp, "chardev: opening backend \"%s\" failed",
qemu_opt_get(opts, "backend"));
@@ -3291,7 +3267,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
int len = strlen(qemu_opts_id(opts)) + 6;
base->label = g_malloc(len);
snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
- chr = qemu_chr_open_mux(base);
+ chr = qemu_chr_open_mux(chr, base);
chr->filename = base->filename;
chr->avail_connections = MAX_MUX;
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
@@ -3560,7 +3536,8 @@ QemuOptsList qemu_chardev_opts = {
#ifdef _WIN32
-static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+static CharDriverState *qmp_chardev_open_file(CharDriverState *chr,
+ ChardevFile *file, Error **errp)
{
HANDLE out;
@@ -3578,13 +3555,15 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
return qemu_chr_open_win_file(out);
}
-static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
+static CharDriverState *qmp_chardev_open_serial(CharDriverState *chr,
+ ChardevHostdev *serial,
Error **errp)
{
return qemu_chr_open_win_path(serial->device);
}
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(CharDriverState *chr,
+ ChardevHostdev *parallel,
Error **errp)
{
error_setg(errp, "character device backend type 'parallel' not supported");
@@ -3605,7 +3584,8 @@ static int qmp_chardev_open_file_source(char *src, int flags,
return fd;
}
-static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+static CharDriverState *qmp_chardev_open_file(CharDriverState *chr,
+ ChardevFile *file, Error **errp)
{
int flags, in = -1, out = -1;
@@ -3624,10 +3604,11 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
}
}
- return qemu_chr_open_fd(in, out);
+ return qemu_chr_open_fd(chr, in, out);
}
-static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
+static CharDriverState *qmp_chardev_open_serial(CharDriverState *chr,
+ ChardevHostdev *serial,
Error **errp)
{
#ifdef HAVE_CHARDEV_TTY
@@ -3638,14 +3619,15 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
return NULL;
}
qemu_set_nonblock(fd);
- return qemu_chr_open_tty_fd(fd);
+ return qemu_chr_open_tty_fd(chr, fd);
#else
error_setg(errp, "character device backend type 'serial' not supported");
return NULL;
#endif
}
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(CharDriverState *chr,
+ ChardevHostdev *parallel,
Error **errp)
{
#ifdef HAVE_CHARDEV_PARPORT
@@ -3655,7 +3637,7 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
if (error_is_set(errp)) {
return NULL;
}
- return qemu_chr_open_pp_fd(fd);
+ return qemu_chr_open_pp_fd(chr, fd);
#else
error_setg(errp, "character device backend type 'parallel' not supported");
return NULL;
@@ -3664,7 +3646,8 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
#endif /* WIN32 */
-static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
+static CharDriverState *qmp_chardev_open_socket(CharDriverState *chr,
+ ChardevSocket *sock,
Error **errp)
{
SocketAddress *addr = sock->addr;
@@ -3682,11 +3665,12 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
if (error_is_set(errp)) {
return NULL;
}
- return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
+ return qemu_chr_open_socket_fd(chr, fd, do_nodelay, is_listen,
is_telnet, is_waitconnect, errp);
}
-static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
+static CharDriverState *qmp_chardev_open_udp(CharDriverState *chr,
+ ChardevUdp *udp,
Error **errp)
{
int fd;
@@ -3695,14 +3679,14 @@ static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
if (error_is_set(errp)) {
return NULL;
}
- return qemu_chr_open_udp_fd(fd);
+ return qemu_chr_open_udp_fd(chr, fd);
}
ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
Error **errp)
{
ChardevReturn *ret = g_new0(ChardevReturn, 1);
- CharDriverState *base, *chr = NULL;
+ CharDriverState *base, *chr, *newchr;
chr = qemu_chr_find(id);
if (chr) {
@@ -3711,32 +3695,34 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
return NULL;
}
+ newchr = g_malloc0(sizeof(CharDriverState));
+
switch (backend->kind) {
case CHARDEV_BACKEND_KIND_FILE:
- chr = qmp_chardev_open_file(backend->file, errp);
+ chr = qmp_chardev_open_file(newchr, backend->file, errp);
break;
case CHARDEV_BACKEND_KIND_SERIAL:
- chr = qmp_chardev_open_serial(backend->serial, errp);
+ chr = qmp_chardev_open_serial(newchr, backend->serial, errp);
break;
case CHARDEV_BACKEND_KIND_PARALLEL:
- chr = qmp_chardev_open_parallel(backend->parallel, errp);
+ chr = qmp_chardev_open_parallel(newchr, backend->parallel, errp);
break;
case CHARDEV_BACKEND_KIND_PIPE:
- chr = qemu_chr_open_pipe(backend->pipe);
+ chr = qemu_chr_open_pipe(newchr, backend->pipe);
break;
case CHARDEV_BACKEND_KIND_SOCKET:
- chr = qmp_chardev_open_socket(backend->socket, errp);
+ chr = qmp_chardev_open_socket(newchr, backend->socket, errp);
break;
case CHARDEV_BACKEND_KIND_UDP:
- chr = qmp_chardev_open_udp(backend->udp, errp);
+ chr = qmp_chardev_open_udp(newchr, backend->udp, errp);
break;
#ifdef HAVE_CHARDEV_TTY
case CHARDEV_BACKEND_KIND_PTY:
- chr = qemu_chr_open_pty(id, ret);
+ chr = qemu_chr_open_pty(newchr, id, ret);
break;
#endif
case CHARDEV_BACKEND_KIND_NULL:
- chr = qemu_chr_open_null();
+ chr = qemu_chr_open_null(newchr);
break;
case CHARDEV_BACKEND_KIND_MUX:
base = qemu_chr_find(backend->mux->chardev);
@@ -3745,38 +3731,38 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
backend->mux->chardev);
break;
}
- chr = qemu_chr_open_mux(base);
+ chr = qemu_chr_open_mux(newchr, base);
break;
case CHARDEV_BACKEND_KIND_MSMOUSE:
- chr = qemu_chr_open_msmouse();
+ chr = qemu_chr_open_msmouse(newchr);
break;
#ifdef CONFIG_BRLAPI
case CHARDEV_BACKEND_KIND_BRAILLE:
- chr = chr_baum_init();
+ chr = chr_baum_init(newchr);
break;
#endif
case CHARDEV_BACKEND_KIND_STDIO:
- chr = qemu_chr_open_stdio(backend->stdio);
+ chr = qemu_chr_open_stdio(newchr, backend->stdio);
break;
#ifdef _WIN32
case CHARDEV_BACKEND_KIND_CONSOLE:
- chr = qemu_chr_open_win_con();
+ chr = qemu_chr_open_win_con(newchr);
break;
#endif
#ifdef CONFIG_SPICE
case CHARDEV_BACKEND_KIND_SPICEVMC:
- chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
+ chr = qemu_chr_open_spice_vmc(newchr, backend->spicevmc->type);
break;
case CHARDEV_BACKEND_KIND_SPICEPORT:
- chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
+ chr = qemu_chr_open_spice_port(newchr, backend->spiceport->fqdn);
break;
#endif
case CHARDEV_BACKEND_KIND_VC:
- chr = vc_init(backend->vc);
+ chr = vc_init(newchr, backend->vc);
break;
case CHARDEV_BACKEND_KIND_RINGBUF:
case CHARDEV_BACKEND_KIND_MEMORY:
- chr = qemu_chr_open_ringbuf(backend->ringbuf, errp);
+ chr = qemu_chr_open_ringbuf(newchr, backend->ringbuf, errp);
break;
default:
error_setg(errp, "unknown chardev backend (%d)", backend->kind);
@@ -3799,6 +3785,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
return ret;
} else {
+ g_free(newchr);
g_free(ret);
return NULL;
}
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 6624559..c2f5375 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -261,14 +261,11 @@ static void print_allowed_subtypes(void)
fprintf(stderr, "\n");
}
-static CharDriverState *chr_open(const char *subtype,
+static CharDriverState *chr_open(CharDriverState *chr, const char *subtype,
void (*set_fe_open)(struct CharDriverState *, int))
-
{
- CharDriverState *chr;
SpiceCharDriver *s;
- chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(SpiceCharDriver));
s->chr = chr;
s->active = false;
@@ -286,7 +283,7 @@ static CharDriverState *chr_open(const char *subtype,
return chr;
}
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
+CharDriverState *qemu_chr_open_spice_vmc(CharDriverState *chr, const char *type)
{
const char **psubtype = spice_server_char_device_recognized_subtypes();
@@ -306,13 +303,13 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
return NULL;
}
- return chr_open(type, spice_vmc_set_fe_open);
+ return chr_open(chr, type, spice_vmc_set_fe_open);
}
#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
+CharDriverState *qemu_chr_open_spice_port(CharDriverState *chr,
+ const char *name)
{
- CharDriverState *chr;
SpiceCharDriver *s;
if (name == NULL) {
@@ -320,7 +317,7 @@ CharDriverState *qemu_chr_open_spice_port(const char *name)
return NULL;
}
- chr = chr_open("port", spice_port_set_fe_open);
+ chr = chr_open(chr, "port", spice_port_set_fe_open);
s = chr->opaque;
s->sin.portname = g_strdup(name);
diff --git a/ui/console.c b/ui/console.c
index 502e160..0ac45c5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1724,15 +1724,12 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
chr->init(chr);
}
-static CharDriverState *text_console_init(ChardevVC *vc)
+static CharDriverState *text_console_init(CharDriverState *chr, ChardevVC *vc)
{
- CharDriverState *chr;
QemuConsole *s;
unsigned width = 0;
unsigned height = 0;
- chr = g_malloc0(sizeof(CharDriverState));
-
if (vc->has_width) {
width = vc->width;
} else if (vc->has_cols) {
@@ -1754,7 +1751,6 @@ static CharDriverState *text_console_init(ChardevVC *vc)
}
if (!s) {
- g_free(chr);
return NULL;
}
@@ -1774,9 +1770,9 @@ static CharDriverState *text_console_init(ChardevVC *vc)
static VcHandler *vc_handler = text_console_init;
-CharDriverState *vc_init(ChardevVC *vc)
+CharDriverState *vc_init(CharDriverState *chr, ChardevVC *vc)
{
- return vc_handler(vc);
+ return vc_handler(chr, vc);
}
void register_vc_handler(VcHandler *handler)
diff --git a/ui/gtk.c b/ui/gtk.c
index a633d89..0ecc26a 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1125,11 +1125,8 @@ static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
static int nb_vcs;
static CharDriverState *vcs[MAX_VCS];
-static CharDriverState *gd_vc_handler(ChardevVC *unused)
+static CharDriverState *gd_vc_handler(CharDriverState *chr, ChardevVC *unused)
{
- CharDriverState *chr;
-
- chr = g_malloc0(sizeof(*chr));
chr->chr_write = gd_vc_chr_write;
/* defer OPENED events until our vc is fully initialized */
chr->explicit_be_open = true;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 1/7] qemu-char: Allocate CharDriverState in qemu_chr_new_from_opts minyard
@ 2014-03-05 0:38 ` minyard
2014-03-06 6:47 ` Weidong Huang
2014-03-06 7:43 ` Michael S. Tsirkin
2014-03-05 0:38 ` [Qemu-devel] [PATCH 3/7] qemu-char: Wait until socket connect to report connected minyard
` (4 subsequent siblings)
6 siblings, 2 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
Allow a socket that connects to reconnect on a periodic basis if it
fails to connect at startup or if the connection drops while in use.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
include/sysemu/char.h | 16 ++++-
qemu-char.c | 165 +++++++++++++++++++++++++++++++++++++++++++++-----
qemu-options.hx | 11 +++-
3 files changed, 173 insertions(+), 19 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index f6844dd..1800c54 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -53,6 +53,17 @@ typedef struct {
typedef void IOEventHandler(void *opaque, int event);
+struct ReconData {
+ void *opaque;
+ uint64_t recon_time;
+ struct ReconHandlers *handlers;
+ void (*state_change)(void *open_opaque, int is_open);
+ void *state_change_opaque;
+ void (*reconnected)(struct ReconData *r);
+ void (*connection_lost)(struct ReconData *r);
+ void (*shutdown)(struct ReconData *r);
+};
+
struct CharDriverState {
void (*init)(struct CharDriverState *s);
int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
@@ -82,6 +93,8 @@ struct CharDriverState {
guint fd_in_tag;
QemuOpts *opts;
QTAILQ_ENTRY(CharDriverState) next;
+ GSList *backend;
+ struct ReconData *recon;
};
/**
@@ -293,7 +306,8 @@ CharDriverState *qemu_chr_find(const char *name);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
void register_char_driver(const char *name,
- CharDriverState *(*open)(CharDriverState *, QemuOpts *));
+ CharDriverState *(*open)(CharDriverState *, QemuOpts *),
+ int (*recon_setup)(CharDriverState *));
void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
diff --git a/qemu-char.c b/qemu-char.c
index fc688cf..d9838aa 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -88,6 +88,44 @@
/***********************************************************/
/* character device */
+struct GenericReconData {
+ QEMUTimer *recon_timer;
+};
+
+static void generic_recon_timeout(void *opaque)
+{
+ struct ReconData *r = opaque;
+ struct GenericReconData *d = r->opaque;
+
+ timer_mod(d->recon_timer,
+ (get_clock() + (r->recon_time * get_ticks_per_sec())));
+ r->state_change(r->state_change_opaque, 0);
+}
+
+static void generic_reconnected(struct ReconData *r)
+{
+ struct GenericReconData *d = r->opaque;
+
+ timer_del(d->recon_timer);
+}
+
+static void generic_connection_lost(struct ReconData *r)
+{
+ struct GenericReconData *d = r->opaque;
+
+ r->state_change(r->state_change_opaque, 1);
+ timer_mod(d->recon_timer,
+ (get_clock() + (r->recon_time * get_ticks_per_sec())));
+}
+
+static void generic_recon_shutdown(struct ReconData *r)
+{
+ struct GenericReconData *d = r->opaque;
+ timer_free(d->recon_timer);
+ free(d);
+ r->opaque = NULL;
+}
+
static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
QTAILQ_HEAD_INITIALIZER(chardevs);
@@ -96,9 +134,15 @@ void qemu_chr_be_event(CharDriverState *s, int event)
/* Keep track if the char device is open */
switch (event) {
case CHR_EVENT_OPENED:
+ if (s->recon) {
+ s->recon->reconnected(s->recon);
+ }
s->be_open = 1;
break;
case CHR_EVENT_CLOSED:
+ if (s->recon) {
+ s->recon->connection_lost(s->recon);
+ }
s->be_open = 0;
break;
}
@@ -2582,6 +2626,7 @@ static void tcp_chr_close(CharDriverState *chr)
closesocket(s->listen_fd);
}
g_free(s);
+ chr->opaque = NULL;
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
@@ -2641,8 +2686,6 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
chr->get_msgfd = tcp_get_msgfd;
chr->chr_add_client = tcp_chr_add_client;
chr->chr_add_watch = tcp_chr_add_watch;
- /* be isn't opened until we get a connection */
- chr->explicit_be_open = true;
if (is_listen) {
s->listen_fd = fd;
@@ -2680,6 +2723,9 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true);
bool is_unix = qemu_opt_get(opts, "path") != NULL;
+ /* be isn't opened until we get a connection */
+ chr->explicit_be_open = true;
+
if (is_unix) {
if (is_listen) {
fd = unix_listen_opts(opts, &local_err);
@@ -2716,8 +2762,9 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
if (fd >= 0) {
closesocket(fd);
}
- if (chr) {
+ if (chr && chr->opaque) {
g_free(chr->opaque);
+ chr->opaque = NULL;
}
return NULL;
}
@@ -3128,6 +3175,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
typedef struct CharDriver {
const char *name;
+ int (*recon_setup)(CharDriverState *);
/* old, pre qapi */
CharDriverState *(*open)(CharDriverState *chr, QemuOpts *opts);
/* new, qapi-based */
@@ -3138,13 +3186,15 @@ typedef struct CharDriver {
static GSList *backends;
void register_char_driver(const char *name,
- CharDriverState *(*open)(CharDriverState*,QemuOpts *))
+ CharDriverState *(*open)(CharDriverState*,QemuOpts *),
+ int (*recon_setup)(CharDriverState *))
{
CharDriver *s;
s = g_malloc0(sizeof(*s));
s->name = g_strdup(name);
s->open = open;
+ s->recon_setup = recon_setup;
backends = g_slist_append(backends, s);
}
@@ -3162,6 +3212,50 @@ void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
backends = g_slist_append(backends, s);
}
+static void generic_recon_state_change(void *opaque, int is_open)
+{
+ struct CharDriverState *chr = opaque;
+
+ if (!is_open) {
+ struct CharDriver *cd = chr->backend->data;
+
+ if (chr->be_open) {
+ return;
+ }
+
+ cd->open(chr, chr->opts);
+ } else {
+ void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
+
+ if (chr_close) {
+ chr->chr_close = NULL;
+ chr_close(chr);
+ }
+ }
+}
+
+static int generic_recon_setup(struct CharDriverState *chr)
+{
+ struct GenericReconData *d;
+
+ d = g_malloc(sizeof(*d));
+ chr->recon->opaque = d;
+
+ chr->recon->reconnected = generic_reconnected;
+ chr->recon->connection_lost = generic_connection_lost;
+ chr->recon->shutdown = generic_recon_shutdown;
+ chr->recon->state_change = generic_recon_state_change;
+ chr->recon->state_change_opaque = chr;
+
+ d->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS,
+ generic_recon_timeout, chr->recon);
+
+ /* Make sure it connects in time. */
+ timer_mod(d->recon_timer,
+ (get_clock() + (chr->recon->recon_time * get_ticks_per_sec())));
+ return 1;
+}
+
CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
void (*init)(struct CharDriverState *s),
Error **errp)
@@ -3169,6 +3263,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
CharDriver *cd;
CharDriverState *chr;
GSList *i;
+ uint64_t recon_time;
if (qemu_opts_id(opts) == NULL) {
error_setg(errp, "chardev: no id specified");
@@ -3244,11 +3339,41 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
}
chr = g_malloc0(sizeof(CharDriverState));
- chr = cd->open(chr, opts);
- if (!chr) {
- error_setg(errp, "chardev: opening backend \"%s\" failed",
- qemu_opt_get(opts, "backend"));
- goto err;
+
+ chr->backend = i;
+ recon_time = qemu_opt_get_number(opts, "reconnect", 0);
+ if (recon_time) {
+ CharDriver *d = chr->backend->data;
+ if (!d->recon_setup) {
+ g_free(chr);
+ fprintf(stderr, "chardev: reconnect not supported on %s\n",
+ qemu_opt_get(opts, "backend"));
+ return NULL;
+ }
+ if (qemu_opt_get_bool(opts, "server", 0)) {
+ g_free(chr);
+ fprintf(stderr, "chardev: server device cannot reconnect\n");
+ return NULL;
+ }
+ chr->opts = opts;
+ chr->recon = g_malloc(sizeof(*chr->recon));
+ chr->recon->recon_time = recon_time;
+ if (!d->recon_setup(chr)) {
+ g_free(chr->recon);
+ g_free(chr);
+ fprintf(stderr, "chardev: Unable to set up reconnect\n");
+ return NULL;
+ }
+ }
+
+ if (!cd->open(chr, opts)) {
+ if (!chr->recon) {
+ /* Reconnect is not enabled, give up */
+ fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
+ qemu_opt_get(opts, "backend"));
+ g_free(chr);
+ return NULL;
+ }
}
if (!chr->filename)
@@ -3267,7 +3392,8 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
int len = strlen(qemu_opts_id(opts)) + 6;
base->label = g_malloc(len);
snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
- chr = qemu_chr_open_mux(chr, base);
+ chr = g_malloc0(sizeof(CharDriverState));
+ qemu_chr_open_mux(chr, base);
chr->filename = base->filename;
chr->avail_connections = MAX_MUX;
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
@@ -3275,7 +3401,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
chr->avail_connections = 1;
}
chr->label = g_strdup(qemu_opts_id(opts));
- chr->opts = opts;
return chr;
err:
@@ -3378,9 +3503,16 @@ void qemu_chr_fe_release(CharDriverState *s)
void qemu_chr_delete(CharDriverState *chr)
{
+ void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
+
QTAILQ_REMOVE(&chardevs, chr, next);
- if (chr->chr_close) {
- chr->chr_close(chr);
+ if (chr_close) {
+ chr->chr_close = NULL;
+ chr_close(chr);
+ }
+ if (chr->recon) {
+ chr->recon->shutdown(chr->recon);
+ g_free(chr->recon);
}
g_free(chr->filename);
g_free(chr->label);
@@ -3515,6 +3647,9 @@ QemuOptsList qemu_chardev_opts = {
.name = "mux",
.type = QEMU_OPT_BOOL,
},{
+ .name = "reconnect",
+ .type = QEMU_OPT_NUMBER,
+ },{
.name = "signal",
.type = QEMU_OPT_BOOL,
},{
@@ -3811,8 +3946,8 @@ void qmp_chardev_remove(const char *id, Error **errp)
static void register_types(void)
{
register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
- register_char_driver("socket", qemu_chr_open_socket);
- register_char_driver("udp", qemu_chr_open_udp);
+ register_char_driver("socket", qemu_chr_open_socket, generic_recon_setup);
+ register_char_driver("udp", qemu_chr_open_udp, NULL);
register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
qemu_chr_parse_ringbuf);
register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
diff --git a/qemu-options.hx b/qemu-options.hx
index 56e5fdf..1002a3d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1787,8 +1787,9 @@ ETEXI
DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
"-chardev null,id=id[,mux=on|off]\n"
"-chardev socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay]\n"
- " [,server][,nowait][,telnet][,mux=on|off] (tcp)\n"
- "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
+ " [,server][,nowait][,telnet][,mux=on|off][,reconnect=seconds] (tcp)\n"
+ "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,mux=on|off]\n"
+ " [,reconnect=seconds] (unix)\n"
"-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
" [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
"-chardev msmouse,id=id[,mux=on|off]\n"
@@ -1860,7 +1861,7 @@ Options to each backend are described below.
A void device. This device will not emit any data, and will drop any data it
receives. The null backend does not take any options.
-@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
+@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,reconnect=@var{seconds}]
Create a two-way stream socket, which can be either a TCP or a unix socket. A
unix socket will be created if @option{path} is specified. Behaviour is
@@ -1874,6 +1875,10 @@ connect to a listening socket.
@option{telnet} specifies that traffic on the socket should interpret telnet
escape sequences.
+@option{reconnect} specifies that if the client socket does not connect at
+startup, or if the client socket is closed for some reason (like the other
+end exited), wait the given number of seconds and attempt to reconnect.
+
TCP and unix socket options are given below:
@table @option
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 3/7] qemu-char: Wait until socket connect to report connected
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 1/7] qemu-char: Allocate CharDriverState in qemu_chr_new_from_opts minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected minyard
@ 2014-03-05 0:38 ` minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 4/7] qemu-char: remove free of chr from win_stdio_close minyard
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
The socket code was reporting that a socket connection was connected
as soon as the connect call was issued. If the connection failed, it
would then report it was not connected. With the reconnect code, it's
better to wait until the connection is actually operational before
reporting that the socket is connected.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 9 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index d9838aa..6d6dd36 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2310,9 +2310,10 @@ static CharDriverState *qemu_chr_open_udp(CharDriverState *chr, QemuOpts *opts)
typedef struct {
GIOChannel *chan, *listen_chan;
+ int waitsrc;
guint listen_tag;
int fd, listen_fd;
- int connected;
+ enum { TCP_NOT_CONNECTED, TCP_WAITING_CONNECT, TCP_CONNECTED } state;
int max_size;
int do_telnetopt;
int do_nodelay;
@@ -2325,7 +2326,7 @@ static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque
static int tcp_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
TCPCharDriver *s = chr->opaque;
- if (s->connected) {
+ if (s->state == TCP_CONNECTED) {
return io_channel_send(s->chan, buf, len);
} else {
/* XXX: indicate an error ? */
@@ -2337,7 +2338,7 @@ static int tcp_chr_read_poll(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr->opaque;
- if (!s->connected)
+ if (s->state != TCP_CONNECTED)
return 0;
s->max_size = qemu_chr_be_can_write(chr);
return s->max_size;
@@ -2482,7 +2483,7 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
uint8_t buf[READ_BUF_LEN];
int len, size;
- if (!s->connected || s->max_size <= 0) {
+ if (s->state != TCP_CONNECTED || s->max_size <= 0) {
return TRUE;
}
len = sizeof(buf);
@@ -2491,7 +2492,7 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
size = tcp_chr_recv(chr, (void *)buf, len);
if (size == 0) {
/* connection closed */
- s->connected = 0;
+ s->state = TCP_NOT_CONNECTED;
if (s->listen_chan) {
s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr);
}
@@ -2518,17 +2519,48 @@ CharDriverState *qemu_chr_open_eventfd(CharDriverState *chr, int eventfd)
}
#endif
-static void tcp_chr_connect(void *opaque)
+static gboolean tcp_wait_connect(GIOChannel *source,
+ GIOCondition condition,
+ gpointer opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr->opaque;
- s->connected = 1;
+ if (s->state != TCP_WAITING_CONNECT) {
+ return FALSE;
+ }
+ if (condition & G_IO_ERR || condition & G_IO_HUP) {
+ /* The connected failed */
+ s->state = TCP_NOT_CONNECTED;
+ g_io_channel_unref(s->chan);
+ s->chan = NULL;
+ closesocket(s->fd);
+ s->fd = -1;
+ return FALSE;
+ }
+
+ s->state = TCP_CONNECTED;
if (s->chan) {
chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
tcp_chr_read, chr);
}
qemu_chr_be_generic_open(chr);
+ return FALSE;
+}
+
+static void tcp_chr_connect(void *opaque)
+{
+ CharDriverState *chr = opaque;
+ TCPCharDriver *s = chr->opaque;
+
+ s->state = TCP_WAITING_CONNECT;
+ if (s->chan) {
+ /* Wait until write becomes ready before reporting connected. */
+ s->waitsrc = g_io_add_watch(s->chan,
+ G_IO_OUT | G_IO_HUP | G_IO_ERR,
+ tcp_wait_connect,
+ chr);
+ }
}
#define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
@@ -2650,7 +2682,7 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
s = g_malloc0(sizeof(TCPCharDriver));
- s->connected = 0;
+ s->state = TCP_NOT_CONNECTED;
s->fd = -1;
s->listen_fd = -1;
s->msgfd = -1;
@@ -2695,7 +2727,6 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
s->do_telnetopt = 1;
}
} else {
- s->connected = 1;
s->fd = fd;
socket_set_nodelay(fd);
s->chan = io_channel_from_socket(s->fd);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 4/7] qemu-char: remove free of chr from win_stdio_close
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
` (2 preceding siblings ...)
2014-03-05 0:38 ` [Qemu-devel] [PATCH 3/7] qemu-char: Wait until socket connect to report connected minyard
@ 2014-03-05 0:38 ` minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 5/7] qemu-char: Close fd at end of file minyard
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
This will result in a double free on close, because it's freed
in qemu_chr_delete() right after calling the close function.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/qemu-char.c b/qemu-char.c
index 6d6dd36..4ac131d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2105,7 +2105,6 @@ static void win_stdio_close(CharDriverState *chr)
}
g_free(chr->opaque);
- g_free(chr);
}
static CharDriverState *qemu_chr_open_stdio(CharDriverState *chr,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 5/7] qemu-char: Close fd at end of file
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
` (3 preceding siblings ...)
2014-03-05 0:38 ` [Qemu-devel] [PATCH 4/7] qemu-char: remove free of chr from win_stdio_close minyard
@ 2014-03-05 0:38 ` minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 6/7] qemu-char: Clean up error handling in qmp_chardev_add minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 7/7] console: Don't use the console if NULL minyard
6 siblings, 0 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
The chardev backends that used qemu_chr_open_fd did not get their
file descriptors closed at end of file or when the chardev was closed.
This could result in a file descriptor leak.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
qemu-char.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 4ac131d..427bb34 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -851,6 +851,8 @@ typedef struct FDCharDriver {
GIOChannel *fd_in, *fd_out;
int max_size;
QTAILQ_ENTRY(FDCharDriver) node;
+ int close_fdin;
+ int close_fdout;
} FDCharDriver;
static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -860,6 +862,18 @@ static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
return io_channel_send(s->fd_out, buf, len);
}
+static void fd_close_fds(FDCharDriver *s)
+{
+ if ((s->close_fdin != s->close_fdout) && (s->close_fdout != -1)) {
+ close(s->close_fdout);
+ }
+ s->close_fdout = -1;
+ if (s->close_fdin != -1) {
+ close(s->close_fdin);
+ }
+ s->close_fdin = -1;
+}
+
static gboolean fd_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
@@ -881,6 +895,7 @@ static gboolean fd_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
len, &bytes_read, NULL);
if (status == G_IO_STATUS_EOF) {
remove_fd_in_watch(chr);
+ fd_close_fds(s);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
return FALSE;
}
@@ -929,19 +944,27 @@ static void fd_chr_close(struct CharDriverState *chr)
g_io_channel_unref(s->fd_out);
}
+ fd_close_fds(s);
g_free(s);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
/* open a character device to a unix fd */
static CharDriverState *qemu_chr_open_fd(CharDriverState *chr,
- int fd_in, int fd_out)
+ int fd_in, int fd_out,
+ int close_fds_on_close)
{
FDCharDriver *s;
s = g_malloc0(sizeof(FDCharDriver));
s->fd_in = io_channel_from_fd(fd_in);
s->fd_out = io_channel_from_fd(fd_out);
+ if (close_fds_on_close) {
+ s->close_fdin = fd_in;
+ s->close_fdout = fd_out;
+ } else {
+ s->close_fdin = s->close_fdout = -1;
+ }
fcntl(fd_out, F_SETFL, O_NONBLOCK);
s->chr = chr;
chr->opaque = s;
@@ -979,7 +1002,7 @@ static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
return NULL;
}
}
- return qemu_chr_open_fd(chr, fd_in, fd_out);
+ return qemu_chr_open_fd(chr, fd_in, fd_out, TRUE);
}
/* init terminal so that we can grab keys */
@@ -1032,7 +1055,7 @@ static CharDriverState *qemu_chr_open_stdio(CharDriverState *chr,
fcntl(0, F_SETFL, O_NONBLOCK);
atexit(term_exit);
- qemu_chr_open_fd(chr, 0, 1);
+ qemu_chr_open_fd(chr, 0, 1, FALSE);
chr->chr_close = qemu_chr_close_stdio;
chr->chr_set_echo = qemu_chr_set_echo_stdio;
if (opts->has_signal) {
@@ -1438,7 +1461,7 @@ static void qemu_chr_close_tty(CharDriverState *chr)
static CharDriverState *qemu_chr_open_tty_fd(CharDriverState *chr, int fd)
{
tty_serial_init(fd, 115200, 'N', 8, 1);
- qemu_chr_open_fd(chr, fd, fd);
+ qemu_chr_open_fd(chr, fd, fd, TRUE);
chr->chr_ioctl = tty_serial_ioctl;
chr->chr_close = qemu_chr_close_tty;
return chr;
@@ -2514,7 +2537,7 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
#ifndef _WIN32
CharDriverState *qemu_chr_open_eventfd(CharDriverState *chr, int eventfd)
{
- return qemu_chr_open_fd(chr, eventfd, eventfd);
+ return qemu_chr_open_fd(chr, eventfd, eventfd, FALSE);
}
#endif
@@ -3769,7 +3792,7 @@ static CharDriverState *qmp_chardev_open_file(CharDriverState *chr,
}
}
- return qemu_chr_open_fd(chr, in, out);
+ return qemu_chr_open_fd(chr, in, out, TRUE);
}
static CharDriverState *qmp_chardev_open_serial(CharDriverState *chr,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 6/7] qemu-char: Clean up error handling in qmp_chardev_add
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
` (4 preceding siblings ...)
2014-03-05 0:38 ` [Qemu-devel] [PATCH 5/7] qemu-char: Close fd at end of file minyard
@ 2014-03-05 0:38 ` minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 7/7] console: Don't use the console if NULL minyard
6 siblings, 0 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
The open functions called by qmp_chardev_add could either return NULL
or set errp to return an error. Modify to just return errp, as that can
pass useful information where returning NULL cannot.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
backends/baum.c | 6 +-
backends/msmouse.c | 4 +-
hw/misc/ivshmem.c | 11 +-
include/sysemu/char.h | 10 +-
include/ui/console.h | 4 +-
include/ui/qemu-spice.h | 15 ++-
qemu-char.c | 336 ++++++++++++++++++++++--------------------------
spice-qemu-char.c | 15 ++-
ui/console.c | 10 +-
ui/gtk.c | 4 +-
10 files changed, 190 insertions(+), 225 deletions(-)
diff --git a/backends/baum.c b/backends/baum.c
index 9910a74..56cd00f 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -561,7 +561,7 @@ static void baum_close(struct CharDriverState *chr)
g_free(baum);
}
-CharDriverState *chr_baum_init(CharDriverState *chr)
+void chr_baum_init(CharDriverState *chr, Error **errp)
{
BaumDriverState *baum;
brlapi_handle_t *handle;
@@ -610,15 +610,15 @@ CharDriverState *chr_baum_init(CharDriverState *chr)
qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
- return chr;
+ return;
fail:
timer_free(baum->cellCount_timer);
brlapi__closeConnection(handle);
fail_handle:
+ error_setg(errp, "Failed to initialize baum");
g_free(handle);
g_free(baum);
- return NULL;
}
static void register_types(void)
diff --git a/backends/msmouse.c b/backends/msmouse.c
index b4e7a23..78998f9 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -63,15 +63,13 @@ static void msmouse_chr_close (struct CharDriverState *chr)
g_free (chr);
}
-CharDriverState *qemu_chr_open_msmouse(CharDriverState *chr)
+void qemu_chr_open_msmouse(CharDriverState *chr, Error **errp)
{
chr->chr_write = msmouse_chr_write;
chr->chr_close = msmouse_chr_close;
chr->explicit_be_open = true;
qemu_add_mouse_event_handler(msmouse_event, chr, 0, "QEMU Microsoft Mouse");
-
- return chr;
}
static void register_types(void)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 75e83c3..5813f79 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -291,20 +291,15 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier *
{
/* create a event character device based on the passed eventfd */
IVShmemState *s = opaque;
- CharDriverState *chr, *newchr;
+ CharDriverState *chr;
int eventfd = event_notifier_get_fd(n);
- newchr = g_malloc0(sizeof(*chr));
- if (newchr == NULL) {
- fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd);
- exit(-1);
- }
- chr = qemu_chr_open_eventfd(newchr, eventfd);
+ chr = g_malloc0(sizeof(*chr));
if (chr == NULL) {
- g_free(newchr);
fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd);
exit(-1);
}
+ qemu_chr_open_eventfd(chr, eventfd);
qemu_chr_fe_claim_no_fail(chr);
/* if MSI is supported we need multiple interrupts */
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 1800c54..8604041 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -306,22 +306,22 @@ CharDriverState *qemu_chr_find(const char *name);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
void register_char_driver(const char *name,
- CharDriverState *(*open)(CharDriverState *, QemuOpts *),
- int (*recon_setup)(CharDriverState *));
+ void (*open)(CharDriverState *, QemuOpts *, Error **),
+ int (*recon_setup)(CharDriverState *));
void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
/* add an eventfd to the qemu devices that are polled */
-CharDriverState *qemu_chr_open_eventfd(CharDriverState *chr, int eventfd);
+void qemu_chr_open_eventfd(CharDriverState *chr, int eventfd);
extern int term_escape_char;
CharDriverState *qemu_char_get_next_serial(void);
/* msmouse */
-CharDriverState *qemu_chr_open_msmouse(CharDriverState *chr);
+void qemu_chr_open_msmouse(CharDriverState *chr, Error **errp);
/* baum.c */
-CharDriverState *chr_baum_init(CharDriverState *chr);
+void chr_baum_init(CharDriverState *chr, Error **errp);
#endif
diff --git a/include/ui/console.h b/include/ui/console.h
index 572a1ff..18776d6 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -299,9 +299,9 @@ void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
DisplaySurface *qemu_console_surface(QemuConsole *con);
DisplayState *qemu_console_displaystate(QemuConsole *console);
-typedef CharDriverState *(VcHandler)(CharDriverState *chr, ChardevVC *vc);
+typedef void (VcHandler)(CharDriverState *chr, ChardevVC *vc, Error **errp);
-CharDriverState *vc_init(CharDriverState *chr, ChardevVC *vc);
+void vc_init(CharDriverState *chr, ChardevVC *vc, Error **errp);
void register_vc_handler(VcHandler *handler);
/* sdl.c */
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 15220a1..85b1be3 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -48,16 +48,17 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
void do_info_spice_print(Monitor *mon, const QObject *data);
void do_info_spice(Monitor *mon, QObject **ret_data);
-CharDriverState *qemu_chr_open_spice_vmc(CharDriverState *chr,
- const char *type);
+void qemu_chr_open_spice_vmc(CharDriverState *chr, const char *type,
+ Error **errp);
#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(CharDriverState *chr,
- const char *name);
+void qemu_chr_open_spice_port(CharDriverState *chr, const char *name,
+ Error **errp);
void qemu_spice_register_ports(void);
#else
-static inline CharDriverState *qemu_chr_open_spice_port(CharDriverState *chr,
- const char *name)
-{ return NULL; }
+static inline void qemu_chr_open_spice_port(CharDriverState *chr,
+ const char *name,
+ Error **errp)
+{ error_setg(errp, "open spice port not supported"); }
#endif
#else /* CONFIG_SPICE */
diff --git a/qemu-char.c b/qemu-char.c
index 427bb34..ae63836 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -560,8 +560,7 @@ static Notifier muxes_realize_notify = {
.notify = muxes_realize_done,
};
-static CharDriverState *qemu_chr_open_mux(CharDriverState *chr,
- CharDriverState *drv)
+static void qemu_chr_open_mux(CharDriverState *chr, CharDriverState *drv)
{
MuxDriver *d;
@@ -580,8 +579,6 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState *chr,
*/
chr->explicit_be_open = muxes_realized ? 0 : 1;
chr->is_mux = 1;
-
- return chr;
}
@@ -950,9 +947,8 @@ static void fd_chr_close(struct CharDriverState *chr)
}
/* open a character device to a unix fd */
-static CharDriverState *qemu_chr_open_fd(CharDriverState *chr,
- int fd_in, int fd_out,
- int close_fds_on_close)
+static void qemu_chr_open_fd(CharDriverState *chr, int fd_in, int fd_out,
+ int close_fds_on_close)
{
FDCharDriver *s;
@@ -972,20 +968,18 @@ static CharDriverState *qemu_chr_open_fd(CharDriverState *chr,
chr->chr_write = fd_chr_write;
chr->chr_update_read_handler = fd_chr_update_read_handler;
chr->chr_close = fd_chr_close;
-
- return chr;
}
-static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
- ChardevHostdev *opts)
+static void qemu_chr_open_pipe(CharDriverState *chr, ChardevHostdev *opts,
+ Error **errp)
{
int fd_in, fd_out;
char filename_in[256], filename_out[256];
const char *filename = opts->device;
if (filename == NULL) {
- fprintf(stderr, "chardev: pipe: no filename given\n");
- return NULL;
+ error_setg(errp, "chardev: pipe: no filename given");
+ return;
}
snprintf(filename_in, 256, "%s.in", filename);
@@ -999,10 +993,11 @@ static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
close(fd_out);
TFR(fd_in = fd_out = qemu_open(filename, O_RDWR | O_BINARY));
if (fd_in < 0) {
- return NULL;
+ error_setg_file_open(errp, errno, filename);
+ return;
}
}
- return qemu_chr_open_fd(chr, fd_in, fd_out, TRUE);
+ qemu_chr_open_fd(chr, fd_in, fd_out, TRUE);
}
/* init terminal so that we can grab keys */
@@ -1043,12 +1038,12 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr)
fd_chr_close(chr);
}
-static CharDriverState *qemu_chr_open_stdio(CharDriverState *chr,
- ChardevStdio *opts)
+static void qemu_chr_open_stdio(CharDriverState *chr, ChardevStdio *opts,
+ Error **errp)
{
if (is_daemonized()) {
- error_report("cannot use stdio with -daemonize");
- return NULL;
+ error_setg(errp, "cannot use stdio with -daemonize");
+ return;
}
old_fd0_flags = fcntl(0, F_GETFL);
tcgetattr (0, &oldtty);
@@ -1062,8 +1057,6 @@ static CharDriverState *qemu_chr_open_stdio(CharDriverState *chr,
stdio_allow_signal = opts->signal;
}
qemu_chr_fe_set_echo(chr, false);
-
- return chr;
}
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
@@ -1222,9 +1215,8 @@ static void pty_chr_close(struct CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pty(CharDriverState *chr,
- const char *id,
- ChardevReturn *ret)
+static void qemu_chr_open_pty(CharDriverState *chr, const char *id,
+ ChardevReturn *ret, Error **errp)
{
PtyCharDriver *s;
int master_fd, slave_fd;
@@ -1232,7 +1224,8 @@ static CharDriverState *qemu_chr_open_pty(CharDriverState *chr,
master_fd = qemu_openpty_raw(&slave_fd, pty_name);
if (master_fd < 0) {
- return NULL;
+ error_setg_errno(errp, errno, "getsockname");
+ return;
}
close(slave_fd);
@@ -1254,8 +1247,6 @@ static CharDriverState *qemu_chr_open_pty(CharDriverState *chr,
s->fd = io_channel_from_fd(master_fd);
s->timer_tag = 0;
-
- return chr;
}
static void tty_serial_init(int fd, int speed,
@@ -1581,13 +1572,14 @@ static void pp_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pp_fd(CharDriverState *chr, int fd)
+static void qemu_chr_open_pp_fd(CharDriverState *chr, int fd, Error **errp)
{
ParallelCharDriver *drv;
if (ioctl(fd, PPCLAIM) < 0) {
+ error_setg_errno(errp, errno, "ioctl");
close(fd);
- return NULL;
+ return;
}
drv = g_malloc0(sizeof(ParallelCharDriver));
@@ -1598,8 +1590,6 @@ static CharDriverState *qemu_chr_open_pp_fd(CharDriverState *chr, int fd)
chr->chr_ioctl = pp_ioctl;
chr->chr_close = pp_close;
chr->opaque = drv;
-
- return chr;
}
#endif /* __linux__ */
@@ -1644,13 +1634,12 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
return 0;
}
-static CharDriverState *qemu_chr_open_pp_fd(CharDriverState *chr, int fd)
+static void qemu_chr_open_pp_fd(CharDriverState *chr, int fd, Error **errp)
{
chr->opaque = (void *)(intptr_t)fd;
chr->chr_write = null_chr_write;
chr->chr_ioctl = pp_ioctl;
chr->explicit_be_open = true;
- return chr;
}
#endif
@@ -1704,7 +1693,8 @@ static void win_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static int win_chr_init(CharDriverState *chr, const char *filename)
+static int win_chr_init(CharDriverState *chr, const char *filename,
+ Error **errp)
{
WinCharState *s = chr->opaque;
COMMCONFIG comcfg;
@@ -1715,25 +1705,25 @@ static int win_chr_init(CharDriverState *chr, const char *filename)
s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hsend) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
goto fail;
}
s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hrecv) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
goto fail;
}
s->hcom = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, 0, NULL,
OPEN_EXISTING, FILE_FLAG_OVERLAPPED, 0);
if (s->hcom == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "Failed CreateFile (%lu)\n", GetLastError());
+ error_setg(errp, "Failed CreateFile (%lu)", GetLastError());
s->hcom = NULL;
goto fail;
}
if (!SetupComm(s->hcom, NRECVBUF, NSENDBUF)) {
- fprintf(stderr, "Failed SetupComm\n");
+ error_setg(errp, "Failed SetupComm");
goto fail;
}
@@ -1744,23 +1734,23 @@ static int win_chr_init(CharDriverState *chr, const char *filename)
CommConfigDialog(filename, NULL, &comcfg);
if (!SetCommState(s->hcom, &comcfg.dcb)) {
- fprintf(stderr, "Failed SetCommState\n");
+ error_setg(errp, "Failed SetCommState");
goto fail;
}
if (!SetCommMask(s->hcom, EV_ERR)) {
- fprintf(stderr, "Failed SetCommMask\n");
+ error_setg(errp, "Failed SetCommMask");
goto fail;
}
cto.ReadIntervalTimeout = MAXDWORD;
if (!SetCommTimeouts(s->hcom, &cto)) {
- fprintf(stderr, "Failed SetCommTimeouts\n");
+ error_setg(errp, "Failed SetCommTimeouts");
goto fail;
}
if (!ClearCommError(s->hcom, &err, &comstat)) {
- fprintf(stderr, "Failed ClearCommError\n");
+ error_setg(errp, "Failed ClearCommError");
goto fail;
}
qemu_add_polling_cb(win_chr_poll, chr);
@@ -1864,8 +1854,8 @@ static int win_chr_poll(void *opaque)
return 0;
}
-static CharDriverState *qemu_chr_open_win_path(CharDriverState *chr,
- const char *filename)
+static void qemu_chr_open_win_path(CharDriverState *chr, const char *filename,
+ Error **errp)
{
WinCharState *s;
@@ -1874,11 +1864,10 @@ static CharDriverState *qemu_chr_open_win_path(CharDriverState *chr,
chr->chr_write = win_chr_write;
chr->chr_close = win_chr_close;
- if (win_chr_init(chr, filename) < 0) {
+ if (win_chr_init(chr, filename, errp) < 0) {
g_free(s);
- return NULL;
+ return;
}
- return chr;
}
static int win_chr_pipe_poll(void *opaque)
@@ -1897,7 +1886,8 @@ static int win_chr_pipe_poll(void *opaque)
return 0;
}
-static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
+static int win_chr_pipe_init(CharDriverState *chr, const char *filename,
+ Error **errp)
{
WinCharState *s = chr->opaque;
OVERLAPPED ov;
@@ -1909,12 +1899,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hsend) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
goto fail;
}
s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hrecv) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
goto fail;
}
@@ -1924,7 +1914,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
PIPE_WAIT,
MAXCONNECT, NSENDBUF, NRECVBUF, NTIMEOUT, NULL);
if (s->hcom == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "Failed CreateNamedPipe (%lu)\n", GetLastError());
+ error_setg(errp, "Failed CreateNamedPipe (%lu)", GetLastError());
s->hcom = NULL;
goto fail;
}
@@ -1933,13 +1923,13 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
ov.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
ret = ConnectNamedPipe(s->hcom, &ov);
if (ret) {
- fprintf(stderr, "Failed ConnectNamedPipe\n");
+ error_setg(errp, "Failed ConnectNamedPipe");
goto fail;
}
ret = GetOverlappedResult(s->hcom, &ov, &size, TRUE);
if (!ret) {
- fprintf(stderr, "Failed GetOverlappedResult\n");
+ error_setg(errp, "Failed GetOverlappedResult");
if (ov.hEvent) {
CloseHandle(ov.hEvent);
ov.hEvent = NULL;
@@ -1960,8 +1950,8 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
}
-static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
- ChardevHostdev *opts)
+static void qemu_chr_open_pipe(CharDriverState *chr, ChardevHostdev *opts,
+ Error **errp)
{
const char *filename = opts->device;
WinCharState *s;
@@ -1971,15 +1961,13 @@ static CharDriverState *qemu_chr_open_pipe(CharDriverState *chr,
chr->chr_write = win_chr_write;
chr->chr_close = win_chr_close;
- if (win_chr_pipe_init(chr, filename) < 0) {
+ if (win_chr_pipe_init(chr, filename, errp) < 0) {
g_free(s);
- return NULL;
+ return;
}
- return chr;
}
-static CharDriverState *qemu_chr_open_win_file(CharDriverState *chr,
- HANDLE fd_out)
+static voidqemu_chr_open_win_file(CharDriverState *chr, HANDLE fd_out)
{
WinCharState *s;
@@ -1987,12 +1975,11 @@ static CharDriverState *qemu_chr_open_win_file(CharDriverState *chr,
s->hcom = fd_out;
chr->opaque = s;
chr->chr_write = win_chr_write;
- return chr;
}
-static CharDriverState *qemu_chr_open_win_con(CharDriverState *chr)
+static void qemu_chr_open_win_con(CharDriverState *chr)
{
- return qemu_chr_open_win_file(chr, GetStdHandle(STD_OUTPUT_HANDLE));
+ qemu_chr_open_win_file(chr, GetStdHandle(STD_OUTPUT_HANDLE));
}
static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -2293,7 +2280,7 @@ static void udp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_udp_fd(CharDriverState *chr, int fd)
+static void qemu_chr_open_udp_fd(CharDriverState *chr, int fd, Error **errp)
{
NetCharDriver *s = NULL;
@@ -2309,21 +2296,18 @@ static CharDriverState *qemu_chr_open_udp_fd(CharDriverState *chr, int fd)
chr->chr_close = udp_chr_close;
/* be isn't opened until we get a connection */
chr->explicit_be_open = true;
- return chr;
}
-static CharDriverState *qemu_chr_open_udp(CharDriverState *chr, QemuOpts *opts)
+static void qemu_chr_open_udp(CharDriverState *chr, QemuOpts *opts,
+ Error **errp)
{
- Error *local_err = NULL;
int fd = -1;
- fd = inet_dgram_opts(opts, &local_err);
+ fd = inet_dgram_opts(opts, errp);
if (fd < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
- return NULL;
+ return;
}
- return qemu_chr_open_udp_fd(chr, fd);
+ qemu_chr_open_udp_fd(chr, fd, errp);
}
/***********************************************************/
@@ -2535,9 +2519,9 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
}
#ifndef _WIN32
-CharDriverState *qemu_chr_open_eventfd(CharDriverState *chr, int eventfd)
+void qemu_chr_open_eventfd(CharDriverState *chr, int eventfd)
{
- return qemu_chr_open_fd(chr, eventfd, eventfd, FALSE);
+ qemu_chr_open_fd(chr, eventfd, eventfd, FALSE);
}
#endif
@@ -2684,11 +2668,10 @@ static void tcp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
- int fd, bool do_nodelay,
- bool is_listen, bool is_telnet,
- bool is_waitconnect,
- Error **errp)
+static void qemu_chr_open_socket_fd(CharDriverState *chr,
+ int fd, bool do_nodelay,
+ bool is_listen, bool is_telnet,
+ bool is_waitconnect, Error **errp)
{
TCPCharDriver *s = NULL;
char host[NI_MAXHOST], serv[NI_MAXSERV];
@@ -2699,7 +2682,7 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
memset(&ss, 0, ss_len);
if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
error_setg_errno(errp, errno, "getsockname");
- return NULL;
+ return;
}
s = g_malloc0(sizeof(TCPCharDriver));
@@ -2761,13 +2744,11 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
qemu_set_nonblock(s->listen_fd);
}
- return chr;
}
-static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
- QemuOpts *opts)
+static void qemu_chr_open_socket(CharDriverState *chr,
+ QemuOpts *opts, Error **errp)
{
- Error *local_err = NULL;
int fd = -1;
bool is_listen = qemu_opt_get_bool(opts, "server", false);
@@ -2781,15 +2762,15 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
if (is_unix) {
if (is_listen) {
- fd = unix_listen_opts(opts, &local_err);
+ fd = unix_listen_opts(opts, errp);
} else {
- fd = unix_connect_opts(opts, &local_err, NULL, NULL);
+ fd = unix_connect_opts(opts, errp, NULL, NULL);
}
} else {
if (is_listen) {
- fd = inet_listen_opts(opts, 0, &local_err);
+ fd = inet_listen_opts(opts, 0, errp);
} else {
- fd = inet_connect_opts(opts, &local_err, NULL, NULL);
+ fd = inet_connect_opts(opts, errp, NULL, NULL);
}
}
if (fd < 0) {
@@ -2800,18 +2781,12 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
qemu_set_nonblock(fd);
qemu_chr_open_socket_fd(chr, fd, do_nodelay, is_listen, is_telnet,
- is_waitconnect, &local_err);
- if (local_err) {
- goto fail;
+ is_waitconnect, errp);
+ if (!error_is_set(errp)) {
+ return;
}
- return chr;
-
fail:
- if (local_err) {
- qerror_report_err(local_err);
- error_free(local_err);
- }
if (fd >= 0) {
closesocket(fd);
}
@@ -2819,7 +2794,6 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
g_free(chr->opaque);
chr->opaque = NULL;
}
- return NULL;
}
/*********************************************************/
@@ -2879,9 +2853,8 @@ static void ringbuf_chr_close(struct CharDriverState *chr)
chr->opaque = NULL;
}
-static CharDriverState *qemu_chr_open_ringbuf(struct CharDriverState *chr,
- ChardevRingbuf *opts,
- Error **errp)
+static void qemu_chr_open_ringbuf(struct CharDriverState *chr,
+ ChardevRingbuf *opts, Error **errp)
{
RingBufCharDriver *d;
@@ -2892,7 +2865,8 @@ static CharDriverState *qemu_chr_open_ringbuf(struct CharDriverState *chr,
/* The size must be power of 2 */
if (d->size & (d->size - 1)) {
error_setg(errp, "size of ringbuf chardev must be power of two");
- goto fail;
+ g_free(d);
+ return;
}
d->prod = 0;
@@ -2902,12 +2876,6 @@ static CharDriverState *qemu_chr_open_ringbuf(struct CharDriverState *chr,
chr->opaque = d;
chr->chr_write = ringbuf_chr_write;
chr->chr_close = ringbuf_chr_close;
-
- return chr;
-
-fail:
- g_free(d);
- return NULL;
}
static bool chr_is_ringbuf(const CharDriverState *chr)
@@ -3230,7 +3198,7 @@ typedef struct CharDriver {
const char *name;
int (*recon_setup)(CharDriverState *);
/* old, pre qapi */
- CharDriverState *(*open)(CharDriverState *chr, QemuOpts *opts);
+ void (*open)(CharDriverState *chr, QemuOpts *opts, Error **errp);
/* new, qapi-based */
ChardevBackendKind kind;
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
@@ -3239,7 +3207,7 @@ typedef struct CharDriver {
static GSList *backends;
void register_char_driver(const char *name,
- CharDriverState *(*open)(CharDriverState*,QemuOpts *),
+ void (*open)(CharDriverState *, QemuOpts *, Error **),
int (*recon_setup)(CharDriverState *))
{
CharDriver *s;
@@ -3271,12 +3239,20 @@ static void generic_recon_state_change(void *opaque, int is_open)
if (!is_open) {
struct CharDriver *cd = chr->backend->data;
+ Error *local_err = NULL;
if (chr->be_open) {
return;
}
- cd->open(chr, chr->opts);
+ cd->open(chr, chr->opts, &local_err);
+ if (error_is_set(&local_err)) {
+ fprintf(stderr, "chardev: opening backend \"%s\" failed: %s."
+ " Will retry in %llu seconds\n",
+ chr->label, error_get_pretty(local_err),
+ (unsigned long long) chr->recon->recon_time);
+ error_free(local_err);
+ }
} else {
void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
@@ -3313,6 +3289,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
void (*init)(struct CharDriverState *s),
Error **errp)
{
+ Error *local_err = NULL;
CharDriver *cd;
CharDriverState *chr;
GSList *i;
@@ -3419,14 +3396,21 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
}
}
- if (!cd->open(chr, opts)) {
- if (!chr->recon) {
+ cd->open(chr, opts, &local_err);
+ if (error_is_set(&local_err)) {
+ if (chr->recon) {
+ fprintf(stderr, "chardev: opening backend \"%s\" failed: %s."
+ " Will retry in %llu seconds\n",
+ qemu_opts_id(opts), error_get_pretty(local_err),
+ (unsigned long long) chr->recon->recon_time);
+ } else {
/* Reconnect is not enabled, give up */
- fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
- qemu_opt_get(opts, "backend"));
+ error_setg(errp, "chardev: opening backend \"%s\" failed: %s",
+ qemu_opts_id(opts), error_get_pretty(local_err));
g_free(chr);
return NULL;
}
+ error_free(local_err);
}
if (!chr->filename)
@@ -3724,38 +3708,35 @@ QemuOptsList qemu_chardev_opts = {
#ifdef _WIN32
-static CharDriverState *qmp_chardev_open_file(CharDriverState *chr,
- ChardevFile *file, Error **errp)
+static void qmp_chardev_open_file(CharDriverState *chr,
+ ChardevFile *file, Error **errp)
{
HANDLE out;
if (file->has_in) {
error_setg(errp, "input file not supported");
- return NULL;
+ return;
}
out = CreateFile(file->out, GENERIC_WRITE, FILE_SHARE_READ, NULL,
OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if (out == INVALID_HANDLE_VALUE) {
error_setg(errp, "open %s failed", file->out);
- return NULL;
+ return;
}
- return qemu_chr_open_win_file(out);
+ qemu_chr_open_win_file(out);
}
-static CharDriverState *qmp_chardev_open_serial(CharDriverState *chr,
- ChardevHostdev *serial,
- Error **errp)
+static void qmp_chardev_open_serial(CharDriverState *chr,
+ ChardevHostdev *serial, Error **errp)
{
- return qemu_chr_open_win_path(serial->device);
+ qemu_chr_open_win_path(serial->device, errp);
}
-static CharDriverState *qmp_chardev_open_parallel(CharDriverState *chr,
- ChardevHostdev *parallel,
- Error **errp)
+static void qmp_chardev_open_parallel(CharDriverState *chr,
+ ChardevHostdev *parallel, Error **errp)
{
error_setg(errp, "character device backend type 'parallel' not supported");
- return NULL;
}
#else /* WIN32 */
@@ -3772,15 +3753,15 @@ static int qmp_chardev_open_file_source(char *src, int flags,
return fd;
}
-static CharDriverState *qmp_chardev_open_file(CharDriverState *chr,
- ChardevFile *file, Error **errp)
+static void qmp_chardev_open_file(CharDriverState *chr,
+ ChardevFile *file, Error **errp)
{
int flags, in = -1, out = -1;
flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
out = qmp_chardev_open_file_source(file->out, flags, errp);
if (error_is_set(errp)) {
- return NULL;
+ return;
}
if (file->has_in) {
@@ -3788,55 +3769,50 @@ static CharDriverState *qmp_chardev_open_file(CharDriverState *chr,
in = qmp_chardev_open_file_source(file->in, flags, errp);
if (error_is_set(errp)) {
qemu_close(out);
- return NULL;
+ return;
}
}
- return qemu_chr_open_fd(chr, in, out, TRUE);
+ qemu_chr_open_fd(chr, in, out, TRUE);
}
-static CharDriverState *qmp_chardev_open_serial(CharDriverState *chr,
- ChardevHostdev *serial,
- Error **errp)
+static void qmp_chardev_open_serial(CharDriverState *chr,
+ ChardevHostdev *serial, Error **errp)
{
#ifdef HAVE_CHARDEV_TTY
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
if (error_is_set(errp)) {
- return NULL;
+ return;
}
qemu_set_nonblock(fd);
- return qemu_chr_open_tty_fd(chr, fd);
+ qemu_chr_open_tty_fd(chr, fd);
#else
error_setg(errp, "character device backend type 'serial' not supported");
- return NULL;
#endif
}
-static CharDriverState *qmp_chardev_open_parallel(CharDriverState *chr,
- ChardevHostdev *parallel,
- Error **errp)
+static void qmp_chardev_open_parallel(CharDriverState *chr,
+ ChardevHostdev *parallel, Error **errp)
{
#ifdef HAVE_CHARDEV_PARPORT
int fd;
fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
if (error_is_set(errp)) {
- return NULL;
+ return;
}
- return qemu_chr_open_pp_fd(chr, fd);
+ qemu_chr_open_pp_fd(chr, fd, errp);
#else
error_setg(errp, "character device backend type 'parallel' not supported");
- return NULL;
#endif
}
#endif /* WIN32 */
-static CharDriverState *qmp_chardev_open_socket(CharDriverState *chr,
- ChardevSocket *sock,
- Error **errp)
+static void qmp_chardev_open_socket(CharDriverState *chr,
+ ChardevSocket *sock, Error **errp)
{
SocketAddress *addr = sock->addr;
bool do_nodelay = sock->has_nodelay ? sock->nodelay : false;
@@ -3851,30 +3827,29 @@ static CharDriverState *qmp_chardev_open_socket(CharDriverState *chr,
fd = socket_connect(addr, errp, NULL, NULL);
}
if (error_is_set(errp)) {
- return NULL;
+ return;
}
- return qemu_chr_open_socket_fd(chr, fd, do_nodelay, is_listen,
- is_telnet, is_waitconnect, errp);
+ qemu_chr_open_socket_fd(chr, fd, do_nodelay, is_listen,
+ is_telnet, is_waitconnect, errp);
}
-static CharDriverState *qmp_chardev_open_udp(CharDriverState *chr,
- ChardevUdp *udp,
- Error **errp)
+static void qmp_chardev_open_udp(CharDriverState *chr,
+ ChardevUdp *udp, Error **errp)
{
int fd;
fd = socket_dgram(udp->remote, udp->local, errp);
if (error_is_set(errp)) {
- return NULL;
+ return;
}
- return qemu_chr_open_udp_fd(chr, fd);
+ qemu_chr_open_udp_fd(chr, fd, errp);
}
ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
Error **errp)
{
ChardevReturn *ret = g_new0(ChardevReturn, 1);
- CharDriverState *base, *chr, *newchr;
+ CharDriverState *base, *chr;
chr = qemu_chr_find(id);
if (chr) {
@@ -3883,34 +3858,34 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
return NULL;
}
- newchr = g_malloc0(sizeof(CharDriverState));
+ chr = g_malloc0(sizeof(CharDriverState));
switch (backend->kind) {
case CHARDEV_BACKEND_KIND_FILE:
- chr = qmp_chardev_open_file(newchr, backend->file, errp);
+ qmp_chardev_open_file(chr, backend->file, errp);
break;
case CHARDEV_BACKEND_KIND_SERIAL:
- chr = qmp_chardev_open_serial(newchr, backend->serial, errp);
+ qmp_chardev_open_serial(chr, backend->serial, errp);
break;
case CHARDEV_BACKEND_KIND_PARALLEL:
- chr = qmp_chardev_open_parallel(newchr, backend->parallel, errp);
+ qmp_chardev_open_parallel(chr, backend->parallel, errp);
break;
case CHARDEV_BACKEND_KIND_PIPE:
- chr = qemu_chr_open_pipe(newchr, backend->pipe);
+ qemu_chr_open_pipe(chr, backend->pipe, errp);
break;
case CHARDEV_BACKEND_KIND_SOCKET:
- chr = qmp_chardev_open_socket(newchr, backend->socket, errp);
+ qmp_chardev_open_socket(chr, backend->socket, errp);
break;
case CHARDEV_BACKEND_KIND_UDP:
- chr = qmp_chardev_open_udp(newchr, backend->udp, errp);
+ qmp_chardev_open_udp(chr, backend->udp, errp);
break;
#ifdef HAVE_CHARDEV_TTY
case CHARDEV_BACKEND_KIND_PTY:
- chr = qemu_chr_open_pty(newchr, id, ret);
+ qemu_chr_open_pty(chr, id, ret, errp);
break;
#endif
case CHARDEV_BACKEND_KIND_NULL:
- chr = qemu_chr_open_null(newchr);
+ chr = qemu_chr_open_null(chr);
break;
case CHARDEV_BACKEND_KIND_MUX:
base = qemu_chr_find(backend->mux->chardev);
@@ -3919,48 +3894,45 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
backend->mux->chardev);
break;
}
- chr = qemu_chr_open_mux(newchr, base);
+ qemu_chr_open_mux(chr, base);
break;
case CHARDEV_BACKEND_KIND_MSMOUSE:
- chr = qemu_chr_open_msmouse(newchr);
+ qemu_chr_open_msmouse(chr, errp);
break;
#ifdef CONFIG_BRLAPI
case CHARDEV_BACKEND_KIND_BRAILLE:
- chr = chr_baum_init(newchr);
+ chr_baum_init(chr, errp);
break;
#endif
case CHARDEV_BACKEND_KIND_STDIO:
- chr = qemu_chr_open_stdio(newchr, backend->stdio);
+ qemu_chr_open_stdio(chr, backend->stdio, errp);
break;
#ifdef _WIN32
case CHARDEV_BACKEND_KIND_CONSOLE:
- chr = qemu_chr_open_win_con(newchr);
+ qemu_chr_open_win_con(chr);
break;
#endif
#ifdef CONFIG_SPICE
case CHARDEV_BACKEND_KIND_SPICEVMC:
- chr = qemu_chr_open_spice_vmc(newchr, backend->spicevmc->type);
+ qemu_chr_open_spice_vmc(chr, backend->spicevmc->type, errp);
break;
case CHARDEV_BACKEND_KIND_SPICEPORT:
- chr = qemu_chr_open_spice_port(newchr, backend->spiceport->fqdn);
+ qemu_chr_open_spice_port(chr, backend->spiceport->fqdn, errp);
break;
#endif
case CHARDEV_BACKEND_KIND_VC:
- chr = vc_init(newchr, backend->vc);
+ vc_init(chr, backend->vc, errp);
break;
case CHARDEV_BACKEND_KIND_RINGBUF:
case CHARDEV_BACKEND_KIND_MEMORY:
- chr = qemu_chr_open_ringbuf(newchr, backend->ringbuf, errp);
+ qemu_chr_open_ringbuf(chr, backend->ringbuf, errp);
break;
default:
error_setg(errp, "unknown chardev backend (%d)", backend->kind);
break;
}
- if (chr == NULL && !error_is_set(errp)) {
- error_setg(errp, "Failed to create chardev");
- }
- if (chr) {
+ if (!error_is_set(errp)) {
chr->label = g_strdup(id);
chr->avail_connections =
(backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
@@ -3971,12 +3943,12 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
qemu_chr_be_event(chr, CHR_EVENT_OPENED);
}
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
- return ret;
} else {
- g_free(newchr);
+ g_free(chr);
g_free(ret);
- return NULL;
+ ret = NULL;
}
+ return ret;
}
void qmp_chardev_remove(const char *id, Error **errp)
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index c2f5375..8d999f1 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -261,8 +261,8 @@ static void print_allowed_subtypes(void)
fprintf(stderr, "\n");
}
-static CharDriverState *chr_open(CharDriverState *chr, const char *subtype,
- void (*set_fe_open)(struct CharDriverState *, int))
+static void chr_open(CharDriverState *chr, const char *subtype,
+ void (*set_fe_open)(struct CharDriverState *, int))
{
SpiceCharDriver *s;
@@ -279,18 +279,18 @@ static CharDriverState *chr_open(CharDriverState *chr, const char *subtype,
chr->chr_fe_event = spice_chr_fe_event;
QLIST_INSERT_HEAD(&spice_chars, s, next);
-
- return chr;
}
-CharDriverState *qemu_chr_open_spice_vmc(CharDriverState *chr, const char *type)
+void qemu_chr_open_spice_vmc(CharDriverState *chr, const char *type,
+ Error *errp)
{
const char **psubtype = spice_server_char_device_recognized_subtypes();
if (type == NULL) {
fprintf(stderr, "spice-qemu-char: missing name parameter\n");
print_allowed_subtypes();
- return NULL;
+ error_setg(errp, "open spice vmc failed");
+ return;
}
for (; *psubtype != NULL; ++psubtype) {
if (strcmp(type, *psubtype) == 0) {
@@ -300,7 +300,8 @@ CharDriverState *qemu_chr_open_spice_vmc(CharDriverState *chr, const char *type)
if (*psubtype == NULL) {
fprintf(stderr, "spice-qemu-char: unsupported type: %s\n", type);
print_allowed_subtypes();
- return NULL;
+ error_setg(errp, "open spice vmc failed");
+ return;
}
return chr_open(chr, type, spice_vmc_set_fe_open);
diff --git a/ui/console.c b/ui/console.c
index 0ac45c5..a204ce2 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1724,7 +1724,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
chr->init(chr);
}
-static CharDriverState *text_console_init(CharDriverState *chr, ChardevVC *vc)
+static void text_console_init(CharDriverState *chr, ChardevVC *vc, Error **errp)
{
QemuConsole *s;
unsigned width = 0;
@@ -1751,7 +1751,8 @@ static CharDriverState *text_console_init(CharDriverState *chr, ChardevVC *vc)
}
if (!s) {
- return NULL;
+ error_setg(errp, "Unable to create a new vc console");
+ return;
}
s->chr = chr;
@@ -1765,14 +1766,13 @@ static CharDriverState *text_console_init(CharDriverState *chr, ChardevVC *vc)
if (display_state) {
text_console_do_init(chr, display_state);
}
- return chr;
}
static VcHandler *vc_handler = text_console_init;
-CharDriverState *vc_init(CharDriverState *chr, ChardevVC *vc)
+void vc_init(CharDriverState *chr, ChardevVC *vc, Error **errp)
{
- return vc_handler(chr, vc);
+ vc_handler(chr, vc, errp);
}
void register_vc_handler(VcHandler *handler)
diff --git a/ui/gtk.c b/ui/gtk.c
index 0ecc26a..a970860 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1125,15 +1125,13 @@ static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
static int nb_vcs;
static CharDriverState *vcs[MAX_VCS];
-static CharDriverState *gd_vc_handler(CharDriverState *chr, ChardevVC *unused)
+static void gd_vc_handler(CharDriverState *chr, ChardevVC *unused, Error **errp)
{
chr->chr_write = gd_vc_chr_write;
/* defer OPENED events until our vc is fully initialized */
chr->explicit_be_open = true;
vcs[nb_vcs++] = chr;
-
- return chr;
}
void early_gtk_display_init(void)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 7/7] console: Don't use the console if NULL
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
` (5 preceding siblings ...)
2014-03-05 0:38 ` [Qemu-devel] [PATCH 6/7] qemu-char: Clean up error handling in qmp_chardev_add minyard
@ 2014-03-05 0:38 ` minyard
6 siblings, 0 replies; 14+ messages in thread
From: minyard @ 2014-03-05 0:38 UTC (permalink / raw)
To: qemu-devel; +Cc: bcketchum, Corey Minyard, hwd, afaerber, mst
From: Corey Minyard <cminyard@mvista.com>
Two spots used an allocated console, even though new_console could
return NULL. Check the return value first.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
ui/console.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/ui/console.c b/ui/console.c
index a204ce2..8d4ca81 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1582,6 +1582,9 @@ QemuConsole *graphic_console_init(DeviceState *dev,
ds = get_alloc_displaystate();
trace_console_gfx_new();
s = new_console(ds, GRAPHIC_CONSOLE);
+ if (!s) {
+ return NULL;
+ }
s->hw_ops = hw_ops;
s->hw = opaque;
if (dev) {
@@ -1747,7 +1750,9 @@ static void text_console_init(CharDriverState *chr, ChardevVC *vc, Error **errp)
s = new_console(NULL, TEXT_CONSOLE);
} else {
s = new_console(NULL, TEXT_CONSOLE_FIXED_SIZE);
- s->surface = qemu_create_displaysurface(width, height);
+ if (s) {
+ s->surface = qemu_create_displaysurface(width, height);
+ }
}
if (!s) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-05 0:38 ` [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected minyard
@ 2014-03-06 6:47 ` Weidong Huang
2014-03-06 17:18 ` Corey Minyard
2014-03-06 7:43 ` Michael S. Tsirkin
1 sibling, 1 reply; 14+ messages in thread
From: Weidong Huang @ 2014-03-06 6:47 UTC (permalink / raw)
To: minyard; +Cc: bcketchum, Corey Minyard, qemu-devel, afaerber, mst
On 2014/3/5 8:38, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Allow a socket that connects to reconnect on a periodic basis if it
> fails to connect at startup or if the connection drops while in use.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> include/sysemu/char.h | 16 ++++-
> qemu-char.c | 165 +++++++++++++++++++++++++++++++++++++++++++++-----
> qemu-options.hx | 11 +++-
> 3 files changed, 173 insertions(+), 19 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index f6844dd..1800c54 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -53,6 +53,17 @@ typedef struct {
>
> typedef void IOEventHandler(void *opaque, int event);
>
> +struct ReconData {
> + void *opaque;
> + uint64_t recon_time;
> + struct ReconHandlers *handlers;
> + void (*state_change)(void *open_opaque, int is_open);
> + void *state_change_opaque;
> + void (*reconnected)(struct ReconData *r);
> + void (*connection_lost)(struct ReconData *r);
> + void (*shutdown)(struct ReconData *r);
> +};
> +
> struct CharDriverState {
> void (*init)(struct CharDriverState *s);
> int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
> @@ -82,6 +93,8 @@ struct CharDriverState {
> guint fd_in_tag;
> QemuOpts *opts;
> QTAILQ_ENTRY(CharDriverState) next;
> + GSList *backend;
> + struct ReconData *recon;
> };
>
> /**
> @@ -293,7 +306,8 @@ CharDriverState *qemu_chr_find(const char *name);
> QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
>
> void register_char_driver(const char *name,
> - CharDriverState *(*open)(CharDriverState *, QemuOpts *));
> + CharDriverState *(*open)(CharDriverState *, QemuOpts *),
> + int (*recon_setup)(CharDriverState *));
> void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
> void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
>
> diff --git a/qemu-char.c b/qemu-char.c
> index fc688cf..d9838aa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -88,6 +88,44 @@
> /***********************************************************/
> /* character device */
>
> +struct GenericReconData {
> + QEMUTimer *recon_timer;
> +};
> +
> +static void generic_recon_timeout(void *opaque)
> +{
> + struct ReconData *r = opaque;
> + struct GenericReconData *d = r->opaque;
> +
> + timer_mod(d->recon_timer,
> + (get_clock() + (r->recon_time * get_ticks_per_sec())));
> + r->state_change(r->state_change_opaque, 0);
> +}
> +
> +static void generic_reconnected(struct ReconData *r)
> +{
> + struct GenericReconData *d = r->opaque;
> +
> + timer_del(d->recon_timer);
> +}
> +
> +static void generic_connection_lost(struct ReconData *r)
> +{
> + struct GenericReconData *d = r->opaque;
> +
> + r->state_change(r->state_change_opaque, 1);
> + timer_mod(d->recon_timer,
> + (get_clock() + (r->recon_time * get_ticks_per_sec())));
> +}
> +
> +static void generic_recon_shutdown(struct ReconData *r)
> +{
> + struct GenericReconData *d = r->opaque;
> + timer_free(d->recon_timer);
> + free(d);
> + r->opaque = NULL;
> +}
> +
> static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
> QTAILQ_HEAD_INITIALIZER(chardevs);
>
> @@ -96,9 +134,15 @@ void qemu_chr_be_event(CharDriverState *s, int event)
> /* Keep track if the char device is open */
> switch (event) {
> case CHR_EVENT_OPENED:
> + if (s->recon) {
> + s->recon->reconnected(s->recon);
> + }
> s->be_open = 1;
> break;
> case CHR_EVENT_CLOSED:
> + if (s->recon) {
> + s->recon->connection_lost(s->recon);
> + }
> s->be_open = 0;
> break;
> }
> @@ -2582,6 +2626,7 @@ static void tcp_chr_close(CharDriverState *chr)
> closesocket(s->listen_fd);
> }
> g_free(s);
> + chr->opaque = NULL;
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
>
> @@ -2641,8 +2686,6 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
> chr->get_msgfd = tcp_get_msgfd;
> chr->chr_add_client = tcp_chr_add_client;
> chr->chr_add_watch = tcp_chr_add_watch;
> - /* be isn't opened until we get a connection */
> - chr->explicit_be_open = true;
>
> if (is_listen) {
> s->listen_fd = fd;
> @@ -2680,6 +2723,9 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
> bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true);
> bool is_unix = qemu_opt_get(opts, "path") != NULL;
>
> + /* be isn't opened until we get a connection */
> + chr->explicit_be_open = true;
> +
> if (is_unix) {
> if (is_listen) {
> fd = unix_listen_opts(opts, &local_err);
> @@ -2716,8 +2762,9 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
> if (fd >= 0) {
> closesocket(fd);
> }
> - if (chr) {
> + if (chr && chr->opaque) {
> g_free(chr->opaque);
> + chr->opaque = NULL;
> }
> return NULL;
> }
> @@ -3128,6 +3175,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
>
> typedef struct CharDriver {
> const char *name;
> + int (*recon_setup)(CharDriverState *);
> /* old, pre qapi */
> CharDriverState *(*open)(CharDriverState *chr, QemuOpts *opts);
> /* new, qapi-based */
> @@ -3138,13 +3186,15 @@ typedef struct CharDriver {
> static GSList *backends;
>
> void register_char_driver(const char *name,
> - CharDriverState *(*open)(CharDriverState*,QemuOpts *))
> + CharDriverState *(*open)(CharDriverState*,QemuOpts *),
> + int (*recon_setup)(CharDriverState *))
> {
> CharDriver *s;
>
> s = g_malloc0(sizeof(*s));
> s->name = g_strdup(name);
> s->open = open;
> + s->recon_setup = recon_setup;
>
> backends = g_slist_append(backends, s);
> }
> @@ -3162,6 +3212,50 @@ void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
> backends = g_slist_append(backends, s);
> }
>
> +static void generic_recon_state_change(void *opaque, int is_open)
> +{
> + struct CharDriverState *chr = opaque;
> +
> + if (!is_open) {
> + struct CharDriver *cd = chr->backend->data;
> +
> + if (chr->be_open) {
> + return;
> + }
> +
> + cd->open(chr, chr->opts);
> + } else {
> + void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
> +
> + if (chr_close) {
> + chr->chr_close = NULL;
> + chr_close(chr);
> + }
> + }
> +}
> +
> +static int generic_recon_setup(struct CharDriverState *chr)
> +{
> + struct GenericReconData *d;
> +
> + d = g_malloc(sizeof(*d));
> + chr->recon->opaque = d;
> +
> + chr->recon->reconnected = generic_reconnected;
> + chr->recon->connection_lost = generic_connection_lost;
> + chr->recon->shutdown = generic_recon_shutdown;
> + chr->recon->state_change = generic_recon_state_change;
> + chr->recon->state_change_opaque = chr;
> +
> + d->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS,
> + generic_recon_timeout, chr->recon);
> +
> + /* Make sure it connects in time. */
> + timer_mod(d->recon_timer,
> + (get_clock() + (chr->recon->recon_time * get_ticks_per_sec())));
> + return 1;
> +}
> +
> CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> void (*init)(struct CharDriverState *s),
> Error **errp)
> @@ -3169,6 +3263,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> CharDriver *cd;
> CharDriverState *chr;
> GSList *i;
> + uint64_t recon_time;
>
> if (qemu_opts_id(opts) == NULL) {
> error_setg(errp, "chardev: no id specified");
> @@ -3244,11 +3339,41 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> }
>
> chr = g_malloc0(sizeof(CharDriverState));
> - chr = cd->open(chr, opts);
> - if (!chr) {
> - error_setg(errp, "chardev: opening backend \"%s\" failed",
> - qemu_opt_get(opts, "backend"));
> - goto err;
> +
> + chr->backend = i;
> + recon_time = qemu_opt_get_number(opts, "reconnect", 0);
> + if (recon_time) {
> + CharDriver *d = chr->backend->data;
> + if (!d->recon_setup) {
> + g_free(chr);
> + fprintf(stderr, "chardev: reconnect not supported on %s\n",
> + qemu_opt_get(opts, "backend"));
> + return NULL;
> + }
> + if (qemu_opt_get_bool(opts, "server", 0)) {
> + g_free(chr);
> + fprintf(stderr, "chardev: server device cannot reconnect\n");
> + return NULL;
> + }
> + chr->opts = opts;
> + chr->recon = g_malloc(sizeof(*chr->recon));
> + chr->recon->recon_time = recon_time;
> + if (!d->recon_setup(chr)) {
> + g_free(chr->recon);
> + g_free(chr);
> + fprintf(stderr, "chardev: Unable to set up reconnect\n");
> + return NULL;
> + }
> + }
> +
> + if (!cd->open(chr, opts)) {
> + if (!chr->recon) {
> + /* Reconnect is not enabled, give up */
> + fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
> + qemu_opt_get(opts, "backend"));
> + g_free(chr);
> + return NULL;
> + }
> }
>
> if (!chr->filename)
> @@ -3267,7 +3392,8 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> int len = strlen(qemu_opts_id(opts)) + 6;
> base->label = g_malloc(len);
> snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
> - chr = qemu_chr_open_mux(chr, base);
> + chr = g_malloc0(sizeof(CharDriverState));
> + qemu_chr_open_mux(chr, base);
> chr->filename = base->filename;
> chr->avail_connections = MAX_MUX;
> QTAILQ_INSERT_TAIL(&chardevs, chr, next);
> @@ -3275,7 +3401,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> chr->avail_connections = 1;
> }
> chr->label = g_strdup(qemu_opts_id(opts));
> - chr->opts = opts;
> return chr;
>
> err:
> @@ -3378,9 +3503,16 @@ void qemu_chr_fe_release(CharDriverState *s)
>
> void qemu_chr_delete(CharDriverState *chr)
> {
> + void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
> +
> QTAILQ_REMOVE(&chardevs, chr, next);
> - if (chr->chr_close) {
> - chr->chr_close(chr);
> + if (chr_close) {
> + chr->chr_close = NULL;
> + chr_close(chr);
> + }
> + if (chr->recon) {
> + chr->recon->shutdown(chr->recon);
> + g_free(chr->recon);
> }
> g_free(chr->filename);
> g_free(chr->label);
> @@ -3515,6 +3647,9 @@ QemuOptsList qemu_chardev_opts = {
> .name = "mux",
> .type = QEMU_OPT_BOOL,
> },{
> + .name = "reconnect",
> + .type = QEMU_OPT_NUMBER,
> + },{
> .name = "signal",
> .type = QEMU_OPT_BOOL,
> },{
> @@ -3811,8 +3946,8 @@ void qmp_chardev_remove(const char *id, Error **errp)
> static void register_types(void)
> {
> register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
> - register_char_driver("socket", qemu_chr_open_socket);
> - register_char_driver("udp", qemu_chr_open_udp);
> + register_char_driver("socket", qemu_chr_open_socket, generic_recon_setup);
> + register_char_driver("udp", qemu_chr_open_udp, NULL);
> register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
> qemu_chr_parse_ringbuf);
> register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 56e5fdf..1002a3d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1787,8 +1787,9 @@ ETEXI
> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev null,id=id[,mux=on|off]\n"
> "-chardev socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay]\n"
> - " [,server][,nowait][,telnet][,mux=on|off] (tcp)\n"
> - "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
> + " [,server][,nowait][,telnet][,mux=on|off][,reconnect=seconds] (tcp)\n"
> + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,mux=on|off]\n"
> + " [,reconnect=seconds] (unix)\n"
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
> "-chardev msmouse,id=id[,mux=on|off]\n"
> @@ -1860,7 +1861,7 @@ Options to each backend are described below.
> A void device. This device will not emit any data, and will drop any data it
> receives. The null backend does not take any options.
>
> -@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
> +@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,reconnect=@var{seconds}]
>
> Create a two-way stream socket, which can be either a TCP or a unix socket. A
> unix socket will be created if @option{path} is specified. Behaviour is
> @@ -1874,6 +1875,10 @@ connect to a listening socket.
> @option{telnet} specifies that traffic on the socket should interpret telnet
> escape sequences.
>
> +@option{reconnect} specifies that if the client socket does not connect at
> +startup, or if the client socket is closed for some reason (like the other
> +end exited), wait the given number of seconds and attempt to reconnect.
> +
> TCP and unix socket options are given below:
>
> @table @option
The client will reconnect for ever when the server is dead. Is it better that try to reconnect several times?
Or add a option which specifies times of reconnect?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-05 0:38 ` [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected minyard
2014-03-06 6:47 ` Weidong Huang
@ 2014-03-06 7:43 ` Michael S. Tsirkin
2014-03-06 18:04 ` Corey Minyard
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2014-03-06 7:43 UTC (permalink / raw)
To: minyard; +Cc: bcketchum, Corey Minyard, hwd, qemu-devel, afaerber
On Tue, Mar 04, 2014 at 06:38:52PM -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Allow a socket that connects to reconnect on a periodic basis if it
> fails to connect at startup or if the connection drops while in use.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> include/sysemu/char.h | 16 ++++-
> qemu-char.c | 165 +++++++++++++++++++++++++++++++++++++++++++++-----
> qemu-options.hx | 11 +++-
> 3 files changed, 173 insertions(+), 19 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index f6844dd..1800c54 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -53,6 +53,17 @@ typedef struct {
>
> typedef void IOEventHandler(void *opaque, int event);
>
> +struct ReconData {
> + void *opaque;
> + uint64_t recon_time;
> + struct ReconHandlers *handlers;
> + void (*state_change)(void *open_opaque, int is_open);
> + void *state_change_opaque;
> + void (*reconnected)(struct ReconData *r);
> + void (*connection_lost)(struct ReconData *r);
> + void (*shutdown)(struct ReconData *r);
> +};
> +
> struct CharDriverState {
> void (*init)(struct CharDriverState *s);
> int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
> @@ -82,6 +93,8 @@ struct CharDriverState {
> guint fd_in_tag;
> QemuOpts *opts;
> QTAILQ_ENTRY(CharDriverState) next;
> + GSList *backend;
> + struct ReconData *recon;
> };
>
> /**
> @@ -293,7 +306,8 @@ CharDriverState *qemu_chr_find(const char *name);
> QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
>
> void register_char_driver(const char *name,
> - CharDriverState *(*open)(CharDriverState *, QemuOpts *));
> + CharDriverState *(*open)(CharDriverState *, QemuOpts *),
> + int (*recon_setup)(CharDriverState *));
> void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
> void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
>
> diff --git a/qemu-char.c b/qemu-char.c
> index fc688cf..d9838aa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -88,6 +88,44 @@
> /***********************************************************/
> /* character device */
>
> +struct GenericReconData {
> + QEMUTimer *recon_timer;
> +};
> +
> +static void generic_recon_timeout(void *opaque)
> +{
> + struct ReconData *r = opaque;
> + struct GenericReconData *d = r->opaque;
> +
> + timer_mod(d->recon_timer,
> + (get_clock() + (r->recon_time * get_ticks_per_sec())));
> + r->state_change(r->state_change_opaque, 0);
> +}
> +
> +static void generic_reconnected(struct ReconData *r)
> +{
> + struct GenericReconData *d = r->opaque;
> +
> + timer_del(d->recon_timer);
> +}
> +
> +static void generic_connection_lost(struct ReconData *r)
> +{
> + struct GenericReconData *d = r->opaque;
> +
> + r->state_change(r->state_change_opaque, 1);
> + timer_mod(d->recon_timer,
> + (get_clock() + (r->recon_time * get_ticks_per_sec())));
> +}
> +
> +static void generic_recon_shutdown(struct ReconData *r)
> +{
> + struct GenericReconData *d = r->opaque;
> + timer_free(d->recon_timer);
> + free(d);
> + r->opaque = NULL;
> +}
> +
> static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
> QTAILQ_HEAD_INITIALIZER(chardevs);
>
> @@ -96,9 +134,15 @@ void qemu_chr_be_event(CharDriverState *s, int event)
> /* Keep track if the char device is open */
> switch (event) {
> case CHR_EVENT_OPENED:
> + if (s->recon) {
> + s->recon->reconnected(s->recon);
> + }
> s->be_open = 1;
> break;
> case CHR_EVENT_CLOSED:
> + if (s->recon) {
> + s->recon->connection_lost(s->recon);
> + }
> s->be_open = 0;
> break;
> }
> @@ -2582,6 +2626,7 @@ static void tcp_chr_close(CharDriverState *chr)
> closesocket(s->listen_fd);
> }
> g_free(s);
> + chr->opaque = NULL;
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
>
> @@ -2641,8 +2686,6 @@ static CharDriverState *qemu_chr_open_socket_fd(CharDriverState *chr,
> chr->get_msgfd = tcp_get_msgfd;
> chr->chr_add_client = tcp_chr_add_client;
> chr->chr_add_watch = tcp_chr_add_watch;
> - /* be isn't opened until we get a connection */
> - chr->explicit_be_open = true;
>
> if (is_listen) {
> s->listen_fd = fd;
> @@ -2680,6 +2723,9 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
> bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true);
> bool is_unix = qemu_opt_get(opts, "path") != NULL;
>
> + /* be isn't opened until we get a connection */
> + chr->explicit_be_open = true;
> +
> if (is_unix) {
> if (is_listen) {
> fd = unix_listen_opts(opts, &local_err);
> @@ -2716,8 +2762,9 @@ static CharDriverState *qemu_chr_open_socket(CharDriverState *chr,
> if (fd >= 0) {
> closesocket(fd);
> }
> - if (chr) {
> + if (chr && chr->opaque) {
> g_free(chr->opaque);
> + chr->opaque = NULL;
> }
> return NULL;
> }
> @@ -3128,6 +3175,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
>
> typedef struct CharDriver {
> const char *name;
> + int (*recon_setup)(CharDriverState *);
> /* old, pre qapi */
> CharDriverState *(*open)(CharDriverState *chr, QemuOpts *opts);
> /* new, qapi-based */
> @@ -3138,13 +3186,15 @@ typedef struct CharDriver {
> static GSList *backends;
>
> void register_char_driver(const char *name,
> - CharDriverState *(*open)(CharDriverState*,QemuOpts *))
> + CharDriverState *(*open)(CharDriverState*,QemuOpts *),
> + int (*recon_setup)(CharDriverState *))
> {
> CharDriver *s;
>
> s = g_malloc0(sizeof(*s));
> s->name = g_strdup(name);
> s->open = open;
> + s->recon_setup = recon_setup;
>
> backends = g_slist_append(backends, s);
> }
> @@ -3162,6 +3212,50 @@ void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
> backends = g_slist_append(backends, s);
> }
>
> +static void generic_recon_state_change(void *opaque, int is_open)
> +{
> + struct CharDriverState *chr = opaque;
> +
> + if (!is_open) {
> + struct CharDriver *cd = chr->backend->data;
> +
> + if (chr->be_open) {
> + return;
> + }
> +
> + cd->open(chr, chr->opts);
> + } else {
> + void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
> +
> + if (chr_close) {
> + chr->chr_close = NULL;
> + chr_close(chr);
> + }
> + }
> +}
> +
> +static int generic_recon_setup(struct CharDriverState *chr)
> +{
> + struct GenericReconData *d;
> +
> + d = g_malloc(sizeof(*d));
> + chr->recon->opaque = d;
> +
> + chr->recon->reconnected = generic_reconnected;
> + chr->recon->connection_lost = generic_connection_lost;
> + chr->recon->shutdown = generic_recon_shutdown;
> + chr->recon->state_change = generic_recon_state_change;
> + chr->recon->state_change_opaque = chr;
> +
> + d->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS,
> + generic_recon_timeout, chr->recon);
> +
> + /* Make sure it connects in time. */
> + timer_mod(d->recon_timer,
> + (get_clock() + (chr->recon->recon_time * get_ticks_per_sec())));
> + return 1;
> +}
> +
> CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> void (*init)(struct CharDriverState *s),
> Error **errp)
> @@ -3169,6 +3263,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> CharDriver *cd;
> CharDriverState *chr;
> GSList *i;
> + uint64_t recon_time;
>
> if (qemu_opts_id(opts) == NULL) {
> error_setg(errp, "chardev: no id specified");
> @@ -3244,11 +3339,41 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> }
>
> chr = g_malloc0(sizeof(CharDriverState));
> - chr = cd->open(chr, opts);
> - if (!chr) {
> - error_setg(errp, "chardev: opening backend \"%s\" failed",
> - qemu_opt_get(opts, "backend"));
> - goto err;
> +
> + chr->backend = i;
> + recon_time = qemu_opt_get_number(opts, "reconnect", 0);
> + if (recon_time) {
> + CharDriver *d = chr->backend->data;
> + if (!d->recon_setup) {
> + g_free(chr);
> + fprintf(stderr, "chardev: reconnect not supported on %s\n",
> + qemu_opt_get(opts, "backend"));
> + return NULL;
> + }
> + if (qemu_opt_get_bool(opts, "server", 0)) {
> + g_free(chr);
> + fprintf(stderr, "chardev: server device cannot reconnect\n");
> + return NULL;
> + }
> + chr->opts = opts;
> + chr->recon = g_malloc(sizeof(*chr->recon));
> + chr->recon->recon_time = recon_time;
> + if (!d->recon_setup(chr)) {
> + g_free(chr->recon);
> + g_free(chr);
> + fprintf(stderr, "chardev: Unable to set up reconnect\n");
> + return NULL;
> + }
> + }
> +
> + if (!cd->open(chr, opts)) {
> + if (!chr->recon) {
> + /* Reconnect is not enabled, give up */
> + fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
> + qemu_opt_get(opts, "backend"));
> + g_free(chr);
> + return NULL;
> + }
> }
>
> if (!chr->filename)
> @@ -3267,7 +3392,8 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> int len = strlen(qemu_opts_id(opts)) + 6;
> base->label = g_malloc(len);
> snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
> - chr = qemu_chr_open_mux(chr, base);
> + chr = g_malloc0(sizeof(CharDriverState));
> + qemu_chr_open_mux(chr, base);
> chr->filename = base->filename;
> chr->avail_connections = MAX_MUX;
> QTAILQ_INSERT_TAIL(&chardevs, chr, next);
> @@ -3275,7 +3401,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> chr->avail_connections = 1;
> }
> chr->label = g_strdup(qemu_opts_id(opts));
> - chr->opts = opts;
> return chr;
>
> err:
> @@ -3378,9 +3503,16 @@ void qemu_chr_fe_release(CharDriverState *s)
>
> void qemu_chr_delete(CharDriverState *chr)
> {
> + void (*chr_close)(struct CharDriverState *chr) = chr->chr_close;
> +
> QTAILQ_REMOVE(&chardevs, chr, next);
> - if (chr->chr_close) {
> - chr->chr_close(chr);
> + if (chr_close) {
> + chr->chr_close = NULL;
> + chr_close(chr);
> + }
> + if (chr->recon) {
> + chr->recon->shutdown(chr->recon);
> + g_free(chr->recon);
> }
> g_free(chr->filename);
> g_free(chr->label);
> @@ -3515,6 +3647,9 @@ QemuOptsList qemu_chardev_opts = {
> .name = "mux",
> .type = QEMU_OPT_BOOL,
> },{
> + .name = "reconnect",
> + .type = QEMU_OPT_NUMBER,
> + },{
> .name = "signal",
> .type = QEMU_OPT_BOOL,
> },{
> @@ -3811,8 +3946,8 @@ void qmp_chardev_remove(const char *id, Error **errp)
> static void register_types(void)
> {
> register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
> - register_char_driver("socket", qemu_chr_open_socket);
> - register_char_driver("udp", qemu_chr_open_udp);
> + register_char_driver("socket", qemu_chr_open_socket, generic_recon_setup);
> + register_char_driver("udp", qemu_chr_open_udp, NULL);
> register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
> qemu_chr_parse_ringbuf);
> register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 56e5fdf..1002a3d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1787,8 +1787,9 @@ ETEXI
> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev null,id=id[,mux=on|off]\n"
> "-chardev socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay]\n"
> - " [,server][,nowait][,telnet][,mux=on|off] (tcp)\n"
> - "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
> + " [,server][,nowait][,telnet][,mux=on|off][,reconnect=seconds] (tcp)\n"
> + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,mux=on|off]\n"
> + " [,reconnect=seconds] (unix)\n"
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
> "-chardev msmouse,id=id[,mux=on|off]\n"
> @@ -1860,7 +1861,7 @@ Options to each backend are described below.
> A void device. This device will not emit any data, and will drop any data it
> receives. The null backend does not take any options.
>
> -@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
> +@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,reconnect=@var{seconds}]
>
> Create a two-way stream socket, which can be either a TCP or a unix socket. A
> unix socket will be created if @option{path} is specified. Behaviour is
> @@ -1874,6 +1875,10 @@ connect to a listening socket.
> @option{telnet} specifies that traffic on the socket should interpret telnet
> escape sequences.
>
> +@option{reconnect} specifies that if the client socket does not connect at
> +startup, or if the client socket is closed for some reason (like the other
> +end exited), wait the given number of seconds and attempt to reconnect.
> +
> TCP and unix socket options are given below:
>
> @table @option
This looks useful but it bothers me that this
new option can't be used together with others:
server and nowait, which limits its usefulness
and would make it harder to extend in the future.
- I think that specifying a time-out should be separate from the option to
retry.
E.g. it might be quite reasonable to retry connecting immediately.
- it seems to me that if we don't specify nowait, this
should stop the VM until reconnect.
- I think this can co-exist with server option.
E.g. if we detect that the other side closed the socket,
start listening again. Or even listen all the time
and accept if disconnected.
I'd like to suggest we generalize this a bit:
When we detect a disconnect, three thinkable things to
do would be:
- keep going as if nothing happened
(default for all sockets except monitor)
- exit qemu
(default for monitor)
- retry
When waiting for retry it seems reasonable to
either stop the VM or keep going (nowait option).
When retrying, it seems reasonable to specify a
timeout to make it fail.
All of the above seem to apply to server
and client sockets, with or without nowait option.
For clients only, it might be somewhat useful to
limit the network traffic caused by reconnects.
Thanks,
MST
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-06 6:47 ` Weidong Huang
@ 2014-03-06 17:18 ` Corey Minyard
2014-03-06 17:41 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Corey Minyard @ 2014-03-06 17:18 UTC (permalink / raw)
To: Weidong Huang, minyard; +Cc: bcketchum, qemu-devel, afaerber, mst
On 03/06/2014 12:47 AM, Weidong Huang wrote:
> escape sequences.
>
> +@option{reconnect} specifies that if the client socket does not connect at
> +startup, or if the client socket is closed for some reason (like the other
> +end exited), wait the given number of seconds and attempt to reconnect.
> +
> TCP and unix socket options are given below:
>
> @table @option
> The client will reconnect for ever when the server is dead. Is it better that try to reconnect several times?
> Or add a option which specifies times of reconnect?
>
I'm not really sure about this. For a remote IPMI BMC, you would want
it to reconnect for forever, there's no point it having it stop trying
after a while, since you want it to come back even if the BMC is down
for a day. I would think the same if you wanted a remote console or
something of that nature.
What's the use case where you would want it to stop trying? I can't
think of any.
Thanks,
-corey
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-06 17:18 ` Corey Minyard
@ 2014-03-06 17:41 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2014-03-06 17:41 UTC (permalink / raw)
To: Corey Minyard; +Cc: bcketchum, Weidong Huang, qemu-devel, minyard, afaerber
On Thu, Mar 06, 2014 at 11:18:00AM -0600, Corey Minyard wrote:
> On 03/06/2014 12:47 AM, Weidong Huang wrote:
> > escape sequences.
> >
> > +@option{reconnect} specifies that if the client socket does not connect at
> > +startup, or if the client socket is closed for some reason (like the other
> > +end exited), wait the given number of seconds and attempt to reconnect.
> > +
> > TCP and unix socket options are given below:
> >
> > @table @option
> > The client will reconnect for ever when the server is dead. Is it better that try to reconnect several times?
> > Or add a option which specifies times of reconnect?
> >
> I'm not really sure about this. For a remote IPMI BMC, you would want
> it to reconnect for forever, there's no point it having it stop trying
> after a while, since you want it to come back even if the BMC is down
> for a day. I would think the same if you wanted a remote console or
> something of that nature.
>
> What's the use case where you would want it to stop trying? I can't
> think of any.
>
> Thanks,
>
> -corey
Depends on what happens after it fails.
Imagine whoever was using the socket is gone.
It might be useful to stop VM or exit
after a while rather than just hang about.
--
MST
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-06 7:43 ` Michael S. Tsirkin
@ 2014-03-06 18:04 ` Corey Minyard
2014-03-06 18:43 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Corey Minyard @ 2014-03-06 18:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: bcketchum, hwd, qemu-devel, afaerber
On 03/06/2014 01:43 AM, Michael S. Tsirkin wrote:
>
> +@option{reconnect} specifies that if the client socket does not connect at
> +startup, or if the client socket is closed for some reason (like the other
> +end exited), wait the given number of seconds and attempt to reconnect.
> +
> TCP and unix socket options are given below:
>
> @table @option
> This looks useful but it bothers me that this
> new option can't be used together with others:
> server and nowait, which limits its usefulness
> and would make it harder to extend in the future.
>
> - I think that specifying a time-out should be separate from the option to
> retry.
> E.g. it might be quite reasonable to retry connecting immediately.
Maybe, but that would basically disable qemu until the connection came
back, because it would just be spinning trying to connect. That doesn't
seem like a good idea to me.
> - it seems to me that if we don't specify nowait, this
> should stop the VM until reconnect.
That's an interesting thought. Currently the option basically implies
nowait. It's probably easy to add this, but I'm not sure of the use
case. Anyone else have any opinions on this?
> - I think this can co-exist with server option.
> E.g. if we detect that the other side closed the socket,
> start listening again. Or even listen all the time
> and accept if disconnected.
IIRC, that's what it does already, even without this patch. If in
server mode, I believe you can disconnect and then reconnect a socket
and it works fine. So I see no need for this function in server mode.
>
> I'd like to suggest we generalize this a bit:
>
> When we detect a disconnect, three thinkable things to
> do would be:
> - keep going as if nothing happened
> (default for all sockets except monitor)
> - exit qemu
> (default for monitor)
> - retry
There's also
- stop the VM until the socket reconnects.
I'm not sure why you would want to single out monitor as special.
Especially if it is mux-ed into another connection.
>
> When waiting for retry it seems reasonable to
> either stop the VM or keep going (nowait option).
>
> When retrying, it seems reasonable to specify a
> timeout to make it fail.
I just don't see why you would want to do this. It means that the
connection is gone until the VM starts back up again.
One thing I did think of was to add a monitor command to try to
reconnect. Then I could understand giving up, since you can get into
the monitor and kick it back off.
Thanks,
-corey
> All of the above seem to apply to server
> and client sockets, with or without nowait option.
>
> For clients only, it might be somewhat useful to
> limit the network traffic caused by reconnects.
>
> Thanks,
> MST
>
>> --
>> 1.8.3.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected
2014-03-06 18:04 ` Corey Minyard
@ 2014-03-06 18:43 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2014-03-06 18:43 UTC (permalink / raw)
To: Corey Minyard; +Cc: bcketchum, hwd, qemu-devel, afaerber
On Thu, Mar 06, 2014 at 12:04:24PM -0600, Corey Minyard wrote:
> On 03/06/2014 01:43 AM, Michael S. Tsirkin wrote:
> >
> > +@option{reconnect} specifies that if the client socket does not connect at
> > +startup, or if the client socket is closed for some reason (like the other
> > +end exited), wait the given number of seconds and attempt to reconnect.
> > +
> > TCP and unix socket options are given below:
> >
> > @table @option
> > This looks useful but it bothers me that this
> > new option can't be used together with others:
> > server and nowait, which limits its usefulness
> > and would make it harder to extend in the future.
> >
> > - I think that specifying a time-out should be separate from the option to
> > retry.
> > E.g. it might be quite reasonable to retry connecting immediately.
>
> Maybe, but that would basically disable qemu until the connection came
> back, because it would just be spinning trying to connect.
Why spin? queue the request in bh.
> That doesn't
> seem like a good idea to me.
>
> > - it seems to me that if we don't specify nowait, this
> > should stop the VM until reconnect.
>
> That's an interesting thought. Currently the option basically implies
> nowait. It's probably easy to add this, but I'm not sure of the use
> case.
To avoid losing information that would have been sent there.
> Anyone else have any opinions on this?
>
> > - I think this can co-exist with server option.
> > E.g. if we detect that the other side closed the socket,
> > start listening again. Or even listen all the time
> > and accept if disconnected.
>
> IIRC, that's what it does already, even without this patch. If in
> server mode, I believe you can disconnect and then reconnect a socket
> and it works fine. So I see no need for this function in server mode.
>
> >
> > I'd like to suggest we generalize this a bit:
> >
> > When we detect a disconnect, three thinkable things to
> > do would be:
> > - keep going as if nothing happened
> > (default for all sockets except monitor)
> > - exit qemu
> > (default for monitor)
> > - retry
> There's also
> - stop the VM until the socket reconnects.
>
> I'm not sure why you would want to single out monitor as special.
> Especially if it is mux-ed into another connection.
it seems useful to make qemu exit when monitor
closes. many programs behave this way, this
makes sure if you lose connection,
the server does not hang around doing nothing.
combine with keepalive socket option,
you get a way to make qemu exit if it loses
networking, which just might help restore networking.
> >
> > When waiting for retry it seems reasonable to
> > either stop the VM or keep going (nowait option).
> >
> > When retrying, it seems reasonable to specify a
> > timeout to make it fail.
>
> I just don't see why you would want to do this. It means that the
> connection is gone until the VM starts back up again.
e.g. if socket is used for serial, you might want to stop
VM to avoid losing output.
> One thing I did think of was to add a monitor command to try to
> reconnect. Then I could understand giving up, since you can get into
> the monitor and kick it back off.
>
> Thanks,
>
> -corey
>
if qemu exits on timeout, then that's also useful
as a way of automatically cleaning up resources
if you lose connection to qemu.
which might help host recover from e.g. oom
situation, and become responsive again.
> > All of the above seem to apply to server
> > and client sockets, with or without nowait option.
> >
> > For clients only, it might be somewhat useful to
> > limit the network traffic caused by reconnects.
> >
> > Thanks,
> > MST
> >
> >> --
> >> 1.8.3.1
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-03-06 19:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-05 0:38 [Qemu-devel] [PATCH 0/7] Allow a client chardev to reconnect if disconnected minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 1/7] qemu-char: Allocate CharDriverState in qemu_chr_new_from_opts minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 2/7] qemu-char: Allow a chardev to reconnect if disconnected minyard
2014-03-06 6:47 ` Weidong Huang
2014-03-06 17:18 ` Corey Minyard
2014-03-06 17:41 ` Michael S. Tsirkin
2014-03-06 7:43 ` Michael S. Tsirkin
2014-03-06 18:04 ` Corey Minyard
2014-03-06 18:43 ` Michael S. Tsirkin
2014-03-05 0:38 ` [Qemu-devel] [PATCH 3/7] qemu-char: Wait until socket connect to report connected minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 4/7] qemu-char: remove free of chr from win_stdio_close minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 5/7] qemu-char: Close fd at end of file minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 6/7] qemu-char: Clean up error handling in qmp_chardev_add minyard
2014-03-05 0:38 ` [Qemu-devel] [PATCH 7/7] console: Don't use the console if NULL minyard
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.