Re: [Qemu-devel] [PATCH] qemu-img: align is_allocated_sectors to 4k

[Max Reitz]

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 <address@hidden>
> ---
>  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