[Spice-devel] [PATCH 1/2] server: allows to set maximum monitors
Christophe Fergeau
cfergeau at redhat.com
Thu Jun 18 06:03:37 PDT 2015
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 at redhat.com>
> ---
> 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)
> };
>
> 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.
> +{
> + 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 ?
> 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
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150618/fb95f456/attachment-0001.sig>
More information about the Spice-devel
mailing list