From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTTLm-0001ig-OP for qemu-devel@nongnu.org; Tue, 08 Oct 2013 05:13:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VTTLg-00038G-FZ for qemu-devel@nongnu.org; Tue, 08 Oct 2013 05:13:06 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:40447 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTTLg-00037t-40 for qemu-devel@nongnu.org; Tue, 08 Oct 2013 05:13:00 -0400 Message-ID: <5253CC99.6000906@kamp.de> Date: Tue, 08 Oct 2013 11:12:57 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1380029714-5239-1-git-send-email-pl@kamp.de> <20131007084259.GF6254@stefanha-thinkpad.redhat.com> <525281F6.5070709@redhat.com> <5253BBC3.3050203@kamp.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv3 00/20] block: logical block provisioning enhancements List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel , Anthony Liguori , Paolo Bonzini , ronnie sahlberg On 08.10.2013 10:59, Stefan Hajnoczi wrote: > On Tue, Oct 8, 2013 at 10:01 AM, Peter Lieven wrote: >> On 08.10.2013 09:02, Stefan Hajnoczi wrote: >>> On Mon, Oct 7, 2013 at 11:42 AM, Paolo Bonzini >>> wrote: >>>> Il 07/10/2013 10:42, Stefan Hajnoczi ha scritto: >>>>> Could you make bdrv_co_write_zeroes() always use UNMAP, if possible, and >>>>> avoid adding the new BDRV_REQ_MAY_UNMAP flag? While reading the first >>>>> few patches in this series I wondered why there is a need to expose >>>>> flags at all... >>>>> >>>>> Sometimes it is useful to distinguish between zeroing at the image >>>>> format level from discarding at the device level, but I don't think we >>>>> make use of that yet. I'd prefer to keep the interface simple for now >>>>> and add flags later, if necessary. >>>>> >>>>> Or maybe I just missed something ;) >>>> The flag is needed to implement the right semantics for the SCSI WRITE >>>> SAME command, which are: >>>> >>>> - if the UNMAP bit is off, always write the sectors (that's >>>> bdrv_aio_write_zeroes without BDRV_REQ_MAY_UNMAP if the payload is zero, >>>> otherwise it's emulated with bdrv_aio_writev) >>>> >>>> - if the target can "discard and write the specified payload", you can >>>> discard, else you must write the sectors with the correct payload >>>> (that's bdrv_aio_write_zeroes with BDRV_REQ_MAY_UNMAP). >>>> >>>> Contrast this with the UNMAP command, which does not make any guarantee >>>> on the content of the sectors after the command is completed (a few >>>> months ago we agreed that, even if you have discard_zeroes=true in the >>>> target, it is fine for UNMAP to do nothing). >>> Okay, then let's keep the patches to expose the flag. >> Okay, then I can keep those. >> >> Can you give a short hint if my approach with brdv_make_empty is what >> you want? I would like to not change the parameters, so use >> BDRV_REQ_MAY_UNMAP >> unconditionally. >> >> int bdrv_make_empty(BlockDriverState *bs) > The semantics of bdrv_make_empty() today are: deallocate all data in > the top layer of the image file. If there is a backing file, reads > will fall back to the backing file. > > The semantics that you want are zeroing the entire disk image > (efficiently, when possible). > > A flags argument is needed to support both of sets of semantics. If > you don't like that, then I suggest creating a new function called > bdrv_make_zero(). Ok, that is what I would like to do. In this case I only have to rename bdrv_zeroize to bdrv_make_zero. Ok ? Peter