All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] chardev: add nodelay option
@ 2021-03-03 12:32 Paolo Bonzini
  2021-03-03 13:24 ` Markus Armbruster
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2021-03-03 12:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

The "delay" option was introduced as a way to enable Nagle's algorithm
with ",nodelay".  Since the short form for boolean options has now been
deprecated, introduce a more properly named "nodelay" option.  The "delay"
option remains as an undocumented option.

"delay" and "nodelay" are mutually exclusive.  Because the check is
done at consumption time, the code also rejects them if one of the
two is specified via -set.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/char-socket.c | 13 +++++++++++--
 gdbstub.c             |  2 +-
 qemu-options.hx       | 14 +++++++-------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 06a37c0cc8..c8bced76b7 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1472,8 +1472,17 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
     sock = backend->u.socket.data = g_new0(ChardevSocket, 1);
     qemu_chr_parse_common(opts, qapi_ChardevSocket_base(sock));
 
-    sock->has_nodelay = qemu_opt_get(opts, "delay");
-    sock->nodelay = !qemu_opt_get_bool(opts, "delay", true);
+    if (qemu_opt_get(opts, "delay") && qemu_opt_get(opts, "nodelay")) {
+        error_setg(errp, "'delay' and 'nodelay' are mutually exclusive");
+        return;
+    }
+    sock->has_nodelay =
+        qemu_opt_get(opts, "delay") ||
+        qemu_opt_get(opts, "nodelay");
+    sock->nodelay =
+        !qemu_opt_get_bool(opts, "delay", true) ||
+        qemu_opt_get_bool(opts, "nodelay", false);
+
     /*
      * We have different default to QMP for 'server', hence
      * we can't just check for existence of 'server'
diff --git a/gdbstub.c b/gdbstub.c
index 3ee40479b6..16d7c8f534 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3505,7 +3505,7 @@ int gdbserver_start(const char *device)
         if (strstart(device, "tcp:", NULL)) {
             /* enforce required TCP attributes */
             snprintf(gdbstub_device_name, sizeof(gdbstub_device_name),
-                     "%s,wait=off,delay=off,server=on", device);
+                     "%s,wait=off,nodelay=on,server=on", device);
             device = gdbstub_device_name;
         }
 #ifndef _WIN32
diff --git a/qemu-options.hx b/qemu-options.hx
index 252db9357c..90801286c6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3033,7 +3033,7 @@ DEFHEADING(Character device options:)
 DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
     "-chardev help\n"
     "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
-    "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,delay=on|off][,reconnect=seconds]\n"
+    "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off][,reconnect=seconds]\n"
     "         [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
     "         [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
     "-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
@@ -3184,7 +3184,7 @@ The available backends are:
 
     TCP and unix socket options are given below:
 
-    ``TCP options: port=port[,host=host][,to=to][,ipv4=on|off][,ipv6=on|off][,delay=on|off]``
+    ``TCP options: port=port[,host=host][,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]``
         ``host`` for a listening socket specifies the local address to
         be bound. For a connecting socket species the remote host to
         connect to. ``host`` is optional for listening sockets. If not
@@ -3204,7 +3204,7 @@ The available backends are:
         or IPv6 must be used. If neither is specified the socket may
         use either protocol.
 
-        ``delay=on|off`` disables the Nagle algorithm.
+        ``nodelay=on|off`` disables the Nagle algorithm.
 
     ``unix options: path=path[,abstract=on|off][,tight=on|off]``
         ``path`` specifies the local path of the unix socket. ``path``
@@ -3593,13 +3593,13 @@ SRST
         ``telnet options:``
             localhost 5555
 
-    ``tcp:[host]:port[,server=on|off][,wait=on|off][,delay=on|off][,reconnect=seconds]``
+    ``tcp:[host]:port[,server=on|off][,wait=on|off][,nodelay=on|off][,reconnect=seconds]``
         The TCP Net Console has two modes of operation. It can send the
         serial I/O to a location or wait for a connection from a
         location. By default the TCP Net Console is sent to host at the
         port. If you use the ``server=on`` option QEMU will wait for a client
         socket application to connect to the port before continuing,
-        unless the ``wait=on|off`` option was specified. The ``delay=on|off``
+        unless the ``wait=on|off`` option was specified. The ``nodelay=on|off``
         option disables the Nagle buffering algorithm. The ``reconnect=on``
         option only applies if ``server=no`` is set, if the connection goes
         down it will attempt to reconnect at the given interval. If host
@@ -3616,7 +3616,7 @@ SRST
         ``Example to not wait and listen on ip 192.168.0.100 port 4444``
             -serial tcp:192.168.0.100:4444,server=on,wait=off
 
-    ``telnet:host:port[,server=on|off][,wait=on|off][,delay=on|off]``
+    ``telnet:host:port[,server=on|off][,wait=on|off][,nodelay=on|off]``
         The telnet protocol is used instead of raw tcp sockets. The
         options work the same as if you had specified ``-serial tcp``.
         The difference is that the port acts like a telnet server or
@@ -3626,7 +3626,7 @@ SRST
         you do it with Control-] and then type "send break" followed by
         pressing the enter key.
 
-    ``websocket:host:port,server=on[,wait=on|off][,delay=on|off]``
+    ``websocket:host:port,server=on[,wait=on|off][,nodelay=on|off]``
         The WebSocket protocol is used instead of raw tcp socket. The
         port acts as a WebSocket server. Client mode is not supported.
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] chardev: add nodelay option
  2021-03-03 12:32 [PATCH v2] chardev: add nodelay option Paolo Bonzini
@ 2021-03-03 13:24 ` Markus Armbruster
  2021-03-03 13:31   ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2021-03-03 13:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Paolo Bonzini <pbonzini@redhat.com> writes:

> The "delay" option was introduced as a way to enable Nagle's algorithm
> with ",nodelay".  Since the short form for boolean options has now been
> deprecated, introduce a more properly named "nodelay" option.  The "delay"
> option remains as an undocumented option.
>
> "delay" and "nodelay" are mutually exclusive.  Because the check is
> done at consumption time, the code also rejects them if one of the
> two is specified via -set.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  chardev/char-socket.c | 13 +++++++++++--
>  gdbstub.c             |  2 +-
>  qemu-options.hx       | 14 +++++++-------
>  3 files changed, 19 insertions(+), 10 deletions(-)

I believe this is
Based-on: <20210226080526.651705-1-pbonzini@redhat.com>

>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 06a37c0cc8..c8bced76b7 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1472,8 +1472,17 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>      sock = backend->u.socket.data = g_new0(ChardevSocket, 1);
>      qemu_chr_parse_common(opts, qapi_ChardevSocket_base(sock));
>  
> -    sock->has_nodelay = qemu_opt_get(opts, "delay");
> -    sock->nodelay = !qemu_opt_get_bool(opts, "delay", true);
> +    if (qemu_opt_get(opts, "delay") && qemu_opt_get(opts, "nodelay")) {
> +        error_setg(errp, "'delay' and 'nodelay' are mutually exclusive");
> +        return;
> +    }
> +    sock->has_nodelay =
> +        qemu_opt_get(opts, "delay") ||
> +        qemu_opt_get(opts, "nodelay");
> +    sock->nodelay =
> +        !qemu_opt_get_bool(opts, "delay", true) ||
> +        qemu_opt_get_bool(opts, "nodelay", false);
> +
>      /*
>       * We have different default to QMP for 'server', hence
>       * we can't just check for existence of 'server'

$ qemu-system-x86_64 -chardev socket,id=chr0,path=sock,nodelay=on
qemu-system-x86_64: -chardev socket,id=chr0,path=sock,nodelay=on: Invalid parameter 'nodelay'

You forgot to update qemu_chardev_opts:

   diff --git a/chardev/char.c b/chardev/char.c
   index 288efebd12..e6128c046f 100644
   --- a/chardev/char.c
   +++ b/chardev/char.c
   @@ -864,6 +864,9 @@ QemuOptsList qemu_chardev_opts = {
            },{
                .name = "server",
                .type = QEMU_OPT_BOOL,
   +        },{
   +            .name = "nodelay",
   +            .type = QEMU_OPT_BOOL,
            },{
                .name = "delay",
                .type = QEMU_OPT_BOOL,

[...]



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] chardev: add nodelay option
  2021-03-03 13:24 ` Markus Armbruster
@ 2021-03-03 13:31   ` Paolo Bonzini
  2021-03-03 13:51     ` Markus Armbruster
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2021-03-03 13:31 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

On 03/03/21 14:24, Markus Armbruster wrote:
> $ qemu-system-x86_64 -chardev socket,id=chr0,path=sock,nodelay=on
> qemu-system-x86_64: -chardev socket,id=chr0,path=sock,nodelay=on: Invalid parameter 'nodelay'
> 
> You forgot to update qemu_chardev_opts:
> 
>     diff --git a/chardev/char.c b/chardev/char.c
>     index 288efebd12..e6128c046f 100644
>     --- a/chardev/char.c
>     +++ b/chardev/char.c
>     @@ -864,6 +864,9 @@ QemuOptsList qemu_chardev_opts = {
>              },{
>                  .name = "server",
>                  .type = QEMU_OPT_BOOL,
>     +        },{
>     +            .name = "nodelay",
>     +            .type = QEMU_OPT_BOOL,
>              },{
>                  .name = "delay",
>                  .type = QEMU_OPT_BOOL,

Well, I forgot to commit it.  But the outcome is the same.  Thanks. :(

Paolo



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] chardev: add nodelay option
  2021-03-03 13:31   ` Paolo Bonzini
@ 2021-03-03 13:51     ` Markus Armbruster
  0 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2021-03-03 13:51 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 03/03/21 14:24, Markus Armbruster wrote:
>> $ qemu-system-x86_64 -chardev socket,id=chr0,path=sock,nodelay=on
>> qemu-system-x86_64: -chardev socket,id=chr0,path=sock,nodelay=on: Invalid parameter 'nodelay'
>> 
>> You forgot to update qemu_chardev_opts:
>> 
>>     diff --git a/chardev/char.c b/chardev/char.c
>>     index 288efebd12..e6128c046f 100644
>>     --- a/chardev/char.c
>>     +++ b/chardev/char.c
>>     @@ -864,6 +864,9 @@ QemuOptsList qemu_chardev_opts = {
>>              },{
>>                  .name = "server",
>>                  .type = QEMU_OPT_BOOL,
>>     +        },{
>>     +            .name = "nodelay",
>>     +            .type = QEMU_OPT_BOOL,
>>              },{
>>                  .name = "delay",
>>                  .type = QEMU_OPT_BOOL,
>
> Well, I forgot to commit it.  But the outcome is the same.  Thanks. :(

Happens to the best of us :)

I'm glad I didn't accuse you of forgetting to test!



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-03 13:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 12:32 [PATCH v2] chardev: add nodelay option Paolo Bonzini
2021-03-03 13:24 ` Markus Armbruster
2021-03-03 13:31   ` Paolo Bonzini
2021-03-03 13:51     ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.