[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/5] vhost-user-blk: Use Error more consistently
From: |
Raphael Norwitz |
Subject: |
Re: [PATCH 2/5] vhost-user-blk: Use Error more consistently |
Date: |
Wed, 28 Apr 2021 18:08:48 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Code looks ok - question about the commit message.
Acked-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
On Thu, Apr 22, 2021 at 07:02:18PM +0200, Kevin Wolf wrote:
> Now that vhost_user_blk_connect() is not called from an event handler
> any more, but directly from vhost_user_blk_device_realize(), we don't
> have to resort to error_report() any more, but can use Error again.
vhost_user_blk_connect() is still called by vhost_user_blk_event() which
is registered as an event handler. I don't understand your point around
us having had to use error_report() before, but not anymore. Can you
clarify?
>
> With Error, the callers are responsible for adding context if necessary
> (such as the "-device" option the error refers to). Additional prefixes
> are redundant and better omitted.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> hw/block/vhost-user-blk.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index e824b0a759..8422a29470 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -311,7 +311,7 @@ static void vhost_user_blk_reset(VirtIODevice *vdev)
> vhost_dev_free_inflight(s->inflight);
> }
>
> -static int vhost_user_blk_connect(DeviceState *dev)
> +static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> @@ -331,8 +331,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
>
> ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER,
> 0);
> if (ret < 0) {
> - error_report("vhost-user-blk: vhost initialization failed: %s",
> - strerror(-ret));
> + error_setg_errno(errp, -ret, "vhost initialization failed");
> return ret;
> }
>
> @@ -340,8 +339,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
> if (virtio_device_started(vdev, vdev->status)) {
> ret = vhost_user_blk_start(vdev);
> if (ret < 0) {
> - error_report("vhost-user-blk: vhost start failed: %s",
> - strerror(-ret));
> + error_setg_errno(errp, -ret, "vhost start failed");
> return ret;
> }
> }
> @@ -380,10 +378,12 @@ static void vhost_user_blk_event(void *opaque,
> QEMUChrEvent event)
> DeviceState *dev = opaque;
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> + Error *local_err = NULL;
>
> switch (event) {
> case CHR_EVENT_OPENED:
> - if (vhost_user_blk_connect(dev) < 0) {
> + if (vhost_user_blk_connect(dev, &local_err) < 0) {
> + error_report_err(local_err);
> qemu_chr_fe_disconnect(&s->chardev);
> return;
> }
> @@ -427,7 +427,7 @@ static void vhost_user_blk_device_realize(DeviceState
> *dev, Error **errp)
> int i, ret;
>
> if (!s->chardev.chr) {
> - error_setg(errp, "vhost-user-blk: chardev is mandatory");
> + error_setg(errp, "chardev is mandatory");
> return;
> }
>
> @@ -435,16 +435,16 @@ static void vhost_user_blk_device_realize(DeviceState
> *dev, Error **errp)
> s->num_queues = 1;
> }
> if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
> - error_setg(errp, "vhost-user-blk: invalid number of IO queues");
> + error_setg(errp, "invalid number of IO queues");
> return;
> }
>
> if (!s->queue_size) {
> - error_setg(errp, "vhost-user-blk: queue size must be non-zero");
> + error_setg(errp, "queue size must be non-zero");
> return;
> }
> if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
> - error_setg(errp, "vhost-user-blk: queue size must not exceed %d",
> + error_setg(errp, "queue size must not exceed %d",
> VIRTQUEUE_MAX_SIZE);
> return;
> }
> @@ -471,7 +471,7 @@ static void vhost_user_blk_device_realize(DeviceState
> *dev, Error **errp)
> goto virtio_err;
> }
>
> - if (vhost_user_blk_connect(dev) < 0) {
> + if (vhost_user_blk_connect(dev, errp) < 0) {
> qemu_chr_fe_disconnect(&s->chardev);
> goto virtio_err;
> }
> --
> 2.30.2
>
[PATCH 5/5] vhost-user-blk: Check that num-queues is supported by backend, Kevin Wolf, 2021/04/22
[PATCH 4/5] virtio: Fail if iommu_platform is requested, but unsupported, Kevin Wolf, 2021/04/22
[PATCH 3/5] vhost-user-blk: Get more feature flags from vhost device, Kevin Wolf, 2021/04/22