Re: [Devel] [PATCH VZ10 2/5] drivers/vhost/blk: report correct used-ring lengths

[Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>]

On 8/20/26 16:11, Vasileios Almpanis wrote:
>> We are expected to return amount of bytes written to the guest,
>> which also includes status.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-138640
>> Fixes: 40a5928ec730 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
>> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
>>
>> diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
>> index 3def988cdf18..ae6b916f96e1 100644
>> --- a/drivers/vhost/blk.c
>> +++ b/drivers/vhost/blk.c
>> @@ -439,12 +439,14 @@ static int vhost_blk_req_submit(struct vhost_blk_req *req)
>>   
>>   static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>>   				struct virtio_blk_outhdr *hdr,
>> -				u16 head, u16 total_iov_nr)
>> +				u16 head, u16 out, u16 in)
>>   {
>>   	struct vhost_blk *blk = container_of(vq->dev, struct vhost_blk, dev);
>>   	struct vhost_blk_vq *blk_vq = container_of(vq, struct vhost_blk_vq, vq);
>>   	struct vhost_blk_req *req;
>> +	u16 total_iov_nr = out + in;
>>   	struct iov_iter iter;
>> +	size_t in_len;
>>   	int ret, len;
>>   	u8 status;
>>   
>> @@ -455,8 +457,9 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>>   	req->sector	= hdr->sector;
>>   	req->iov	= blk_vq->iov;
>>   	req->bio_err	= 0;
>> +	in_len		= iov_length(vq->iov + out, in);
>>   
>> -	if (iov_length(vq->iov, total_iov_nr) < sizeof(status))
>> +	if (in_len < sizeof(status) || in_len > INT_MAX)
>>   		return -EINVAL;
>>   
>>   	req->len	= iov_length(vq->iov, total_iov_nr) - sizeof(status);
>> @@ -492,14 +495,14 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>>   		ret = vhost_blk_set_status(req, status);
>>   		if (ret)
>>   			break;
>> -		vhost_add_used_and_signal(&blk->dev, vq, head, len);
>> +		vhost_add_used_and_signal(&blk->dev, vq, head, in_len);
>>   		break;
>>   	default:
>>   		status = VIRTIO_BLK_S_UNSUPP;
>>   		ret = vhost_blk_set_status(req, status);
>>   		if (ret)
>>   			break;
>> -		vhost_add_used_and_signal(&blk->dev, vq, head, 0);
>> +		vhost_add_used_and_signal(&blk->dev, vq, head, sizeof(status));
>>   	}
>>   
>>   	return ret;
>> @@ -555,7 +558,7 @@ static void vhost_blk_handle_guest_kick(struct vhost_work *work)
>>   			break;
>>   		}
>>   
>> -		ret = vhost_blk_req_handle(vq, &hdr, head, out + in);
>> +		ret = vhost_blk_req_handle(vq, &hdr, head, out, in);
>>   		if (ret == -EAGAIN || ret == -ENOMEM) {
>>   			vhost_discard_vq_desc(vq, 1);
>>   			vhost_poll_queue(&vq->poll);
>> @@ -604,7 +607,7 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
>>   		if (vhost_blk_set_status(req, status)) {
>>   			vhostblk_vq_err(blk, vq, "Failed to write status");
>>   		} else {
>> -			vhost_add_used(vq, req->head, req->len);
>> +			vhost_add_used(vq, req->head, req->len + sizeof(status));
> Is req->len + sizeof(status) the right thing to use for write requests?
> We should only write the status byte not status + payload length
> 

Oh yes. I fixed the missing status but not this bug. Thanks!