[PATCH] audio: add help option (?) for -audiodev

[PATCH] audio: add help option (?) for -audiodev

[Claudio Fontana]

add a simple help option for -audiodev, so users can do

qemu -audiodev ?

to get the list of drivers available.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 audio/audio.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 4f4bb10cce..bd8c18c3cd 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -32,6 +32,7 @@
 #include "qapi/qapi-visit-audio.h"
 #include "qemu/cutils.h"
 #include "qemu/module.h"
+#include "qemu/help_option.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/replay.h"
 #include "sysemu/runstate.h"
@@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
     }
 }
 
+static void audio_help(void)
+{
+    int i;
+
+    printf("Available audiodev types:\n");
+    printf("none\n");
+
+    for (i = 0; audio_prio_list[i]; i++) {
+        audio_driver *driver = audio_driver_lookup(audio_prio_list[i]);
+        if (driver) {
+            printf("%s\n", driver->name);
+        }
+    }
+}
+
 void audio_parse_option(const char *opt)
 {
     Audiodev *dev = NULL;
 
+    if (is_help_option(opt)) {
+        audio_help();
+        exit(0);
+    }
     Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
     visit_type_Audiodev(v, NULL, &dev, &error_fatal);
     visit_free(v);
-- 
2.26.2

Re: [PATCH] audio: add help option (?) for -audiodev

[Markus Armbruster]

Claudio Fontana <cfontana@suse.de> writes:

> add a simple help option for -audiodev, so users can do
>
> qemu -audiodev ?

The preferred form is actually '-audiodev help'.  The other one is
deprecated.  Recommend to stay away from it even in commit messages.

>
> to get the list of drivers available.
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  audio/audio.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 4f4bb10cce..bd8c18c3cd 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -32,6 +32,7 @@
>  #include "qapi/qapi-visit-audio.h"
>  #include "qemu/cutils.h"
>  #include "qemu/module.h"
> +#include "qemu/help_option.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/replay.h"
>  #include "sysemu/runstate.h"
> @@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
>      }
>  }
>  
> +static void audio_help(void)
> +{
> +    int i;
> +
> +    printf("Available audiodev types:\n");
> +    printf("none\n");
> +
> +    for (i = 0; audio_prio_list[i]; i++) {
> +        audio_driver *driver = audio_driver_lookup(audio_prio_list[i]);
> +        if (driver) {
> +            printf("%s\n", driver->name);
> +        }
> +    }
> +}
> +
>  void audio_parse_option(const char *opt)
>  {
>      Audiodev *dev = NULL;
>  
> +    if (is_help_option(opt)) {
> +        audio_help();
> +        exit(0);
> +    }
>      Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
>      visit_type_Audiodev(v, NULL, &dev, &error_fatal);
>      visit_free(v);

Re: [PATCH] audio: add help option (?) for -audiodev

[Daniel P. Berrangé]

On Wed, Sep 07, 2022 at 05:06:36PM +0200, Markus Armbruster wrote:
> Claudio Fontana <cfontana@suse.de> writes:
> 
> > add a simple help option for -audiodev, so users can do
> >
> > qemu -audiodev ?
> 
> The preferred form is actually '-audiodev help'.  The other one is
> deprecated.  Recommend to stay away from it even in commit messages.

We introduced 'help' many many years ago, but don't thing we
ever formally deprecated '?'.  Should we do so and aim to
remove it, or are we happy to keep '?' forever, despite it
tripping up shell filename expansion with single char filenames.

(tangential to this patch, NOT a request to fix something in v2)

> >
> > to get the list of drivers available.
> >
> > Signed-off-by: Claudio Fontana <cfontana@suse.de>
> > ---
> >  audio/audio.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)

Update qemu-options.hx ?


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] audio: add help option (?) for -audiodev

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On Wed, 7 Sep 2022, Daniel P. Berrangé wrote:
> On Wed, Sep 07, 2022 at 05:06:36PM +0200, Markus Armbruster wrote:
>> Claudio Fontana <cfontana@suse.de> writes:
>>
>>> add a simple help option for -audiodev, so users can do
>>>
>>> qemu -audiodev ?
>>
>> The preferred form is actually '-audiodev help'.  The other one is
>> deprecated.  Recommend to stay away from it even in commit messages.
>
> We introduced 'help' many many years ago, but don't thing we
> ever formally deprecated '?'.  Should we do so and aim to
> remove it, or are we happy to keep '?' forever, despite it
> tripping up shell filename expansion with single char filenames.

Had this conversation before and I think we agreed to keep ? as a 
convenient shorthand even if help is preferred so it should not be 
deprecated but you can keep it from appearing in docs to advertise help as 
the preferred option. (\? is still shorter to type than help if you're 
worried about shell expansion which is rarely a problem in practice)

Regards,
BALATON Zoltan

> (tangential to this patch, NOT a request to fix something in v2)
>
>>>
>>> to get the list of drivers available.
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>> ---
>>>  audio/audio.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>
> Update qemu-options.hx ?
>
>
> With regards,
> Daniel
>

Re: [PATCH] audio: add help option (?) for -audiodev

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, Sep 07, 2022 at 05:06:36PM +0200, Markus Armbruster wrote:
>> Claudio Fontana <cfontana@suse.de> writes:
>> 
>> > add a simple help option for -audiodev, so users can do
>> >
>> > qemu -audiodev ?
>> 
>> The preferred form is actually '-audiodev help'.  The other one is
>> deprecated.  Recommend to stay away from it even in commit messages.
>
> We introduced 'help' many many years ago, but don't thing we
> ever formally deprecated '?'.

is_help_option()'s function comment says "'?' is deprecated".  Goes back
to

commit c8057f951d64de93bfd01569c0a725baa9f94372
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Thu Aug 2 13:45:54 2012 +0100

    Support 'help' as a synonym for '?' in command line options
    
    For command line options which permit '?' meaning 'please list the
    permitted values', add support for 'help' as a synonym, by abstracting
    the check out into a helper function.
    
    This change means that in some cases where we were being lazy in
    our string parsing, "?junk" will now be rejected as an invalid option
    rather than being (undocumentedly) treated the same way as "?".
    
    Update the documentation to use 'help' rather than '?', since '?'
    is a shell metacharacter and thus prone to fail confusingly if there
    is a single character filename in the current working directory and
    the '?' has not been escaped. It's therefore better to steer users
    towards 'help', though '?' is retained for backwards compatibility.
    
    We do not, however, update the output of the system emulator's -help
    (or any documentation autogenerated from the qemu-options.hx which
    is the source of the -help text) because libvirt parses our -help
    output and will break. At a later date when QEMU provides a better
    interface so libvirt can avoid having to do this, we can update the
    -help text too.
    
    Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
    Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

The more formal deprecation process we use today didn't exist in 2012.
Looks like we simply de-documented '?'.

Note that output of -help has been updated since.

>                                Should we do so and aim to
> remove it, or are we happy to keep '?' forever, despite it
> tripping up shell filename expansion with single char filenames.

I'm not sure a belated formal notice of deprecation would be useful.

Emitting a warning might be.  Would have to be done at roughly 20 call
sites, I guess.

I'm cool with removing a feature that has been undocumented for a
decade.  However, experience tells us that every feature on the chopping
block will find a champion, and I believe this one is still around
simply because it's not worth a fight.

[...]

Re: [PATCH] audio: add help option (?) for -audiodev

[Claudio Fontana]

On 9/7/22 17:06, Markus Armbruster wrote:
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> add a simple help option for -audiodev, so users can do
>>
>> qemu -audiodev ?
> 
> The preferred form is actually '-audiodev help'.  The other one is
> deprecated.  Recommend to stay away from it even in commit messages.

I have no problem mentioning "help" instead of "?" in the commit message (or both).

The function used (is_help_option) checks for both, which seems the best way to me.

Ciao

C

> 
>>
>> to get the list of drivers available.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  audio/audio.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 4f4bb10cce..bd8c18c3cd 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -32,6 +32,7 @@
>>  #include "qapi/qapi-visit-audio.h"
>>  #include "qemu/cutils.h"
>>  #include "qemu/module.h"
>> +#include "qemu/help_option.h"
>>  #include "sysemu/sysemu.h"
>>  #include "sysemu/replay.h"
>>  #include "sysemu/runstate.h"
>> @@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
>>      }
>>  }
>>  
>> +static void audio_help(void)
>> +{
>> +    int i;
>> +
>> +    printf("Available audiodev types:\n");
>> +    printf("none\n");
>> +
>> +    for (i = 0; audio_prio_list[i]; i++) {
>> +        audio_driver *driver = audio_driver_lookup(audio_prio_list[i]);
>> +        if (driver) {
>> +            printf("%s\n", driver->name);
>> +        }
>> +    }
>> +}
>> +
>>  void audio_parse_option(const char *opt)
>>  {
>>      Audiodev *dev = NULL;
>>  
>> +    if (is_help_option(opt)) {
>> +        audio_help();
>> +        exit(0);
>> +    }
>>      Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
>>      visit_type_Audiodev(v, NULL, &dev, &error_fatal);
>>      visit_free(v);
>