On Fri, 2018-10-12 at 11:27 +0200, Gerd Hoffmann wrote: > On Thu, Oct 11, 2018 at 05:37:46PM +0200, Lukáš Hrázký wrote: > > On Thu, 2018-10-11 at 17:09 +0200, Gerd Hoffmann wrote: > > > > > 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. > > > > Ok.. and what's actually the problem using qemu_console_get_head()? It > > just doesn't feel right to me using channel_id as an index into this > > array. It is not the right index strictly speaking. > > Assume you have one qxl and one virtio-gpu device (one head each), for > example: > > 00:02.0 qxl channel 0 > 00:03.0 virtio-gpu channel 1 > > So the client will assign display 0 to qxl and display 1 to virtio-gpu. > In interface_client_monitors_config() we have to pick the correct array > entry. > > When using the channel_id it works correctly. > > When using qemu_console_get_head() it doesn't work correctly, it would > use the qxl card's data. It would work if spice-server would filter the > list to only include the entries for the given display channel before > calling the ->client_monitors_config() callback. But it doesn't, we get > the complete set. > > > > > 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. > > > > That's right. But if we don't actually break any currently supported > > use cases, it might be fine? The only thing we would be breaking is > > the virtio-gpu, I think? Is that already something we don't want to > > break? > > It would break any multihead configuration which uses multiple display > channels. Yes, virtio-gpu is one case. Breaking that would not be very > nice, but maybe acceptable. The other case is multiple qxl devices > (i.e. windows guest style multihead). Breaking that is out of > question. The windows QXL driver doesn't support the monitors_config callback, at least that's what Frediano said here: https://lists.freedesktop.org/archives/spice-devel/2018-October/045965.html So I don't think this is a problem and if breaking virtio-gpu would be acceptable, we should probably consider fixing the interface... Cheers, Lukas > cheers, > Gerd > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel