From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkBTm-0006Lx-2H for qemu-devel@nongnu.org; Tue, 13 May 2014 08:06:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkBTg-0007G6-Av for qemu-devel@nongnu.org; Tue, 13 May 2014 08:06:42 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:38126 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkBTg-0007Fy-16 for qemu-devel@nongnu.org; Tue, 13 May 2014 08:06:36 -0400 Message-ID: <53720AB9.4040508@kamp.de> Date: Tue, 13 May 2014 14:06:17 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1399566139-24140-1-git-send-email-pl@kamp.de> <53712EFC.5080108@redhat.com> In-Reply-To: <53712EFC.5080108@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv4] block: optimize zero writes with bdrv_write_zeroes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, famz@redhat.com, stefanha@redhat.com, mreitz@redhat.com On 12.05.2014 22:28, Eric Blake wrote: > On 05/08/2014 10:22 AM, Peter Lieven wrote: >> this patch tries to optimize zero write requests >> by automatically using bdrv_write_zeroes if it is >> supported by the format. >> >> This significantly speeds up file system initialization and >> should speed zero write test used to test backend storage >> performance. >> >> Signed-off-by: Peter Lieven >> --- >> v3->v4: - use QAPI generated enum and lookup table [Kevin] >> - added more details about the options in the comments >> of the qapi-schema [Eric] >> - changed the type of detect_zeroes from str to >> BlockdevDetectZeroesOptions. I left the name >> as is because it is consistent with e.g. >> BlockdevDiscardOptions or BlockdevAioOptions [Eric] >> - changed the parse function in blockdev_init to >> be generic usable for other enum parameters > If you wouldn't mind, I think the generic function is useful enough that > people might want to backport it independently from this optimization. > It would be better to split this into a two-patch series, one for the > new parse_enum_option, the other for bdrv_write_zeroes utilizing it. > > >> + },{ >> + .name = "detect-zeroes", >> + .type = QEMU_OPT_STRING, >> + .help = "try to optimize zero writes", > Might be worth listing (off, on, unmap) in the text. > > Everything else looked okay, but I'll wait for R-b until I see a > response about the idea of splitting the patch (even if that response is > justification for keeping it as one) > I did not split because currently there is no other possible user in the function. The on_error settings and discard settings would be possible users, but for on_error there is a hardcoded difference between read and write which is not reflected in the qapi and for discard settings we have ignore and unmap, but we have also off and on which are not in qapi as well. Peter