The 'port' argument can be determined from VDIPortState, so no need to
pass both (through RedsState) to vdi_port_read_buf_process().
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Pass RedsState variable to snd_attach_(playback|record) and get a
reference to this variable when needed by various functions.
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
Requires changing a bunch of internal API to take MainDispatcher
arguments, etc. The main dispatcher object is now owned by RedsState,
since that is the object that previously created (initialized) it.
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
Allows us to remove use of global 'reds' variable from reds-stream.c.
Requires changing the RedsStream constructor to accept a RedsState arg.
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
Since these are server-level variables, move them into RedsState.
However, num_active_workers was removed because:
- each dispatcher always has 1 active worker, so we can determine the
number of active workers by counting the dispatchers
- it was never actually set correctly. Even if there was more than one
worker, this variable was always only set to either 0 or 1.
This change required moving a bunch of helper code into RedsState as
well, an providing some RedDispatcher interfaces to access dispatcher
information from RedsState.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
Instead of using list in RedDispatcher (which is going to be removed)
use the one in RedsState.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
This duplicate the other list which will be removed
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
They are now carried in the appropriate command through
QXLReleaseInfoExt, so we don't need to pass them around explicitly.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Now it no longer needs external state in order to destroy the
RedDrawable instance when needed, so we can remove this from the
prototype.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Storing a QXLReleaseInfoExt rather than a QXLReleaseInfo in RedDrawable
gives us a place to store group_id for free. Since we are building a
QXLReleaseInfoExt from QXLReleaseInfo and group_id when destroying a
RedDrawable instance, this is a move in the right direction.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's only used in a few debug printfs. If we want these stats, we'll
have to find a better way than forcing a dependency on a high-level
class in a lower-level _unref method just to maintain the stats.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Was causing sporadic crashes.
Also cause compatibility problems with RHEL 6.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Check that cancelling a timer the timer callback is not called.
This can happen in latency code (red-channel.c).
In red_channel_client_cancel_ping_timer latency timer is cancelled and
state is set to PING_STATE_NONE however if timer was already active what
happens is that the red_channel_client_ping_timer is called and the line
spice_assert(rcc->latency_monitor.state == PING_STATE_TIMER);
is triggered causing spice-server to abort.
This happens as GLib loop add all active sources to an array but if the
timer is deactivated before the event is dispatched the event will be
dispatched unless the source is destroyed.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Add RedsState arg to main_channel_new(), and use the 'reds' property
that is stored in the base RedChannel struct rather than the global
'reds' variable.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
These automatic variable are used just inside the function.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
CommonChannel does not need to know about RedWorker.
This reduce a bit dependencies between objects.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
The only usage of this ring was to have a message when there was no
commands on the list.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Add a new 'state' property to VDIReadBuf so that we can refer back to
the VDIPortState struct from the readbuf functions.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Store a reference to the server in the SpiceCharDeviceState struct and
use that rather than the global 'reds' variable
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This allows us to use local 'reds' variables in all of the various async
callbacks rather than using the global 'reds' variable.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Rather than relying on a global 'reds' variable, we can pass the needed
instance through the callback user data.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Commit 3a66b75 changed reds_accept_ssl_connection() to expect a RedState
instance when its called, but the core->watch_add() which is calling it
was not changed accordingly. This causes a crash when connecting through
SSL.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When a client is done with drawing and sends
SPICE_MSGC_DISPLAY_GL_DRAW_DONE, or when it ends, update the number of
async counts.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
[reduce critical message to a warning; do not reset gl_draw_ongoing
- Frediano Ziglio]
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Create an async, and marshall the GL_DRAW message. Count number of
clients, and wait until gl_draw_async_count is 0 to complete the async.
The count is going to be updated in the following patch when the client
is done with the draw.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
[removed unused sent field; move gl_draw_async_count to DisplayChannel
- Frediano Ziglio]
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Go through dispatcher and marshall scanout message. Since the marshaller
and the QXL state are manipulated from different threads, add a mutex to
protect the current scanout.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
- spice_qxl_gl_scanout() to take the current scanout
- spice_qxl_gl_draw_async() to draw the scanout, is like other Spice async
functions, it takes a cookie and will return in the
QXLInterface.async_complete()
Two new fields are also added to QXLState, in order to save the current
scanout, and the pending async.
A scanout can't be updated if there are pending draw atm. Discarding
outdated draws is left as a future improvement to allow updating the
scanout without waiting for draw async to be done.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
[make QXL function names more coherent - Frediano Ziglio]
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Instead of using CPU time use a timer depending on real times.
Currently that time in the record log is not used.
However if we want to reproduce problems with stream would be useful
to have real times instead.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Now that processing is correctly restored there is no need to keep
polling to avoid main loop hangs. Reduce the polling count to 1
(just try once).
This reduce cpu usage if guests are mainly idle.
If you consider 100 guests waiting to login with cursor blinking
and considering the polling was done 200 times every 10ms (maximum)
just the cursor blinking was causing 10100 loop iterations per second
while now only 200 are needed (considering cursor blinking every
second).
Signed-by: Frediano Ziglio <figlio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make sure we process commands after we can send data to client.
If during processing we detected that there was too much data in the
clients queues the processing of commands just stop till the next
iteration.
However if all data are pushed in a single iteration of the loop
and commands were already processed there was the (remote) possibility
that the loop hangs till a separate event (like a screen resize on
client window) arrive.
I manage to reproduce and after half an hour no events arrived.
This patch detect that processing was stuck and now we can process new
commands and force a new iteration.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
The req_cmd_notification callback is called by spice-server when it
has processed all commands and wants to be notified (by a wakeup) that
new commands have been appended to the command queue.
Replay utility tries to fill the commands when it detects that
spice-server is trying to read commands but there are no more commands.
However, new commands are appended in a separate thread so if the main
red worker loop on spice-server is really tight this request can
happen.
Avoid printing any message on this condition, message will be appended
and loop woken up when replay code can do it.
Signed-by: Frediano Ziglio <figlio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
During every iteration of the main worker loop, outgoing data was pushed to
the client. However, there was no guarantee that the loop would be woken up
in every situation. So there were some conditions where the loop would stop
iterating until a new event was sent.
Currently, the events that can wake up the main worker loop include:
- data from dispatcher (including wakeups from the guest)
- data from clients
- timeouts on a stream
- other timeouts
- polling
This patch adds a new wakeup event: when we have items that are queued to
be sent to a client, we set up a watch event for writing data to the
client. If no items are waiting to be sent, this watch will be disabled.
This allows us to remove the explicit push from the main worker loop.
This has some advantages:
- it could lower latency as we don't have to wait for a polling timeout.
From my experiments using a tight loop (so not really the ideal
condition to see the improvements) the total time was reduced by 2-3%)
- helps reduce the possibility of hanging loops
- avoids having to scan all clients to detect which one can accept data.
Signed-by: Frediano Ziglio <figlio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This patch use old code to implement timer events.
The new version use some function from GLib 2.34 however some systems
(like RHEL 6) have former GLib version (RHEL 6 has GLib 2.28 installed)
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Since this is public API, we can't easily change the signature of the
function to take a RedsState argument, so instead we apply a hack and
store the reds argument inside the device state struct when the
interface is added, and retrieve it for use later when it is removed.
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Send the fd associated to the last message sent.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The default rule being "all", it builds noinst_PROGRAMS but not
check_PROGRAMS. Let's build all the tests by default.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Was used to just assure there was at least one renderer in the list.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Use the glib mainloop instead of writing our own. The glib loop is both
cleaner to use and is more extensible. It is also very mature and
reduces the maintenance burden on the spice server.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make the RedsState object own an InputsChannel object rather than
having a global inputs channel. This means changing a lot of
inputs-related API to take an InputsChannel* argument and moving the
keyboard, mouse, and tablet objects into the InputsChannel object.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
red_channel_max_pipe_size returns 0 if no client (channel disconnected)
no need to check if cursor_channel/display_channel are NULL or
connected.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
Calling cursor_channel_disconnect does not free cursor_display
so this causes a leak.
Is the only code where this pointer is reset preventing any
further cursor channel connection. If a client is lazy reading
cursor data during the flush connection is closed and further clients
won't be able to use the cursor.
This also prevents future use of a NULL pointer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
Use a custom GSource.
This to avoid having to allocate a timer all the time we add one.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Send wakeups only when a command is available.
This emulate better what a guest driver should do (append the command
to the ring and then signal).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
This increase code reuse and make Glib integration more straight forward.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This is mainly question of style.
Instead of repeating same conversion use the variable we set at the
beginning of the function.
Note also that I used same name to make the two functions more
similar.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
All checks for full channel pipes have to be consistents
so there is no point in passing as a parameter.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
If the creator was not able to produce the item, no need to call
pipe_add().
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat>
The replay file should start with a header such as
SPICE_REPLAY 1
Instead of soldiering on if we don't encounter this header, print a
warning and return NULL. Also exit with a failure if spice_replay_new()
returns a NULL object in main.
Acked-by: Pavel Grunt <pgrunt@redhat.com>
In preparation for getting rid of the global 'reds' variable, we need to
pass the RedsState variable to all functions where it is needed. For now
the callers just pass in the global reds variable.
Functions changed:
- reds_mig_fill_wait_disconnect;
- reds_mig_cleanup_wait_disconnect;
- reds_mig_remove_wait_disconnect_client;
- reds_migrate_channels_seamless;
- reds_mig_finished;
- reds_mig_switch;
- reds_enable_mm_time;
- reds_disable_mm_time;
- attach_to_red_agent;
- reds_char_device_add_state;
- reds_char_device_remove_state;
- reds_on_char_device_state_destroy;
- spice_server_char_device_remove_interface;
- migrate_timeout.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Since SpiceCoreInterfaceInternal is a private data structure, we can
extend it as we see fit without breaking ABI. In particular, adding a
GMainContext member to it allows us to remove the need for
the event loop template which is currently included in the
basic_event_loop.c test file.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
They call the functions provided by event_loop_core, but with a NULL
SpiceCoreInterfaceInternal parameter. It makes more sense to pass
event_loop_core rather than NULL.
This will allow to pass the GMainContext to be used through
SpiceCoreInterfaceInternal rather than through a template parameter.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
In preparation for getting rid of the global 'reds' variable, we need to
pass the RedsState variable to all functions where it is needed. For now
the callers just pass in the global reds variable.
Functions changed:
- reds_link_mig_target_channels;
- reds_on_migrate_dst_set_seamless;
- reds_on_client_seamless_migrate_complete;
- reds_on_client_semi_seamless_migrate_complete;
- reds_handle_other_links;
- reds_handle_link;
- reds_send_mm_time;
- reds_set_client_mm_time_latency;
- reds_init_net;
- do_spice_init;
- reds_init_ssl;
- on_activating_ticketing;
- reds_mig_release to take RedsState arg
- reds_mig_started.
Acked-by: Pavel Grunt <pgrunt@redhat.com>
In preparation for getting rid of the global 'reds' variable, we need to
pass the RedsState variable to all functions where it is needed. For now
the callers just pass in the global reds variable.
Functions changed:
- reds_on_main_migrate_connected;
- reds_on_main_mouse_mode_request;
- reds_on_main_channel_migrate;
- reds_marshall_migrate_data;
- reds_agent_state_restore;
- reds_handle_migrate_data;
- reds_send_link_ack;
- reds_mig_target_client_add;
- reds_mig_target_client_find;
- reds_mig_target_client_free;
- reds_mig_target_client_disconnect_all;
- reds_find_client;
- reds_get_client;
- reds_handle_main_link;
- reds_set_client_mouse_allowed.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This fixes a crash if red_channel_client disconnect is called
handling a message.
This can happen for instance while handling SPICE_MSGC_ACK which calls
red_channel_client_push which tries to detect write errors while writing
to a socket (for instance socket disconnection).
Messages are read in a loop and red_channel_client_disconnect would
cause rcc->stream to be NULL which will result in a use-after-free
problem (stream in red_peer_handle_incoming will use cached stream value).
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This could happen for instance if a given timer remove all clients
which have associated timers.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make sure we don't handle event reserved to other loop contexts.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
spice_warn_if_fail() is doing the same thing except for the inverted
condition. spice_warn_if() is being removed from spice-common to avoid
having potentially confusing redundancy.
More consistent with glib naming conventions. Also make the function
static since it's not used outside of this source file.
Acked-by: Pavel Grunt <pgrunt@redhat.com>
In preparation for getting rid of the global 'reds' variable, we need to
pass the RedsState variable to all functions where it is needed. For now
the callers just pass in the global reds variable.
Functions changed:
- vdi_port_read_buf_process;
- vdi_port_read_buf_get;
- vdi_port_read_buf_unref;
- reds_handle_agent_mouse_event;
- reds_num_of_channels;
- reds_num_of_clients;
- reds_fill_channels;
- reds_on_main_agent_start;
- reds_get_agent_data_buffer;
- reds_release_agent_data_buffer;
- reds_client_monitors_config_cleanup;
- red_on_main_agent_data.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
This make happy address sanitizer during make check.
Otherwise memory leak detector can keep in and make tests fails.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
This allows us to access the RedsState variable non-globally without
changing the signature of the callback functions.
Acked-by: Pavel Grunt <pgrunt@redhat.com>
In preparation for getting rid of the global 'reds' variable, we need to
pass the RedsState variable to all functions where it is needed. For now
the callers just pass in the global reds variable.
Functions changed:
- reds_register_channel;
- reds_unregister_channel;
- reds_get_mouse_mode;
- reds_set_mouse_mode;
- reds_update_mouse_mode;
- reds_agent_remove;
- reds_find_channel;
- reds_mig_cleanup;
- reds_reset_vdp;
- reds_main_channel_connected;
- reds_client_disconnect;
- reds_disconnect;
- reds_mig_disconnect.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
This allows it to be accessed from other files. This is a temporary step
toward getting rid of the global-ness of this variable, and it allows us
to update the function signature bit-by-bit.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Check that two consecutive msgfd are read back from two different reads.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
These function report memory allocation errors.
spice_malloc0 also reset memory after allocation.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Group the test utility in a library, to avoid repeating the same
sources. In this case, automake already figues out what the source of
the programs to build is.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
