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",
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 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>
© 2016 - 2026 Red Hat, Inc.