[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH spice v2 1/2] QXL interface: add functions t
From: |
Gerd Hoffmann |
Subject: |
Re: [Qemu-devel] [RFC PATCH spice v2 1/2] QXL interface: add functions to identify monitors in the guest |
Date: |
Mon, 5 Nov 2018 14:08:08 +0100 |
User-agent: |
NeoMutt/20180716 |
On Mon, Nov 05, 2018 at 01:18:57PM +0100, Lukáš Hrázký wrote:
> On Mon, 2018-11-05 at 07:52 +0100, Gerd Hoffmann wrote:
> > > 2. Have a single function as follows:
> > >
> > > void spice_qxl_set_device_info(QXLInstance *instance,
> > > const char *device_address,
> > > uint32_t device_display_id_start,
> > > uint32_t device_display_id_count);
> >
> > How about:
> >
> > void spice_qxl_set_device_info(QXLInstance *instance,
> > const char *device_address,
> > uint32_t device_display_id);
> >
> > I don't think we need start+count:
> >
> > * For single-head devices device_display_id will be zero.
> > * For one-channel-per-head multihead devices (i.e. virtio-gpu)
> > device_display_id will enumerate the heads (so everybody can figure
> > which channel is which head).
> > * For one-channel-per-device multihead devices (i.e. qxl/linux)
> > device_display_id will be zero too. Number of heads is set via
> > spice_qxl_set_max_monitors().
>
> That requires nontrivial and unexpected logic for the one-channel-per-
> device multihead devices case. The API should be doing what it says and
> the dumber the better, this seems too smart to me...
Well, the device_display_id_count argument is redundant with
spice_qxl_set_max_monitors(). That isn't a great API either.
I can see that it simplifies the logic in spice-server if we have a
single function call instead of two. So we could deprecate
spice_qxl_set_max_monitors() in favour of your
spice_qxl_set_device_info() variant.
spice_qxl_set_max_monitors() would then basically do this:
spice_qxl_set_max_monitors(qxl, max)
{
spice_qxl_set_device_info(qxl, NULL, 0, max);
}
cheers,
Gerd