From nobody Tue Oct 28 12:15:16 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1515398523906974.8197983399249; Mon, 8 Jan 2018 00:02:03 -0800 (PST) Received: from localhost ([::1]:46771 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eYSNe-0000Ve-T3 for importer@patchew.org; Mon, 08 Jan 2018 03:02:02 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37439) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eYSLg-0007pE-4R for qemu-devel@nongnu.org; Mon, 08 Jan 2018 03:00:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eYSLe-00051I-8U for qemu-devel@nongnu.org; Mon, 08 Jan 2018 03:00:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47590) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eYSLV-0004pg-Ca; Mon, 08 Jan 2018 02:59:49 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5BC9A5AFED; Mon, 8 Jan 2018 07:59:48 +0000 (UTC) Received: from lemon.usersys.redhat.com (ovpn-12-144.pek2.redhat.com [10.72.12.144]) by smtp.corp.redhat.com (Postfix) with ESMTP id 072266A02E; Mon, 8 Jan 2018 07:59:45 +0000 (UTC) From: Fam Zheng To: qemu-devel@nongnu.org Date: Mon, 8 Jan 2018 15:59:34 +0800 Message-Id: <20180108075935.23690-2-famz@redhat.com> In-Reply-To: <20180108075935.23690-1-famz@redhat.com> References: <20180108075935.23690-1-famz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 08 Jan 2018 07:59:48 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 1/2] qemu-img: Move img_open error reporting to callers 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: Kevin Wolf , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" In the next patch one caller will have a special error handling logic rather than reporting it. Add "Error **" parameters to functions and give control back to callers, to make that possible. Update iotests output accordingly. Reviewed-by: Eric Blake Signed-off-by: Fam Zheng --- qemu-img.c | 115 +++++++++++++++++++++++++++--------------= ---- tests/qemu-iotests/043.out | 6 +-- 2 files changed, 73 insertions(+), 48 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 68b375f998..828e3b3b88 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -268,24 +268,22 @@ static int print_block_option_help(const char *filena= me, const char *fmt) =20 static BlockBackend *img_open_opts(const char *optstr, QemuOpts *opts, int flags, bool writeth= rough, - bool quiet, bool force_share) + bool quiet, bool force_share, Error **e= rrp) { QDict *options; - Error *local_err =3D NULL; BlockBackend *blk; options =3D qemu_opts_to_qdict(opts, NULL); if (force_share) { if (qdict_haskey(options, BDRV_OPT_FORCE_SHARE) && !qdict_get_bool(options, BDRV_OPT_FORCE_SHARE)) { - error_report("--force-share/-U conflicts with image options"); + error_setg(errp, "--force-share/-U conflicts with image option= s"); QDECREF(options); return NULL; } qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true); } - blk =3D blk_new_open(NULL, NULL, options, flags, &local_err); + blk =3D blk_new_open(NULL, NULL, options, flags, errp); if (!blk) { - error_reportf_err(local_err, "Could not open '%s': ", optstr); return NULL; } blk_set_enable_write_cache(blk, !writethrough); @@ -297,10 +295,10 @@ static BlockBackend *img_open_file(const char *filena= me, QDict *options, const char *fmt, int flags, bool writethrough, bool quiet, - bool force_share) + bool force_share, + Error **errp) { BlockBackend *blk; - Error *local_err =3D NULL; =20 if (!options) { options =3D qdict_new(); @@ -312,9 +310,8 @@ static BlockBackend *img_open_file(const char *filename, if (force_share) { qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true); } - blk =3D blk_new_open(filename, NULL, options, flags, &local_err); + blk =3D blk_new_open(filename, NULL, options, flags, errp); if (!blk) { - error_reportf_err(local_err, "Could not open '%s': ", filename); return NULL; } blk_set_enable_write_cache(blk, !writethrough); @@ -340,7 +337,8 @@ static BlockBackend *img_open_new_file(const char *file= name, QemuOpts *create_opts, const char *fmt, int flags, bool writethrough, bool quiet, - bool force_share) + bool force_share, + Error **errp) { QDict *options =3D NULL; =20 @@ -348,32 +346,32 @@ static BlockBackend *img_open_new_file(const char *fi= lename, qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abo= rt); =20 return img_open_file(filename, options, fmt, flags, writethrough, quie= t, - force_share); + force_share, errp); } =20 =20 static BlockBackend *img_open(bool image_opts, const char *filename, const char *fmt, int flags, bool writethroug= h, - bool quiet, bool force_share) + bool quiet, bool force_share, Error **errp) { BlockBackend *blk; if (image_opts) { QemuOpts *opts; if (fmt) { - error_report("--image-opts and --format are mutually exclusive= "); + error_setg(errp, + "--image-opts and --format are mutually exclusive"); return NULL; } - opts =3D qemu_opts_parse_noisily(qemu_find_opts("source"), - filename, true); + opts =3D qemu_opts_parse(qemu_find_opts("source"), filename, true,= errp); if (!opts) { return NULL; } blk =3D img_open_opts(filename, opts, flags, writethrough, quiet, - force_share); + force_share, errp); } else { blk =3D img_open_file(filename, NULL, fmt, flags, writethrough, qu= iet, - force_share); + force_share, errp); } return blk; } @@ -670,6 +668,7 @@ static int img_check(int argc, char **argv) bool quiet =3D false; bool image_opts =3D false; bool force_share =3D false; + Error *local_err =3D NULL; =20 fmt =3D NULL; output =3D NULL; @@ -769,8 +768,9 @@ static int img_check(int argc, char **argv) } =20 blk =3D img_open(image_opts, filename, fmt, flags, writethrough, quiet, - force_share); + force_share, &local_err); if (!blk) { + error_reportf_err(local_err, "Could not open '%s': ", filename); return 1; } bs =3D blk_bs(blk); @@ -979,8 +979,9 @@ static int img_commit(int argc, char **argv) } =20 blk =3D img_open(image_opts, filename, fmt, flags, writethrough, quiet, - false); + false, &local_err); if (!blk) { + error_reportf_err(local_err, "Could not open '%s': ", filename); return 1; } bs =3D blk_bs(blk); @@ -1251,6 +1252,7 @@ static int img_compare(int argc, char **argv) uint64_t progress_base; bool image_opts =3D false; bool force_share =3D false; + Error *local_err =3D NULL; =20 cache =3D BDRV_DEFAULT_CACHE; for (;;) { @@ -1343,15 +1345,17 @@ static int img_compare(int argc, char **argv) } =20 blk1 =3D img_open(image_opts, filename1, fmt1, flags, writethrough, qu= iet, - force_share); + force_share, &local_err); if (!blk1) { + error_reportf_err(local_err, "Could not open '%s': ", filename1); ret =3D 2; goto out3; } =20 blk2 =3D img_open(image_opts, filename2, fmt2, flags, writethrough, qu= iet, - force_share); + force_share, &local_err); if (!blk2) { + error_reportf_err(local_err, "Could not open '%s': ", filename2); ret =3D 2; goto out2; } @@ -2121,8 +2125,10 @@ static int img_convert(int argc, char **argv) for (bs_i =3D 0; bs_i < s.src_num; bs_i++) { s.src[bs_i] =3D img_open(image_opts, argv[optind + bs_i], fmt, src_flags, src_writethrough, quiet, - force_share); + force_share, &local_err); if (!s.src[bs_i]) { + error_reportf_err(local_err, "Could not open '%s': ", + argv[optind + bs_i]); ret =3D -1; goto out; } @@ -2273,7 +2279,7 @@ static int img_convert(int argc, char **argv) =20 if (skip_create) { s.target =3D img_open(tgt_image_opts, out_filename, out_fmt, - flags, writethrough, quiet, false); + flags, writethrough, quiet, false, &local_err); } else { /* TODO ultimately we should allow --target-image-opts * to be used even when -n is not given. @@ -2281,9 +2287,11 @@ static int img_convert(int argc, char **argv) * to allow filenames in option syntax */ s.target =3D img_open_new_file(out_filename, opts, out_fmt, - flags, writethrough, quiet, false); + flags, writethrough, quiet, false, + &local_err); } if (!s.target) { + error_reportf_err(local_err, "Could not open '%s': ", out_filename= ); ret =3D -1; goto out; } @@ -2439,12 +2447,13 @@ static gboolean str_equal_func(gconstpointer a, gco= nstpointer b) static ImageInfoList *collect_image_info_list(bool image_opts, const char *filename, const char *fmt, - bool chain, bool force_share) + bool chain, bool force_share, + Error **errp) { ImageInfoList *head =3D NULL; ImageInfoList **last =3D &head; GHashTable *filenames; - Error *err =3D NULL; + Error *local_err =3D NULL; =20 filenames =3D g_hash_table_new_full(g_str_hash, str_equal_func, NULL, = NULL); =20 @@ -2455,23 +2464,24 @@ static ImageInfoList *collect_image_info_list(bool = image_opts, ImageInfoList *elem; =20 if (g_hash_table_lookup_extended(filenames, filename, NULL, NULL))= { - error_report("Backing file '%s' creates an infinite loop.", - filename); + error_setg(errp, + "Backing file '%s' creates an infinite loop", + filename); goto err; } g_hash_table_insert(filenames, (gpointer)filename, NULL); =20 blk =3D img_open(image_opts, filename, fmt, BDRV_O_NO_BACKING | BDRV_O_NO_IO, false, false, - force_share); + force_share, errp); if (!blk) { goto err; } bs =3D blk_bs(blk); =20 - bdrv_query_image_info(bs, &info, &err); - if (err) { - error_report_err(err); + bdrv_query_image_info(bs, &info, &local_err); + if (local_err) { + error_propagate(errp, local_err); blk_unref(blk); goto err; } @@ -2488,9 +2498,10 @@ static ImageInfoList *collect_image_info_list(bool i= mage_opts, if (info->has_full_backing_filename) { filename =3D info->full_backing_filename; } else if (info->has_backing_filename) { - error_report("Could not determine absolute backing filenam= e," - " but backing filename '%s' present", - info->backing_filename); + error_setg(errp, + "Could not determine absolute backing filename," + " but backing filename '%s' present", + info->backing_filename); goto err; } if (info->has_backing_filename_format) { @@ -2516,6 +2527,7 @@ static int img_info(int argc, char **argv) ImageInfoList *list; bool image_opts =3D false; bool force_share =3D false; + Error *local_err =3D NULL; =20 fmt =3D NULL; output =3D NULL; @@ -2592,8 +2604,9 @@ static int img_info(int argc, char **argv) } =20 list =3D collect_image_info_list(image_opts, filename, fmt, chain, - force_share); + force_share, &local_err); if (!list) { + error_reportf_err(local_err, "Could not open '%s': ", filename); return 1; } =20 @@ -2739,6 +2752,7 @@ static int img_map(int argc, char **argv) int ret =3D 0; bool image_opts =3D false; bool force_share =3D false; + Error *local_err =3D NULL; =20 fmt =3D NULL; output =3D NULL; @@ -2810,8 +2824,10 @@ static int img_map(int argc, char **argv) return 1; } =20 - blk =3D img_open(image_opts, filename, fmt, 0, false, false, force_sha= re); + blk =3D img_open(image_opts, filename, fmt, 0, false, false, force_sha= re, + &local_err); if (!blk) { + error_reportf_err(local_err, "Could not open '%s': ", filename); return 1; } bs =3D blk_bs(blk); @@ -2961,8 +2977,9 @@ static int img_snapshot(int argc, char **argv) =20 /* Open the image */ blk =3D img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet, - force_share); + force_share, &err); if (!blk) { + error_reportf_err(err, "Could not open '%s': ", filename); return 1; } bs =3D blk_bs(blk); @@ -3147,8 +3164,9 @@ static int img_rebase(int argc, char **argv) * the reference to a renamed or moved backing file. */ blk =3D img_open(image_opts, filename, fmt, flags, writethrough, quiet, - false); + false, &local_err); if (!blk) { + error_reportf_err(local_err, "Could not open '%s': ", filename); ret =3D -1; goto out; } @@ -3507,8 +3525,9 @@ static int img_resize(int argc, char **argv) =20 blk =3D img_open(image_opts, filename, fmt, BDRV_O_RDWR | BDRV_O_RESIZE, false, quiet, - false); + false, &err); if (!blk) { + error_reportf_err(err, "Could not open '%s': ", filename); ret =3D -1; goto out; } @@ -3693,8 +3712,9 @@ static int img_amend(int argc, char **argv) } =20 blk =3D img_open(image_opts, filename, fmt, flags, writethrough, quiet, - false); + false, &err); if (!blk) { + error_reportf_err(err, "Could not open '%s': ", filename); ret =3D -1; goto out; } @@ -3862,6 +3882,7 @@ static int img_bench(int argc, char **argv) struct timeval t1, t2; int i; bool force_share =3D false; + Error *local_err =3D NULL; =20 for (;;) { static const struct option long_options[] =3D { @@ -4018,8 +4039,9 @@ static int img_bench(int argc, char **argv) } =20 blk =3D img_open(image_opts, filename, fmt, flags, writethrough, quiet, - force_share); + force_share, &local_err); if (!blk) { + error_reportf_err(local_err, "Could not open '%s': ", filename); ret =3D -1; goto out; } @@ -4301,9 +4323,10 @@ static int img_dd(int argc, char **argv) } =20 blk1 =3D img_open(image_opts, in.filename, fmt, 0, false, false, - force_share); + force_share, &local_err); =20 if (!blk1) { + error_reportf_err(local_err, "Could not open '%s': ", in.filename); ret =3D -1; goto out; } @@ -4374,10 +4397,11 @@ static int img_dd(int argc, char **argv) * support image-opts style. */ blk2 =3D img_open_file(out.filename, NULL, out_fmt, BDRV_O_RDWR, - false, false, false); + false, false, false, &local_err); =20 if (!blk2) { ret =3D -1; + error_reportf_err(local_err, "Could not open '%s': ", out.filename= ); goto out; } =20 @@ -4600,8 +4624,9 @@ static int img_measure(int argc, char **argv) =20 if (filename) { in_blk =3D img_open(image_opts, filename, fmt, 0, - false, false, force_share); + false, false, force_share, &local_err); if (!in_blk) { + error_reportf_err(local_err, "Could not open '%s': ", filename= ); goto out; } =20 diff --git a/tests/qemu-iotests/043.out b/tests/qemu-iotests/043.out index b37d2a3807..840f333678 100644 --- a/tests/qemu-iotests/043.out +++ b/tests/qemu-iotests/043.out @@ -2,19 +2,19 @@ QA output created by 043 Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 =20 =3D=3D backing file references self =3D=3D -qemu-img: Backing file 'TEST_DIR/t.IMGFMT' creates an infinite loop. +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Backing file 'TEST_DIR/t.IMG= FMT' creates an infinite loop Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 backing_file= =3DTEST_DIR/t.IMGFMT.base =20 =3D=3D parent references self =3D=3D -qemu-img: Backing file 'TEST_DIR/t.IMGFMT' creates an infinite loop. +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Backing file 'TEST_DIR/t.IMG= FMT' creates an infinite loop Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 backing_file= =3DTEST_DIR/t.IMGFMT.1.base Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 backing_file= =3DTEST_DIR/t.IMGFMT.2.base Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 backing_file= =3DTEST_DIR/t.IMGFMT.3.base =20 =3D=3D ancestor references another ancestor =3D=3D -qemu-img: Backing file 'TEST_DIR/t.IMGFMT.2.base' creates an infinite loop. +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Backing file 'TEST_DIR/t.IMG= FMT.2.base' creates an infinite loop Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 backing_file= =3DTEST_DIR/t.IMGFMT.1.base Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 backing_file= =3DTEST_DIR/t.IMGFMT.2.base --=20 2.14.3