Message ID | 20210609154658.350308-4-kwolf@redhat.com |
---|---|
State | New |
Headers | show |
Series | vhost-user-blk: Implement reconnection during realize | expand |
On Wed, Jun 09, 2021 at 05:46:54PM +0200, Kevin Wolf wrote: >Instead of just returning 0/-1 and letting the caller make up a >meaningless error message, switch to 0/-errno so that different kinds of >errors can be distinguished in the caller. > >This involves changing a few more callbacks in VhostOps to return >0/-errno: .vhost_set_owner(), .vhost_get_features() and >.vhost_virtqueue_set_busyloop_timeout(). The implementations of these >functions are trivial as they generally just send a message to the >backend. > >Signed-off-by: Kevin Wolf <kwolf@redhat.com> >--- > hw/virtio/vhost-backend.c | 4 +++- > hw/virtio/vhost-user.c | 10 +++++++--- > hw/virtio/vhost-vdpa.c | 4 +++- > hw/virtio/vhost.c | 8 ++++---- > 4 files changed, 17 insertions(+), 9 deletions(-) Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
On Wed, Jun 09, 2021 at 05:46:54PM +0200, Kevin Wolf wrote: > Instead of just returning 0/-1 and letting the caller make up a > meaningless error message, switch to 0/-errno so that different kinds of > errors can be distinguished in the caller. > > This involves changing a few more callbacks in VhostOps to return > 0/-errno: .vhost_set_owner(), .vhost_get_features() and > .vhost_virtqueue_set_busyloop_timeout(). The implementations of these > functions are trivial as they generally just send a message to the > backend. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > --- > hw/virtio/vhost-backend.c | 4 +++- > hw/virtio/vhost-user.c | 10 +++++++--- > hw/virtio/vhost-vdpa.c | 4 +++- > hw/virtio/vhost.c | 8 ++++---- > 4 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > index f4f71cf58a..594d770b75 100644 > --- a/hw/virtio/vhost-backend.c > +++ b/hw/virtio/vhost-backend.c > @@ -24,10 +24,12 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request, > void *arg) > { > int fd = (uintptr_t) dev->opaque; > + int ret; > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); > > - return ioctl(fd, request, arg); > + ret = ioctl(fd, request, arg); > + return ret < 0 ? -errno : ret; > } > > static int vhost_kernel_init(struct vhost_dev *dev, void *opaque, Error **errp) > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 024cb201bb..889559d86a 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1353,7 +1353,11 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) > > static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) > { > - return vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features); > + if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { > + return -EPROTO; > + } > + > + return 0; > } > > static int vhost_user_set_owner(struct vhost_dev *dev) > @@ -1364,7 +1368,7 @@ static int vhost_user_set_owner(struct vhost_dev *dev) > }; > > if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > - return -1; > + return -EPROTO; > } > > return 0; > @@ -1872,7 +1876,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque, > > err = vhost_user_get_features(dev, &features); > if (err < 0) { > - return -EPROTO; > + return err; > } > > if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) { > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index c2aadb57cb..71897c1a01 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -253,10 +253,12 @@ static int vhost_vdpa_call(struct vhost_dev *dev, unsigned long int request, > { > struct vhost_vdpa *v = dev->opaque; > int fd = v->device_fd; > + int ret; > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); > > - return ioctl(fd, request, arg); > + ret = ioctl(fd, request, arg); > + return ret < 0 ? -errno : ret; > } > > static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index fd13135706..c7f9d8bb06 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1309,13 +1309,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > > r = hdev->vhost_ops->vhost_set_owner(hdev); > if (r < 0) { > - error_setg(errp, "vhost_set_owner failed"); > + error_setg_errno(errp, -r, "vhost_set_owner failed"); > goto fail; > } > > r = hdev->vhost_ops->vhost_get_features(hdev, &features); > if (r < 0) { > - error_setg(errp, "vhost_get_features failed"); > + error_setg_errno(errp, -r, "vhost_get_features failed"); > goto fail; > } > > @@ -1332,7 +1332,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > r = vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, > busyloop_timeout); > if (r < 0) { > - error_setg(errp, "Failed to set busyloop timeout"); > + error_setg_errno(errp, -r, "Failed to set busyloop timeout"); > goto fail_busyloop; > } > } > @@ -1391,7 +1391,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > if (used_memslots > hdev->vhost_ops->vhost_backend_memslots_limit(hdev)) { > error_setg(errp, "vhost backend memory slots limit is less" > " than current number of present memory slots"); > - r = -1; > + r = -EINVAL; > goto fail_busyloop; > } > > -- > 2.30.2 >
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c index f4f71cf58a..594d770b75 100644 --- a/hw/virtio/vhost-backend.c +++ b/hw/virtio/vhost-backend.c @@ -24,10 +24,12 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request, void *arg) { int fd = (uintptr_t) dev->opaque; + int ret; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); - return ioctl(fd, request, arg); + ret = ioctl(fd, request, arg); + return ret < 0 ? -errno : ret; } static int vhost_kernel_init(struct vhost_dev *dev, void *opaque, Error **errp) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 024cb201bb..889559d86a 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1353,7 +1353,11 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) { - return vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features); + if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { + return -EPROTO; + } + + return 0; } static int vhost_user_set_owner(struct vhost_dev *dev) @@ -1364,7 +1368,7 @@ static int vhost_user_set_owner(struct vhost_dev *dev) }; if (vhost_user_write(dev, &msg, NULL, 0) < 0) { - return -1; + return -EPROTO; } return 0; @@ -1872,7 +1876,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque, err = vhost_user_get_features(dev, &features); if (err < 0) { - return -EPROTO; + return err; } if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) { diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index c2aadb57cb..71897c1a01 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -253,10 +253,12 @@ static int vhost_vdpa_call(struct vhost_dev *dev, unsigned long int request, { struct vhost_vdpa *v = dev->opaque; int fd = v->device_fd; + int ret; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); - return ioctl(fd, request, arg); + ret = ioctl(fd, request, arg); + return ret < 0 ? -errno : ret; } static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index fd13135706..c7f9d8bb06 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1309,13 +1309,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, r = hdev->vhost_ops->vhost_set_owner(hdev); if (r < 0) { - error_setg(errp, "vhost_set_owner failed"); + error_setg_errno(errp, -r, "vhost_set_owner failed"); goto fail; } r = hdev->vhost_ops->vhost_get_features(hdev, &features); if (r < 0) { - error_setg(errp, "vhost_get_features failed"); + error_setg_errno(errp, -r, "vhost_get_features failed"); goto fail; } @@ -1332,7 +1332,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, r = vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, busyloop_timeout); if (r < 0) { - error_setg(errp, "Failed to set busyloop timeout"); + error_setg_errno(errp, -r, "Failed to set busyloop timeout"); goto fail_busyloop; } } @@ -1391,7 +1391,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, if (used_memslots > hdev->vhost_ops->vhost_backend_memslots_limit(hdev)) { error_setg(errp, "vhost backend memory slots limit is less" " than current number of present memory slots"); - r = -1; + r = -EINVAL; goto fail_busyloop; }
Instead of just returning 0/-1 and letting the caller make up a meaningless error message, switch to 0/-errno so that different kinds of errors can be distinguished in the caller. This involves changing a few more callbacks in VhostOps to return 0/-errno: .vhost_set_owner(), .vhost_get_features() and .vhost_virtqueue_set_busyloop_timeout(). The implementations of these functions are trivial as they generally just send a message to the backend. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- hw/virtio/vhost-backend.c | 4 +++- hw/virtio/vhost-user.c | 10 +++++++--- hw/virtio/vhost-vdpa.c | 4 +++- hw/virtio/vhost.c | 8 ++++---- 4 files changed, 17 insertions(+), 9 deletions(-)