| Message ID | 20190115145256.9593-4-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | chardev: refactoring & many bugfixes related tcp_chr_wait_connected | expand |
On Tue, Jan 15, 2019 at 6:53 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > The 'wait'/'nowait' parameter is used to tell server sockets whether to > block until a client is accepted during initialization. Client chardevs > have always silently ignored this option. Various tests were mistakenly > passing this option for their client chardevs. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > chardev/char-socket.c | 12 +++++++++++- > tests/ivshmem-test.c | 2 +- > tests/libqtest.c | 4 ++-- > tests/test-filter-redirector.c | 4 ++-- > 4 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index 4570755adf..233f16f72d 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -1047,6 +1047,12 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock, > error_setg(errp, "%s", "Websocket client is not implemented"); > return false; > } > + if (sock->has_wait) { > + error_setg(errp, "%s", > + "'wait' option is incompatible with " > + "socket in client connect mode"); > + return false; > + } > } > > return true; > @@ -1220,7 +1226,11 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, > sock->tn3270 = is_tn3270; > sock->has_websocket = true; > sock->websocket = is_websock; > - sock->has_wait = true; > + /* > + * We have different default to QMP for 'wait' when 'server' > + * is set, hence we can't just check for existance of 'wait' > + */ > + sock->has_wait = qemu_opt_find(opts, "wait") || is_listen; > sock->wait = is_waitconnect; > sock->has_reconnect = qemu_opt_find(opts, "reconnect"); > sock->reconnect = reconnect; > diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c > index fe5eb304b1..faffc1c624 100644 > --- a/tests/ivshmem-test.c > +++ b/tests/ivshmem-test.c > @@ -293,7 +293,7 @@ static void *server_thread(void *data) > > static void setup_vm_with_server(IVState *s, int nvectors, bool msi) > { > - char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s,nowait " > + char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s " > "-device ivshmem%s,chardev=chr0,vectors=%d", > tmpserver, > msi ? "-doorbell" : ",size=1M,msi=off", > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 55750dd68d..79bcb24b1c 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -232,9 +232,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args) > qtest_add_abrt_handler(kill_qemu_hook_func, s); > > command = g_strdup_printf("exec %s " > - "-qtest unix:%s,nowait " > + "-qtest unix:%s " > "-qtest-log %s " > - "-chardev socket,path=%s,nowait,id=char0 " > + "-chardev socket,path=%s,id=char0 " > "-mon chardev=char0,mode=control " > "-machine accel=qtest " > "-display none " > diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c > index 9ca9feabf8..6dc21dd4fb 100644 > --- a/tests/test-filter-redirector.c > +++ b/tests/test-filter-redirector.c > @@ -96,7 +96,7 @@ static void test_redirector_tx(void) > "-device %s,netdev=qtest-bn0,id=qtest-e0 " > "-chardev socket,id=redirector0,path=%s,server,nowait " > "-chardev socket,id=redirector1,path=%s,server,nowait " > - "-chardev socket,id=redirector2,path=%s,nowait " > + "-chardev socket,id=redirector2,path=%s " > "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0," > "queue=tx,outdev=redirector0 " > "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0," > @@ -166,7 +166,7 @@ static void test_redirector_rx(void) > "-device %s,netdev=qtest-bn0,id=qtest-e0 " > "-chardev socket,id=redirector0,path=%s,server,nowait " > "-chardev socket,id=redirector1,path=%s,server,nowait " > - "-chardev socket,id=redirector2,path=%s,nowait " > + "-chardev socket,id=redirector2,path=%s " > "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0," > "queue=rx,indev=redirector0 " > "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0," > -- > 2.20.1 >
On 2019-01-15 15:52, Daniel P. Berrangé wrote: > The 'wait'/'nowait' parameter is used to tell server sockets whether to > block until a client is accepted during initialization. Client chardevs > have always silently ignored this option. Various tests were mistakenly > passing this option for their client chardevs. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > chardev/char-socket.c | 12 +++++++++++- > tests/ivshmem-test.c | 2 +- > tests/libqtest.c | 4 ++-- > tests/test-filter-redirector.c | 4 ++-- > 4 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index 4570755adf..233f16f72d 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -1047,6 +1047,12 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock, > error_setg(errp, "%s", "Websocket client is not implemented"); > return false; > } > + if (sock->has_wait) { > + error_setg(errp, "%s", > + "'wait' option is incompatible with " > + "socket in client connect mode"); > + return false; > + } > } > > return true; > @@ -1220,7 +1226,11 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, > sock->tn3270 = is_tn3270; > sock->has_websocket = true; > sock->websocket = is_websock; > - sock->has_wait = true; > + /* > + * We have different default to QMP for 'wait' when 'server' > + * is set, hence we can't just check for existance of 'wait' > + */ > + sock->has_wait = qemu_opt_find(opts, "wait") || is_listen; > sock->wait = is_waitconnect; > sock->has_reconnect = qemu_opt_find(opts, "reconnect"); > sock->reconnect = reconnect; > diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c > index fe5eb304b1..faffc1c624 100644 > --- a/tests/ivshmem-test.c > +++ b/tests/ivshmem-test.c > @@ -293,7 +293,7 @@ static void *server_thread(void *data) > > static void setup_vm_with_server(IVState *s, int nvectors, bool msi) > { > - char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s,nowait " > + char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s " > "-device ivshmem%s,chardev=chr0,vectors=%d", > tmpserver, > msi ? "-doorbell" : ",size=1M,msi=off", I think there will be a conflict with the patch 'Remove deprecated "ivshmem" legacy device' that is currently in Michael's PULL request ... could you please rebase this patch in a day or two once that PULL has been merged? Apart from that, patch looks fine to me. Acked-by: Thomas Huth <thuth@redhat.com>
diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 4570755adf..233f16f72d 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -1047,6 +1047,12 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock, error_setg(errp, "%s", "Websocket client is not implemented"); return false; } + if (sock->has_wait) { + error_setg(errp, "%s", + "'wait' option is incompatible with " + "socket in client connect mode"); + return false; + } } return true; @@ -1220,7 +1226,11 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, sock->tn3270 = is_tn3270; sock->has_websocket = true; sock->websocket = is_websock; - sock->has_wait = true; + /* + * We have different default to QMP for 'wait' when 'server' + * is set, hence we can't just check for existance of 'wait' + */ + sock->has_wait = qemu_opt_find(opts, "wait") || is_listen; sock->wait = is_waitconnect; sock->has_reconnect = qemu_opt_find(opts, "reconnect"); sock->reconnect = reconnect; diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c index fe5eb304b1..faffc1c624 100644 --- a/tests/ivshmem-test.c +++ b/tests/ivshmem-test.c @@ -293,7 +293,7 @@ static void *server_thread(void *data) static void setup_vm_with_server(IVState *s, int nvectors, bool msi) { - char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s,nowait " + char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s " "-device ivshmem%s,chardev=chr0,vectors=%d", tmpserver, msi ? "-doorbell" : ",size=1M,msi=off", diff --git a/tests/libqtest.c b/tests/libqtest.c index 55750dd68d..79bcb24b1c 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -232,9 +232,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args) qtest_add_abrt_handler(kill_qemu_hook_func, s); command = g_strdup_printf("exec %s " - "-qtest unix:%s,nowait " + "-qtest unix:%s " "-qtest-log %s " - "-chardev socket,path=%s,nowait,id=char0 " + "-chardev socket,path=%s,id=char0 " "-mon chardev=char0,mode=control " "-machine accel=qtest " "-display none " diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c index 9ca9feabf8..6dc21dd4fb 100644 --- a/tests/test-filter-redirector.c +++ b/tests/test-filter-redirector.c @@ -96,7 +96,7 @@ static void test_redirector_tx(void) "-device %s,netdev=qtest-bn0,id=qtest-e0 " "-chardev socket,id=redirector0,path=%s,server,nowait " "-chardev socket,id=redirector1,path=%s,server,nowait " - "-chardev socket,id=redirector2,path=%s,nowait " + "-chardev socket,id=redirector2,path=%s " "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0," "queue=tx,outdev=redirector0 " "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0," @@ -166,7 +166,7 @@ static void test_redirector_rx(void) "-device %s,netdev=qtest-bn0,id=qtest-e0 " "-chardev socket,id=redirector0,path=%s,server,nowait " "-chardev socket,id=redirector1,path=%s,server,nowait " - "-chardev socket,id=redirector2,path=%s,nowait " + "-chardev socket,id=redirector2,path=%s " "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0," "queue=rx,indev=redirector0 " "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
The 'wait'/'nowait' parameter is used to tell server sockets whether to block until a client is accepted during initialization. Client chardevs have always silently ignored this option. Various tests were mistakenly passing this option for their client chardevs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- chardev/char-socket.c | 12 +++++++++++- tests/ivshmem-test.c | 2 +- tests/libqtest.c | 4 ++-- tests/test-filter-redirector.c | 4 ++-- 4 files changed, 16 insertions(+), 6 deletions(-)