From nobody Wed Feb 11 04:00:55 2026 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1525899953922922.0454142333755; Wed, 9 May 2018 14:05:53 -0700 (PDT) Received: from localhost ([::1]:59036 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGWHZ-0004R0-3J for importer@patchew.org; Wed, 09 May 2018 17:05:53 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39275) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGWCW-0008WM-Rn for qemu-devel@nongnu.org; Wed, 09 May 2018 17:00:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fGWCU-0002BO-Ci for qemu-devel@nongnu.org; Wed, 09 May 2018 17:00:40 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:45604 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fGWCP-00026k-1h; Wed, 09 May 2018 17:00:33 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E6527430A67E; Wed, 9 May 2018 21:00:31 +0000 (UTC) Received: from localhost (unknown [10.40.205.23]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E727510B0F2D; Wed, 9 May 2018 21:00:30 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 9 May 2018 23:00:18 +0200 Message-Id: <20180509210023.20283-3-mreitz@redhat.com> In-Reply-To: <20180509210023.20283-1-mreitz@redhat.com> References: <20180509210023.20283-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 09 May 2018 21:00:31 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 09 May 2018 21:00:31 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 2/7] block: Add Error parameter to bdrv_amend_options 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 , John Snow , qemu-devel@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" Looking at the qcow2 code that is riddled with error_report() calls, this is really how it should have been from the start. Along the way, turn the target_version/current_version comparisons at the beginning of qcow2_downgrade() into assertions (the caller has to make sure these conditions are met), and rephrase the error message on using compat=3D1.1 to get refcount widths other than 16 bits. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: John Snow --- include/block/block.h | 3 +- include/block/block_int.h | 3 +- block.c | 8 ++++-- block/qcow2.c | 72 ++++++++++++++++++++++++++----------------= ---- qemu-img.c | 4 +-- tests/qemu-iotests/060.out | 4 +-- tests/qemu-iotests/061.out | 7 ----- tests/qemu-iotests/080.out | 4 +-- tests/qemu-iotests/112.out | 3 -- 9 files changed, 57 insertions(+), 51 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 3894edda9d..30f2a3364b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -343,7 +343,8 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *r= es, BdrvCheckMode fix); typedef void BlockDriverAmendStatusCB(BlockDriverState *bs, int64_t offset, int64_t total_work_size, void *opaqu= e); int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts, - BlockDriverAmendStatusCB *status_cb, void *cb_opaqu= e); + BlockDriverAmendStatusCB *status_cb, void *cb_opaqu= e, + Error **errp); =20 /* external snapshots */ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs, diff --git a/include/block/block_int.h b/include/block/block_int.h index 76b589da57..92b0807c9a 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -322,7 +322,8 @@ struct BlockDriver { =20 int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts, BlockDriverAmendStatusCB *status_cb, - void *cb_opaque); + void *cb_opaque, + Error **errp); =20 void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event); =20 diff --git a/block.c b/block.c index 676e57f562..1a73c6a476 100644 --- a/block.c +++ b/block.c @@ -4996,15 +4996,19 @@ void bdrv_remove_aio_context_notifier(BlockDriverSt= ate *bs, } =20 int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts, - BlockDriverAmendStatusCB *status_cb, void *cb_opaqu= e) + BlockDriverAmendStatusCB *status_cb, void *cb_opaqu= e, + Error **errp) { if (!bs->drv) { + error_setg(errp, "Node is ejected"); return -ENOMEDIUM; } if (!bs->drv->bdrv_amend_options) { + error_setg(errp, "Block driver '%s' does not support option amendm= ent", + bs->drv->format_name); return -ENOTSUP; } - return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque); + return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque, err= p); } =20 /* This function will be called by the bdrv_recurse_is_first_non_filter me= thod diff --git a/block/qcow2.c b/block/qcow2.c index 6d532470a8..9106512f97 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4049,22 +4049,21 @@ static int qcow2_load_vmstate(BlockDriverState *bs,= QEMUIOVector *qiov, * have to be removed. */ static int qcow2_downgrade(BlockDriverState *bs, int target_version, - BlockDriverAmendStatusCB *status_cb, void *cb_o= paque) + BlockDriverAmendStatusCB *status_cb, void *cb_o= paque, + Error **errp) { BDRVQcow2State *s =3D bs->opaque; int current_version =3D s->qcow_version; int ret; =20 - if (target_version =3D=3D current_version) { - return 0; - } else if (target_version > current_version) { - return -EINVAL; - } else if (target_version !=3D 2) { - return -EINVAL; - } + /* This is qcow2_downgrade(), not qcow2_upgrade() */ + assert(target_version < current_version); + + /* There are no other versions (now) that you can downgrade to */ + assert(target_version =3D=3D 2); =20 if (s->refcount_order !=3D 4) { - error_report("compat=3D0.10 requires refcount_bits=3D16"); + error_setg(errp, "compat=3D0.10 requires refcount_bits=3D16"); return -ENOTSUP; } =20 @@ -4072,6 +4071,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int = target_version, if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) { ret =3D qcow2_mark_clean(bs); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to make the image clean"); return ret; } } @@ -4081,6 +4081,8 @@ static int qcow2_downgrade(BlockDriverState *bs, int = target_version, * best thing to do anyway */ =20 if (s->incompatible_features) { + error_setg(errp, "Cannot downgrade an image with incompatible feat= ures " + "%#" PRIx64 " set", s->incompatible_features); return -ENOTSUP; } =20 @@ -4094,6 +4096,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int = target_version, =20 ret =3D qcow2_expand_zero_clusters(bs, status_cb, cb_opaque); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to turn zero into data cluste= rs"); return ret; } =20 @@ -4101,6 +4104,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int = target_version, ret =3D qcow2_update_header(bs); if (ret < 0) { s->qcow_version =3D current_version; + error_setg_errno(errp, -ret, "Failed to update the image header"); return ret; } return 0; @@ -4178,7 +4182,8 @@ static void qcow2_amend_helper_cb(BlockDriverState *b= s, =20 static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, BlockDriverAmendStatusCB *status_cb, - void *cb_opaque) + void *cb_opaque, + Error **errp) { BDRVQcow2State *s =3D bs->opaque; int old_version =3D s->qcow_version, new_version =3D old_version; @@ -4190,7 +4195,6 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, bool encrypt; int encformat; int refcount_bits =3D s->refcount_bits; - Error *local_err =3D NULL; int ret; QemuOptDesc *desc =3D opts->list->desc; Qcow2AmendHelperCBInfo helper_cb_info; @@ -4211,11 +4215,11 @@ static int qcow2_amend_options(BlockDriverState *bs= , QemuOpts *opts, } else if (!strcmp(compat, "1.1")) { new_version =3D 3; } else { - error_report("Unknown compatibility level %s", compat); + error_setg(errp, "Unknown compatibility level %s", compat); return -EINVAL; } } else if (!strcmp(desc->name, BLOCK_OPT_PREALLOC)) { - error_report("Cannot change preallocation mode"); + error_setg(errp, "Cannot change preallocation mode"); return -ENOTSUP; } else if (!strcmp(desc->name, BLOCK_OPT_SIZE)) { new_size =3D qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0); @@ -4228,7 +4232,8 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, !!s->crypto); =20 if (encrypt !=3D !!s->crypto) { - error_report("Changing the encryption flag is not supporte= d"); + error_setg(errp, + "Changing the encryption flag is not supported"= ); return -ENOTSUP; } } else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT_FORMAT)) { @@ -4236,17 +4241,19 @@ static int qcow2_amend_options(BlockDriverState *bs= , QemuOpts *opts, qemu_opt_get(opts, BLOCK_OPT_ENCRYPT_FORMAT)); =20 if (encformat !=3D s->crypt_method_header) { - error_report("Changing the encryption format is not suppor= ted"); + error_setg(errp, + "Changing the encryption format is not supporte= d"); return -ENOTSUP; } } else if (g_str_has_prefix(desc->name, "encrypt.")) { - error_report("Changing the encryption parameters is not suppor= ted"); + error_setg(errp, + "Changing the encryption parameters is not supporte= d"); return -ENOTSUP; } else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) { cluster_size =3D qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZ= E, cluster_size); if (cluster_size !=3D s->cluster_size) { - error_report("Changing the cluster size is not supported"); + error_setg(errp, "Changing the cluster size is not support= ed"); return -ENOTSUP; } } else if (!strcmp(desc->name, BLOCK_OPT_LAZY_REFCOUNTS)) { @@ -4259,8 +4266,8 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, if (refcount_bits <=3D 0 || refcount_bits > 64 || !is_power_of_2(refcount_bits)) { - error_report("Refcount width must be a power of two and ma= y " - "not exceed 64 bits"); + error_setg(errp, "Refcount width must be a power of two an= d " + "may not exceed 64 bits"); return -EINVAL; } } else { @@ -4285,6 +4292,7 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, ret =3D qcow2_update_header(bs); if (ret < 0) { s->qcow_version =3D old_version; + error_setg_errno(errp, -ret, "Failed to update the image heade= r"); return ret; } } @@ -4293,18 +4301,17 @@ static int qcow2_amend_options(BlockDriverState *bs= , QemuOpts *opts, int refcount_order =3D ctz32(refcount_bits); =20 if (new_version < 3 && refcount_bits !=3D 16) { - error_report("Different refcount widths than 16 bits require " - "compatibility level 1.1 or above (use compat=3D1= .1 or " - "greater)"); + error_setg(errp, "Refcount widths other than 16 bits require " + "compatibility level 1.1 or above (use compat=3D1.1= or " + "greater)"); return -EINVAL; } =20 helper_cb_info.current_operation =3D QCOW2_CHANGING_REFCOUNT_ORDER; ret =3D qcow2_change_refcount_order(bs, refcount_order, &qcow2_amend_helper_cb, - &helper_cb_info, &local_err); + &helper_cb_info, errp); if (ret < 0) { - error_report_err(local_err); return ret; } } @@ -4314,6 +4321,7 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, backing_file ?: s->image_backing_file, backing_format ?: s->image_backing_format); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to change the backing fil= e"); return ret; } } @@ -4321,14 +4329,16 @@ static int qcow2_amend_options(BlockDriverState *bs= , QemuOpts *opts, if (s->use_lazy_refcounts !=3D lazy_refcounts) { if (lazy_refcounts) { if (new_version < 3) { - error_report("Lazy refcounts only supported with compatibi= lity " - "level 1.1 and above (use compat=3D1.1 or gre= ater)"); + error_setg(errp, "Lazy refcounts only supported with " + "compatibility level 1.1 and above (use compat= =3D1.1 " + "or greater)"); return -EINVAL; } s->compatible_features |=3D QCOW2_COMPAT_LAZY_REFCOUNTS; ret =3D qcow2_update_header(bs); if (ret < 0) { s->compatible_features &=3D ~QCOW2_COMPAT_LAZY_REFCOUNTS; + error_setg_errno(errp, -ret, "Failed to update the image h= eader"); return ret; } s->use_lazy_refcounts =3D true; @@ -4336,6 +4346,7 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, /* make image clean first */ ret =3D qcow2_mark_clean(bs); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to make the image cle= an"); return ret; } /* now disallow lazy refcounts */ @@ -4343,6 +4354,7 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, ret =3D qcow2_update_header(bs); if (ret < 0) { s->compatible_features |=3D QCOW2_COMPAT_LAZY_REFCOUNTS; + error_setg_errno(errp, -ret, "Failed to update the image h= eader"); return ret; } s->use_lazy_refcounts =3D false; @@ -4351,17 +4363,15 @@ static int qcow2_amend_options(BlockDriverState *bs= , QemuOpts *opts, =20 if (new_size) { BlockBackend *blk =3D blk_new(BLK_PERM_RESIZE, BLK_PERM_ALL); - ret =3D blk_insert_bs(blk, bs, &local_err); + ret =3D blk_insert_bs(blk, bs, errp); if (ret < 0) { - error_report_err(local_err); blk_unref(blk); return ret; } =20 - ret =3D blk_truncate(blk, new_size, PREALLOC_MODE_OFF, &local_err); + ret =3D blk_truncate(blk, new_size, PREALLOC_MODE_OFF, errp); blk_unref(blk); if (ret < 0) { - error_report_err(local_err); return ret; } } @@ -4370,7 +4380,7 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, if (new_version < old_version) { helper_cb_info.current_operation =3D QCOW2_DOWNGRADING; ret =3D qcow2_downgrade(bs, new_version, &qcow2_amend_helper_cb, - &helper_cb_info); + &helper_cb_info, errp); if (ret < 0) { return ret; } diff --git a/qemu-img.c b/qemu-img.c index f38aa51166..347cf2ed9f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3761,10 +3761,10 @@ static int img_amend(int argc, char **argv) =20 /* In case the driver does not call amend_status_cb() */ qemu_progress_print(0.f, 0); - ret =3D bdrv_amend_options(bs, opts, &amend_status_cb, NULL); + ret =3D bdrv_amend_options(bs, opts, &amend_status_cb, NULL, &err); qemu_progress_print(100.f, 0); if (ret < 0) { - error_report("Error while amending options: %s", strerror(-ret)); + error_report_err(err); goto out; } =20 diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index 25d5c3938b..5f4264cff6 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -129,7 +129,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D671= 08864 wrote 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qcow2: Marking image as corrupt: L2 table offset 0x42a00 unaligned (L1 ind= ex: 0); further corruption events will be suppressed -qemu-img: Error while amending options: Input/output error +qemu-img: Failed to turn zero into data clusters: Input/output error =20 =3D=3D=3D Testing unaligned L2 entry =3D=3D=3D =20 @@ -145,7 +145,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D671= 08864 wrote 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unalign= ed (L2 offset: 0x40000, L2 index: 0); further corruption events will be sup= pressed -qemu-img: Error while amending options: Input/output error +qemu-img: Failed to turn zero into data clusters: Input/output error =20 =3D=3D=3D Testing unaligned reftable entry =3D=3D=3D =20 diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index e857ef9a7d..183f7dd690 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -358,18 +358,12 @@ No errors were found on the image. =20 Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 qemu-img: Lazy refcounts only supported with compatibility level 1.1 and a= bove (use compat=3D1.1 or greater) -qemu-img: Error while amending options: Invalid argument qemu-img: Lazy refcounts only supported with compatibility level 1.1 and a= bove (use compat=3D1.1 or greater) -qemu-img: Error while amending options: Invalid argument qemu-img: Unknown compatibility level 0.42 -qemu-img: Error while amending options: Invalid argument qemu-img: Invalid parameter 'foo' qemu-img: Changing the cluster size is not supported -qemu-img: Error while amending options: Operation not supported qemu-img: Changing the encryption flag is not supported -qemu-img: Error while amending options: Operation not supported qemu-img: Cannot change preallocation mode -qemu-img: Error while amending options: Operation not supported =20 =3D=3D=3D Testing correct handling of unset value =3D=3D=3D =20 @@ -377,7 +371,6 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D671= 08864 Should work: Should not work: qemu-img: Changing the cluster size is not supported -qemu-img: Error while amending options: Operation not supported =20 =3D=3D=3D Testing zero expansion on inactive clusters =3D=3D=3D =20 diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out index 4e0f7f7b92..281c7e0d1d 100644 --- a/tests/qemu-iotests/080.out +++ b/tests/qemu-iotests/080.out @@ -65,7 +65,7 @@ wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qemu-img: Failed to load snapshot: Snapshot L1 table offset invalid qemu-img: Snapshot L1 table offset invalid -qemu-img: Error while amending options: Invalid argument +qemu-img: Failed to turn zero into data clusters: Invalid argument Failed to flush the refcount block cache: Invalid argument write failed: Invalid argument qemu-img: Snapshot L1 table offset invalid @@ -88,7 +88,7 @@ wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qemu-img: Failed to load snapshot: Snapshot L1 table too large qemu-img: Snapshot L1 table too large -qemu-img: Error while amending options: File too large +qemu-img: Failed to turn zero into data clusters: File too large Failed to flush the refcount block cache: File too large write failed: File too large qemu-img: Snapshot L1 table too large diff --git a/tests/qemu-iotests/112.out b/tests/qemu-iotests/112.out index 86f041075d..a23aa1be75 100644 --- a/tests/qemu-iotests/112.out +++ b/tests/qemu-iotests/112.out @@ -99,13 +99,11 @@ refcount bits: 64 =3D=3D=3D Amend to compat=3D0.10 =3D=3D=3D =20 qemu-img: compat=3D0.10 requires refcount_bits=3D16 -qemu-img: Error while amending options: Operation not supported refcount bits: 64 No errors were found on the image. refcount bits: 16 refcount bits: 16 qemu-img: Different refcount widths than 16 bits require compatibility lev= el 1.1 or above (use compat=3D1.1 or greater) -qemu-img: Error while amending options: Invalid argument refcount bits: 16 =20 =3D=3D=3D Amend with snapshot =3D=3D=3D @@ -113,7 +111,6 @@ refcount bits: 16 wrote 16777216/16777216 bytes at offset 0 16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qemu-img: Cannot decrease refcount entry width to 1 bits: Cluster at offse= t 0x50000 has a refcount of 2 -qemu-img: Error while amending options: Invalid argument No errors were found on the image. refcount bits: 16 No errors were found on the image. --=20 2.14.3