[Qemu-devel] [PATCH v3 20/22] block: Move request_alignment into BlockLimit

[Eric Blake <eblake@redhat.com>]

It makes more sense to have ALL block size limit constraints
in the same struct.  Improve the documentation while at it.

Simplify a couple of conditionals, now that we have audited and
documented that request_alignment is always non-zero.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v3: drop dead conditionals
v2: drop hacks for save/restore of alignment, now that earlier
patches handled setting up BlockLimits more sanely
---
 include/block/block_int.h | 22 +++++++++++++---------
 block.c                   |  2 +-
 block/blkdebug.c          |  2 +-
 block/bochs.c             |  2 +-
 block/cloop.c             |  2 +-
 block/dmg.c               |  2 +-
 block/io.c                | 14 +++++++-------
 block/iscsi.c             |  2 +-
 block/qcow2.c             |  2 +-
 block/raw-posix.c         | 16 ++++++++--------
 block/raw-win32.c         |  6 +++---
 block/vvfat.c             |  2 +-
 12 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 388ef80..845800e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -324,6 +324,12 @@ struct BlockDriver {
 };

 typedef struct BlockLimits {
+    /* Alignment requirement, in bytes, for offset/length of I/O
+     * requests. Must be a power of 2 less than INT_MAX. A value of 0
+     * defaults to 1 for drivers with modern byte interfaces, and to
+     * 512 otherwise. */
+    uint32_t request_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
@@ -332,8 +338,8 @@ typedef struct BlockLimits {

     /* optimal alignment for discard requests in bytes, must be power
      * of 2, less than max_discard if that is set, and multiple of
-     * bs->request_alignment. May be 0 if bs->request_alignment is
-     * good enough */
+     * bl.request_alignment. May be 0 if bl.request_alignment is good
+     * enough */
     uint32_t pdiscard_alignment;

     /* maximum number of bytes that can zeroized at once (since it is
@@ -343,12 +349,12 @@ typedef struct BlockLimits {

     /* optimal alignment for write zeroes requests in bytes, must be
      * power of 2, less than max_pwrite_zeroes if that is set, and
-     * multiple of bs->request_alignment. May be 0 if
-     * bs->request_alignment is good enough */
+     * multiple of bl.request_alignment. May be 0 if
+     * bl.request_alignment is good enough */
     uint32_t pwrite_zeroes_alignment;

     /* optimal transfer length in bytes (must be power of 2, and
-     * multiple of bs->request_alignment), or 0 if no preferred size */
+     * multiple of bl.request_alignment), or 0 if no preferred size */
     uint32_t opt_transfer;

     /* maximal transfer length in bytes (need not be power of 2, but
@@ -356,10 +362,10 @@ typedef struct BlockLimits {
      * For now, anything larger than INT_MAX is clamped down. */
     uint32_t max_transfer;

-    /* memory alignment so that no bounce buffer is needed */
+    /* memory alignment, in bytes so that no bounce buffer is needed */
     size_t min_mem_alignment;

-    /* memory alignment for bounce buffer */
+    /* memory alignment, in bytes, for bounce buffer */
     size_t opt_mem_alignment;

     /* maximum number of iovec elements */
@@ -465,8 +471,6 @@ struct BlockDriverState {
     /* I/O Limits */
     BlockLimits bl;

-    /* Alignment requirement for offset/length of I/O requests */
-    unsigned int request_alignment;
     /* Flags honored during pwrite (so far: BDRV_REQ_FUA) */
     unsigned int supported_write_flags;
     /* Flags honored during pwrite_zeroes (so far: BDRV_REQ_FUA,
diff --git a/block.c b/block.c
index c2fbf06..34894ad 100644
--- a/block.c
+++ b/block.c
@@ -1016,7 +1016,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,

     assert(bdrv_opt_mem_align(bs) != 0);
     assert(bdrv_min_mem_align(bs) != 0);
-    assert(is_power_of_2(bs->request_alignment) || bdrv_is_sg(bs));
+    assert(is_power_of_2(bs->bl.request_alignment));

     qemu_opts_del(opts);
     return 0;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 54b6870..b6ecee3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -726,7 +726,7 @@ static void blkdebug_refresh_limits(BlockDriverState *bs, Error **errp)
     BDRVBlkdebugState *s = bs->opaque;

     if (s->align) {
-        bs->request_alignment = s->align;
+        bs->bl.request_alignment = s->align;
     }
 }

diff --git a/block/bochs.c b/block/bochs.c
index 182c50b..4194f1d 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -190,7 +190,7 @@ fail:

 static void bochs_refresh_limits(BlockDriverState *bs, Error **errp)
 {
-    bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
+    bs->bl.request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O */
 }

 static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
diff --git a/block/cloop.c b/block/cloop.c
index d574003..b5dc286 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -200,7 +200,7 @@ fail:

 static void cloop_refresh_limits(BlockDriverState *bs, Error **errp)
 {
-    bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
+    bs->bl.request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O */
 }

 static inline int cloop_read_block(BlockDriverState *bs, int block_num)
diff --git a/block/dmg.c b/block/dmg.c
index 1e53cd8..9612c21 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -548,7 +548,7 @@ fail:

 static void dmg_refresh_limits(BlockDriverState *bs, Error **errp)
 {
-    bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
+    bs->bl.request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O */
 }

 static inline int is_sector_in_chunk(BDRVDMGState* s,
diff --git a/block/io.c b/block/io.c
index 69dbbd3..b9e53e3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -90,7 +90,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
     }

     /* Default alignment based on whether driver has byte interface */
-    bs->request_alignment = drv->bdrv_co_preadv ? 1 : 512;
+    bs->bl.request_alignment = drv->bdrv_co_preadv ? 1 : 512;

     /* Take some limits from the children as a default */
     if (bs->file) {
@@ -459,7 +459,7 @@ static int bdrv_get_cluster_size(BlockDriverState *bs)

     ret = bdrv_get_info(bs, &bdi);
     if (ret < 0 || bdi.cluster_size == 0) {
-        return bs->request_alignment;
+        return bs->bl.request_alignment;
     } else {
         return bdi.cluster_size;
     }
@@ -1068,7 +1068,7 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
     BlockDriver *drv = bs->drv;
     BdrvTrackedRequest req;

-    uint64_t align = bs->request_alignment;
+    uint64_t align = bs->bl.request_alignment;
     uint8_t *head_buf = NULL;
     uint8_t *tail_buf = NULL;
     QEMUIOVector local_qiov;
@@ -1164,8 +1164,8 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
     int tail = 0;

     int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
-    int alignment = MAX(bs->bl.pwrite_zeroes_alignment ?: 1,
-                        bs->request_alignment);
+    int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
+                        bs->bl.request_alignment);

     assert(is_power_of_2(alignment));
     head = offset & (alignment - 1);
@@ -1324,7 +1324,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs,
     uint8_t *buf = NULL;
     QEMUIOVector local_qiov;
     struct iovec iov;
-    uint64_t align = bs->request_alignment;
+    uint64_t align = bs->bl.request_alignment;
     unsigned int head_padding_bytes, tail_padding_bytes;
     int ret = 0;

@@ -1411,7 +1411,7 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
     BdrvRequestFlags flags)
 {
     BdrvTrackedRequest req;
-    uint64_t align = bs->request_alignment;
+    uint64_t align = bs->bl.request_alignment;
     uint8_t *head_buf = NULL;
     uint8_t *tail_buf = NULL;
     QEMUIOVector local_qiov;
diff --git a/block/iscsi.c b/block/iscsi.c
index 0d16c31..6e8d4fe 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1704,7 +1704,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
     IscsiLun *iscsilun = bs->opaque;
     uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;

-    bs->request_alignment = iscsilun->block_size;
+    bs->bl.request_alignment = iscsilun->block_size;

     if (iscsilun->bl.max_xfer_len) {
         max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len);
diff --git a/block/qcow2.c b/block/qcow2.c
index 48f80b6..fdf13cb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1201,7 +1201,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)

     if (bs->encrypted) {
         /* Encryption works on a sector granularity */
-        bs->request_alignment = BDRV_SECTOR_SIZE;
+        bs->bl.request_alignment = BDRV_SECTOR_SIZE;
     }
     bs->bl.pwrite_zeroes_alignment = s->cluster_size;
 }
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 8da2f94..d3d7cce 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -302,22 +302,22 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
     /* For SCSI generic devices the alignment is not really used.
        With buffered I/O, we don't have any restrictions. */
     if (bdrv_is_sg(bs) || !s->needs_alignment) {
-        bs->request_alignment = 1;
+        bs->bl.request_alignment = 1;
         s->buf_align = 1;
         return;
     }

-    bs->request_alignment = 0;
+    bs->bl.request_alignment = 0;
     s->buf_align = 0;
     /* Let's try to use the logical blocksize for the alignment. */
-    if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
-        bs->request_alignment = 0;
+    if (probe_logical_blocksize(fd, &bs->bl.request_alignment) < 0) {
+        bs->bl.request_alignment = 0;
     }
 #ifdef CONFIG_XFS
     if (s->is_xfs) {
         struct dioattr da;
         if (xfsctl(NULL, fd, XFS_IOC_DIOINFO, &da) >= 0) {
-            bs->request_alignment = da.d_miniosz;
+            bs->bl.request_alignment = da.d_miniosz;
             /* The kernel returns wrong information for d_mem */
             /* s->buf_align = da.d_mem; */
         }
@@ -337,19 +337,19 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
         qemu_vfree(buf);
     }

-    if (!bs->request_alignment) {
+    if (!bs->bl.request_alignment) {
         size_t align;
         buf = qemu_memalign(s->buf_align, max_align);
         for (align = 512; align <= max_align; align <<= 1) {
             if (raw_is_io_aligned(fd, buf, align)) {
-                bs->request_alignment = align;
+                bs->bl.request_alignment = align;
                 break;
             }
         }
         qemu_vfree(buf);
     }

-    if (!s->buf_align || !bs->request_alignment) {
+    if (!s->buf_align || !bs->bl.request_alignment) {
         error_setg(errp, "Could not find working O_DIRECT alignment. "
                          "Try cache.direct=off.");
     }
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 88382d9..62edb1a 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -230,14 +230,14 @@ static void raw_probe_alignment(BlockDriverState *bs, Error **errp)
     BOOL status;

     if (s->type == FTYPE_CD) {
-        bs->request_alignment = 2048;
+        bs->bl.request_alignment = 2048;
         return;
     }
     if (s->type == FTYPE_HARDDISK) {
         status = DeviceIoControl(s->hfile, IOCTL_DISK_GET_DRIVE_GEOMETRY_EX,
                                  NULL, 0, &dg, sizeof(dg), &count, NULL);
         if (status != 0) {
-            bs->request_alignment = dg.Geometry.BytesPerSector;
+            bs->bl.request_alignment = dg.Geometry.BytesPerSector;
             return;
         }
         /* try GetDiskFreeSpace too */
@@ -247,7 +247,7 @@ static void raw_probe_alignment(BlockDriverState *bs, Error **errp)
         GetDiskFreeSpace(s->drive_path, &sectorsPerCluster,
                          &dg.Geometry.BytesPerSector,
                          &freeClusters, &totalClusters);
-        bs->request_alignment = dg.Geometry.BytesPerSector;
+        bs->bl.request_alignment = dg.Geometry.BytesPerSector;
     }
 }

diff --git a/block/vvfat.c b/block/vvfat.c
index 4d44636..fc948cb 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1210,7 +1210,7 @@ fail:

 static void vvfat_refresh_limits(BlockDriverState *bs, Error **errp)
 {
-    bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
+    bs->bl.request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O */
 }

 static inline void vvfat_close_current_file(BDRVVVFATState *s)
-- 
2.5.5