[PATCH] vl: Print display options for -display help

Akihiko Odaki posted 1 patch 1 year, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231214-help-v1-1-23823ac5a023@daynix.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
include/ui/console.h |  1 -
system/vl.c          | 11 ++++++-----
ui/console.c         | 20 --------------------
3 files changed, 6 insertions(+), 26 deletions(-)
[PATCH] vl: Print display options for -display help
Posted by Akihiko Odaki 1 year, 11 months ago
-display lists display backends, but does not tell their options.
Use the help messages from qemu-options.def, which include the list of
options.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 include/ui/console.h |  1 -
 system/vl.c          | 11 ++++++-----
 ui/console.c         | 20 --------------------
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index a4a49ffc640c..58f757350647 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -470,7 +470,6 @@ bool qemu_display_find_default(DisplayOptions *opts);
 void qemu_display_early_init(DisplayOptions *opts);
 void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
 const char *qemu_display_get_vc(DisplayOptions *opts);
-void qemu_display_help(void);
 
 /* vnc.c */
 void vnc_display_init(const char *id, Error **errp);
diff --git a/system/vl.c b/system/vl.c
index 2bcd9efb9a64..f9656667ee54 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -915,16 +915,17 @@ enum {
 
 typedef struct QEMUOption {
     const char *name;
+    const char *help;
     int flags;
     int index;
     uint32_t arch_mask;
 } QEMUOption;
 
 static const QEMUOption qemu_options[] = {
-    { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
+    { "h", NULL, 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
 
 #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
-    { option, opt_arg, opt_enum, arch_mask },
+    { option, opt_help, opt_arg, opt_enum, arch_mask },
 #define DEFHEADING(text)
 #define ARCHHEADING(text, arch_mask)
 
@@ -1094,10 +1095,10 @@ DisplayOptions *qmp_query_display_options(Error **errp)
     return QAPI_CLONE(DisplayOptions, &dpy);
 }
 
-static void parse_display(const char *p)
+static void parse_display(const char *p, const char *help)
 {
     if (is_help_option(p)) {
-        qemu_display_help();
+        fputs(help, stdout);
         exit(0);
     }
 
@@ -2880,7 +2881,7 @@ void qemu_init(int argc, char **argv)
                 }
                 break;
             case QEMU_OPTION_display:
-                parse_display(optarg);
+                parse_display(optarg, popt->help);
                 break;
             case QEMU_OPTION_nographic:
                 qdict_put_str(machine_opts_dict, "graphics", "off");
diff --git a/ui/console.c b/ui/console.c
index 7db921e3b7d6..6aee5e9a7ffb 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions *opts)
     }
     return vc;
 }
-
-void qemu_display_help(void)
-{
-    int idx;
-
-    printf("Available display backend types:\n");
-    printf("none\n");
-    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
-        if (!dpys[idx]) {
-            Error *local_err = NULL;
-            int rv = ui_module_load(DisplayType_str(idx), &local_err);
-            if (rv < 0) {
-                error_report_err(local_err);
-            }
-        }
-        if (dpys[idx]) {
-            printf("%s\n",  DisplayType_str(dpys[idx]->type));
-        }
-    }
-}

---
base-commit: 4705fc0c8511d073bee4751c3c974aab2b10a970
change-id: 20231214-help-9fa146fc6e95

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>
Re: [PATCH] vl: Print display options for -display help
Posted by Philippe Mathieu-Daudé 1 year, 11 months ago
Hi Akihiko,

On 14/12/23 07:47, Akihiko Odaki wrote:
> -display lists display backends, but does not tell their options.
> Use the help messages from qemu-options.def, which include the list of
> options.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   include/ui/console.h |  1 -
>   system/vl.c          | 11 ++++++-----
>   ui/console.c         | 20 --------------------
>   3 files changed, 6 insertions(+), 26 deletions(-)


> diff --git a/ui/console.c b/ui/console.c
> index 7db921e3b7d6..6aee5e9a7ffb 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions *opts)
>       }
>       return vc;
>   }
> -
> -void qemu_display_help(void)
> -{
> -    int idx;
> -
> -    printf("Available display backend types:\n");
> -    printf("none\n");
> -    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
> -        if (!dpys[idx]) {
> -            Error *local_err = NULL;
> -            int rv = ui_module_load(DisplayType_str(idx), &local_err);
> -            if (rv < 0) {
> -                error_report_err(local_err);
> -            }
> -        }
> -        if (dpys[idx]) {
> -            printf("%s\n",  DisplayType_str(dpys[idx]->type));

Is the "qapi/qapi-commands-ui.h" header still necessary?

> -        }
> -    }
> -}

So we go from:

$ ./qemu-system-aarch64 -display help
Available display backend types:
none
gtk
sdl
curses
cocoa
dbus

to:

$ ./qemu-system-aarch64 -display help
-display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
             [,window-close=on|off]
-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
             [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
             [,show-menubar=on|off]
-display vnc=<display>[,<optargs>]
-display curses[,charset=<encoding>]
-display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
-display dbus[,addr=<dbusaddr>]
              [,gl=on|core|es|off][,rendernode=<file>]
-display cocoa[,show-cursor=on|off][,left-command-key=on|off]
-display none
                 select display backend type
                 The default display is equivalent to
                 "-display gtk"

The latter is indeed more helpful.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Re: [PATCH] vl: Print display options for -display help
Posted by Marc-André Lureau 1 year, 11 months ago
Hi

On Thu, Dec 14, 2023 at 1:29 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Akihiko,
>
> On 14/12/23 07:47, Akihiko Odaki wrote:
> > -display lists display backends, but does not tell their options.
> > Use the help messages from qemu-options.def, which include the list of
> > options.
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> >   include/ui/console.h |  1 -
> >   system/vl.c          | 11 ++++++-----
> >   ui/console.c         | 20 --------------------
> >   3 files changed, 6 insertions(+), 26 deletions(-)
>
>
> > diff --git a/ui/console.c b/ui/console.c
> > index 7db921e3b7d6..6aee5e9a7ffb 100644
> > --- a/ui/console.c
> > +++ b/ui/console.c
> > @@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions *opts)
> >       }
> >       return vc;
> >   }
> > -
> > -void qemu_display_help(void)
> > -{
> > -    int idx;
> > -
> > -    printf("Available display backend types:\n");
> > -    printf("none\n");
> > -    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
> > -        if (!dpys[idx]) {
> > -            Error *local_err = NULL;
> > -            int rv = ui_module_load(DisplayType_str(idx), &local_err);
> > -            if (rv < 0) {
> > -                error_report_err(local_err);
> > -            }
> > -        }
> > -        if (dpys[idx]) {
> > -            printf("%s\n",  DisplayType_str(dpys[idx]->type));
>
> Is the "qapi/qapi-commands-ui.h" header still necessary?
>
> > -        }
> > -    }
> > -}
>
> So we go from:
>
> $ ./qemu-system-aarch64 -display help
> Available display backend types:
> none
> gtk
> sdl
> curses
> cocoa
> dbus

I think this used to show only the available display/ui modules

>
> to:
>
> $ ./qemu-system-aarch64 -display help
> -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>              [,window-close=on|off]
> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>              [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>              [,show-menubar=on|off]
> -display vnc=<display>[,<optargs>]
> -display curses[,charset=<encoding>]
> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
> -display dbus[,addr=<dbusaddr>]
>               [,gl=on|core|es|off][,rendernode=<file>]
> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
> -display none
>                  select display backend type
>                  The default display is equivalent to
>                  "-display gtk"
>
> The latter is indeed more helpful.

While this will provide help for all the modules built.

Not a big deal to me, but probably worth mentioning in the commit message.
Re: [PATCH] vl: Print display options for -display help
Posted by BALATON Zoltan 1 year, 11 months ago
On Thu, 14 Dec 2023, Philippe Mathieu-Daudé wrote:
> Hi Akihiko,
>
> On 14/12/23 07:47, Akihiko Odaki wrote:
>> -display lists display backends, but does not tell their options.
>> Use the help messages from qemu-options.def, which include the list of
>> options.
>> 
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   include/ui/console.h |  1 -
>>   system/vl.c          | 11 ++++++-----
>>   ui/console.c         | 20 --------------------
>>   3 files changed, 6 insertions(+), 26 deletions(-)
>
>
>> diff --git a/ui/console.c b/ui/console.c
>> index 7db921e3b7d6..6aee5e9a7ffb 100644
>> --- a/ui/console.c
>> +++ b/ui/console.c
>> @@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions 
>> *opts)
>>       }
>>       return vc;
>>   }
>> -
>> -void qemu_display_help(void)
>> -{
>> -    int idx;
>> -
>> -    printf("Available display backend types:\n");
>> -    printf("none\n");
>> -    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
>> -        if (!dpys[idx]) {
>> -            Error *local_err = NULL;
>> -            int rv = ui_module_load(DisplayType_str(idx), &local_err);
>> -            if (rv < 0) {
>> -                error_report_err(local_err);
>> -            }
>> -        }
>> -        if (dpys[idx]) {
>> -            printf("%s\n",  DisplayType_str(dpys[idx]->type));
>
> Is the "qapi/qapi-commands-ui.h" header still necessary?
>
>> -        }
>> -    }
>> -}
>
> So we go from:
>
> $ ./qemu-system-aarch64 -display help
> Available display backend types:
> none
> gtk
> sdl
> curses
> cocoa
> dbus
>
> to:
>
> $ ./qemu-system-aarch64 -display help
> -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>            [,window-close=on|off]
> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>            [,show-menubar=on|off]
> -display vnc=<display>[,<optargs>]
> -display curses[,charset=<encoding>]
> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
> -display dbus[,addr=<dbusaddr>]
>             [,gl=on|core|es|off][,rendernode=<file>]
> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
> -display none
>                select display backend type
>                The default display is equivalent to
>                "-display gtk"
>
> The latter is indeed more helpful.

It is more helpful but maybe a bit overwhelming. Would it be possible to 
only print the options with -display cocoa,help similar to how -device 
help lists devices and -device sm501,help lists options for one device? 
Adding info about default to -display help is really helpful though (that 
could also be marked with (default) like in -machine help.

I'm not complaining, thanks for taking care of this so quickly but if it's 
not too difficult to add separate -display cocoa,help and not list options 
in -display help maybe that would be better and more consistent with other 
help options.

Regards,
BALATON Zoltan

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
Re: [PATCH] vl: Print display options for -display help
Posted by Akihiko Odaki 1 year, 11 months ago
On 2023/12/14 22:00, BALATON Zoltan wrote:
> On Thu, 14 Dec 2023, Philippe Mathieu-Daudé wrote:
>> Hi Akihiko,
>>
>> On 14/12/23 07:47, Akihiko Odaki wrote:
>>> -display lists display backends, but does not tell their options.
>>> Use the help messages from qemu-options.def, which include the list of
>>> options.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>>   include/ui/console.h |  1 -
>>>   system/vl.c          | 11 ++++++-----
>>>   ui/console.c         | 20 --------------------
>>>   3 files changed, 6 insertions(+), 26 deletions(-)
>>
>>
>>> diff --git a/ui/console.c b/ui/console.c
>>> index 7db921e3b7d6..6aee5e9a7ffb 100644
>>> --- a/ui/console.c
>>> +++ b/ui/console.c
>>> @@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions 
>>> *opts)
>>>       }
>>>       return vc;
>>>   }
>>> -
>>> -void qemu_display_help(void)
>>> -{
>>> -    int idx;
>>> -
>>> -    printf("Available display backend types:\n");
>>> -    printf("none\n");
>>> -    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
>>> -        if (!dpys[idx]) {
>>> -            Error *local_err = NULL;
>>> -            int rv = ui_module_load(DisplayType_str(idx), &local_err);
>>> -            if (rv < 0) {
>>> -                error_report_err(local_err);
>>> -            }
>>> -        }
>>> -        if (dpys[idx]) {
>>> -            printf("%s\n",  DisplayType_str(dpys[idx]->type));
>>
>> Is the "qapi/qapi-commands-ui.h" header still necessary?
>>
>>> -        }
>>> -    }
>>> -}
>>
>> So we go from:
>>
>> $ ./qemu-system-aarch64 -display help
>> Available display backend types:
>> none
>> gtk
>> sdl
>> curses
>> cocoa
>> dbus
>>
>> to:
>>
>> $ ./qemu-system-aarch64 -display help
>> -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>>            [,window-close=on|off]
>> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>>            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>>            [,show-menubar=on|off]
>> -display vnc=<display>[,<optargs>]
>> -display curses[,charset=<encoding>]
>> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
>> -display dbus[,addr=<dbusaddr>]
>>             [,gl=on|core|es|off][,rendernode=<file>]
>> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
>> -display none
>>                select display backend type
>>                The default display is equivalent to
>>                "-display gtk"
>>
>> The latter is indeed more helpful.
> 
> It is more helpful but maybe a bit overwhelming. Would it be possible to 
> only print the options with -display cocoa,help similar to how -device 
> help lists devices and -device sm501,help lists options for one device? 
> Adding info about default to -display help is really helpful though 
> (that could also be marked with (default) like in -machine help.

It's copied from what qemu-system-aarch64 -h outputs. At least it's less 
overwhelming than qemu-system-aarch64 -h.

> 
> I'm not complaining, thanks for taking care of this so quickly but if 
> it's not too difficult to add separate -display cocoa,help and not list 
> options in -display help maybe that would be better and more consistent 
> with other help options.

Yes, that will require some major refactoring so I'm not going to do 
that for now.

Re: [PATCH] vl: Print display options for -display help
Posted by BALATON Zoltan 1 year, 11 months ago
On Fri, 15 Dec 2023, Akihiko Odaki wrote:
> On 2023/12/14 22:00, BALATON Zoltan wrote:
>> On Thu, 14 Dec 2023, Philippe Mathieu-Daudé wrote:
>>> Hi Akihiko,
>>> 
>>> On 14/12/23 07:47, Akihiko Odaki wrote:
>>>> -display lists display backends, but does not tell their options.
>>>> Use the help messages from qemu-options.def, which include the list of
>>>> options.
>>>> 
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>>   include/ui/console.h |  1 -
>>>>   system/vl.c          | 11 ++++++-----
>>>>   ui/console.c         | 20 --------------------
>>>>   3 files changed, 6 insertions(+), 26 deletions(-)
>>> 
>>> 
>>>> diff --git a/ui/console.c b/ui/console.c
>>>> index 7db921e3b7d6..6aee5e9a7ffb 100644
>>>> --- a/ui/console.c
>>>> +++ b/ui/console.c
>>>> @@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions 
>>>> *opts)
>>>>       }
>>>>       return vc;
>>>>   }
>>>> -
>>>> -void qemu_display_help(void)
>>>> -{
>>>> -    int idx;
>>>> -
>>>> -    printf("Available display backend types:\n");
>>>> -    printf("none\n");
>>>> -    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
>>>> -        if (!dpys[idx]) {
>>>> -            Error *local_err = NULL;
>>>> -            int rv = ui_module_load(DisplayType_str(idx), &local_err);
>>>> -            if (rv < 0) {
>>>> -                error_report_err(local_err);
>>>> -            }
>>>> -        }
>>>> -        if (dpys[idx]) {
>>>> -            printf("%s\n",  DisplayType_str(dpys[idx]->type));
>>> 
>>> Is the "qapi/qapi-commands-ui.h" header still necessary?
>>> 
>>>> -        }
>>>> -    }
>>>> -}
>>> 
>>> So we go from:
>>> 
>>> $ ./qemu-system-aarch64 -display help
>>> Available display backend types:
>>> none
>>> gtk
>>> sdl
>>> curses
>>> cocoa
>>> dbus
>>> 
>>> to:
>>> 
>>> $ ./qemu-system-aarch64 -display help
>>> -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>>>            [,window-close=on|off]
>>> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>>>            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>>>            [,show-menubar=on|off]
>>> -display vnc=<display>[,<optargs>]
>>> -display curses[,charset=<encoding>]
>>> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
>>> -display dbus[,addr=<dbusaddr>]
>>>             [,gl=on|core|es|off][,rendernode=<file>]
>>> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
>>> -display none
>>>                select display backend type
>>>                The default display is equivalent to
>>>                "-display gtk"
>>> 
>>> The latter is indeed more helpful.
>> 
>> It is more helpful but maybe a bit overwhelming. Would it be possible to 
>> only print the options with -display cocoa,help similar to how -device help 
>> lists devices and -device sm501,help lists options for one device? Adding 
>> info about default to -display help is really helpful though (that could 
>> also be marked with (default) like in -machine help.
>
> It's copied from what qemu-system-aarch64 -h outputs. At least it's less 
> overwhelming than qemu-system-aarch64 -h.

This changes what -display help does so if some script depends on that it 
may not be a good idea. Since the same info is already in -help maybe this 
change to add that to -display help as well is not the best solution so 
I'd say drop this patch and leave it as it is for now.

Adding (default) to show default as with -machine help would be useful but 
the default in help seems to be added by preprocessor magic so it's not 
easy to use that in qemu_display_help(). Maybe if a constant would be 
defined with the default value instead of adding it directly to help text 
then that could be used but we have '-vnc some-arguments' as opposed to 
'-display something' for all other casess so if that -vnc option is 
correct and it's not like '-display vnc' then that's not trivial either. 
So I'd say just fotget about this for now as it's not that important so 
may not worth the effort.

>> I'm not complaining, thanks for taking care of this so quickly but if it's 
>> not too difficult to add separate -display cocoa,help and not list options 
>> in -display help maybe that would be better and more consistent with other 
>> help options.
>
> Yes, that will require some major refactoring so I'm not going to do that for 
> now.

I've also looked at that and concluded the same that it would take some 
qapi expert to solve this. It seems the options are parsed into some qapi 
types which may have help to display but I could not find out how that 
works. For device it may be handled in qdev_device_help() but I don't know 
if that would be applicable for display backends and if so how. Maybe 
someone who knows about this could chime in and give some idea how e.g. 
-display gtk,help could be implemented similar to -device somedevice,help. 
(Other than passing through the help text as your patch does and rhen cut 
the relevant part from that with string functions but that's likely not 
the right way to do this.)

Regards,
BALATON Zoltan
Re: [PATCH] vl: Print display options for -display help
Posted by Daniel P. Berrangé 1 year, 11 months ago
On Fri, Dec 15, 2023 at 01:43:37PM +0100, BALATON Zoltan wrote:
> On Fri, 15 Dec 2023, Akihiko Odaki wrote:
> > On 2023/12/14 22:00, BALATON Zoltan wrote:
> > > > So we go from:
> > > > 
> > > > $ ./qemu-system-aarch64 -display help
> > > > Available display backend types:
> > > > none
> > > > gtk
> > > > sdl
> > > > curses
> > > > cocoa
> > > > dbus
> > > > 
> > > > to:
> > > > 
> > > > $ ./qemu-system-aarch64 -display help
> > > > -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
> > > >            [,window-close=on|off]
> > > > -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
> > > >            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
> > > >            [,show-menubar=on|off]
> > > > -display vnc=<display>[,<optargs>]
> > > > -display curses[,charset=<encoding>]
> > > > -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
> > > > -display dbus[,addr=<dbusaddr>]
> > > >             [,gl=on|core|es|off][,rendernode=<file>]
> > > > -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
> > > > -display none
> > > >                select display backend type
> > > >                The default display is equivalent to
> > > >                "-display gtk"
> > > > 
> > > > The latter is indeed more helpful.
> > > 
> > > It is more helpful but maybe a bit overwhelming. Would it be
> > > possible to only print the options with -display cocoa,help similar
> > > to how -device help lists devices and -device sm501,help lists
> > > options for one device? Adding info about default to -display help
> > > is really helpful though (that could also be marked with (default)
> > > like in -machine help.
> > 
> > It's copied from what qemu-system-aarch64 -h outputs. At least it's less
> > overwhelming than qemu-system-aarch64 -h.
> 
> This changes what -display help does so if some script depends on that it
> may not be a good idea. Since the same info is already in -help maybe this
> change to add that to -display help as well is not the best solution so I'd
> say drop this patch and leave it as it is for now.

We consider help output to be only for humans.

No scripts should ever be parsing any QEMU output, as we provide
QMP for automated detection/querying of features.

IOW, if some script is parsing help output we are fine to break
them if it improves QEMU's output for humans in a justiable way.


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] vl: Print display options for -display help
Posted by BALATON Zoltan 1 year, 11 months ago
On Fri, 15 Dec 2023, Daniel P. Berrangé wrote:
> On Fri, Dec 15, 2023 at 01:43:37PM +0100, BALATON Zoltan wrote:
>> On Fri, 15 Dec 2023, Akihiko Odaki wrote:
>>> On 2023/12/14 22:00, BALATON Zoltan wrote:
>>>>> So we go from:
>>>>>
>>>>> $ ./qemu-system-aarch64 -display help
>>>>> Available display backend types:
>>>>> none
>>>>> gtk
>>>>> sdl
>>>>> curses
>>>>> cocoa
>>>>> dbus
>>>>>
>>>>> to:
>>>>>
>>>>> $ ./qemu-system-aarch64 -display help
>>>>> -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>>>>>            [,window-close=on|off]
>>>>> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>>>>>            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>>>>>            [,show-menubar=on|off]
>>>>> -display vnc=<display>[,<optargs>]
>>>>> -display curses[,charset=<encoding>]
>>>>> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
>>>>> -display dbus[,addr=<dbusaddr>]
>>>>>             [,gl=on|core|es|off][,rendernode=<file>]
>>>>> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
>>>>> -display none
>>>>>                select display backend type
>>>>>                The default display is equivalent to
>>>>>                "-display gtk"
>>>>>
>>>>> The latter is indeed more helpful.
>>>>
>>>> It is more helpful but maybe a bit overwhelming. Would it be
>>>> possible to only print the options with -display cocoa,help similar
>>>> to how -device help lists devices and -device sm501,help lists
>>>> options for one device? Adding info about default to -display help
>>>> is really helpful though (that could also be marked with (default)
>>>> like in -machine help.
>>>
>>> It's copied from what qemu-system-aarch64 -h outputs. At least it's less
>>> overwhelming than qemu-system-aarch64 -h.
>>
>> This changes what -display help does so if some script depends on that it
>> may not be a good idea. Since the same info is already in -help maybe this
>> change to add that to -display help as well is not the best solution so I'd
>> say drop this patch and leave it as it is for now.
>
> We consider help output to be only for humans.
>
> No scripts should ever be parsing any QEMU output, as we provide
> QMP for automated detection/querying of features.
>
> IOW, if some script is parsing help output we are fine to break
> them if it improves QEMU's output for humans in a justiable way.

OK fair enough. I'd still like some more consitency with -cpu help and 
-machine help and -device help which all list avalable options and then 
can take help for further help for individual items where applicable. So 
it would be better if -display and -audio could behave the same. (Are 
there any other options that might be missing this?)

Out of curiousicy what's the way to query available display backends for 
programs by QMP then?

Regards,
BALATON Zoltan
Re: [PATCH] vl: Print display options for -display help
Posted by Akihiko Odaki 1 year, 11 months ago
On 2023/12/15 22:07, BALATON Zoltan wrote:
> On Fri, 15 Dec 2023, Daniel P. Berrangé wrote:
>> On Fri, Dec 15, 2023 at 01:43:37PM +0100, BALATON Zoltan wrote:
>>> On Fri, 15 Dec 2023, Akihiko Odaki wrote:
>>>> On 2023/12/14 22:00, BALATON Zoltan wrote:
>>>>>> So we go from:
>>>>>>
>>>>>> $ ./qemu-system-aarch64 -display help
>>>>>> Available display backend types:
>>>>>> none
>>>>>> gtk
>>>>>> sdl
>>>>>> curses
>>>>>> cocoa
>>>>>> dbus
>>>>>>
>>>>>> to:
>>>>>>
>>>>>> $ ./qemu-system-aarch64 -display help
>>>>>> -display 
>>>>>> sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>>>>>>            [,window-close=on|off]
>>>>>> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>>>>>>            
>>>>>> [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>>>>>>            [,show-menubar=on|off]
>>>>>> -display vnc=<display>[,<optargs>]
>>>>>> -display curses[,charset=<encoding>]
>>>>>> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
>>>>>> -display dbus[,addr=<dbusaddr>]
>>>>>>             [,gl=on|core|es|off][,rendernode=<file>]
>>>>>> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
>>>>>> -display none
>>>>>>                select display backend type
>>>>>>                The default display is equivalent to
>>>>>>                "-display gtk"
>>>>>>
>>>>>> The latter is indeed more helpful.
>>>>>
>>>>> It is more helpful but maybe a bit overwhelming. Would it be
>>>>> possible to only print the options with -display cocoa,help similar
>>>>> to how -device help lists devices and -device sm501,help lists
>>>>> options for one device? Adding info about default to -display help
>>>>> is really helpful though (that could also be marked with (default)
>>>>> like in -machine help.
>>>>
>>>> It's copied from what qemu-system-aarch64 -h outputs. At least it's 
>>>> less
>>>> overwhelming than qemu-system-aarch64 -h.
>>>
>>> This changes what -display help does so if some script depends on 
>>> that it
>>> may not be a good idea. Since the same info is already in -help maybe 
>>> this
>>> change to add that to -display help as well is not the best solution 
>>> so I'd
>>> say drop this patch and leave it as it is for now.
>>
>> We consider help output to be only for humans.
>>
>> No scripts should ever be parsing any QEMU output, as we provide
>> QMP for automated detection/querying of features.
>>
>> IOW, if some script is parsing help output we are fine to break
>> them if it improves QEMU's output for humans in a justiable way.
> 
> OK fair enough. I'd still like some more consitency with -cpu help and 
> -machine help and -device help which all list avalable options and then 
> can take help for further help for individual items where applicable. So 
> it would be better if -display and -audio could behave the same. (Are 
> there any other options that might be missing this?)
> 
> Out of curiousicy what's the way to query available display backends for 
> programs by QMP then?

There is no method available as far as I know. All of these quirks are 
caused by the design of the display infrastructure not integrated well 
with QOM.