[PATCH v2] qemu-option: do not suggest using the delay option

[PATCH v2] qemu-option: do not suggest using the delay option

[Paolo Bonzini]

The "delay" option was a hack that was introduced to allow writing "nodelay".
We are adding a "nodelay" option to be used as "nodelay=on", so recommend it
instead of "delay".

This is quite ugly, but a proper deprecation of "delay"
cannot be done if QEMU starts suggesting it.  Since it's the
only case I opted for this very much ad-hoc patch.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/system/deprecated.rst | 6 ++++++
 util/qemu-option.c         | 6 +++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index fcf0ca4068..cfabe69846 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -134,6 +134,12 @@ Boolean options such as ``share=on``/``share=off`` could be written
 in short form as ``share`` and ``noshare``.  This is now deprecated
 and will cause a warning.
 
+``delay`` option for socket character devices (since 6.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
+rather than ``delay=off``.
+
 ``--enable-fips`` (since 6.0)
 '''''''''''''''''''''''''''''
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 40564a12eb..9678d5b682 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -785,7 +785,11 @@ static const char *get_opt_name_value(const char *params,
             }
             if (!is_help && warn_on_flag) {
                 warn_report("short-form boolean option '%s%s' deprecated", prefix, *name);
-                error_printf("Please use %s=%s instead\n", *name, *value);
+                if (g_str_equal(*name, "delay")) {
+                    error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
+                } else {
+                    error_printf("Please use %s=%s instead\n", *name, *value);
+                }
             }
         }
     } else {
-- 
2.29.2

Re: [PATCH v2] qemu-option: do not suggest using the delay option

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210304111743.118752-1-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210304111743.118752-1-pbonzini@redhat.com
Subject: [PATCH v2] qemu-option: do not suggest using the delay option

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210304111743.118752-1-pbonzini@redhat.com -> patchew/20210304111743.118752-1-pbonzini@redhat.com
Switched to a new branch 'test'
495c9b0 qemu-option: do not suggest using the delay option

=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#50: FILE: util/qemu-option.c:789:
+                    error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");

total: 1 errors, 0 warnings, 24 lines checked

Commit 495c9b09732a (qemu-option: do not suggest using the delay option) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210304111743.118752-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v2] qemu-option: do not suggest using the delay option

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> The "delay" option was a hack that was introduced to allow writing "nodelay".
> We are adding a "nodelay" option to be used as "nodelay=on", so recommend it
> instead of "delay".
>
> This is quite ugly, but a proper deprecation of "delay"
> cannot be done if QEMU starts suggesting it.  Since it's the
> only case I opted for this very much ad-hoc patch.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  docs/system/deprecated.rst | 6 ++++++
>  util/qemu-option.c         | 6 +++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index fcf0ca4068..cfabe69846 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -134,6 +134,12 @@ Boolean options such as ``share=on``/``share=off`` could be written
>  in short form as ``share`` and ``noshare``.  This is now deprecated
>  and will cause a warning.
>  
> +``delay`` option for socket character devices (since 6.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
> +rather than ``delay=off``.
> +
>  ``--enable-fips`` (since 6.0)
>  '''''''''''''''''''''''''''''
>  
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 40564a12eb..9678d5b682 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -785,7 +785,11 @@ static const char *get_opt_name_value(const char *params,
>              }
>              if (!is_help && warn_on_flag) {
>                  warn_report("short-form boolean option '%s%s' deprecated", prefix, *name);
> -                error_printf("Please use %s=%s instead\n", *name, *value);
> +                if (g_str_equal(*name, "delay")) {
> +                    error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
> +                } else {
> +                    error_printf("Please use %s=%s instead\n", *name, *value);
> +                }
>              }
>          }
>      } else {

I agree it's ugly, but I don't have better ideas, and it'll go away
eventually.

The warning gets emitted for any QemuOpts parameter named "delay" used
without a value, not just socket chardev's parameter.  Could result in
somewhat misleading warnings, but trying to avoid them seems a waste of
our time.

Should we mention this in the commit message?  Or even in a code
comment?  Up do you.

Reviewed-by: Markus Armbruster <armbru@redhat.com>