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

[Marc-André Lureau]

Hi

On Tue, Aug 21, 2018 at 2:08 PM Lukáš Hrázký <lhrazky at redhat.com> wrote:
>
> On Tue, 2018-08-21 at 13:09 +0200, Marc-André Lureau wrote:
> > 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?

In fact it should be written: channelid || monitorid imho. Is this
really so ugly? How adding more details is going to make it look less
"ugly"?

This formula isn't used or necessary to set the monitor config at the
lower level: spice-gtk, protocol etc.. The client requests a display
configuration (without specifying channel/monitor id), the
server/guest side does its best, and the client reflects the
current/selected configuration. what is wrong?

How is the client supposed to know that it should use monitor id or
multiple channel id, or which display id is associated with which gpu
etc? In general, does he need to care, can't we decide the best
configuration for him?

>
> > > > 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 he did configure it that way, it's his responsability, yes.

But then again, I am not sure this is all necessary depending on how
the problem is solved (if we expose all devices with seperate display
channel, or if we don't have to support mixing QXL & vgpu)

> 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...

Be sure I try to avoid that: breaking thing is digging deeper hole imho

> > > > 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.

We'll have to disagree then.


-- 
Marc-André Lureau