From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40442) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aP9Ln-00085J-15 for qemu-devel@nongnu.org; Fri, 29 Jan 2016 08:44:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aP9Ll-0007l6-Ok for qemu-devel@nongnu.org; Fri, 29 Jan 2016 08:44:34 -0500 References: <1453917600-2663-1-git-send-email-mreitz@redhat.com> <1453917600-2663-13-git-send-email-mreitz@redhat.com> <20160128033348.GM7877@ad.usersys.redhat.com> From: Max Reitz Message-ID: <56AB6CB8.8000905@redhat.com> Date: Fri, 29 Jan 2016 14:44:24 +0100 MIME-Version: 1.0 In-Reply-To: <20160128033348.GM7877@ad.usersys.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="TKbhDntKHXSgQNN6VNxPDuFnlojvDI0eP" Subject: Re: [Qemu-devel] [PATCH v8 12/16] blockdev: Keep track of monitor-owned BDS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, John Snow , qemu-devel@nongnu.org, Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --TKbhDntKHXSgQNN6VNxPDuFnlojvDI0eP Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 28.01.2016 04:33, Fam Zheng wrote: > On Wed, 01/27 18:59, Max Reitz wrote: >> Signed-off-by: Max Reitz >> --- >> blockdev.c | 26 +++++++++++++++++++++++++= + >> include/block/block_int.h | 4 ++++ >> stubs/Makefile.objs | 1 + >> stubs/blockdev-close-all-bdrv-states.c | 5 +++++ >> 4 files changed, 36 insertions(+) >> create mode 100644 stubs/blockdev-close-all-bdrv-states.c >> >> diff --git a/blockdev.c b/blockdev.c >> index 09d4621..ac93f43 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -50,6 +50,9 @@ >> #include "trace.h" >> #include "sysemu/arch_init.h" >> =20 >> +static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =3D >> + QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states); >> + >> static const char *const if_name[IF_COUNT] =3D { >> [IF_NONE] =3D "none", >> [IF_IDE] =3D "ide", >> @@ -702,6 +705,19 @@ fail: >> return NULL; >> } >> =20 >> +void blockdev_close_all_bdrv_states(void) >> +{ >> + BlockDriverState *bs, *next_bs; >> + >> + QTAILQ_FOREACH_SAFE(bs, &monitor_bdrv_states, monitor_list, next_= bs) { >> + AioContext *ctx =3D bdrv_get_aio_context(bs); >> + >> + aio_context_acquire(ctx); >> + bdrv_unref(bs); >> + aio_context_release(ctx); >> + } >> +} >> + >> static void qemu_opt_rename(QemuOpts *opts, const char *from, const c= har *to, >> Error **errp) >> { >> @@ -3875,12 +3891,15 @@ void qmp_blockdev_add(BlockdevOptions *options= , Error **errp) >> if (!bs) { >> goto fail; >> } >> + >> + QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list); >> } >> =20 >> if (bs && bdrv_key_required(bs)) { >> if (blk) { >> blk_unref(blk); >> } else { >> + QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list); >> bdrv_unref(bs); >> } >> error_setg(errp, "blockdev-add doesn't support encrypted devi= ces"); >> @@ -3945,11 +3964,18 @@ void qmp_x_blockdev_del(bool has_id, const cha= r *id, >> bdrv_get_device_or_node_name(bs)); >> goto out; >> } >> + >> + if (!blk && !bs->monitor_list.tqe_prev) { >> + error_setg(errp, "Node %s is not owned by the monitor", >> + bs->node_name); >> + goto out; >> + } >=20 > Is this an extra restriction added by this patch? I hope not. This is just an additional check that should not change behavior; if it does, we did something wrong. > Deserve some words in= the > commit message? I'll see if I can come up with something. Max >> } >> =20 >> if (blk) { >> blk_unref(blk); >> } else { >> + QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list); >> bdrv_unref(bs); >> } >> =20 >> diff --git a/include/block/block_int.h b/include/block/block_int.h >> index 1e4c518..dd00d12 100644 >> --- a/include/block/block_int.h >> +++ b/include/block/block_int.h >> @@ -445,6 +445,8 @@ struct BlockDriverState { >> QTAILQ_ENTRY(BlockDriverState) device_list; >> /* element of the list of all BlockDriverStates (all_bdrv_states)= */ >> QTAILQ_ENTRY(BlockDriverState) bs_list; >> + /* element of the list of monitor-owned BDS */ >> + QTAILQ_ENTRY(BlockDriverState) monitor_list; >> QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; >> int refcnt; >> =20 >> @@ -707,4 +709,6 @@ bool bdrv_requests_pending(BlockDriverState *bs); >> void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);= >> void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *i= n); >> =20 >> +void blockdev_close_all_bdrv_states(void); >> + >> #endif /* BLOCK_INT_H */ >> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs >> index d7898a0..e922de9 100644 >> --- a/stubs/Makefile.objs >> +++ b/stubs/Makefile.objs >> @@ -1,5 +1,6 @@ >> stub-obj-y +=3D arch-query-cpu-def.o >> stub-obj-y +=3D bdrv-commit-all.o >> +stub-obj-y +=3D blockdev-close-all-bdrv-states.o >> stub-obj-y +=3D clock-warp.o >> stub-obj-y +=3D cpu-get-clock.o >> stub-obj-y +=3D cpu-get-icount.o >> diff --git a/stubs/blockdev-close-all-bdrv-states.c b/stubs/blockdev-c= lose-all-bdrv-states.c >> new file mode 100644 >> index 0000000..12d2442 >> --- /dev/null >> +++ b/stubs/blockdev-close-all-bdrv-states.c >> @@ -0,0 +1,5 @@ >> +#include "block/block_int.h" >> + >> +void blockdev_close_all_bdrv_states(void) >> +{ >> +} >> --=20 >> 2.7.0 >> --TKbhDntKHXSgQNN6VNxPDuFnlojvDI0eP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWq2y4AAoJEDuxQgLoOKytErkH/RuJt9brMvHnbCz6OOYy8lxb cbR/hJSo8s4hvHknE0v8eU63XvjSyeVoZb1Zn5wiGVqLpXvkIslASuHjF5YB7ILl hiqfhrsNaotvvjwzG6L2Bzkhzh2A4YLAMNx79/57FvY8CIStR2dUTEkULjoCQsYn 2RsWeWLvgpzsTSqiRZMRuzSc1R/8cWOEaJC/b0O0zhHpQoK1iF2QuU8IsyzXe7vw hcRdWf+Bec4FOxH4THfXiTTZ5Z2cZV8GRGG1SYHP8cUDUbr9QBBluPJQbi245i6B 17zkjAkW2+h3rzonGBj86YfJso2JDQEKUKncDF3z4f9Mfej3VMiHOMTE0DcMKMM= =0haL -----END PGP SIGNATURE----- --TKbhDntKHXSgQNN6VNxPDuFnlojvDI0eP--