From 7b4c4781e390a041fa0ef70817678f1b97fc6db6 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Wed, 4 Dec 2013 17:10:54 +0800 Subject: snapshot: distinguish id and name in load_tmp Since later this function will be used so improve it. The only caller of it now is qemu-img, and it is not impacted by introduce function bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp() twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return int to let caller know the errno, and errno will be used later. Also fix a typo in comments of bdrv_snapshot_delete(). Signed-off-by: Wenchao Xia Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index dc0c2f0ed3..685c566d22 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1264,8 +1264,12 @@ static int img_convert(int argc, char **argv) ret = -1; goto out; } - if (bdrv_snapshot_load_tmp(bs[0], snapshot_name) < 0) { - error_report("Failed to load snapshot"); + + bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err); + if (error_is_set(&local_err)) { + error_report("Failed to load snapshot: %s", + error_get_pretty(local_err)); + error_free(local_err); ret = -1; goto out; } -- cgit 1.4.1 From ef80654d0dc1edf2dd2a51feff8cc3e1102a6583 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Wed, 4 Dec 2013 17:10:57 +0800 Subject: qemu-img: add -l for snapshot in convert Now qemu-img convert have similar options as qemu-nbd for internal snapshot. Signed-off-by: Wenchao Xia Signed-off-by: Stefan Hajnoczi --- qemu-img-cmds.hx | 4 ++-- qemu-img.c | 44 +++++++++++++++++++++++++++++++++++--------- qemu-img.texi | 12 ++++++++---- 3 files changed, 45 insertions(+), 15 deletions(-) (limited to 'qemu-img.c') diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index da1d965f3e..d02960921a 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_id_or_name] [-l snapshot_param] [-S sparse_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_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} ETEXI DEF("info", img_info, diff --git a/qemu-img.c b/qemu-img.c index 685c566d22..54ae984979 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -93,6 +93,11 @@ static void help(void) " 'options' is a comma separated list of format specific options in a\n" " name=value format. Use -o ? for an overview of the options supported by the\n" " used format\n" + " 'snapshot_param' is param used for internal snapshot, format\n" + " is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n" + " '[ID_OR_NAME]'\n" + " 'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n" + " instead\n" " '-c' indicates that target image must be compressed (qcow format only)\n" " '-u' enables unsafe rebasing. It is assumed that old and new backing file\n" " match exactly. The image doesn't need a working backing file before\n" @@ -1144,6 +1149,7 @@ static int img_convert(int argc, char **argv) int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ bool quiet = false; Error *local_err = NULL; + QemuOpts *sn_opts = NULL; fmt = NULL; out_fmt = "raw"; @@ -1152,7 +1158,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:t:qnl:"); if (c == -1) { break; } @@ -1187,6 +1193,18 @@ static int img_convert(int argc, char **argv) case 's': snapshot_name = optarg; break; + case 'l': + if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) { + sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0); + if (!sn_opts) { + error_report("Failed in parsing snapshot param '%s'", + optarg); + return 1; + } + } else { + snapshot_name = optarg; + } + break; case 'S': { int64_t sval; @@ -1258,7 +1276,12 @@ static int img_convert(int argc, char **argv) total_sectors += bs_sectors; } - if (snapshot_name != NULL) { + if (sn_opts) { + ret = bdrv_snapshot_load_tmp(bs[0], + qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID), + qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME), + &local_err); + } else if (snapshot_name != NULL) { if (bs_n > 1) { error_report("No support for concatenating multiple snapshot"); ret = -1; @@ -1266,13 +1289,13 @@ static int img_convert(int argc, char **argv) } bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err); - if (error_is_set(&local_err)) { - error_report("Failed to load snapshot: %s", - error_get_pretty(local_err)); - error_free(local_err); - ret = -1; - goto out; - } + } + if (error_is_set(&local_err)) { + error_report("Failed to load snapshot: %s", + error_get_pretty(local_err)); + error_free(local_err); + ret = -1; + goto out; } /* Find driver and parse its options */ @@ -1571,6 +1594,9 @@ out: free_option_parameters(create_options); free_option_parameters(param); qemu_vfree(buf); + if (sn_opts) { + qemu_opts_del(sn_opts); + } if (out_bs) { bdrv_unref(out_bs); } diff --git a/qemu-img.texi b/qemu-img.texi index da36975d70..be31191e43 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -46,7 +46,11 @@ is the destination disk image filename is a comma separated list of format specific options in a name=value format. Use @code{-o ?} for an overview of the options supported by the used format or see the format descriptions below for details. - +@item snapshot_param +is param used for internal snapshot, format is +'snapshot.id=[ID],snapshot.name=[NAME]' or '[ID_OR_NAME]' +@item snapshot_id_or_name +is deprecated, use snapshot_param instead @item -c indicates that target image must be compressed (qcow format only) @@ -179,10 +183,10 @@ 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_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_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} +Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated) +to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c} option) or use any format specific options like encryption (@code{-o} option). Only the formats @code{qcow} and @code{qcow2} support compression. The -- cgit 1.4.1 From 13c28af87a5541a9b09a59502b876a1725fb502d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 27 Nov 2013 11:07:01 +0100 Subject: qemu-img: add support for skipping zeroes in input during convert we currently do not check if a sector is allocated during convert. This means if a sector is unallocated that we allocate a bounce buffer of zeroes, find out its zero later and do not write it in the best case. In the worst case this can lead to reading blocks from a raw device (like iSCSI) altough we could easily know via get_block_status that they are zero and simply skip them. This patch also fixes the progress output not being at 100% after a successful conversion. Signed-off-by: Peter Lieven Reviewed-by: Paolo Bonzini Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 80 +++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 42 insertions(+), 38 deletions(-) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index 54ae984979..9d1b6b500a 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1130,13 +1130,15 @@ out3: static int img_convert(int argc, char **argv) { - int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, + int c, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors, skip_create; + int64_t ret = 0; int progress = 0, flags; const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename; BlockDriver *drv, *proto_drv; BlockDriverState **bs = NULL, *out_bs = NULL; - int64_t total_sectors, nb_sectors, sector_num, bs_offset; + int64_t total_sectors, nb_sectors, sector_num, bs_offset, + sector_num_next_status = 0; uint64_t bs_sectors; uint8_t * buf = NULL; const uint8_t *buf1; @@ -1145,7 +1147,6 @@ static int img_convert(int argc, char **argv) QEMUOptionParameter *out_baseimg_param; char *options = NULL; const char *snapshot_name = NULL; - float local_progress = 0; int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ bool quiet = false; Error *local_err = NULL; @@ -1430,10 +1431,6 @@ static int img_convert(int argc, char **argv) sector_num = 0; nb_sectors = total_sectors; - if (nb_sectors != 0) { - local_progress = (float)100 / - (nb_sectors / MIN(nb_sectors, cluster_sectors)); - } for(;;) { int64_t bs_num; @@ -1491,7 +1488,7 @@ static int img_convert(int argc, char **argv) } } sector_num += n; - qemu_progress_print(local_progress, 100); + qemu_progress_print(100.0 * sector_num / total_sectors, 0); } /* signal EOF to align */ bdrv_write_compressed(out_bs, 0, NULL, 0); @@ -1508,21 +1505,13 @@ static int img_convert(int argc, char **argv) sector_num = 0; // total number of sectors converted so far nb_sectors = total_sectors - sector_num; - if (nb_sectors != 0) { - local_progress = (float)100 / - (nb_sectors / MIN(nb_sectors, IO_BUF_SIZE / 512)); - } for(;;) { nb_sectors = total_sectors - sector_num; if (nb_sectors <= 0) { + ret = 0; break; } - if (nb_sectors >= (IO_BUF_SIZE / 512)) { - n = (IO_BUF_SIZE / 512); - } else { - n = nb_sectors; - } while (sector_num - bs_offset >= bs_sectors) { bs_i ++; @@ -1534,34 +1523,46 @@ static int img_convert(int argc, char **argv) sector_num, bs_i, bs_offset, bs_sectors); */ } - if (n > bs_offset + bs_sectors - sector_num) { - n = bs_offset + bs_sectors - sector_num; - } - - /* If the output image is being created as a copy on write image, - assume that sectors which are unallocated in the input image - are present in both the output's and input's base images (no - need to copy them). */ - if (out_baseimg) { - ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, - n, &n1); + if ((out_baseimg || has_zero_init) && + sector_num >= sector_num_next_status) { + n = nb_sectors > INT_MAX ? INT_MAX : nb_sectors; + ret = bdrv_get_block_status(bs[bs_i], sector_num - bs_offset, + n, &n1); if (ret < 0) { - error_report("error while reading metadata for sector " - "%" PRId64 ": %s", - sector_num - bs_offset, strerror(-ret)); + error_report("error while reading block status of sector %" + PRId64 ": %s", sector_num - bs_offset, + strerror(-ret)); goto out; } - if (!ret) { + /* If the output image is zero initialized, we are not working + * on a shared base and the input is zero we can skip the next + * n1 sectors */ + if (has_zero_init && !out_baseimg && (ret & BDRV_BLOCK_ZERO)) { sector_num += n1; continue; } - /* The next 'n1' sectors are allocated in the input image. Copy - only those as they may be followed by unallocated sectors. */ - n = n1; - } else { - n1 = n; + /* If the output image is being created as a copy on write + * image, assume that sectors which are unallocated in the + * input image are present in both the output's and input's + * base images (no need to copy them). */ + if (out_baseimg) { + if (!(ret & BDRV_BLOCK_DATA)) { + sector_num += n1; + continue; + } + /* The next 'n1' sectors are allocated in the input image. + * Copy only those as they may be followed by unallocated + * sectors. */ + nb_sectors = n1; + } + /* avoid redundant callouts to get_block_status */ + sector_num_next_status = sector_num + n1; } + n = MIN(nb_sectors, IO_BUF_SIZE / 512); + n = MIN(n, bs_sectors - (sector_num - bs_offset)); + n1 = n; + ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n); if (ret < 0) { error_report("error while reading sector %" PRId64 ": %s", @@ -1586,10 +1587,13 @@ static int img_convert(int argc, char **argv) n -= n1; buf1 += n1 * 512; } - qemu_progress_print(local_progress, 100); + qemu_progress_print(100.0 * sector_num / total_sectors, 0); } } out: + if (!ret) { + qemu_progress_print(100, 0); + } qemu_progress_end(); free_option_parameters(create_options); free_option_parameters(param); -- cgit 1.4.1 From 049b09825fe479f4caa013ccde0ff87fc9d82856 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 27 Nov 2013 11:07:02 +0100 Subject: qemu-img: fix usage instruction for qemu-img convert Reviewed-by: Eric Blake Reviewed-by: Paolo Bonzini Signed-off-by: Peter Lieven Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 1 - 1 file changed, 1 deletion(-) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index 9d1b6b500a..9fe0ede40b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -110,7 +110,6 @@ 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" - " images will always be fully allocated\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" -- cgit 1.4.1 From f2521c9023067a007d18b844fe7639c1c5b6f2ac Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 27 Nov 2013 11:07:06 +0100 Subject: qemu-img: dynamically adjust iobuffer size during convert 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 a greater value. This patch increases the I/O buffer size if the output driver advertises an optimal transfer length or discard alignment that is greater than the default buffer size of 2M. Reviewed-by: Paolo Bonzini Signed-off-by: Peter Lieven Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index 9fe0ede40b..0725f22cc2 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1140,6 +1140,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; @@ -1398,7 +1399,16 @@ 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); + + /* increase bufsectors from the default 4096 (2M) if opt_transfer_length + * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB) + * as maximum. */ + bufsectors = MIN(32768, + MAX(bufsectors, MAX(out_bs->bl.opt_transfer_length, + out_bs->bl.discard_alignment)) + ); + + buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE); if (skip_create) { int64_t output_length = bdrv_getlength(out_bs); @@ -1421,7 +1431,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; @@ -1558,7 +1568,7 @@ static int img_convert(int argc, char **argv) sector_num_next_status = sector_num + n1; } - n = MIN(nb_sectors, IO_BUF_SIZE / 512); + n = MIN(nb_sectors, bufsectors); n = MIN(n, bs_sectors - (sector_num - bs_offset)); n1 = n; -- cgit 1.4.1 From 24f833cd43dbfb5f8ae99e8a6d3691671622d3ea Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 27 Nov 2013 11:07:07 +0100 Subject: qemu-img: round down request length to an aligned sector this patch shortens requests to end at an aligned sector so that the next request starts aligned. [Squashed Peter's fix for bdrv_get_info() failure discussed on the mailing list. --Stefan] Reviewed-by: Paolo Bonzini Signed-off-by: Peter Lieven Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index 0725f22cc2..76f05f2e94 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1129,8 +1129,7 @@ out3: static int img_convert(int argc, char **argv) { - int c, n, n1, bs_n, bs_i, compress, cluster_size, - cluster_sectors, skip_create; + int c, n, n1, bs_n, bs_i, compress, cluster_sectors, skip_create; int64_t ret = 0; int progress = 0, flags; const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename; @@ -1424,19 +1423,23 @@ static int img_convert(int argc, char **argv) } } - if (compress) { - ret = bdrv_get_info(out_bs, &bdi); - if (ret < 0) { + cluster_sectors = 0; + ret = bdrv_get_info(out_bs, &bdi); + if (ret < 0) { + if (compress) { error_report("could not get block driver info"); goto out; } - cluster_size = bdi.cluster_size; - if (cluster_size <= 0 || cluster_size > bufsectors * BDRV_SECTOR_SIZE) { + } else { + cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE; + } + + if (compress) { + if (cluster_sectors <= 0 || cluster_sectors > bufsectors) { error_report("invalid cluster size"); ret = -1; goto out; } - cluster_sectors = cluster_size >> 9; sector_num = 0; nb_sectors = total_sectors; @@ -1569,6 +1572,19 @@ static int img_convert(int argc, char **argv) } n = MIN(nb_sectors, bufsectors); + + /* round down request length to an aligned sector, but + * do not bother doing this on short requests. They happen + * when we found an all-zero area, and the next sector to + * write will not be sector_num + n. */ + if (cluster_sectors > 0 && n >= cluster_sectors) { + int64_t next_aligned_sector = (sector_num + n); + next_aligned_sector -= next_aligned_sector % cluster_sectors; + if (sector_num + n > next_aligned_sector) { + n = next_aligned_sector - sector_num; + } + } + n = MIN(n, bs_sectors - (sector_num - bs_offset)); n1 = n; -- cgit 1.4.1 From 405889820bcd5c2abf4eb70598e96f525f862c0f Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 27 Nov 2013 11:07:09 +0100 Subject: qemu-img: decrease progress update interval on convert when doing very large jobs updating the progress only every 2% is too rare. Signed-off-by: Peter Lieven Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index 76f05f2e94..7dfe982b0c 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1245,7 +1245,7 @@ static int img_convert(int argc, char **argv) out_filename = argv[argc - 1]; /* Initialize before goto out */ - qemu_progress_init(progress, 2.0); + qemu_progress_init(progress, 1.0); if (options && is_help_option(options)) { ret = print_block_option_help(out_filename, out_fmt); -- cgit 1.4.1