Start reducing the usage of spice_new*/spice_malloc allocations.
They were designed in a similar way to GLib ones.
Now that we use GLib make sense to remove them.
However the versions we support for GLib can use different memory
allocators so we have to match g_free with GLib allocations
and spice_* ones (which uses always malloc allocator) with free().
This patch remove some easy ones.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Now the push is done automatically when a PipeItem is added
(cfr commit 5c460de1a3
"worker: push data when clients can receive them"),
forcing a push cause only network fragmentation and is required
only if you are handling data in a loop instead of using the
default loop.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This vfunc only has a RedChannelClient * argument, and most of the time,
it operates on RedChannelClient, not on RedChannel. Moreover, the only
time it's used is from RedChannelClient. This commit moves the vfunc to
RedChannelClient, which seems like a better fit for it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The leak detector we use currently is not enough to detect
some kind of leak in DisplayChannel so manually test.
These tests are enabled only when --enable-extra-checks is passed
to configure.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
RedWorker should not handle directly to client but
defer the job to DisplayChannel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
To help avoid stream.c and dcc.c to access display-channel private
structure to get the nth Stream structure pointer.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This is causing issues with potential improvements to the logging
system, and I've always found this usage a bit odd anyway.
Using spice_debug(""); was not possible as this triggers
-Wformat-zero-length warnings from our use of -Wall.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When a guest is rebooted, the QXL driver gets unloaded at some point in
the reboot process. When the driver is unloaded, the spice server sets a
single flag to FALSE: RedWorker::driver_cap_monitors_config. This flag
indicates whether the driver is capable of sending its own monitors config
messages to the client.
The only place this flag is used is when a new primary surface is created. If
this flag is true, the server assumes that the driver will send its own
monitors config very soon after the surface is created. If it's false, the
server directly sends its own temporary monitors config message to the client
based on the size of the just-created primary surface. This temporary monitors
config message always has a maximum monitor count of 1.
This flag is set to false at startup so that in early boot (e.g. vga text
mode), the server will send out these 'temporary' monitor config messages to
the client whenever the primary surface is destroyed and re-created. This
causes the client to resize its window to match the size of the guest. When the
QXL driver is loaded and starts taking over the monitors config
responsibilities, we set this flag to true and the server stops sending
monitors config messages out on its own.
If we reboot and set this flag to false, it will result in the server sending a
monitors config message to the client indicating that the guest now supports a
maximum of 1 monitor. If the guest happens to have more than one display window
open, it will destroy those extra windows because they exceed the maximum
allowed number of monitors. This means that if you reboot a guest with 4
monitors, after reboot it will only have 1 monitor.
To avoid this, we assume that if we had the ability to support multiple
monitors at some point, that will return at some point. So when the server
constructs its own monitors config message to send to the client (when the
driver_cap_monitors_config flag is false), we send the previous maximum monitor
count instead of always sending a maximum of 1.
Resolves: rhbz#1274447
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This is a particularly opaque part of the code for managing pending
Drawable operations. This patch adds documentation atempting to explain
these functions.
- glz_enc_dictionary_reset
- monitors_config_new
- red_channel_any_blocked
- red_channel_no_item_being_sent
- red_client_get_channel
are only used in the file where they are defined, so they can as well be
static.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The code that manages pending QXL Drawable operations is fairly complex
and difficult to understand. This is an attempt to start documenting
that code to save time when we have to work on this code in the future.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This commit changes all functions returning TRUE/FALSE from having an
'int' return value to 'bool'.
This way it's obvious that such a function is not going to return
anything else than TRUE or FALSE.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
config_socket is configuring the client stream socket.
As is responsibility of RedChannelClient to handle the stream
it make more sense to have the function in this object.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
[0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
This message provides a list of video codecs based on client's order
of preference.
We duplicate the video codecs array from reds.c and sort it using the
order of codecs as reference.
This message will not change an ongoing streaming but it could change
newly created streams depending the rank value of each video codec
that can be set by spice_server_set_video_codecs()
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Using spice_info() gets in the way of tests using
g_test_expect_message() as all the messages emitted using
a non-debug log level must be listed as expected, otherwise we get a
critical about an expected message not having been logged.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Use new structures and functions to implement the statistics code.
Use inline functions instead of macros for increased type-safety.
If statistics are disabled, the structures and functions become
empty. This confines the configuration-specific #defines to the
statistics implementation itself and avoids the need for #defines in
the calling functions. This greatly reduces the chance of accidentally
breaking the build for one configuration or the other. The reds option
was removed from stat_inc_counter() as it was not used.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
red_channel_client_parse() currently does roughly:
if (klass->parser) {
parsed = klass->parser(msg, msg_size, &parsed_size);
klass->handle_parsed(rcc, parsed_size, msg_type, parsed);
} else {
klass->handle_message(rcc, msg_type, msg, msg_size);
}
The handle_parsed implementation expects a void * 'parsed' argument,
which will then be cast to the correct type. There is not really a need
to provide distinct handle_parsed/handle_message vfuncs, instead we can
say that if a RedChannel subclass provides a 'parser' vfunc, then it's
'handle_message' vfunc will be called with the parsed message, otherwise
it will be called with unparsed data (ie what handle_message currently
expects).
This makes the code slightly easier to follow as messages will always be
handled by the same vfunc.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This field is used only for debugging.
Remove it reducing a bit all these "current" fields around.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
It shadows the outer one.
Renamed also the outer 'container' variable.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This patch creates display_channel_get_video_codecs() helper to let
stream.c get the video-codecs without accessing the internal structure
of DisplayChannel.
As video-codecs is a property of DisplayChannel, this change means
making this property readable as well.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make finalization of DisplayChannel consistent with other code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Including g_clear_pointer() in glib-compat.h and using it to avoid
warnings in odd situations.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The verify macro used currently has some problem
as it raise a warning in RHEL6.
Use new SPICE_VERIFY macro defined in spice-common
to avoid this issue.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
In commit beec1b41, we manually limited this property value in
_set_property(). But there's a simpler way to do it: via the param spec
for the property.
This also means that we can remove the warning log in red_worker_new()
since GObject will automatically warn if a property is assigned a value
outside of its valid range.
Change the minimum and default value for this property from 0 to 1 so
that we always have a primary surface.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
These functions were implementing the same stuff as empty
messages functions provided by RedChannel so reuse them.
The implementation seems a bit different but the result
is the same. Specifically:
- RedEmptyMsgPipeItem::msg is int while RedVerbItem::verb was
uint16_t however this data goes into the message type which
is uint16_t (a 16 bit on the network protocol);
- red_channel_client_send_empty_msg calls
red_channel_client_begin_send_message while red_marshall_verb
does not. However red_marshall_verb is called only by
cursor_channel_send_item and dcc_send_item which always
calls red_channel_client_begin_send_message.
Note that in dcc_send_item when an empty message is sent
red_channel_client_send_message_pending always returns
true;
- when a PipeItem is created red_channel_client_pipe_add_empty_msg
calls red_channel_client_push while red_pipe_add_verb does not.
This actually make very little difference as this kind of item are
never removed from the queue and a push is forced in every case
running the event handler for the stream watch (see
prepare_pipe_add and red_channel_client_event).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
No reason why RedWorker should know the capabilities of
DisplayChannel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This was introduced with 96e94c6f32
(Convert RedChannel hierarchy to GObject).
The id of CursorChannel/DisplayChannel were always 0 causing
a wrong assertion on the code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This was introduced with 96e94c6f32
(Convert RedChannel hierarchy to GObject).
The handle-acks settings was TRUE for CursorChannel and DisplayChannel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
In preparation for converting RedChannel to GObject, switch to using
RED_CHANNEL()-type macros for casting. For now they just do a regular
cast, but it helps reduce the size of the GObject patch to make it
easier to review.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Move out of red-worker.c. This requires a little bit of minor
refactoring to avoid accessing some RedWorker internals in the
constructor function, etc.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Convert the RedChannelClient heirarchy into GObjects. Since the existing
constructors could fail and return NULL, I inherited the base channel
client from GInitable, which introduces a dependency on gio.
When using private structs with GObject, there's a maximum size of (I
think) 64k, which was exceeded by some of the private structs. To avoid
this limitation I changed some members to dynamically allocated.
Attempt to use consistent naming.
Usually we use surface name for RedSurface.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Attempt to use consistent naming.
Usually we use surface name for RedSurface so make sure
code reader do not get confused using a different name
for RedSurfaceCmd.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This function deal only with Stream.
Also the name was misleading and was not clear if it detached the stream
from the DisplayChannel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
FOREACH_DCC should be more DisplayChannel related.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Instead of using a Ring (and having a ring item link in every pipe
item), store them in a GQueue. This also necesitated changing
RedCharDeviceVDIPort->priv->read_bufs to a GList as well.
Also Optimise client pipe by passing pipe position instead of data.
This avoids having the search the data scanning all the queue changing
the order of these operations from O(n) to O(1).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
