From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Zhadchenko Date: Mon, 24 Aug 2026 17:04:42 +0200 Subject: Re: [Devel] [PATCH VZ10 2/5] drivers/vhost/blk: report correct used-ring lengths In-Reply-To: <178723506422.906560.13876408914701409678.b4-review@b4> References: <20260818150920.13123-1-andrey.zhadchenko@virtuozzo.com> <20260818150920.13123-3-andrey.zhadchenko@virtuozzo.com> <178723506422.906560.13876408914701409678.b4-review@b4> Message-ID: <7eca02f1-26e5-4bc4-ad87-2c45ff2a5a9c@virtuozzo.com> List-Id: 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 >> >> 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!