From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52066) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zvpeo-0002fJ-G1 for qemu-devel@nongnu.org; Mon, 09 Nov 2015 11:51:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zvpen-00033r-FH for qemu-devel@nongnu.org; Mon, 09 Nov 2015 11:51:02 -0500 References: <1446663467-22485-1-git-send-email-mreitz@redhat.com> <1446663467-22485-8-git-send-email-mreitz@redhat.com> <20151109160443.GG3621@noname.redhat.com> From: Max Reitz Message-ID: <5640CEEE.80609@redhat.com> Date: Mon, 9 Nov 2015 17:50:54 +0100 MIME-Version: 1.0 In-Reply-To: <20151109160443.GG3621@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="OrXBlmPvassMEos171X1GTwVC6LdU0dkj" Subject: Re: [Qemu-devel] [PATCH v6 07/15] block: Move BDS close notifiers into BB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Alberto Garcia , qemu-block@nongnu.org, qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --OrXBlmPvassMEos171X1GTwVC6LdU0dkj Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 09.11.2015 17:04, Kevin Wolf wrote: > Am 04.11.2015 um 19:57 hat Max Reitz geschrieben: >> The only remaining user of the BDS close notifiers is NBD which uses >> them to determine when a BDS tree is being ejected. This patch removes= >> the BDS-level close notifiers and adds a notifier list to the >> BlockBackend structure that is invoked whenever a BDS is removed. >> >> Symmetrically to that, another notifier list is added that is invoked >> whenever a BDS is inserted. The dataplane implementations for virtio-b= lk >> and virtio-scsi use both notifier types for setting up and removing op= >> blockers. This is not only important for setting up the op blockers on= >> insertion, but also for removing them on ejection since bdrv_delete() >> asserts that there are no op blockers set up. >> >> Signed-off-by: Max Reitz >=20 > I think this needs to be split into smaller patches: >=20 > 1. Add the new BlockBackend notifiers > 2. Use them in virtio-blk in order to fix... removable virtio-blk > devices, or what is it? > 3. Convert NBD > 4. Remove old close notifiers I'll do my best. >> block.c | 7 ---- >> block/block-backend.c | 19 +++++++--- >> blockdev-nbd.c | 37 +------------------- >> hw/block/dataplane/virtio-blk.c | 77 +++++++++++++++++++++++++++++++-= --------- >> hw/scsi/virtio-scsi.c | 59 +++++++++++++++++++++++++++++++ >> include/block/block.h | 1 - >> include/block/block_int.h | 2 -- >> include/hw/virtio/virtio-scsi.h | 10 ++++++ >> include/sysemu/block-backend.h | 3 +- >> nbd.c | 13 +++++++ >> 10 files changed, 159 insertions(+), 69 deletions(-) >=20 >> diff --git a/block/block-backend.c b/block/block-backend.c >> index 6f9309f..38580f7 100644 >> --- a/block/block-backend.c >> +++ b/block/block-backend.c >> @@ -48,6 +48,8 @@ struct BlockBackend { >> BlockdevOnError on_read_error, on_write_error; >> bool iostatus_enabled; >> BlockDeviceIoStatus iostatus; >> + >> + NotifierList remove_bs_notifiers, insert_bs_notifiers; >> }; >> =20 >> typedef struct BlockBackendAIOCB { >> @@ -98,6 +100,8 @@ BlockBackend *blk_new(const char *name, Error **err= p) >> blk =3D g_new0(BlockBackend, 1); >> blk->name =3D g_strdup(name); >> blk->refcnt =3D 1; >> + notifier_list_init(&blk->remove_bs_notifiers); >> + notifier_list_init(&blk->insert_bs_notifiers); >> QTAILQ_INSERT_TAIL(&blk_backends, blk, link); >> return blk; >> } >> @@ -343,6 +347,8 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBack= end *blk) >> */ >> void blk_remove_bs(BlockBackend *blk) >> { >> + notifier_list_notify(&blk->remove_bs_notifiers, blk); >> + >> blk_update_root_state(blk); >> =20 >> blk->bs->blk =3D NULL; >> @@ -359,6 +365,8 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverS= tate *bs) >> bdrv_ref(bs); >> blk->bs =3D bs; >> bs->blk =3D blk; >> + >> + notifier_list_notify(&blk->insert_bs_notifiers, blk); >> } >=20 > Do we want to notify on BB deletion, too? It's also some kind of remova= l > of a connection between BB and BDS. In other words, should blk_delete(= ) > call blk_remove_bs() rather than bdrv_unref()? >=20 > [ Edit: I see that's what the next patch does. Good. ] >=20 > Should blk_unref() also assert that the notifier list is empty? > Otherwise we would be leaking notifiers. You mean blk_delete()? I can do that, yes. Max --OrXBlmPvassMEos171X1GTwVC6LdU0dkj 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 iQEcBAEBCAAGBQJWQM7uAAoJEDuxQgLoOKytJMoH/2iU62c8RNocH7faVZbXLmI8 6edRQhygCWBFCRVYLlFKzfMn4KidojGN8rYAtASjqxE29Ye9cj5/klS5ymDGF2/3 KHXI88dIua/yeOMiKWyV0IE2Qp4wuLvgMC88uuI2gJ8FtI2Rn4JB64sGv4PlqTsT sUifsSWLvFGHrzghHsd3Gikmdswwu3s9nw0QcCII5V5XIvXLkSbFdPxz/ZKdQvY6 7NfdNx2qmlp0NBeup20/jzxBGX9ZrO9wOC41PxCeRJcyoPQFFSJSY1t8r5HFjovn zWTGGdULF7rbV4C+iHzXVv2brxzfluDwFo39h84hLWwXUbk42uhSSnVIJUf2Ojg= =yt0L -----END PGP SIGNATURE----- --OrXBlmPvassMEos171X1GTwVC6LdU0dkj--