reds.c is using strncpy with a length one byte less than the
destination buffer size, and is relying on the fact that the
destination buffers are static global variables.
Now that we depend on glib, we can use g_strlcpy instead, which
avoids relying on such a subtle trick to get a nul-terminated
string.
We currently output a warning when getaddrinfo fails, but then
we go on trying to use the information it couldn't read. Make
sure we bail out of reds_init_socket if getaddrinfo fails.
spice_server_set_ticket and spice_server_set_addr get (library)
user-provided strings as arguments, and copy them to fixed-size
buffers using strncpy. However, if these strings are too long,
the copied string will not be 0-terminated, which will cause issues
later. This commit copies one byte less than the size of the
destination buffer. In both cases, this buffer is a static global
variable, so its memory will be set to 0.
red_proccess_commands calls were added after calling
guest_set_client_capabilities in order to cleanup the command ring from
old commands that the client might not be able to handle.
However, calling red_process_commands at this stage does send messages
to the client.
In addition, since setting the client capabilities at the guest is not
synchronized, emptying the command ring is not enough in order to make
sure the following commands will be supported by the client.
The call to red_proccess_commands before initializing the display
streams (the call to red_display_start_streams), caused inconsistencies
related to video streaming upon reconnecting (rhbz#883564).
I'm reverting this patch till another solution for the capabilities
mismatch is introduced.
Resolves: rhbz#883564
A Spice port channel carry arbitrary data between the Spice client and
the Spice server. It may be used to provide additional services on top
of a Spice connection. For example, a channel can be associated with
the qemu monitor for the client to interact with it, just like any
qemu chardev. Or it may be used with various protocols, such as the
Spice Controller.
A port kind is identified simply by its fqdn, such as org.qemu.monitor,
org.spice.spicy.test or org.ovirt.controller...
The channel is based on Spicevmc which simply tunnels data between
client and server, with a few additional messages.
See the description of the channel protocol in spice-common history.
The server can receive from the client agent data even when the agent
is disconnected. This can happen if the client sends the agent data
before it receives the AGENT_DISCONNECTED msg. We should receive and handle such msgs, instead
of disconnecting the client.
This bug can also lead to a server crash if the agent gets reconnected
fast enough, and it receives an agent data msg from the client before MSGC_AGENT_START.
upstream bz#55726
rhbz#881980
Internal images are just read from the surface, compressed, and sent to the client.
Then, they are destroyed. I can't find any reason for aligning their memory.
rhbz#876685
The current lz implementation does not support such bitmaps.
The following patch will actually prevent allocating stride > bpp*width
for internal images.
Previously, there was no check for the size of the message received from
the client, and all messages were read into a buffer of size 1024.
However, migration data can be bigger than 1024. In such cases, memory
corruption occurred.
red_wait_outgoing_item only waits till the currently outgoing msg is
completely sent.
red_wait_outgoing_items does the same for multi-clients. handle_dev_stop erroneously called
red_wait_outgoing_items, instead of waiting till all the items in the
pipes are sent.
This waiting is necessary because after drawables are sent to the client, we release them from the
device. The device might have been stopped due to moving to the non-live
phase of migration. Accessing the device memory during this phase can lead
to inconsistencies.
Also, MSG_MIGRATE should be the last message sent to the client, before
MSG_MIGRATE_DATA. Due to this bug, msgs were marshalled and sent after
handle_dev_stop and after handle_dev_display_migrate which sometimes led
to the release of surfaces, and inserting MSG_DISPLAY_DESTROY_SURFACE
after MSG_MIGRATE.
This patch also removes the calls to red_wait_outgoing_items, from
dev_flush_surfaces. They were unnecessary.
The current solution just copy the buffer. Currently data that is read
from the guest is always copied before sending it to the client. When we
will have ref count for these buffers, we can also use it for marshalling
the migration data.
The ref count is used in order to keep buffers that were in the write
queue and now are part of migration data, in case the char_device state
is destroyed before we complete sending the migration data.
fix: rhbz#866929
At migration destination side, we need to restore the client's surfaces
state, before sending surfaces related messages.
Before this patch, we stopped the processing of only the cmd ring, till migration data
arrived.
However, some QXL_IOs require reading and rendering the cmd ring (e.g.,
update_area). Moreover, when the device is reset, after destroying all
surfaces, we assert (in qemu) if the cmd ring is not empty (see
rhbz#866929).
This fix makes the red_worker thread wait till the migration data arrives
(or till a timeout), and not process any input from the device after the
vm is started.
We try to inject an interrupt to the vm in this case, which we cannot do
if it is stopped. Instead log this and update when vm restarts.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=870972
(that bz is on qemu, it will be cloned or just changed, not
sure yet)
New API: spice_server_set_ws_ports
This adds an optional dependency on libwebsockets. You need to get my
patched 0.0.3 version here:
git://people.freedesktop.org/~alon/libwebsockets
There is no qemu patches yet, to test change in reds.c the default value
of spice_ws_port to 5959 (for the default of spice-html5).
For testing there is an online client at
http://spice-space.org/spice-html5/spice.html
Known issues:
1. The tester (server/tests/test_display_no_ssl) gets into dropping all
data after a few seconds, I think it's an issue with the implemented
watches, but haven't figured it out.
2. libwebsocket's read interface is inverted to what our code expects,
i.e. there is no libwebsocket_read, so there is an additional copy
involved (see RedsWebSocket). This can be fixed.
3. Listening on a separate port. Since the headers are different, we
could listen on the same port (first three bytes RED/GET). I don't know
if we want to?
Todos:
1. SSL not implemented yet. Needs some thought as to how.
2. Serve spice-html5 when accessed as a http server. Nice to have.
This solves a problem with new spice-server and old qemu-kvm, where spice thinks
qif->client_monitors_config exists, while it does not exist in qemu-kvm.
Also "major > required_major" was added to the condition.
Also only the specific RedDispatcher is checked (and not all dispatchers).
The client will send 0x000000## codes for regular keys, and 0x0000##e0 codes
for extended keys. The current code which simply walks the uint32_t code in
memory order relies on the memory order being little endian, which will
clearly fail on big endian machines, this fixes this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
snd_channel_put freed "channel", and then channel->worker was accessed.
It caused segmentation faults during connections and disconnections of the client.
This caused a jenkins build failure:
snd_worker.c:148: error: redefinition of typedef 'PlaybackChannel'
snd_worker.c:126: note: previous declaration of 'PlaybackChannel' was here
The client of _get_buffer() holds a ref to the SndChannel, and we
should access that SndChannel when _put_samples() is called, not the one
that happens to currently be attached to the Interface.
When we release the SndChannel reference during
snd_disconnect_channel(), we need to set the pointer to NULL so it
doesn't get released again on client reconnect during
snd_set_playback_peer(). This can happen when a reference is held from
_playback_get_buffer().
This used to abort with spice_error. The caller currently does
not check spice_server_char_device_add_interface return value, but
it's still cleaner to report an error in this case.
Errors occurring in reds_init_ssl used to be fatal through the use
of spice_error, but this was downgraded to non-fatal spice_warning
calls recently. This means we no longer error out when invalid SSL
(certificates, ...) parameters are passed by the user.
This commit changes reds_init_ssl return value from void to int so
that errors can be reported to the caller.
We used to be aborting in such situations, but this was changed
during the big spice_error/printerr cleanup. We are currently
outputting a warning but not reporting the error with the caller
when reds_init_net fails to register listening watches with the
mainloop. As it's unlikely that things will work as expected in
such cases, better to error out of the function instead of pretending
everything is all right.
BN_new returns NULL on allocation failures. Given that we abort
on malloc allocation failures, we should also abort here. The
current code will segfault when BN_new fails as it immediatly tries
to use the NULL pointer.
If the guest supports client monitors config we pass it the
VDAgentMonitorsConfig message via the
QXLInterface::client_monitors_config api instead of via the vdagent.
Adds two functions:
- red_dispatcher_use_client_monitors_config:
check that QXLInterface supports client_monitors_config and that it's
functional.
- red_dispatcher_client_monitors_config:
send the client monitors configuration to the guest.
