diff mbox

[v3,09/22] blkdebug: Set request_alignment during .bdrv_refresh_limits()

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

Commit Message

Eric Blake June 23, 2016, 10:37 p.m. UTC
We want to eventually stick request_alignment alongside other
BlockLimits, but first, we must ensure it is populated at the
same time as all other limits, rather than being a special case
that is set only when a block is first opened.

Note that when the user does not provide "align", then we were
defaulting to bs->request_alignment - but at this stage in the
initialization, that was always 512.  We were also rejecting an
explicit "align":0 from the user; this patch now allows that,
as an explicit request for the default alignment (which may not
always be 512 in the future).

qemu-iotests 77 is particularly sensitive to the fact that we
can specify an artificial alignment override in blkdebug, and
that override must continue to work even when limits are
refreshed on an already open device.

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

---
v3: rework to allow user to specify "align":0 for default
v2: new patch
---
 qapi/block-core.json |  3 ++-
 block/blkdebug.c     | 19 +++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

Comments

Fam Zheng June 24, 2016, 5:42 a.m. UTC | #1
On Thu, 06/23 16:37, Eric Blake wrote:
> We want to eventually stick request_alignment alongside other
> BlockLimits, but first, we must ensure it is populated at the
> same time as all other limits, rather than being a special case
> that is set only when a block is first opened.
> 
> Note that when the user does not provide "align", then we were
> defaulting to bs->request_alignment - but at this stage in the
> initialization, that was always 512.  We were also rejecting an
> explicit "align":0 from the user; this patch now allows that,
> as an explicit request for the default alignment (which may not
> always be 512 in the future).
> 
> qemu-iotests 77 is particularly sensitive to the fact that we
> can specify an artificial alignment override in blkdebug, and
> that override must continue to work even when limits are
> refreshed on an already open device.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v3: rework to allow user to specify "align":0 for default
> v2: new patch
> ---
>  qapi/block-core.json |  3 ++-
>  block/blkdebug.c     | 19 +++++++++++++++----
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 98a20d2..ac8f5f6 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1961,7 +1961,8 @@
>  #
>  # @config:          #optional filename of the configuration file
>  #
> -# @align:           #optional required alignment for requests in bytes
> +# @align:           #optional required alignment for requests in bytes,
> +#                   must be power of 2, or 0 for default
>  #
>  # @inject-error:    #optional array of error injection descriptions
>  #
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 20d25bd..54b6870 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -37,6 +37,7 @@
>  typedef struct BDRVBlkdebugState {
>      int state;
>      int new_state;
> +    int align;
> 
>      QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX];
>      QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
> @@ -382,10 +383,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
>      }
> 
>      /* Set request alignment */
> -    align = qemu_opt_get_size(opts, "align", bs->request_alignment);
> -    if (align > 0 && align < INT_MAX && !(align & (align - 1))) {
> -        bs->request_alignment = align;
> -    } else {
> +    align = qemu_opt_get_size(opts, "align", 0);
> +    if (align < INT_MAX && is_power_of_2(align)) {
> +        s->align = align;
> +    } else if (align) {
>          error_setg(errp, "Invalid alignment");
>          ret = -EINVAL;
>          goto fail_unref;
> @@ -720,6 +721,15 @@ static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
>      bs->full_open_options = opts;
>  }
> 
> +static void blkdebug_refresh_limits(BlockDriverState *bs, Error **errp)
> +{
> +    BDRVBlkdebugState *s = bs->opaque;
> +
> +    if (s->align) {
> +        bs->request_alignment = s->align;
> +    }
> +}
> +
>  static int blkdebug_reopen_prepare(BDRVReopenState *reopen_state,
>                                     BlockReopenQueue *queue, Error **errp)
>  {
> @@ -738,6 +748,7 @@ static BlockDriver bdrv_blkdebug = {
>      .bdrv_getlength         = blkdebug_getlength,
>      .bdrv_truncate          = blkdebug_truncate,
>      .bdrv_refresh_filename  = blkdebug_refresh_filename,
> +    .bdrv_refresh_limits    = blkdebug_refresh_limits,
> 
>      .bdrv_aio_readv         = blkdebug_aio_readv,
>      .bdrv_aio_writev        = blkdebug_aio_writev,
> -- 
> 2.5.5
> 

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

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 98a20d2..ac8f5f6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1961,7 +1961,8 @@ 
 #
 # @config:          #optional filename of the configuration file
 #
-# @align:           #optional required alignment for requests in bytes
+# @align:           #optional required alignment for requests in bytes,
+#                   must be power of 2, or 0 for default
 #
 # @inject-error:    #optional array of error injection descriptions
 #
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 20d25bd..54b6870 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -37,6 +37,7 @@ 
 typedef struct BDRVBlkdebugState {
     int state;
     int new_state;
+    int align;

     QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX];
     QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
@@ -382,10 +383,10 @@  static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
     }

     /* Set request alignment */
-    align = qemu_opt_get_size(opts, "align", bs->request_alignment);
-    if (align > 0 && align < INT_MAX && !(align & (align - 1))) {
-        bs->request_alignment = align;
-    } else {
+    align = qemu_opt_get_size(opts, "align", 0);
+    if (align < INT_MAX && is_power_of_2(align)) {
+        s->align = align;
+    } else if (align) {
         error_setg(errp, "Invalid alignment");
         ret = -EINVAL;
         goto fail_unref;
@@ -720,6 +721,15 @@  static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
     bs->full_open_options = opts;
 }

+static void blkdebug_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    BDRVBlkdebugState *s = bs->opaque;
+
+    if (s->align) {
+        bs->request_alignment = s->align;
+    }
+}
+
 static int blkdebug_reopen_prepare(BDRVReopenState *reopen_state,
                                    BlockReopenQueue *queue, Error **errp)
 {
@@ -738,6 +748,7 @@  static BlockDriver bdrv_blkdebug = {
     .bdrv_getlength         = blkdebug_getlength,
     .bdrv_truncate          = blkdebug_truncate,
     .bdrv_refresh_filename  = blkdebug_refresh_filename,
+    .bdrv_refresh_limits    = blkdebug_refresh_limits,

     .bdrv_aio_readv         = blkdebug_aio_readv,
     .bdrv_aio_writev        = blkdebug_aio_writev,