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

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

>> @@ -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.
The vhostblk_vq_err() was added at an earlier reviewer's request.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel