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 <f...@redhat.com> > --- > 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
