|
From: | Peter Lieven |
Subject: | Re: [Qemu-devel] [PATCHv3 00/20] block: logical block provisioning enhancements |
Date: | Tue, 08 Oct 2013 11:12:57 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 |
On 08.10.2013 10:59, Stefan Hajnoczi wrote:
On Tue, Oct 8, 2013 at 10:01 AM, Peter Lieven <address@hidden> wrote:On 08.10.2013 09:02, Stefan Hajnoczi wrote:On Mon, Oct 7, 2013 at 11:42 AM, Paolo Bonzini <address@hidden> 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
[Prev in Thread] | Current Thread | [Next in Thread] |