From: Stefano Garzarella <sgarzare@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, mst@redhat.com
Subject: Re: [PATCH 5/7] vhost: Distinguish errors in vhost_dev_get_config()
Date: Thu, 10 Jun 2021 11:16:20 +0200 [thread overview]
Message-ID: <20210610091620.t5z44zo6nu547grq@steredhat> (raw)
In-Reply-To: <20210609154658.350308-6-kwolf@redhat.com>
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 <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 71897c1a01..6b6f974a83 100644
>--- a/hw/virtio/vhost-vdpa.c
>+++ b/hw/virtio/vhost-vdpa.c
>@@ -451,7 +451,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..1b66f1de70 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_dev_get_config not implemented");
^
Maybe I'd replace s/vhost_dev_get_config/vhost_get_config
But it's not that important:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
next prev parent reply other threads:[~2021-06-10 9:17 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-09 15:46 [PATCH 0/7] vhost-user-blk: Implement reconnection during realize Kevin Wolf
2021-06-09 15:46 ` [PATCH 1/7] vhost: Add Error parameter to vhost_dev_init() Kevin Wolf
2021-06-10 9:05 ` Stefano Garzarella
2021-06-11 19:14 ` Raphael Norwitz
2021-06-09 15:46 ` [PATCH 2/7] vhost: Distinguish errors in vhost_backend_init() Kevin Wolf
2021-06-10 9:07 ` Stefano Garzarella
2021-06-11 19:43 ` Raphael Norwitz
2021-06-09 15:46 ` [PATCH 3/7] vhost: Return 0/-errno in vhost_dev_init() Kevin Wolf
2021-06-10 9:09 ` Stefano Garzarella
2021-06-11 19:49 ` Raphael Norwitz
2021-06-09 15:46 ` [PATCH 4/7] vhost-user-blk: Add Error parameter to vhost_user_blk_start() Kevin Wolf
2021-06-10 9:11 ` Stefano Garzarella
2021-06-11 19:55 ` Raphael Norwitz
2021-06-09 15:46 ` [PATCH 5/7] vhost: Distinguish errors in vhost_dev_get_config() Kevin Wolf
2021-06-10 9:16 ` Stefano Garzarella [this message]
2021-06-11 20:08 ` Raphael Norwitz
2021-06-09 15:46 ` [PATCH 6/7] vhost-user-blk: Factor out vhost_user_blk_realize_connect() Kevin Wolf
2021-06-10 9:18 ` Stefano Garzarella
2021-06-11 20:11 ` Raphael Norwitz
2021-06-09 15:46 ` [PATCH 7/7] vhost-user-blk: Implement reconnection during realize Kevin Wolf
2021-06-10 9:23 ` Stefano Garzarella
2021-06-11 20:15 ` Raphael Norwitz
2021-06-30 12:39 ` [PATCH 0/7] " Kevin Wolf
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=20210610091620.t5z44zo6nu547grq@steredhat \
--to=sgarzare@redhat.com \
--cc=kwolf@redhat.com \
--cc=mst@redhat.com \
--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.