[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 11/16] chardev: use a state machine for socket co
From: |
Daniel P . Berrangé |
Subject: |
[Qemu-devel] [PATCH v2 11/16] chardev: use a state machine for socket connection state |
Date: |
Wed, 23 Jan 2019 17:27:35 +0000 |
The socket connection state is indicated via the 'bool connected' field
in the SocketChardev struct. This variable is somewhat misleading
though, as it is only set to true once the connection has completed all
required handshakes (eg for TLS, telnet or websockets). IOW there is a
period of time in which the socket is connected, but the "connected"
flag is still false.
The socket chardev really has three states that it can be in,
disconnected, connecting and connected and those should be tracked
explicitly.
Reviewed-by: Marc-André Lureau <address@hidden>
Signed-off-by: Daniel P. Berrangé <address@hidden>
---
chardev/char-socket.c | 63 +++++++++++++++++++++++++++++++++----------
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 90dafef7d4..d6de5d2305 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -46,6 +46,12 @@ typedef struct {
size_t buflen;
} TCPChardevTelnetInit;
+typedef enum {
+ TCP_CHARDEV_STATE_DISCONNECTED,
+ TCP_CHARDEV_STATE_CONNECTING,
+ TCP_CHARDEV_STATE_CONNECTED,
+} TCPChardevState;
+
typedef struct {
Chardev parent;
QIOChannel *ioc; /* Client I/O channel */
@@ -53,7 +59,7 @@ typedef struct {
QIONetListener *listener;
GSource *hup_source;
QCryptoTLSCreds *tls_creds;
- int connected;
+ TCPChardevState state;
int max_size;
int do_telnetopt;
int do_nodelay;
@@ -82,6 +88,21 @@ typedef struct {
static gboolean socket_reconnect_timeout(gpointer opaque);
static void tcp_chr_telnet_init(Chardev *chr);
+static void tcp_chr_change_state(SocketChardev *s, TCPChardevState state)
+{
+ switch (state) {
+ case TCP_CHARDEV_STATE_DISCONNECTED:
+ break;
+ case TCP_CHARDEV_STATE_CONNECTING:
+ assert(s->state == TCP_CHARDEV_STATE_DISCONNECTED);
+ break;
+ case TCP_CHARDEV_STATE_CONNECTED:
+ assert(s->state == TCP_CHARDEV_STATE_CONNECTING);
+ break;
+ }
+ s->state = state;
+}
+
static void tcp_chr_reconn_timer_cancel(SocketChardev *s)
{
if (s->reconnect_timer) {
@@ -96,7 +117,7 @@ static void qemu_chr_socket_restart_timer(Chardev *chr)
SocketChardev *s = SOCKET_CHARDEV(chr);
char *name;
- assert(s->connected == 0);
+ assert(s->state == TCP_CHARDEV_STATE_DISCONNECTED);
name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label);
s->reconnect_timer = qemu_chr_timeout_add_ms(chr,
s->reconnect_time * 1000,
@@ -131,7 +152,7 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf,
int len)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
- if (s->connected) {
+ if (s->state == TCP_CHARDEV_STATE_CONNECTED) {
int ret = io_channel_send_full(s->ioc, buf, len,
s->write_msgfds,
s->write_msgfds_num);
@@ -164,7 +185,7 @@ static int tcp_chr_read_poll(void *opaque)
{
Chardev *chr = CHARDEV(opaque);
SocketChardev *s = SOCKET_CHARDEV(opaque);
- if (!s->connected) {
+ if (s->state != TCP_CHARDEV_STATE_CONNECTED) {
return 0;
}
s->max_size = qemu_chr_be_can_write(chr);
@@ -277,7 +298,7 @@ static int tcp_set_msgfds(Chardev *chr, int *fds, int num)
s->write_msgfds = NULL;
s->write_msgfds_num = 0;
- if (!s->connected ||
+ if ((s->state != TCP_CHARDEV_STATE_CONNECTED) ||
!qio_channel_has_feature(s->ioc,
QIO_CHANNEL_FEATURE_FD_PASS)) {
return -1;
@@ -389,7 +410,7 @@ static void tcp_chr_free_connection(Chardev *chr)
s->ioc = NULL;
g_free(chr->filename);
chr->filename = NULL;
- s->connected = 0;
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
}
static const char *qemu_chr_socket_protocol(SocketChardev *s)
@@ -442,12 +463,12 @@ static void update_disconnected_filename(SocketChardev *s)
/* NB may be called even if tcp_chr_connect has not been
* reached, due to TLS or telnet initialization failure,
- * so can *not* assume s->connected == true
+ * so can *not* assume s->state == TCP_CHARDEV_STATE_CONNECTED
*/
static void tcp_chr_disconnect(Chardev *chr)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
- bool emit_close = s->connected;
+ bool emit_close = s->state == TCP_CHARDEV_STATE_CONNECTED;
tcp_chr_free_connection(chr);
@@ -471,7 +492,8 @@ static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition
cond, void *opaque)
uint8_t buf[CHR_READ_BUF_LEN];
int len, size;
- if (!s->connected || s->max_size <= 0) {
+ if ((s->state != TCP_CHARDEV_STATE_CONNECTED) ||
+ s->max_size <= 0) {
return TRUE;
}
len = sizeof(buf);
@@ -508,7 +530,7 @@ static int tcp_chr_sync_read(Chardev *chr, const uint8_t
*buf, int len)
SocketChardev *s = SOCKET_CHARDEV(chr);
int size;
- if (!s->connected) {
+ if (s->state != TCP_CHARDEV_STATE_CONNECTED) {
return 0;
}
@@ -564,7 +586,7 @@ static void update_ioc_handlers(SocketChardev *s)
{
Chardev *chr = CHARDEV(s);
- if (!s->connected) {
+ if (s->state != TCP_CHARDEV_STATE_CONNECTED) {
return;
}
@@ -589,7 +611,7 @@ static void tcp_chr_connect(void *opaque)
g_free(chr->filename);
chr->filename = qemu_chr_compute_filename(s);
- s->connected = 1;
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTED);
update_ioc_handlers(s);
qemu_chr_be_event(chr, CHR_EVENT_OPENED);
}
@@ -828,7 +850,7 @@ static int tcp_chr_new_client(Chardev *chr,
QIOChannelSocket *sioc)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
- if (s->ioc != NULL) {
+ if (s->state != TCP_CHARDEV_STATE_CONNECTING) {
return -1;
}
@@ -865,11 +887,17 @@ static int tcp_chr_add_client(Chardev *chr, int fd)
{
int ret;
QIOChannelSocket *sioc;
+ SocketChardev *s = SOCKET_CHARDEV(chr);
+
+ if (s->state != TCP_CHARDEV_STATE_DISCONNECTED) {
+ return -1;
+ }
sioc = qio_channel_socket_new_fd(fd, NULL);
if (!sioc) {
return -1;
}
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
tcp_chr_set_client_ioc_name(chr, sioc);
ret = tcp_chr_new_client(chr, sioc);
object_unref(OBJECT(sioc));
@@ -881,7 +909,9 @@ static void tcp_chr_accept(QIONetListener *listener,
void *opaque)
{
Chardev *chr = CHARDEV(opaque);
+ SocketChardev *s = SOCKET_CHARDEV(chr);
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
tcp_chr_set_client_ioc_name(chr, cioc);
tcp_chr_new_client(chr, cioc);
}
@@ -891,8 +921,10 @@ static int tcp_chr_connect_client_sync(Chardev *chr, Error
**errp)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
QIOChannelSocket *sioc = qio_channel_socket_new();
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
tcp_chr_set_client_ioc_name(chr, sioc);
if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) {
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
object_unref(OBJECT(sioc));
return -1;
}
@@ -908,6 +940,7 @@ static void tcp_chr_accept_server_sync(Chardev *chr)
QIOChannelSocket *sioc;
info_report("QEMU waiting for connection on: %s",
chr->filename);
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
sioc = qio_net_listener_wait_client(s->listener);
tcp_chr_set_client_ioc_name(chr, sioc);
tcp_chr_new_client(chr, sioc);
@@ -963,6 +996,7 @@ static void qemu_chr_socket_connected(QIOTask *task, void
*opaque)
Error *err = NULL;
if (qio_task_propagate_error(task, &err)) {
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
check_report_connect_error(chr, err);
error_free(err);
goto cleanup;
@@ -980,6 +1014,7 @@ static void tcp_chr_connect_client_async(Chardev *chr)
SocketChardev *s = SOCKET_CHARDEV(chr);
QIOChannelSocket *sioc;
+ tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
sioc = qio_channel_socket_new();
tcp_chr_set_client_ioc_name(chr, sioc);
qio_channel_socket_connect_async(sioc, s->addr,
@@ -1307,7 +1342,7 @@ char_socket_get_connected(Object *obj, Error **errp)
{
SocketChardev *s = SOCKET_CHARDEV(obj);
- return s->connected;
+ return s->state == TCP_CHARDEV_STATE_CONNECTED;
}
static void char_socket_class_init(ObjectClass *oc, void *data)
--
2.20.1
- [Qemu-devel] [PATCH v2 02/16] io: add qio_task_wait_thread to join with a background thread, (continued)
- [Qemu-devel] [PATCH v2 02/16] io: add qio_task_wait_thread to join with a background thread, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 03/16] chardev: fix validation of options for QMP created chardevs, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 04/16] chardev: forbid 'reconnect' option with server sockets, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 05/16] chardev: forbid 'wait' option with client sockets, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 06/16] chardev: remove many local variables in qemu_chr_parse_socket, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 12/16] chardev: honour the reconnect setting in tcp_chr_wait_connected, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 07/16] chardev: ensure qemu_chr_parse_compat reports missing driver error, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 09/16] chardev: split tcp_chr_wait_connected into two methods, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 08/16] chardev: remove unused 'sioc' variable & cleanup paths, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 10/16] chardev: split up qmp_chardev_open_socket connection code, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 11/16] chardev: use a state machine for socket connection state,
Daniel P . Berrangé <=
- [Qemu-devel] [PATCH v2 13/16] chardev: disallow TLS/telnet/websocket with tcp_chr_wait_connected, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 14/16] chardev: fix race with client connections in tcp_chr_wait_connected, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 15/16] tests: expand coverage of socket chardev test, Daniel P . Berrangé, 2019/01/23
- [Qemu-devel] [PATCH v2 16/16] chardev: ensure termios is fully initialized, Daniel P . Berrangé, 2019/01/23