On Fri, Oct 12, 2018 at 06:46:37AM -0400, Frediano Ziglio wrote:
> >
> > Hi,
> >
> > > > 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.
> > >
> > > That's why I said is a bug, IMHO it should.
> >
> > Hmm, this should be easily detectable on qemu side I think. If
> > num_of_monitors (for a non-qxl card) is > 1, then we have an old,
> > non-filtering spice-server. If num_of_monitors == 1, then it is a new,
> > filtering spice-server. Or a single-head channel configuration, in
> > which case it doesn't matter whenever it filters or not.
> >
> > So this should be fixable without causing too much compatibility pain.
> >
> > cheers,
> > Gerd
> >
>
> Sorry, why fixing in Qemu is the bug is in spice-server?
> I really don't follow your reasoning.
I had a thinko in one of the previous messages.
qemu_console_get_head() is never correct, so qemu is buggy too.
It should be either qemu_console_get_index() (equals channel id), for
the non-filtering case, or just 0, for the filtering case, because every
virtio-gpu head has its own display channel. Patch below.
> For me Qemu should not change at all and spice-server should
> do the filtering. Is this not fixing everything?
Well, fixed qemu being able to deal with all spice-server versions
(fixed and unifixed) is certainly useful to reduce the damage caused
by this incompatible change.
The qxl version of that callback can stay as-is, the spice-server side
fix should make sure the current code continues to work when we start
supporting new configurations (qxl and non-qxl devices mixed).
cheers,
Gerd
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 2f8adb6b9f..52f8cb5ae1 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -674,10 +674,28 @@ static int
interface_client_monitors_config(QXLInstance *sin,
memset(&info, 0, sizeof(info));
- head = qemu_console_get_head(ssd->dcl.con);
- if (mc->num_of_monitors > head) {
- info.width = mc->monitors[head].width;
- info.height = mc->monitors[head].height;
+ if (mc->num_of_monitors == 1) {
+ /*
+ * New spice-server version which filters the list of monitors
+ * to only include those that belong to our display channel.
+ *
+ * single-head configuration (where filtering doesn't matter)
+ * takes this code path too.
+ */
+ info.width = mc->monitors[0].width;
+ info.height = mc->monitors[0].height;
+ } else {
+ /*
+ * Old spice-server which gives us all monitors, so we have to
+ * figure ourself which entry we need. Array index is the
+ * channel_id, which is the qemu console index, see
+ * qemu_spice_add_display_interface().
+ */
+ head = qemu_console_get_index(ssd->dcl.con);
+ if (mc->num_of_monitors > head) {
+ info.width = mc->monitors[head].width;
+ info.height = mc->monitors[head].height;
+ }
}
trace_qemu_spice_ui_info(ssd->qxl.id, info.width, info.height);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel