Do not offset the time attempting to fix client latency.
Client should handle it by itself.
This remove entirely the delay introduced by the server.
This avoids surely possible time drifts in the client.
The server just sends it's concept of time without trying to force any
delay. Only one end should handle this delay in an attempt to
synchronize audio and video instead that doing it in both ends.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
Changes since v1:
- more deep clean;
- remove the RFC intention;
- remove some test notes not strictly related.
Spice uses rand() to generate the random id, but qemu (at least in the case
of qemu-system-x86) fails to initialize the RNG seed (with e.g. srand()).
The result is, that every SPICE session started (by e.g. libvirtd) has the
same client_id. Usually, this is not a problem, but running something like
a SPICE proxy, relying on the client_id to correctly route connections,
this creates problems.
Fixes:
https://gitlab.com/qemu-project/qemu/-/issues/163
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Due to reds->vm_running being initialized to TRUE (since c302e12c
"spice.h: add entries for tracking vm state") the assumption in c23cbd6f
"reds: start QXL devices if VM is running" was wrong and we can't check
on vm_running until that initialization isn't on TRUE (it is that way for
backward compatibility).
Without this revert on qemu initializing spice we will have the
display_init side of qemu not yet ready and therefore respond badly when
spice sends an event as reaction to `red_qxl_start`:
"qxl_send_events: spice-server bug: guest stopped, ignoring."
At least with qemu > v2.0 as a spice consumer is not showing issues as
`red_qxl_start` will be called just after the qemu side is ready
`qemu_spice_display_start` -> `spice_server_vm_start` ... `red_qxl_start`.
Therefore - for now to avoid the current regression - Revert c23cbd6f
"reds: start QXL devices if VM is running" until that old (2012)
initialization is updated (probably an ABI change and therefore taking
some time).
Fixes: https://gitlab.freedesktop.org/spice/spice/-/issues/64
This reverts commit c23cbd6fa8.
Some systems use 32-bits, other 64-bits.
Some systems use signed integers, other unsigned integers.
Compute maximum constant based on time_t type.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
When spice_server_destroy is called with pending clients (currently
not a big issue, usually the programs using SPICE server are
exiting then), some leaks can happen.
This is due to the fact that some dispatcher messages are queued
to handle some serialization but then they are neved executed as
the entire system is closed.
Close all connections and handle all main dispatcher messages
to remove these leaks.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Since the conversion to a for range loop, there's no point to this
macro.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with google-readability-casting
https://google.github.io/styleguide/cppguide.html#Casting
Makes the operation clearer.
This commit uses const_cast where needed.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with performance-move-const-arg
Allows better optimization as the compiler does not have to deal with an
rvalue reference. Especially in C++17 where std::move can prevent copy
elision.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with modernize-use-nullptr
NULL in C++ is 0 whereas it is a void pointer in C. Avoids implicit
conversions.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with performance-for-range-copy
Avoids unnecessary copying when the loop does not modify the variable.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with performance-for-range-copy
Avoids unnecessary copying when the loop does not modify the variable.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with modernize-use-override
This can be useful as compilers can generate a compile time error when:
The base class implementation function signature changes.
The user has not created the override with the correct
signature.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Like char devices, QXL devices need to be explicily started.
For some historical reason, char devices are started when in running
state. See commi bf1d9007b. Reading that commit comments, there was a
plan to provide an API to stop/start devices invidually, but that never
happened. Whether that API would really be useful now, I wonder.
For now, just follow the char devices behaviour and start QXL devices
added when vm_running.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.uuuuucom>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Reduce code duplication.
Also this improve support for big endian machines as
agent_check_message fix also message endianess.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
Big endian machines on server are not officially supported but won't
hurt.
Messages from client are always encoded as little endian (as all
SPICE protocol).
Convert fields from little endian to host endian on some places
where numbers are used and not just binary copied.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
Remove some possible warning, like
reds.cpp:216:18: warning: 'remove_client' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]
virtual void remove_client(RedCharDeviceClientOpaque *opaque);
^
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
Some compiler could generate some warning, like
reds.cpp:2678:5: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
strcpy(buf, pass);
^
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Jakub Janků <jjanku@redhat.com>
Reduces usage of manual reference counting.
Avoids usage of new and use instead red::make_shared to both
create and own pipe items.
Use move semantic to reduce useless copies.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Provides a base class to allows RedCharDeviceVDIPort to pass
a RedPipeItem to MainChannelClient instead of having to wrap
into another item.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Most of the times each class has an associated constant so
allows to be able to define the constant at declaration time.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
This allows to:
- reuse reference counting;
- avoid having to manually call g_free to release item memory;
- assure item is initialized;
- avoids some manual casts.
It will also allows to use smart pointers.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
"password" is pretty small, allocate on stack directly.
RSA is currently 1024 bits, that is more or less 128 bytes.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Kevin Pouget <kpouget@redhat.com>
Not much used, can be retrieved from the class if needed.
There are some apparent test removal on reds.cpp.
The test "if (!reds->vdagent)" apparently disappeared but it's included in
the while.
reds->agent_dev cannot be NULL as SPICE server allows only one agent and we
are into a member of an object so it must be reds->agent_dev.get() == this.
As there's only an agent and as the interface (sin) is extracted from that
agent it must be reds->agent_dev.get() == sin->st and either reds->vdagent
== sin or reds->vdagent == NULL.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
socket_close is mapped to close under Unix but under Windows
you should call closesocket instead so call socket_close which
will map to the proper close function on both environments.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
Darwin does not have MSG_NOSIGNAL but allows to set a SO_NOSIGPIPE
option to disable sending SIGPIPE writing to closed socket.
Note that *BSD has the SO_NOSIGPIPE option but does not affect all
write calls so instead continue to use MSG_NOSIGNAL instead on
that systems.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
