From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38949) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzOx9-0004R0-6A for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:27:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzOx8-0006Wl-0m for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:27:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35905) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzOx7-0006Wc-MU for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:27:21 -0400 Date: Wed, 17 Jul 2013 12:27:15 +0200 From: Kevin Wolf Message-ID: <20130717102714.GG2458@dhcp-200-207.str.redhat.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <74B76DD7-FBF6-42CD-8B9D-62661B98A860@kamp.de> 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: Peter Lieven Cc: Paolo Bonzini , ronniesahlberg@gmail.com, qemu-devel@nongnu.org, stefanha@redhat.com 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. Kevin