Message ID | 1397130226-7332-1-git-send-email-arei.gonglei@huawei.com |
---|---|
State | New |
Headers | show |
On 04/10/2014 06:43 AM, arei.gonglei@huawei.com wrote: > From: Huangweidong <weidong.huang@huawei.com> > > Allow a socket chardev reconnect if the connection drops while in use. > > Signed-off-by: Huangweidong <weidong.huang@huawei.com> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > This patch is modified according to corey's patch. Some changes below: > 1. IMO it's unnecessary that chardev reconnect if it fails to connect at startup. > Qemu exit in this scene. In this way the patch does not change interface of chardev. > It would be much more simple. I believe that it should not stop qemu if it fails at startup. Otherwise you constrain the start order and you can prevent a server from coming up because of a missing resource that may not be that critical at the moment. With the current implementation, client sockets really aren't that useful for a critical system. Reconnecting makes it usable in a critical system. > 2. I set the reconnect timer one second, just like pty. I'm not too picky about the time. A couple of things about this: With this patch, the default behavior changes to reconnect. That might cause issues for some users. Adding a configurable timeout is easy if you have to specify something on the command line, that's why I did it. Also, if something is listening to connect/disconnect events from the device, it will get a connect then disconnect every second. It's probably better to wait until the connection is actually established before you report it up. -corey > > include/sysemu/char.h | 2 ++ > qemu-char.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/include/sysemu/char.h b/include/sysemu/char.h > index b81a6ff..f646ac8 100644 > --- a/include/sysemu/char.h > +++ b/include/sysemu/char.h > @@ -19,6 +19,7 @@ > #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */ > #define CHR_EVENT_CLOSED 5 /* connection closed */ > > +#define CHR_SOCK_RECONNECT_TIME 1 /* reconnection time (second) */ > > #define CHR_IOCTL_SERIAL_SET_PARAMS 1 > typedef struct { > @@ -82,6 +83,7 @@ struct CharDriverState { > guint fd_in_tag; > QemuOpts *opts; > QTAILQ_ENTRY(CharDriverState) next; > + QEMUTimer *recon_timer; > }; > > /** > diff --git a/qemu-char.c b/qemu-char.c > index 54ed244..a87a345 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -96,9 +96,17 @@ 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_timer) { > + timer_del(s->recon_timer); > + } > s->be_open = 1; > break; > case CHR_EVENT_CLOSED: > + if (s->recon_timer) { > + timer_mod(s->recon_timer, > + (get_clock() + > + (CHR_SOCK_RECONNECT_TIME * get_ticks_per_sec()))); > + } > s->be_open = 0; > break; > } > @@ -2619,6 +2627,43 @@ static void tcp_chr_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > +static void recon_timeout(void *opaque) > +{ > + CharDriverState *chr = opaque; > + QemuOpts *opts = chr->opts; > + TCPCharDriver *tcp = (TCPCharDriver *)chr->opaque; > + int fd = -1; > + Error *local_err = NULL; > + > + if (chr->be_open) { > + return; > + } > + > + if (tcp->is_unix) { > + fd = unix_connect_opts(opts, &local_err, NULL, NULL); > + } else { > + fd = inet_connect_opts(opts, &local_err, NULL, NULL); > + } > + > + if (fd < 0) { > + goto fail; > + } > + > + tcp->fd = fd; > + socket_set_nodelay(fd); > + tcp->chan = io_channel_from_socket(tcp->fd); > + tcp_chr_connect(chr); > + printf("chardev: socket reconnect sucess\n"); > + return; > + > +fail: > + if (local_err) { > + qerror_report_err(local_err); > + error_free(local_err); > + } > + qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > +} > + > static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, > bool is_listen, bool is_telnet, > bool is_waitconnect, > @@ -2693,6 +2738,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, > socket_set_nodelay(fd); > s->chan = io_channel_from_socket(s->fd); > tcp_chr_connect(chr); > + chr->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS, > + recon_timeout, chr); > + timer_mod(chr->recon_timer, > + (get_clock() + > + (CHR_SOCK_RECONNECT_TIME * get_ticks_per_sec()))); > } > > if (is_listen && is_waitconnect) {
Hi, > > This patch is modified according to corey's patch. Some changes below: > > 1. IMO it's unnecessary that chardev reconnect if it fails to connect at startup. > > Qemu exit in this scene. In this way the patch does not change interface of chardev. > > It would be much more simple. > > I believe that it should not stop qemu if it fails at startup. > Otherwise you constrain the start order and you can prevent a server > from coming up because of a missing resource that may not be that > critical at the moment. With reconnect being implemented (and active, guess we should add a option for it) it we might rethink this indeed. > With the current implementation, client sockets > really aren't that useful for a critical system. Reconnecting makes it > usable in a critical system. I consider client sockets not very useful at all, I always put my sockets into server mode. YMMV though. > > 2. I set the reconnect timer one second, just like pty. > > I'm not too picky about the time. A couple of things about this: Well, sockets and pty are different. pty is a local thing. sockets (at least tcp) goes out to the network. After each failed reconnect the time to retry should be increased a bit so you don't flood the non-responding server with connect requests. > With this patch, the default behavior changes to reconnect. That might > cause issues for some users. Adding a configurable timeout is easy if > you have to specify something on the command line, that's why I did it. Specifying the timeout (best with min+max) could enable reconnect at the same time, i.e. something like -chardev socket,host=1.2.3.4,port=5678,reconnect=1-300 would turn on reconnect, with 1 second min and 300 seconds (5min) max delay. cheers, Gerd
diff --git a/include/sysemu/char.h b/include/sysemu/char.h index b81a6ff..f646ac8 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -19,6 +19,7 @@ #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */ #define CHR_EVENT_CLOSED 5 /* connection closed */ +#define CHR_SOCK_RECONNECT_TIME 1 /* reconnection time (second) */ #define CHR_IOCTL_SERIAL_SET_PARAMS 1 typedef struct { @@ -82,6 +83,7 @@ struct CharDriverState { guint fd_in_tag; QemuOpts *opts; QTAILQ_ENTRY(CharDriverState) next; + QEMUTimer *recon_timer; }; /** diff --git a/qemu-char.c b/qemu-char.c index 54ed244..a87a345 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -96,9 +96,17 @@ 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_timer) { + timer_del(s->recon_timer); + } s->be_open = 1; break; case CHR_EVENT_CLOSED: + if (s->recon_timer) { + timer_mod(s->recon_timer, + (get_clock() + + (CHR_SOCK_RECONNECT_TIME * get_ticks_per_sec()))); + } s->be_open = 0; break; } @@ -2619,6 +2627,43 @@ static void tcp_chr_close(CharDriverState *chr) qemu_chr_be_event(chr, CHR_EVENT_CLOSED); } +static void recon_timeout(void *opaque) +{ + CharDriverState *chr = opaque; + QemuOpts *opts = chr->opts; + TCPCharDriver *tcp = (TCPCharDriver *)chr->opaque; + int fd = -1; + Error *local_err = NULL; + + if (chr->be_open) { + return; + } + + if (tcp->is_unix) { + fd = unix_connect_opts(opts, &local_err, NULL, NULL); + } else { + fd = inet_connect_opts(opts, &local_err, NULL, NULL); + } + + if (fd < 0) { + goto fail; + } + + tcp->fd = fd; + socket_set_nodelay(fd); + tcp->chan = io_channel_from_socket(tcp->fd); + tcp_chr_connect(chr); + printf("chardev: socket reconnect sucess\n"); + return; + +fail: + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); + } + qemu_chr_be_event(chr, CHR_EVENT_CLOSED); +} + static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, bool is_listen, bool is_telnet, bool is_waitconnect, @@ -2693,6 +2738,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, socket_set_nodelay(fd); s->chan = io_channel_from_socket(s->fd); tcp_chr_connect(chr); + chr->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS, + recon_timeout, chr); + timer_mod(chr->recon_timer, + (get_clock() + + (CHR_SOCK_RECONNECT_TIME * get_ticks_per_sec()))); } if (is_listen && is_waitconnect) {