Re: [Qemu-devel] [Spice-devel] [RFC PATCH spice 1/2] QXL interface: add functions to identify monitors in the guest

[Gerd Hoffmann]

  Hi,

> > How does all this monitors_config work currently in case multiple
> > display channels are present?
> > 
> > There is the QXLInterface->client_monitors_config() callback.
> > 
> > There is the spice_qxl_monitors_config_async() function.
> > 
> > Both are linked to a display channel.  The server/client messages go
> > through the main channel though ...
> 
> Not really, it actually is like this:
> 
>        display channel
> server --------------> client
> 
> 
>         main channel
> client --------------> server
> 
> So the monitors_configs go each on its own display channel to the
> client, the client puts it all together and sends back an aggregated
> list on the main channel.

Ok.

> > So what happens if a message from the client arrives?  Is that
> > broadcasted as-is to all display channels?  The current qemu code
> > (interface_client_monitors_config in spice-display.c, which is used with
> > virtio-gpu) simply uses the head as index into the array, so it looks
> > like spice-server doesn't do any filtering here (like only filling in
> > the monitors which belong to the display channel).
> 
> Correct, the spice server doesn't do any filtering and sends the whole
> monitors_config to all the interfaces...

Ok.  We probably should fix interface_client_monitors_config() to use
the channel_id instead of qemu_console_get_head() then.

> > spice_qxl_monitors_config_async() is never called with virtio-gpu,
> > except when opengl is enabled.  In that case qemu simply sets
> > QXLMonitorsConfig->count to 1 and fills in QXLMonitorsConfig->head[0]
> > (see qemu_spice_gl_monitor_config in spice-display.c).
> 
> As a side note, I have no idea about opengl. I got a slight impression
> from some earlier conversation that this is actually not used, but I
> don't really know.

Not by default, but when virgl is enabled (-spice gl=on -vga virtio).

> My wild guess at this moment is, that if you don't call
> spice_qxl_monitors_config_async() with virtio-gpu, meaning you don't
> send any monitors config, a simple one containing a single monitor for
> the surface is created somewhere along the way.

Probably.

> > Which would be
> > ok in case spice-server merges the configs from all channels before
> > sending it off to the client.  Not sure this actually happens ...
> 
> A bit differently, as I said, but the configs are merged on the client,
> which should be an equivalent outcome.

Indeed.  So the qemu_spice_gl_monitor_config() function is correct then.

> > > Interstingly enough, that seems to be the ID we want to have in the
> > > device_display_id attribute.  I expect (still need to look that up, I'm
> > > out of time right now) that for virtio-gpu the ID is a bit different,
> > 
> > Keep in mind that multiple display devices don't really work right now,
> > and possibly we need to fix not only spice but qemu too.
> 
> What exactly do you mean by multiple devices not working?

qxl + vgpu (which is why we are discussing this).
qxl + virtio-gpu will have simliar issues I think.

> > I don't think we need the monitors_id in qemu then, qemu can simply use
> > the channel_id (except for the legacy qxl case).
> 
> Ok. Given the fact that the monitor_ids actually come from the driver,
> QEMU should actually know them already, right?

Yes.

> No need to pass them back anyway? (except for the virtio-gpu case,
> which doesn't send monitors_config and so a single monitor_id = 0 is
> deduced in spice server)

When virtio-gpu sends monitors_config monitor_id will be zero too,
because we have one channel per monitor.


Another story is linking display channels to device heads, i.e.
virtio-gpu registers one display channel per head, each channel
registers the same device path of course, and now you need to
figure in the guest agent which xrandr output is which head.

Simplest way would be to require display channels being registered in
order, so the channel with the smallest channel_id is head 0, ...

cheers,
  Gerd