block: Switch transfer length bounds to byte-based

Sector-based limits are awkward to think about; in our on-going
quest to move to byte-based interfaces, convert max_transfer_length
and opt_transfer_length.  Rename them (dropping the _length suffix)
so that the compiler will help us catch the change in semantics
across any rebased code, and improve the documentation.  Use unsigned
values, so that we don't have to worry about negative values and
so that bit-twiddling is easier; however, we are still constrained
by 2^31 of signed int in most APIs.

When a value comes from an external source (iscsi and raw-posix),
sanitize the results to ensure that opt_transfer is a power of 2.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

diff --git a/block/block-backend.c b/block/block-backend.c
index 1fb070b4e4778f6406995e893283db2e3239d4f6..e0425440257b568fed0552715968b17143bfe3e8 100644 (file)
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1303,16 +1303,16 @@ int blk_get_flags(BlockBackend *blk)
     }
 }
 
-/* Returns the maximum transfer length, in sectors; guaranteed nonzero */
-int blk_get_max_transfer_length(BlockBackend *blk)
+/* Returns the maximum transfer length, in bytes; guaranteed nonzero */
+uint32_t blk_get_max_transfer(BlockBackend *blk)
 {
     BlockDriverState *bs = blk_bs(blk);
-    int max = 0;
+    uint32_t max = 0;
 
     if (bs) {
-        max = bs->bl.max_transfer_length;
+        max = bs->bl.max_transfer;
     }
-    return MIN_NON_ZERO(max, BDRV_REQUEST_MAX_SECTORS);
+    return MIN_NON_ZERO(max, INT_MAX);
 }
 
 int blk_get_max_iov(BlockBackend *blk)

diff --git a/block/io.c b/block/io.c
index 323e822dd45e29de194b71bf0a2ee79570948453..8ca9d4372415bccf7600b53420575767e473ead7 100644 (file)
--- a/block/io.c
+++ b/block/io.c
@@ -88,8 +88,8 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
             error_propagate(errp, local_err);
             return;
         }
-        bs->bl.opt_transfer_length = bs->file->bs->bl.opt_transfer_length;
-        bs->bl.max_transfer_length = bs->file->bs->bl.max_transfer_length;
+        bs->bl.opt_transfer = bs->file->bs->bl.opt_transfer;
+        bs->bl.max_transfer = bs->file->bs->bl.max_transfer;
         bs->bl.min_mem_alignment = bs->file->bs->bl.min_mem_alignment;
         bs->bl.opt_mem_alignment = bs->file->bs->bl.opt_mem_alignment;
         bs->bl.max_iov = bs->file->bs->bl.max_iov;
@@ -107,12 +107,10 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
             error_propagate(errp, local_err);
             return;
         }
-        bs->bl.opt_transfer_length =
-            MAX(bs->bl.opt_transfer_length,
-                bs->backing->bs->bl.opt_transfer_length);
-        bs->bl.max_transfer_length =
-            MIN_NON_ZERO(bs->bl.max_transfer_length,
-                         bs->backing->bs->bl.max_transfer_length);
+        bs->bl.opt_transfer = MAX(bs->bl.opt_transfer,
+                                  bs->backing->bs->bl.opt_transfer);
+        bs->bl.max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
+                                           bs->backing->bs->bl.max_transfer);
         bs->bl.opt_mem_alignment =
             MAX(bs->bl.opt_mem_alignment,
                 bs->backing->bs->bl.opt_mem_alignment);
@@ -1156,7 +1154,8 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
     return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov, 0);
 }
 
-#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768
+/* Maximum buffer for write zeroes fallback, in bytes */
+#define MAX_WRITE_ZEROES_BOUNCE_BUFFER (32768 << BDRV_SECTOR_BITS)
 
 static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
     int64_t offset, int count, BdrvRequestFlags flags)
@@ -1214,7 +1213,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
 
         if (ret == -ENOTSUP) {
             /* Fall back to bounce buffer if write zeroes is unsupported */
-            int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer_length,
+            int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
                                             MAX_WRITE_ZEROES_BOUNCE_BUFFER);
             BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE;
 
@@ -1225,7 +1224,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
                 write_flags &= ~BDRV_REQ_FUA;
                 need_flush = true;
             }
-            num = MIN(num, max_xfer_len << BDRV_SECTOR_BITS);
+            num = MIN(num, max_transfer);
             iov.iov_len = num;
             if (iov.iov_base == NULL) {
                 iov.iov_base = qemu_try_blockalign(bs, num);
@@ -1242,7 +1241,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
             /* Keep bounce buffer around if it is big enough for all
              * all future requests.
              */
-            if (num < max_xfer_len << BDRV_SECTOR_BITS) {
+            if (num < max_transfer) {
                 qemu_vfree(iov.iov_base);
                 iov.iov_base = NULL;
             }

diff --git a/block/iscsi.c b/block/iscsi.c
index 721afb7a748cd27715b02d3b4479b5b9db366f4d..bde4a04a74e49f652dfae63d4236a39aa16c2e03 100644 (file)
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -473,9 +473,10 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
         return -EINVAL;
     }
 
-    if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length) {
+    if (bs->bl.max_transfer &&
+        nb_sectors << BDRV_SECTOR_BITS > bs->bl.max_transfer) {
         error_report("iSCSI Error: Write of %d sectors exceeds max_xfer_len "
-                     "of %d sectors", nb_sectors, bs->bl.max_transfer_length);
+                     "of %" PRIu32 " bytes", nb_sectors, bs->bl.max_transfer);
         return -EINVAL;
     }
 
@@ -650,9 +651,10 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
         return -EINVAL;
     }
 
-    if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length) {
+    if (bs->bl.max_transfer &&
+        nb_sectors << BDRV_SECTOR_BITS > bs->bl.max_transfer) {
         error_report("iSCSI Error: Read of %d sectors exceeds max_xfer_len "
-                     "of %d sectors", nb_sectors, bs->bl.max_transfer_length);
+                     "of %" PRIu32 " bytes", nb_sectors, bs->bl.max_transfer);
         return -EINVAL;
     }
 
@@ -1708,7 +1710,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
      * iscsi_open(): iscsi targets don't change their limits. */
 
     IscsiLun *iscsilun = bs->opaque;
-    uint32_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
+    uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
 
     bs->request_alignment = iscsilun->block_size;
 
@@ -1716,7 +1718,9 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
         max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len);
     }
 
-    bs->bl.max_transfer_length = sector_limits_lun2qemu(max_xfer_len, iscsilun);
+    if (max_xfer_len * iscsilun->block_size < INT_MAX) {
+        bs->bl.max_transfer = max_xfer_len * iscsilun->block_size;
+    }
 
     if (iscsilun->lbp.lbpu) {
         if (iscsilun->bl.max_unmap < 0xffffffff) {
@@ -1739,8 +1743,11 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
     } else {
         bs->bl.pwrite_zeroes_alignment = iscsilun->block_size;
     }
-    bs->bl.opt_transfer_length =
-        sector_limits_lun2qemu(iscsilun->bl.opt_xfer_len, iscsilun);
+    if (iscsilun->bl.opt_xfer_len &&
+        iscsilun->bl.opt_xfer_len < INT_MAX / iscsilun->block_size) {
+        bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len *
+                                        iscsilun->block_size);
+    }
 }
 
 /* Note that this will not re-establish a connection with an iSCSI target - it

diff --git a/block/nbd.c b/block/nbd.c
index bf67c8a70b997056febd9b4af4d473449bd2eee6..f5511ea1d48936995d25f82ae021f908e5502287 100644 (file)
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -363,7 +363,7 @@ static int nbd_co_flush(BlockDriverState *bs)
 static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     bs->bl.max_discard = NBD_MAX_SECTORS;
-    bs->bl.max_transfer_length = NBD_MAX_SECTORS;
+    bs->bl.max_transfer = NBD_MAX_BUFFER_SIZE;
 }
 
 static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,

diff --git a/block/raw-posix.c b/block/raw-posix.c
index bef7a671a6af4b778da14ba80f44ad00fb2f1862..8da2f94ec414a8a5ac742a1bf7b9818b7c29564d 100644 (file)
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -745,8 +745,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
     if (!fstat(s->fd, &st)) {
         if (S_ISBLK(st.st_mode)) {
             int ret = hdev_get_max_transfer_length(s->fd);
-            if (ret >= 0) {
-                bs->bl.max_transfer_length = ret;
+            if (ret > 0 && ret <= BDRV_REQUEST_MAX_SECTORS) {
+                bs->bl.max_transfer = pow2floor(ret << BDRV_SECTOR_BITS);
             }
         }
     }

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index dd94cd4dc70adafe8e164b07f716a542923f27f7..ae86e944ea724b339d688c66e42b85471f688144 100644 (file)
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -384,7 +384,7 @@ static int multireq_compare(const void *a, const void *b)
 void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
 {
     int i = 0, start = 0, num_reqs = 0, niov = 0, nb_sectors = 0;
-    int max_xfer_len;
+    uint32_t max_transfer;
     int64_t sector_num = 0;
 
     if (mrb->num_reqs == 1) {
@@ -393,7 +393,7 @@ void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
         return;
     }
 
-    max_xfer_len = blk_get_max_transfer_length(mrb->reqs[0]->dev->blk);
+    max_transfer = blk_get_max_transfer(mrb->reqs[0]->dev->blk);
 
     qsort(mrb->reqs, mrb->num_reqs, sizeof(*mrb->reqs),
           &multireq_compare);
@@ -409,8 +409,9 @@ void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
              */
             if (sector_num + nb_sectors != req->sector_num ||
                 niov > blk_get_max_iov(blk) - req->qiov.niov ||
-                req->qiov.size / BDRV_SECTOR_SIZE > max_xfer_len ||
-                nb_sectors > max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE) {
+                req->qiov.size > max_transfer ||
+                nb_sectors > (max_transfer -
+                              req->qiov.size) / BDRV_SECTOR_SIZE) {
                 submit_requests(blk, mrb, start, num_reqs, niov);
                 num_reqs = 0;
             }

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 0cb8568f1f5af0763bc6f4df5f81ef74d9dd161d..7a588a7ad43f0d5a4d0e071ff6159549b6b397ec 100644 (file)
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -225,14 +225,14 @@ static void scsi_read_complete(void * opaque, int ret)
     if (s->type == TYPE_DISK &&
         r->req.cmd.buf[0] == INQUIRY &&
         r->req.cmd.buf[2] == 0xb0) {
-        uint32_t max_xfer_len = blk_get_max_transfer_length(s->conf.blk) /
-            (s->blocksize / BDRV_SECTOR_SIZE);
+        uint32_t max_transfer =
+            blk_get_max_transfer(s->conf.blk) / s->blocksize;
 
-        assert(max_xfer_len);
-        stl_be_p(&r->buf[8], max_xfer_len);
+        assert(max_transfer);
+        stl_be_p(&r->buf[8], max_transfer);
         /* Also take care of the opt xfer len. */
-        if (ldl_be_p(&r->buf[12]) > max_xfer_len) {
-            stl_be_p(&r->buf[12], max_xfer_len);
+        if (ldl_be_p(&r->buf[12]) > max_transfer) {
+            stl_be_p(&r->buf[12], max_transfer);
         }
     }
     scsi_req_data(&r->req, len);

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 205715600be1d81bec3df74d21089717b956c1f3..7d2b152779df10861591850544f113052899aa3a 100644 (file)
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -338,11 +338,14 @@ typedef struct BlockLimits {
      * power of 2, and less than max_pwrite_zeroes if that is set */
     uint32_t pwrite_zeroes_alignment;
 
-    /* optimal transfer length in sectors */
-    int opt_transfer_length;
-
-    /* maximal transfer length in sectors */
-    int max_transfer_length;
+    /* optimal transfer length in bytes (must be power of 2, and
+     * multiple of bs->request_alignment), or 0 if no preferred size */
+    uint32_t opt_transfer;
+
+    /* maximal transfer length in bytes (need not be power of 2, but
+     * should be multiple of opt_transfer), or 0 for no 32-bit limit.
+     * For now, anything larger than INT_MAX is clamped down. */
+    uint32_t max_transfer;
 
     /* memory alignment so that no bounce buffer is needed */
     size_t min_mem_alignment;

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index c04af8ea46e22b9f4b993a5262ea647f0e9f590a..2469a1c15b9750ff13bbc3487875c03a224a8998 100644 (file)
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -170,7 +170,7 @@ bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
 void blk_eject(BlockBackend *blk, bool eject_flag);
 int blk_get_flags(BlockBackend *blk);
-int blk_get_max_transfer_length(BlockBackend *blk);
+uint32_t blk_get_max_transfer(BlockBackend *blk);
 int blk_get_max_iov(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
 void *blk_try_blockalign(BlockBackend *blk, size_t size);

diff --git a/qemu-img.c b/qemu-img.c
index 235168646ebe49b9edcf139cbcefa27585b2d0d1..046b267420bfe65ef02f1244a7118259b25eba50 100644 (file)
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2085,13 +2085,13 @@ static int img_convert(int argc, char **argv)
     }
     out_bs = blk_bs(out_blk);
 
-    /* increase bufsectors from the default 4096 (2M) if opt_transfer_length
+    /* increase bufsectors from the default 4096 (2M) if opt_transfer
      * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)
      * as maximum. */
     bufsectors = MIN(32768,
-                     MAX(bufsectors, MAX(out_bs->bl.opt_transfer_length,
-                                         out_bs->bl.discard_alignment))
-                    );
+                     MAX(bufsectors,
+                         MAX(out_bs->bl.opt_transfer >> BDRV_SECTOR_BITS,
+                             out_bs->bl.discard_alignment)));
 
     if (skip_create) {
         int64_t output_sectors = blk_nb_sectors(out_blk);