| Message ID | 20221012111935.117480-1-f.ebner@proxmox.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/channel-block: fix return value for qio_channel_block_{readv, writev} | expand |
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!
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
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;
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(-)