block: Switch discard 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_discard and
discard_alignment.  Rename them, using 'pdiscard' as an aid to
track which remaining discard interfaces need conversion, and so
that the compiler will help us catch the change in semantics
across any rebased code.  The BlockLimits type is now completely
byte-based; and in iscsi.c, sector_limits_lun2qemu() is no
longer needed.

pdiscard_alignment is made unsigned (we use power-of-2 alignments
as bitmasks, where unsigned is easier to think about) while
leaving max_pdiscard signed (since we still have an 'int'
interface); this is comparable to what commit cf081fc did for
write zeroes limits.  We may later want to make everything an
unsigned 64-bit limit - but that requires a bigger code audit.

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/io.c b/block/io.c
index 8ca9d4372415bccf7600b53420575767e473ead7..0f15d05297804ee75e517c8c67e66ac24c645558 100644 (file)
--- a/block/io.c
+++ b/block/io.c
@@ -2368,19 +2368,21 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
         goto out;
     }
 
-    max_discard = MIN_NON_ZERO(bs->bl.max_discard, BDRV_REQUEST_MAX_SECTORS);
+    max_discard = MIN_NON_ZERO(bs->bl.max_pdiscard >> BDRV_SECTOR_BITS,
+                               BDRV_REQUEST_MAX_SECTORS);
     while (nb_sectors > 0) {
         int ret;
         int num = nb_sectors;
+        int discard_alignment = bs->bl.pdiscard_alignment >> BDRV_SECTOR_BITS;
 
         /* align request */
-        if (bs->bl.discard_alignment &&
-            num >= bs->bl.discard_alignment &&
-            sector_num % bs->bl.discard_alignment) {
-            if (num > bs->bl.discard_alignment) {
-                num = bs->bl.discard_alignment;
+        if (discard_alignment &&
+            num >= discard_alignment &&
+            sector_num % discard_alignment) {
+            if (num > discard_alignment) {
+                num = discard_alignment;
             }
-            num -= sector_num % bs->bl.discard_alignment;
+            num -= sector_num % discard_alignment;
         }
 
         /* limit request size */

diff --git a/block/iscsi.c b/block/iscsi.c
index bde4a04a74e49f652dfae63d4236a39aa16c2e03..342f6b81865e5cb5c3e88d3b45c8c78b8783dce8 100644 (file)
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1697,13 +1697,6 @@ static void iscsi_close(BlockDriverState *bs)
     memset(iscsilun, 0, sizeof(IscsiLun));
 }
 
-static int sector_limits_lun2qemu(int64_t sector, IscsiLun *iscsilun)
-{
-    int limit = MIN(sector_lun2qemu(sector, iscsilun), INT_MAX / 2 + 1);
-
-    return limit < BDRV_REQUEST_MAX_SECTORS ? limit : 0;
-}
-
 static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     /* We don't actually refresh here, but just return data queried in
@@ -1723,14 +1716,14 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
     }
 
     if (iscsilun->lbp.lbpu) {
-        if (iscsilun->bl.max_unmap < 0xffffffff) {
-            bs->bl.max_discard =
-                sector_limits_lun2qemu(iscsilun->bl.max_unmap, iscsilun);
+        if (iscsilun->bl.max_unmap < 0xffffffff / iscsilun->block_size) {
+            bs->bl.max_pdiscard =
+                iscsilun->bl.max_unmap * iscsilun->block_size;
         }
-        bs->bl.discard_alignment =
-            sector_limits_lun2qemu(iscsilun->bl.opt_unmap_gran, iscsilun);
+        bs->bl.pdiscard_alignment =
+            iscsilun->bl.opt_unmap_gran * iscsilun->block_size;
     } else {
-        bs->bl.discard_alignment = iscsilun->block_size >> BDRV_SECTOR_BITS;
+        bs->bl.pdiscard_alignment = iscsilun->block_size;
     }
 
     if (iscsilun->bl.max_ws_len < 0xffffffff / iscsilun->block_size) {

diff --git a/block/nbd.c b/block/nbd.c
index f5511ea1d48936995d25f82ae021f908e5502287..08e5b67b2f197193cfb68a8f492c9ebad481d5a3 100644 (file)
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -362,7 +362,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_pdiscard = NBD_MAX_BUFFER_SIZE;
     bs->bl.max_transfer = NBD_MAX_BUFFER_SIZE;
 }
 

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7a4a00fdfcce9e62dc93775c571fb352553ca520..a3e69fd93a1c0f2b14d97938a8ddcbb891632eaa 100644 (file)
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -324,11 +324,17 @@ struct BlockDriver {
 };
 
 typedef struct BlockLimits {
-    /* maximum number of sectors that can be discarded at once */
-    int max_discard;
-
-    /* optimal alignment for discard requests in sectors */
-    int64_t discard_alignment;
+    /* maximum number of bytes that can be discarded at once (since it
+     * is signed, it must be < 2G, if set), should be multiple of
+     * pdiscard_alignment, but need not be power of 2. May be 0 if no
+     * inherent 32-bit limit */
+    int32_t max_pdiscard;
+
+    /* optimal alignment for discard requests in bytes, must be power
+     * of 2, less than max_pdiscard if that is set, and multiple of
+     * bs->request_alignment. May be 0 if bs->request_alignment is
+     * good enough */
+    uint32_t pdiscard_alignment;
 
     /* maximum number of bytes that can zeroized at once (since it is
      * signed, it must be < 2G, if set), should be multiple of

diff --git a/qemu-img.c b/qemu-img.c
index 046b267420bfe65ef02f1244a7118259b25eba50..3a7c162fb052ece62530fcacbe45167baa1f3c88 100644 (file)
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2091,7 +2091,8 @@ static int img_convert(int argc, char **argv)
     bufsectors = MIN(32768,
                      MAX(bufsectors,
                          MAX(out_bs->bl.opt_transfer >> BDRV_SECTOR_BITS,
-                             out_bs->bl.discard_alignment)));
+                             out_bs->bl.pdiscard_alignment >>
+                             BDRV_SECTOR_BITS)));
 
     if (skip_create) {
         int64_t output_sectors = blk_nb_sectors(out_blk);