diff mbox

[V2] block: introduce BDRV_REQUEST_MAX_SECTORS

Message ID 1423220051-16058-1-git-send-email-pl@kamp.de
State New
Headers show

Commit Message

Peter Lieven Feb. 6, 2015, 10:54 a.m. UTC
we check and adjust request sizes at several places with
sometimes inconsistent checks or default values:
 INT_MAX
 INT_MAX >> BDRV_SECTOR_BITS
 UINT_MAX >> BDRV_SECTOR_BITS
 SIZE_MAX >> BDRV_SECTOR_BITS

This patches introdocues a macro for the maximal allowed sectors
per request and uses it at several places.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
v1->v2: use macro in bdrv_check_byte_request alse [Den]

 block.c               |   21 +++++++++------------
 hw/block/virtio-blk.c |    4 ++--
 include/block/block.h |    3 +++
 3 files changed, 14 insertions(+), 14 deletions(-)

Comments

Denis V. Lunev Feb. 6, 2015, 11:24 a.m. UTC | #1
On 06/02/15 13:54, Peter Lieven wrote:
> we check and adjust request sizes at several places with
> sometimes inconsistent checks or default values:
>   INT_MAX
>   INT_MAX >> BDRV_SECTOR_BITS
>   UINT_MAX >> BDRV_SECTOR_BITS
>   SIZE_MAX >> BDRV_SECTOR_BITS
>
> This patches introdocues a macro for the maximal allowed sectors
> per request and uses it at several places.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> v1->v2: use macro in bdrv_check_byte_request alse [Den]
>
>   block.c               |   21 +++++++++------------
>   hw/block/virtio-blk.c |    4 ++--
>   include/block/block.h |    3 +++
>   3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/block.c b/block.c
> index 8272ef9..49e0073 100644
> --- a/block.c
> +++ b/block.c
> @@ -2647,7 +2647,7 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
>   {
>       int64_t len;
>   
> -    if (size > INT_MAX) {
> +    if (size > BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS) {
>           return -EIO;
>       }
>   
> @@ -2671,7 +2671,7 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
>   static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
>                                 int nb_sectors)
>   {
> -    if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
> +    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
>           return -EIO;
>       }
>   
> @@ -2758,7 +2758,7 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
>           .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
>       };
>   
> -    if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
> +    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
>           return -EINVAL;
>       }
>   
> @@ -2826,13 +2826,10 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
>       }
>   
>       for (;;) {
> -        nb_sectors = target_sectors - sector_num;
> +        nb_sectors = MIN(target_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS);
>           if (nb_sectors <= 0) {
>               return 0;
>           }
> -        if (nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
> -            nb_sectors = INT_MAX / BDRV_SECTOR_SIZE;
> -        }
>           ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
>           if (ret < 0) {
>               error_report("error getting block status at sector %" PRId64 ": %s",
> @@ -3167,7 +3164,7 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
>       int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
>       BdrvRequestFlags flags)
>   {
> -    if (nb_sectors < 0 || nb_sectors > (UINT_MAX >> BDRV_SECTOR_BITS)) {
> +    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
>           return -EINVAL;
>       }
>   
> @@ -3202,8 +3199,8 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
>       struct iovec iov = {0};
>       int ret = 0;
>   
> -    int max_write_zeroes = bs->bl.max_write_zeroes ?
> -                           bs->bl.max_write_zeroes : INT_MAX;
> +    int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_write_zeroes,
> +                                        BDRV_REQUEST_MAX_SECTORS);
>   
>       while (nb_sectors > 0 && !ret) {
>           int num = nb_sectors;
> @@ -3458,7 +3455,7 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
>       int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
>       BdrvRequestFlags flags)
>   {
> -    if (nb_sectors < 0 || nb_sectors > (INT_MAX >> BDRV_SECTOR_BITS)) {
> +    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
>           return -EINVAL;
>       }
>   
> @@ -5120,7 +5117,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
>           return 0;
>       }
>   
> -    max_discard = bs->bl.max_discard ?  bs->bl.max_discard : INT_MAX;
> +    max_discard = MIN_NON_ZERO(bs->bl.max_discard, BDRV_REQUEST_MAX_SECTORS);
>       while (nb_sectors > 0) {
>           int ret;
>           int num = nb_sectors;
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 8c51a29..1a8a176 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -381,7 +381,7 @@ void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
>       }
>   
>       max_xfer_len = blk_get_max_transfer_length(mrb->reqs[0]->dev->blk);
> -    max_xfer_len = MIN_NON_ZERO(max_xfer_len, INT_MAX);
> +    max_xfer_len = MIN_NON_ZERO(max_xfer_len, BDRV_REQUEST_MAX_SECTORS);
>   
>       qsort(mrb->reqs, mrb->num_reqs, sizeof(*mrb->reqs),
>             &multireq_compare);
> @@ -447,7 +447,7 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
>       uint64_t nb_sectors = size >> BDRV_SECTOR_BITS;
>       uint64_t total_sectors;
>   
> -    if (nb_sectors > INT_MAX) {
> +    if (nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
>           return false;
>       }
>       if (sector & dev->sector_mask) {
> diff --git a/include/block/block.h b/include/block/block.h
> index 3082d2b..25a6d62 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -83,6 +83,9 @@ typedef enum {
>   #define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)
>   #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
>   
> +#define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
> +                                     INT_MAX >> BDRV_SECTOR_BITS)
> +
>   /*
>    * Allocation status flags
>    * BDRV_BLOCK_DATA: data is read from bs->file or another file
Reviewed-by: Denis V. Lunev <den@openvz.org>
diff mbox

Patch

diff --git a/block.c b/block.c
index 8272ef9..49e0073 100644
--- a/block.c
+++ b/block.c
@@ -2647,7 +2647,7 @@  static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
 {
     int64_t len;
 
-    if (size > INT_MAX) {
+    if (size > BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS) {
         return -EIO;
     }
 
@@ -2671,7 +2671,7 @@  static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
 static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
                               int nb_sectors)
 {
-    if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
+    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
         return -EIO;
     }
 
@@ -2758,7 +2758,7 @@  static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
         .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
     };
 
-    if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
+    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
         return -EINVAL;
     }
 
@@ -2826,13 +2826,10 @@  int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
     }
 
     for (;;) {
-        nb_sectors = target_sectors - sector_num;
+        nb_sectors = MIN(target_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS);
         if (nb_sectors <= 0) {
             return 0;
         }
-        if (nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
-            nb_sectors = INT_MAX / BDRV_SECTOR_SIZE;
-        }
         ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
         if (ret < 0) {
             error_report("error getting block status at sector %" PRId64 ": %s",
@@ -3167,7 +3164,7 @@  static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
     BdrvRequestFlags flags)
 {
-    if (nb_sectors < 0 || nb_sectors > (UINT_MAX >> BDRV_SECTOR_BITS)) {
+    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
         return -EINVAL;
     }
 
@@ -3202,8 +3199,8 @@  static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
     struct iovec iov = {0};
     int ret = 0;
 
-    int max_write_zeroes = bs->bl.max_write_zeroes ?
-                           bs->bl.max_write_zeroes : INT_MAX;
+    int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_write_zeroes,
+                                        BDRV_REQUEST_MAX_SECTORS);
 
     while (nb_sectors > 0 && !ret) {
         int num = nb_sectors;
@@ -3458,7 +3455,7 @@  static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
     BdrvRequestFlags flags)
 {
-    if (nb_sectors < 0 || nb_sectors > (INT_MAX >> BDRV_SECTOR_BITS)) {
+    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
         return -EINVAL;
     }
 
@@ -5120,7 +5117,7 @@  int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
         return 0;
     }
 
-    max_discard = bs->bl.max_discard ?  bs->bl.max_discard : INT_MAX;
+    max_discard = MIN_NON_ZERO(bs->bl.max_discard, BDRV_REQUEST_MAX_SECTORS);
     while (nb_sectors > 0) {
         int ret;
         int num = nb_sectors;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8c51a29..1a8a176 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -381,7 +381,7 @@  void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
     }
 
     max_xfer_len = blk_get_max_transfer_length(mrb->reqs[0]->dev->blk);
-    max_xfer_len = MIN_NON_ZERO(max_xfer_len, INT_MAX);
+    max_xfer_len = MIN_NON_ZERO(max_xfer_len, BDRV_REQUEST_MAX_SECTORS);
 
     qsort(mrb->reqs, mrb->num_reqs, sizeof(*mrb->reqs),
           &multireq_compare);
@@ -447,7 +447,7 @@  static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
     uint64_t nb_sectors = size >> BDRV_SECTOR_BITS;
     uint64_t total_sectors;
 
-    if (nb_sectors > INT_MAX) {
+    if (nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
         return false;
     }
     if (sector & dev->sector_mask) {
diff --git a/include/block/block.h b/include/block/block.h
index 3082d2b..25a6d62 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -83,6 +83,9 @@  typedef enum {
 #define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
+#define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
+                                     INT_MAX >> BDRV_SECTOR_BITS)
+
 /*
  * Allocation status flags
  * BDRV_BLOCK_DATA: data is read from bs->file or another file