Re: [PATCH 5/7] vhost: Distinguish errors in vhost_dev_get_config()

2021-06-11 Thread Raphael Norwitz
On Wed, Jun 09, 2021 at 05:46:56PM +0200, Kevin Wolf wrote:
> Instead of just returning 0/-1 and letting the caller make up a
> meaningless error message, add an Error parameter to allow reporting the
> real error and switch to 0/-errno so that different kind of errors can
> be distinguished in the caller.
> 
> Signed-off-by: Kevin Wolf 

Just one commmit message suggestion.

Reviewed-by: Raphael Norwitz 

> ---
>  include/hw/virtio/vhost-backend.h |  2 +-
>  include/hw/virtio/vhost.h |  4 ++--
>  hw/block/vhost-user-blk.c |  9 +
>  hw/display/vhost-user-gpu.c   |  6 --
>  hw/input/vhost-user-input.c   |  6 --
>  hw/net/vhost_net.c|  2 +-
>  hw/virtio/vhost-user-vsock.c  |  9 +
>  hw/virtio/vhost-user.c| 24 
>  hw/virtio/vhost-vdpa.c|  2 +-
>  hw/virtio/vhost.c | 14 +++---
>  10 files changed, 46 insertions(+), 32 deletions(-)
> 
> diff --git a/include/hw/virtio/vhost-backend.h 
> b/include/hw/virtio/vhost-backend.h
> index 728ebb0ed9..8475c5a29d 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -98,7 +98,7 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, 
> const uint8_t *data,
> uint32_t offset, uint32_t size,
> uint32_t flags);
>  typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
> -   uint32_t config_len);
> +   uint32_t config_len, Error **errp);
>  
>  typedef int (*vhost_crypto_create_session_op)(struct vhost_dev *dev,
>void *session_info,
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 2d7aaad67b..045d0fd9f2 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -130,8 +130,8 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
>struct vhost_vring_file *file);
>  
>  int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
> -int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
> - uint32_t config_len);
> +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
> + uint32_t config_len, Error **errp);
>  int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
>   uint32_t offset, uint32_t size, uint32_t flags);
>  /* notifier callback in case vhost device config space changed
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index e9382e152a..3770f715da 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -91,11 +91,13 @@ static int vhost_user_blk_handle_config_change(struct 
> vhost_dev *dev)
>  int ret;
>  struct virtio_blk_config blkcfg;
>  VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
> +Error *local_err = NULL;
>  
>  ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
> -   sizeof(struct virtio_blk_config));
> +   sizeof(struct virtio_blk_config),
> +   &local_err);
>  if (ret < 0) {
> -error_report("get config space failed");
> +error_report_err(local_err);
>  return -1;
>  }
>  
> @@ -478,9 +480,8 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  assert(s->connected);
>  
>  ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
> -   sizeof(struct virtio_blk_config));
> +   sizeof(struct virtio_blk_config), errp);
>  if (ret < 0) {
> -error_setg(errp, "vhost-user-blk: get block config failed");
>  goto vhost_err;
>  }
>  
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 6cdaa1c73b..389199e6ca 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -415,14 +415,16 @@ vhost_user_gpu_get_config(VirtIODevice *vdev, uint8_t 
> *config_data)
>  VirtIOGPUBase *b = VIRTIO_GPU_BASE(vdev);
>  struct virtio_gpu_config *vgconfig =
>  (struct virtio_gpu_config *)config_data;
> +Error *local_err = NULL;
>  int ret;
>  
>  memset(config_data, 0, sizeof(struct virtio_gpu_config));
>  
>  ret = vhost_dev_get_config(&g->vhost->dev,
> -   config_data, sizeof(struct 
> virtio_gpu_config));
> +   config_data, sizeof(struct virtio_gpu_config),
> +   &local_err);
>  if (ret) {
> -error_report("vhost-user-gpu: get device config space failed");
> +error_report_err(local_err);
>  return;
>  }
>  
> diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c
> index 63984a8ba7..273e96a7b1 100644

Re: [PATCH 5/7] vhost: Distinguish errors in vhost_dev_get_config()

2021-06-10 Thread Stefano Garzarella

On Wed, Jun 09, 2021 at 05:46:56PM +0200, Kevin Wolf wrote:

Instead of just returning 0/-1 and letting the caller make up a
meaningless error message, add an Error parameter to allow reporting the
real error and switch to 0/-errno so that different kind of errors can
be distinguished in the caller.

Signed-off-by: Kevin Wolf 
---
include/hw/virtio/vhost-backend.h |  2 +-
include/hw/virtio/vhost.h |  4 ++--
hw/block/vhost-user-blk.c |  9 +
hw/display/vhost-user-gpu.c   |  6 --
hw/input/vhost-user-input.c   |  6 --
hw/net/vhost_net.c|  2 +-
hw/virtio/vhost-user-vsock.c  |  9 +
hw/virtio/vhost-user.c| 24 
hw/virtio/vhost-vdpa.c|  2 +-
hw/virtio/vhost.c | 14 +++---
10 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index 728ebb0ed9..8475c5a29d 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -98,7 +98,7 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, 
const uint8_t *data,
   uint32_t offset, uint32_t size,
   uint32_t flags);
typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
-   uint32_t config_len);
+   uint32_t config_len, Error **errp);

typedef int (*vhost_crypto_create_session_op)(struct vhost_dev *dev,
  void *session_info,
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 2d7aaad67b..045d0fd9f2 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -130,8 +130,8 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
  struct vhost_vring_file *file);

int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
-int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
- uint32_t config_len);
+int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
+ uint32_t config_len, Error **errp);
int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
 uint32_t offset, uint32_t size, uint32_t flags);
/* notifier callback in case vhost device config space changed
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index e9382e152a..3770f715da 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -91,11 +91,13 @@ static int vhost_user_blk_handle_config_change(struct 
vhost_dev *dev)
int ret;
struct virtio_blk_config blkcfg;
VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
+Error *local_err = NULL;

ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
-   sizeof(struct virtio_blk_config));
+   sizeof(struct virtio_blk_config),
+   &local_err);
if (ret < 0) {
-error_report("get config space failed");
+error_report_err(local_err);
return -1;
}

@@ -478,9 +480,8 @@ static void vhost_user_blk_device_realize(DeviceState *dev, 
Error **errp)
assert(s->connected);

ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
-   sizeof(struct virtio_blk_config));
+   sizeof(struct virtio_blk_config), errp);
if (ret < 0) {
-error_setg(errp, "vhost-user-blk: get block config failed");
goto vhost_err;
}

diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 6cdaa1c73b..389199e6ca 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -415,14 +415,16 @@ vhost_user_gpu_get_config(VirtIODevice *vdev, uint8_t 
*config_data)
VirtIOGPUBase *b = VIRTIO_GPU_BASE(vdev);
struct virtio_gpu_config *vgconfig =
(struct virtio_gpu_config *)config_data;
+Error *local_err = NULL;
int ret;

memset(config_data, 0, sizeof(struct virtio_gpu_config));

ret = vhost_dev_get_config(&g->vhost->dev,
-   config_data, sizeof(struct virtio_gpu_config));
+   config_data, sizeof(struct virtio_gpu_config),
+   &local_err);
if (ret) {
-error_report("vhost-user-gpu: get device config space failed");
+error_report_err(local_err);
return;
}

diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c
index 63984a8ba7..273e96a7b1 100644
--- a/hw/input/vhost-user-input.c
+++ b/hw/input/vhost-user-input.c
@@ -49,13 +49,15 @@ static void vhost_input_get_config(VirtIODevice *vdev, 
uint8_t *config_data)
{
VirtIOInput *vinput = VIRTIO_INPUT(vdev);
VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
+Error *local_err = NULL;
int ret;

memset(config_data, 0, vi

[PATCH 5/7] vhost: Distinguish errors in vhost_dev_get_config()

2021-06-09 Thread Kevin Wolf
Instead of just returning 0/-1 and letting the caller make up a
meaningless error message, add an Error parameter to allow reporting the
real error and switch to 0/-errno so that different kind of errors can
be distinguished in the caller.

Signed-off-by: Kevin Wolf 
---
 include/hw/virtio/vhost-backend.h |  2 +-
 include/hw/virtio/vhost.h |  4 ++--
 hw/block/vhost-user-blk.c |  9 +
 hw/display/vhost-user-gpu.c   |  6 --
 hw/input/vhost-user-input.c   |  6 --
 hw/net/vhost_net.c|  2 +-
 hw/virtio/vhost-user-vsock.c  |  9 +
 hw/virtio/vhost-user.c| 24 
 hw/virtio/vhost-vdpa.c|  2 +-
 hw/virtio/vhost.c | 14 +++---
 10 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index 728ebb0ed9..8475c5a29d 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -98,7 +98,7 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, 
const uint8_t *data,
uint32_t offset, uint32_t size,
uint32_t flags);
 typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
-   uint32_t config_len);
+   uint32_t config_len, Error **errp);
 
 typedef int (*vhost_crypto_create_session_op)(struct vhost_dev *dev,
   void *session_info,
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 2d7aaad67b..045d0fd9f2 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -130,8 +130,8 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
   struct vhost_vring_file *file);
 
 int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
-int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
- uint32_t config_len);
+int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
+ uint32_t config_len, Error **errp);
 int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
  uint32_t offset, uint32_t size, uint32_t flags);
 /* notifier callback in case vhost device config space changed
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index e9382e152a..3770f715da 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -91,11 +91,13 @@ static int vhost_user_blk_handle_config_change(struct 
vhost_dev *dev)
 int ret;
 struct virtio_blk_config blkcfg;
 VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
+Error *local_err = NULL;
 
 ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
-   sizeof(struct virtio_blk_config));
+   sizeof(struct virtio_blk_config),
+   &local_err);
 if (ret < 0) {
-error_report("get config space failed");
+error_report_err(local_err);
 return -1;
 }
 
@@ -478,9 +480,8 @@ static void vhost_user_blk_device_realize(DeviceState *dev, 
Error **errp)
 assert(s->connected);
 
 ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
-   sizeof(struct virtio_blk_config));
+   sizeof(struct virtio_blk_config), errp);
 if (ret < 0) {
-error_setg(errp, "vhost-user-blk: get block config failed");
 goto vhost_err;
 }
 
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 6cdaa1c73b..389199e6ca 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -415,14 +415,16 @@ vhost_user_gpu_get_config(VirtIODevice *vdev, uint8_t 
*config_data)
 VirtIOGPUBase *b = VIRTIO_GPU_BASE(vdev);
 struct virtio_gpu_config *vgconfig =
 (struct virtio_gpu_config *)config_data;
+Error *local_err = NULL;
 int ret;
 
 memset(config_data, 0, sizeof(struct virtio_gpu_config));
 
 ret = vhost_dev_get_config(&g->vhost->dev,
-   config_data, sizeof(struct virtio_gpu_config));
+   config_data, sizeof(struct virtio_gpu_config),
+   &local_err);
 if (ret) {
-error_report("vhost-user-gpu: get device config space failed");
+error_report_err(local_err);
 return;
 }
 
diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c
index 63984a8ba7..273e96a7b1 100644
--- a/hw/input/vhost-user-input.c
+++ b/hw/input/vhost-user-input.c
@@ -49,13 +49,15 @@ static void vhost_input_get_config(VirtIODevice *vdev, 
uint8_t *config_data)
 {
 VirtIOInput *vinput = VIRTIO_INPUT(vdev);
 VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
+Error *local_err = NULL;
 int ret;
 
 memset(config_data, 0, vinput-