introduce max_transfer_length

PATCH Superseded Peter Lieven 1 participant 5 messages Sep 12, 2014 Block layer

Share & tools

Peter Lieven

author

2014-09-12 11:23 UTC #0 lore raw

v2 cover introduce max_transfer_length

This series adds the basics for introducing a maximum transfer length to the block layer. Its main purpose is currently avoiding that a multiwrite_merge exceeds the max_xfer_len of an attached iSCSI LUN. This is a required bug fix.

Discussed reporting of this maximum in the SCSI Disk Inquiry Emulation of the Block Limits VPD is currently not added as we do not import any of the other limits there. This has to be addresses in a seperate series.

v1->v2: do not throw errors but generate trace events in Patch 2 [Paolo]

Peter Lieven (4):
BlockLimits: introduce max_transfer_length
block: immediately cancel oversized read/write requests block/iscsi: set max_transfer_length
block: avoid creating oversized writes in multiwrite_merge

 block.c                   |   23 +++++++++++++++++++++++
 block/iscsi.c             |   12 ++++++++++--
 include/block/block_int.h |    3 +++
 trace-events              |    2 ++
 4 files changed, 38 insertions(+), 2 deletions(-)

Signature · 1 line

1.7.9.5

Peter Lieven

author

2014-09-12 11:23 UTC #1 lore raw

PATCH v2 1/4 BlockLimits: introduce max_transfer_length

Signed-off-by Peter Lieven <[email protected]>

Reviewed-by Ronnie Sahlberg <[email protected]>

---
 block.c                   |    4 ++++
 include/block/block_int.h |    3 +++
 2 files changed, 7 insertions(+)

block.c +4 -0

index d06dd51..c87e9fd 100644
@@ -529,6 +529,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
             return;
         }
         bs->bl.opt_transfer_length = bs->file->bl.opt_transfer_length;
+        bs->bl.max_transfer_length = bs->file->bl.max_transfer_length;
         bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
     } else {
         bs->bl.opt_mem_alignment = 512;

@@ -543,6 +544,9 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.opt_transfer_length =
             MAX(bs->bl.opt_transfer_length,
                 bs->backing_hd->bl.opt_transfer_length);
+        bs->bl.max_transfer_length =
+            MIN(bs->bl.max_transfer_length,
+                bs->backing_hd->bl.max_transfer_length);
         bs->bl.opt_mem_alignment =
             MAX(bs->bl.opt_mem_alignment,
                 bs->backing_hd->bl.opt_mem_alignment);

include/block/block_int.h +3 -0

index 8a61215..e178782 100644
@@ -288,6 +288,9 @@ typedef struct BlockLimits {
     /* optimal transfer length in sectors */
     int opt_transfer_length;
 
+    /* maximal transfer length in sectors */
+    int max_transfer_length;
+
     /* memory alignment so that no bounce buffer is needed */
     size_t opt_mem_alignment;
 } BlockLimits;

Signature · 1 line

1.7.9.5

Peter Lieven

author

2014-09-12 11:23 UTC #2 lore raw

PATCH v2 2/4 block: immediately cancel oversized read/write requests

In reply to Peter Lieven

Signed-off-by Peter Lieven <[email protected]>

---
 block.c      |   14 ++++++++++++++
 trace-events |    2 ++
 2 files changed, 16 insertions(+)

block.c +14 -0

index c87e9fd..965e9bc 100644
@@ -3215,6 +3215,13 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
         return -EINVAL;
     }
 
+    if (bs->bl.max_transfer_length &&
+        nb_sectors > bs->bl.max_transfer_length) {
+        trace_bdrv_co_do_readv_toobig(bs, sector_num, nb_sectors,
+                                      bs->bl.max_transfer_length);
+        return -EINVAL;
+    }
+
     return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
                              nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
 }

@@ -3507,6 +3514,13 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
         return -EINVAL;
     }
 
+    if (bs->bl.max_transfer_length &&
+        nb_sectors > bs->bl.max_transfer_length) {
+        trace_bdrv_co_do_writev_toobig(bs, sector_num, nb_sectors,
+                                       bs->bl.max_transfer_length);
+        return -EINVAL;
+    }
+
     return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
                               nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
 }

trace-events +2 -0

index fb58963..fe2c5d8 100644
@@ -68,8 +68,10 @@ bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs
 bdrv_aio_write_zeroes(void *bs, int64_t sector_num, int nb_sectors, int flags, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x opaque %p"
 bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
 bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
+bdrv_co_do_readv_toobig(void *bs, int64_t sector_num, int nb_sector, int max_transfer_length) "bs %p sector_num %"PRId64" nb_sectors %d bs->bl.max_transfer_length %d"
 bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
+bdrv_co_do_writev_toobig(void *bs, int64_t sector_num, int nb_sector, int max_transfer_length) "bs %p sector_num %"PRId64" nb_sectors %d bs->bl.max_transfer_length %d"
 bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector, int flags) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x"
 bdrv_co_io_em(void *bs, int64_t sector_num, int nb_sectors, int is_write, void *acb) "bs %p sector_num %"PRId64" nb_sectors %d is_write %d acb %p"
 bdrv_co_do_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t cluster_sector_num, int cluster_nb_sectors) "bs %p sector_num %"PRId64" nb_sectors %d cluster_sector_num %"PRId64" cluster_nb_sectors %d"

Signature · 1 line

1.7.9.5

Peter Lieven

author

2014-09-12 11:23 UTC #3 lore raw

PATCH v2 3/4 block/iscsi: set max_transfer_length

In reply to Peter Lieven

the limit of 0xffffff for 16 byte CDBs is intentional to avoid overflows on 32-bit architectures.

Signed-off-by Peter Lieven <[email protected]>

Reviewed-by Ronnie Sahlberg <[email protected]>

---
 block/iscsi.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

block/iscsi.c +10 -2

index 3e19202..a4b625c 100644
@@ -1455,10 +1455,18 @@ static void iscsi_close(BlockDriverState *bs)
 
 static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
 {
-    IscsiLun *iscsilun = bs->opaque;
-
     /* We don't actually refresh here, but just return data queried in
      * iscsi_open(): iscsi targets don't change their limits. */
+
+    IscsiLun *iscsilun = bs->opaque;
+    uint32_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffff : 0xffff;
+
+    if (iscsilun->bl.max_xfer_len) {
+        max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len);
+    }
+
+    bs->bl.max_transfer_length = sector_lun2qemu(max_xfer_len, iscsilun);
+
     if (iscsilun->lbp.lbpu) {
         if (iscsilun->bl.max_unmap < 0xffffffff) {
             bs->bl.max_discard = sector_lun2qemu(iscsilun->bl.max_unmap,

Signature · 1 line

1.7.9.5

Peter Lieven

author

2014-09-12 11:23 UTC #4 lore raw

PATCH v2 4/4 block: avoid creating oversized writes in multiwrite_merge

In reply to Peter Lieven

Signed-off-by Peter Lieven <[email protected]>

Reviewed-by Ronnie Sahlberg <[email protected]>

---
 block.c |    5 +++++
 1 file changed, 5 insertions(+)

block.c +5 -0

index 965e9bc..2b9be99 100644
@@ -4554,6 +4554,11 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
             merge = 0;
         }
 
+        if (bs->bl.max_transfer_length && reqs[outidx].nb_sectors +
+            reqs[i].nb_sectors > bs->bl.max_transfer_length) {
+            merge = 0;
+        }
+
         if (merge) {
             size_t size;
             QEMUIOVector *qiov = g_malloc0(sizeof(*qiov));

Signature · 1 line

1.7.9.5