From nobody Fri Nov 7 05:09:30 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.zoho.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 1488226895008429.0324041397205; Mon, 27 Feb 2017 12:21:35 -0800 (PST) Received: from localhost ([::1]:56562 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciRnZ-0000Yh-7K for importer@patchew.org; Mon, 27 Feb 2017 15:21:33 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57024) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciRcy-0008Ae-Gd for qemu-devel@nongnu.org; Mon, 27 Feb 2017 15:10:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ciRcw-0003It-Gj for qemu-devel@nongnu.org; Mon, 27 Feb 2017 15:10:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54848) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ciRcs-0003CH-6I; Mon, 27 Feb 2017 15:10:30 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 13E5E42BC7; Mon, 27 Feb 2017 20:10:30 +0000 (UTC) Received: from noname.redhat.com (ovpn-116-148.ams2.redhat.com [10.36.116.148]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1RK9n8R003294; Mon, 27 Feb 2017 15:10:27 -0500 From: Kevin Wolf To: qemu-block@nongnu.org Date: Mon, 27 Feb 2017 21:09:15 +0100 Message-Id: <1488226184-9044-15-git-send-email-kwolf@redhat.com> In-Reply-To: <1488226184-9044-1-git-send-email-kwolf@redhat.com> References: <1488226184-9044-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 27 Feb 2017 20:10:30 +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 14/43] block: Add error parameter to blk_insert_bs() 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, jcody@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com 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" Now that blk_insert_bs() requests the BlockBackend permissions for the node it attaches to, it can fail. Instead of aborting, pass the errors to the callers. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 5 ++++- block/backup.c | 5 ++++- block/block-backend.c | 13 ++++++++----- block/commit.c | 38 ++++++++++++++++++++++++++++++------= -- block/mirror.c | 15 ++++++++++++--- block/qcow2.c | 10 ++++++++-- blockdev.c | 11 +++++++++-- blockjob.c | 7 ++++++- hmp.c | 6 +++++- hw/core/qdev-properties-system.c | 7 ++++++- include/sysemu/block-backend.h | 2 +- migration/block.c | 2 +- nbd/server.c | 6 +++++- tests/test-blockjob.c | 2 +- 14 files changed, 100 insertions(+), 29 deletions(-) diff --git a/block.c b/block.c index 50c94ce..189351e 100644 --- a/block.c +++ b/block.c @@ -2185,8 +2185,11 @@ static BlockDriverState *bdrv_open_inherit(const cha= r *filename, } if (file_bs !=3D NULL) { file =3D blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL); - blk_insert_bs(file, file_bs); + blk_insert_bs(file, file_bs, &local_err); bdrv_unref(file_bs); + if (local_err) { + goto fail; + } =20 qdict_put(options, "file", qstring_from_str(bdrv_get_node_name(file_bs))); diff --git a/block/backup.c b/block/backup.c index 4b3c94c..f38d1d0 100644 --- a/block/backup.c +++ b/block/backup.c @@ -626,7 +626,10 @@ BlockJob *backup_job_create(const char *job_id, BlockD= riverState *bs, =20 /* FIXME Use real permissions */ job->target =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(job->target, target); + ret =3D blk_insert_bs(job->target, target, errp); + if (ret < 0) { + goto error; + } =20 job->on_source_error =3D on_source_error; job->on_target_error =3D on_target_error; diff --git a/block/block-backend.c b/block/block-backend.c index 0319220..299948f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -508,19 +508,22 @@ void blk_remove_bs(BlockBackend *blk) /* * Associates a new BlockDriverState with @blk. */ -void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) +int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp) { - bdrv_ref(bs); - /* FIXME Error handling */ blk->root =3D bdrv_root_attach_child(bs, "root", &child_root, - blk->perm, blk->shared_perm, blk, - &error_abort); + blk->perm, blk->shared_perm, blk, e= rrp); + if (blk->root =3D=3D NULL) { + return -EPERM; + } + bdrv_ref(bs); =20 notifier_list_notify(&blk->insert_bs_notifiers, blk); if (blk->public.throttle_state) { throttle_timers_attach_aio_context( &blk->public.throttle_timers, bdrv_get_aio_context(bs)); } + + return 0; } =20 /* diff --git a/block/commit.c b/block/commit.c index 1897e98..2ad8138 100644 --- a/block/commit.c +++ b/block/commit.c @@ -220,6 +220,7 @@ void commit_start(const char *job_id, BlockDriverState = *bs, BlockDriverState *iter; BlockDriverState *overlay_bs; Error *local_err =3D NULL; + int ret; =20 assert(top !=3D bs); if (top =3D=3D base) { @@ -256,8 +257,7 @@ void commit_start(const char *job_id, BlockDriverState = *bs, bdrv_reopen_multiple(bdrv_get_aio_context(bs), reopen_queue, &loca= l_err); if (local_err !=3D NULL) { error_propagate(errp, local_err); - block_job_unref(&s->common); - return; + goto fail; } } =20 @@ -277,11 +277,17 @@ void commit_start(const char *job_id, BlockDriverStat= e *bs, =20 /* FIXME Use real permissions */ s->base =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(s->base, base); + ret =3D blk_insert_bs(s->base, base, errp); + if (ret < 0) { + goto fail; + } =20 /* FIXME Use real permissions */ s->top =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(s->top, top); + ret =3D blk_insert_bs(s->top, top, errp); + if (ret < 0) { + goto fail; + } =20 s->active =3D bs; =20 @@ -294,6 +300,16 @@ void commit_start(const char *job_id, BlockDriverState= *bs, =20 trace_commit_start(bs, base, top, s); block_job_start(&s->common); + return; + +fail: + if (s->base) { + blk_unref(s->base); + } + if (s->top) { + blk_unref(s->top); + } + block_job_unref(&s->common); } =20 =20 @@ -332,11 +348,17 @@ int bdrv_commit(BlockDriverState *bs) =20 /* FIXME Use real permissions */ src =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(src, bs); - - /* FIXME Use real permissions */ backing =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(backing, bs->backing->bs); + + ret =3D blk_insert_bs(src, bs, NULL); + if (ret < 0) { + goto ro_cleanup; + } + + ret =3D blk_insert_bs(backing, bs->backing->bs, NULL); + if (ret < 0) { + goto ro_cleanup; + } =20 length =3D blk_getlength(src); if (length < 0) { diff --git a/block/mirror.c b/block/mirror.c index 13d42d7..7eeeb97 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -525,9 +525,12 @@ static void mirror_exit(BlockJob *job, void *opaque) bdrv_replace_in_backing_chain(to_replace, target_bs); bdrv_drained_end(target_bs); =20 - /* We just changed the BDS the job BB refers to */ + /* We just changed the BDS the job BB refers to, so switch the BB = back + * so the cleanup does the right thing. We don't need any permissi= ons + * any more now. */ blk_remove_bs(job->blk); - blk_insert_bs(job->blk, src); + blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); + blk_insert_bs(job->blk, src, &error_abort); } if (s->to_replace) { bdrv_op_unblock_all(s->to_replace, s->replace_blocker); @@ -995,6 +998,7 @@ static void mirror_start_job(const char *job_id, BlockD= riverState *bs, bool auto_complete) { MirrorBlockJob *s; + int ret; =20 if (granularity =3D=3D 0) { granularity =3D bdrv_get_default_bitmap_granularity(target); @@ -1019,7 +1023,12 @@ static void mirror_start_job(const char *job_id, Blo= ckDriverState *bs, =20 /* FIXME Use real permissions */ s->target =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(s->target, target); + ret =3D blk_insert_bs(s->target, target, errp); + if (ret < 0) { + blk_unref(s->target); + block_job_unref(&s->common); + return; + } =20 s->replaces =3D g_strdup(replaces); s->on_source_error =3D on_source_error; diff --git a/block/qcow2.c b/block/qcow2.c index 0356e69..6f79df8 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3113,6 +3113,7 @@ static int qcow2_amend_options(BlockDriverState *bs, = QemuOpts *opts, uint64_t cluster_size =3D s->cluster_size; bool encrypt; int refcount_bits =3D s->refcount_bits; + Error *local_err =3D NULL; int ret; QemuOptDesc *desc =3D opts->list->desc; Qcow2AmendHelperCBInfo helper_cb_info; @@ -3263,10 +3264,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); - blk_insert_bs(blk, bs); + ret =3D blk_insert_bs(blk, bs, &local_err); + if (ret < 0) { + error_report_err(local_err); + blk_unref(blk); + return ret; + } + ret =3D blk_truncate(blk, new_size); blk_unref(blk); - if (ret < 0) { return ret; } diff --git a/blockdev.c b/blockdev.c index dc9ed3b..011871b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2435,6 +2435,7 @@ static void qmp_blockdev_insert_anon_medium(BlockBack= end *blk, BlockDriverState *bs, Error **= errp) { bool has_device; + int ret; =20 /* For BBs without a device, we can exchange the BDS tree at will */ has_device =3D blk_get_attached_dev(blk); @@ -2454,7 +2455,10 @@ static void qmp_blockdev_insert_anon_medium(BlockBac= kend *blk, return; } =20 - blk_insert_bs(blk, bs); + ret =3D blk_insert_bs(blk, bs, errp); + if (ret < 0) { + return; + } =20 if (!blk_dev_has_tray(blk)) { /* For tray-less devices, blockdev-close-tray is a no-op (or may n= ot be @@ -2890,7 +2894,10 @@ void qmp_block_resize(bool has_device, const char *d= evice, } =20 blk =3D blk_new(BLK_PERM_RESIZE, BLK_PERM_ALL); - blk_insert_bs(blk, bs); + ret =3D blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto out; + } =20 /* complete all in-flight operations before resizing the device */ bdrv_drain_all(); diff --git a/blockjob.c b/blockjob.c index 508e0e5..72b7d4c 100644 --- a/blockjob.c +++ b/blockjob.c @@ -128,6 +128,7 @@ void *block_job_create(const char *job_id, const BlockJ= obDriver *driver, { BlockBackend *blk; BlockJob *job; + int ret; =20 if (bs->job) { error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); @@ -161,7 +162,11 @@ void *block_job_create(const char *job_id, const Block= JobDriver *driver, =20 /* FIXME Use real permissions */ blk =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(blk, bs); + ret =3D blk_insert_bs(blk, bs, errp); + if (ret < 0) { + blk_unref(blk); + return NULL; + } =20 job =3D g_malloc0(driver->instance_size); error_setg(&job->blocker, "block device is in use by block job: %s", diff --git a/hmp.c b/hmp.c index 020141b..e219f97 100644 --- a/hmp.c +++ b/hmp.c @@ -2045,6 +2045,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) const char* device =3D qdict_get_str(qdict, "device"); const char* command =3D qdict_get_str(qdict, "command"); Error *err =3D NULL; + int ret; =20 blk =3D blk_by_name(device); if (!blk) { @@ -2052,7 +2053,10 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) if (bs) { /* FIXME Use real permissions */ blk =3D local_blk =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(blk, bs); + ret =3D blk_insert_bs(blk, bs, &err); + if (ret < 0) { + goto fail; + } } else { goto fail; } diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-sys= tem.c index cca4775..66ba367 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -73,6 +73,7 @@ static void parse_drive(DeviceState *dev, const char *str= , void **ptr, { BlockBackend *blk; bool blk_created =3D false; + int ret; =20 blk =3D blk_by_name(str); if (!blk) { @@ -80,8 +81,12 @@ static void parse_drive(DeviceState *dev, const char *st= r, void **ptr, if (bs) { /* FIXME Use real permissions */ blk =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(blk, bs); blk_created =3D true; + + ret =3D blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto fail; + } } } if (!blk) { diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 6651f43..0861113 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -102,7 +102,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public); =20 BlockDriverState *blk_bs(BlockBackend *blk); void blk_remove_bs(BlockBackend *blk); -void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs); +int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp); bool bdrv_has_blk(BlockDriverState *bs); bool bdrv_is_root_node(BlockDriverState *bs); int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm, diff --git a/migration/block.c b/migration/block.c index 6b7ffd4..d259936 100644 --- a/migration/block.c +++ b/migration/block.c @@ -446,7 +446,7 @@ static void init_blk_migration(QEMUFile *f) BlockDriverState *bs =3D bmds_bs[i].bs; =20 if (bmds) { - blk_insert_bs(bmds->blk, bs); + blk_insert_bs(bmds->blk, bs, &error_abort); =20 alloc_aio_bitmap(bmds); error_setg(&bmds->blocker, "block device is in use by migratio= n"); diff --git a/nbd/server.c b/nbd/server.c index 936d5aa..89362ba 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -891,10 +891,14 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t= dev_offset, off_t size, { BlockBackend *blk; NBDExport *exp =3D g_malloc0(sizeof(NBDExport)); + int ret; =20 /* FIXME Use real permissions */ blk =3D blk_new(0, BLK_PERM_ALL); - blk_insert_bs(blk, bs); + ret =3D blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto fail; + } blk_set_enable_write_cache(blk, !writethrough); =20 exp->refcount =3D 1; diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 1dd1cfa..143ce96 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -60,7 +60,7 @@ static BlockBackend *create_blk(const char *name) bs =3D bdrv_open("null-co://", NULL, NULL, 0, &error_abort); g_assert_nonnull(bs); =20 - blk_insert_bs(blk, bs); + blk_insert_bs(blk, bs, &error_abort); bdrv_unref(bs); =20 if (name) { --=20 1.8.3.1