On 04/24/2017 08:50 PM, 858585 jemmy wrote: > On Mon, Apr 24, 2017 at 10:43 PM, Eric Blake wrote: >> On 04/23/2017 09:33 AM, jemmy858585@gmail.com wrote: >>> From: Lidong Chen >>> >>> is_allocated_sectors_min don't guarantee to contain the >>> consecutive number of zero bytes. this patch fixes this bug. >> >> This message was sent without an 'In-Reply-To' header pointing to a 0/2 >> cover letter. When sending a series, please always thread things to a >> cover letter; you may find 'git config format.coverletter auto' to be >> helpful. > > Thanks for your kind advises. > >> >> I seem to recall past attempts to try and patch this function, which >> were then turned down, although I haven't scrubbed the archives for a >> quick URL to point to. I'm worried that there are more subtleties here >> than what you realize. > > Hi Eric: > Do you mean this URL? > https://lists.gnu.org/archive/html/qemu-block/2017-01/msg00306.html Yes, that's probably the one. > > But I think the code is not consistent with qemu-img --help. > qemu-img --help > '-S' indicates the consecutive number of bytes (defaults to 4k) that must > contain only zeros for qemu-img to create a sparse image during > conversion. If the number of bytes is 0, the source will not be > scanned for > unallocated or zero sectors, and the destination image will always be > fully allocated. If you still think this patch is needed, the best way to convince me of it is accompany your patch by a qemu-iotests enhancement that covers the change in behavior (running the test pre-patch would show that we are broken without the patch, and having the test ensures we can't later regress). That's a lot more work than the vague two lines of the commit message body. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org