Am 17.07.2013 um 11:58 hat Paolo Bonzini geschrieben: > 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").
Maybe we need a flag to bdrv_write_zeroes() that specifies whether to use discard or not, kind of like the unmap bit in WRITE SAME. On the other hand, I think you're right that this is really policy, and as such shouldn't be hardcoded based on our guesses, but be configurable. > I think what Peter did is a good compromise in the end. At the very least it must become a separate function. img_convert() is already too big and too deeply nested. > BTW, Peter and Ronnie: we were assuming that UNMAP with LBPRZ=1 always > zeroes blocks, but is that true for unaligned operations? SBC-3 Rev. 35e, 5.16 READ CAPACITY (16) command: "A logical block provisioning read zeros (LBPRZ) bit set to one indicates that, for read commands specifying an unmapped LBA (see 4.7.4.5), the device server returns user data set to zero [...]" So it depends on the block provisioning state of the LBA, not on the operations that were performed on it. 5.28 UNMAP command: If the ANCHOR bit in the CDB is set to zero, and the logical unit is thin provisioned (see 4.7.3.3), then the logical block provisioning state for each specified LBA: a) should become deallocated; b) may become anchored; or c) may remain unchanged. So with UNMAP, I think you don't have any guarantees that the LBA becomes unmapped and therefore zeroed. It could just keep its current data. No matter whether your request was aligned or not. Kevin