| Message ID | 1385546829-3839-9-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 27, 2013 at 11:07:08AM +0100, Peter Lieven wrote: > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > qemu-img.c | 4 ++-- > qemu-img.texi | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index 9bb1f6f..8b5f3da 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -100,7 +100,7 @@ static void help(void) > " '-h' with or without a command shows this help and lists the supported formats\n" > " '-p' show progress of command (only certain commands)\n" > " '-q' use Quiet mode - do not print any output (except errors)\n" > - " '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n" > + " '-S' indicates the consecutive number of bytes (defaults to 64k) that must\n" > " contain only zeros for qemu-img to create a sparse image during\n" > " conversion. If the number of bytes is 0, the source will not be scanned for\n" > " unallocated or zero sectors, and the destination image will always be\n" > @@ -1141,7 +1141,7 @@ static int img_convert(int argc, char **argv) > QEMUOptionParameter *out_baseimg_param; > char *options = NULL; > const char *snapshot_name = NULL; > - int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ > + int min_sparse = 128; /* Need at least 64k of zeros for sparse detection */ > bool quiet = false; > Error *local_err = NULL; I guess a sane size would be cluster size. For a raw file 4 KB is reasonable since that's the file system block size. Is it necessary to increase to 64 KB here?
Am 04.12.2013 17:43, schrieb Stefan Hajnoczi: > On Wed, Nov 27, 2013 at 11:07:08AM +0100, Peter Lieven wrote: >> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> qemu-img.c | 4 ++-- >> qemu-img.texi | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 9bb1f6f..8b5f3da 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -100,7 +100,7 @@ static void help(void) >> " '-h' with or without a command shows this help and lists the supported formats\n" >> " '-p' show progress of command (only certain commands)\n" >> " '-q' use Quiet mode - do not print any output (except errors)\n" >> - " '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n" >> + " '-S' indicates the consecutive number of bytes (defaults to 64k) that must\n" >> " contain only zeros for qemu-img to create a sparse image during\n" >> " conversion. If the number of bytes is 0, the source will not be scanned for\n" >> " unallocated or zero sectors, and the destination image will always be\n" >> @@ -1141,7 +1141,7 @@ static int img_convert(int argc, char **argv) >> QEMUOptionParameter *out_baseimg_param; >> char *options = NULL; >> const char *snapshot_name = NULL; >> - int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ >> + int min_sparse = 128; /* Need at least 64k of zeros for sparse detection */ >> bool quiet = false; >> Error *local_err = NULL; > I guess a sane size would be cluster size. For a raw file 4 KB is > reasonable since that's the file system block size. in case of iscsi the cluster size could be much too high as for example my storage has a cluster_size of 15MB. > > Is it necessary to increase to 64 KB here? No, its indepent of the rest. Paolo suggested to increase it and I can confirm that for my usage case its faster than 4K. I would drop this patch for now. Peter
On 12/04/2013 09:46 AM, Peter Lieven wrote: >> I guess a sane size would be cluster size. For a raw file 4 KB is >> reasonable since that's the file system block size. > in case of iscsi the cluster size could be much too high as for example > my storage has a cluster_size of 15MB. >> >> Is it necessary to increase to 64 KB here? > No, its indepent of the rest. Paolo suggested to increase it and I can confirm > that for my usage case its faster than 4K. At least on NTFS file systems, 64k is the minimum size of a hole in a sparse file. While many file systems support smaller holes, there are definitely systems where trying to detect smaller holes only results in wasted efforts. Is it worth making the default dynamic based on stat() information regarding optimum IO size for the given destination file system?
> Am 05.12.2013 um 03:12 schrieb Eric Blake <eblake@redhat.com>: > > On 12/04/2013 09:46 AM, Peter Lieven wrote: > >>> I guess a sane size would be cluster size. For a raw file 4 KB is >>> reasonable since that's the file system block size. >> in case of iscsi the cluster size could be much too high as for example >> my storage has a cluster_size of 15MB. >>> >>> Is it necessary to increase to 64 KB here? >> No, its indepent of the rest. Paolo suggested to increase it and I can confirm >> that for my usage case its faster than 4K. > > At least on NTFS file systems, 64k is the minimum size of a hole in a > sparse file. While many file systems support smaller holes, there are > definitely systems where trying to detect smaller holes only results in > wasted efforts. Is it worth making the default dynamic based on stat() > information regarding optimum IO size for the given destination file system? it is definetely worth it, but i would require additional work and testing. the current code does not create holes that are aligned to min_sparse and min_sparse has to be limited to a reasonable size. and i wonder if the right value is bs->bl.opt_transfer_lenght, bs->bl.discard_alignment or bdi->cluster_size/9. maybe depepnding on if its a cow Image or not. i can look at this. but i would leave the patch out for now. Peter > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
On Thu, Dec 05, 2013 at 05:55:22AM +0100, Peter Lieven wrote: > > > > Am 05.12.2013 um 03:12 schrieb Eric Blake <eblake@redhat.com>: > > > > On 12/04/2013 09:46 AM, Peter Lieven wrote: > > > >>> I guess a sane size would be cluster size. For a raw file 4 KB is > >>> reasonable since that's the file system block size. > >> in case of iscsi the cluster size could be much too high as for example > >> my storage has a cluster_size of 15MB. > >>> > >>> Is it necessary to increase to 64 KB here? > >> No, its indepent of the rest. Paolo suggested to increase it and I can confirm > >> that for my usage case its faster than 4K. > > > > At least on NTFS file systems, 64k is the minimum size of a hole in a > > sparse file. While many file systems support smaller holes, there are > > definitely systems where trying to detect smaller holes only results in > > wasted efforts. Is it worth making the default dynamic based on stat() > > information regarding optimum IO size for the given destination file system? > > it is definetely worth it, but i would require additional work and testing. the current code does not create holes that are aligned to min_sparse and min_sparse has to be limited to a reasonable size. and i wonder if the right value is bs->bl.opt_transfer_lenght, bs->bl.discard_alignment or bdi->cluster_size/9. maybe depepnding on if its a cow Image or not. > > i can look at this. but i would leave the patch out for now. Okay, I'll drop this patch for now. It can be improved in a separate series. Stefan
diff --git a/qemu-img.c b/qemu-img.c index 9bb1f6f..8b5f3da 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -100,7 +100,7 @@ static void help(void) " '-h' with or without a command shows this help and lists the supported formats\n" " '-p' show progress of command (only certain commands)\n" " '-q' use Quiet mode - do not print any output (except errors)\n" - " '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n" + " '-S' indicates the consecutive number of bytes (defaults to 64k) that must\n" " contain only zeros for qemu-img to create a sparse image during\n" " conversion. If the number of bytes is 0, the source will not be scanned for\n" " unallocated or zero sectors, and the destination image will always be\n" @@ -1141,7 +1141,7 @@ static int img_convert(int argc, char **argv) QEMUOptionParameter *out_baseimg_param; char *options = NULL; const char *snapshot_name = NULL; - int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ + int min_sparse = 128; /* Need at least 64k of zeros for sparse detection */ bool quiet = false; Error *local_err = NULL; diff --git a/qemu-img.texi b/qemu-img.texi index da36975..4add6ce 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -193,7 +193,7 @@ Image conversion is also useful to get smaller image when using a growable format such as @code{qcow} or @code{cow}: the empty sectors are detected and suppressed from the destination image. -@var{sparse_size} indicates the consecutive number of bytes (defaults to 4k) +@var{sparse_size} indicates the consecutive number of bytes (defaults to 64k) that must contain only zeros for qemu-img to create a sparse image during conversion. If @var{sparse_size} is 0, the source will not be scanned for unallocated or zero sectors, and the destination image will always be