diff mbox

[v3,19/22] block: Split bdrv_merge_limits() from bdrv_refresh_limits()

Message ID 1466721446-27737-20-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake June 23, 2016, 10:37 p.m. UTC
During bdrv_merge_limits(), we were computing initial limits
based on another BDS in two places.  At first glance, the two
computations are not identical (one is doing straight copying,
the other is doing merging towards or away from zero) - but
when you realize that the first round is starting with all-0
memory, all of the merging happens to work.  Factoring out the
merging makes it easier to track how two BDS limits are merged,
in case we have future reasons to merge in even more limits.

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

---
v3: Split raw block driver changes to its own patch, make new
function static
v2: new patch
---
 block/io.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

Comments

Fam Zheng June 24, 2016, 6:48 a.m. UTC | #1
On Thu, 06/23 16:37, Eric Blake wrote:
> During bdrv_merge_limits(), we were computing initial limits
> based on another BDS in two places.  At first glance, the two
> computations are not identical (one is doing straight copying,
> the other is doing merging towards or away from zero) - but
> when you realize that the first round is starting with all-0
> memory, all of the merging happens to work.  Factoring out the
> merging makes it easier to track how two BDS limits are merged,
> in case we have future reasons to merge in even more limits.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v3: Split raw block driver changes to its own patch, make new
> function static
> v2: new patch
> ---
>  block/io.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 0f15d05..69dbbd3 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -67,6 +67,17 @@ static void bdrv_parent_drained_end(BlockDriverState *bs)
>      }
>  }
> 
> +static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
> +{
> +    dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer);
> +    dst->max_transfer = MIN_NON_ZERO(dst->max_transfer, src->max_transfer);
> +    dst->opt_mem_alignment = MAX(dst->opt_mem_alignment,
> +                                 src->opt_mem_alignment);
> +    dst->min_mem_alignment = MAX(dst->min_mem_alignment,
> +                                 src->min_mem_alignment);
> +    dst->max_iov = MIN_NON_ZERO(dst->max_iov, src->max_iov);
> +}
> +
>  void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>  {
>      BlockDriver *drv = bs->drv;
> @@ -88,11 +99,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>              error_propagate(errp, local_err);
>              return;
>          }
> -        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;
> +        bdrv_merge_limits(&bs->bl, &bs->file->bs->bl);
>      } else {
>          bs->bl.min_mem_alignment = 512;
>          bs->bl.opt_mem_alignment = getpagesize();
> @@ -107,19 +114,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>              error_propagate(errp, local_err);
>              return;
>          }
> -        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);
> -        bs->bl.min_mem_alignment =
> -            MAX(bs->bl.min_mem_alignment,
> -                bs->backing->bs->bl.min_mem_alignment);
> -        bs->bl.max_iov =
> -            MIN(bs->bl.max_iov,
> -                bs->backing->bs->bl.max_iov);
> +        bdrv_merge_limits(&bs->bl, &bs->backing->bs->bl);
>      }
> 
>      /* Then let the driver override it */
> -- 
> 2.5.5
> 

Reviewed-by: Fam Zheng <famz@redhat.com>
diff mbox

Patch

diff --git a/block/io.c b/block/io.c
index 0f15d05..69dbbd3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -67,6 +67,17 @@  static void bdrv_parent_drained_end(BlockDriverState *bs)
     }
 }

+static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
+{
+    dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer);
+    dst->max_transfer = MIN_NON_ZERO(dst->max_transfer, src->max_transfer);
+    dst->opt_mem_alignment = MAX(dst->opt_mem_alignment,
+                                 src->opt_mem_alignment);
+    dst->min_mem_alignment = MAX(dst->min_mem_alignment,
+                                 src->min_mem_alignment);
+    dst->max_iov = MIN_NON_ZERO(dst->max_iov, src->max_iov);
+}
+
 void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     BlockDriver *drv = bs->drv;
@@ -88,11 +99,7 @@  void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
             error_propagate(errp, local_err);
             return;
         }
-        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;
+        bdrv_merge_limits(&bs->bl, &bs->file->bs->bl);
     } else {
         bs->bl.min_mem_alignment = 512;
         bs->bl.opt_mem_alignment = getpagesize();
@@ -107,19 +114,7 @@  void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
             error_propagate(errp, local_err);
             return;
         }
-        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);
-        bs->bl.min_mem_alignment =
-            MAX(bs->bl.min_mem_alignment,
-                bs->backing->bs->bl.min_mem_alignment);
-        bs->bl.max_iov =
-            MIN(bs->bl.max_iov,
-                bs->backing->bs->bl.max_iov);
+        bdrv_merge_limits(&bs->bl, &bs->backing->bs->bl);
     }

     /* Then let the driver override it */