Am 17.07.2013 um 12:47 schrieb Paolo Bonzini <pbonz...@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 <pbonz...@redhat.com>: >>> >>>> 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 <p...@kamp.de> >>>>> >>>>> 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). Good idea > > Paolo
