From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aX7s5-0001o0-Qo for qemu-devel@nongnu.org; Sat, 20 Feb 2016 08:46:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aX7s4-0006fp-Nx for qemu-devel@nongnu.org; Sat, 20 Feb 2016 08:46:53 -0500 References: <1455646106-2047-1-git-send-email-mreitz@redhat.com> <1455646106-2047-10-git-send-email-mreitz@redhat.com> <20160217155126.GH29494@noname.str.redhat.com> From: Max Reitz Message-ID: <56C86E41.6070700@redhat.com> Date: Sat, 20 Feb 2016 14:46:41 +0100 MIME-Version: 1.0 In-Reply-To: <20160217155126.GH29494@noname.str.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="LlNBn74QAk88wJuhkbLaw45ph7VsJtSKT" Subject: Re: [Qemu-devel] [PATCH v3 09/14] block: Move some bdrv_*_all() functions to BB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, qemu-block@nongnu.org, Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --LlNBn74QAk88wJuhkbLaw45ph7VsJtSKT Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 17.02.2016 16:51, Kevin Wolf wrote: > Am 16.02.2016 um 19:08 hat Max Reitz geschrieben: >> Move bdrv_commit_all() and bdrv_flush_all() to the BlockBackend level.= >> >> Signed-off-by: Max Reitz >> --- >> block.c | 20 ------------ >> block/block-backend.c | 44 ++++++++++++++++++= +++++---- >> block/io.c | 20 ------------ >> include/block/block.h | 2 -- >> stubs/Makefile.objs | 2 +- >> stubs/{bdrv-commit-all.c =3D> blk-commit-all.c} | 4 +-- >> 6 files changed, 41 insertions(+), 51 deletions(-) >> rename stubs/{bdrv-commit-all.c =3D> blk-commit-all.c} (53%) >> [...] >> diff --git a/block/block-backend.c b/block/block-backend.c >> index a10fe44..a918c35 100644 >> --- a/block/block-backend.c >> +++ b/block/block-backend.c [...] >> @@ -1361,5 +1356,42 @@ BlockBackendRootState *blk_get_root_state(Block= Backend *blk) >> =20 >> int blk_commit_all(void) >> { >> - return bdrv_commit_all(); >> + BlockBackend *blk =3D NULL; >> + >> + while ((blk =3D blk_all_next(blk)) !=3D NULL) { >> + AioContext *aio_context =3D blk_get_aio_context(blk); >> + >> + aio_context_acquire(aio_context); >> + if (blk_is_available(blk) && blk->bs->drv && blk->bs->backing= ) { >=20 > blk->bs->drv is already implied by blk_is_available(), so it's > unnecessary, even though it doesn't actively hurt. >=20 > Also, using blk_is_available() instead of blk_is_inserted() as in > blk_flush_all() means that a drive with an open tray, but inserted > medium isn't committed. I assume you did this on purpose because you're= > using two different functions in this patch, but isn't this a change in= > behaviour? If so, and we really want it, it should at least be mentione= d > in the commit message. Drives with open trays are strange. I found it reasonable that every medium in the system should be flushed on blk_flush_all(), because flushing data should only bring the on-disk state to a state it is supposed to be in anyway. On the other hand, the commit operation actively changes data. Therefore, I decided to treat it like a guest write operation. However, considering that blk_commit_all() is basically only available through HMP and is thus more of a legacy operation, I don't think its behavior should be changed here. I'm therefore going to change this to blk_is_inserted(), thanks. Max --LlNBn74QAk88wJuhkbLaw45ph7VsJtSKT 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 iQEcBAEBCAAGBQJWyG5BAAoJEDuxQgLoOKytyEcIAKVbhY+99/F5QvG7o7x+S42m syT5b7/6B2d6x0igZb5rOlPKG5DW8DdlQXGZwDi5Qe5JWnYg5I+P0/i8McQhGwM4 YL20Wu6A1/LxcW/VwfuRi11x8mG+8VpSPcuLwbpQbLslA7pyPCyUgdBvonXxXt2l RQ+zMT2uIdZpa1He0rN5jsqkPnjeFsdYYr+IvUGKPPxXCDTEfEFvEYiN7w2OPfkr 7D3gPzBWCVhCwf7zv+1z3WfRakYPdORG8qV4rKyVkJXv0ECt15X6AdXvub5zHDiy pIn/trKkCf9LvQFDjeTlP22cFFnTYMNE1bWlo4haC3/w0tHrnhVi6o78wdR2aEU= =/BXL -----END PGP SIGNATURE----- --LlNBn74QAk88wJuhkbLaw45ph7VsJtSKT--