From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49600) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yqdvh-0007G8-RN for qemu-devel@nongnu.org; Fri, 08 May 2015 04:46:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yqdvg-0001Cq-Pw for qemu-devel@nongnu.org; Fri, 08 May 2015 04:46:45 -0400 Date: Fri, 8 May 2015 09:46:37 +0100 From: Stefan Hajnoczi Message-ID: <20150508084637.GB11717@stefanha-thinkpad.redhat.com> References: <1430911419-8256-1-git-send-email-famz@redhat.com> <20150507134343.GM13985@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="8GpibOaaTibBMecb" Content-Disposition: inline In-Reply-To: <20150507134343.GM13985@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [RFC PATCH 0/7] Fix transactional snapshot with virtio-blk dataplane List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Fam Zheng , qemu-block@nongnu.org, armbru@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, pbonzini@redhat.com --8GpibOaaTibBMecb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, May 07, 2015 at 02:43:43PM +0100, Stefan Hajnoczi wrote: > On Wed, May 06, 2015 at 07:23:32PM +0800, Fam Zheng wrote: > > Reported by Paolo. > >=20 > > Unlike the iohandler in main loop, iothreads currently process the event > > notifier used as virtio-blk ioeventfd in all nested aio_poll. This is d= angerous > > without proper protection, because guest requests could sneak to block = layer > > where they mustn't. > >=20 > > For example, a QMP transaction may involve multiple bdrv_drain_all() in > > handling the list of AioContext it works on. If an aio_poll in one of t= he > > bdrv_drain_all() happens to process a guest VQ kick by dispatching the > > ioeventfd event, a new guest write is then submitted, and voila, the > > transaction semantics is violated. > >=20 > > This series avoids this problem by disabling virtio-blk handlers during > > bdrv_drain_all() and transactions. > >=20 > > Notes: > >=20 > > If the general approach is right, other transaction types could get the > > blockers similarly, in next revision. And some related bdrv_drain_all()= could > > also be changed to bdrv_drain(). > >=20 > > virtio-scsi-dataplane will be a bit more complicated, but still doable.= It > > would probably need one more interface abstraction between scsi-disk, s= csi-bus > > and virtio-scsi. > >=20 > > Although other devices don't have a pause/resume callback yet, the > > blk_check_request, which returns -EBUSY if "device io" op blocker is se= t, could > > hopefully cover most cases already. > >=20 > > Timers and block jobs also generate IO, but it should be fine as long a= s they > > don't change guest visible data, which is true AFAICT. > >=20 > >=20 > > Fam Zheng (7): > > block: Add op blocker type "device IO" > > block: Block "device IO" during bdrv_drain and bdrv_drain_all > > block: Add op blocker notifier list > > block-backend: Add blk_op_blocker_add_notifier > > virtio-blk: Move complete_request to 'ops' structure > > virtio-blk: Don't handle output when there is "device IO" op blocker > > blockdev: Add "device IO" op blocker during snapshot transaction > >=20 > > block.c | 20 ++++++++++++ > > block/block-backend.c | 10 ++++++ > > block/io.c | 12 +++++++ > > blockdev.c | 7 +++++ > > hw/block/dataplane/virtio-blk.c | 36 ++++++++++++++++++--- > > hw/block/virtio-blk.c | 69 +++++++++++++++++++++++++++++++++= ++++++-- > > include/block/block.h | 9 ++++++ > > include/block/block_int.h | 3 ++ > > include/hw/virtio/virtio-blk.h | 17 ++++++++-- > > include/sysemu/block-backend.h | 2 ++ > > 10 files changed, 174 insertions(+), 11 deletions(-) >=20 > Please also add a listener to nbd.c so NBD exports cannot submit I/O > during bdrv_drain_all(). qmp_transaction probably also needs this. Stefan --8GpibOaaTibBMecb Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVTHftAAoJEJykq7OBq3PIJasH/2XWkKcdUIAXLbGAfYSbM5U1 de5zn8CYQHdvUZ8vzsMSiHUINge8GJDnJInq4yOw14ckGFA03IXqVo+MmTzK4wzv SZNZrJKcJqAE2yLi9A1kJ3Oyjr0PcY1CsCnYdzcGwuYyV+6kaQqtOFPDvKnpAkZC D3Ap1fxl2QTVPL9ORpW9dZqd8FetGEjxWCyLyBtHdK28gyOXttACHlc9eJhIeQ91 xs7Y5axOGnB5u6XGnNCg9pHsM6Bf9TDvYujvmqgx19pYRFGmjWb/W/ZVMwPk4V4x suR/AQdYHhSef328UItKj52droPLoCl0DctdmCToe8S6dPR+iB5IqmE5DiZXgvU= =eJ// -----END PGP SIGNATURE----- --8GpibOaaTibBMecb--