On Fri, 2018-10-12 at 13:45 +0200, Gerd Hoffmann wrote:
> Future spice-server versions will call the client_monitors_config
> callback with the monitors list filtered to only include the monitors
> of the given display channel (aka QXLInstance). Luckily this is easily
> detectable at runtime, so we can prepare for that in advance and also
> make qemu compatible with both old and new spice-server versions.
>
> While being at it also use the console index instead of head number as
> array index. The later doesn't work correctly in case multiple display
> devices are present.
>
> Cc: spice-devel@xxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
FWIW LGTM :)
With a small note below.
Reviewed-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> ---
> ui/spice-display.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 2f8adb6b9f..52f8cb5ae1 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -674,10 +674,28 @@ static int interface_client_monitors_config(QXLInstance *sin,
>
> memset(&info, 0, sizeof(info));
>
> - head = qemu_console_get_head(ssd->dcl.con);
> - if (mc->num_of_monitors > head) {
> - info.width = mc->monitors[head].width;
> - info.height = mc->monitors[head].height;
> + if (mc->num_of_monitors == 1) {
> + /*
> + * New spice-server version which filters the list of monitors
> + * to only include those that belong to our display channel.
> + *
> + * single-head configuration (where filtering doesn't matter)
> + * takes this code path too.
> + */
> + info.width = mc->monitors[0].width;
> + info.height = mc->monitors[0].height;
> + } else {
> + /*
> + * Old spice-server which gives us all monitors, so we have to
> + * figure ourself which entry we need. Array index is the
> + * channel_id, which is the qemu console index, see
> + * qemu_spice_add_display_interface().
> + */
> + head = qemu_console_get_index(ssd->dcl.con);
> + if (mc->num_of_monitors > head) {
I consider it a convention to put the variable index (head) on the left
of the condition and the fixed size (mc->num_of_monitors) on the right.
So maybe swap those while touching the code.
Cheers,
Lukas
> + info.width = mc->monitors[head].width;
> + info.height = mc->monitors[head].height;
> + }
> }
>
> trace_qemu_spice_ui_info(ssd->qxl.id, info.width, info.height);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel