>
> Hey,
>
>
> Looks good to me, a few comments below. A bit worried that the guest
> does not really know about the maximum number of monitors and about
> truncating monitors info behind its back, but I don't have actual issues
> to point out... (well, with an UMS driver, you can use xrandr to enable
> more monitors than available, the guest OS will see them, but you won't
> be able to display them with remote-viewer).
> So ACK from me with the smaller issues fixed
>
> Christophe
>
> On Fri, Jun 12, 2015 at 03:17:37PM +0100, Frediano Ziglio wrote:
> > spice-server will attempt to limit number of monitors.
> > Guest machine can send monitor list it accepts. Limiting the number sent
> > by guest will limit the number of monitors client will try to enable.
> > The guest usually see client monitors enabled and start using it so
> > not seeing client monitor won't try to enable more monitor.
> > In this case the additional monitor guest can support will always be
> > seen as heads with no attached monitors.
> > This allows limiting monitors number without changing guest drivers.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/red_dispatcher.c | 11 +++++++++++
> > server/red_dispatcher.h | 1 +
> > server/red_worker.c | 12 +++++++-----
> > server/spice-qxl.h | 3 +++
> > server/spice-server.syms | 5 +++++
> > 5 files changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> > index f5f3e52..3838bbb 100644
> > --- a/server/red_dispatcher.c
> > +++ b/server/red_dispatcher.c
> > @@ -66,6 +66,7 @@ struct RedDispatcher {
> > Ring async_commands;
> > pthread_mutex_t async_lock;
> > QXLDevSurfaceCreate surface_create;
> > + unsigned max_monitors;
>
> 'unsigned int' (applies throughout this patch)
>
Never saw a compiler complaining about it. Anyway, if is a coding style rule I'll update it.
> > };
> >
> > extern uint32_t streaming_video;
> > @@ -693,6 +694,7 @@ static void
> > red_dispatcher_monitors_config_async(RedDispatcher *dispatcher,
> > payload.base.cmd = async_command_alloc(dispatcher, message, cookie);
> > payload.monitors_config = monitors_config;
> > payload.group_id = group_id;
> > + payload.max_monitors = dispatcher->max_monitors;
> >
> > dispatcher_send_message(&dispatcher->dispatcher, message, &payload);
> > }
> > @@ -987,6 +989,13 @@ void spice_qxl_monitors_config_async(QXLInstance
> > *instance, QXLPHYSICAL monitors
> > }
> >
> > SPICE_GNUC_VISIBLE
> > +void spice_qxl_set_monitors_config_limit(QXLInstance *instance,
> > + unsigned max_monitors)
>
> set_max_monitors() maybe ? But the name you used is fine too.
> unsigned int rather than unsigned.
>
Ok, I like shorter names.
> > +{
> > + instance->st->dispatcher->max_monitors = MAX(1u, max_monitors);
> > +}
> > +
> > +SPICE_GNUC_VISIBLE
> > void spice_qxl_driver_unload(QXLInstance *instance)
> > {
> > red_dispatcher_driver_unload(instance->st->dispatcher);
> > @@ -1110,6 +1119,8 @@ void red_dispatcher_init(QXLInstance *qxl)
> > red_dispatcher->base.destroy_surface_wait =
> > qxl_worker_destroy_surface_wait;
> > red_dispatcher->base.loadvm_commands = qxl_worker_loadvm_commands;
> >
> > + red_dispatcher->max_monitors = UINT16_MAX;
> > +
>
> Why UINT16_MAX ?
>
16 bit are used for same information in different structure. But UINT_MAX will do the same.
> > qxl->st->qif->get_init_info(qxl, &init_info);
> >
> > init_data.memslot_id_bits = init_info.memslot_id_bits;
> > diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
> > index 907b7c7..70b8a62 100644
> > --- a/server/red_dispatcher.h
> > +++ b/server/red_dispatcher.h
> > @@ -200,6 +200,7 @@ typedef struct RedWorkerMessageMonitorsConfigAsync {
> > RedWorkerMessageAsync base;
> > QXLPHYSICAL monitors_config;
> > int group_id;
> > + unsigned int max_monitors;
> > } RedWorkerMessageMonitorsConfigAsync;
> >
> > typedef struct RedWorkerMessageDriverUnload {
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 5deb30b..564bfba 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -11234,11 +11234,13 @@ static inline void
> > red_monitors_config_item_add(DisplayChannelClient *dcc)
> > }
> >
> > static void worker_update_monitors_config(RedWorker *worker,
> > - QXLMonitorsConfig
> > *dev_monitors_config)
> > + QXLMonitorsConfig
> > *dev_monitors_config,
> > + unsigned max_monitors)
> > {
> > int heads_size;
> > MonitorsConfig *monitors_config;
> > int i;
> > + unsigned count = MIN(dev_monitors_config->count, max_monitors);
> >
> > monitors_config_decref(worker->monitors_config);
> >
> > @@ -11252,13 +11254,13 @@ static void
> > worker_update_monitors_config(RedWorker *worker,
> > dev_monitors_config->heads[i].width,
> > dev_monitors_config->heads[i].height);
> > }
> > - heads_size = dev_monitors_config->count * sizeof(QXLHead);
> > + heads_size = count * sizeof(QXLHead);
> > worker->monitors_config = monitors_config =
> > spice_malloc(sizeof(*monitors_config) + heads_size);
> > monitors_config->refs = 1;
> > monitors_config->worker = worker;
> > - monitors_config->count = dev_monitors_config->count;
> > - monitors_config->max_allowed = dev_monitors_config->max_allowed;
> > + monitors_config->count = count;
> > + monitors_config->max_allowed = MIN(dev_monitors_config->max_allowed,
> > max_monitors);
> > memcpy(monitors_config->heads, dev_monitors_config->heads,
> > heads_size);
> > }
> >
> > @@ -11668,7 +11670,7 @@ static void handle_dev_monitors_config_async(void
> > *opaque, void *payload)
> > dev_monitors_config->max_allowed);
> > return;
> > }
> > - worker_update_monitors_config(worker, dev_monitors_config);
> > + worker_update_monitors_config(worker, dev_monitors_config,
> > msg->max_monitors);
> > red_worker_push_monitors_config(worker);
> > }
> >
> > diff --git a/server/spice-qxl.h b/server/spice-qxl.h
> > index 31ff742..eed4b54 100644
> > --- a/server/spice-qxl.h
> > +++ b/server/spice-qxl.h
> > @@ -97,6 +97,9 @@ void spice_qxl_monitors_config_async(QXLInstance
> > *instance, QXLPHYSICAL monitors
> > int group_id, uint64_t cookie);
> > /* since spice 0.12.3 */
> > void spice_qxl_driver_unload(QXLInstance *instance);
> > +/* since spice 0.12.6 */
> > +void spice_qxl_set_monitors_config_limit(QXLInstance *instance,
> > + unsigned max_monitors);
> >
> > typedef struct QXLDrawArea {
> > uint8_t *buf;
> > diff --git a/server/spice-server.syms b/server/spice-server.syms
> > index 2193811..eba3c83 100644
> > --- a/server/spice-server.syms
> > +++ b/server/spice-server.syms
> > @@ -153,3 +153,8 @@ global:
> > spice_server_get_best_record_rate;
> > spice_server_set_record_rate;
> > } SPICE_SERVER_0.12.4;
> > +
> > +SPICE_SERVER_0.12.6 {
> > +global:
> > + spice_qxl_set_monitors_config_limit;
> > +} SPICE_SERVER_0.12.5;
> > --
> > 2.1.0
> >
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel