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

[Lukáš Hrázký]

On Tue, 2018-08-21 at 13:09 +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 21, 2018 at 11:44 AM Lukáš Hrázký <lhrazky at redhat.com> wrote:
> > > Well it's a switching point, you need to define it carefully. It may
> > > be simple or not, but it is just a condition, And the code to switch
> > > from one to the other shouldn't be so terrible.
> > 
> > It was also my first idea of a solution when I was presented the
> > problem. As I see it, there are two issues here:
> > 
> > 1. The logic used to switch something for something and when - You need
> > to define somehow you have this QXL device that is showing the boot in
> > client display 1 and then you start X and want to replace client
> > display 1 with X monitor 1. Then the user switches VTs and you need to
> > switch client display 1 back to QXL and so on.
> > 
> > There is also the possibility the user will not configure any QXL
> > device in the VM, so then you do no switching (this sounds simple :))
> 
> See my answer to Frediano for a simple suggestion.
> 
> Note that if all we want to support with the associated QXL device is
> text console, we may perhaps just drop the QXL device and use
> console/VT directly using spice ports, like I proposed in virt-viewer
> "[PATCH 00/22] Add QEMU-like UI: VT console & basic VM state" series.

That's certainly intersting. I have gone through those patches, but
didn't really understand what the VT support is about. Now I realized
what the intention is... I'll still have to study the details to see
how it blends in with the rest of the picture though.

> > 2. The actual switching code - this is well beyond my understanding of
> > the code in SPICE server, from what I understood, it is far from
> > trivial though. Frediano did some experiments and wasn't successful. In
> > theory (as that's the best I can do here :)), should be doable,
> > question is, how much complexity it would bring to the server, possibly
> > how much refactoring would be needed.
> > 
> 
> Ok
> 
> 
> > 
> > > > It's true that the user experience of having two separate displays is
> > > > not good, and we need to address that at some point. It's a bit of an
> > > > open question how this should be handled in a user-friendly way. But
> > > > I'm not sure tricky switching in the server is the answer. Feel free to
> > > > convince me, though.
> > > 
> > > From a user point of view, it seems pretty bad if you have half of
> > > your displays that are disabled. Is it really what we want to provide,
> > > even as the first step?
> > 
> > You're right, and I'm worried about it myself too.
> > 
> > On the other hand, if we go with your suggestion of switching the
> > display, the client displays are no longer 1:1 with the monitor
> > configuration on the guest, adding some nontrivial mapping logic
> > inbetween. Thus we make decissions for the user, trying to make it
> > simple for him, but we could also be too clever and get in his way.
> > 
> > I don't really know the use cases well enough to tell what's better.
> 
> Tbh, I don't think users "want" QXL & vGPU mixed together.
> 
> > > And it doesn't seem justified to change all the client side, api and
> > > protocol just because the guest X server is configured in an
> > > "uncommon" or "unsupported" configuration.
> > 
> > That is not the right way to put it. The protocol and the API is
> > flawed, this series is an attempt to fix it. If it wasn't for the flaw,
> > support for this "unsupported configuration" would be way more simple
> > than either this series or implementing the switching you suggest.
> 
> It's not flawed under the (reasonable) constrains we adopted.
> 
> The API & protocol abstracted away the channel ID/monitor ID details
> for the client. You want to expose it now, but the reasons aren't well
> justified, and you are pushing the problems to the client side while
> breaking API/protocol etc - given our experience with this, this is
> quite risky.

I'm sorry, but this statement is 100% false and in fact, it is actually
the opposite.

Please have a look at virt-viewer-display-spice.c:295 in virt-viewer
(current master):

    self = g_object_new(VIRT_VIEWER_TYPE_DISPLAY_SPICE,
                        "session", session,
                        // either monitorid is always 0 or channelid
                        // is, we can't have display (0, 2) and (2, 0)
                        // for example
                        "nth-display", channelid + monitorid,
                        NULL);

Do you see the "channelid + monitorid"? Now where are the channel
ID/monitor ID details abstracted away from the client? Where are the
problems now, if not on the client side, where you have to do this ugly
nonsense?

> > > Have you tried to teach
> > > vdagent to handle such X server setup instead (to handle the monitor
> > > config and avoid the cursor issue you described).
> > 
> > Not sure how you mean to solve the issue with this, but I'm quite
> > certain it wouldn't work.
> 
> The agent shouldn't just rely on XRandr, it could correctly associate
> a display_id with a device, knowing that some QXL devices are
> disabled. Imho, it's more an guest/agent issue than a client issue.

So you are suggesting that the vd_agent should look for a QXL device,
verify that it is not configured under X, assume that this device's
monitors precede the xrandr output IDs in the sequence and use this
information to shift the IDs it received?

If so, it's complicated, fragile, will not work for the mjpeg streaming
of a QXL device case and in general just digs us a deeper hole...

> > > What is setting up the X server config?
> > 
> > Nothing is atm., there's a fixed config for a single monitor. We still
> > need to do this.
> 
> I would explore that kind of options before breaking things.

If you're still talking about generating Xorg config for Nvidia vGPU, I
don't see how it can actually help with the monitor ID issues in
general. IMO it's mostly unrelated.