* [PATCH v3] qapi: give available enum values as error hint @ 2023-03-07 14:58 marcandre.lureau 2023-03-08 13:55 ` Markus Armbruster 0 siblings, 1 reply; 4+ messages in thread From: marcandre.lureau @ 2023-03-07 14:58 UTC (permalink / raw) To: qemu-devel; +Cc: armbru, Marc-André Lureau, Michael Roth From: Marc-André Lureau <marcandre.lureau@redhat.com> This allows for a more pleasant user experience. Before: $ ./qemu-system-x86_64 -display egl-headless,gl= qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value '' After: $ ./qemu-system-x86_64 -display egl-headless,gl= qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value '' Acceptable values are 'off', 'on', 'core', 'es' Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- qapi/qapi-visit-core.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 6c13510a2b..7468b8c7b8 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -392,9 +392,25 @@ static bool output_type_enum(Visitor *v, const char *name, int *obj, return visit_type_str(v, name, &enum_str, errp); } +static void error_append_qapi_enum_hint(Error *const *errp, const QEnumLookup *lookup) +{ + int i; + + error_append_hint(errp, "Acceptable values are "); + for (i = 0; i < lookup->size; i++) { + error_append_hint(errp, "'%s'", lookup->array[i]); + if (i + 1 < lookup->size) { + error_append_hint(errp, ", "); + } + } + error_append_hint(errp, "\n"); +} + + static bool input_type_enum(Visitor *v, const char *name, int *obj, const QEnumLookup *lookup, Error **errp) { + ERRP_GUARD(); int64_t value; g_autofree char *enum_str = NULL; @@ -406,6 +422,7 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj, if (value < 0) { error_setg(errp, "Parameter '%s' does not accept value '%s'", name ? name : "null", enum_str); + error_append_qapi_enum_hint(errp, lookup); return false; } -- 2.39.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] qapi: give available enum values as error hint 2023-03-07 14:58 [PATCH v3] qapi: give available enum values as error hint marcandre.lureau @ 2023-03-08 13:55 ` Markus Armbruster 2023-03-08 13:59 ` Marc-André Lureau 0 siblings, 1 reply; 4+ messages in thread From: Markus Armbruster @ 2023-03-08 13:55 UTC (permalink / raw) To: marcandre.lureau; +Cc: qemu-devel, Michael Roth marcandre.lureau@redhat.com writes: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > This allows for a more pleasant user experience. > > Before: > $ ./qemu-system-x86_64 -display egl-headless,gl= > qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value '' > > After: > $ ./qemu-system-x86_64 -display egl-headless,gl= > qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value '' > Acceptable values are 'off', 'on', 'core', 'es' > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Nice improvement here. Slightly ugly: $ qemu-system-x86_64 -nic bad upstream-qemu: -nic bad: Parameter 'type' does not accept value 'bad' Acceptable values are 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'stream', 'dgram', 'vde', 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' Outright annoying: $ upstream-qemu -object bad upstream-qemu: -object bad: Parameter 'qom-type' does not accept value 'bad' Acceptable values are 'authz-list', 'authz-listfile', 'authz-pam', 'authz-simple', 'can-bus', 'can-host-socketcan', 'colo-compare', 'cryptodev-backend', 'cryptodev-backend-builtin', 'cryptodev-backend-lkcf', 'dbus-vmstate', 'filter-buffer', 'filter-dump', 'filter-mirror', 'filter-redirector', 'filter-replay', 'filter-rewriter', 'input-barrier', 'input-linux', 'iothread', 'main-loop', 'memory-backend-epc', 'memory-backend-file', 'memory-backend-memfd', 'memory-backend-ram', 'pef-guest', 'pr-manager-helper', 'qtest', 'rng-builtin', 'rng-egd', 'rng-random', 'secret', 'secret_keyring', 'sev-guest', 'thread-context', 's390-pv-guest', 'throttle-group', 'tls-creds-anon', 'tls-creds-psk', 'tls-creds-x509', 'tls-cipher-suites', 'x-remote-object', 'x-vfio-user-server' Note we already let users ask for this information with -object help or -object qom-type=help. Sadly, we can't hint at that here, because it's implemented much further up the call chain, and other call chains don't. If HMP command sendkey didn't bypass the input visitor, the 26 screen lines of hint for QKeyCode would likely scroll the error message off the screen. Should we suppress this hint when it's too long to be useful? ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] qapi: give available enum values as error hint 2023-03-08 13:55 ` Markus Armbruster @ 2023-03-08 13:59 ` Marc-André Lureau 2023-03-09 12:37 ` Markus Armbruster 0 siblings, 1 reply; 4+ messages in thread From: Marc-André Lureau @ 2023-03-08 13:59 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, Michael Roth [-- Attachment #1: Type: text/plain, Size: 2643 bytes --] Hi On Wed, Mar 8, 2023 at 5:55 PM Markus Armbruster <armbru@redhat.com> wrote: > marcandre.lureau@redhat.com writes: > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > This allows for a more pleasant user experience. > > > > Before: > > $ ./qemu-system-x86_64 -display egl-headless,gl= > > qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not > accept value '' > > > > After: > > $ ./qemu-system-x86_64 -display egl-headless,gl= > > qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not > accept value '' > > Acceptable values are 'off', 'on', 'core', 'es' > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Nice improvement here. > > Slightly ugly: > > $ qemu-system-x86_64 -nic bad > upstream-qemu: -nic bad: Parameter 'type' does not accept value 'bad' > Acceptable values are 'none', 'nic', 'user', 'tap', 'l2tpv3', > 'socket', 'stream', 'dgram', 'vde', 'bridge', 'hubport', 'netmap', > 'vhost-user', 'vhost-vdpa' > > Outright annoying: > > $ upstream-qemu -object bad > upstream-qemu: -object bad: Parameter 'qom-type' does not accept value > 'bad' > Acceptable values are 'authz-list', 'authz-listfile', 'authz-pam', > 'authz-simple', 'can-bus', 'can-host-socketcan', 'colo-compare', > 'cryptodev-backend', 'cryptodev-backend-builtin', 'cryptodev-backend-lkcf', > 'dbus-vmstate', 'filter-buffer', 'filter-dump', 'filter-mirror', > 'filter-redirector', 'filter-replay', 'filter-rewriter', 'input-barrier', > 'input-linux', 'iothread', 'main-loop', 'memory-backend-epc', > 'memory-backend-file', 'memory-backend-memfd', 'memory-backend-ram', > 'pef-guest', 'pr-manager-helper', 'qtest', 'rng-builtin', 'rng-egd', > 'rng-random', 'secret', 'secret_keyring', 'sev-guest', 'thread-context', > 's390-pv-guest', 'throttle-group', 'tls-creds-anon', 'tls-creds-psk', > 'tls-creds-x509', 'tls-cipher-suites', 'x-remote-object', > 'x-vfio-user-server' > > Note we already let users ask for this information with -object help or > -object qom-type=help. Sadly, we can't hint at that here, because it's > implemented much further up the call chain, and other call chains don't. > > If HMP command sendkey didn't bypass the input visitor, the 26 screen > lines of hint for QKeyCode would likely scroll the error message off the > screen. > > Should we suppress this hint when it's too long to be useful? > I don't have strong opinions.. perhaps stop after first 5 with "..." ? (Ideally, we would have shell completion scripts that would be able to help us, but hey that's another level! :) [-- Attachment #2: Type: text/html, Size: 3847 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] qapi: give available enum values as error hint 2023-03-08 13:59 ` Marc-André Lureau @ 2023-03-09 12:37 ` Markus Armbruster 0 siblings, 0 replies; 4+ messages in thread From: Markus Armbruster @ 2023-03-09 12:37 UTC (permalink / raw) To: Marc-André Lureau; +Cc: qemu-devel, Michael Roth Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Hi > > On Wed, Mar 8, 2023 at 5:55 PM Markus Armbruster <armbru@redhat.com> wrote: > >> marcandre.lureau@redhat.com writes: >> >> > From: Marc-André Lureau <marcandre.lureau@redhat.com> >> > >> > This allows for a more pleasant user experience. >> > >> > Before: >> > $ ./qemu-system-x86_64 -display egl-headless,gl= >> > qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value '' >> > >> > After: >> > $ ./qemu-system-x86_64 -display egl-headless,gl= >> > qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value '' >> > Acceptable values are 'off', 'on', 'core', 'es' >> > >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> Nice improvement here. >> >> Slightly ugly: >> >> $ qemu-system-x86_64 -nic bad >> upstream-qemu: -nic bad: Parameter 'type' does not accept value 'bad' >> Acceptable values are 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'stream', 'dgram', 'vde', 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' >> >> Outright annoying: >> >> $ upstream-qemu -object bad >> upstream-qemu: -object bad: Parameter 'qom-type' does not accept value 'bad' >> Acceptable values are 'authz-list', 'authz-listfile', 'authz-pam', 'authz-simple', 'can-bus', 'can-host-socketcan', 'colo-compare', 'cryptodev-backend', 'cryptodev-backend-builtin', 'cryptodev-backend-lkcf', 'dbus-vmstate', 'filter-buffer', 'filter-dump', 'filter-mirror', 'filter-redirector', 'filter-replay', 'filter-rewriter', 'input-barrier', 'input-linux', 'iothread', 'main-loop', 'memory-backend-epc', 'memory-backend-file', 'memory-backend-memfd', 'memory-backend-ram', 'pef-guest', 'pr-manager-helper', 'qtest', 'rng-builtin', 'rng-egd', 'rng-random', 'secret', 'secret_keyring', 'sev-guest', 'thread-context', 's390-pv-guest', 'throttle-group', 'tls-creds-anon', 'tls-creds-psk', 'tls-creds-x509', 'tls-cipher-suites', 'x-remote-object', 'x-vfio-user-server' >> >> Note we already let users ask for this information with -object help or >> -object qom-type=help. Sadly, we can't hint at that here, because it's >> implemented much further up the call chain, and other call chains don't. >> >> If HMP command sendkey didn't bypass the input visitor, the 26 screen >> lines of hint for QKeyCode would likely scroll the error message off the >> screen. >> >> Should we suppress this hint when it's too long to be useful? > > I don't have strong opinions.. perhaps stop after first 5 with "..." ? A hint like "Use CMD to show the possible values" would be better, but it's not feasible now. Instead of cutting off after five, we cut off when the hint goes beyond a length limit. A bit more involved. Up to you. Enumerations with more than five members: 6 BlkdebugIOType 6 EvtchnPortType 6 GrabToggleKeys 6 HmatLBDataType 6 SevState 7 AudioFormat 7 QCryptoHashAlgorithm 7 SchemaMetaType 7 WatchdogAction 8 JSONType 9 DisplayType 9 InputButton 9 JobType 9 VncPrimaryAuth 9 VncVencryptSubAuth 11 JobStatus 11 ShutdownCause 12 AudiodevDriver 12 QCryptoCipherAlgorithm 12 TransactionActionKind 14 MigrationStatus 16 RunState 17 NetClientDriver 21 MigrationCapability 22 ChardevBackendKind 31 SysEmuTarget 44 ObjectType 47 BlockdevDriver 48 BlkdebugEvent 150 QKeyCode Not all of them are can be visited from CLI or HMP (which is how the hint gets used). > (Ideally, we would have shell completion scripts that would be able to help > us, but hey that's another level! :) True :) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-09 12:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-07 14:58 [PATCH v3] qapi: give available enum values as error hint marcandre.lureau 2023-03-08 13:55 ` Markus Armbruster 2023-03-08 13:59 ` Marc-André Lureau 2023-03-09 12:37 ` Markus Armbruster
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).