On 17.11.2016 21:14, Eric Blake wrote: > Make it easier to simulate the Dell Equallogic iSCSI with its
Somehow I feel bad putting company and product names into commit messages...
> unusual 15M preferred and maximum unmap and write zero sizing,
> or to simulate Linux loopback block devices enforcing a small
> max_transfer of 64k, by allowing blkdebug to wrap any other
> device with further restrictions on various alignments.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> qapi/block-core.json | 27 ++++++++++++++-
> block/blkdebug.c | 97
> ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 121 insertions(+), 3 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index c29bef7..26f3e9f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2068,6 +2068,29 @@
> # @align: #optional required alignment for requests in bytes,
> # must be power of 2, or 0 for default
> #
> +# @max-transfer: #optional maximum size for I/O transfers in bytes,
> +# must be multiple of the larger of @align and and 512
> +# (but need not be a power of 2), or 0 for default
> +# (since 2.9)
> +#
> +# @opt-write-zero: #optional preferred alignment for write zero requests
> +# in bytes, must be multiple of the larger of @align
> +# and 512 (but need not be a power of 2), or 0 for
> +# default (since 2.9)
> +#
> +# @max-write-zero: #optional maximum size for write zero requests in bytes,
> +# must be multiple of the larger of @align and 512 (but
> +# need not be a power of 2), or 0 for default (since 2.9)
> +#
> +# @opt-discard: #optional preferred alignment for discard requests
> +# in bytes, must be multiple of the larger of @align
> +# and 512 (but need not be a power of 2), or 0 for
> +# default (since 2.9)
> +#
> +# @max-discard: #optional maximum size for discard requests in bytes,
> +# must be multiple of the larger of @align and 512 (but
> +# need not be a power of 2), or 0 for default (since 2.9)
> +#
> # @inject-error: #optional array of error injection descriptions
> #
> # @set-state: #optional array of state-change descriptions
> @@ -2077,7 +2100,9 @@
> { 'struct': 'BlockdevOptionsBlkdebug',
> 'data': { 'image': 'BlockdevRef',
> '*config': 'str',
> - '*align': 'int',
> + '*align': 'int', '*max-transfer': 'int32',
> + '*opt-write-zero': 'int32', '*max-write-zero': 'int32',
> + '*opt-discard': 'int32', '*max-discard': 'int32',
> '*inject-error': ['BlkdebugInjectErrorOptions'],
> '*set-state': ['BlkdebugSetStateOptions'] } }
>
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index d45826d..3ba7a96 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -39,6 +39,11 @@ typedef struct BDRVBlkdebugState {
> int state;
> int new_state;
> int align;
> + int max_transfer;
> + int opt_write_zero;
> + int max_write_zero;
> + int opt_discard;
> + int max_discard;
>
> /* For blkdebug_refresh_filename() */
> char *config_file;
> @@ -344,6 +349,31 @@ static QemuOptsList runtime_opts = {
> .type = QEMU_OPT_SIZE,
> .help = "Required alignment in bytes",
> },
> + {
> + .name = "max-transfer",
> + .type = QEMU_OPT_SIZE,
> + .help = "Maximum transfer size in bytes",
> + },
> + {
> + .name = "opt-write-zero",
> + .type = QEMU_OPT_SIZE,
> + .help = "Optimum write zero size in bytes",
s/size/alignment/?
> + },
> + {
> + .name = "max-write-zero",
> + .type = QEMU_OPT_SIZE,
> + .help = "Maximum write zero size in bytes",
> + },
> + {
> + .name = "opt-discard",
> + .type = QEMU_OPT_SIZE,
> + .help = "Optimum discard size in bytes",
s/size/alignment/?
> + },
> + {
> + .name = "max-discard",
> + .type = QEMU_OPT_SIZE,
> + .help = "Maximum discard size in bytes",
> + },
> { /* end of list */ }
> },
> };
> @@ -354,7 +384,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict
> *options, int flags,
> BDRVBlkdebugState *s = bs->opaque;
> QemuOpts *opts;
> Error *local_err = NULL;
> - uint64_t align;
> + uint64_t align, max_transfer;
> + uint64_t opt_write_zero, max_write_zero, opt_discard, max_discard;
> int ret;
>
> opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> @@ -389,7 +420,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict
> *options, int flags,
> bs->supported_zero_flags = (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
> bs->file->bs->supported_zero_flags;
>
> - /* Set request alignment */
> + /* Set alignment overrides */
> align = qemu_opt_get_size(opts, "align", 0);
> if (align < INT_MAX && is_power_of_2(align)) {
> s->align = align;
> @@ -398,6 +429,53 @@ static int blkdebug_open(BlockDriverState *bs, QDict
> *options, int flags,
> ret = -EINVAL;
> goto fail_unref;
> }
> + max_transfer = qemu_opt_get_size(opts, "max-transfer", 0);
> + if (max_transfer < INT_MAX &&
> + QEMU_IS_ALIGNED(max_transfer, MAX(align, BDRV_SECTOR_SIZE))) {
> + s->max_transfer = max_transfer;
> + } else if (max_transfer) {
> + error_setg(errp, "Invalid argument");
Could you be more specific? Same in all of the error_setg() calls below.
> + ret = -EINVAL;
> + goto fail_unref;
> + }
Also, the way this is formatted seems not intuitive to me. I know it's
the same as it's been done for "align", but normally I'd use the following:
s->value = qemu_opt_get_size(...);
if (s->value is set and invalid) {
/* error out */
}
Instead of:
value = qemu_opt_get_size(...);
if (value is valid) {
s->value = value;
} else if (value is set) {
/* error out */
}
Same for all blocks below.
Finally, my eyes would be really grateful for some empty lines here and
there, at least one between the handling of different options.
Max
> + opt_write_zero = qemu_opt_get_size(opts, "opt-write-zero", 0);
> + if (opt_write_zero < INT_MAX &&
> + QEMU_IS_ALIGNED(opt_write_zero, MAX(align, BDRV_SECTOR_SIZE))) {
> + s->opt_write_zero = opt_write_zero;
> + } else if (opt_write_zero) {
> + error_setg(errp, "Invalid argument");
> + ret = -EINVAL;
> + goto fail_unref;
> + }
> + max_write_zero = qemu_opt_get_size(opts, "max-write-zero", 0);
> + if (max_write_zero < INT_MAX &&
> + QEMU_IS_ALIGNED(max_write_zero, MAX(opt_write_zero,
> + MAX(align, BDRV_SECTOR_SIZE)))) {
> + s->max_write_zero = max_write_zero;
> + } else if (max_write_zero) {
> + error_setg(errp, "Invalid argument");
> + ret = -EINVAL;
> + goto fail_unref;
> + }
> + opt_discard = qemu_opt_get_size(opts, "opt-discard", 0);
> + if (opt_discard < INT_MAX &&
> + QEMU_IS_ALIGNED(opt_discard, MAX(align, BDRV_SECTOR_SIZE))) {
> + s->opt_discard = opt_discard;
> + } else if (opt_discard) {
> + error_setg(errp, "Invalid argument");
> + ret = -EINVAL;
> + goto fail_unref;
> + }
> + max_discard = qemu_opt_get_size(opts, "max-discard", 0);
> + if (max_discard < INT_MAX &&
> + QEMU_IS_ALIGNED(max_discard, MAX(opt_discard,
> + MAX(align, BDRV_SECTOR_SIZE)))) {
> + s->max_discard = max_discard;
> + } else if (max_discard) {
> + error_setg(errp, "Invalid argument");
> + ret = -EINVAL;
> + goto fail_unref;
> + }
>
> ret = 0;
> goto out;
> @@ -807,6 +885,21 @@ static void blkdebug_refresh_limits(BlockDriverState
> *bs, Error **errp)
> if (s->align) {
> bs->bl.request_alignment = s->align;
> }
> + if (s->max_transfer) {
> + bs->bl.max_transfer = s->max_transfer;
> + }
> + if (s->opt_write_zero) {
> + bs->bl.pwrite_zeroes_alignment = s->opt_write_zero;
> + }
> + if (s->max_write_zero) {
> + bs->bl.max_pwrite_zeroes = s->max_write_zero;
> + }
> + if (s->opt_discard) {
> + bs->bl.pdiscard_alignment = s->opt_discard;
> + }
> + if (s->max_discard) {
> + bs->bl.max_pdiscard = s->max_discard;
> + }
> }
>
> static int blkdebug_reopen_prepare(BDRVReopenState *reopen_state,
>
signature.asc
Description: OpenPGP digital signature
