From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44454) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzPHW-0001TB-5y for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:48:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzPHV-0005q3-73 for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:48:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35676) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzPHU-0005px-UK for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:48:25 -0400 Message-ID: <51E6765E.3050205@redhat.com> Date: Wed, 17 Jul 2013 12:47:58 +0200 From: Paolo Bonzini MIME-Version: 1.0 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> In-Reply-To: <20130717102714.GG2458@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: Kevin Wolf Cc: ronniesahlberg@gmail.com, Peter Lieven , qemu-devel@nongnu.org, stefanha@redhat.com Il 17/07/2013 12:27, Kevin Wolf ha scritto: > Am 17.07.2013 um 12:21 hat Peter Lieven geschrieben: >> >> Am 17.07.2013 um 11:58 schrieb Paolo Bonzini : >> >>> 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 = 0, but it supports >>>>> discard zeroes use discard to convert the target >>>>> into an all zero device. >>>>> >>>>> Signed-off-by: Peter Lieven >>>> >>>> Wouldn't it be better to use bdrv_write_zeroes() and extend the >>>> implementation of that to use discard internally in those block drivers >>>> where it makes sense? >>>> >>>> Because here you're not really discarding (i.e. don't care about the >>>> sectors any more), but you want them to be zeroed. >>> >>> I thought the same yesterday when reviewing the series, but I'm not >>> convinced. >>> >>> 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 write >>> 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. >>> >>> BTW, Peter and Ronnie: we were assuming that UNMAP with LBPRZ=1 always >>> zeroes blocks, but is that true for unaligned operations? >> >> Good question, I will pass it to ronnie. My guess is that the command will fail with >> a check condition if it failed to unmap the data. From what Ronnie sent earlier >> it should be guaranteed that the blocks are at least zero after the unmap command. >> >> As for the qemu-img patch this shouldn't matter. It uses always blocks of bdi->max_unmap >> which should be a multiple of the alignment. It also checks if sectors are deallocated >> after the unmap afterwards. If the unmap fails it falls back to has_zero_init =1. > > Well, you use bdrv_discard(), and ignoring discards is valid. Just > another reason to use bdrv_write_zeroes() instead. 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). Paolo