Re: [PATCH v5 3/8] block/export: Abstract out the logic of virtio-blk I/O process

[Stefan Hajnoczi]

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.
Please mention this in the commit description.

> +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;

signature.asc
Description: PGP signature