All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: "Lukáš Hrázký" <lhrazky@redhat.com>
Cc: Jonathon Jongsma <jjongsma@redhat.com>,
	spice-devel@lists.freedesktop.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Spice-devel] [RFC PATCH spice 1/2] QXL interface: add functions to identify monitors in the guest
Date: Fri, 12 Oct 2018 11:27:00 +0200	[thread overview]
Message-ID: <20181012092700.np256rvz6octswia@sirius.home.kraxel.org> (raw)
In-Reply-To: <1539272266.16655.154.camel@redhat.com>

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.

cheers,
  Gerd

  reply	other threads:[~2018-10-12  9:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-09 13:10 [Qemu-devel] [RFC PATCH spice/qemu 0/2] QXL interface to set monitor ID Lukáš Hrázký
2018-10-09 13:10 ` [Qemu-devel] [RFC PATCH spice 1/2] QXL interface: add functions to identify monitors in the guest Lukáš Hrázký
2018-10-09 19:33   ` [Qemu-devel] [Spice-devel] " Jonathon Jongsma
2018-10-10 10:37     ` Gerd Hoffmann
2018-10-11 12:55       ` Lukáš Hrázký
2018-10-11 13:20         ` Gerd Hoffmann
2018-10-11 13:31           ` Lukáš Hrázký
2018-10-11 13:45             ` Gerd Hoffmann
2018-10-10 16:36     ` Lukáš Hrázký
2018-10-11 12:27       ` Gerd Hoffmann
2018-10-11 13:07         ` Frediano Ziglio
2018-10-11 13:12         ` Lukáš Hrázký
2018-10-11 13:43           ` Gerd Hoffmann
2018-10-11 14:30             ` Lukáš Hrázký
2018-10-11 15:09               ` Gerd Hoffmann
2018-10-11 15:37                 ` Lukáš Hrázký
2018-10-12  9:27                   ` Gerd Hoffmann [this message]
2018-10-12  9:54                     ` Lukáš Hrázký
2018-10-12 10:15                     ` Frediano Ziglio
2018-10-12 10:42                       ` Gerd Hoffmann
2018-10-12 10:46                         ` Frediano Ziglio
2018-10-12 11:05                           ` Gerd Hoffmann
2018-10-09 13:10 ` [Qemu-devel] [RFC PATCH qemu 2/2] spice: set PCI path and device display ID in QXL interface Lukáš Hrázký

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181012092700.np256rvz6octswia@sirius.home.kraxel.org \
    --to=kraxel@redhat.com \
    --cc=jjongsma@redhat.com \
    --cc=lhrazky@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=spice-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.