From nobody Mon Apr 29 01:57:13 2024 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 1503667893053920.706757647832; Fri, 25 Aug 2017 06:31:33 -0700 (PDT) Received: from localhost ([::1]:53346 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEhv-0004mL-KB for importer@patchew.org; Fri, 25 Aug 2017 09:31:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38421) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbT-0006zP-K2 for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbO-0003QO-Qq for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:51 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40221) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbJ-0003NK-JR; Fri, 25 Aug 2017 09:24:42 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDO5Dw025167 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:06 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:26 +0300 Message-Id: <20170825132332.6734-2-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 1/7] block: skip implicit nodes in snapshots, blockjobs 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" Implicit filter nodes added at the top of nodes can interfere with block jobs. This is not a problem when they are added by other jobs since adding another job will issue a QERR_DEVICE_IN_USE, but it can happen in the next commit which introduces an implicitly created throttle filter node below BlockBackend, which we want to be skipped during automatic operations on the graph since the user does not necessarily know about their existence. All implicit filters will have either bs->file or bs->backing set. This is a needed assumption so we can know which direction we will skip down the graph. Signed-off-by: Manos Pitsidianakis Reviewed-by: Alberto Garcia --- include/block/block_int.h | 17 +++++++++++++++++ block.c | 10 ++++++++++ block/qapi.c | 14 +++++--------- blockdev.c | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index 7571c0aaaf..9e0f70e055 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -699,6 +699,21 @@ static inline BlockDriverState *backing_bs(BlockDriver= State *bs) return bs->backing ? bs->backing->bs : NULL; } =20 +static inline BlockDriverState *file_bs(BlockDriverState *bs) +{ + return bs->file ? bs->file->bs : NULL; +} + +/* This is a convenience function to get either bs->file->bs or + * bs->backing->bs * */ +static inline BlockDriverState *child_bs(BlockDriverState *bs) +{ + BlockDriverState *backing =3D backing_bs(bs); + BlockDriverState *file =3D file_bs(bs); + assert(!(file && backing)); + return backing ?: file; +} + =20 /* Essential block drivers which must always be statically linked into qem= u, and * which therefore can be accessed without using bdrv_find_format() */ @@ -980,4 +995,6 @@ void bdrv_dec_in_flight(BlockDriverState *bs); =20 void blockdev_close_all_bdrv_states(void); =20 +BlockDriverState *bdrv_get_first_explicit(BlockDriverState *bs); + #endif /* BLOCK_INT_H */ diff --git a/block.c b/block.c index 3615a6809e..e35d546c08 100644 --- a/block.c +++ b/block.c @@ -4945,3 +4945,13 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverStat= e *bs, const char *name, =20 return drv->bdrv_can_store_new_dirty_bitmap(bs, name, granularity, err= p); } + +/* Get first explicit node down a bs chain. */ +BlockDriverState *bdrv_get_first_explicit(BlockDriverState *bs) +{ + while (bs && bs->drv && bs->implicit) { + bs =3D child_bs(bs); + assert(bs); + } + return bs; +} diff --git a/block/qapi.c b/block/qapi.c index 7fa2437923..847b044d13 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -147,9 +147,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *b= lk, =20 /* Skip automatically inserted nodes that the user isn't aware of = for * query-block (blk !=3D NULL), but not for query-named-block-node= s */ - while (blk && bs0->drv && bs0->implicit) { - bs0 =3D backing_bs(bs0); - assert(bs0); + if (blk) { + bs0 =3D bdrv_get_first_explicit(bs0); } } =20 @@ -336,9 +335,7 @@ static void bdrv_query_info(BlockBackend *blk, BlockInf= o **p_info, char *qdev; =20 /* Skip automatically inserted nodes that the user isn't aware of */ - while (bs && bs->drv && bs->implicit) { - bs =3D backing_bs(bs); - } + bs =3D bdrv_get_first_explicit(bs); =20 info->device =3D g_strdup(blk_name(blk)); info->type =3D g_strdup("unknown"); @@ -465,9 +462,8 @@ static BlockStats *bdrv_query_bds_stats(BlockDriverStat= e *bs, /* Skip automatically inserted nodes that the user isn't aware of in * a BlockBackend-level command. Stay at the exact node for a node-lev= el * command. */ - while (blk_level && bs->drv && bs->implicit) { - bs =3D backing_bs(bs); - assert(bs); + if (blk_level) { + bs =3D bdrv_get_first_explicit(bs); } =20 if (bdrv_get_node_name(bs)[0]) { diff --git a/blockdev.c b/blockdev.c index 23475abb72..fc7b65c3f0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1300,6 +1300,10 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_= sync(const char *device, if (!bs) { return NULL; } + + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 @@ -1508,6 +1512,9 @@ static void internal_snapshot_prepare(BlkActionState = *common, return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + /* AioContext is released in .clean() */ state->aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(state->aio_context); @@ -1664,6 +1671,9 @@ static void external_snapshot_prepare(BlkActionState = *common, return; } =20 + /* Skip implicit filter nodes */ + state->old_bs =3D bdrv_get_first_explicit(state->old_bs); + /* Acquire AioContext now so any threads operating on old_bs stop */ state->aio_context =3D bdrv_get_aio_context(state->old_bs); aio_context_acquire(state->aio_context); @@ -1844,6 +1854,9 @@ static void drive_backup_prepare(BlkActionState *comm= on, Error **errp) return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + /* AioContext is released in .clean() */ state->aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(state->aio_context); @@ -1908,6 +1921,9 @@ static void blockdev_backup_prepare(BlkActionState *c= ommon, Error **errp) return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + target =3D bdrv_lookup_bs(backup->target, backup->target, errp); if (!target) { return; @@ -2988,6 +3004,9 @@ void qmp_block_stream(bool has_job_id, const char *jo= b_id, const char *device, return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 @@ -3095,6 +3114,9 @@ void qmp_block_commit(bool has_job_id, const char *jo= b_id, const char *device, return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 @@ -3209,6 +3231,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup,= BlockJobTxn *txn, return NULL; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 @@ -3484,6 +3509,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 @@ -3638,6 +3666,9 @@ void qmp_blockdev_mirror(bool has_job_id, const char = *job_id, return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + target_bs =3D bdrv_lookup_bs(target, target, errp); if (!target_bs) { return; @@ -3786,6 +3817,9 @@ void qmp_change_backing_file(const char *device, return; } =20 + /* Skip implicit filter nodes */ + bs =3D bdrv_get_first_explicit(bs); + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 --=20 2.11.0 From nobody Mon Apr 29 01:57:13 2024 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 1503667684196966.1622125609603; Fri, 25 Aug 2017 06:28:04 -0700 (PDT) Received: from localhost ([::1]:53318 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEeY-0001Ix-SR for importer@patchew.org; Fri, 25 Aug 2017 09:28:02 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38325) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbQ-0006hl-5E for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbO-0003Q3-Lf for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:48 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40219) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbJ-0003NE-FU; Fri, 25 Aug 2017 09:24:41 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDO7aR025182 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:07 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:27 +0300 Message-Id: <20170825132332.6734-3-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 2/7] block: add options parameter to bdrv_new_open_driver() 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" Allow passing a QDict *options parameter to bdrv_new_open_driver() so that it can be used if a driver needs it upon creation. The previous behaviour (empty bs->options and bs->explicit_options) remains when options is NULL. Reviewed-by: Alberto Garcia Signed-off-by: Manos Pitsidianakis Reviewed-by: Kevin Wolf --- include/block/block.h | 2 +- block.c | 16 +++++++++++++--- block/commit.c | 4 ++-- block/mirror.c | 2 +- block/vvfat.c | 2 +- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index ab80195378..d1f03cb48b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -263,7 +263,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict = *parent_options, BlockDriverState *bdrv_open(const char *filename, const char *reference, QDict *options, int flags, Error **errp); BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_= name, - int flags, Error **errp); + int flags, QDict *options, Error **= errp); BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, BlockDriverState *bs, QDict *options, int flags); diff --git a/block.c b/block.c index e35d546c08..2de1c29eb3 100644 --- a/block.c +++ b/block.c @@ -1153,16 +1153,26 @@ open_failed: return ret; } =20 +/* + * If options is not NULL, its ownership is transferred to the block layer= . The + * caller must use QINCREF() if they wish to keep ownership. + */ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_= name, - int flags, Error **errp) + int flags, QDict *options, Error **= errp) { BlockDriverState *bs; int ret; =20 bs =3D bdrv_new(); bs->open_flags =3D flags; - bs->explicit_options =3D qdict_new(); - bs->options =3D qdict_new(); + if (options) { + bs->explicit_options =3D qdict_clone_shallow(options); + bs->options =3D qdict_clone_shallow(options); + QDECREF(options); + } else { + bs->explicit_options =3D qdict_new(); + bs->options =3D qdict_new(); + } bs->opaque =3D NULL; =20 update_options_from_flags(bs->options, flags); diff --git a/block/commit.c b/block/commit.c index c7857c3321..539e23c3f8 100644 --- a/block/commit.c +++ b/block/commit.c @@ -342,7 +342,7 @@ void commit_start(const char *job_id, BlockDriverState = *bs, /* Insert commit_top block node above top, so we can block consistent = read * on the backing chain below it */ commit_top_bs =3D bdrv_new_open_driver(&bdrv_commit_top, filter_node_n= ame, 0, - errp); + NULL, errp); if (commit_top_bs =3D=3D NULL) { goto fail; } @@ -494,7 +494,7 @@ int bdrv_commit(BlockDriverState *bs) backing_file_bs =3D backing_bs(bs); =20 commit_top_bs =3D bdrv_new_open_driver(&bdrv_commit_top, NULL, BDRV_O_= RDWR, - &local_err); + NULL, &local_err); if (commit_top_bs =3D=3D NULL) { error_report_err(local_err); goto ro_cleanup; diff --git a/block/mirror.c b/block/mirror.c index c9a6a3ca86..e1a160e6ea 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1164,7 +1164,7 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, * reads on the top, while disabling it in the intermediate nodes, and= make * the backing chain writable. */ mirror_top_bs =3D bdrv_new_open_driver(&bdrv_mirror_top, filter_node_n= ame, - BDRV_O_RDWR, errp); + BDRV_O_RDWR, NULL, errp); if (mirror_top_bs =3D=3D NULL) { return; } diff --git a/block/vvfat.c b/block/vvfat.c index a9e207f7f0..6c59473baf 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3194,7 +3194,7 @@ static int enable_write_target(BlockDriverState *bs, = Error **errp) #endif =20 backing =3D bdrv_new_open_driver(&vvfat_write_target, NULL, BDRV_O_ALL= OW_RDWR, - &error_abort); + NULL, &error_abort); *(void**) backing->opaque =3D s; =20 bdrv_set_backing_hd(s->bs, backing, &error_abort); --=20 2.11.0 From nobody Mon Apr 29 01:57:13 2024 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 1503667774531854.2250718626681; Fri, 25 Aug 2017 06:29:34 -0700 (PDT) Received: from localhost ([::1]:53328 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEg0-0002YF-VQ for importer@patchew.org; Fri, 25 Aug 2017 09:29:33 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbQ-0006kc-N0 for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbP-0003Qc-1h for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:48 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbJ-0003NF-Fu; Fri, 25 Aug 2017 09:24:41 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDO8cR025201 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:09 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:28 +0300 Message-Id: <20170825132332.6734-4-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 3/7] block: require job-id when device is a node name 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" With implicit filter nodes on the top of the graph it is not possible to generate job-ids with the name of the device in block_job_create() anymore, since the job's bs will not be a child_root. Instead we can require that job-id is not NULL in block_job_create(), and check that a job-id has been set in the callers of block_job_create() in blockdev.c. It is more consistent to require an explicit job-id when the device parameter in the job creation command, eg { "execute": "drive-backup", "arguments": { "device": "drive0", "sync": "full", "target": "backup.img" } } is not a BlockBackend name, instead of automatically getting it from the root BS if device is a node name. That information is lost after calling block_job_create(), so we can do it in its caller instead. Signed-off-by: Manos Pitsidianakis Reviewed-by: Alberto Garcia Reviewed-by: Kevin Wolf --- include/block/blockjob_int.h | 4 +-- blockdev.c | 65 +++++++++++++++++++++++++++++++++++++++-= ---- blockjob.c | 19 ++++++------- tests/test-blockjob.c | 9 +++--- 4 files changed, 72 insertions(+), 25 deletions(-) diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index f13ad05c0d..a4ab7f3d59 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -112,8 +112,8 @@ struct BlockJobDriver { =20 /** * block_job_create: - * @job_id: The id of the newly-created job, or %NULL to have one - * generated automatically. + * @job_id: The id of the newly-created job, must be non %NULL for external + * jobs and %NULL for internal jobs. * @job_type: The class object for the newly-created job. * @bs: The block * @perm, @shared_perm: Permissions to request for @bs diff --git a/blockdev.c b/blockdev.c index fc7b65c3f0..6ffa5b0b04 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3004,6 +3004,16 @@ void qmp_block_stream(bool has_job_id, const char *j= ob_id, const char *device, return; } =20 + /* Always require a job-id when device is a node name */ + if (!has_job_id) { + if (blk_by_name(device)) { + job_id =3D device; + } else { + error_setg(errp, "An explicit job ID is required for this node= "); + return; + } + } + /* Skip implicit filter nodes */ bs =3D bdrv_get_first_explicit(bs); =20 @@ -3058,7 +3068,7 @@ void qmp_block_stream(bool has_job_id, const char *jo= b_id, const char *device, /* backing_file string overrides base bs filename */ base_name =3D has_backing_file ? backing_file : base_name; =20 - stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name, + stream_start(job_id, bs, base_bs, base_name, has_speed ? speed : 0, on_error, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -3117,6 +3127,16 @@ void qmp_block_commit(bool has_job_id, const char *j= ob_id, const char *device, /* Skip implicit filter nodes */ bs =3D bdrv_get_first_explicit(bs); =20 + /* Always require a job-id when device is a node name */ + if (!has_job_id) { + if (blk_by_name(device)) { + job_id =3D device; + } else { + error_setg(errp, "An explicit job ID is required for this node= "); + return; + } + } + aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 @@ -3171,7 +3191,7 @@ void qmp_block_commit(bool has_job_id, const char *jo= b_id, const char *device, " but 'top' is the active layer"); goto out; } - commit_active_start(has_job_id ? job_id : NULL, bs, base_bs, + commit_active_start(job_id, bs, base_bs, BLOCK_JOB_DEFAULT, speed, on_error, filter_node_name, NULL, NULL, false, &local_er= r); } else { @@ -3179,7 +3199,7 @@ void qmp_block_commit(bool has_job_id, const char *jo= b_id, const char *device, if (bdrv_op_is_blocked(overlay_bs, BLOCK_OP_TYPE_COMMIT_TARGET, er= rp)) { goto out; } - commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, spee= d, + commit_start(job_id, bs, base_bs, top_bs, speed, on_error, has_backing_file ? backing_file : NULL, filter_node_name, &local_err); } @@ -3220,7 +3240,13 @@ static BlockJob *do_drive_backup(DriveBackup *backup= , BlockJobTxn *txn, backup->mode =3D NEW_IMAGE_MODE_ABSOLUTE_PATHS; } if (!backup->has_job_id) { - backup->job_id =3D NULL; + /* Always require a job-id when device is a node name */ + if (blk_by_name(backup->device)) { + backup->job_id =3D backup->device; + } else { + error_setg(errp, "An explicit job ID is required for this node= "); + return NULL; + } } if (!backup->has_compress) { backup->compress =3D false; @@ -3366,7 +3392,13 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup,= BlockJobTxn *txn, backup->on_target_error =3D BLOCKDEV_ON_ERROR_REPORT; } if (!backup->has_job_id) { - backup->job_id =3D NULL; + /* Always require a job-id when device is a node name */ + if (blk_by_name(backup->device)) { + backup->job_id =3D backup->device; + } else { + error_setg(errp, "An explicit job ID is required for this node= "); + return NULL; + } } if (!backup->has_compress) { backup->compress =3D false; @@ -3509,6 +3541,16 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) return; } =20 + /* Always require a job-id when device is a node name */ + if (!arg->has_job_id) { + if (blk_by_name(arg->device)) { + arg->job_id =3D arg->device; + } else { + error_setg(errp, "An explicit job ID is required for this node= "); + return; + } + } + /* Skip implicit filter nodes */ bs =3D bdrv_get_first_explicit(bs); =20 @@ -3624,7 +3666,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) =20 bdrv_set_aio_context(target_bs, aio_context); =20 - blockdev_mirror_common(arg->has_job_id ? arg->job_id : NULL, bs, targe= t_bs, + blockdev_mirror_common(arg->job_id, bs, target_bs, arg->has_replaces, arg->replaces, arg->sync, backing_mode, arg->has_speed, arg->speed, arg->has_granularity, arg->granularity, @@ -3674,12 +3716,21 @@ void qmp_blockdev_mirror(bool has_job_id, const cha= r *job_id, return; } =20 + /* Always require a job-id when device is a node name */ + if (!has_job_id) { + if (blk_by_name(device)) { + job_id =3D device; + } else { + error_setg(errp, "An explicit job ID is required for this node= "); + return; + } + } aio_context =3D bdrv_get_aio_context(bs); aio_context_acquire(aio_context); =20 bdrv_set_aio_context(target_bs, aio_context); =20 - blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs, + blockdev_mirror_common(job_id, bs, target_bs, has_replaces, replaces, sync, backing_mode, has_speed, speed, has_granularity, granularity, diff --git a/blockjob.c b/blockjob.c index 70a78188b7..1231be5ce3 100644 --- a/blockjob.c +++ b/blockjob.c @@ -622,20 +622,17 @@ void *block_job_create(const char *job_id, const Bloc= kJobDriver *driver, return NULL; } =20 - if (job_id =3D=3D NULL && !(flags & BLOCK_JOB_INTERNAL)) { - job_id =3D bdrv_get_device_name(bs); - if (!*job_id) { - error_setg(errp, "An explicit job ID is required for this node= "); - return NULL; - } - } - - if (job_id) { - if (flags & BLOCK_JOB_INTERNAL) { + if (flags & BLOCK_JOB_INTERNAL) { + if (job_id) { error_setg(errp, "Cannot specify job ID for internal block job= "); return NULL; } - + } else { + /* Require job-id. */ + if (!job_id) { + error_setg(errp, "A job ID must be specified"); + return NULL; + } if (!id_wellformed(job_id)) { error_setg(errp, "Invalid job ID '%s'", job_id); return NULL; diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 23bdf1a932..fc41ea2147 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -93,7 +93,7 @@ static void test_job_ids(void) blk[1] =3D create_blk("drive1"); blk[2] =3D create_blk("drive2"); =20 - /* No job ID provided and the block backend has no name */ + /* No job ID provided */ job[0] =3D do_test_id(blk[0], NULL, false); =20 /* These are all invalid job IDs */ @@ -119,16 +119,15 @@ static void test_job_ids(void) block_job_early_fail(job[0]); job[1] =3D do_test_id(blk[1], "id0", true); =20 - /* No job ID specified, defaults to the backend name ('drive1') */ block_job_early_fail(job[1]); - job[1] =3D do_test_id(blk[1], NULL, true); + job[1] =3D do_test_id(blk[1], "drive1", true); =20 /* Duplicate job ID */ job[2] =3D do_test_id(blk[2], "drive1", false); =20 - /* The ID of job[2] would default to 'drive2' but it is already in use= */ + /* The ID of job[2] is already in use */ job[0] =3D do_test_id(blk[0], "drive2", true); - job[2] =3D do_test_id(blk[2], NULL, false); + job[2] =3D do_test_id(blk[2], "drive2", false); =20 /* This one is valid */ job[2] =3D do_test_id(blk[2], "id_2", true); --=20 2.11.0 From nobody Mon Apr 29 01:57:13 2024 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 1503668184450916.8441429290423; Fri, 25 Aug 2017 06:36:24 -0700 (PDT) Received: from localhost ([::1]:53377 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEmd-0000FB-3U for importer@patchew.org; Fri, 25 Aug 2017 09:36:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38473) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbY-0007Ol-Rl for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbW-0003Tn-QQ for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:56 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40283) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbQ-0003RQ-Su; Fri, 25 Aug 2017 09:24:49 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDOATZ025227 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:11 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:29 +0300 Message-Id: <20170825132332.6734-5-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 4/7] block: remove legacy I/O throttling 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" This commit removes all I/O throttling from block/block-backend.c. In order to support the existing interface, it is changed to use the block/throttle.c filter driver. The throttle filter node that is created by the legacy interface is stored in a 'throttle_node' field in the BlockBackendPublic of the device. The legacy throttle node is managed by the legacy interface completely. More advanced configurations with the filter drive are possible using the QMP API, but these will be ignored by the legacy interface. Signed-off-by: Manos Pitsidianakis Reviewed-by: Alberto Garcia --- include/block/throttle-groups.h | 1 + include/sysemu/block-backend.h | 6 +- block/block-backend.c | 134 +++++++++++++++++++++++++-----------= ---- block/qapi.c | 10 +-- block/throttle.c | 8 +++ blockdev.c | 37 ++++++++--- tests/test-throttle.c | 19 +++--- 7 files changed, 140 insertions(+), 75 deletions(-) diff --git a/include/block/throttle-groups.h b/include/block/throttle-group= s.h index e2fd0513c4..8493540766 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -81,5 +81,6 @@ void throttle_group_detach_aio_context(ThrottleGroupMembe= r *tgm); * mutex. */ bool throttle_group_exists(const char *name); +ThrottleGroupMember *throttle_get_tgm(BlockDriverState *bs); =20 #endif diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 0e0cda7521..4a7ca53685 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -73,7 +73,7 @@ typedef struct BlockDevOps { * friends so that BlockBackends can be kept in lists outside block-backen= d.c * */ typedef struct BlockBackendPublic { - ThrottleGroupMember throttle_group_member; + BlockDriverState *throttle_node; } BlockBackendPublic; =20 BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm); @@ -225,7 +225,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk, =20 void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg); void blk_io_limits_disable(BlockBackend *blk); -void blk_io_limits_enable(BlockBackend *blk, const char *group); -void blk_io_limits_update_group(BlockBackend *blk, const char *group); +void blk_io_limits_enable(BlockBackend *blk, const char *group, Error **er= rp); +void blk_io_limits_update_group(BlockBackend *blk, const char *group, Erro= r **errp); =20 #endif diff --git a/block/block-backend.c b/block/block-backend.c index c51fb8c8aa..693ad27fc9 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -15,6 +15,7 @@ #include "block/block_int.h" #include "block/blockjob.h" #include "block/throttle-groups.h" +#include "qemu/throttle-options.h" #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "qapi-event.h" @@ -319,7 +320,7 @@ static void blk_delete(BlockBackend *blk) assert(!blk->refcnt); assert(!blk->name); assert(!blk->dev); - if (blk->public.throttle_group_member.throttle_state) { + if (blk->public.throttle_node) { blk_io_limits_disable(blk); } if (blk->root) { @@ -634,13 +635,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public) */ void blk_remove_bs(BlockBackend *blk) { - ThrottleTimers *tt; - notifier_list_notify(&blk->remove_bs_notifiers, blk); - if (blk->public.throttle_group_member.throttle_state) { - tt =3D &blk->public.throttle_group_member.throttle_timers; - throttle_timers_detach_aio_context(tt); - } =20 blk_update_root_state(blk); =20 @@ -661,12 +656,6 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState = *bs, Error **errp) bdrv_ref(bs); =20 notifier_list_notify(&blk->insert_bs_notifiers, blk); - if (blk->public.throttle_group_member.throttle_state) { - throttle_timers_attach_aio_context( - &blk->public.throttle_group_member.throttle_timers, - bdrv_get_aio_context(bs)); - } - return 0; } =20 @@ -1024,13 +1013,6 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, in= t64_t offset, } =20 bdrv_inc_in_flight(bs); - - /* throttling disk I/O */ - if (blk->public.throttle_group_member.throttle_state) { - throttle_group_co_io_limits_intercept(&blk->public.throttle_group_= member, - bytes, false); - } - ret =3D bdrv_co_preadv(blk->root, offset, bytes, qiov, flags); bdrv_dec_in_flight(bs); return ret; @@ -1051,11 +1033,6 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, i= nt64_t offset, } =20 bdrv_inc_in_flight(bs); - /* throttling disk I/O */ - if (blk->public.throttle_group_member.throttle_state) { - throttle_group_co_io_limits_intercept(&blk->public.throttle_group_= member, - bytes, true); - } =20 if (!blk->enable_write_cache) { flags |=3D BDRV_REQ_FUA; @@ -1723,13 +1700,8 @@ static AioContext *blk_aiocb_get_aio_context(BlockAI= OCB *acb) void blk_set_aio_context(BlockBackend *blk, AioContext *new_context) { BlockDriverState *bs =3D blk_bs(blk); - ThrottleGroupMember *tgm =3D &blk->public.throttle_group_member; =20 if (bs) { - if (tgm->throttle_state) { - throttle_group_detach_aio_context(tgm); - throttle_group_attach_aio_context(tgm, new_context); - } bdrv_set_aio_context(bs, new_context); } } @@ -1948,45 +1920,101 @@ int blk_commit_all(void) /* throttling disk I/O limits */ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg) { - throttle_group_config(&blk->public.throttle_group_member, cfg); + assert(blk->public.throttle_node); + throttle_group_config(throttle_get_tgm(blk->public.throttle_node), cfg= ); } =20 void blk_io_limits_disable(BlockBackend *blk) { - assert(blk->public.throttle_group_member.throttle_state); - bdrv_drained_begin(blk_bs(blk)); - throttle_group_unregister_tgm(&blk->public.throttle_group_member); - bdrv_drained_end(blk_bs(blk)); + BlockDriverState *bs, *throttle_node; + + throttle_node =3D blk_get_public(blk)->throttle_node; + + assert(throttle_node); + + bs =3D throttle_node->file->bs; + bdrv_drained_begin(bs); + + /* Ref throttle_node's child bs to ensure it won't go away */ + bdrv_ref(bs); + + bdrv_child_try_set_perm(throttle_node->file, 0, BLK_PERM_ALL, + &error_abort); + /* Replace throttle_node with bs. While throttle_node was inserted und= er + * blk, at this point it might have more than one parent, so use + * bdrv_replace_node(). This destroys throttle_node */ + bdrv_replace_node(throttle_node, bs, &error_abort); + blk_get_public(blk)->throttle_node =3D NULL; + + bdrv_unref(bs); + bdrv_drained_end(bs); + } =20 /* should be called before blk_set_io_limits if a limit is set */ -void blk_io_limits_enable(BlockBackend *blk, const char *group) +void blk_io_limits_enable(BlockBackend *blk, const char *group, Error **e= rrp) { - assert(!blk->public.throttle_group_member.throttle_state); - throttle_group_register_tgm(&blk->public.throttle_group_member, - group, blk_get_aio_context(blk)); + BlockDriverState *bs =3D blk_bs(blk), *throttle_node; + QDict *options =3D qdict_new(); + Error *local_err =3D NULL; + ThrottleState *ts; + + bdrv_drained_begin(bs); + + /* Force creation of group in case it doesn't exist */ + ts =3D throttle_group_incref(group); + qdict_set_default_str(options, "file", bs->node_name); + qdict_set_default_str(options, QEMU_OPT_THROTTLE_GROUP_NAME, group); + throttle_node =3D bdrv_new_open_driver(bdrv_find_format("throttle"), N= ULL, + bdrv_get_flags(bs), options, errp= ); + if (!throttle_node) { + goto end; + } + throttle_node->implicit =3D true; + + blk_remove_bs(blk); + blk_insert_bs(blk, throttle_node, &local_err); + if (local_err) { + error_propagate(errp, local_err); + blk_insert_bs(blk, bs, &error_abort); + bdrv_unref(throttle_node); + throttle_node =3D NULL; + goto end; + } + bdrv_unref(throttle_node); + + assert(throttle_node->file->bs =3D=3D bs); + assert(throttle_node->refcnt =3D=3D 1); + +end: + throttle_group_unref(ts); + bdrv_drained_end(bs); + blk_get_public(blk)->throttle_node =3D throttle_node; } =20 -void blk_io_limits_update_group(BlockBackend *blk, const char *group) +void blk_io_limits_update_group(BlockBackend *blk, const char *group, Erro= r **errp) { + ThrottleGroupMember *tgm; + /* this BB is not part of any group */ - if (!blk->public.throttle_group_member.throttle_state) { + if (!blk->public.throttle_node) { return; } =20 + tgm =3D throttle_get_tgm(blk->public.throttle_node); /* this BB is a part of the same group than the one we want */ - if (!g_strcmp0(throttle_group_get_name(&blk->public.throttle_group_mem= ber), - group)) { + if (!g_strcmp0(throttle_group_get_name(tgm), group)) { return; } =20 - /* need to change the group this bs belong to */ + /* need to change the group this bs belongs to */ blk_io_limits_disable(blk); - blk_io_limits_enable(blk, group); + blk_io_limits_enable(blk, group, errp); } =20 static void blk_root_drained_begin(BdrvChild *child) { + ThrottleGroupMember *tgm; BlockBackend *blk =3D child->opaque; =20 if (++blk->quiesce_counter =3D=3D 1) { @@ -1997,19 +2025,25 @@ static void blk_root_drained_begin(BdrvChild *child) =20 /* Note that blk->root may not be accessible here yet if we are just * attaching to a BlockDriverState that is drained. Use child instead.= */ - - if (atomic_fetch_inc(&blk->public.throttle_group_member.io_limits_disa= bled) =3D=3D 0) { - throttle_group_restart_tgm(&blk->public.throttle_group_member); + if (blk->public.throttle_node) { + tgm =3D throttle_get_tgm(blk->public.throttle_node); + if (atomic_fetch_inc(&tgm->io_limits_disabled) =3D=3D 0) { + throttle_group_restart_tgm(tgm); + } } } =20 static void blk_root_drained_end(BdrvChild *child) { + ThrottleGroupMember *tgm; BlockBackend *blk =3D child->opaque; assert(blk->quiesce_counter); =20 - assert(blk->public.throttle_group_member.io_limits_disabled); - atomic_dec(&blk->public.throttle_group_member.io_limits_disabled); + if (blk->public.throttle_node) { + tgm =3D throttle_get_tgm(blk->public.throttle_node); + assert(tgm->io_limits_disabled); + atomic_dec(&tgm->io_limits_disabled); + } =20 if (--blk->quiesce_counter =3D=3D 0) { if (blk->dev_ops && blk->dev_ops->drained_end) { diff --git a/block/qapi.c b/block/qapi.c index 847b044d13..ab55db7134 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -66,11 +66,12 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *b= lk, =20 info->detect_zeroes =3D bs->detect_zeroes; =20 - if (blk && blk_get_public(blk)->throttle_group_member.throttle_state) { + if (blk && blk_get_public(blk)->throttle_node) { ThrottleConfig cfg; - BlockBackendPublic *blkp =3D blk_get_public(blk); + BlockDriverState *throttle_node =3D blk_get_public(blk)->throttle_= node; + ThrottleGroupMember *tgm =3D throttle_get_tgm(throttle_node); =20 - throttle_group_get_config(&blkp->throttle_group_member, &cfg); + throttle_group_get_config(tgm, &cfg); =20 info->bps =3D cfg.buckets[THROTTLE_BPS_TOTAL].avg; info->bps_rd =3D cfg.buckets[THROTTLE_BPS_READ].avg; @@ -118,8 +119,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *b= lk, info->iops_size =3D cfg.op_size; =20 info->has_group =3D true; - info->group =3D - g_strdup(throttle_group_get_name(&blkp->throttle_group_member)= ); + info->group =3D g_strdup(throttle_group_get_name(tgm)); } =20 info->write_threshold =3D bdrv_write_threshold_get(bs); diff --git a/block/throttle.c b/block/throttle.c index 5ee1e2a7ca..7804ded709 100644 --- a/block/throttle.c +++ b/block/throttle.c @@ -35,6 +35,14 @@ static QemuOptsList throttle_opts =3D { }, }; =20 +static BlockDriver bdrv_throttle; + +ThrottleGroupMember *throttle_get_tgm(BlockDriverState *bs) +{ + assert(bs->drv =3D=3D &bdrv_throttle); + return (ThrottleGroupMember *)bs->opaque; +} + static int throttle_configure_tgm(BlockDriverState *bs, ThrottleGroupMember *tgm, QDict *options, Error **errp) diff --git a/blockdev.c b/blockdev.c index 6ffa5b0b04..3f76eed2aa 100644 --- a/blockdev.c +++ b/blockdev.c @@ -607,7 +607,14 @@ static BlockBackend *blockdev_init(const char *file, Q= Dict *bs_opts, if (!throttling_group) { throttling_group =3D id; } - blk_io_limits_enable(blk, throttling_group); + blk_io_limits_enable(blk, throttling_group, &error); + if (error) { + error_propagate(errp, error); + blk_unref(blk); + blk =3D NULL; + goto err_no_bs_opts; + + } blk_set_io_limits(blk, &cfg); } =20 @@ -2629,6 +2636,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, = Error **errp) BlockDriverState *bs; BlockBackend *blk; AioContext *aio_context; + Error *local_err =3D NULL; =20 blk =3D qmp_get_blk(arg->has_device ? arg->device : NULL, arg->has_id ? arg->id : NULL, @@ -2704,18 +2712,29 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg= , Error **errp) if (throttle_enabled(&cfg)) { /* Enable I/O limits if they're not enabled yet, otherwise * just update the throttling group. */ - if (!blk_get_public(blk)->throttle_group_member.throttle_state) { - blk_io_limits_enable(blk, - arg->has_group ? arg->group : - arg->has_device ? arg->device : - arg->id); + if (!blk_get_public(blk)->throttle_node) { + blk_io_limits_enable(blk, arg->has_group ? arg->group : + arg->has_device ? arg->device : arg->id, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } } else if (arg->has_group) { - blk_io_limits_update_group(blk, arg->group); + /* move throttle node membership to arg->group */ + blk_io_limits_update_group(blk, arg->group, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } } /* Set the new throttling configuration */ blk_set_io_limits(blk, &cfg); - } else if (blk_get_public(blk)->throttle_group_member.throttle_state) { - /* If all throttling settings are set to 0, disable I/O limits */ + } else if (blk_get_public(blk)->throttle_node) { + /* + * If all throttling settings are set to 0, disable I/O limits + * by deleting the legacy throttle node + * */ blk_io_limits_disable(blk); } =20 diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 0ea9093eee..eef2b1c707 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -594,7 +594,6 @@ static void test_groups(void) { ThrottleConfig cfg1, cfg2; BlockBackend *blk1, *blk2, *blk3; - BlockBackendPublic *blkp1, *blkp2, *blkp3; ThrottleGroupMember *tgm1, *tgm2, *tgm3; =20 /* No actual I/O is performed on these devices */ @@ -602,13 +601,9 @@ static void test_groups(void) blk2 =3D blk_new(0, BLK_PERM_ALL); blk3 =3D blk_new(0, BLK_PERM_ALL); =20 - blkp1 =3D blk_get_public(blk1); - blkp2 =3D blk_get_public(blk2); - blkp3 =3D blk_get_public(blk3); - - tgm1 =3D &blkp1->throttle_group_member; - tgm2 =3D &blkp2->throttle_group_member; - tgm3 =3D &blkp3->throttle_group_member; + tgm1 =3D g_new0(ThrottleGroupMember, 1); + tgm2 =3D g_new0(ThrottleGroupMember, 1); + tgm3 =3D g_new0(ThrottleGroupMember, 1); =20 g_assert(tgm1->throttle_state =3D=3D NULL); g_assert(tgm2->throttle_state =3D=3D NULL); @@ -655,6 +650,14 @@ static void test_groups(void) g_assert(tgm1->throttle_state =3D=3D NULL); g_assert(tgm2->throttle_state =3D=3D NULL); g_assert(tgm3->throttle_state =3D=3D NULL); + + g_free(tgm1); + g_free(tgm2); + g_free(tgm3); + + blk_unref(blk1); + blk_unref(blk2); + blk_unref(blk3); } =20 int main(int argc, char **argv) --=20 2.11.0 From nobody Mon Apr 29 01:57:13 2024 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 150366791161416.949570882023863; Fri, 25 Aug 2017 06:31:51 -0700 (PDT) Received: from localhost ([::1]:53347 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEiE-00051c-EV for importer@patchew.org; Fri, 25 Aug 2017 09:31:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38440) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbV-00077t-JH for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbT-0003Sl-0Q for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:53 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40256) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbO-0003PL-6a; Fri, 25 Aug 2017 09:24:46 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDOCit025232 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:12 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:30 +0300 Message-Id: <20170825132332.6734-6-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 5/7] block/throttle-groups.c: remove throttle-groups list 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" block/throttle-groups.c uses a list to keep track of all groups and make sure all names are unique. This patch moves all ThrottleGroup objects under the root container. While block/throttle.c requires that all throttle groups are created with object-add or -object, the legacy throttling interface still used the throttle_groups list. By using the root container we get the name collision check for free and have all groups, legacy or not, available through the qom-get/qom-set commands. Legacy groups are marked and freed when they have no users left instead of waiting for an explicit object-del. Signed-off-by: Manos Pitsidianakis --- include/block/throttle-groups.h | 1 + block/block-backend.c | 15 +++-- block/throttle-groups.c | 145 +++++++++++++++++++++++-------------= ---- tests/test-throttle.c | 3 + 4 files changed, 96 insertions(+), 68 deletions(-) diff --git a/include/block/throttle-groups.h b/include/block/throttle-group= s.h index 8493540766..13fbc63f1e 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -58,6 +58,7 @@ typedef struct ThrottleGroupMember { =20 const char *throttle_group_get_name(ThrottleGroupMember *tgm); =20 +void throttle_group_new_legacy(const char *name, Error **errp); ThrottleState *throttle_group_incref(const char *name); void throttle_group_unref(ThrottleState *ts); =20 diff --git a/block/block-backend.c b/block/block-backend.c index 693ad27fc9..65f458ce8f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1957,12 +1957,18 @@ void blk_io_limits_enable(BlockBackend *blk, const = char *group, Error **errp) BlockDriverState *bs =3D blk_bs(blk), *throttle_node; QDict *options =3D qdict_new(); Error *local_err =3D NULL; - ThrottleState *ts; - - bdrv_drained_begin(bs); =20 /* Force creation of group in case it doesn't exist */ - ts =3D throttle_group_incref(group); + if (!throttle_group_exists(group)) { + throttle_group_new_legacy(group, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + + bdrv_drained_begin(bs); + qdict_set_default_str(options, "file", bs->node_name); qdict_set_default_str(options, QEMU_OPT_THROTTLE_GROUP_NAME, group); throttle_node =3D bdrv_new_open_driver(bdrv_find_format("throttle"), N= ULL, @@ -1987,7 +1993,6 @@ void blk_io_limits_enable(BlockBackend *blk, const ch= ar *group, Error **errp) assert(throttle_node->refcnt =3D=3D 1); =20 end: - throttle_group_unref(ts); bdrv_drained_end(bs); blk_get_public(blk)->throttle_node =3D throttle_node; } diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 454bfcb067..238b648489 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -33,8 +33,10 @@ #include "qapi-visit.h" #include "qom/object.h" #include "qom/object_interfaces.h" +#include "qapi/qobject-input-visitor.h" =20 static void throttle_group_obj_init(Object *obj); +static bool throttle_group_can_be_deleted(UserCreatable *uc, Error **errp); static void throttle_group_obj_complete(UserCreatable *obj, Error **errp); =20 /* The ThrottleGroup structure (with its ThrottleState) is shared @@ -66,6 +68,7 @@ typedef struct ThrottleGroup { =20 /* refuse individual property change if initialization is complete */ bool is_initialized; + bool legacy; char *name; /* This is constant during the lifetime of the group */ =20 QemuMutex lock; /* This lock protects the following four fields */ @@ -74,34 +77,47 @@ typedef struct ThrottleGroup { ThrottleGroupMember *tokens[2]; bool any_timer_armed[2]; QEMUClockType clock_type; - - /* This field is protected by the global QEMU mutex */ - QTAILQ_ENTRY(ThrottleGroup) list; } ThrottleGroup; =20 -/* This is protected by the global QEMU mutex */ -static QTAILQ_HEAD(, ThrottleGroup) throttle_groups =3D - QTAILQ_HEAD_INITIALIZER(throttle_groups); =20 +struct ThrottleGroupQuery { + const char *name; + ThrottleGroup *result; +}; =20 -/* This function reads throttle_groups and must be called under the global - * mutex. +static int query_throttle_groups_foreach(Object *obj, void *data) +{ + ThrottleGroup *tg; + struct ThrottleGroupQuery *query =3D data; + + tg =3D (ThrottleGroup *)object_dynamic_cast(obj, TYPE_THROTTLE_GROUP); + if (!tg) { + return 0; + } + + if (!g_strcmp0(query->name, tg->name)) { + query->result =3D tg; + return 1; + } + + return 0; +} + + +/* This function reads the QOM root container and must be called under the + * global mutex. */ static ThrottleGroup *throttle_group_by_name(const char *name) { - ThrottleGroup *iter; + struct ThrottleGroupQuery query =3D { name =3D name }; =20 /* Look for an existing group with that name */ - QTAILQ_FOREACH(iter, &throttle_groups, list) { - if (!g_strcmp0(name, iter->name)) { - return iter; - } - } - - return NULL; + object_child_foreach(object_get_objects_root(), + query_throttle_groups_foreach, &query); + return query.result; } =20 -/* This function reads throttle_groups and must be called under the global +/* This function must be called under the global * mutex. */ bool throttle_group_exists(const char *name) @@ -109,34 +125,49 @@ bool throttle_group_exists(const char *name) return throttle_group_by_name(name) !=3D NULL; } =20 +/* + * Create a new ThrottleGroup, insert it in the object root container so t= hat + * we can refer to it by id and set tg->legacy to true + * + * This function edits the QOM root container and must be called under the + * global mutex. + * + * @name: the name of the ThrottleGroup. + * @errp: Error object. Will be set if @name collides with a non-ThrottleG= roup + * QOM object + */ +void throttle_group_new_legacy(const char *name, Error **errp) +{ + ThrottleGroup *tg =3D NULL; + + /* Create an empty property qdict. Caller is responsible for + * setting up limits */ + QDict *pdict =3D qdict_new(); + Visitor *v =3D qobject_input_visitor_new(QOBJECT(pdict)); + + /* tg will have a ref count of 2, one for the object root container + * and one for the caller */ + tg =3D THROTTLE_GROUP(user_creatable_add_type(TYPE_THROTTLE_GROUP, + name, pdict, v, errp)); + visit_free(v); + QDECREF(pdict); + if (!tg) { + return; + } + tg->legacy =3D true; +} + /* Increments the reference count of a ThrottleGroup given its name. * - * If no ThrottleGroup is found with the given name a new one is - * created. - * - * This function edits throttle_groups and must be called under the global - * mutex. - * * @name: the name of the ThrottleGroup * @ret: the ThrottleState member of the ThrottleGroup */ ThrottleState *throttle_group_incref(const char *name) { - ThrottleGroup *tg =3D NULL; - - /* Look for an existing group with that name */ - tg =3D throttle_group_by_name(name); - - if (tg) { - object_ref(OBJECT(tg)); - } else { - /* Create a new one if not found */ - /* new ThrottleGroup obj will have a refcnt =3D 1 */ - tg =3D THROTTLE_GROUP(object_new(TYPE_THROTTLE_GROUP)); - tg->name =3D g_strdup(name); - throttle_group_obj_complete(USER_CREATABLE(tg), &error_abort); - } + ThrottleGroup *tg =3D throttle_group_by_name(name); =20 + assert(tg); + object_ref(OBJECT(tg)); return &tg->ts; } =20 @@ -145,8 +176,8 @@ ThrottleState *throttle_group_incref(const char *name) * When the reference count reaches zero the ThrottleGroup is * destroyed. * - * This function edits throttle_groups and must be called under the global - * mutex. + * This function edits the QOM root container and must be called under the + * global mutex. * * @ts: The ThrottleGroup to unref, given by its ThrottleState member */ @@ -154,6 +185,13 @@ void throttle_group_unref(ThrottleState *ts) { ThrottleGroup *tg =3D container_of(ts, ThrottleGroup, ts); object_unref(OBJECT(tg)); + /* ThrottleGroups will always have an extra reference from their conta= iner, + * so accessing it now is safe */ + if (tg->legacy && OBJECT(tg)->ref =3D=3D 1) { + tg->legacy =3D false; + /* Drop object created from legacy interface manually */ + user_creatable_del(tg->name, &error_abort); + } } =20 /* Get the name from a ThrottleGroupMember's group. The name (and the poin= ter) @@ -490,14 +528,10 @@ static void write_timer_cb(void *opaque) } =20 /* Register a ThrottleGroupMember from the throttling group, also initiali= zing - * its timers and updating its throttle_state pointer to point to it. If a - * throttling group with that name does not exist yet, it will be created. - * - * This function edits throttle_groups and must be called under the global - * mutex. + * its timers and updating its throttle_state pointer to point to it. * * @tgm: the ThrottleGroupMember to insert - * @groupname: the name of the group + * @groupname: the name of the group. It must already exist. * @ctx: the AioContext to use */ void throttle_group_register_tgm(ThrottleGroupMember *tgm, @@ -690,8 +724,6 @@ static ThrottleParamInfo properties[] =3D { } }; =20 -/* This function edits throttle_groups and must be called under the global - * mutex */ static void throttle_group_obj_init(Object *obj) { ThrottleGroup *tg =3D THROTTLE_GROUP(obj); @@ -702,49 +734,36 @@ static void throttle_group_obj_init(Object *obj) tg->clock_type =3D QEMU_CLOCK_VIRTUAL; } tg->is_initialized =3D false; + tg->legacy =3D false; qemu_mutex_init(&tg->lock); throttle_init(&tg->ts); QLIST_INIT(&tg->head); } =20 -/* This function edits throttle_groups and must be called under the global - * mutex */ static void throttle_group_obj_complete(UserCreatable *obj, Error **errp) { ThrottleGroup *tg =3D THROTTLE_GROUP(obj); ThrottleConfig cfg; =20 - /* set group name to object id if it exists */ + /* set group name to object id */ if (!tg->name && tg->parent_obj.parent) { tg->name =3D object_get_canonical_path_component(OBJECT(obj)); } /* We must have a group name at this point */ assert(tg->name); =20 - /* error if name is duplicate */ - if (throttle_group_exists(tg->name)) { - error_setg(errp, "A group with this name already exists"); - return; - } - /* check validity */ throttle_get_config(&tg->ts, &cfg); if (!throttle_is_valid(&cfg, errp)) { return; } throttle_config(&tg->ts, tg->clock_type, &cfg); - QTAILQ_INSERT_TAIL(&throttle_groups, tg, list); tg->is_initialized =3D true; } =20 -/* This function edits throttle_groups and must be called under the global - * mutex */ static void throttle_group_obj_finalize(Object *obj) { ThrottleGroup *tg =3D THROTTLE_GROUP(obj); - if (tg->is_initialized) { - QTAILQ_REMOVE(&throttle_groups, tg, list); - } qemu_mutex_destroy(&tg->lock); g_free(tg->name); } @@ -881,7 +900,7 @@ static void throttle_group_get_limits(Object *obj, Visi= tor *v, =20 static bool throttle_group_can_be_deleted(UserCreatable *uc, Error **errp) { - return OBJECT(uc)->ref =3D=3D 1; + return OBJECT(uc)->ref =3D=3D 1 && THROTTLE_GROUP(uc)->legacy =3D=3D f= alse; } =20 static void throttle_group_obj_class_init(ObjectClass *klass, void *class_= data) diff --git a/tests/test-throttle.c b/tests/test-throttle.c index eef2b1c707..927117ecab 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -609,6 +609,9 @@ static void test_groups(void) g_assert(tgm2->throttle_state =3D=3D NULL); g_assert(tgm3->throttle_state =3D=3D NULL); =20 + throttle_group_new_legacy("foo", &error_fatal); + throttle_group_new_legacy("bar", &error_fatal); + throttle_group_register_tgm(tgm1, "bar", blk_get_aio_context(blk1)); throttle_group_register_tgm(tgm2, "foo", blk_get_aio_context(blk2)); throttle_group_register_tgm(tgm3, "bar", blk_get_aio_context(blk3)); --=20 2.11.0 From nobody Mon Apr 29 01:57:13 2024 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 1503668029516554.7647309083684; Fri, 25 Aug 2017 06:33:49 -0700 (PDT) Received: from localhost ([::1]:53357 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEk8-0006QZ-7T for importer@patchew.org; Fri, 25 Aug 2017 09:33:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38441) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbV-00077u-JM for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbS-0003SX-CK for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:53 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbO-0003PO-8E; Fri, 25 Aug 2017 09:24:46 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDOD6x025245 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:13 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:31 +0300 Message-Id: <20170825132332.6734-7-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 6/7] block: remove BlockBackendPublic 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" All BlockBackend level throttling (via the implicit throttle filter node) is done in block/block-backend.c and block/throttle-groups.c doesn't know about BlockBackends anymore. Since BlockBackendPublic is not needed anymore= , remove it. Reviewed-by: Alberto Garcia Signed-off-by: Manos Pitsidianakis --- include/sysemu/block-backend.h | 12 +----------- block/block-backend.c | 43 +++++++++++++++++++-------------------= ---- block/qapi.c | 4 ++-- blockdev.c | 4 ++-- 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 4a7ca53685..a05d75fa5f 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -68,14 +68,6 @@ typedef struct BlockDevOps { void (*drained_end)(void *opaque); } BlockDevOps; =20 -/* This struct is embedded in (the private) BlockBackend struct and contai= ns - * fields that must be public. This is in particular for QLIST_ENTRY() and - * friends so that BlockBackends can be kept in lists outside block-backen= d.c - * */ -typedef struct BlockBackendPublic { - BlockDriverState *throttle_node; -} BlockBackendPublic; - BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm); BlockBackend *blk_new_open(const char *filename, const char *reference, QDict *options, int flags, Error **errp); @@ -90,9 +82,7 @@ BlockBackend *blk_all_next(BlockBackend *blk); bool monitor_add_blk(BlockBackend *blk, const char *name, Error **errp); void monitor_remove_blk(BlockBackend *blk); =20 -BlockBackendPublic *blk_get_public(BlockBackend *blk); -BlockBackend *blk_by_public(BlockBackendPublic *public); - +BlockDriverState *blk_get_throttle_node(BlockBackend *blk); BlockDriverState *blk_bs(BlockBackend *blk); void blk_remove_bs(BlockBackend *blk); int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp); diff --git a/block/block-backend.c b/block/block-backend.c index 65f458ce8f..c70e1c5cf7 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -37,7 +37,10 @@ struct BlockBackend { DriveInfo *legacy_dinfo; /* null unless created by drive_new() */ QTAILQ_ENTRY(BlockBackend) link; /* for block_backends */ QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends= */ - BlockBackendPublic public; + + /* implicit throttle filter node for backwards compatibility with lega= cy + * throttling commands */ + BlockDriverState *throttle_node; =20 void *dev; /* attached device model, if any */ bool legacy_dev; /* true if dev is not a DeviceState */ @@ -320,7 +323,7 @@ static void blk_delete(BlockBackend *blk) assert(!blk->refcnt); assert(!blk->name); assert(!blk->dev); - if (blk->public.throttle_node) { + if (blk->throttle_node) { blk_io_limits_disable(blk); } if (blk->root) { @@ -615,19 +618,11 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo) } =20 /* - * Returns a pointer to the publicly accessible fields of @blk. + * Returns the throttle_node field of @blk. */ -BlockBackendPublic *blk_get_public(BlockBackend *blk) +BlockDriverState *blk_get_throttle_node(BlockBackend *blk) { - return &blk->public; -} - -/* - * Returns a BlockBackend given the associated @public fields. - */ -BlockBackend *blk_by_public(BlockBackendPublic *public) -{ - return container_of(public, BlockBackend, public); + return blk->throttle_node; } =20 /* @@ -1920,15 +1915,15 @@ int blk_commit_all(void) /* throttling disk I/O limits */ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg) { - assert(blk->public.throttle_node); - throttle_group_config(throttle_get_tgm(blk->public.throttle_node), cfg= ); + assert(blk->throttle_node); + throttle_group_config(throttle_get_tgm(blk->throttle_node), cfg); } =20 void blk_io_limits_disable(BlockBackend *blk) { BlockDriverState *bs, *throttle_node; =20 - throttle_node =3D blk_get_public(blk)->throttle_node; + throttle_node =3D blk->throttle_node; =20 assert(throttle_node); =20 @@ -1944,7 +1939,7 @@ void blk_io_limits_disable(BlockBackend *blk) * blk, at this point it might have more than one parent, so use * bdrv_replace_node(). This destroys throttle_node */ bdrv_replace_node(throttle_node, bs, &error_abort); - blk_get_public(blk)->throttle_node =3D NULL; + blk->throttle_node =3D NULL; =20 bdrv_unref(bs); bdrv_drained_end(bs); @@ -1994,7 +1989,7 @@ void blk_io_limits_enable(BlockBackend *blk, const ch= ar *group, Error **errp) =20 end: bdrv_drained_end(bs); - blk_get_public(blk)->throttle_node =3D throttle_node; + blk->throttle_node =3D throttle_node; } =20 void blk_io_limits_update_group(BlockBackend *blk, const char *group, Erro= r **errp) @@ -2002,11 +1997,11 @@ void blk_io_limits_update_group(BlockBackend *blk, = const char *group, Error **er ThrottleGroupMember *tgm; =20 /* this BB is not part of any group */ - if (!blk->public.throttle_node) { + if (!blk->throttle_node) { return; } =20 - tgm =3D throttle_get_tgm(blk->public.throttle_node); + tgm =3D throttle_get_tgm(blk->throttle_node); /* this BB is a part of the same group than the one we want */ if (!g_strcmp0(throttle_group_get_name(tgm), group)) { return; @@ -2030,8 +2025,8 @@ static void blk_root_drained_begin(BdrvChild *child) =20 /* Note that blk->root may not be accessible here yet if we are just * attaching to a BlockDriverState that is drained. Use child instead.= */ - if (blk->public.throttle_node) { - tgm =3D throttle_get_tgm(blk->public.throttle_node); + if (blk->throttle_node) { + tgm =3D throttle_get_tgm(blk->throttle_node); if (atomic_fetch_inc(&tgm->io_limits_disabled) =3D=3D 0) { throttle_group_restart_tgm(tgm); } @@ -2044,8 +2039,8 @@ static void blk_root_drained_end(BdrvChild *child) BlockBackend *blk =3D child->opaque; assert(blk->quiesce_counter); =20 - if (blk->public.throttle_node) { - tgm =3D throttle_get_tgm(blk->public.throttle_node); + if (blk->throttle_node) { + tgm =3D throttle_get_tgm(blk->throttle_node); assert(tgm->io_limits_disabled); atomic_dec(&tgm->io_limits_disabled); } diff --git a/block/qapi.c b/block/qapi.c index ab55db7134..2be44a6758 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -66,9 +66,9 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, =20 info->detect_zeroes =3D bs->detect_zeroes; =20 - if (blk && blk_get_public(blk)->throttle_node) { + if (blk && blk_get_throttle_node(blk)) { ThrottleConfig cfg; - BlockDriverState *throttle_node =3D blk_get_public(blk)->throttle_= node; + BlockDriverState *throttle_node =3D blk_get_throttle_node(blk); ThrottleGroupMember *tgm =3D throttle_get_tgm(throttle_node); =20 throttle_group_get_config(tgm, &cfg); diff --git a/blockdev.c b/blockdev.c index 3f76eed2aa..df8316b1c3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2712,7 +2712,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, = Error **errp) if (throttle_enabled(&cfg)) { /* Enable I/O limits if they're not enabled yet, otherwise * just update the throttling group. */ - if (!blk_get_public(blk)->throttle_node) { + if (!blk_get_throttle_node(blk)) { blk_io_limits_enable(blk, arg->has_group ? arg->group : arg->has_device ? arg->device : arg->id, &local_err); @@ -2730,7 +2730,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, = Error **errp) } /* Set the new throttling configuration */ blk_set_io_limits(blk, &cfg); - } else if (blk_get_public(blk)->throttle_node) { + } else if (blk_get_throttle_node(blk)) { /* * If all throttling settings are set to 0, disable I/O limits * by deleting the legacy throttle node --=20 2.11.0 From nobody Mon Apr 29 01:57:13 2024 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 1503667764612821.7054245235604; Fri, 25 Aug 2017 06:29:24 -0700 (PDT) Received: from localhost ([::1]:53327 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEfr-0002QT-7E for importer@patchew.org; Fri, 25 Aug 2017 09:29:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38418) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbT-0006z5-I8 for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dlEbR-0003S8-P9 for qemu-devel@nongnu.org; Fri, 25 Aug 2017 09:24:51 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:40254) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dlEbO-0003PJ-4I; Fri, 25 Aug 2017 09:24:46 -0400 Received: from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v7PDOECj025252 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Aug 2017 16:24:14 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host carp0.noc.ntua.gr [147.102.222.60] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 25 Aug 2017 16:23:32 +0300 Message-Id: <20170825132332.6734-8-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170825132332.6734-1-el13635@mail.ntua.gr> References: <20170825132332.6734-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 7/7] qemu-iotests: add 191 for legacy throttling interface 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 , Alberto Garcia , Stefan Hajnoczi , qemu-block 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" Check that the implicit throttle filter driver node, used for compatibility with the legacy throttling interface on the BlockBackend level, works. Reviewed-by: Alberto Garcia Signed-off-by: Manos Pitsidianakis --- tests/qemu-iotests/191 | 138 +++++++++++++++++++++++++++++++++++++++++= ++++ tests/qemu-iotests/191.out | 5 ++ tests/qemu-iotests/group | 1 + 3 files changed, 144 insertions(+) create mode 100644 tests/qemu-iotests/191 create mode 100644 tests/qemu-iotests/191.out diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191 new file mode 100644 index 0000000000..82fd0b2fe5 --- /dev/null +++ b/tests/qemu-iotests/191 @@ -0,0 +1,138 @@ +#!/usr/bin/env python +# +# Tests that the legacy throttling interface using an implicit throttle fi= lter +# driver node works +# +# Copyright (C) 2017 Manos Pitsidianakis +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import iotests + +class TestLegacyThrottling(iotests.QMPTestCase): + test_img =3D os.path.join(iotests.test_dir, "test.img") + target_img =3D os.path.join(iotests.test_dir, "target.img") + base_img =3D os.path.join(iotests.test_dir, "base.img") + + def setUp(self): + iotests.qemu_img("create", "-f", iotests.imgfmt, self.base_img, "1= G") + iotests.qemu_img("create", "-f", iotests.imgfmt, self.test_img, "-= b", self.base_img) + iotests.qemu_io("-f", iotests.imgfmt, "-c", "write -P0x5d 1M 128M"= , self.test_img) + self.vm =3D iotests.VM().add_drive(self.test_img) + self.vm.launch() + + def tearDown(self): + self.do_check_throttle_node(expect=3DTrue) + params =3D {"device": "drive0", + "bps": 0, + "bps_rd": 0, + "bps_wr": 0, + "iops": 0, + "iops_rd": 0, + "iops_wr": 0, + } + """ + This must remove the implicit throttle_node + """ + result =3D self.vm.qmp("block_set_io_throttle", conv_keys=3DFalse, + **params) + self.do_check_throttle_node(expect=3DFalse) + self.vm.shutdown() + + def do_test_job(self, cmd, **args): + params =3D {"device": "drive0", + "bps": 1024, + "bps_rd": 0, + "bps_wr": 0, + "iops": 0, + "iops_rd": 0, + "iops_wr": 0, + } + result =3D self.vm.qmp("block_set_io_throttle", conv_keys=3DFalse, + **params) + self.assert_qmp(result, "return", {}) + result =3D self.vm.qmp(cmd, **args) + self.assert_qmp(result, "return", {}) + result =3D self.vm.qmp("query-block-jobs") + self.assert_qmp(result, "return[0]/device", "drive0") + + def do_check_throttle_node(self, expect): + result =3D self.vm.qmp("query-named-block-nodes") + for r in result["return"]: + if r["drv"] =3D=3D "throttle": + self.assertTrue(expect) + return + if expect: + """ throttle_node missing! """ + self.assertTrue(False) + + def do_check_params(self, file): + result =3D self.vm.qmp("query-block") + self.assert_qmp(result, "return[0]/inserted/bps", 1024) + self.assert_qmp(result, "return[0]/inserted/drv", iotests.imgfmt) + self.assert_qmp(result, "return[0]/inserted/file", file) + + """ + Check that query-block reports the correct throttling parameters while + ignoring the implicit throttle node. + """ + def test_query_block(self): + params =3D {"device": "drive0", + "bps": 1024, + "bps_rd": 0, + "bps_wr": 0, + "iops": 0, + "iops_rd": 0, + "iops_wr": 0, + } + result =3D self.vm.qmp("block_set_io_throttle", conv_keys=3DFalse, + **params) + self.assert_qmp(result, "return", {}) + self.do_check_params(file=3Dself.test_img) + + """ + Check that the throttle node doesn't get removed by block jobs, and th= at + query-block reports the correct throttling parameters + """ + def test_drive_mirror(self): + self.do_test_job("drive-mirror", device=3D"drive0", + target=3Dself.target_img, + sync=3D"full") + self.vm.event_wait("BLOCK_JOB_READY") + self.vm.qmp("block-job-complete", device=3D"drive0") + """ + query-block should report `target_img` now + """ + self.do_check_params(file=3Dself.target_img) + + def test_drive_backup(self): + self.do_test_job("drive-backup", device=3D"drive0", + target=3Dself.target_img, + sync=3D"full") + self.vm.event_wait("BLOCK_JOB_COMPLETED") + self.do_check_params(file=3Dself.test_img) + + def test_block_commit(self): + self.do_test_job("block-commit", device=3D"drive0") + self.vm.event_wait("BLOCK_JOB_READY") + self.vm.qmp("block-job-complete", device=3D"drive0") + """ + query-block should report the backing file `base_img` now + """ + self.do_check_params(file=3Dself.base_img) + +if __name__ =3D=3D "__main__": + iotests.main(supported_fmts=3D["qcow2"]) diff --git a/tests/qemu-iotests/191.out b/tests/qemu-iotests/191.out new file mode 100644 index 0000000000..89968f35d7 --- /dev/null +++ b/tests/qemu-iotests/191.out @@ -0,0 +1,5 @@ +.... +---------------------------------------------------------------------- +Ran 4 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index fee98440a5..ff732f0385 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -187,4 +187,5 @@ 188 rw auto quick 189 rw auto 190 rw auto quick +191 rw auto quick 192 rw auto quick --=20 2.11.0