[Qemu-devel] [PATCH] char: report errors from qio_channel_{read, write}v_full

[Qemu-devel] [PATCH] char: report errors from qio_channel_{read, write}v_full

[Greg Edwards]

Two callers of qio_channel_{read,write}v_full were not passing in an
Error pointer, missing any error messages from the channel class
io_{read,write}v methods.

Signed-off-by: Greg Edwards <gedwards@ddn.com>
---
 chardev/char-io.c     | 7 ++++++-
 chardev/char-socket.c | 8 ++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/chardev/char-io.c b/chardev/char-io.c
index f81052481a55..7d8287cbfe31 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "chardev/char-io.h"
 
 typedef struct IOWatchPoll {
@@ -152,6 +153,7 @@ int io_channel_send_full(QIOChannel *ioc,
                          int *fds, size_t nfds)
 {
     size_t offset = 0;
+    Error *local_err = NULL;
 
     while (offset < len) {
         ssize_t ret = 0;
@@ -160,7 +162,10 @@ int io_channel_send_full(QIOChannel *ioc,
 
         ret = qio_channel_writev_full(
             ioc, &iov, 1,
-            fds, nfds, NULL);
+            fds, nfds, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+        }
         if (ret == QIO_CHANNEL_ERR_BLOCK) {
             if (offset) {
                 return offset;
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index e65148fe973c..4e27370440ff 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -272,15 +272,19 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
     size_t i;
     int *msgfds = NULL;
     size_t msgfds_num = 0;
+    Error *local_err = NULL;
 
     if (qio_channel_has_feature(s->ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
         ret = qio_channel_readv_full(s->ioc, &iov, 1,
                                      &msgfds, &msgfds_num,
-                                     NULL);
+                                     &local_err);
     } else {
         ret = qio_channel_readv_full(s->ioc, &iov, 1,
                                      NULL, NULL,
-                                     NULL);
+                                     &local_err);
+    }
+    if (local_err) {
+        error_report_err(local_err);
     }
 
     if (ret == QIO_CHANNEL_ERR_BLOCK) {
-- 
2.13.5

Re: [Qemu-devel] [PATCH] char: report errors from qio_channel_{read, write}v_full

[Paolo Bonzini]

On 28/09/2017 22:52, Greg Edwards wrote:
> Two callers of qio_channel_{read,write}v_full were not passing in an
> Error pointer, missing any error messages from the channel class
> io_{read,write}v methods.
> 
> Signed-off-by: Greg Edwards <gedwards@ddn.com>

This is on purpose in order to avoid "spamming" the logs.  In
particular, for sockets it can be a normal thing for the other side to
disconnect.

Paolo

> ---
>  chardev/char-io.c     | 7 ++++++-
>  chardev/char-socket.c | 8 ++++++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/chardev/char-io.c b/chardev/char-io.c
> index f81052481a55..7d8287cbfe31 100644
> --- a/chardev/char-io.c
> +++ b/chardev/char-io.c
> @@ -22,6 +22,7 @@
>   * THE SOFTWARE.
>   */
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "chardev/char-io.h"
>  
>  typedef struct IOWatchPoll {
> @@ -152,6 +153,7 @@ int io_channel_send_full(QIOChannel *ioc,
>                           int *fds, size_t nfds)
>  {
>      size_t offset = 0;
> +    Error *local_err = NULL;
>  
>      while (offset < len) {
>          ssize_t ret = 0;
> @@ -160,7 +162,10 @@ int io_channel_send_full(QIOChannel *ioc,
>  
>          ret = qio_channel_writev_full(
>              ioc, &iov, 1,
> -            fds, nfds, NULL);
> +            fds, nfds, &local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +        }
>          if (ret == QIO_CHANNEL_ERR_BLOCK) {
>              if (offset) {
>                  return offset;
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index e65148fe973c..4e27370440ff 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -272,15 +272,19 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>      size_t i;
>      int *msgfds = NULL;
>      size_t msgfds_num = 0;
> +    Error *local_err = NULL;
>  
>      if (qio_channel_has_feature(s->ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
>          ret = qio_channel_readv_full(s->ioc, &iov, 1,
>                                       &msgfds, &msgfds_num,
> -                                     NULL);
> +                                     &local_err);
>      } else {
>          ret = qio_channel_readv_full(s->ioc, &iov, 1,
>                                       NULL, NULL,
> -                                     NULL);
> +                                     &local_err);
> +    }
> +    if (local_err) {
> +        error_report_err(local_err);
>      }
>  
>      if (ret == QIO_CHANNEL_ERR_BLOCK) {
> 

Re: [Qemu-devel] [PATCH] char: report errors from qio_channel_{read, write}v_full

[Greg Edwards]

On Fri, Sep 29, 2017 at 11:12:41AM +0200, Paolo Bonzini wrote:
> On 28/09/2017 22:52, Greg Edwards wrote:
>> Two callers of qio_channel_{read,write}v_full were not passing in an
>> Error pointer, missing any error messages from the channel class
>> io_{read,write}v methods.
>
> This is on purpose in order to avoid "spamming" the logs.  In
> particular, for sockets it can be a normal thing for the other side to
> disconnect.

Thanks for the background, Paolo.

We had encountered a vhost-user-scsi initialization failure, and were
looking for the errno from the recvmsg failure from
VHOST_USER_GET_FEATURES:

qemu-system-x86_64: -device vhost-user-scsi-pci,chardev=vus0,bootindex=2: Failed to read msg header. Read -1 instead of 12. Original request 1.
qemu-system-x86_64: -device vhost-user-scsi-pci,chardev=vus0,bootindex=2: vhost-user-scsi: vhost initialization failed: Operation not permitted

In this case, the strerror in vhost_user_scsi_realize is just reporting
on the -1 returned from the vhost_user_read failure, not the errno of
original offender.  That's what started me down this path.

Greg