1. Patchew
  2. QEMU
  3. [PATCH 0/7] vhost-user-blk: Implement reconnection during realize
  4. View patch

[PATCH 3/7] vhost: Return 0/-errno in vhost_dev_init()

Kevin Wolf posted 7 patches 4 years, 8 months ago
Maintainers: Fam Zheng <fam@euphon.net>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Max Reitz <mreitz@redhat.com>, Jason Wang <jasowang@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>
[PATCH 3/7] vhost: Return 0/-errno in vhost_dev_init()
Posted by Kevin Wolf 4 years, 8 months ago 
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
Re: [PATCH 3/7] vhost: Return 0/-errno in vhost_dev_init()
Posted by Raphael Norwitz 4 years, 8 months ago 
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
>
Re: [PATCH 3/7] vhost: Return 0/-errno in vhost_dev_init()
Posted by Stefano Garzarella 4 years, 8 months ago 
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>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.