Message ID | 1457544504-8548-15-git-send-email-berrange@redhat.com |
---|---|
State | New |
Headers | show |
On 09/03/2016 18:28, Daniel P. Berrange wrote: > @@ -3131,7 +3118,9 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp) > if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) { > goto fail; > } > - qemu_chr_finish_socket_connection(chr, sioc); > + s->listen_ioc = sioc; > + s->listen_tag = qio_channel_add_watch( > + QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL); > } else if (s->reconnect_time) { > qio_channel_socket_connect_async(sioc, s->addr, > qemu_chr_socket_connected, Aha, yes, this could be it. If you move WSAEventSelect to qio_channel_set_blocking, the previous patch will probably become unnecessary. Paolo
On Wed, Mar 09, 2016 at 06:49:43PM +0100, Paolo Bonzini wrote: > > > On 09/03/2016 18:28, Daniel P. Berrange wrote: > > @@ -3131,7 +3118,9 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp) > > if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) { > > goto fail; > > } > > - qemu_chr_finish_socket_connection(chr, sioc); > > + s->listen_ioc = sioc; > > + s->listen_tag = qio_channel_add_watch( > > + QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL); > > } else if (s->reconnect_time) { > > qio_channel_socket_connect_async(sioc, s->addr, > > qemu_chr_socket_connected, > > Aha, yes, this could be it. If you move WSAEventSelect to > qio_channel_set_blocking, the previous patch will probably become > unnecessary. Yes, I've tested that and you're correct, so I've dropped the previous patch. Regards, Daniel
diff --git a/qemu-char.c b/qemu-char.c index 18890f7..917750b 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3091,20 +3091,6 @@ static void tcp_chr_close(CharDriverState *chr) qemu_chr_be_event(chr, CHR_EVENT_CLOSED); } -static void qemu_chr_finish_socket_connection(CharDriverState *chr, - QIOChannelSocket *sioc) -{ - TCPCharDriver *s = chr->opaque; - - if (s->is_listen) { - s->listen_ioc = sioc; - s->listen_tag = qio_channel_add_watch( - QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL); - } else { - tcp_chr_new_client(chr, sioc); - object_unref(OBJECT(sioc)); - } -} static void qemu_chr_socket_connected(Object *src, Error *err, void *opaque) { @@ -3119,7 +3105,8 @@ static void qemu_chr_socket_connected(Object *src, Error *err, void *opaque) } s->connect_err_reported = false; - qemu_chr_finish_socket_connection(chr, sioc); + tcp_chr_new_client(chr, sioc); + object_unref(OBJECT(sioc)); } static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp) @@ -3131,7 +3118,9 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp) if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) { goto fail; } - qemu_chr_finish_socket_connection(chr, sioc); + s->listen_ioc = sioc; + s->listen_tag = qio_channel_add_watch( + QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL); } else if (s->reconnect_time) { qio_channel_socket_connect_async(sioc, s->addr, qemu_chr_socket_connected, @@ -3140,7 +3129,8 @@ static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp) if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) { goto fail; } - qemu_chr_finish_socket_connection(chr, sioc); + tcp_chr_new_client(chr, sioc); + object_unref(OBJECT(sioc)); } return true;
The qemu_chr_finish_socket_connection method is multiplexing two different actions into one method. Each caller of it though, only wants one specific action. The code is shorter & clearer if we thus remove the method and just inline the specific actions where needed. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemu-char.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)