From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47987) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yvt4u-0004FQ-VA for qemu-devel@nongnu.org; Fri, 22 May 2015 15:57:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yvt4t-0003aM-Kn for qemu-devel@nongnu.org; Fri, 22 May 2015 15:57:56 -0400 Message-ID: <555F8A3D.6090909@redhat.com> Date: Fri, 22 May 2015 13:57:49 -0600 From: Eric Blake MIME-Version: 1.0 References: <1432266060-22104-1-git-send-email-famz@redhat.com> <1432266060-22104-3-git-send-email-famz@redhat.com> In-Reply-To: <1432266060-22104-3-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SKIdPcPdkMrqeORGueRFNR1vRWPvsxhAP" Subject: Re: [Qemu-devel] [PATCH v4 2/8] qmp: Add optional bool "unmap" to drive-mirror List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , qemu-stable@nongnu.org, Stefan Hajnoczi , pbonzini@redhat.com, jsnow@redhat.com, wangxiaolong@ucloud.cn This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --SKIdPcPdkMrqeORGueRFNR1vRWPvsxhAP Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/21/2015 09:40 PM, Fam Zheng wrote: > If specified as "true", it allows discarding on target sectors where so= urce is > not allocated. >=20 > Signed-off-by: Fam Zheng > --- > 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(-) >=20 > +++ b/blockdev.c > @@ -2632,6 +2632,7 @@ void qmp_drive_mirror(const char *device, const c= har *target, > bool has_buf_size, int64_t buf_size, > bool has_on_source_error, BlockdevOnError on_sou= rce_error, > bool has_on_target_error, BlockdevOnError on_tar= get_error, > + bool has_unmap, bool unmap, > Error **errp) > { > BlockBackend *blk; > @@ -2663,6 +2664,9 @@ void qmp_drive_mirror(const char *device, const c= har *target, > if (!has_buf_size) { > buf_size =3D DEFAULT_MIRROR_BUF_SIZE; > } > + if (!has_unmap) { > + unmap =3D 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_T= OP, > true, mode, false, 0, false, 0, false, 0, > - false, 0, false, 0, &err); > + false, 0, false, 0, false, false, &err); =2E..even though you are passing unmap=3Dfalse here. It might be a bit l= ess confusing to pass unmap=3Dtrue, even though the has_unmap=3Dfalse means t= o 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= =2E > hmp_handle_error(mon, &err); > } > =20 > 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, Bloc= kDriverState *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 so= urce. > * @on_target_error: The action to take upon error writing to the targ= et. > + * @unmap: Whether unmap target where source sectors only contain zero= es. 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 ta= rget, > # default 'report' (no limitations, since this appli= es to > # a different block device than @device). > +# @unmap: #optional Whether unmap target sectors where source has zero= =2E 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? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --SKIdPcPdkMrqeORGueRFNR1vRWPvsxhAP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVX4o9AAoJEKeha0olJ0Nq7SUH/3bIAWqocey16tLg0Jm9siVy SklDCKPlOVo566goCHL54DWjjTAZQMp2oHYuXTDF9mcUY72INmBxnUEUuwbLaJ44 Ag3KlIG1Ct1vdBkRYubbM7gJC7SzDL5nMCvs5EaA7Le7WWkcC+lhs7CJfq+qPwam 1s/Jn5mhQyRBCcroI/T2Q6ymgMhv9ht1GItr8TCA2BAPwGhx3yOBHee+cVZBYp5M mlIehF8KPy9oc2kmZnkT8lupEL3r8l+++O+pkFv/hVTUrmiQadr1Eq4eQQywOACs HOpT6dZP79N9PWaYmHZjpjYIKQEQC6If1dXw8I0Pb7guiD1IhYcRLNPy22W5cGE= =L8DB -----END PGP SIGNATURE----- --SKIdPcPdkMrqeORGueRFNR1vRWPvsxhAP--