From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43694) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vkxfc-0004l7-Hb for qemu-devel@nongnu.org; Mon, 25 Nov 2013 10:01:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VkxfW-0000rP-Ua for qemu-devel@nongnu.org; Mon, 25 Nov 2013 10:01:52 -0500 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:43931 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VkxfW-0000o5-ER for qemu-devel@nongnu.org; Mon, 25 Nov 2013 10:01:46 -0500 Message-ID: <52936665.3050302@kamp.de> Date: Mon, 25 Nov 2013 16:01:57 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1385387840-17307-1-git-send-email-pl@kamp.de> <1385387840-17307-4-git-send-email-pl@kamp.de> <529364B5.6000202@redhat.com> In-Reply-To: <529364B5.6000202@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1.8 3/6] qemu-img: add option to specify alternate iobuffer size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On 25.11.2013 15:54, Paolo Bonzini wrote: > Il 25/11/2013 14:57, Peter Lieven ha scritto: >> since the convert process is basically a sync operation it might >> be benificial in some case to change the hardcoded I/O buffer >> size to an alternate (greater) value. > Do you really need the extra knob? You can just add to BlockLimits the > optimal transfer length, and use it unconditionally. That would be possible. > > (It would be interesting to add a double-buffer to qemu-img convert, > using asynchronous I/O...). That would be the best benefit of all. But I thought this would be too complicated. Peter > > Paolo > >> Signed-off-by: Peter Lieven >> --- >> qemu-img-cmds.hx | 4 ++-- >> qemu-img.c | 25 ++++++++++++++++++++----- >> qemu-img.texi | 4 +++- >> 3 files changed, 25 insertions(+), 8 deletions(-) >> >> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx >> index da1d965..e0b8ab4 100644 >> --- a/qemu-img-cmds.hx >> +++ b/qemu-img-cmds.hx >> @@ -34,9 +34,9 @@ STEXI >> ETEXI >> >> DEF("convert", img_convert, >> - "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename") >> + "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] [-m iobuf_size] filename [filename2 [...]] output_filename") >> STEXI >> -@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} >> +@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] [-m @var{iobuf_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} >> ETEXI >> >> DEF("info", img_info, >> diff --git a/qemu-img.c b/qemu-img.c >> index e2d1a0a..0ce5d14 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -105,6 +105,7 @@ static void help(void) >> " 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" >> " fully allocated\n" >> + " '-m' specify I/O buffer size in bytes (default 2M)\n" >> " '--output' takes the format in which the output must be done (human or json)\n" >> " '-n' skips the target volume creation (useful if the volume is created\n" >> " prior to running qemu-img)\n" >> @@ -1135,6 +1136,7 @@ static int img_convert(int argc, char **argv) >> sector_num_next_status = 0; >> uint64_t bs_sectors; >> uint8_t * buf = NULL; >> + size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; >> const uint8_t *buf1; >> BlockDriverInfo bdi; >> QEMUOptionParameter *param = NULL, *create_options = NULL; >> @@ -1152,7 +1154,7 @@ static int img_convert(int argc, char **argv) >> compress = 0; >> skip_create = 0; >> for(;;) { >> - c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qn"); >> + c = getopt(argc, argv, "f:O:B:s:hce6o:pS:m:t:qn"); >> if (c == -1) { >> break; >> } >> @@ -1200,6 +1202,19 @@ static int img_convert(int argc, char **argv) >> min_sparse = sval / BDRV_SECTOR_SIZE; >> break; >> } >> + case 'm': >> + { >> + int64_t sval; >> + char *end; >> + sval = strtosz_suffix(optarg, &end, STRTOSZ_DEFSUFFIX_B); >> + if (sval < BDRV_SECTOR_SIZE || *end) { >> + error_report("Invalid I/O buffer size specified"); >> + return 1; >> + } >> + >> + bufsectors = sval / BDRV_SECTOR_SIZE; >> + break; >> + } >> case 'p': >> progress = 1; >> break; >> @@ -1371,7 +1386,7 @@ static int img_convert(int argc, char **argv) >> bs_i = 0; >> bs_offset = 0; >> bdrv_get_geometry(bs[0], &bs_sectors); >> - buf = qemu_blockalign(out_bs, IO_BUF_SIZE); >> + buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE); >> >> if (skip_create) { >> int64_t output_length = bdrv_getlength(out_bs); >> @@ -1394,7 +1409,7 @@ static int img_convert(int argc, char **argv) >> goto out; >> } >> cluster_size = bdi.cluster_size; >> - if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) { >> + if (cluster_size <= 0 || cluster_size > bufsectors * BDRV_SECTOR_SIZE) { >> error_report("invalid cluster size"); >> ret = -1; >> goto out; >> @@ -1531,8 +1546,8 @@ static int img_convert(int argc, char **argv) >> sector_num_next_status = sector_num + n1; >> } >> >> - if (nb_sectors >= (IO_BUF_SIZE / 512)) { >> - n = (IO_BUF_SIZE / 512); >> + if (nb_sectors >= bufsectors) { >> + n = bufsectors; >> } else { >> n = nb_sectors; >> } >> diff --git a/qemu-img.texi b/qemu-img.texi >> index da36975..87f9d0f 100644 >> --- a/qemu-img.texi >> +++ b/qemu-img.texi >> @@ -179,7 +179,7 @@ Error on reading data >> >> @end table >> >> -@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} >> +@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] [-m @var{iobuf_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} >> >> Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename} >> using format @var{output_fmt}. It can be optionally compressed (@code{-c} >> @@ -199,6 +199,8 @@ 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 >> fully allocated. >> >> +@var{iobuf_size} indicates the size of the I/O buffer (defaults to 2M). >> + >> You can use the @var{backing_file} option to force the output image to be >> created as a copy on write image of the specified base image; the >> @var{backing_file} should have the same content as the input's base image, >> -- Mit freundlichen Grüßen Peter Lieven ........................................................... KAMP Netzwerkdienste GmbH Vestische Str. 89-91 | 46117 Oberhausen Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40 pl@kamp.de | http://www.kamp.de Geschäftsführer: Heiner Lante | Michael Lante Amtsgericht Duisburg | HRB Nr. 12154 USt-Id-Nr.: DE 120607556 ...........................................................