Commit Graph

4280 Commits

Author SHA1 Message Date
Victor Toso
a3bf9f06b3 docs: include protocol and for-newbies documents
Only by building and sharing the documents we will be able to get them
up to date.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-10-14 13:48:49 +01:00
Frediano Ziglio
43fa3b549b docs: Update many names in spice_protocol.txt
Using an old "renames" file found in spice-protocol repository
I update some old names in the documentation protocol.
Also updated some other names manually.
I processed the file and fixed some code indentation.
File looks much more up to date.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-14 12:38:30 +01:00
Frediano Ziglio
5308ab84b9 event-loop: Change internal core interface
Allow to modify/cancel timers/watches without having to retrieve
the code interface.
This will make sure that you are not using the wrong interface.
Simplify code to deal with timers/watches.
Remove the requirement to have the core interface available
for removing timers/watches.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:55:52 +01:00
Frediano Ziglio
97d2d1f129 event-loop: Move adapter interface from reds.c
Put more event loop code in event-loop.c.
This is a preparation patch for the next one.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:55:38 +01:00
Frediano Ziglio
ebe676697f smartcard: Reset vheader value
The buffer could change inside smartcard_read_buf_prepare.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:51:59 +01:00
Frediano Ziglio
60a61be549 smartcard: Fix statement termination
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:51:48 +01:00
Frediano Ziglio
d8d5c48ad6 test-smardcard: Improve test coverage
Using coverage utility exercise more code paths:
- message from channel with wrong type;
- remove message from channel with already removed reader;
- init message from channel (ignored);
- data from devices, ADPU;
- error from device;
- messages split in different ways;
- invalid reader_id values.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:25 +01:00
Frediano Ziglio
344ce666cf test-smartcard: Add test for Smartcard device
Create Smardcard device.
Connect to it and test some messages are parsed and processed
as expected.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:25 +01:00
Frediano Ziglio
b4e508880a test-stream-device: Factor out VMC emulation
Allows to reuse code for emulating a character device.
It will be used for Smardcard test.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:25 +01:00
Frediano Ziglio
07d9328f89 smartcard: Fix parsing multiple messages from the device
This patch handles the scenario when a single read to guest device
brings multiple requests to be handled. When this happens, we will
iterate till all requests are handled and no more requests can be read
from guest device.

If the remaining buffer contains a full request we don't need to read
other bytes (note that there could be no bytes left), just parse the
request.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:22 +01:00
Frediano Ziglio
6fa12db104 smartcard: Fix copying remaining request
Use memmove instead of memcpy as the buffer can overlap if the second
request if bigger than the first.
"buf_pos" points to the point of the buffer after we read, if we want
the first part of the next request is "buf_pos - remaining".
Same consideration setting "buf_pos" for the next iteration.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:17 +01:00
Frediano Ziglio
57d02c2d17 smartcard: Do not crash if reader_id is invalid
Avoid client to trigger crash. The value of smartcard_readers_get
is checked for NULL so returning it it's not an issue.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 08:53:10 +01:00
Frediano Ziglio
18877d1af8 smartcard-channel-client: Use log instead of printf
More coherent. Also it's not good for a library to output on
standard output.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 08:53:06 +01:00
Frediano Ziglio
8d4d4e3a39 smartcard-channel-client: Remove properties code
Not used

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 08:45:25 +01:00
Frediano Ziglio
d546696424 red-channel-client: Use SpiceMsgcAckSync structure
red_channel_client_handle_message is called after parsing the
message so it's not necessary to check it again or parse manually.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 17:12:59 +01:00
Frediano Ziglio
2b56398c34 red-channel-client: Remove useless check
Message is checked by generated message parser.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 17:12:59 +01:00
Frediano Ziglio
774d466cf8 red-channel: Make parser function compulsory
As base messages require parsing better channels should always use
the generated parser.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 17:12:59 +01:00
Frediano Ziglio
bae2d1f00f smartcard: Use generated parse for messages
The generated code handle possible endianess mismatch and check
for message format.
The copy back to "write_buf" allows to use that buffer to send
data back to device.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 16:01:56 +01:00
Frediano Ziglio
5d2b204e48 Update spice-common submodule
This brings in the following changes:

Frediano Ziglio (1):
      proto: Demarshal Smartcard data field

Kevin Pouget (1):
      common/recorder.h: do not complain on unused (dummy) recorders

This is in preparation to use the generated code for Smartcard
(currently not used so won't create regressions).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 16:01:56 +01:00
Frediano Ziglio
33d81bc3fa smartcard-channel-client: Remove unused parameter
"name" parameter of smartcard_channel_client_add_reader it's not
used.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 15:34:14 +01:00
Frediano Ziglio
6c5b7017ee ci: Update gitlab makecheck-centos Job to support CentOS 8
Disable celt0.51, now obsolete.
Update package names and repositories.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-04 15:39:00 +01:00
Victor Toso
e86d4e3e48 tests: migrate: fix migration with --vdagent option
Before this patch, running the test with --vdagent option would error
in the second migration attempt with:

 | qemu-system-x86_64: Unknown savevm section or instance
 | '0000:00:04.0/virtio-console' 0. Make sure that your current VM setup
 | matches your saved VM setup, including any hotplugged devices

The reason is that target host created for migration was lacking the
configuration for vdagent that is present in the first source/target
migration VMs.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-10-04 13:32:13 +02:00
Victor Toso
6777ec31f1 tests: migrate: wait user launch client option
Useful to test different clients running different tools (gdb,
valgrind).

Signed-off-by: Victor Toso <victortoso@redhat.com>
2019-10-04 13:31:55 +02:00
Victor Toso
878d3bd1ab tests: migrate: default to not launch client
This supports doing migration without any client to be connected.
If tester wants client, it needs to pass an option to --client.

Change of default still is to keep the test as simple as possible
when no arguments are given.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-10-04 13:31:20 +02:00
Victor Toso
bc928ad2a8 tests: migrate: add counter for tests
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:53 +02:00
Victor Toso
37507c928a tests: migrate: add option --wait-user-input
The iterate() method already considers it. This is useful if one wants
to attach gdb on qemu for instance.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:53 +02:00
Victor Toso
4241806de7 tests: migrate: bool instead of on/off option in cmd line
Simpler. Make the default to be False as well as
 1) No args should run as simple as possible
 2) True is currently broken

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:53 +02:00
Victor Toso
0d351864b1 tests: migrate: remove multiple client option
Not supported feature to be tested so reduce unused/untested code for
now.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:48 +02:00
Victor Toso
6ca34325f5 tests: migrate: fix relative qmp.py path
Moved in qemu, see:

 | commit 8f8fd9edba4bd6768da2c8e2bea49ad5c16ced1a
 | Author: Cleber Rosa <crosa@redhat.com>
 | Date:   Wed Feb 6 11:29:01 2019 -0500
 |
 |     Introduce a Python module structure
 |
 |     This is a simple move of Python code that wraps common QEMU
 |     functionality, and are used by a number of different tests and
 |     scripts.
 |
 |     By treating that code as a real Python module, we can more easily:
 |      * reuse code
 |      * have a proper place for the module's own unittests
 |      * apply a more consistent style
 |      * generate documentation

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:07 +02:00
Victor Toso
973779961f tests: migrate: add support to run with remote-viewer
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:07 +02:00
Victor Toso
a0a310befe tests: migrate: use uri for default's spicy client
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:07 +02:00
Victor Toso
f82cf87e65 tests: migrate: remove spicec option
Removed, see:

 | commit 1876971442
 | Author: Christophe Fergeau <cfergeau@redhat.com>
 | Date:   Fri Nov 21 11:01:17 2014 +0100
 |
 |     client: Remove client code
 |
 |     The client has been superseded by virt-viewer (
 |     http://virt-manager.org/download/sources/virt-viewer/ )
 |     and is no longer being maintained.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-25 14:44:07 +02:00
Frediano Ziglio
abbd985c78 spicevmc: Fix g_object_new call for 32 bit machines
"self-tokens" property is 64 bit and must be passed as 64 bit on
32 bit machines to avoid memory corruptions.
This was introduced by 01de3b8922 ("spicevmc: Avoids DoS if
guest device is not able to get data faster enough"), detected by CI.

It caused this error (split into multiple lines):

  (./test-leaks:15879): GLib-GObject-CRITICAL **: 14:03:59.650: \
  g_object_new_is_valid_property: object class 'RedCharDeviceSpiceVmc' has \
  no property named '\xb0/@\xf3\u0001'

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 16:22:46 +01:00
Frediano Ziglio
01de3b8922 spicevmc: Avoids DoS if guest device is not able to get data faster enough
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file from the client
using WebDAV.
Previously the queue to the device was unbound. If device was not
getting data fast enough the server started queuing data.
To easily test this you can suspend the WebDAV daemon while transferring
a big file from the client.
To simplify the code and reduce the changes server buffers are
used. This as client token implementation is written with agent
in mind. While when we exhaust server tokens RedCharDevice doesn't
close the client when we exhaust client tokens RedCharDevice closes
the client which in this case it's not wanted.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:38:01 +01:00
Frediano Ziglio
726b1824e3 red-channel-client: Allows to block receiving data
If the client is keeping sending data while we can't handle them
(for instance because we need to forward to a device but the
device is not fast enough to receive that amount of data) allows
to stop RedChannelClient to read data.
This after a bit will stop the client sending data as its output
buffer will become full.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:37:55 +01:00
Frediano Ziglio
1157588cc1 spicevmc: Avoids DoS if client is not able to get data faster enough
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file to the client
using WebDAV.
Previously the queue to the client was unbound. If client was not
getting data fast enough the server started queuing data.
To easily test this you can use a tool to limit bandwidth and
transfer a big file (I used a "dd if=/dev/zero bs=1M of=test") from
the guest.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:12:58 +01:00
Frediano Ziglio
540f70351d spicevmc: Do not use RedCharDevice pipe items handling
As we don't use any token there's no reason to not queue directly instead
of passing through RedCharDevice.
This will make easier to limit the queue which currently is unlimited.

RedCharDevice flow control has some problems:
- it's really designed with VDI in mind. This for SpiceVMC would cause
  code implementation to be confusing having to satisfy the agent token
  handling;
- it's using items as unit not allowing counting bytes;
- it duplicates some queue management between RedChannelClient;
- it's broken (see comments in char-device.h);
- it forces "clients" to behave in some way not altering for instance the
  queued items (which is very useful if items can be collapsed together);
- it replicates the token handling on the device queue too. This could
  seems right but is not that if you have a network card going @ 1 GBit/s
  you are able to upload more than 1 Gbit/s just using multiple
  connections. The kernel will use a single queue for the network interface
  limiting and sharing de facto the various buffers between the multiple
  connections.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:12:52 +01:00
Victor Toso
c83420feae main-channel-client: style fixup, indentation of return
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-24 09:46:13 +02:00
Frediano Ziglio
4f2d90a784 red-qxl: Make sure we have at least one monitor
It does not make sense to have a graphic card without a monitor.
In spice_qxl_set_max_monitors we prevent to set 0 monitors, do
the same in spice_qxl_set_device_info.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1691721.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-19 14:54:53 +01:00
Frediano Ziglio
b862bee93f smartcard: Use RedChannelClient as the type for RedCharDevice client
As now is an opaque type for RedCharDevice use the type that
better suits us.
This avoid useless conversions or look ups.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 12:17:31 +01:00
Frediano Ziglio
14fe2c3766 char-device: Don't use RedClient API
RedClient was an opaque structure for RedCharDevice.
It started to be used when RedsState started to contain all
the global state.
Make it opaque again using a new RedCharDeviceClientOpaque.
The RedCharDeviceClientOpaque define in the header allows users
of the class to override the type to get a more safe type
than RedClient.
The define at the beginning of C file is to make sure we don't
use the opaque type as a specific one.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 12:17:26 +01:00
Frediano Ziglio
6ccf0e4800 red-channel-client: Inline red_channel_client_get_channel in macro
Inline red_channel_client_get_channel in spice_channel_client_error
macro.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 09:40:29 +01:00
Frediano Ziglio
609398fb77 reds: Inline reds_mig_switch function
No much reason for not inlining it, it's quite small and do
not reduce readability.
Note that spice_server_migrate_switch is deprecated and not
used by Qemu since commit 67be6726b6459472103ee87ceaf2e8e97c154965
(cfr "spice: raise requirement to 0.12" September 2012).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 09:40:24 +01:00
Frediano Ziglio
2d425ee6ae reds: Fix indentation of spice_server_char_device_add_interface
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-02 10:52:09 +01:00
Kevin Pouget
9ccf7bee7a stream-channel: Add preferred video codec capability
This patch enables the SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE
capability for the stream-channel.

video_stream_parse_preferred_codecs: new function for parsing the
SPICE protocol message. This code used to in inside
dcc_handle_preferred_video_codec_type.

struct StreamChannelClient::client_preferred_video_codecs: new field.

Signed-off-by: Kevin Pouget <kpouget@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-02 09:53:27 +01:00
Frediano Ziglio
84358a175e meson: Fix compatibility with Meson 0.48
The "install" argument for configure_file is available since 0.50.
However this is already "false" if "install_dir" is not provided.

This will drop the following warning:
"Project targetting '>= 0.48' but tried to use feature introduced
in '0.50.0': install arg in configure_file"

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2019-08-30 14:16:43 +01:00
Frediano Ziglio
41387cf1e1 meson: Remove a warning in spice-common
This will drop the following warning:
"Project targetting '>= 0.48.0' but tried to use feature introduced
in '0.50.0': install arg in configure_file"

This brings in the following changes:

Frediano Ziglio (5):
      codegen: Use has_end_attr instead of has_attr("end")
      codegen: Exit with error on error generating C structures
      protocol: Removed unneeded type specifications
      codegen: Add a check for C structure fields
      meson: Remove "install" argument from configure_file

Uri Lublin (2):
      ptypes.py: remove useless condition member != None
      test-marshallers.proto: ArrayMessage: make space for name

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2019-08-29 17:04:59 +01:00
Uri Lublin
a9d53f3cd1 test-loop: increment a variable outside of spice_assert
spice_assert is a macro and covscan reports that:
  Argument "++twice_remove_called" of spice_assert() has a side effect.

Doesn't matter if there is a side effects or not,
it's a good practice and it makes covscan happy, so
increment the variable one line above.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-08-22 21:27:35 +01:00
Uri Lublin
e9c8b25904 valgrind/spice.supp: add missing ...
There may to be another function (cache_alt_names) between
  gnutls_x509_ext_import_subject_alt_names and
  gnutls_x509_crt_import

cache_alt_names is a static function in gnutls/lib/x509/x509.c
used only in gnutls_x509_crt_import and may be inlined by
the compiler.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-08-22 08:54:00 +01:00
Frediano Ziglio
80bb7eb774 stat-file: Use proper macro for container computation
This is currently more style patch as the "value" field is the
first field of SpiceStatNode structure, so has offset 0. However
to compute the containing structure it better to use the proper
macro to avoid confusion.
If the offset won't be 0 the subtraction would compute the
wrong pointer as the offset is expressed in bytes but the
element size are uint64_t.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-08-22 08:53:23 +01:00