On Wed, May 18, 2022 at 9:14 PM Stefan Hajnoczi <stefa...@redhat.com> wrote:
>
> On Wed, May 04, 2022 at 03:40:46PM +0800, Xie Yongji wrote:
> > -static void vu_blk_req_complete(VuBlkReq *req)
> > +static void vu_blk_req_complete(VuBlkReq *req, size_t in_len)
> > {
> > VuDev *vu_dev = &req->server->vu_dev;
> >
> > - /* IO size with 1 extra status byte */
> > - vu_queue_push(vu_dev, req->vq, &req->elem, req->size + 1);
> > + vu_queue_push(vu_dev, req->vq, &req->elem, in_len);
>
> I think this silently fixes a bug: now the correct len value is
> calculated. Before the I/O buffer wasn't counted in read requests.
Yes.
> Please mention this in the commit description.
>
OK.
> > +static bool virtio_blk_sect_range_ok(BlockBackend *blk,
> > + uint64_t sector, size_t size)
> > +{
> > + uint64_t nb_sectors;
> > + uint64_t total_sectors;
> > +
> > + if (size % VIRTIO_BLK_SECTOR_SIZE) {
> > + return false;
> > + }
> > +
> > + nb_sectors = size >> VIRTIO_BLK_SECTOR_BITS;
> > +
> > + QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != VIRTIO_BLK_SECTOR_SIZE);
> > + if (nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
> > + return false;
> > + }
> > + if ((sector << VIRTIO_BLK_SECTOR_BITS) %
> > blk_get_guest_block_size(blk)) {
>
> Please use VirtioBlkHandler->logical_block_size instead (see below).
>
> > +int coroutine_fn virtio_blk_process_req(BlockBackend *blk, bool writable,
> > + const char *serial,
>
> I suggest defining a struct instead of passing individual arguments:
>
> typedef struct {
> BlockBackend *blk;
> const char *serial;
> uint32_t logical_block_size;
> bool writable;
> } VirtioBlkHandler;
OK, will do it in v6.
Thanks,
Yongji
