Without this it's not obvious that a compression setting took effect.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Is possible that port 5913 is already in use as tests that uses
test_new will attempt to use ports from 5912 to 5921 so use a port
not in that range.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
test_new function attempts to detect attempts to listen to tcp ports
already in listening state detecting some messages during
spice_server_init. However the check is wrong (broken in recent
34a44d3e94 "test-display-base: Avoid spurious errors due to listen
failures") and incomplete (missing message).
To better test this conditions put some of the ports in listening
state (like with a "nc -l 5912 & nc -l 5913 &" command) and run
tests in parallel (like with a "make check -j" command).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
To set up a listening socket usually you call in sequence:
- socket;
- bind;
- listen.
If you try to bind() to a port when another socket is already
listening on that port, the bind() will fail.
However, it is possible that the bind() may succeed and the listen()
will fail, as demonstrated in the following sequence:
- socket() create socket 1;
- bind() to port N on socket 1;
- socket() create socket 2;
- bind() to port N on socket 2;
- listen() on socket 1;
- listen() on socket 2 <-- failure.
When running tests (especially multiple tests running in parallel), it
may sometimes happen that there are other tests already listening on
the port that we are trying to use. In this case, we want to ignore
this error and simply try to listen on a different port. We already
attempted to handle this scenario, but we were only ignoring bind()
errors and not listen() errors. So in the scenario mentioned above,
the listen() error was causing the entire test to fail instead of
allowing us to try to listen on another port.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Just cosmetic changes, the static function had underscores to
distinguish from the exported one which was recently renamed.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
Adds a function to create a write buffer for sending a message to
vdagent from the server to prevent code duplication.
Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Instead of having a single red_char_device_write_buffer_get function to
get both client and server buffers and decide by testing client == NULL,
have separate function for a client and for a server. The situation
should always be clear (you're either on the client or on the server
side) and you shouldn't need to parametrize that.
For the server case, add a use_token parameter instead of a separate
red_char_device_write_buffer_get_server_no_token function, as you may
want to parametrize that.
Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Avoid time adjustment issues.
For instance ticket validity can change when daylight time changes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
Do not convert RedStreamSslStatus enumeration type back to int.
This allows compilers to perform some more type safe checks.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This is a thin wrapper over g_get_monotonic_time_ms, and is called only
once, so we can call directly g_get_monotonic_time_ms instead.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Having a single QXL interface is not enough, there can be other (e.g.
streaming) display channels that make the tablet unusable. Add a check for the
number of display channels also being equal to 1. We still need the check for
QXL interaces, because the tablet only works with QXL.
Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
fix below compile error:
format '%d' expects argument of type 'int', but argument 6 has
type 'long unsigned int' [-Werror=format=]
spice compile failed on 32bit system, since upstream commit
9541cd2fe change %ld to %PRIdPTR, %PRIdPTR is %d, but argument
strm.total_out is uLong.
Signed-off-by: Changqing Li <changqing.li@windriver.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
In a comparison with current autotools build system, meson/ninja
provides a huge improvement in build speed, while keeping the same
functionalities currently available and being considered more user
friendly.
The new system coexists within the same repository with the current one,
so we can do more extensive testing of its functionality before deciding
if the old system can be removed, or for some reason, has to stay for
good.
- Meson: https://mesonbuild.com
This is the equivalent of autogen/configure step in autotools. It
generates the files that will be used by ninja to actually build the
source code.
The project has received lots of traction recently, with many GNOME
projects willing to move to this new build system. The following wiki
page has more details of the status of the many projects being ported:
https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Meson has a python-like syntax, easy to read, and the documentation
on the project is very complete, with a dedicated page on how to port
from autotools, explaining how most common use cases can be
implemented using meson.
http://mesonbuild.com/Porting-from-autotools.html
Other important sources of information:
http://mesonbuild.com/howtox.htmlhttp://mesonbuild.com/Syntax.htmlhttp://mesonbuild.com/Reference-manual.html
- Ninja: https://ninja-build.org
Ninja is the equivalent of make in an autotools setup, which actually
builds the source code. It has being used by large and complex
projects such as Google Chrome, Android and LLVM. There is not much to
say about ninja (other than it is much faster than make) because we
won't interact directly with it as much, as meson does the middle man
job here. The reasoning for creating ninja in the first place is
explained on the following post:
http://neugierig.org/software/chromium/notes/2011/02/ninja.html
Also its manual provides more in-depth information about the design
principles:
https://ninja-build.org/manual.html
- Basic workflow:
Meson package is available for most if not all distros, so, taking
Fedora as an example, we only need to run:
# dnf -y install meson ninja-build.
With Meson, building in-tree is not possible at all, so we need to
pass a directory as argument to meson where we want the build to be
done. This has the advantage of creating builds with different options
under the same parent directory, e.g.:
$ meson ./build --prefix=/usr
$ meson ./build-extra -Dextra-checks=true -Dalignment-checks=true
After configuration is done, we call ninja to actually do the build.
$ ninja -C ./build
$ ninja -C ./build install
Ninja defaults to parallel builds, and this can be changed with the -j
flag.
$ ninja -j 10 -C ./build
- Hacking:
* meson.build: Mandatory for the project root and usually found under
each directory you want something to be built.
* meson_options.txt: Options that can interfere with the result of the
build.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This commit needs an unreleased version of spice-protocol.
The revert is temporary in order to get the spice-server 0.14.1 release
out.
This reverts commit 9f5859c3ba.
The structure usage was removed from commit
2ba69f9f88
("libspice: add surface 0 support").
They were never used by Qemu.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Fixes test-stream-device after adding a log warning about an invalid
message received on the stream device, glib tests fail on unexpected
warning messages.
Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Following commit fcaf8d1a1, build from tarballs/make distcheck is broken
as spice-server-enums.h is not regenerated when building from tarballs,
and we don't have a -I$(top_srcdir) in our build flags, just
-I$(srcdir). This commit changes #include <server/spice-server-enums.h>
to #include <spice-server-enums.h> which avoids the problem fixed by
commit fcaf8d1a1 without breaking make distcheck.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When using #include "spice-server-enums.h", it will be looked up first
in the directory containing the file being build, which is going to be
$srcdir when dcc.c includes it. However, spice-server-enums.h is a
generated file, so it will be in $builddir, not in $srcdir. This most of
the time won't be causing any problems, except when you happen to have
an invalid spice-server-enums.h in $srcdir and you are doing an
out-of-tree build.
Using #include <spice-server-enums.h> instead allows the correct
spice-server-enums.h file to be looked up.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
The reason for this commit is that Meson expects all submodules to be
placed in this subdirectory, and since autotools build is more flexible
in this case, we make some small adjustments to configure.ac and
Makefile.am files to accommodate for this change.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Rather than using
#ifdef COMPRESS_DEBUG
spice_info(...);
#endif
we can #define COMPRESS_DEBUG to spice_debug() or to do nothing for a
slight readability improvement. This opportunity is used to replace
these spice_debug() calls with g_debug(). The "do nothing" macro is a bit
convoluted to ensure that we will have a compile-time check for our
g_debug args.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
There was a small regression introduced in get_compression_for_bitmap()
by f401eb07f dcc: Rewrite dcc_image_compress.
If SPICE_IMAGE_COMPRESSION_AUTO_GLZ is specified, and the bitmap has a
stride which is bigger than its width (ie it has padding), then
get_compression_for_bitmap() will return SPICE_IMAGE_COMPRESSION_OFF
while in that case, we used to use QUIC for compression.
This happens because that function in the AUTO_GLZ case first checks if
QUIC should be used, if not, it decides to use GLZ, but then decides it
can't because of the stride, so falls back to OFF, while it used to
fall back to QUIC.
This commit only slightly reworks a preexisting if (!can_lz_compress())
check so that it's unconditional rather than depending on the previous
checks having been unsuccessful.
This issue could be observed by using a spice-html5 without support for
uncompressed bitmaps with end-of-line padding by simply starting a f28
VM and connecting to it/moving the mouse cursor in it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Check that data sent to device are collapsed in a single message.
The StreamChannel object is mocked in the test.
This checks that commit dcc3f995d9
("stream-device: handle_data: send whole message") is doing the
right thing.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
If guest sent an empty data message this was not parsed correctly.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Leak detectors did not manage to find leaks, possibly as double list
have all elements likely with a pointer to them.
The reference from the agent is necessary for inserting it into
the list.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
CentOS 7 compiler generate this warning:
glz-encoder-dict.c: In function 'glz_dictionary_pre_encode':
glz-encoder-dict.c:516:30: error: 'prev_seg_id' may be used uninitialized in this function [-Werror=maybe-uninitialized]
dict->window.segs[prev_seg_id].next = seg_id;
^
glz-encoder-dict.c:492:22: note: 'prev_seg_id' was declared here
uint32_t seg_id, prev_seg_id;
^
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
G_PID_FORMAT was only added in glib 2.50.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
SPICE expects to have each frame in a single message.
So far the stream-device did not do that.
That works fine for H264 streams but not for MJPEG.
The client handles by itself MJPEG streams, and not via
gstreamer, and is assuming that a message contains the
whole frame. Since it currently not, using spice-streaming-agent
with MJPEG plugin, confuses the client which burns CPU
till it fails and keeps complaining:
"GSpice-CRITICAL **: 15:53:36.984: need more input data"
This patch fixes that, by reading the whole message from the
device (the streaming agent) and sending it over to the client.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's currently using a GPLv2+ header, which was probably a mistake given
the project overall license. It was created by a Red Hat employee, and
only modified by Red Hat employees since then, so the (c) Red Hat is
correct, and there are no other copyright holders to contact.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
self_bitmap flag is used for some complex drawing not possible
by QXL_DRAW_COPY commands. Having this flag set causes
spice-server do draw part of the screen, copy that part on new
allocated image and reduce network optimisations with no visual
changes.
Some drivers (like Windows 10 DOD) set this flag by mistake for
this command so reset it.
More details follow.
The self_bitmap flag is used for some drawing command requiring to mix
the frame buffer with some other image. For this specific
QXL_DRAW_COPY command self_bitmap is used by spice-server code during
cachine/sending (the reason for the cache is to cache images sent to
client so the relationship between the two parts of the code).
However the self_bitmap_image (an image created in spice-server if
this flags is set) is used only if src_bitmap of SpiceCopy structure
(the structure used to store the QXL_DRAW_COPY command inside
spice-server) is NULL. But in red_get_copy_ptr (red-parse-qxl.c, the
function that parse the QXL_DRAW_COPY command form the QXL device)
not having a src_bitmap is considered an error so the
self_bitmap_image won't be used.
Why this flag affects network performance?
When spice-server see this flag it update the frame buffer according
to the pending commands (commands to be sent or still to be drawn on
frame buffer). spice-server maintain a tree of commands used to reduce
rendering and command to send. More or less if a command is covering
other commands (for instance filling the entire screen with a single
color) the pending commands can be removed from the queue and not sent
to the client. However when an update of the frame buffer is requested
spice-server update the frame buffer removing the commands from the
tree but not from the client queue.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Pointers to memory allocated in user space are never NULL.
The only exception can be if you explicitly map memory at zero.
There is however no reasons for such requirement and this practise
was also removed from Linux due to security reasons.
This API looks copied from a kernel environment where valid virtual
addresses can be NULL.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
In case we pass something like "spice:mjpeg$%*" the last part is
ignore making the string parse correctly.
A single pair should end by either string terminator or pair terminator.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
