[Spice-devel] [PATCH 5/9] server: dispatcher_init/dispatcher_new
Frediano Ziglio
fziglio at redhat.com
Fri Oct 23 09:36:59 PDT 2015
>
> On Thu, Oct 22, 2015 at 09:51:09AM -0400, Frediano Ziglio wrote:
> > > I tried to do these changes. Unfortunately after this setting
> > > red_dispatcher_new
> > > calls some callbacks which require qxl->st->dispatcher to be set,
> > > specifically
> > >
> > > qxl->st->qif->attache_worker(qxl, &red_dispatcher->base);
> > > qxl->st->qif->set_compression_level(qxl, calc_compression_level());
> > >
> > > I would suggest either
> > > - move these line in the caller;
> > > - ditch the patch.
> > >
> >
> > Tried to move the lines in the caller (spice_server_add_interface in
> > server/reds.c),
> > they called a call_compression_level function which is defined in
> > red_dispatcher.c.
> > Made the function not static and exported but one of the lines access
> > qxl->st->dispatcher
> > which is not defined entirely.
>
> Maybe there could have an additional red_dispatcher_attach(qxl); call
> or whatever doing the additional setup after _new() has been called?
>
> I don't mind dropping this, unless some patches later on make much more
> sense with a _new() method rather than an _init().
>
> Christophe
>
I already removed the patch, there were very few changes.
Putting more code in reds.c to initialize the instance is IMHO worst.
I'd rather move QXLState initialization in red_dispatcher_init (perhaps renaming
it to red_instance_init).
Does a patch like this make sense?
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 83aace4..70c9e48 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -54,6 +54,7 @@ struct AsyncCommand {
struct RedDispatcher {
QXLWorker base;
+ QXLState state;
QXLInstance *qxl;
Dispatcher dispatcher;
uint32_t pending;
@@ -75,6 +76,11 @@ extern spice_wan_compression_t zlib_glz_state;
static RedDispatcher *dispatchers = NULL;
+static RedDispatcher *get_qxl_dispatcher(QXLInstance *qxl)
+{
+ return SPICE_CONTAINEROF(qxl->st, RedDispatcher, state);
+}
+
static int red_dispatcher_check_qxl_version(RedDispatcher *rd, int major, int minor)
{
int qxl_major = rd->qxl->st->qif->base.major_version;
@@ -845,25 +851,25 @@ uint32_t red_dispatcher_qxl_ram_size(void)
SPICE_GNUC_VISIBLE
void spice_qxl_wakeup(QXLInstance *instance)
{
- red_dispatcher_wakeup(instance->st->dispatcher);
+ red_dispatcher_wakeup(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_oom(QXLInstance *instance)
{
- red_dispatcher_oom(instance->st->dispatcher);
+ red_dispatcher_oom(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_start(QXLInstance *instance)
{
- red_dispatcher_start(instance->st->dispatcher);
+ red_dispatcher_start(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_stop(QXLInstance *instance)
{
- red_dispatcher_stop(instance->st->dispatcher);
+ red_dispatcher_stop(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
@@ -871,133 +877,133 @@ void spice_qxl_update_area(QXLInstance *instance, uint32_t surface_id,
struct QXLRect *area, struct QXLRect *dirty_rects,
uint32_t num_dirty_rects, uint32_t clear_dirty_region)
{
- red_dispatcher_update_area(instance->st->dispatcher, surface_id, area, dirty_rects,
+ red_dispatcher_update_area(get_qxl_dispatcher(instance), surface_id, area, dirty_rects,
num_dirty_rects, clear_dirty_region);
}
SPICE_GNUC_VISIBLE
void spice_qxl_add_memslot(QXLInstance *instance, QXLDevMemSlot *slot)
{
- red_dispatcher_add_memslot(instance->st->dispatcher, slot);
+ red_dispatcher_add_memslot(get_qxl_dispatcher(instance), slot);
}
SPICE_GNUC_VISIBLE
void spice_qxl_del_memslot(QXLInstance *instance, uint32_t slot_group_id, uint32_t slot_id)
{
- red_dispatcher_del_memslot(instance->st->dispatcher, slot_group_id, slot_id);
+ red_dispatcher_del_memslot(get_qxl_dispatcher(instance), slot_group_id, slot_id);
}
SPICE_GNUC_VISIBLE
void spice_qxl_reset_memslots(QXLInstance *instance)
{
- red_dispatcher_reset_memslots(instance->st->dispatcher);
+ red_dispatcher_reset_memslots(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_destroy_surfaces(QXLInstance *instance)
{
- red_dispatcher_destroy_surfaces(instance->st->dispatcher);
+ red_dispatcher_destroy_surfaces(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_destroy_primary_surface(QXLInstance *instance, uint32_t surface_id)
{
- red_dispatcher_destroy_primary_surface(instance->st->dispatcher, surface_id, 0, 0);
+ red_dispatcher_destroy_primary_surface(get_qxl_dispatcher(instance), surface_id, 0, 0);
}
SPICE_GNUC_VISIBLE
void spice_qxl_create_primary_surface(QXLInstance *instance, uint32_t surface_id,
QXLDevSurfaceCreate *surface)
{
- red_dispatcher_create_primary_surface(instance->st->dispatcher, surface_id, surface, 0, 0);
+ red_dispatcher_create_primary_surface(get_qxl_dispatcher(instance), surface_id, surface, 0, 0);
}
SPICE_GNUC_VISIBLE
void spice_qxl_reset_image_cache(QXLInstance *instance)
{
- red_dispatcher_reset_image_cache(instance->st->dispatcher);
+ red_dispatcher_reset_image_cache(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_reset_cursor(QXLInstance *instance)
{
- red_dispatcher_reset_cursor(instance->st->dispatcher);
+ red_dispatcher_reset_cursor(get_qxl_dispatcher(instance));
}
SPICE_GNUC_VISIBLE
void spice_qxl_destroy_surface_wait(QXLInstance *instance, uint32_t surface_id)
{
- red_dispatcher_destroy_surface_wait(instance->st->dispatcher, surface_id, 0, 0);
+ red_dispatcher_destroy_surface_wait(get_qxl_dispatcher(instance), surface_id, 0, 0);
}
SPICE_GNUC_VISIBLE
void spice_qxl_loadvm_commands(QXLInstance *instance, struct QXLCommandExt *ext, uint32_t count)
{
- red_dispatcher_loadvm_commands(instance->st->dispatcher, ext, count);
+ red_dispatcher_loadvm_commands(get_qxl_dispatcher(instance), ext, count);
}
SPICE_GNUC_VISIBLE
void spice_qxl_update_area_async(QXLInstance *instance, uint32_t surface_id, QXLRect *qxl_area,
uint32_t clear_dirty_region, uint64_t cookie)
{
- red_dispatcher_update_area_async(instance->st->dispatcher, surface_id, qxl_area,
+ red_dispatcher_update_area_async(get_qxl_dispatcher(instance), surface_id, qxl_area,
clear_dirty_region, cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_add_memslot_async(QXLInstance *instance, QXLDevMemSlot *slot, uint64_t cookie)
{
- red_dispatcher_add_memslot_async(instance->st->dispatcher, slot, cookie);
+ red_dispatcher_add_memslot_async(get_qxl_dispatcher(instance), slot, cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_destroy_surfaces_async(QXLInstance *instance, uint64_t cookie)
{
- red_dispatcher_destroy_surfaces_async(instance->st->dispatcher, cookie);
+ red_dispatcher_destroy_surfaces_async(get_qxl_dispatcher(instance), cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_destroy_primary_surface_async(QXLInstance *instance, uint32_t surface_id, uint64_t cookie)
{
- red_dispatcher_destroy_primary_surface(instance->st->dispatcher, surface_id, 1, cookie);
+ red_dispatcher_destroy_primary_surface(get_qxl_dispatcher(instance), surface_id, 1, cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_create_primary_surface_async(QXLInstance *instance, uint32_t surface_id,
QXLDevSurfaceCreate *surface, uint64_t cookie)
{
- red_dispatcher_create_primary_surface(instance->st->dispatcher, surface_id, surface, 1, cookie);
+ red_dispatcher_create_primary_surface(get_qxl_dispatcher(instance), surface_id, surface, 1, cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_destroy_surface_async(QXLInstance *instance, uint32_t surface_id, uint64_t cookie)
{
- red_dispatcher_destroy_surface_wait(instance->st->dispatcher, surface_id, 1, cookie);
+ red_dispatcher_destroy_surface_wait(get_qxl_dispatcher(instance), surface_id, 1, cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_flush_surfaces_async(QXLInstance *instance, uint64_t cookie)
{
- red_dispatcher_flush_surfaces_async(instance->st->dispatcher, cookie);
+ red_dispatcher_flush_surfaces_async(get_qxl_dispatcher(instance), cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_monitors_config_async(QXLInstance *instance, QXLPHYSICAL monitors_config,
int group_id, uint64_t cookie)
{
- red_dispatcher_monitors_config_async(instance->st->dispatcher, monitors_config, group_id, cookie);
+ red_dispatcher_monitors_config_async(get_qxl_dispatcher(instance), monitors_config, group_id, cookie);
}
SPICE_GNUC_VISIBLE
void spice_qxl_set_max_monitors(QXLInstance *instance, unsigned int max_monitors)
{
- instance->st->dispatcher->max_monitors = MAX(1u, max_monitors);
+ get_qxl_dispatcher(instance)->max_monitors = MAX(1u, max_monitors);
}
SPICE_GNUC_VISIBLE
void spice_qxl_driver_unload(QXLInstance *instance)
{
- red_dispatcher_driver_unload(instance->st->dispatcher);
+ red_dispatcher_driver_unload(get_qxl_dispatcher(instance));
}
void red_dispatcher_async_complete(struct RedDispatcher *dispatcher,
@@ -1061,7 +1067,7 @@ static RedChannel *red_dispatcher_cursor_channel_create(RedDispatcher *dispatche
return cursor_channel;
}
-void red_dispatcher_init(QXLInstance *qxl)
+void red_instance_init(QXLInstance *qxl)
{
RedDispatcher *red_dispatcher;
WorkerInitData init_data;
@@ -1070,7 +1076,7 @@ void red_dispatcher_init(QXLInstance *qxl)
RedChannel *cursor_channel;
ClientCbs client_cbs = { NULL, };
- spice_return_if_fail(qxl->st->dispatcher == NULL);
+ spice_return_if_fail(qxl->st == NULL);
static gsize initialized = FALSE;
if (g_once_init_enter(&initialized)) {
@@ -1083,6 +1089,8 @@ void red_dispatcher_init(QXLInstance *qxl)
}
red_dispatcher = spice_new0(RedDispatcher, 1);
+ red_dispatcher->state.qif = SPICE_CONTAINEROF(qxl->base.sif, QXLInterface, base);
+ qxl->st = &red_dispatcher->state;
ring_init(&red_dispatcher->async_commands);
spice_debug("red_dispatcher->async_commands.next %p", red_dispatcher->async_commands.next);
dispatcher_init(&red_dispatcher->dispatcher, RED_WORKER_MESSAGE_COUNT, NULL);
@@ -1159,7 +1167,6 @@ void red_dispatcher_init(QXLInstance *qxl)
reds_register_channel(cursor_channel);
}
- qxl->st->dispatcher = red_dispatcher;
red_dispatcher->next = dispatchers;
dispatchers = red_dispatcher;
diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
index 9ee36d7..95c21e8 100644
--- a/server/red_dispatcher.h
+++ b/server/red_dispatcher.h
@@ -27,7 +27,7 @@ typedef struct RedChannelClient RedChannelClient;
typedef struct AsyncCommand AsyncCommand;
-void red_dispatcher_init(QXLInstance *qxl);
+void red_instance_init(QXLInstance *qxl);
void red_dispatcher_set_mm_time(uint32_t);
void red_dispatcher_on_ic_change(void);
diff --git a/server/reds.c b/server/reds.c
index 2aea688..e9445fa 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3154,9 +3154,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
}
qxl = SPICE_CONTAINEROF(sin, QXLInstance, base);
- qxl->st = spice_new0(QXLState, 1);
- qxl->st->qif = SPICE_CONTAINEROF(interface, QXLInterface, base);
- red_dispatcher_init(qxl);
+ red_instance_init(qxl);
} else if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) {
spice_info("SPICE_INTERFACE_TABLET");
diff --git a/server/reds.h b/server/reds.h
index a9c2df9..8969e00 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -32,7 +32,6 @@
struct QXLState {
QXLInterface *qif;
- struct RedDispatcher *dispatcher;
};
struct TunnelWorker;
Frediano
More information about the Spice-devel
mailing list