[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V5] qemu-img: align result of is_allocated_secto
Re: [Qemu-devel] [PATCH V5] qemu-img: align result of is_allocated_sectors
Thu, 12 Jul 2018 11:26:25 +0200
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0
Am 11.07.2018 um 10:25 schrieb Kevin Wolf:
Am 10.07.2018 um 22:16 hat Peter Lieven geschrieben:
Am 10.07.2018 um 17:31 schrieb Kevin Wolf <address@hidden>:
Am 10.07.2018 um 17:05 hat Peter Lieven geschrieben:
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 have a 4k alignment internally.
This patch modifies is_allocated_sectors so that its *pnum result will always
end at an alignment boundary. This way all requests will end at an alignment
boundary. The start of all requests will also be aligned as long as the results
of get_block_status do not lead to an unaligned offset.
The number of RMW cycles when converting an example image  to a raw device
has 4k sector size is about 4600 4k read requests to perform a total of about
write requests. With this path the additional 4600 read requests are eliminated
the number of total write requests stays constant.
Signed-off-by: Peter Lieven <address@hidden>
It looked convincing, but I'm afraid this is still not correct.
qemu-iotests 122 fails for me with this patch.
I will have a look, where and why exactly it fails, but the allocation
pattern might be slightly different due to the alignment. What counts
is that the output is byte identical or not?
Right, I noticed only after sending this email that it's qemu-img map
output that changes and this might actually be okay. I didn't check,
however, if the exact changes are what is expected and whether we need
to add more test cases to cover what the test originally wanted to
So after all, there's a good chance that all that's missing is just an
update to the test case.
I checked the output of test 122 and what happens is exactly what is expected.
The zero areas align to 4k or 8k respectively. If they don't align they are
as allocated. I would just go ahead and include the test update and send V6 if
are no objections.
If someone feels that the behaviour is undesired we can also just go ahead
with a light version of the patch and use only bs->request_alignment as target
alignment and ignore the value of min_sparse.
In this case we could even think about as treating this as a bug fix as it
avoids these ugly RMW cycles that we were seeing and this is why we created this
Mit freundlichen Grüßen
KAMP Netzwerkdienste GmbH
Vestische Str. 89-91 | 46117 Oberhausen
Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
address@hidden | http://www.kamp.de
Geschäftsführer: Heiner Lante | Michael Lante
Amtsgericht Duisburg | HRB Nr. 12154
USt-Id-Nr.: DE 120607556