All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org
Subject: [PULL 22/24] vhost: Distinguish errors in vhost_dev_get_config()
Date: Wed, 30 Jun 2021 18:02:04 +0200	[thread overview]
Message-ID: <20210630160206.276439-23-kwolf@redhat.com> (raw)
In-Reply-To: <20210630160206.276439-1-kwolf@redhat.com>

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.

config_len in vhost_user_get_config() is defined by the device, so if
it's larger than VHOST_USER_MAX_CONFIG_SIZE, this is a programming
error. Turn the corresponding check into an assertion.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210609154658.350308-6-kwolf@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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->cfg_size);
 
-    ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, vinput->cfg_size);
+    ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, vinput->cfg_size,
+                               &local_err);
     if (ret) {
-        error_report("vhost-user-input: get device config space failed");
+        error_report_err(local_err);
         return;
     }
 }
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 447b119f85..10a7780a13 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -117,7 +117,7 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
 int vhost_net_get_config(struct vhost_net *net,  uint8_t *config,
                          uint32_t config_len)
 {
-    return vhost_dev_get_config(&net->dev, config, config_len);
+    return vhost_dev_get_config(&net->dev, config, config_len, NULL);
 }
 int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
                          uint32_t offset, uint32_t size, uint32_t flags)
diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
index b6a4a25ea1..6095ed7349 100644
--- a/hw/virtio/vhost-user-vsock.c
+++ b/hw/virtio/vhost-user-vsock.c
@@ -34,10 +34,12 @@ static void vuv_get_config(VirtIODevice *vdev, uint8_t *config)
 static int vuv_handle_config_change(struct vhost_dev *dev)
 {
     VHostUserVSock *vsock = VHOST_USER_VSOCK(dev->vdev);
+    Error *local_err = NULL;
     int ret = vhost_dev_get_config(dev, (uint8_t *)&vsock->vsockcfg,
-                                   sizeof(struct virtio_vsock_config));
+                                   sizeof(struct virtio_vsock_config),
+                                   &local_err);
     if (ret < 0) {
-        error_report("get config space failed");
+        error_report_err(local_err);
         return -1;
     }
 
@@ -114,9 +116,8 @@ static void vuv_device_realize(DeviceState *dev, Error **errp)
     }
 
     ret = vhost_dev_get_config(&vvc->vhost_dev, (uint8_t *)&vsock->vsockcfg,
-                               sizeof(struct virtio_vsock_config));
+                               sizeof(struct virtio_vsock_config), errp);
     if (ret < 0) {
-        error_setg_errno(errp, -ret, "get config space failed");
         goto err_vhost_dev;
     }
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 889559d86a..1ac4a2ebec 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2117,7 +2117,7 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
 }
 
 static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
-                                 uint32_t config_len)
+                                 uint32_t config_len, Error **errp)
 {
     VhostUserMsg msg = {
         .hdr.request = VHOST_USER_GET_CONFIG,
@@ -2127,32 +2127,32 @@ static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
 
     if (!virtio_has_feature(dev->protocol_features,
                 VHOST_USER_PROTOCOL_F_CONFIG)) {
-        return -1;
+        error_setg(errp, "VHOST_USER_PROTOCOL_F_CONFIG not supported");
+        return -EINVAL;
     }
 
-    if (config_len > VHOST_USER_MAX_CONFIG_SIZE) {
-        return -1;
-    }
+    assert(config_len <= VHOST_USER_MAX_CONFIG_SIZE);
 
     msg.payload.config.offset = 0;
     msg.payload.config.size = config_len;
     if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
-        return -1;
+        return -EPROTO;
     }
 
     if (vhost_user_read(dev, &msg) < 0) {
-        return -1;
+        return -EPROTO;
     }
 
     if (msg.hdr.request != VHOST_USER_GET_CONFIG) {
-        error_report("Received unexpected msg type. Expected %d received %d",
-                     VHOST_USER_GET_CONFIG, msg.hdr.request);
-        return -1;
+        error_setg(errp,
+                   "Received unexpected msg type. Expected %d received %d",
+                   VHOST_USER_GET_CONFIG, msg.hdr.request);
+        return -EINVAL;
     }
 
     if (msg.hdr.size != VHOST_USER_CONFIG_HDR_SIZE + config_len) {
-        error_report("Received bad msg size.");
-        return -1;
+        error_setg(errp, "Received bad msg size.");
+        return -EINVAL;
     }
 
     memcpy(config, msg.payload.config.region, config_len);
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 0f469f1823..4fa414feea 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -523,7 +523,7 @@ static int vhost_vdpa_set_config(struct vhost_dev *dev, const uint8_t *data,
 }
 
 static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
-                                   uint32_t config_len)
+                                   uint32_t config_len, Error **errp)
 {
     struct vhost_vdpa_config *v_config;
     unsigned long config_size = offsetof(struct vhost_vdpa_config, buf);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index c7f9d8bb06..e8f85a5d2d 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1562,15 +1562,23 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
 }
 
 int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
-                         uint32_t config_len)
+                         uint32_t config_len, Error **errp)
 {
+    ERRP_GUARD();
+    int ret;
+
     assert(hdev->vhost_ops);
 
     if (hdev->vhost_ops->vhost_get_config) {
-        return hdev->vhost_ops->vhost_get_config(hdev, config, config_len);
+        ret = hdev->vhost_ops->vhost_get_config(hdev, config, config_len, errp);
+        if (ret < 0 && !*errp) {
+            error_setg_errno(errp, -ret, "vhost_get_config failed");
+        }
+        return ret;
     }
 
-    return -1;
+    error_setg(errp, "vhost_get_config not implemented");
+    return -ENOTSUP;
 }
 
 int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *data,
-- 
2.31.1



  parent reply	other threads:[~2021-06-30 16:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30 16:01 [PULL 00/24] Block layer patches Kevin Wolf
2021-06-30 16:01 ` [PULL 01/24] Prevent compiler warning on block.c Kevin Wolf
2021-06-30 16:01 ` [PULL 02/24] block: Move read-only check during truncation earlier Kevin Wolf
2021-06-30 16:01 ` [PULL 03/24] block: BDRV_O_NO_IO for backing file on creation Kevin Wolf
2021-06-30 16:01 ` [PULL 04/24] block: rename bdrv_replace_child to bdrv_replace_child_tran Kevin Wolf
2021-06-30 16:01 ` [PULL 05/24] block: comment graph-modifying function not updating permissions Kevin Wolf
2021-06-30 16:01 ` [PULL 06/24] block: introduce bdrv_remove_file_or_backing_child() Kevin Wolf
2021-06-30 16:01 ` [PULL 07/24] block: introduce bdrv_set_file_or_backing_noperm() Kevin Wolf
2021-06-30 16:01 ` [PULL 08/24] block: bdrv_reopen_parse_backing(): don't check aio context Kevin Wolf
2021-06-30 16:01 ` [PULL 09/24] block: bdrv_reopen_parse_backing(): don't check frozen child Kevin Wolf
2021-06-30 16:01 ` [PULL 10/24] block: bdrv_reopen_parse_backing(): simplify handling implicit filters Kevin Wolf
2021-06-30 16:01 ` [PULL 11/24] block: move supports_backing check to bdrv_set_file_or_backing_noperm() Kevin Wolf
2021-06-30 16:01 ` [PULL 12/24] block: BDRVReopenState: drop replace_backing_bs field Kevin Wolf
2021-06-30 16:01 ` [PULL 13/24] block: Allow changing bs->file on reopen Kevin Wolf
2021-06-30 16:01 ` [PULL 14/24] iotests: Test replacing files with x-blockdev-reopen Kevin Wolf
2021-06-30 16:01 ` [PULL 15/24] introduce QEMU_AUTO_VFREE Kevin Wolf
2021-06-30 16:01 ` [PULL 16/24] block/commit: use QEMU_AUTO_VFREE Kevin Wolf
2021-06-30 16:01 ` [PULL 17/24] block/ssh: add support for sha256 host key fingerprints Kevin Wolf
2021-06-30 16:02 ` [PULL 18/24] vhost: Add Error parameter to vhost_dev_init() Kevin Wolf
2021-06-30 16:02 ` [PULL 19/24] vhost: Distinguish errors in vhost_backend_init() Kevin Wolf
2021-06-30 16:02 ` [PULL 20/24] vhost: Return 0/-errno in vhost_dev_init() Kevin Wolf
2021-06-30 16:02 ` [PULL 21/24] vhost-user-blk: Add Error parameter to vhost_user_blk_start() Kevin Wolf
2021-06-30 16:02 ` Kevin Wolf [this message]
2021-06-30 16:02 ` [PULL 23/24] vhost-user-blk: Factor out vhost_user_blk_realize_connect() Kevin Wolf
2021-06-30 16:02 ` [PULL 24/24] vhost-user-blk: Implement reconnection during realize Kevin Wolf
2021-07-02 13:52 ` [PULL 00/24] Block layer patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210630160206.276439-23-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.