From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50184) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzSZh-0002UA-SY for qemu-devel@nongnu.org; Wed, 17 Jul 2013 10:19:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzSZb-0007Vk-HL for qemu-devel@nongnu.org; Wed, 17 Jul 2013 10:19:25 -0400 Received: from ssl.dlhnet.de ([91.198.192.8]:49205 helo=ssl.dlh.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzSZb-0007Vb-3C for qemu-devel@nongnu.org; Wed, 17 Jul 2013 10:19:19 -0400 References: <1373885375-13601-1-git-send-email-pl@kamp.de> <1373885375-13601-5-git-send-email-pl@kamp.de> <20130717084648.GD2458@dhcp-200-207.str.redhat.com> <51E66ACD.70706@redhat.com> <74B76DD7-FBF6-42CD-8B9D-62661B98A860@kamp.de> <20130717102714.GG2458@dhcp-200-207.str.redhat.com> <51E6765E.3050205@redhat.com> Mime-Version: 1.0 (1.0) In-Reply-To: <51E6765E.3050205@redhat.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Message-Id: <78530989-AC55-4071-95F1-A62B09181012@kamp.de> From: Peter Lieven Date: Wed, 17 Jul 2013 16:19:19 +0200 Subject: Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , "ronniesahlberg@gmail.com" , "qemu-devel@nongnu.org" , "stefanha@redhat.com" Am 17.07.2013 um 12:47 schrieb Paolo Bonzini : > Il 17/07/2013 12:27, Kevin Wolf ha scritto: >> Am 17.07.2013 um 12:21 hat Peter Lieven geschrieben: >>>=20 >>> Am 17.07.2013 um 11:58 schrieb Paolo Bonzini : >>>=20 >>>> Il 17/07/2013 10:46, Kevin Wolf ha scritto: >>>>> Am 15.07.2013 um 12:49 hat Peter Lieven geschrieben: >>>>>> if a destination has has_zero_init =3D 0, but it supports >>>>>> discard zeroes use discard to convert the target >>>>>> into an all zero device. >>>>>>=20 >>>>>> Signed-off-by: Peter Lieven >>>>>=20 >>>>> Wouldn't it be better to use bdrv_write_zeroes() and extend the >>>>> implementation of that to use discard internally in those block driver= s >>>>> where it makes sense? >>>>>=20 >>>>> Because here you're not really discarding (i.e. don't care about the >>>>> sectors any more), but you want them to be zeroed. >>>>=20 >>>> I thought the same yesterday when reviewing the series, but I'm not >>>> convinced. >>>>=20 >>>> Discarding is not always the right way to write zeroes, because it can >>>> disrupt performance. It may be fine when you are already going to writ= e >>>> a sparse image (as is the case for qemu-img convert), but not in >>>> general. So if you just used write_zeroes, it would have to fall under= >>>> yet another -drive option (or an extension to "-drive discard"). I >>>> think what Peter did is a good compromise in the end. >>>>=20 >>>> BTW, Peter and Ronnie: we were assuming that UNMAP with LBPRZ=3D1 alway= s >>>> zeroes blocks, but is that true for unaligned operations? >>>=20 >>> Good question, I will pass it to ronnie. My guess is that the command wi= ll fail with >>> a check condition if it failed to unmap the data. =46rom what Ronnie sen= t earlier >>> it should be guaranteed that the blocks are at least zero after the unma= p command. >>>=20 >>> As for the qemu-img patch this shouldn't matter. It uses always blocks o= f bdi->max_unmap >>> which should be a multiple of the alignment. It also checks if sectors a= re deallocated >>> after the unmap afterwards. If the unmap fails it falls back to has_zero= _init =3D1. >>=20 >> Well, you use bdrv_discard(), and ignoring discards is valid. Just >> another reason to use bdrv_write_zeroes() instead. >=20 > He's only using it if discard_zeroes is true in the new BlockDriverInfo. > We can define the semantics of that bit, and I think defining it as > "ignored discards will still write zeroes" is a good thing (same as what > SCSI targets do if you use WRITE SAME to do the discard operation). Good idea >=20 > Paolo