Re: [PATCH] chardev: add nodelay option

[Paolo Bonzini]

On 02/03/21 12:39, Daniel P. Berrangé wrote:
On Tue, Mar 02, 2021 at 12:04:44PM +0100, Paolo Bonzini wrote:
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.
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
  chardev/char-socket.c |  9 +++++++--
  gdbstub.c             |  2 +-
  qemu-options.hx       | 14 +++++++-------
  3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 06a37c0cc8..73a7afe5a0 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1472,8 +1472,13 @@ 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);
+    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 should raise an explicit error if both options are present,
otherwise you get into a debate about prioritization with nonsense
such as

    -chardev socket,.....,delay=on,nodelay=on

Good point, we can squash this in:

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 73a7afe5a0..c8bced76b7 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1472,6 +1472,10 @@ 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));

+    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");

Paolo