On Fri, Sep 01, 2017 at 10:36:57AM +0100, Frediano Ziglio wrote: > RedQxl and RedWorker are quite bound together running > CursorChannel and DisplayChannel in a separate thread > marshalling (RedQxl) and unmarshalling and executing > (RedWorker) requests. > Make the communication between them private trying > to maintaining the implementation in these 2 files. One thing which is not exactly clear from the log is why red-worker.h is "more private" than red-qxl.h. red-qxl.h indeed provides the 'external' api, while red-worker.c takes care of the actual invocation in the thread, and thus its API is internal, and only red-qxl.h makes use of it. Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> Christophe > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > server/red-qxl.h | 241 ---------------------------------------------- > server/red-replay-qxl.c | 1 + > server/red-worker.h | 248 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 249 insertions(+), 241 deletions(-) > > Maybe messages should be moved in a separate file in order to avoid > having to include red-worker.h from red-replay-qxl.c ? > > diff --git a/server/red-qxl.h b/server/red-qxl.h > index f925f065..3ac7bf90 100644 > --- a/server/red-qxl.h > +++ b/server/red-qxl.h > @@ -35,8 +35,6 @@ void red_qxl_set_compression_level(QXLInstance *qxl, int level); > void red_qxl_stop(QXLInstance *qxl); > void red_qxl_start(QXLInstance *qxl); > uint32_t red_qxl_get_ram_size(QXLInstance *qxl); > -void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command); > -struct Dispatcher *red_qxl_get_dispatcher(QXLInstance *qxl); > gboolean red_qxl_use_client_monitors_config(QXLInstance *qxl); > gboolean red_qxl_client_monitors_config(QXLInstance *qxl, VDAgentMonitorsConfig *monitors_config); > gboolean red_qxl_get_primary_active(QXLInstance *qxl); > @@ -64,243 +62,4 @@ void red_qxl_set_client_capabilities(QXLInstance *qxl, > uint8_t client_present, > uint8_t caps[SPICE_CAPABILITIES_SIZE]); > > -typedef uint32_t RedWorkerMessage; > - > -/* Keep message order, only append new messages! > - * Replay code store enum values into save files. > - */ > -enum { > - RED_WORKER_MESSAGE_NOP, > - > - RED_WORKER_MESSAGE_UPDATE, > - RED_WORKER_MESSAGE_WAKEUP, > - RED_WORKER_MESSAGE_OOM, > - RED_WORKER_MESSAGE_READY, /* unused */ > - > - RED_WORKER_MESSAGE_DISPLAY_CONNECT, > - RED_WORKER_MESSAGE_DISPLAY_DISCONNECT, > - RED_WORKER_MESSAGE_DISPLAY_MIGRATE, > - RED_WORKER_MESSAGE_START, > - RED_WORKER_MESSAGE_STOP, > - RED_WORKER_MESSAGE_CURSOR_CONNECT, > - RED_WORKER_MESSAGE_CURSOR_DISCONNECT, > - RED_WORKER_MESSAGE_CURSOR_MIGRATE, > - RED_WORKER_MESSAGE_SET_COMPRESSION, > - RED_WORKER_MESSAGE_SET_STREAMING_VIDEO, > - RED_WORKER_MESSAGE_SET_MOUSE_MODE, > - RED_WORKER_MESSAGE_ADD_MEMSLOT, > - RED_WORKER_MESSAGE_DEL_MEMSLOT, > - RED_WORKER_MESSAGE_RESET_MEMSLOTS, > - RED_WORKER_MESSAGE_DESTROY_SURFACES, > - RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE, > - RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE, > - RED_WORKER_MESSAGE_RESET_CURSOR, > - RED_WORKER_MESSAGE_RESET_IMAGE_CACHE, > - RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT, > - RED_WORKER_MESSAGE_LOADVM_COMMANDS, > - /* async commands */ > - RED_WORKER_MESSAGE_UPDATE_ASYNC, > - RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC, > - RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC, > - RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC, > - RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC, > - RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC, > - /* suspend/windows resolution change command */ > - RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC, > - > - RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE, /* unused */ > - RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE, /* unused */ > - > - RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC, > - RED_WORKER_MESSAGE_DRIVER_UNLOAD, > - RED_WORKER_MESSAGE_GL_SCANOUT, > - RED_WORKER_MESSAGE_GL_DRAW_ASYNC, > - RED_WORKER_MESSAGE_SET_VIDEO_CODECS, > - > - /* close worker thread */ > - RED_WORKER_MESSAGE_CLOSE_WORKER, > - > - RED_WORKER_MESSAGE_COUNT // LAST > -}; > - > -typedef struct RedWorkerMessageDisplayConnect { > - RedClient * client; > - RedsStream * stream; > - RedChannelCapabilities caps; // red_worker should reset > - int migration; > -} RedWorkerMessageDisplayConnect; > - > -typedef struct RedWorkerMessageDisplayDisconnect { > - RedChannelClient *rcc; > -} RedWorkerMessageDisplayDisconnect; > - > -typedef struct RedWorkerMessageDisplayMigrate { > - RedChannelClient *rcc; > -} RedWorkerMessageDisplayMigrate; > - > -typedef struct RedWorkerMessageCursorConnect { > - RedClient *client; > - RedsStream *stream; > - int migration; > - RedChannelCapabilities caps; // red_worker should reset > -} RedWorkerMessageCursorConnect; > - > -typedef struct RedWorkerMessageCursorDisconnect { > - RedChannelClient *rcc; > -} RedWorkerMessageCursorDisconnect; > - > -typedef struct RedWorkerMessageCursorMigrate { > - RedChannelClient *rcc; > -} RedWorkerMessageCursorMigrate; > - > -typedef struct RedWorkerMessageUpdate { > - uint32_t surface_id; > - QXLRect * qxl_area; > - QXLRect * qxl_dirty_rects; > - uint32_t num_dirty_rects; > - uint32_t clear_dirty_region; > -} RedWorkerMessageUpdate; > - > -typedef struct RedWorkerMessageAsync { > - AsyncCommand *cmd; > -} RedWorkerMessageAsync; > - > -typedef struct RedWorkerMessageUpdateAsync { > - RedWorkerMessageAsync base; > - uint32_t surface_id; > - QXLRect qxl_area; > - uint32_t clear_dirty_region; > -} RedWorkerMessageUpdateAsync; > - > -typedef struct RedWorkerMessageAddMemslot { > - QXLDevMemSlot mem_slot; > -} RedWorkerMessageAddMemslot; > - > -typedef struct RedWorkerMessageAddMemslotAsync { > - RedWorkerMessageAsync base; > - QXLDevMemSlot mem_slot; > -} RedWorkerMessageAddMemslotAsync; > - > -typedef struct RedWorkerMessageDelMemslot { > - uint32_t slot_group_id; > - uint32_t slot_id; > -} RedWorkerMessageDelMemslot; > - > -typedef struct RedWorkerMessageDestroySurfaces { > -} RedWorkerMessageDestroySurfaces; > - > -typedef struct RedWorkerMessageDestroySurfacesAsync { > - RedWorkerMessageAsync base; > -} RedWorkerMessageDestroySurfacesAsync; > - > - > -typedef struct RedWorkerMessageDestroyPrimarySurface { > - uint32_t surface_id; > -} RedWorkerMessageDestroyPrimarySurface; > - > -typedef struct RedWorkerMessageDestroyPrimarySurfaceAsync { > - RedWorkerMessageAsync base; > - uint32_t surface_id; > -} RedWorkerMessageDestroyPrimarySurfaceAsync; > - > -typedef struct RedWorkerMessageCreatePrimarySurfaceAsync { > - RedWorkerMessageAsync base; > - uint32_t surface_id; > - QXLDevSurfaceCreate surface; > -} RedWorkerMessageCreatePrimarySurfaceAsync; > - > -typedef struct RedWorkerMessageCreatePrimarySurface { > - uint32_t surface_id; > - QXLDevSurfaceCreate surface; > -} RedWorkerMessageCreatePrimarySurface; > - > -typedef struct RedWorkerMessageResetImageCache { > -} RedWorkerMessageResetImageCache; > - > -typedef struct RedWorkerMessageResetCursor { > -} RedWorkerMessageResetCursor; > - > -typedef struct RedWorkerMessageWakeup { > -} RedWorkerMessageWakeup; > - > -typedef struct RedWorkerMessageOom { > -} RedWorkerMessageOom; > - > -typedef struct RedWorkerMessageStart { > -} RedWorkerMessageStart; > - > -typedef struct RedWorkerMessageFlushSurfacesAsync { > - RedWorkerMessageAsync base; > -} RedWorkerMessageFlushSurfacesAsync; > - > -typedef struct RedWorkerMessageStop { > -} RedWorkerMessageStop; > - > -/* this command is sync, so it's ok to pass a pointer */ > -typedef struct RedWorkerMessageLoadvmCommands { > - uint32_t count; > - QXLCommandExt *ext; > -} RedWorkerMessageLoadvmCommands; > - > -typedef struct RedWorkerMessageSetCompression { > - SpiceImageCompression image_compression; > -} RedWorkerMessageSetCompression; > - > -typedef struct RedWorkerMessageSetStreamingVideo { > - uint32_t streaming_video; > -} RedWorkerMessageSetStreamingVideo; > - > -typedef struct RedWorkerMessageSetVideoCodecs { > - GArray* video_codecs; > -} RedWorkerMessageSetVideoCodecs; > - > -typedef struct RedWorkerMessageSetMouseMode { > - uint32_t mode; > -} RedWorkerMessageSetMouseMode; > - > -typedef struct RedWorkerMessageDisplayChannelCreate { > -} RedWorkerMessageDisplayChannelCreate; > - > -typedef struct RedWorkerMessageCursorChannelCreate { > -} RedWorkerMessageCursorChannelCreate; > - > -typedef struct RedWorkerMessageDestroySurfaceWait { > - uint32_t surface_id; > -} RedWorkerMessageDestroySurfaceWait; > - > -typedef struct RedWorkerMessageDestroySurfaceWaitAsync { > - RedWorkerMessageAsync base; > - uint32_t surface_id; > -} RedWorkerMessageDestroySurfaceWaitAsync; > - > -typedef struct RedWorkerMessageResetMemslots { > -} RedWorkerMessageResetMemslots; > - > -typedef struct RedWorkerMessageMonitorsConfigAsync { > - RedWorkerMessageAsync base; > - QXLPHYSICAL monitors_config; > - int group_id; > - unsigned int max_monitors; > -} RedWorkerMessageMonitorsConfigAsync; > - > -typedef struct RedWorkerMessageDriverUnload { > -} RedWorkerMessageDriverUnload; > - > -typedef struct RedWorkerMessageGlScanout { > -} RedWorkerMessageGlScanout; > - > -typedef struct RedWorkerMessageClose { > -} RedWorkerMessageClose; > - > -typedef struct RedWorkerMessageGlDraw { > - SpiceMsgDisplayGlDraw draw; > -} RedWorkerMessageGlDraw; > - > -enum { > - RED_DISPATCHER_PENDING_WAKEUP, > - RED_DISPATCHER_PENDING_OOM, > -}; > - > -void red_qxl_clear_pending(QXLState *qxl_state, int pending); > - > #endif /* RED_QXL_H_ */ > diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c > index 6172ed22..2f31e9cd 100644 > --- a/server/red-replay-qxl.c > +++ b/server/red-replay-qxl.c > @@ -26,6 +26,7 @@ > > #include "reds.h" > #include "red-qxl.h" > +#include "red-worker.h" > #include "red-common.h" > #include "memslot.h" > #include "red-parse-qxl.h" > diff --git a/server/red-worker.h b/server/red-worker.h > index 8ec28f14..4f64b729 100644 > --- a/server/red-worker.h > +++ b/server/red-worker.h > @@ -15,6 +15,12 @@ > License along with this library; if not, see <http://www.gnu.org/licenses/>. > */ > > +/* This header should contains internal details between RedQxl and > + * RedWorker. > + * Should be included only by red-worker.c, red-qxl.c and > + * red-replay-qxl.c (which uses message values). > + */ > + > #ifndef RED_WORKER_H_ > #define RED_WORKER_H_ > > @@ -31,4 +37,246 @@ RedWorker* red_worker_new(QXLInstance *qxl, > bool red_worker_run(RedWorker *worker); > void red_worker_free(RedWorker *worker); > > +void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command); > +struct Dispatcher *red_qxl_get_dispatcher(QXLInstance *qxl); > + > +typedef uint32_t RedWorkerMessage; > + > +/* Keep message order, only append new messages! > + * Replay code store enum values into save files. > + */ > +enum { > + RED_WORKER_MESSAGE_NOP, > + > + RED_WORKER_MESSAGE_UPDATE, > + RED_WORKER_MESSAGE_WAKEUP, > + RED_WORKER_MESSAGE_OOM, > + RED_WORKER_MESSAGE_READY, /* unused */ > + > + RED_WORKER_MESSAGE_DISPLAY_CONNECT, > + RED_WORKER_MESSAGE_DISPLAY_DISCONNECT, > + RED_WORKER_MESSAGE_DISPLAY_MIGRATE, > + RED_WORKER_MESSAGE_START, > + RED_WORKER_MESSAGE_STOP, > + RED_WORKER_MESSAGE_CURSOR_CONNECT, > + RED_WORKER_MESSAGE_CURSOR_DISCONNECT, > + RED_WORKER_MESSAGE_CURSOR_MIGRATE, > + RED_WORKER_MESSAGE_SET_COMPRESSION, > + RED_WORKER_MESSAGE_SET_STREAMING_VIDEO, > + RED_WORKER_MESSAGE_SET_MOUSE_MODE, > + RED_WORKER_MESSAGE_ADD_MEMSLOT, > + RED_WORKER_MESSAGE_DEL_MEMSLOT, > + RED_WORKER_MESSAGE_RESET_MEMSLOTS, > + RED_WORKER_MESSAGE_DESTROY_SURFACES, > + RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE, > + RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE, > + RED_WORKER_MESSAGE_RESET_CURSOR, > + RED_WORKER_MESSAGE_RESET_IMAGE_CACHE, > + RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT, > + RED_WORKER_MESSAGE_LOADVM_COMMANDS, > + /* async commands */ > + RED_WORKER_MESSAGE_UPDATE_ASYNC, > + RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC, > + RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC, > + RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC, > + RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC, > + RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC, > + /* suspend/windows resolution change command */ > + RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC, > + > + RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE, /* unused */ > + RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE, /* unused */ > + > + RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC, > + RED_WORKER_MESSAGE_DRIVER_UNLOAD, > + RED_WORKER_MESSAGE_GL_SCANOUT, > + RED_WORKER_MESSAGE_GL_DRAW_ASYNC, > + RED_WORKER_MESSAGE_SET_VIDEO_CODECS, > + > + /* close worker thread */ > + RED_WORKER_MESSAGE_CLOSE_WORKER, > + > + RED_WORKER_MESSAGE_COUNT // LAST > +}; > + > +typedef struct RedWorkerMessageDisplayConnect { > + RedClient * client; > + RedsStream * stream; > + RedChannelCapabilities caps; // red_worker should reset > + int migration; > +} RedWorkerMessageDisplayConnect; > + > +typedef struct RedWorkerMessageDisplayDisconnect { > + RedChannelClient *rcc; > +} RedWorkerMessageDisplayDisconnect; > + > +typedef struct RedWorkerMessageDisplayMigrate { > + RedChannelClient *rcc; > +} RedWorkerMessageDisplayMigrate; > + > +typedef struct RedWorkerMessageCursorConnect { > + RedClient *client; > + RedsStream *stream; > + int migration; > + RedChannelCapabilities caps; // red_worker should reset > +} RedWorkerMessageCursorConnect; > + > +typedef struct RedWorkerMessageCursorDisconnect { > + RedChannelClient *rcc; > +} RedWorkerMessageCursorDisconnect; > + > +typedef struct RedWorkerMessageCursorMigrate { > + RedChannelClient *rcc; > +} RedWorkerMessageCursorMigrate; > + > +typedef struct RedWorkerMessageUpdate { > + uint32_t surface_id; > + QXLRect * qxl_area; > + QXLRect * qxl_dirty_rects; > + uint32_t num_dirty_rects; > + uint32_t clear_dirty_region; > +} RedWorkerMessageUpdate; > + > +typedef struct RedWorkerMessageAsync { > + AsyncCommand *cmd; > +} RedWorkerMessageAsync; > + > +typedef struct RedWorkerMessageUpdateAsync { > + RedWorkerMessageAsync base; > + uint32_t surface_id; > + QXLRect qxl_area; > + uint32_t clear_dirty_region; > +} RedWorkerMessageUpdateAsync; > + > +typedef struct RedWorkerMessageAddMemslot { > + QXLDevMemSlot mem_slot; > +} RedWorkerMessageAddMemslot; > + > +typedef struct RedWorkerMessageAddMemslotAsync { > + RedWorkerMessageAsync base; > + QXLDevMemSlot mem_slot; > +} RedWorkerMessageAddMemslotAsync; > + > +typedef struct RedWorkerMessageDelMemslot { > + uint32_t slot_group_id; > + uint32_t slot_id; > +} RedWorkerMessageDelMemslot; > + > +typedef struct RedWorkerMessageDestroySurfaces { > +} RedWorkerMessageDestroySurfaces; > + > +typedef struct RedWorkerMessageDestroySurfacesAsync { > + RedWorkerMessageAsync base; > +} RedWorkerMessageDestroySurfacesAsync; > + > + > +typedef struct RedWorkerMessageDestroyPrimarySurface { > + uint32_t surface_id; > +} RedWorkerMessageDestroyPrimarySurface; > + > +typedef struct RedWorkerMessageDestroyPrimarySurfaceAsync { > + RedWorkerMessageAsync base; > + uint32_t surface_id; > +} RedWorkerMessageDestroyPrimarySurfaceAsync; > + > +typedef struct RedWorkerMessageCreatePrimarySurfaceAsync { > + RedWorkerMessageAsync base; > + uint32_t surface_id; > + QXLDevSurfaceCreate surface; > +} RedWorkerMessageCreatePrimarySurfaceAsync; > + > +typedef struct RedWorkerMessageCreatePrimarySurface { > + uint32_t surface_id; > + QXLDevSurfaceCreate surface; > +} RedWorkerMessageCreatePrimarySurface; > + > +typedef struct RedWorkerMessageResetImageCache { > +} RedWorkerMessageResetImageCache; > + > +typedef struct RedWorkerMessageResetCursor { > +} RedWorkerMessageResetCursor; > + > +typedef struct RedWorkerMessageWakeup { > +} RedWorkerMessageWakeup; > + > +typedef struct RedWorkerMessageOom { > +} RedWorkerMessageOom; > + > +typedef struct RedWorkerMessageStart { > +} RedWorkerMessageStart; > + > +typedef struct RedWorkerMessageFlushSurfacesAsync { > + RedWorkerMessageAsync base; > +} RedWorkerMessageFlushSurfacesAsync; > + > +typedef struct RedWorkerMessageStop { > +} RedWorkerMessageStop; > + > +/* this command is sync, so it's ok to pass a pointer */ > +typedef struct RedWorkerMessageLoadvmCommands { > + uint32_t count; > + QXLCommandExt *ext; > +} RedWorkerMessageLoadvmCommands; > + > +typedef struct RedWorkerMessageSetCompression { > + SpiceImageCompression image_compression; > +} RedWorkerMessageSetCompression; > + > +typedef struct RedWorkerMessageSetStreamingVideo { > + uint32_t streaming_video; > +} RedWorkerMessageSetStreamingVideo; > + > +typedef struct RedWorkerMessageSetVideoCodecs { > + GArray* video_codecs; > +} RedWorkerMessageSetVideoCodecs; > + > +typedef struct RedWorkerMessageSetMouseMode { > + uint32_t mode; > +} RedWorkerMessageSetMouseMode; > + > +typedef struct RedWorkerMessageDisplayChannelCreate { > +} RedWorkerMessageDisplayChannelCreate; > + > +typedef struct RedWorkerMessageCursorChannelCreate { > +} RedWorkerMessageCursorChannelCreate; > + > +typedef struct RedWorkerMessageDestroySurfaceWait { > + uint32_t surface_id; > +} RedWorkerMessageDestroySurfaceWait; > + > +typedef struct RedWorkerMessageDestroySurfaceWaitAsync { > + RedWorkerMessageAsync base; > + uint32_t surface_id; > +} RedWorkerMessageDestroySurfaceWaitAsync; > + > +typedef struct RedWorkerMessageResetMemslots { > +} RedWorkerMessageResetMemslots; > + > +typedef struct RedWorkerMessageMonitorsConfigAsync { > + RedWorkerMessageAsync base; > + QXLPHYSICAL monitors_config; > + int group_id; > + unsigned int max_monitors; > +} RedWorkerMessageMonitorsConfigAsync; > + > +typedef struct RedWorkerMessageDriverUnload { > +} RedWorkerMessageDriverUnload; > + > +typedef struct RedWorkerMessageGlScanout { > +} RedWorkerMessageGlScanout; > + > +typedef struct RedWorkerMessageClose { > +} RedWorkerMessageClose; > + > +typedef struct RedWorkerMessageGlDraw { > + SpiceMsgDisplayGlDraw draw; > +} RedWorkerMessageGlDraw; > + > +enum { > + RED_DISPATCHER_PENDING_WAKEUP, > + RED_DISPATCHER_PENDING_OOM, > +}; > + > +void red_qxl_clear_pending(QXLState *qxl_state, int pending); > + > #endif /* RED_WORKER_H_ */ > -- > 2.13.5 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel