Re: [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] vhost-blk: in-flight head error is logged and signalled twice
--

commit 50430ed7a617303d49fca9a87eb2e315f023f9f7
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

drivers/vhost/blk: reject a head that is still in flight

This patch adds an inflight flag to each vhost_blk_req slot. The flag is set
at bio submission and cleared when completion is handled. If the guest
posts a head whose slot is still in flight, vhost_blk_req_handle() now
returns -EBUSY. The slots are also allocated zeroed.

> diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
> --- a/drivers/vhost/blk.c
> +++ b/drivers/vhost/blk.c

[ ... ]

> @@ -457,6 +459,11 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>  	u8 status;
>  
>  	req		= &blk_vq->req[head];
> +	if (unlikely(req->inflight)) {
> +		vhostblk_vq_err(blk, vq, "Guest reused in-flight head %d", head);
> +		return -EBUSY;
> +	}
> +

[Severity: Low]
Is this error reported twice? vhostblk_vq_err() here already calls
pr_warn_ratelimited() and eventfd_signal() on vq->error_ctx, and then
-EBUSY is returned.

In vhost_blk_handle_guest_kick(), only -EAGAIN and -ENOMEM are special-cased:

	ret = vhost_blk_req_handle(vq, &hdr, head, out, in);
	if (ret == -EAGAIN || ret == -ENOMEM) {
		...
	} else if (ret < 0) {
		vhostblk_vq_err(blk, vq, "Failed to process guest request (head %d) with %d",
				head, ret);
		break;
	}

So -EBUSY reaches the second branch, which calls vhostblk_vq_err() again.
That gives a second warning and a second eventfd_signal() for one guest
misbehaviour.

The commit message says "Log the head and let the caller report the error on
the vq error eventfd". Should the vhostblk_vq_err() in vhost_blk_req_handle()
be a plain log message, such as pr_warn_ratelimited(), so that only the
caller signals the eventfd? Other early errors in this function, such as
-EINVAL, are reported only by the caller.

> [ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029134822.3367197-1-alexander.ivanov@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel