From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33868) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsquZ-0002Y5-76 for qemu-devel@nongnu.org; Thu, 14 May 2015 07:02:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsquY-0005Gp-6y for qemu-devel@nongnu.org; Thu, 14 May 2015 07:02:43 -0400 Date: Thu, 14 May 2015 14:02:34 +0300 From: Dimitris Aragiorgis Message-ID: <20150514110234.GA2964@arr> References: <1431107242-31947-1-git-send-email-dimara@arrikto.com> <1431107242-31947-3-git-send-email-dimara@arrikto.com> <20150511101223.GD4962@noname.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="uAKRQypu60I7Lcqm" Content-Disposition: inline In-Reply-To: <20150511101223.GD4962@noname.redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/5] Fix migration in case of scsi-generic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pbonzini@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, stefanha@redhat.com --uAKRQypu60I7Lcqm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Kevin, * Kevin Wolf [2015-05-11 12:12:23 +0200]: > Am 08.05.2015 um 19:47 hat Dimitris Aragiorgis geschrieben: > > During migration, QEMU uses fsync()/fdatasync() on the open file > > descriptor for read-write block devices to flush data just before > > stopping the VM. > >=20 > > However, fsync() on a scsi-generic device returns -EINVAL which > > causes the migration to fail. This patch skips flushing data in case > > of an SG device, since submitting SCSI commands directly via an SG > > character device (e.g. /dev/sg0) bypasses the page cache completely, > > anyway. >=20 > fsync() doesn't only flush the kernel page cache, but also the disk > cache. The full justification includes at least that the scsi-generic > device never sends flushes, and for migration it's assumed that the same > SCSI device is used by the destination host (rather than another way to > access the same backing storage). If this were not enough, we would have > to send a SCSI flush command. >=20 If I understand correctly you mean that QEMU will not issue any SCSI SYNCHRONIZE CACHE (10) command and thus the disk cache does not get flushed during migration. This requires, as you say, that the same SCSI device is used by the destination host. Note taken. I have added your comment in the commit message of the corresponding patch in v3 just sent to the list. Thanks for clarifying this. Also note that QEMU seems to flush the disk cache explicitly in case of iSCSI, using iscsi_synchronizecache10_task() from libiscsi inside iscsi_co_flush(). Perhaps we can also extend this approach to scsi-generic e.g. by calling sg_ll_sync_cache_10() from libsgutils2. This is a distinct, orthogonal patch that could be added in the future. > On another note, I expect we still neglect to check bs_is_sg() in too > many places. Any monitor command that does normal I/O on such a BDS > should actually fail. >=20 > > Remove the bdrv_is_sg() test from iscsi_co_flush() since this is > > now redundant (we flush the underlying protocol at the end > > of bdrv_co_flush() which, with this patch, we never reach). > >=20 > > Signed-off-by: Dimitris Aragiorgis > > --- > > block/io.c | 2 +- > > block/iscsi.c | 4 ---- > > 2 files changed, 1 insertion(+), 5 deletions(-) > >=20 > > diff --git a/block/io.c b/block/io.c > > index 1ce62c4..d248a4d 100644 > > --- a/block/io.c > > +++ b/block/io.c > > @@ -2231,7 +2231,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *= bs) > > { > > int ret; > > =20 > > - if (!bs || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) { > > + if (!bs || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs) || bdrv_= is_sg(bs)) { >=20 > This line exceeds 80 characters. >=20 > Kevin --uAKRQypu60I7Lcqm Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAEBAgAGBQJVVIDKAAoJEHFDHex6CBG9GQIH/0OTFMbugqvpMVRHHh5b3RfO E1yDimKdJ4bywSRkR3A1uB9q0raQXjE8ngn8HGYR5lHegP6IrCNHR/J23N3vBj0c mVOo3Q0MxR8Ey9PHGUQi5iRv4j6F+8l6W7Qxpgnot4N1pMt8HMLtJPtCB+E1yRO9 BdYAN8TXuxbwH+4cHFfr5VXznLvD+YfQKiDQljb8Y5ATkgHEwWOb07p0by6j8tKV Deblm9uoHMvFsOnzYYu9iFkhMeNtHojBA4bLXihsBFSuo7jINO7leAIJ0nhFsGWp Gm7Gcjzr8TniSY5iaFnyTTy/zH6Z+qagqVRUCiWjRZlgcVbIivjSw2T8TpVNlO8= =2fqZ -----END PGP SIGNATURE----- --uAKRQypu60I7Lcqm--