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(-)
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
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
>
© 2016 - 2025 Red Hat, Inc.