[PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[Fiona Ebner]

in the error case. The documentation in include/io/channel.h states
that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply
passing along the return value from the bdrv-functions has the
potential to confuse the call sides. Non-blocking mode is not
implemented currently, so -1 it is.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 migration/channel-block.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/channel-block.c b/migration/channel-block.c
index c55c8c93ce..aabc4634a4 100644
--- a/migration/channel-block.c
+++ b/migration/channel-block.c
@@ -62,7 +62,8 @@ qio_channel_block_readv(QIOChannel *ioc,
     qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
     ret = bdrv_readv_vmstate(bioc->bs, &qiov, bioc->offset);
     if (ret < 0) {
-        return ret;
+        error_setg(errp, "bdrv_readv_vmstate returned error %d", ret);
+        return -1;
     }
 
     bioc->offset += qiov.size;
@@ -86,7 +87,8 @@ qio_channel_block_writev(QIOChannel *ioc,
     qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
     ret = bdrv_writev_vmstate(bioc->bs, &qiov, bioc->offset);
     if (ret < 0) {
-        return ret;
+        error_setg(errp, "bdrv_writev_vmstate returned error %d", ret);
+        return -1;
     }
 
     bioc->offset += qiov.size;
-- 
2.30.2

Re: [PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[Fiona Ebner]

Am 12.10.22 um 13:19 schrieb Fiona Ebner:
> in the error case. The documentation in include/io/channel.h states
> that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply
> passing along the return value from the bdrv-functions has the
> potential to confuse the call sides. Non-blocking mode is not
> implemented currently, so -1 it is.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
CC-ing dgilbert@redhat.com since I messed up when sending the mail.
Sorry about that!

Re: [PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[Daniel P. Berrangé]

On Wed, Oct 12, 2022 at 01:19:35PM +0200, Fiona Ebner wrote:
> in the error case. The documentation in include/io/channel.h states
> that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply
> passing along the return value from the bdrv-functions has the
> potential to confuse the call sides. Non-blocking mode is not
> implemented currently, so -1 it is.

Opps, yes, my bad in writing this code.

> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  migration/channel-block.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/channel-block.c b/migration/channel-block.c
> index c55c8c93ce..aabc4634a4 100644
> --- a/migration/channel-block.c
> +++ b/migration/channel-block.c
> @@ -62,7 +62,8 @@ qio_channel_block_readv(QIOChannel *ioc,
>      qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
>      ret = bdrv_readv_vmstate(bioc->bs, &qiov, bioc->offset);
>      if (ret < 0) {
> -        return ret;
> +        error_setg(errp, "bdrv_readv_vmstate returned error %d", ret);

IIUC,  the bdrv functions return errno, so should use

    error_setg_errno(errp, -ret, "bdrv_readv_vmstate returned error");


>      bioc->offset += qiov.size;
> @@ -86,7 +87,8 @@ qio_channel_block_writev(QIOChannel *ioc,
>      qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
>      ret = bdrv_writev_vmstate(bioc->bs, &qiov, bioc->offset);
>      if (ret < 0) {
> -        return ret;
> +        error_setg(errp, "bdrv_writev_vmstate returned error %d", ret);
> +        return -1;
>      }
>  
>      bioc->offset += qiov.size;
> -- 
> 2.30.2
> 
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|