[Qemu-devel] [PATCH v3] Fix input-linux reading from device

2017-03-27 Thread Javier Celaya 
The evdev devices in input-linux.c are read in blocks of one whole
event. If there are not enough bytes available, they are discarded,
instead of being kept for the next read operation. This results in
lost events, of even non-working devices.

This patch keeps track of the number of bytes to be read to fill up
a whole event, and then handle it.

Changes from v1 to v2:
- Fix: Calculate offset on each iteration

Changes from v2 to v3:
- Fix coding style
- Store offset instead of bytes to be read

Signed-off-by: Javier Celaya <jcel...@gmail.com>
---
 ui/input-linux.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/input-linux.c b/ui/input-linux.c
index ac31f47719..dc0613ca1f 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -169,6 +169,8 @@ struct InputLinux {
 boolhas_abs_x;
 int num_keys;
 int num_btns;
+struct input_event event;
+int read_offset;
 
 QTAILQ_ENTRY(InputLinux) next;
 };
@@ -327,25 +329,30 @@ static void input_linux_handle_mouse(InputLinux *il, 
struct input_event *event)
 static void input_linux_event(void *opaque)
 {
 InputLinux *il = opaque;
-struct input_event event;
 int rc;
+int read_size;
+uint8_t *p = (uint8_t *)>event;
 
 for (;;) {
-rc = read(il->fd, , sizeof(event));
-if (rc != sizeof(event)) {
+read_size = sizeof(il->event) - il->read_offset;
+rc = read(il->fd, [il->read_offset], read_size);
+if (rc != read_size) {
 if (rc < 0 && errno != EAGAIN) {
 fprintf(stderr, "%s: read: %s\n", __func__, strerror(errno));
 qemu_set_fd_handler(il->fd, NULL, NULL, NULL);
 close(il->fd);
+} else if (rc > 0) {
+il->read_offset += rc;
 }
 break;
 }
+il->read_offset = 0;
 
 if (il->num_keys) {
-input_linux_handle_keyboard(il, );
+input_linux_handle_keyboard(il, >event);
 }
 if (il->has_rel_x && il->num_btns) {
-input_linux_handle_mouse(il, );
+input_linux_handle_mouse(il, >event);
 }
 }
 }
-- 
2.11.0

Re: [Qemu-devel] [PATCH v2] Fix input-linux reading from device

2017-03-27 Thread Javier Celaya 
Hi

Javi

2017-03-27 12:11 GMT+02:00 Gerd Hoffmann <kra...@redhat.com>:

> On So, 2017-03-26 at 11:53 +0200, Javier Celaya wrote:
> > The evdev devices in input-linux.c are read in blocks of one whole
> > event. If there are not enough bytes available, they are discarded,
> > instead of being kept for the next read operation. This results in
> > lost events, of even non-working devices.
>
> Have you seen this happening in practice?
>

Yes, quite frequently, like once per hour. Totally destroys a good gaming
session :)
The curious thing is, the mouse stops working, but in the keyboard I see
some missing keyup events (the keys get stuck), but then it recovers.


>
> > +struct input_event event;
> > +int to_be_read;
>
> I'd suggest to store offset (i.e. bytes already read) instead, should
> make the whole logic a bit simpler and easier to read.
>

OK


>
> > +} else if (rc > 0){
>
> checkpatch.pl complains here:
> ERROR: space required before the open brace '{'
>

Oops, missed that


>
> cheers,
>   Gerd
>
>

[Qemu-devel] [PATCH v2] Fix input-linux reading from device

2017-03-26 Thread Javier Celaya 
The evdev devices in input-linux.c are read in blocks of one whole
event. If there are not enough bytes available, they are discarded,
instead of being kept for the next read operation. This results in
lost events, of even non-working devices.

This patch keeps track of the number of bytes to be read to fill up
a whole event, and then handle it.

Signed-off-by: Javier Celaya <jcel...@gmail.com>
---
 ui/input-linux.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ui/input-linux.c b/ui/input-linux.c
index ac31f47719..02b0d4b2fe 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -169,6 +169,8 @@ struct InputLinux {
 boolhas_abs_x;
 int num_keys;
 int num_btns;
+struct input_event event;
+int to_be_read;
 
 QTAILQ_ENTRY(InputLinux) next;
 };
@@ -327,25 +329,30 @@ static void input_linux_handle_mouse(InputLinux *il, 
struct input_event *event)
 static void input_linux_event(void *opaque)
 {
 InputLinux *il = opaque;
-struct input_event event;
 int rc;
+int offset;
+uint8_t *p = (uint8_t *)>event;
 
 for (;;) {
-rc = read(il->fd, , sizeof(event));
-if (rc != sizeof(event)) {
+offset = sizeof(il->event) - il->to_be_read;
+rc = read(il->fd, [offset], il->to_be_read);
+if (rc != il->to_be_read) {
 if (rc < 0 && errno != EAGAIN) {
 fprintf(stderr, "%s: read: %s\n", __func__, strerror(errno));
 qemu_set_fd_handler(il->fd, NULL, NULL, NULL);
 close(il->fd);
+} else if (rc > 0){
+il->to_be_read -= rc;
 }
 break;
 }
+il->to_be_read = sizeof(il->event);
 
 if (il->num_keys) {
-input_linux_handle_keyboard(il, );
+input_linux_handle_keyboard(il, >event);
 }
 if (il->has_rel_x && il->num_btns) {
-input_linux_handle_mouse(il, );
+input_linux_handle_mouse(il, >event);
 }
 }
 }
@@ -417,6 +424,7 @@ static void input_linux_complete(UserCreatable *uc, Error 
**errp)
 }
 }
 
+il->to_be_read = sizeof(il->event);
 qemu_set_fd_handler(il->fd, input_linux_event, NULL, il);
 if (il->keycount) {
 /* delay grab until all keys are released */
-- 
2.11.0

[Qemu-devel] [PATCH] Fix input-linux reading from device

2017-03-25 Thread Javier Celaya 
The evdev devices in input-linux.c are read in blocks of one whole
event. If there are not enough bytes available, they are discarded,
instead of being kept for the next read operation. This results in
lost events, of even non-working devices.

This patch keeps track of the number of bytes to be read to fill up
a whole event, and then handle it.

Signed-off-by: Javier Celaya <jcel...@gmail.com>
---
 ui/input-linux.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/input-linux.c b/ui/input-linux.c
index ac31f47719..33bcdb00c6 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -169,6 +169,8 @@ struct InputLinux {
 boolhas_abs_x;
 int num_keys;
 int num_btns;
+struct input_event event;
+int to_be_read;
 
 QTAILQ_ENTRY(InputLinux) next;
 };
@@ -327,25 +329,29 @@ static void input_linux_handle_mouse(InputLinux *il, 
struct input_event *event)
 static void input_linux_event(void *opaque)
 {
 InputLinux *il = opaque;
-struct input_event event;
 int rc;
+int offset = sizeof(il->event) - il->to_be_read;
+uint8_t *p = (uint8_t *)>event;
 
 for (;;) {
-rc = read(il->fd, , sizeof(event));
-if (rc != sizeof(event)) {
+rc = read(il->fd, [offset], il->to_be_read);
+if (rc != il->to_be_read) {
 if (rc < 0 && errno != EAGAIN) {
 fprintf(stderr, "%s: read: %s\n", __func__, strerror(errno));
 qemu_set_fd_handler(il->fd, NULL, NULL, NULL);
 close(il->fd);
+} else if (rc > 0){
+il->to_be_read -= rc;
 }
 break;
 }
+il->to_be_read = sizeof(il->event);
 
 if (il->num_keys) {
-input_linux_handle_keyboard(il, );
+input_linux_handle_keyboard(il, >event);
 }
 if (il->has_rel_x && il->num_btns) {
-input_linux_handle_mouse(il, );
+input_linux_handle_mouse(il, >event);
 }
 }
 }
@@ -417,6 +423,7 @@ static void input_linux_complete(UserCreatable *uc, Error 
**errp)
 }
 }
 
+il->to_be_read = sizeof(il->event);
 qemu_set_fd_handler(il->fd, input_linux_event, NULL, il);
 if (il->keycount) {
 /* delay grab until all keys are released */
-- 
2.11.0

Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE

2015-01-28 Thread Javier Celaya 
From what I've seen in QEMU and libvirt's code, I would say that discovering 
whether the spice server supports LZ4 is not a matter of adding a new QMP 
command. That would not scale very well with new command line options. I would 
suggest something more general. For instance, having the command query-
command-line-options return also the allowed values for string parameters. 
That would output lz4 for parameter image-compression in option spice, 
among the other compression methods. Another possibility would be a new QMP 
command that returns the capabilities of the spice server, as defined in the 
spice protocol. They include the SPICE_DISPLAY_CAP_LZ4_COMPRESSION capability, 
if the spice server supports LZ4. In any case, I think it is out of the scope 
of this patch.

El Martes, 27 de enero de 2015 09:26:11 Markus Armbruster escribió:
 Eric Blake ebl...@redhat.com writes:
  On 01/26/2015 01:48 AM, Javier Celaya wrote:
  Sorry, I forgot to patch the command-line help. Hope it helps.
  
  Recently, SPICE included the lz4 compression algorithm. This patch adds
  a command line option to select it.
  
  How is libvirt going to introspect whether the command line supports
  this option?  Is there some QMP command that lists the set of valid
  compression formats understood by a given qemu binary?
  
  No, patching the command line --help does NOT help libvirt.  It needs to
  be discoverable via QMP to be introspectible, as scraping --help output
  is not machine-friendly.  (That said, you DO want to expose it in --help
  output; I'm just complaining that --help output alone is not enough).
 
 We should really, really, really provide access to (the relevant subset
 of) the QAPI schema over QMP!  Until we get that, useful progress is
 delayed by problems like this one, and we keep growing special-purpose
 solutions to problems like this one.

Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE

2015-01-26 Thread Javier Celaya 
Sorry, I forgot to patch the command-line help. Hope it helps.

El Viernes, 23 de enero de 2015 09:25:38 Eric Blake escribió:
 On 01/23/2015 07:06 AM, Javier Celaya wrote:
  Hello, this is the patch with the version check. Christophe, can you check
  that the version is the correct one?
  
  
  
  Recently, SPICE included the lz4 compression algorithm. This patch adds
  a command line option to select it.
  ---
  
   ui/spice-core.c | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/ui/spice-core.c b/ui/spice-core.c
  index 6467fa4..dadcae9 100644
  --- a/ui/spice-core.c
  +++ b/ui/spice-core.c
  @@ -359,6 +359,9 @@ static const char *compression_names[] = {
  
   [ SPICE_IMAGE_COMPRESS_QUIC ] = quic,
   [ SPICE_IMAGE_COMPRESS_GLZ ]  = glz,
   [ SPICE_IMAGE_COMPRESS_LZ ]   = lz,
  
  +#if SPICE_SERVER_VERSION = 0x000c07
  +[ SPICE_IMAGE_COMPRESS_LZ4 ]  = lz4,
 
 How is libvirt going to introspect whether the command line supports
 this option?  Is there some QMP command that lists the set of valid
 compression formats understood by a given qemu binary?
From 26864da931603791c70c570b9b46f21657ebcaec Mon Sep 17 00:00:00 2001
From: Javier Celaya javier.cel...@flexvm.es
Date: Wed, 7 Jan 2015 11:53:24 +0100
Subject: [PATCH] Add lz4 compression option for SPICE.

Recently, SPICE included the lz4 compression algorithm. This patch adds
a command line option to select it.
---
 qemu-options.hx | 4 ++--
 ui/spice-core.c | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 22cf3b9..f5c54ba 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -953,7 +953,7 @@ DEF(spice, HAS_ARG, QEMU_OPTION_spice,
[,tls-channel=[main|display|cursor|inputs|record|playback]]\n
[,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n
[,sasl][,password=secret][,disable-ticketing]\n
-   [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n
+   [,image-compression=[auto_glz|auto_lz|quic|glz|lz|lz4|off]]\n
[,jpeg-wan-compression=[auto|never|always]]\n
[,zlib-glz-wan-compression=[auto|never|always]]\n
[,streaming-video=[off|all|filter]][,disable-copy-paste]\n
@@ -1028,7 +1028,7 @@ channels.  The special name default can be used to set the default
 mode.  For channels which are not explicitly forced into one mode the
 spice client is allowed to pick tls/plaintext as he pleases.
 
-@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
+@item image-compression=[auto_glz|auto_lz|quic|glz|lz|lz4|off]
 Configure image compression (lossless).
 Default is auto_glz.
 
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6467fa4..dadcae9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -359,6 +359,9 @@ static const char *compression_names[] = {
 [ SPICE_IMAGE_COMPRESS_QUIC ] = quic,
 [ SPICE_IMAGE_COMPRESS_GLZ ]  = glz,
 [ SPICE_IMAGE_COMPRESS_LZ ]   = lz,
+#if SPICE_SERVER_VERSION = 0x000c07
+[ SPICE_IMAGE_COMPRESS_LZ4 ]  = lz4,
+#endif
 };
 #define parse_compression(_name)\
 parse_name(_name, image compression,  \
-- 
1.9.3

Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE

2015-01-23 Thread Javier Celaya 
Hello, this is the patch with the version check. Christophe, can you check 
that the version is the correct one?

El Martes, 20 de enero de 2015 17:08:07 Christophe Fergeau escribió:
 Hey,
 
 Version check seems good, we probably can raise spice-server version in
 git preemptively so that you can have a working version check now.
 
 Given that lz4 support is optional, spice_server_set_image_compression
 should probably error out if we try to set lz4 but it's not supported.

Ack

 
 Christophe
From 896a3ea776ee95653a7cb8a0c90111a89b57c73f Mon Sep 17 00:00:00 2001
From: Javier Celaya javier.cel...@flexvm.es
Date: Wed, 7 Jan 2015 11:53:24 +0100
Subject: [PATCH] Add lz4 compression option for SPICE.

Recently, SPICE included the lz4 compression algorithm. This patch adds
a command line option to select it.
---
 ui/spice-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6467fa4..dadcae9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -359,6 +359,9 @@ static const char *compression_names[] = {
 [ SPICE_IMAGE_COMPRESS_QUIC ] = quic,
 [ SPICE_IMAGE_COMPRESS_GLZ ]  = glz,
 [ SPICE_IMAGE_COMPRESS_LZ ]   = lz,
+#if SPICE_SERVER_VERSION = 0x000c07
+[ SPICE_IMAGE_COMPRESS_LZ4 ]  = lz4,
+#endif
 };
 #define parse_compression(_name)\
 parse_name(_name, image compression,  \
-- 
1.9.3

Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE

2015-01-08 Thread Javier Celaya 
Sure, for the help text of the image-compression option, thanks.

El Jueves, 8 de enero de 2015 12:48:57 Fabio Fantoni escribió:
 Il 08/01/2015 11:50, Javier Celaya ha scritto:
  Hello
  
  Recently, SPICE included the lz4 compression algorithm. This patch adds
  a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did
  not exist before the commit that added this compression algorithm, so it
  should be guarded with conditional compilation. How do you think this
  should be done? Wait for the next stable version of spice-server and
  check for
  SPICE_SERVER_VERSION? Or add a specific flag?
  
  Thank you
  ---
  
   ui/spice-core.c | 1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/ui/spice-core.c b/ui/spice-core.c
  index 6467fa4..fb6534e 100644
  --- a/ui/spice-core.c
  +++ b/ui/spice-core.c
  @@ -359,6 +359,7 @@ static const char *compression_names[] = {
  
   [ SPICE_IMAGE_COMPRESS_QUIC ] = quic,
   [ SPICE_IMAGE_COMPRESS_GLZ ]  = glz,
   [ SPICE_IMAGE_COMPRESS_LZ ]   = lz,
  
  +[ SPICE_IMAGE_COMPRESS_LZ4 ]  = lz4,
  
   };
   #define parse_compression(_name)\
   
   parse_name(_name, image compression,  \
  
  --
  1.9.3
 
 I did a small search now and seems that also other small change to
 qemu-options.hx file should be done, search this line:
 @item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
 
  ___
  Spice-devel mailing list
  spice-de...@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE

2015-01-08 Thread Javier Celaya 
Hello

Recently, SPICE included the lz4 compression algorithm. This patch adds
a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did not 
exist before the commit that added this compression algorithm, so it should be 
guarded with conditional compilation. How do you think this should be done? 
Wait for the next stable version of spice-server and check for 
SPICE_SERVER_VERSION? Or add a specific flag?

Thank you
---
 ui/spice-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6467fa4..fb6534e 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -359,6 +359,7 @@ static const char *compression_names[] = {
 [ SPICE_IMAGE_COMPRESS_QUIC ] = quic,
 [ SPICE_IMAGE_COMPRESS_GLZ ]  = glz,
 [ SPICE_IMAGE_COMPRESS_LZ ]   = lz,
+[ SPICE_IMAGE_COMPRESS_LZ4 ]  = lz4,
 };
 #define parse_compression(_name)\
 parse_name(_name, image compression,  \
--
1.9.3