| Message ID | 20210302175524.1290840-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | audio: make audiodev introspectable by mgmt apps | expand |
On 3/2/21 11:55 AM, Daniel P. Berrangé wrote: > Way back in QEMU 4.0, the -audiodev command line option was introduced > for configuring audio backends. This CLI option does not use QemuOpts > so it is not visible for introspection in 'query-command-line-options', > instead using the QAPI Audiodev type. Unfortunately there is also no > QMP command that uses the Audiodev type, so it is not introspectable > with 'query-qmp-schema' either. > > This introduces a 'query-audiodev' command that simply reflects back > the list of configured -audiodev command line options. This in turn > makes Audiodev introspectable via 'query-qmp-schema'. Even if we never call the query-audiodev command, its mere existence is useful ;) > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > audio/audio.c | 19 +++++++++++++++++++ > qapi/audio.json | 13 +++++++++++++ > 2 files changed, 32 insertions(+) > +AudiodevList *qmp_query_audiodevs(Error **errp) > +{ > + AudiodevList *ret = NULL, *prev = NULL, *curr; > + AudiodevListEntry *e; > + QSIMPLEQ_FOREACH(e, &audiodevs, next) { > + curr = g_new0(AudiodevList, 1); > + curr->value = QAPI_CLONE(Audiodev, e->dev); > + if (prev) { > + prev->next = curr; > + prev = curr; > + } else { > + ret = prev = curr; > + } Please use QAPI_LIST_PREPEND here instead of open-coding it. > + > +## > +# @query-audiodevs: > +# > +# Returns information about audiodev configuration > +# > +# Returns: array of @Audiodev > +# > +# Since: 6.0 > +# > +## > +{ 'command': 'query-audiodevs', > + 'returns': ['Audiodev'] } > Otherwise looks nice.
On 3/2/21 6:55 PM, Daniel P. Berrangé wrote: > Way back in QEMU 4.0, the -audiodev command line option was introduced > for configuring audio backends. This CLI option does not use QemuOpts > so it is not visible for introspection in 'query-command-line-options', > instead using the QAPI Audiodev type. Unfortunately there is also no > QMP command that uses the Audiodev type, so it is not introspectable > with 'query-qmp-schema' either. > > This introduces a 'query-audiodev' command that simply reflects back > the list of configured -audiodev command line options. This in turn > makes Audiodev introspectable via 'query-qmp-schema'. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > audio/audio.c | 19 +++++++++++++++++++ > qapi/audio.json | 13 +++++++++++++ > 2 files changed, 32 insertions(+) > + > +## > +# @query-audiodevs: > +# > +# Returns information about audiodev configuration > +# > +# Returns: array of @Audiodev > +# > +# Since: 6.0 > +# > +## > +{ 'command': 'query-audiodevs', > + 'returns': ['Audiodev'] } > Can we use 'query-audiodev-backends' similarly to 'query-chardev-backends'?
On 3/2/21 10:10 PM, Philippe Mathieu-Daudé wrote: > On 3/2/21 6:55 PM, Daniel P. Berrangé wrote: >> Way back in QEMU 4.0, the -audiodev command line option was introduced >> for configuring audio backends. This CLI option does not use QemuOpts >> so it is not visible for introspection in 'query-command-line-options', >> instead using the QAPI Audiodev type. Unfortunately there is also no >> QMP command that uses the Audiodev type, so it is not introspectable >> with 'query-qmp-schema' either. >> >> This introduces a 'query-audiodev' command that simply reflects back >> the list of configured -audiodev command line options. This in turn >> makes Audiodev introspectable via 'query-qmp-schema'. >> >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> --- >> audio/audio.c | 19 +++++++++++++++++++ >> qapi/audio.json | 13 +++++++++++++ >> 2 files changed, 32 insertions(+) > >> + >> +## >> +# @query-audiodevs: >> +# >> +# Returns information about audiodev configuration >> +# >> +# Returns: array of @Audiodev Also with chardev we return ChardevBackendInfo. I'm not sure if this is because I'm custom to read it, but it seems clearer. Can we return a AudiodevBackendInfo type? >> +# >> +# Since: 6.0 >> +# >> +## >> +{ 'command': 'query-audiodevs', >> + 'returns': ['Audiodev'] } >> > > Can we use 'query-audiodev-backends' similarly to > 'query-chardev-backends'? >
On Tue, Mar 02, 2021 at 10:12:43PM +0100, Philippe Mathieu-Daudé wrote: > On 3/2/21 10:10 PM, Philippe Mathieu-Daudé wrote: > > On 3/2/21 6:55 PM, Daniel P. Berrangé wrote: > >> Way back in QEMU 4.0, the -audiodev command line option was introduced > >> for configuring audio backends. This CLI option does not use QemuOpts > >> so it is not visible for introspection in 'query-command-line-options', > >> instead using the QAPI Audiodev type. Unfortunately there is also no > >> QMP command that uses the Audiodev type, so it is not introspectable > >> with 'query-qmp-schema' either. > >> > >> This introduces a 'query-audiodev' command that simply reflects back > >> the list of configured -audiodev command line options. This in turn > >> makes Audiodev introspectable via 'query-qmp-schema'. > >> > >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > >> --- > >> audio/audio.c | 19 +++++++++++++++++++ > >> qapi/audio.json | 13 +++++++++++++ > >> 2 files changed, 32 insertions(+) > > > >> + > >> +## > >> +# @query-audiodevs: > >> +# > >> +# Returns information about audiodev configuration > >> +# > >> +# Returns: array of @Audiodev > > Also with chardev we return ChardevBackendInfo. I'm not sure > if this is because I'm custom to read it, but it seems clearer. > Can we return a AudiodevBackendInfo type? Chardevs are not a good guide as they have this wierd split betweeen manually written Chardev type and QAPI driven ChardeBackendInfo type. With audio backends, we only have the Audiodev QAPI type. There is no AudiodevBackendInfo. Regards, Daniel
On Tue, Mar 02, 2021 at 10:10:56PM +0100, Philippe Mathieu-Daudé wrote: > On 3/2/21 6:55 PM, Daniel P. Berrangé wrote: > > Way back in QEMU 4.0, the -audiodev command line option was introduced > > for configuring audio backends. This CLI option does not use QemuOpts > > so it is not visible for introspection in 'query-command-line-options', > > instead using the QAPI Audiodev type. Unfortunately there is also no > > QMP command that uses the Audiodev type, so it is not introspectable > > with 'query-qmp-schema' either. > > > > This introduces a 'query-audiodev' command that simply reflects back > > the list of configured -audiodev command line options. This in turn > > makes Audiodev introspectable via 'query-qmp-schema'. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > audio/audio.c | 19 +++++++++++++++++++ > > qapi/audio.json | 13 +++++++++++++ > > 2 files changed, 32 insertions(+) > > > + > > +## > > +# @query-audiodevs: > > +# > > +# Returns information about audiodev configuration > > +# > > +# Returns: array of @Audiodev > > +# > > +# Since: 6.0 > > +# > > +## > > +{ 'command': 'query-audiodevs', > > + 'returns': ['Audiodev'] } > > > > Can we use 'query-audiodev-backends' similarly to > 'query-chardev-backends'? IMHO adding "-backends" is redundant, because audiodev and chardev objects are always backends. Regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > Way back in QEMU 4.0, the -audiodev command line option was introduced > for configuring audio backends. This CLI option does not use QemuOpts > so it is not visible for introspection in 'query-command-line-options', > instead using the QAPI Audiodev type. Unfortunately there is also no > QMP command that uses the Audiodev type, so it is not introspectable > with 'query-qmp-schema' either. This is a gap that will only widen. By design, query-qmp-schema covers just QMP. It doesn't cover the QAPIfied parts of the CLI. They have been growing slowly, and this trend will continue. We need schema introspection to cover the CLI, too. Observation, not demand. Work-arounds: 1. When a QMP command equivalent to a QAPIfied CLI option exists, introspect that. Involves hardcoding the connection between the two. Example: -blockdev and blockdev-add. 2. When a QMP query exists that returns the CLI option argument, introspect that. Involves hardcoding the connection between the two. Example: -display and query-display-options. 3. When neither exists, create a suitable query just to enable introspection. Example: query-display-options. Commit e1ca8f7e19 "qapi: add query-display-options command" explains this clearly: Add query-display-options command, which allows querying the qemu display configuration. This isn't particularly useful, except it exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt can discover recently added -display parameter rendernode (commit d4dc4ab133b). Works around lack of sufficiently powerful command line introspection. > This introduces a 'query-audiodev' command that simply reflects back > the list of configured -audiodev command line options. This in turn > makes Audiodev introspectable via 'query-qmp-schema'. Is the query just for enabling introspection, or does it have other uses? If the former, please steal from Gerd's explanation for your commit message. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > audio/audio.c | 19 +++++++++++++++++++ > qapi/audio.json | 13 +++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/audio/audio.c b/audio/audio.c > index 6734c8af70..40a4bbd7ce 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -28,8 +28,10 @@ > #include "monitor/monitor.h" > #include "qemu/timer.h" > #include "qapi/error.h" > +#include "qapi/clone-visitor.h" > #include "qapi/qobject-input-visitor.h" > #include "qapi/qapi-visit-audio.h" > +#include "qapi/qapi-commands-audio.h" > #include "qemu/cutils.h" > #include "qemu/module.h" > #include "sysemu/replay.h" > @@ -2201,3 +2203,20 @@ size_t audio_rate_get_bytes(struct audio_pcm_info *info, RateCtl *rate, > rate->bytes_sent += ret; > return ret; > } > + > +AudiodevList *qmp_query_audiodevs(Error **errp) > +{ > + AudiodevList *ret = NULL, *prev = NULL, *curr; > + AudiodevListEntry *e; > + QSIMPLEQ_FOREACH(e, &audiodevs, next) { > + curr = g_new0(AudiodevList, 1); > + curr->value = QAPI_CLONE(Audiodev, e->dev); > + if (prev) { > + prev->next = curr; > + prev = curr; > + } else { > + ret = prev = curr; > + } Please use QAPI_LIST_APPEND(). > + } > + return ret; > +} > diff --git a/qapi/audio.json b/qapi/audio.json > index 9cba0df8a4..d7b91230d7 100644 > --- a/qapi/audio.json > +++ b/qapi/audio.json > @@ -419,3 +419,16 @@ > 'sdl': 'AudiodevSdlOptions', > 'spice': 'AudiodevGenericOptions', > 'wav': 'AudiodevWavOptions' } } > + > +## > +# @query-audiodevs: > +# > +# Returns information about audiodev configuration > +# > +# Returns: array of @Audiodev > +# > +# Since: 6.0 > +# > +## > +{ 'command': 'query-audiodevs', > + 'returns': ['Audiodev'] }
On Fri, Mar 05, 2021 at 02:01:14PM +0100, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > Way back in QEMU 4.0, the -audiodev command line option was introduced > > for configuring audio backends. This CLI option does not use QemuOpts > > so it is not visible for introspection in 'query-command-line-options', > > instead using the QAPI Audiodev type. Unfortunately there is also no > > QMP command that uses the Audiodev type, so it is not introspectable > > with 'query-qmp-schema' either. > > This is a gap that will only widen. > > By design, query-qmp-schema covers just QMP. It doesn't cover the > QAPIfied parts of the CLI. They have been growing slowly, and this > trend will continue. We need schema introspection to cover the CLI, > too. Observation, not demand. > > Work-arounds: > > 1. When a QMP command equivalent to a QAPIfied CLI option exists, > introspect that. Involves hardcoding the connection between the two. > Example: -blockdev and blockdev-add. I'd say this is the scenario that wil lapply long term. As we aim for a world where you can run "qemu -qmp unix:/tmp/sock,server" and then configure everything over QMP, we should cease to have any scenarios where stuff is missing from query-qmp-schemas, as we'll have a QMP command that covers everything. IOW, in this case we really ought to have a 'audiodev-add' command as a counterpart to -audiodev CLI. Not sure if there are any dragons wrt to creating audio backends on the fly. > 2. When a QMP query exists that returns the CLI option argument, > introspect that. Involves hardcoding the connection between the two. > Example: -display and query-display-options. > > 3. When neither exists, create a suitable query just to enable > introspection. Example: query-display-options. Commit e1ca8f7e19 > "qapi: add query-display-options command" explains this clearly: > > Add query-display-options command, which allows querying the qemu > display configuration. This isn't particularly useful, except it > exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt > can discover recently added -display parameter rendernode (commit > d4dc4ab133b). Works around lack of sufficiently powerful command line > introspection. I figure having a 'query-foo' QMP corresponding to any 'foo-add' QMP is useful as a general rule to allow mgmt (or human support engineers) to inspect an running instance and understand its configuration, especially since it won't be visible in CLI args if we move to 100% QMP based config. > > This introduces a 'query-audiodev' command that simply reflects back > > the list of configured -audiodev command line options. This in turn > > makes Audiodev introspectable via 'query-qmp-schema'. > > Is the query just for enabling introspection, or does it have other > uses? On the libvirt side we only need it for enabling introspection. As a general rule, being able to inspect any aspect of configuration via QMP is useful for support purposes if nothing else. Regards, Daniel
diff --git a/audio/audio.c b/audio/audio.c index 6734c8af70..40a4bbd7ce 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -28,8 +28,10 @@ #include "monitor/monitor.h" #include "qemu/timer.h" #include "qapi/error.h" +#include "qapi/clone-visitor.h" #include "qapi/qobject-input-visitor.h" #include "qapi/qapi-visit-audio.h" +#include "qapi/qapi-commands-audio.h" #include "qemu/cutils.h" #include "qemu/module.h" #include "sysemu/replay.h" @@ -2201,3 +2203,20 @@ size_t audio_rate_get_bytes(struct audio_pcm_info *info, RateCtl *rate, rate->bytes_sent += ret; return ret; } + +AudiodevList *qmp_query_audiodevs(Error **errp) +{ + AudiodevList *ret = NULL, *prev = NULL, *curr; + AudiodevListEntry *e; + QSIMPLEQ_FOREACH(e, &audiodevs, next) { + curr = g_new0(AudiodevList, 1); + curr->value = QAPI_CLONE(Audiodev, e->dev); + if (prev) { + prev->next = curr; + prev = curr; + } else { + ret = prev = curr; + } + } + return ret; +} diff --git a/qapi/audio.json b/qapi/audio.json index 9cba0df8a4..d7b91230d7 100644 --- a/qapi/audio.json +++ b/qapi/audio.json @@ -419,3 +419,16 @@ 'sdl': 'AudiodevSdlOptions', 'spice': 'AudiodevGenericOptions', 'wav': 'AudiodevWavOptions' } } + +## +# @query-audiodevs: +# +# Returns information about audiodev configuration +# +# Returns: array of @Audiodev +# +# Since: 6.0 +# +## +{ 'command': 'query-audiodevs', + 'returns': ['Audiodev'] }
Way back in QEMU 4.0, the -audiodev command line option was introduced for configuring audio backends. This CLI option does not use QemuOpts so it is not visible for introspection in 'query-command-line-options', instead using the QAPI Audiodev type. Unfortunately there is also no QMP command that uses the Audiodev type, so it is not introspectable with 'query-qmp-schema' either. This introduces a 'query-audiodev' command that simply reflects back the list of configured -audiodev command line options. This in turn makes Audiodev introspectable via 'query-qmp-schema'. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- audio/audio.c | 19 +++++++++++++++++++ qapi/audio.json | 13 +++++++++++++ 2 files changed, 32 insertions(+)