[Spice-devel] [RFC PATCH v2 00/20] Monitor ID rework

[Lukáš Hrázký]

Hello list,

this is the reworked second version of the Monitor ID series. The goal
of this series is to make the identification of monitors in the
monitors_config exchange and in the MousePosition messages more robust,
as well as fix the case of guest-side streaming via the
spice-streaming-agent, where the current identification mechanism is no
longer sufficient.

You can also find the patches in the following branches:
https://gitlab.freedesktop.org/lukash/spice-protocol/tree/monitors-config-v2
https://gitlab.freedesktop.org/lukash/spice-common/tree/monitors-config-v2
https://gitlab.freedesktop.org/lukash/spice-streaming-agent/tree/monitors-config-v2
https://gitlab.freedesktop.org/lukash/spice/tree/monitors-config-v2
https://gitlab.freedesktop.org/lukash/spice-gtk/tree/monitors-config-v2
https://gitlab.freedesktop.org/lukash/vd_agent/tree/monitors-config-v2


There are two issues with the IDs used currently to identify guest
monitors:

1. The IDs sent from the client in VDAgentMonitorsConfig and
MousePosition messages are equal to either `channel_id + monitor_id` or
`channel_id ? channel_id : monitor_id`. This is under the assumption
that there is either only one display_channel or more display channels
each with only a single monitor. Under this assumption the
aforementioned expressions are equivalent. It is not a reliable way to
identify monitors though.

2. In the guest-side streaming case, the ID scheme mentioned in 1. no
longer works, because these IDs are sent directly to the vd_agent and
interpreted as xrandr output IDs in the guest context. We have two main
use-cases in both of which the IDs sent to the guest do not match the
guest-side xrandr output IDs:

a) QXL device whose content is streamed with the mjpeg plugin. So there
are two displays on the client, the second one a streamed mirror of the
first. And there are two channels, each of which with one monitor, the IDs
are 0 and 1.  However, in the guest there is only a single output, ID 0.

b) The main use case, vGPU-accelerated streaming from the guest. There
usually is a QXL device to see the boot etc., the streaming only starts
with X. Therefore, the QXL monitor has ID 0 and the streamed monitor has
an ID of 1. However, the QXL monitor is not used in X, only the vGPU is
configured in X and it's first monitor under X has the ID of 0.


In summary, this patch series addresses the issues as follows:

1. It replaces the current aggregate ID sent from the client to the
server with the pair (channel_id, monitor_id). In particular, it
replaces the VDAgentMonitorsConfig message with
SpiceMsgcMainMonitorsConfig and MousePosition with MousePositionV2.

2. On the server, it translates the ID pair to the guest-side ID in
one of the following ways:

a) For the guest-side streaming case, it looks up the ID in the
monitors_guest_output_id hash table, into which the IDs are stored from
StreamInfo messages from the streaming agent (which runs in the guest
context and therefore has the correct IDs).

b) For the regular SPICE case, the calculation is still using the
sub-optimal `channel_id ? channel_id : monitor_id` for lack of better
options. This calculation is now confined in the server though and can
be more easily replaced with something better later.

3. The Messages from the server to the vd_agent now contain the
correct guest-side ID the vd_agent understands. (This involves reworking
the MonitorsConfig message to contain the ID as an item in the message
instead of relying on the array index. This is not strictly necessary
but makes for a cleaner and more explicit code.)


The code is still lacking capabilities to ensure backwards
compatibility, marked with a TODO in a couple of places and possibly
more is needed. It is meant to work only with all components having the
patches from this series. Once this is agreed upon, I'll add the
capabilities in a later version.

Some intrusive changes were needed on the client side, impacting also
the spice-gtk API. I'd especially like feedback on that, I'm not sure
it's good enough as it is.

One known thing the spice-gtk changes break is the fullscreen mode in
virt-viewer. In fullscreen mode, virt-viewer tries to configure the
monitors as soon as it starts, while now updating the monitors only work
after the monitors_config messages were received for them. My plan is to
attempt to fix this in virt-viewer, meaning building an older
virt-viewer with spice-gtk containing these changes will result in
broken fullscreen mode.

I'll also mention I had some redraw glitches and some gnome-shell
crashes during testing, which most probably were bugs unrelated to
SPICE. I decided to get the patches out there and investigate later, if
you test this and get similar issues though, let me know.

Cheers,
Lukas

-- 
2.18.0