From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34372) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byctG-00028H-D0 for qemu-devel@nongnu.org; Mon, 24 Oct 2016 06:54:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byctF-0005nT-8I for qemu-devel@nongnu.org; Mon, 24 Oct 2016 06:54:02 -0400 References: <99a0ff7379a345ed60a6092866c9a42f667b37b5.1476450059.git.berto@igalia.com> <20161019171120.GH5336@noname.redhat.com> From: Paolo Bonzini Message-ID: <4aa7bd58-f80a-946b-f94d-65065943a8dd@redhat.com> Date: Mon, 24 Oct 2016 12:53:41 +0200 MIME-Version: 1.0 In-Reply-To: <20161019171120.GH5336@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bAwhARb3XKpVmsHOFRpT8umaoehtHhAS7" Subject: Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , Alberto Garcia Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Eric Blake , Max Reitz , Markus Armbruster , jsnow@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --bAwhARb3XKpVmsHOFRpT8umaoehtHhAS7 From: Paolo Bonzini To: Kevin Wolf , Alberto Garcia Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Eric Blake , Max Reitz , Markus Armbruster , jsnow@redhat.com Message-ID: <4aa7bd58-f80a-946b-f94d-65065943a8dd@redhat.com> Subject: Re: [PATCH v11 01/19] block: Add bdrv_drain_all_{begin,end}() References: <99a0ff7379a345ed60a6092866c9a42f667b37b5.1476450059.git.berto@igalia.com> <20161019171120.GH5336@noname.redhat.com> In-Reply-To: <20161019171120.GH5336@noname.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 19/10/2016 19:11, Kevin Wolf wrote: > Am 14.10.2016 um 15:08 hat Alberto Garcia geschrieben: >> bdrv_drain_all() doesn't allow the caller to do anything after all >> pending requests have been completed but before block jobs are >> resumed. >> >> This patch splits bdrv_drain_all() into _begin() and _end() for that >> purpose. It also adds aio_{disable,enable}_external() calls to disable= >> external clients in the meantime. >> >> Signed-off-by: Alberto Garcia >=20 > This looks okay as a first step, possibly enough for this series (we'll= > have to review this carefully), but it leaves us with a rather limited > version of bdrv_drain_all_begin/end that excludes many useful cases. On= e > of them is that John wants to use it around QMP transactions. >=20 > Specifically, you can't add a new BDS or a new block job in a drain_all= > section because then bdrv_drain_all_end() would try to unpause the new > thing even though it has never been paused. Depending on what else we > did with it, this will either corrupt the pause counters or just > directly fail an assertion. >=20 > My first thoughts were about how to let an unpause succeed without a > previous pause for these objects, but actually I think this isn't what > we should do. We rather want to actually do the pause instead because > even new BDSes and block jobs should probably start in a quiesced state= > when created inside a drain_all section. Yes, I agree with this. It shouldn't be too hard to implement it. It would require a BlockDriverState to look at the global "inside bdrv_drain_all_begin" state, and ask its BlockBackend to disable itself upon bdrv_replace_child. Basically, "foo->quiesce_counter" should become "foo->quiesce_counter || all_quiesce_counter", I think. It may well be done as a separate patch if there is a TODO comment in bdrv_replace_child; as Kevin said, there are assertions to protect against misuse. Paolo --bAwhARb3XKpVmsHOFRpT8umaoehtHhAS7 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 iQEcBAEBCAAGBQJYDeg2AAoJEL/70l94x66D/cQIAKm89wcoOJYsi+Wr1CBtkDoR j6y3hElPLQwn0KWguHhtTgYzoSB259JP7zOhyLeO9Lg7FCNRZSFZ9WgIGFAgb01J I0ehYyjeZvZGVPCXfZtpjxD+RPcmZEGltilBMmVwYScK9VyBjQzot1h/o/1iySLh an2aIl3h8gswPlxWmhlNpFuRtKAN2xwVOdLsho6t9MO7nxyW4GOJJHNJ8o1u0Lpl a6VkNfoAKuvi69kfMLDlg5jyFoVpDE5EQLzbUh3EsyZ98gI0K1Drf1XBfj0BUjSn Sbok74N1BJsGM0Mig1Nq6wz1QFl3NGjMhfSj5BYzgqGlXg+uyj/hUwdJFm+rU0w= =kvWk -----END PGP SIGNATURE----- --bAwhARb3XKpVmsHOFRpT8umaoehtHhAS7--