From patchwork Tue Feb 28 13:35:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Lieven X-Patchwork-Id: 9595807 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 90B5B600CB for ; Tue, 28 Feb 2017 14:38:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 82CDF2816B for ; Tue, 28 Feb 2017 14:38:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7762D28174; Tue, 28 Feb 2017 14:38:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 1AF0428306 for ; Tue, 28 Feb 2017 14:38:04 +0000 (UTC) Received: from localhost ([::1]:33402 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciiuh-0003rt-4S for patchwork-qemu-devel@patchwork.kernel.org; Tue, 28 Feb 2017 09:38:03 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53324) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cihwJ-0007Oz-On for qemu-devel@nongnu.org; Tue, 28 Feb 2017 08:35:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cihwG-0000jC-I0 for qemu-devel@nongnu.org; Tue, 28 Feb 2017 08:35:39 -0500 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:49781 helo=mx01.kamp.de) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cihwG-0000im-8S for qemu-devel@nongnu.org; Tue, 28 Feb 2017 08:35:36 -0500 Received: (qmail 18145 invoked by uid 89); 28 Feb 2017 13:35:33 -0000 Received: from [195.62.97.28] by client-16-kamp (envelope-from , uid 89) with qmail-scanner-2010/03/19-MF (clamdscan: 0.99.2/23143. avast: 1.2.2/17010300. spamassassin: 3.4.1. Clear:RC:1(195.62.97.28):. Processed in 0.089241 secs); 28 Feb 2017 13:35:33 -0000 Received: from smtp.kamp.de (HELO submission.kamp.de) ([195.62.97.28]) by mx01.kamp.de with ESMTPS (DHE-RSA-AES256-GCM-SHA384 encrypted); 28 Feb 2017 13:35:31 -0000 X-GL_Whitelist: yes Received: (qmail 9102 invoked from network); 28 Feb 2017 13:35:31 -0000 Received: from lieven-pc.kamp-intra.net (HELO lieven-pc) (relay@kamp.de@::ffff:172.21.12.60) by submission.kamp.de with ESMTPS (DHE-RSA-AES256-GCM-SHA384 encrypted) ESMTPA; 28 Feb 2017 13:35:31 -0000 Received: by lieven-pc (Postfix, from userid 1000) id 500B520668; Tue, 28 Feb 2017 14:35:31 +0100 (CET) From: Peter Lieven To: qemu-devel@nongnu.org Date: Tue, 28 Feb 2017 14:35:27 +0100 Message-Id: <1488288927-4607-1-git-send-email-pl@kamp.de> X-Mailer: git-send-email 1.9.1 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a02:248:0:51::16 Subject: [Qemu-devel] [PATCH] qemu-img: simplify img_convert X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-block@nongnu.org, stefanha@gmail.com, Peter Lieven , ct@flyingcircus.io, mreitz@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP img_convert has been around before there was an ImgConvertState or a block backend, but it has never been modified to directly use these structs. Change this by parsing parameters directly into the ImgConvertState and directly use BlockBackend where possible. Futhermore variable initalization has been reworked and sorted. Signed-off-by: Peter Lieven Reviewed-by: Eric Blake --- qemu-img.c | 197 +++++++++++++++++++++++++------------------------------------ 1 file changed, 81 insertions(+), 116 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index caa76a7..f271167 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1488,7 +1488,7 @@ typedef struct ImgConvertState { int min_sparse; size_t cluster_sectors; size_t buf_sectors; - int num_coroutines; + long num_coroutines; int running_coroutines; Coroutine *co[MAX_COROUTINES]; int64_t wait_sector_num[MAX_COROUTINES]; @@ -1882,39 +1882,33 @@ static int convert_do_copy(ImgConvertState *s) static int img_convert(int argc, char **argv) { - int c, bs_n, bs_i, compress, cluster_sectors, skip_create; - int64_t ret = 0; - int progress = 0, flags, src_flags; - bool writethrough, src_writethrough; - const char *fmt, *out_fmt, *cache, *src_cache, *out_baseimg, *out_filename; + int c, bs_i; + int flags, src_flags = 0; + const char *fmt = NULL, *out_fmt = "raw", *cache = "unsafe", + *src_cache = BDRV_DEFAULT_CACHE, *out_baseimg = NULL, + *out_filename; BlockDriver *drv, *proto_drv; - BlockBackend **blk = NULL, *out_blk = NULL; - BlockDriverState **bs = NULL, *out_bs = NULL; - int64_t total_sectors; - int64_t *bs_sectors = NULL; - size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; BlockDriverInfo bdi; + BlockDriverState *out_bs; QemuOpts *opts = NULL; QemuOptsList *create_opts = NULL; const char *out_baseimg_param; - char *options = NULL; const char *snapshot_name = NULL; - int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ - bool quiet = false; + char *options = NULL; Error *local_err = NULL; QemuOpts *sn_opts = NULL; - ImgConvertState state; - bool image_opts = false; - bool wr_in_order = true; - long num_coroutines = 8; + bool writethrough, src_writethrough, quiet = false, image_opts = false, + compress = false, skip_create = false, progress = false; + int64_t ret = -EINVAL; + + ImgConvertState s = (ImgConvertState) { + /* Need at least 4k of zeros for sparse detection */ + .min_sparse = 8, + .buf_sectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE, + .wr_in_order = true, + .num_coroutines = 8, + }; - fmt = NULL; - out_fmt = "raw"; - cache = "unsafe"; - src_cache = BDRV_DEFAULT_CACHE; - out_baseimg = NULL; - compress = 0; - skip_create = 0; for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, @@ -1940,24 +1934,22 @@ static int img_convert(int argc, char **argv) break; case 'B': out_baseimg = optarg; + s.target_has_backing = true; break; case 'c': - compress = 1; + s.compressed = true; break; case 'e': error_report("option -e is deprecated, please use \'-o " "encryption\' instead!"); - ret = -1; goto fail_getopt; case '6': error_report("option -6 is deprecated, please use \'-o " "compat6\' instead!"); - ret = -1; goto fail_getopt; case 'o': if (!is_valid_option_list(optarg)) { error_report("Invalid option list: %s", optarg); - ret = -1; goto fail_getopt; } if (!options) { @@ -1978,7 +1970,6 @@ static int img_convert(int argc, char **argv) if (!sn_opts) { error_report("Failed in parsing snapshot param '%s'", optarg); - ret = -1; goto fail_getopt; } } else { @@ -1992,15 +1983,14 @@ static int img_convert(int argc, char **argv) sval = cvtnum(optarg); if (sval < 0) { error_report("Invalid minimum zero buffer size for sparse output specified"); - ret = -1; goto fail_getopt; } - min_sparse = sval / BDRV_SECTOR_SIZE; + s.min_sparse = sval / BDRV_SECTOR_SIZE; break; } case 'p': - progress = 1; + progress = true; break; case 't': cache = optarg; @@ -2012,19 +2002,18 @@ static int img_convert(int argc, char **argv) quiet = true; break; case 'n': - skip_create = 1; + skip_create = true; break; case 'm': - if (qemu_strtol(optarg, NULL, 0, &num_coroutines) || - num_coroutines < 1 || num_coroutines > MAX_COROUTINES) { + if (qemu_strtol(optarg, NULL, 0, &s.num_coroutines) || + s.num_coroutines < 1 || s.num_coroutines > MAX_COROUTINES) { error_report("Invalid number of coroutines. Allowed number of" " coroutines is between 1 and %d", MAX_COROUTINES); - ret = -1; goto fail_getopt; } break; case 'W': - wr_in_order = false; + s.wr_in_order = false; break; case OPTION_OBJECT: opts = qemu_opts_parse_noisily(&qemu_object_opts, @@ -2045,83 +2034,79 @@ static int img_convert(int argc, char **argv) goto fail_getopt; } - if (!wr_in_order && compress) { + if (!s.wr_in_order && s.compressed) { error_report("Out of order write and compress are mutually exclusive"); - ret = -1; goto fail_getopt; } - /* Initialize before goto out */ - if (quiet) { - progress = 0; - } - qemu_progress_init(progress, 1.0); - - bs_n = argc - optind - 1; - out_filename = bs_n >= 1 ? argv[argc - 1] : NULL; + s.src_num = argc - optind - 1; + out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL; if (options && has_help_option(options)) { ret = print_block_option_help(out_filename, out_fmt); - goto out; + goto fail_getopt; } - if (bs_n < 1) { - error_exit("Must specify image file name"); + if (s.src_num < 1) { + error_report("Must specify image file name"); + goto fail_getopt; } - if (bs_n > 1 && out_baseimg) { + if (s.src_num > 1 && out_baseimg) { error_report("-B makes no sense when concatenating multiple input " "images"); - ret = -1; - goto out; + goto fail_getopt; } - src_flags = 0; + /* ret is still -EINVAL until here */ ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough); if (ret < 0) { error_report("Invalid source cache option: %s", src_cache); - goto out; + goto fail_getopt; } + /* Initialize before goto out */ + if (quiet) { + progress = false; + } + qemu_progress_init(progress, 1.0); qemu_progress_print(0, 100); - blk = g_new0(BlockBackend *, bs_n); - bs = g_new0(BlockDriverState *, bs_n); - bs_sectors = g_new(int64_t, bs_n); + s.src = g_new0(BlockBackend *, s.src_num); + s.src_sectors = g_new(int64_t, s.src_num); - total_sectors = 0; - for (bs_i = 0; bs_i < bs_n; bs_i++) { - blk[bs_i] = img_open(image_opts, argv[optind + bs_i], - fmt, src_flags, src_writethrough, quiet); - if (!blk[bs_i]) { + for (bs_i = 0; bs_i < s.src_num; bs_i++) { + s.src[bs_i] = img_open(image_opts, argv[optind + bs_i], + fmt, src_flags, src_writethrough, quiet); + if (!s.src[bs_i]) { ret = -1; goto out; } - bs[bs_i] = blk_bs(blk[bs_i]); - bs_sectors[bs_i] = blk_nb_sectors(blk[bs_i]); - if (bs_sectors[bs_i] < 0) { + s.src_sectors[bs_i] = blk_nb_sectors(s.src[bs_i]); + if (s.src_sectors[bs_i] < 0) { error_report("Could not get size of %s: %s", - argv[optind + bs_i], strerror(-bs_sectors[bs_i])); + argv[optind + bs_i], strerror(-s.src_sectors[bs_i])); ret = -1; goto out; } - total_sectors += bs_sectors[bs_i]; + s.total_sectors += s.src_sectors[bs_i]; } if (sn_opts) { - bdrv_snapshot_load_tmp(bs[0], + bdrv_snapshot_load_tmp(blk_bs(s.src[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) { + if (s.src_num > 1) { error_report("No support for concatenating multiple snapshot"); ret = -1; goto out; } - bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err); + bdrv_snapshot_load_tmp_by_id_or_name(blk_bs(s.src[0]), snapshot_name, + &local_err); } if (local_err) { error_reportf_err(local_err, "Failed to load snapshot: "); @@ -2172,7 +2157,7 @@ static int img_convert(int argc, char **argv) } } - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_sectors * 512, + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s.total_sectors * 512, &error_abort); ret = add_old_style_options(out_fmt, opts, out_baseimg, NULL); if (ret < 0) { @@ -2187,7 +2172,7 @@ static int img_convert(int argc, char **argv) } /* Check if compression is supported */ - if (compress) { + if (s.compressed) { bool encryption = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, false); const char *preallocation = @@ -2226,7 +2211,7 @@ static int img_convert(int argc, char **argv) } } - flags = min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR; + flags = s.min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR; ret = bdrv_parse_cache_mode(cache, &flags, &writethrough); if (ret < 0) { error_report("Invalid cache option: %s", cache); @@ -2238,37 +2223,36 @@ static int img_convert(int argc, char **argv) * the bdrv_create() call which takes different params. * Not critical right now, so fix can wait... */ - out_blk = img_open_file(out_filename, out_fmt, flags, writethrough, quiet); - if (!out_blk) { + s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet); + if (!s.target) { ret = -1; goto out; } - out_bs = blk_bs(out_blk); + out_bs = blk_bs(s.target); /* increase bufsectors from the default 4096 (2M) if opt_transfer * 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 >> BDRV_SECTOR_BITS, - out_bs->bl.pdiscard_alignment >> - BDRV_SECTOR_BITS))); + s.buf_sectors = MIN(32768, + MAX(s.buf_sectors, + MAX(out_bs->bl.opt_transfer >> BDRV_SECTOR_BITS, + out_bs->bl.pdiscard_alignment >> + BDRV_SECTOR_BITS))); if (skip_create) { - int64_t output_sectors = blk_nb_sectors(out_blk); + int64_t output_sectors = blk_nb_sectors(s.target); if (output_sectors < 0) { error_report("unable to get output image length: %s", strerror(-output_sectors)); ret = -1; goto out; - } else if (output_sectors < total_sectors) { + } else if (output_sectors < s.total_sectors) { error_report("output file is smaller than input file"); ret = -1; goto out; } } - cluster_sectors = 0; ret = bdrv_get_info(out_bs, &bdi); if (ret < 0) { if (compress) { @@ -2276,26 +2260,11 @@ static int img_convert(int argc, char **argv) goto out; } } else { - compress = compress || bdi.needs_compressed_writes; - cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE; - } - - state = (ImgConvertState) { - .src = blk, - .src_sectors = bs_sectors, - .src_num = bs_n, - .total_sectors = total_sectors, - .target = out_blk, - .compressed = compress, - .target_has_backing = (bool) out_baseimg, - .min_sparse = min_sparse, - .cluster_sectors = cluster_sectors, - .buf_sectors = bufsectors, - .wr_in_order = wr_in_order, - .num_coroutines = num_coroutines, - }; - ret = convert_do_copy(&state); + s.compressed = s.compressed || bdi.needs_compressed_writes; + s.cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE; + } + ret = convert_do_copy(&s); out: if (!ret) { qemu_progress_print(100, 0); @@ -2304,22 +2273,18 @@ out: qemu_opts_del(opts); qemu_opts_free(create_opts); qemu_opts_del(sn_opts); - blk_unref(out_blk); - g_free(bs); - if (blk) { - for (bs_i = 0; bs_i < bs_n; bs_i++) { - blk_unref(blk[bs_i]); + blk_unref(s.target); + if (s.src) { + for (bs_i = 0; bs_i < s.src_num; bs_i++) { + blk_unref(s.src[bs_i]); } - g_free(blk); + g_free(s.src); } - g_free(bs_sectors); + g_free(s.src_sectors); fail_getopt: g_free(options); - if (ret) { - return 1; - } - return 0; + return !!ret; }