[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v4 2/8] qmp: Add optional bool "unmap" to drive-
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v4 2/8] qmp: Add optional bool "unmap" to drive-mirror |
Date: |
Fri, 22 May 2015 13:57:49 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 05/21/2015 09:40 PM, Fam Zheng wrote:
> If specified as "true", it allows discarding on target sectors where source is
> not allocated.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> block/mirror.c | 7 +++++--
> blockdev.c | 5 +++++
> hmp.c | 2 +-
> include/block/block_int.h | 2 ++
> qapi/block-core.json | 5 ++++-
> qmp-commands.hx | 3 +++
> 6 files changed, 20 insertions(+), 4 deletions(-)
>
> +++ b/blockdev.c
> @@ -2632,6 +2632,7 @@ void qmp_drive_mirror(const char *device, const char
> *target,
> bool has_buf_size, int64_t buf_size,
> bool has_on_source_error, BlockdevOnError
> on_source_error,
> bool has_on_target_error, BlockdevOnError
> on_target_error,
> + bool has_unmap, bool unmap,
> Error **errp)
> {
> BlockBackend *blk;
> @@ -2663,6 +2664,9 @@ void qmp_drive_mirror(const char *device, const char
> *target,
> if (!has_buf_size) {
> buf_size = DEFAULT_MIRROR_BUF_SIZE;
> }
> + if (!has_unmap) {
> + unmap = true;
> + }
So drive-mirror defaults to true...
> +++ b/hmp.c
> @@ -1056,7 +1056,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
> false, NULL, false, NULL,
> full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> true, mode, false, 0, false, 0, false, 0,
> - false, 0, false, 0, &err);
> + false, 0, false, 0, false, false, &err);
...even though you are passing unmap=false here. It might be a bit less
confusing to pass unmap=true, even though the has_unmap=false means to
fall back to the default, so that someone reading this code doesn't get
confused about the unmap choice being changed by the defaulting mechanism.
> hmp_handle_error(mon, &err);
> }
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index db29b74..d136d34 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -587,6 +587,7 @@ void commit_active_start(BlockDriverState *bs,
> BlockDriverState *base,
> * @mode: Whether to collapse all images in the chain to the target.
> * @on_source_error: The action to take upon error reading from the source.
> * @on_target_error: The action to take upon error writing to the target.
> + * @unmap: Whether unmap target where source sectors only contain zeroes.
s/Whether/Whether to/
> +++ b/qapi/block-core.json
> @@ -954,6 +954,8 @@
> # @on-target-error: #optional the action to take on an error on the target,
> # default 'report' (no limitations, since this applies to
> # a different block device than @device).
> +# @unmap: #optional Whether unmap target sectors where source has zero.
> Default
s/Whether/Whether to/
> +# is true. (Since 2.4)
That says what it does, but not why I would want to ever specify false.
Is it worth a bit more explanation?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v4 0/8] block: Mirror discarded sectors, Fam Zheng, 2015/05/21
- [Qemu-block] [PATCH v4 1/8] block: Add "base" option to bdrv_get_block_status, Fam Zheng, 2015/05/21
- [Qemu-block] [PATCH v4 2/8] qmp: Add optional bool "unmap" to drive-mirror, Fam Zheng, 2015/05/21
- Re: [Qemu-block] [PATCH v4 2/8] qmp: Add optional bool "unmap" to drive-mirror,
Eric Blake <=
- [Qemu-block] [PATCH v4 3/8] mirror: Do zero write on target if sectors not allocated, Fam Zheng, 2015/05/21
- [Qemu-block] [PATCH v4 4/8] block: Fix dirty bitmap in bdrv_co_discard, Fam Zheng, 2015/05/21
- [Qemu-block] [PATCH v4 5/8] block: Remove bdrv_reset_dirty, Fam Zheng, 2015/05/21
- [Qemu-block] [PATCH v4 6/8] qemu-iotests: Make block job methods common, Fam Zheng, 2015/05/21
- [Qemu-block] [PATCH v4 7/8] qemu-iotests: Add test case for mirror with unmap, Fam Zheng, 2015/05/21