client/ contains several .cpp file which only #include a .c file
of the same name. This is unusual and seems to only be done to
get C++ name mangling on the symbols defined in the C file.
Now that all headers files in common/ use extern "C", these
wrappers are no longer useful.
Since some spice C++ code is using code from common/, the C
functions need to be marked as such for the C++ compiler, otherwise
we'll get linkage issues.
configure.ac defines a SPICEC_STATIC_LINKAGE_BDYNAMIC variable
when --enable-static-linkage is not set, but it's never used.
SPICEC_STATIC_LINKAGE_BSTATIC is used in client/, but since we
are using libtool, it might be more appropriate to use
'make LDFLAGS="-all-static"' to achieve static link.
Most uses of AC_ARG_ENABLE were buggy:
- when passing --disable-xxx, configure.ac would behave as if it
was passed --enable-xxx
- passing --enable-xxx=foo would "leak" into the summary, ie the
summary (at the end of configure) would say "xxx: foo" instead
of "xxx: yes"
This patch fixes these 2 issues.
This fixes fdo bug #32896:
"Subject in certificates is stored in following format (values separated by
comma and space):
grep Subject: server-cert.pem | awk -F": " '{print $2}'
O=REDHAT, CN=10.34.58.2
While spicec expects that values in host subject are separated only by comma:
spicec --host-subject "O=REDHAT,CN=10.34.58.2"
"
In this case, ignoring spaces make it much easier to directly copy and paste
the subject line from certificates.
When compiling spice with make CFLAGS="-g3 -ggdb3 -O0 -Wall -Werror",
the build broken because of a few unused variables/missing returns.
This patch fixes these warnings.
Commit 774e5bd36f changed
- dest->source.pixmap.x_image->data +
+ (uint8_t *)pixman_image_get_stride(dest->source.pixmap.pixman_image) +
The correct accessor to use is pixman_image_get_data. Thanks to gcc
4.6 for warning about a "different size" int to pointer conversion.
The server Makefile.am rules for marshallers generation are
prefixed with AM_V_SILENT to integrate nicely with automake silent
rules. The same AM_V_SILENT prefix isn't used in client/Makefile.am
resulting in verbose output even when automake silent mode is
enabled. This commit removes this verbosity.
spice configure.ac has some code to detect if the compiler has
a special attribute to tag some functions so that they generate a
warning when their return value isn't checked. However, this test
is broken (the gcc attribute name is "warn_unused_result", not
"__warn_unused_result__" and WARN_UNUSED_RESULT is unused anyway
since spice-protocol provides SPICE_GNUC_WARN_UNUSED_RESULT. Thus
we can just drop that block of code from configure.ac
The check this patch removes causes us to not set vdagent to NULL, nor
update the mouse mode when the guest agent disconnects when no client is
attached. Which leads to a non working mouse, and on agent reconnect a
"spice_server_char_device_add_interface: vdagent already attached" message
instead of a successful re-add of the agent interface .
This ensures that if the client or agent connects to the client-agent
"tunnel" while the other side is halfway through sending a multi part
message, the rest of the message gets discarded, and the connecting
party starts getting data at the beginning of the next message.
The agent message filter keeps track of messages as they are being send
reset the relevant filter to its initial state when one of the 2 ends
of the agent<->client "tunnel" disconnects.
read_from_vdi_port calls dispatch_vdi_port data, which will disconnect
the guest agent if it sends invalid data. It would then try to read more
data from the disconnected guest agent resulting in a NULL ptr dereference,
this patch fixes this.
write_to_vdi_port() was checking for reds->agent_state.connected to determine
wether it could write queued data. But agent_state.connected reflects if
*both* ends are connected. If the client has disconnected, but the guest agent
is still connected and some data is still pending (like a final clipboard
release from the client), then this data should be written to the guest agent.
We were calling reds_reset_vdp on client disconnect, which is not a good
idea. reds_reset_vdp does 3 things:
1) It resets the state related to reading chunks from the spicevmc virtio
port. If the client disconnects while the guest agent is in the middle
of sending a chunk, this will lead to an inconsistent state, and lots
of printing of "dispatch_vdi_port_data: invalid port" messages caused
by this inconsistent state sometimes followed by a segfault.
This can be triggered by copy and pasting something large (say
a screenshot) from the guest to the spice-gtk client, as the spice-gtk
client currently has a bug causing it to crash when receiving a multi
chunk vdagent messages. Without this patch (and with the spice-gtk bug
present) I can consistently reproduce this.
2) It clears any buffered writes from the client to the guest still pending
because the virtio port cannot consume data fast enough. Since the agent
itself is still running fine, throwing away writes for it because the
client has disconnected makes no sense. Esp, since on clean exit the
client may very well send a clipboard release message directly
before closing the connection, and this may get lost this way.
3) It sets client_agent_started to false, this is the only thing which
actually makes sense to do on client disconnect.
Note that since we no longer reset the vdp state on client disconnect, we
must now reset it on agent disconnect even if we don't have a client. So
the reds_reset_vdp call in reds_agent_remove() gets moved to the top,
above both the agent_state.connected and reds->peer checks which will
both fail in the no client case.
dispatch_vdi_port_data, was calling vdi_read_buf_release when no client
is connected to free the passed in buf. The only difference between
vdi_read_buf_release and directly adding the buffer back to the ring
with ring_add, is that vdi_read_buf_release calls read_from_vdi_port
after adding the buffer back. But dispatch_vdi_port_data only gets called
from read_from_vdi_port itself, thus this would lead to recursing into
read_from_vdi_port. read_from_vdi_port is protected against recursion and
will immediately return if called recursively. Thus calling
vdi_read_buf_release from dispatch_vdi_port_data is pointless, instead
simply putting the buffer back in the ring suffices.
Also bump SPICE_SERVER_VERSION to 0x000801 as 0.8.1 will be the
first version with the new API for this, and we need to be able to
detect the presence of this API in qemu.
Although ICCCM 2.2. Responsibilities of the Selection Owner:
http://tronche.com/gui/x/icccm/sec-2.html#s-2.2
Clearly states (about selection notify events):
The owner should set the specified selection, target, time, and property
arguments to the values received in the SelectionRequest event.
xsel sets the selection notify event target member to the incr atom when it
is going to send the clipboard data incremental, rather then setting it to
the UTF8_STRING atom (which was the target of the SelectionRequest).
Work around this (esp as it is likely other programs may get this wrong too)
and accept the incr atom as a valid target in a selection notify event.
This fixes Alon's test with running:
python -c "print list(range(1000))" | xsel -i -b
on the client.
Some apps (bad xsel, bad!) send invalid Atoms in their TARGETS property,
causing spicec to exit because of an XError. This patch makes spicec survive
this scenario.
For more info on the xsel bug, see:
https://bugzilla.redhat.com/show_bug.cgi?id=690214
clang static analyzer warned that 'len' was computed but never
used in glc_vertex2d. glc_stroke_line_dash has side effects so
we have to call it, but we don't need to save its return value
since it's not used.
When OpenGL is enabled, build fails in DisplayChannel::create_surface
because Canvas *canvas is declared twice. Remove the first
declaration to fix compilation.
In some cases rhev-m changes the hotkey for releasing the mouse grab
to ctrl + alt. This makes it impossible to send ctrl + alt + other-key
to the guest, even when using sticky alt.
What happens is:
-press alt until sticky alt activates
-release alt (but recorded state stays pressed due to sticky alt)
-press ctrl
-hotkey code sees ctrl+alt pressed, releases mouse grab
-mouse grab release code does an unpress all -> end of sticky state.
This patch makes it possible to atleast send ctrl + alt + del (or other key)
using sticky alt. Note: even with this patch it is still a bad idea to
use ctrl + alt as hotkey combi.
The current assert(reds->agent_state.connected) tiggers if when
the agent disconnected there was still data waiting to be sent (for
instance if there is a bug in the client handling clipboard and it
is killed while a large clipboard transfer is in progress). So first
call to reds_agent_remove happens from spice_server_char_device_remove_interface,
and then it is called again (triggering the assert) from free_item_data
from read_from_vdi_port because of the channel destruction.
Other option would be to not call it from one of the paths - but that
is suboptimal:
* if there is no data in the pipe, the second call never happens.
* the second call has to be there anyway, because it may fail during
parsing data from the agent.
This patch fixes a segfault on this assert when a client starts passing
from guest agent to client a large clipboard and dies in the middle. There
is still another assert happening occasionally at marshaller which I don't
have a fix for (but it doesn't seem to be related).
using updated windows libraries:
http://www.spice-space.org/download/stable/wspice-x86_08032011.ziphttp://www.spice-space.org/download/stable/wspice-x64_08032011.zip
-remove IgnoreDefaultLibraryNames="MSVCRT.lib", since pixman is now compiled
using MT threading model similar to other libraries. It used to be mistakenly
compiled with MD.
-downgrade freetype lib to 2.3.11-7, which is the one used/tested with
CEGUI 0.6.2
-pthread lib patched (InterlockedCompareExchange), so x64 client will no
longer crash on SelectClipRgn, BitBlt etc.
Current master is calling red_channel_destroy() on incoming error, but
reds Channels still references it, which causes a double free() later
on (see valgrind report below).
Instead, on error condition, do like the rest of the channels and call
reds_disconnect(), which remove the references and call shutdown(),
which then call red_channel_destroy() and finally free the channel
with red_channel_destroy().
Note: the previous code intention was certainly to be able to keep the
rest of the channels connected when input channel has errors. This is
not addressed by this patch.
red_channel_shutdown:
==29792== Invalid read of size 8
==29792== at 0x4C6F063: red_channel_shutdown (red_channel.c:460)
==29792== by 0x4C51EFA: inputs_shutdown (inputs_channel.c:463)
==29792== by 0x4C48445: reds_shatdown_channels (reds.c:539)
==29792== by 0x4C4868A: reds_disconnect (reds.c:603)
==29792== by 0x4C519E9: main_channel_on_error (main_channel.c:765)
==29792== by 0x4C6E80A: red_channel_peer_on_incoming_error (red_channel.c:215)
==29792== by 0x4C6E22D: red_peer_handle_incoming (red_channel.c:87)
==29792== by 0x4C6E551: red_channel_receive (red_channel.c:154)
==29792== by 0x4C6F329: red_channel_event (red_channel.c:531)
==29792== by 0x41CB8C: main_loop_wait (vl.c:1365)
==29792== by 0x437CDE: kvm_main_loop (qemu-kvm.c:1589)
==29792== by 0x41FE9A: main (vl.c:1411)
==29792== Address 0x30b0f6d0 is 0 bytes inside a block of size 28,648 free'd
==29792== at 0x4A05372: free (vg_replace_malloc.c:366)
==29792== by 0x4C6F032: red_channel_destroy (red_channel.c:454)
==29792== by 0x4C6E80A: red_channel_peer_on_incoming_error (red_channel.c:215)
==29792== by 0x4C6E22D: red_peer_handle_incoming (red_channel.c:87)
==29792== by 0x4C6E551: red_channel_receive (red_channel.c:154)
==29792== by 0x4C6F329: red_channel_event (red_channel.c:531)
==29792== by 0x41CB8C: main_loop_wait (vl.c:1365)
==29792== by 0x437CDE: kvm_main_loop (qemu-kvm.c:1589)
==29792== by 0x41FE9A: main (vl.c:1411)
https://bugs.freedesktop.org/show_bug.cgi?id=34971
