From: Frediano Ziglio <fziglio@redhat.com>
To: Kevin Pouget <kpouget@redhat.com>
Cc: spice-devel@lists.freedesktop.org, qemu-devel@nongnu.org,
Marc-Andre Lureau <marcandre.lureau@redhat.com>
Subject: Re: [Qemu-devel] [RFC] spice-core: allow setting properties from QMP
Date: Fri, 21 Jun 2019 03:16:48 -0400 (EDT) [thread overview]
Message-ID: <2139720774.23871724.1561101408712.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <CADJ1XR3fh0cyOerSM8VQkpde6cHLb8WccP05Rwr7xWMOK59rog@mail.gmail.com>
>
> Hello Eric,
>
> > A new command may be okay, however,
>
> thanks, I've fix the typos and updated the patch to use an Enum, which
> indeed makes more sense.
>
> I've also updated "spice-query" command to provide the current value
> of the "video-codec" property,
> but it made me wonder if I should improve this QMP interface with a
> json list, or keep the current string-based list
> ("enc1:codec1;enc2:codec2").
>
> I CC the spice-devel list to get their point of view
>
> The current behavior is:
>
> --> { "execute": "set-spice", "arguments": { "property":
> "video-codecs", "value": "spice:mjpeg;gstreamer:h264" } }
> <-- {"return":{},"id":"libvirt-23"}
It looks complicated from the user. Why not just
--> { "execute": "set-spice", "arguments": { "video-codecs": "spice:mjpeg;gstreamer:h264" } }
>
> --> { "execute": "query-spice" }
> <-- {.... "video-codecs": "spice:mjpeg;gstreamer:h264;" ....}
>
>
> I put the new version of the RFC patch below
>
> best regards,
>
> Kevin
>
> ---
>
> This patch allows setting spice properties from the QMP interface.
>
> At the moment, only the 'video-codecs' property is supported.
>
> Signed-off-by: Kevin Pouget <kpouget@redhat.com>
> ---
> qapi/ui.json | 42 ++++++++++++++++++++++++++++++++++++++++--
> ui/spice-core.c | 21 +++++++++++++++++++++
> 2 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 59e412139a..5f67096bcb 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -211,12 +211,16 @@
> #
> # @channels: a list of @SpiceChannel for each active spice channel
> #
> +# @video-codecs: The list of encoders:codecs currently allowed for
> +# video streaming (since: ...)
> +#
> # Since: 0.14.0
> ##
> { 'struct': 'SpiceInfo',
> 'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str',
> '*port': 'int',
> '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
> - 'mouse-mode': 'SpiceQueryMouseMode', '*channels':
> ['SpiceChannel']},
> + 'mouse-mode': 'SpiceQueryMouseMode', '*channels':
> ['SpiceChannel'],
> + 'video-codecs': 'str'},
> 'if': 'defined(CONFIG_SPICE)' }
>
> ##
> @@ -257,7 +261,8 @@
> # "tls": false
> # },
> # [ ... more channels follow ... ]
> -# ]
> +# ],
> +# "video-codecs": "spice:mjpeg;gstreamer:h264;"
> # }
> # }
> #
> @@ -265,6 +270,39 @@
> { 'command': 'query-spice', 'returns': 'SpiceInfo',
> 'if': 'defined(CONFIG_SPICE)' }
>
> +##
> +# @SpiceProperty:
> +#
> +# An enumeration of Spice properties that can be set at runtime.
> +#
> +# @video-codecs: the ;-separated list of video-codecs allowed for
> +# spice-server video streaming.
> +#
> +# Since: ...
> +##
> +{ 'enum': 'SpiceProperty',
> + 'data': [ 'video-codecs'],
> + 'if': 'defined(CONFIG_SPICE)' }
> +
> +##
> +# @set-spice:
> +#
> +# Set Spice properties.
> +# @property: the SPICE property to modify
> +# @value: the new value to affect to this property
> +#
> +# Since: ...
> +#
> +# Example:
> +#
> +# -> { "execute": "set-spice", "arguments": { "property": "video-codecs",
> +# "value": "spice:mjpeg;" } }
> +# <- { "returns": {} }
> +##
> +{ 'command': 'set-spice',
> + 'data': {'property': 'SpiceProperty', 'value': 'str'},
> + 'if': 'defined(CONFIG_SPICE)' }
> +
> ##
> # @SPICE_CONNECTED:
> #
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 63e8694df8..1660f49f15 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -506,6 +506,25 @@ static QemuOptsList qemu_spice_opts = {
> },
> };
>
> +void qmp_set_spice(SpiceProperty property, const char *value, Error **errp)
> +{
> + int invalid_codecs;
> +
> + switch(property) {
> + case SPICE_PROPERTY_VIDEO_CODECS:
> + invalid_codecs = spice_server_set_video_codecs(spice_server, value);
> +
> + if (invalid_codecs) {
> + error_setg(errp, "Found %d invalid video-codecs while
> setting spice"
> + " property 'video-codec=%s'", invalid_codecs, value);
> + }
> + break;
> + default:
> + /* only reached in case of version mismatched */
> + error_setg(errp, "Property #%d not supported.", property);
> + }
> +}
> +
> SpiceInfo *qmp_query_spice(Error **errp)
> {
> QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
> @@ -555,6 +574,8 @@ SpiceInfo *qmp_query_spice(Error **errp)
> SPICE_QUERY_MOUSE_MODE_SERVER :
> SPICE_QUERY_MOUSE_MODE_CLIENT;
>
> + info->video_codecs = spice_server_get_video_codecs(spice_server);
> +
> /* for compatibility with the original command */
> info->has_channels = true;
> info->channels = qmp_query_spice_channels();
> --
> 2.21.0
>
>
next prev parent reply other threads:[~2019-06-21 7:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-19 12:30 [Qemu-devel] [RFC] spice-core: allow setting properties from QMP Kevin Pouget
2019-06-19 12:39 ` no-reply
2019-06-19 15:19 ` Eric Blake
2019-06-20 11:54 ` Kevin Pouget
2019-06-21 7:16 ` Frediano Ziglio [this message]
2019-06-21 7:56 ` Kevin Pouget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2139720774.23871724.1561101408712.JavaMail.zimbra@redhat.com \
--to=fziglio@redhat.com \
--cc=kpouget@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=spice-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).