From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45530) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTsas-0002lK-6m for qemu-devel@nongnu.org; Thu, 11 Feb 2016 09:51:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTsap-0007Ge-0f for qemu-devel@nongnu.org; Thu, 11 Feb 2016 09:51:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33732) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTsao-0007GZ-Hr for qemu-devel@nongnu.org; Thu, 11 Feb 2016 09:51:38 -0500 Date: Thu, 11 Feb 2016 14:51:35 +0000 From: Stefan Hajnoczi Message-ID: <20160211145135.GB7285@stefanha-x1.localdomain> References: <20160210091259.10024.74767.stgit@PASHA-ISP> <20160210091323.10024.5684.stgit@PASHA-ISP> <20160211095605.GB4647@stefanha-x1.localdomain> <002c01d164d3$7a8b4680$6fa1d380$@Dovgaluk@ispras.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="A6N2fC+uXW/VQSAv" Content-Disposition: inline In-Reply-To: <002c01d164d3$7a8b4680$6fa1d380$@Dovgaluk@ispras.ru> Subject: Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk Cc: edgar.iglesias@xilinx.com, peter.maydell@linaro.org, igor.rubinov@gmail.com, mark.burton@greensocs.com, real@ispras.ru, hines@cert.org, qemu-devel@nongnu.org, maria.klimushenkova@ispras.ru, kwolf@redhat.com, pbonzini@redhat.com, batuzovk@ispras.ru, alex.bennee@linaro.org, fred.konrad@greensocs.com --A6N2fC+uXW/VQSAv Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 11, 2016 at 04:52:42PM +0300, Pavel Dovgalyuk wrote: > > From: Stefan Hajnoczi [mailto:stefanha@redhat.com] > > On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote: > > > @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk, > > > return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM); > > > } > > > > > > - return bdrv_aio_flush(blk->bs, cb, opaque); > > > + if (replay_mode =3D=3D REPLAY_MODE_NONE) { > > > + return bdrv_aio_flush(blk->bs, cb, opaque); > > > + } else { > > > + return replay_aio_flush(blk->bs, cb, opaque); > > > + } > > > } > >=20 > > I am only looking at this patch in isolation and am not familiar with > > the record/replay work, but these changes appear invasive. In order for > > record/replay to keep working in the future, everyone touching block > > layer code must be familiar with the principles of how record/replay > > works. This patch does not include documentation. >=20 > We've already discussed it with Kevin. > He proposes adding new block driver for record/replay. >=20 > > Can you point me to documentation that explains the how record replay > > works? >=20 > There is some information about it in docs/replay.txt and in > http://wiki.qemu.org/Features/record-replay I was thinking about developer documentation. A feature like this is "cross-cutting" - it affects many components. There should be documentation that explains the semantics so everyone modifying code can follow the rules to keep record/replay working. Live migration is similar in that it touches many components. The docs/migration.txt file explains the APIs and general idea. It communicates the assumptions, limitations, and requirements that live migration imposes. Many QEMU developers understand migration basics and keep them in mind during code review. This way we can prevent most (but not all) migration breakage. Record/replay should aim for the same level of education/awareness, otherwise the feature will break and bitrot. The alternative is to implement it in a way that isn't invasive. Then other developers don't need to understand how it works. Maybe implementing it in a block driver can achieve this. Stefan --A6N2fC+uXW/VQSAv Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWvJ/3AAoJEJykq7OBq3PIy3EH/jJidty1wYgnlbrqQw+9goI0 +ZPZXrvhyWvjVb5FnmreE8uazJtIkWBO/r3L644A6knf+V/LGkILFUZaKaVmvS+T UWB/MOY4Yspeio1Fs7He70u0VzdBKZlQxh+WubqmUTX3fJ131vvdpjcffCtISJ4N RRPXEih5Ju18UJfHBt+eAX5q1WjmO8eJmDZtHAF8xfSSnh/IxSAJmc57O4vz9VRG ANcHp8F8kMW9EMu83tl9pKpc32+glyVY5MdP8KUoXe+lS+BCCxd+9SDTL6UXKqcG IXED5oTXCfOct3CBflAyfVu6gf5ub0XqfD5iuWTyKfliYu5A7UYxXKiBMTa/kgQ= =I20R -----END PGP SIGNATURE----- --A6N2fC+uXW/VQSAv--