On 2018-06-07 14:46, Peter Lieven wrote: > We currently don't enforce that the sparse segments we detect during convert > are > aligned. This leads to unnecessary and costly read-modify-write cycles either > internally in Qemu or in the background on the storage device as nearly all > modern filesystems or hardware has a 4k alignment internally. > > As we per default set the min_sparse size to 4k it makes perfectly sense to > ensure > that these sparse holes in the file are placed at 4k boundaries. > > The number of RMW cycles when converting an example image [1] to a raw device > that > has 4k sector size is about 4600 4k read requests to perform a total of about > 15000 > write requests. With this path the 4600 additional read requests are > eliminated. > > [1] > https://cloud-images.ubuntu.com/releases/16.04/release/ubuntu-16.04-server-cloudimg-amd64-disk1.vmdk > > Signed-off-by: Peter Lieven <p...@kamp.de> > --- > qemu-img.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-)
I like the idea, but it doesn't seem guaranteed that is_allocated_sectors() is called on aligned offsets, so this alignment work may still leave things unaligned. Furthermore, we should probably not blindly assume 4k but instead use some block limit of the target, like pwrite_zeroes_alignment, or pdiscard_alignment, depending on the case. (Or probably still min_sparse, if that's less.) Since is_allocated_sectors_min() (the only caller of is_allocated_sectors()) is called from just a single place, taking those factors into account should be possible. Max
signature.asc
Description: OpenPGP digital signature
