On Wed, 26 Jun 2013 13:38:04 +0200
Gerd Hoffmann <kra...@redhat.com> wrote:
> This patch adds a fbdev monitor command to enable/disable
> the fbdev display at runtime to both qmp and hmp.
>
> qmp: framebuffer-display enable=on|off scale=on|off device=/dev/fb<n>
> hmp: framebuffer-display on|off
>
> There is also a query-framebuffer command for qmp.
>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> hmp-commands.hx | 14 ++++++++++++++
> hmp.c | 9 +++++++++
> hmp.h | 1 +
> include/ui/console.h | 1 +
> qapi-schema.json | 43 +++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 12 ++++++++++++
> qmp.c | 31 +++++++++++++++++++++++++++++++
> ui/fbdev.c | 20 ++++++++++++++++++++
> 8 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 915b0d1..283106d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1563,7 +1563,21 @@ STEXI
> @findex qemu-io
>
> Executes a qemu-io command on the given block device.
> +ETEXI
> +
> + {
> + .name = "framebuffer-display",
> + .args_type = "enable:b",
> + .params = "on|off",
> + .help = "enable/disable linux console framebuffer display",
> + .mhandler.cmd = hmp_framebuffer_display,
> + },
> +
> +STEXI
> +@item framebuffer-display on | off
> +@findex framebuffer-display
>
> +enable/disable linux console framebuffer display.
> ETEXI
>
> {
> diff --git a/hmp.c b/hmp.c
> index 494a9aa..55f195f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1464,3 +1464,12 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
>
> hmp_handle_error(mon, &err);
> }
> +
> +void hmp_framebuffer_display(Monitor *mon, const QDict *qdict)
> +{
> + int enable = qdict_get_bool(qdict, "enable");
> + Error *errp = NULL;
> +
> + qmp_framebuffer_display(enable, false, false, false, NULL, &errp);
> + hmp_handle_error(mon, &errp);
> +}
> diff --git a/hmp.h b/hmp.h
> index 56d2e92..c3a48e4 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -86,5 +86,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
> void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
> void hmp_qemu_io(Monitor *mon, const QDict *qdict);
> +void hmp_framebuffer_display(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 71b538a..5a9207d 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -311,6 +311,7 @@ void sdl_display_init(DisplayState *ds, int full_screen,
> int no_frame);
> /* fbdev.c */
> int fbdev_display_init(const char *device, bool scale, Error **err);
> void fbdev_display_uninit(void);
> +FramebufferInfo *framebuffer_info(void);
>
> /* cocoa.m */
> void cocoa_display_init(DisplayState *ds, int full_screen);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 6cc07c2..715dc1f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3608,3 +3608,46 @@
> '*cpuid-input-ecx': 'int',
> 'cpuid-register': 'X86CPURegister32',
> 'features': 'int' } }
> +
> +##
> +# @framebuffer-display:
Let me bike-shed: we're trying to make command's names verbs. So, we
could call this framebuffer-display-set or maybe have two commands,
framebuffer-display-enable and framebuffer-display-disable. I prefer
the latter.
> +#
> +# Enable/disable linux console framebuffer display.
> +#
> +# @enable: whenever the framebuffer display should be enabled or disabled.
> +#
> +# @scale: #optional enables display scaling, default: off
> +#
> +# @device: #optional specifies framebuffer device, default: /dev/fb0
Actually, it will try to get the device name from an env variable first,
which sounds too automatic for a building block API like QMP. You can do
it for HMP, but QMP I guess it would be more appropriate to make the
device name mandatory.
> +#
> +# Returns: Nothing.
> +#
> +# Since: 1.6
> +#
> +##
> +{ 'command': 'framebuffer-display', 'data': {'enable' : 'bool',
> + '*scale' : 'bool',
> + '*device' : 'str' } }
> +
> +##
> +# @FramebufferInfo:
> +#
Missing docs.
> +# Since 1.6
> +##
> +{ 'type': 'FramebufferInfo',
> + 'data': { 'enabled': 'bool',
> + '*scale' : 'bool',
> + '*device': 'str',
Why is device optional?
> + '*vtno' : 'int' } }
> +
> +##
> +# @query-framebuffer:
> +#
> +# Query linux console framebuffer state.
> +#
> +# Returns: FramebufferInfo.
> +#
> +# Since: 1.6
> +#
> +##
> +{ 'command': 'query-framebuffer', 'returns': 'FramebufferInfo' }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 8cea5e5..e0661f0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2997,3 +2997,15 @@ Example:
> <- { "return": {} }
>
> EQMP
> +
> + {
> + .name = "framebuffer-display",
> + .args_type = "enable:b,scale:b?,device:s?",
> + .mhandler.cmd_new = qmp_marshal_input_framebuffer_display,
> + },
> +
> + {
> + .name = "query-framebuffer",
> + .args_type = "",
> + .mhandler.cmd_new = qmp_marshal_input_query_framebuffer,
> + },
> diff --git a/qmp.c b/qmp.c
> index 4c149b3..b6d826c 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -404,6 +404,37 @@ void qmp_change(const char *device, const char *target,
> }
> }
>
> +void qmp_framebuffer_display(bool enable,
> + bool has_scale, bool scale,
> + bool has_device, const char *device,
> + Error **errp)
> +{
> +#if defined(CONFIG_FBDEV)
> + if (enable) {
> + if (fbdev_display_init(has_device ? device : NULL,
> + has_scale ? scale : false,
> + errp) != 0) {
> + if (!error_is_set(errp)) {
> + error_setg(errp, "fbdev initialization failed");
You should use error_propagate() in order to propagate the error
information filled by fbdev_display_init(). Also, I'd move
the generic error_setg() above to fbdev_display_init() and make it
return void.
> + }
> + }
> + } else {
> + fbdev_display_uninit();
> + }
> +#else
> + error_setg(errp, "fbdev support disabled at compile time");
> +#endif
> +}
> +
> +FramebufferInfo *qmp_query_framebuffer(Error **errp)
> +{
> +#if defined(CONFIG_FBDEV)
> + return framebuffer_info();
> +#else
> + error_setg(errp, "fbdev support disabled at compile time");
> +#endif
> +}
> +
> static void qom_list_types_tramp(ObjectClass *klass, void *data)
> {
> ObjectTypeInfoList *e, **pret = data;
> diff --git a/ui/fbdev.c b/ui/fbdev.c
> index 91e11e5..326bba1 100644
> --- a/ui/fbdev.c
> +++ b/ui/fbdev.c
> @@ -1162,3 +1162,23 @@ void fbdev_display_uninit(void)
> g_free(s);
> fb = NULL;
> }
> +
> +FramebufferInfo *framebuffer_info(void)
> +{
> + FramebufferInfo *info = g_new0(FramebufferInfo, 1);
> + FBDevState *s = fb;
> +
> + if (s == NULL) {
> + info->enabled = false;
> + return info;
> + }
> +
> + info->enabled = true;
> + info->has_device = true;
> + info->device = g_strdup(s->device);
> + info->has_scale = true;
> + info->scale = s->use_scale;
> + info->has_vtno = true;
> + info->vtno = s->vtno;
> + return info;
> +}
