> > Ok. We probably should fix interface_client_monitors_config() to use
> > the channel_id instead of qemu_console_get_head() then.
>
> It's not that simple. This would break the QXL with multiple monitors
> per channel case.
It is that simple.
qxl doesn't use that code path and has its own version of the callback
(in qxl.c). Fixing it there is a bit more tricky.
> I think we should fix spice server to actually do the filtering and
> only send monitors_config that belongs to the device to the QXL
> interface. As Frediano mentioned, it looks more like a bug.
Only problem is changing the callback semantics changes the library api.
We could add a second version of the callback which gets called with a
filtered list (if present). Not sure this is worth the effort though.
> > > 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.
>
> I don't follow you reasoning for this conclusion... Is it correct
> because it sends a single monitor in the monitors_config?
Yes (again, qxl doesn't run this code path so it'll only see the
one-monitor-per-channel cases).
> Why is this
> function needed for the opengl case and not for regular virtio-gpu
> case?
Not fully sure why opengl needs this. Maybe because the texture can be
larger than the actual scanout (for padding reasons) so we need to
communicate the scanout rectangle.
> > 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.
>
> This is actually the reason for the
> spice_qxl_device_monitor_add_display_id(qxl, device_display_id)
> function (using this opportunity to merge the other email thread
> discussion into one).
Ah, ok. We should *not* call this thing display_id then, that'll be
confusing. Or at very least rename the function to something like ...
spice_qxl_monitor_add_device_display_id()
^^^^^^^^^^^^^^^^^ don't split this
... to make more clear this is something else than the display_id.
> > Simplest way would be to require display channels being registered in
> > order, so the channel with the smallest channel_id is head 0, ...
>
> I don't like this, you once again rely on implicit information derived
> from the order of registration of the channels. We should make this
> explicit, so that it doesn't cause trouble in the future...
Fine with me too.
I'd suggest to split the patches, one for the path and one for the
device_display_id thing (or whatever else we call this to make it less
likely being confused with display_id, even though I don't have a good
suggestion offhand).
cheers,
Gerd
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel