From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zpdar-0000ei-VP for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:45:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zpdaq-0000eW-Rr for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:45:21 -0400 Date: Fri, 23 Oct 2015 16:45:12 +0200 From: Kevin Wolf Message-ID: <20151023144512.GL3797@noname.redhat.com> References: <1445270025-22999-1-git-send-email-mreitz@redhat.com> <1445270025-22999-29-git-send-email-mreitz@redhat.com> <20151023132644.GC3797@noname.redhat.com> <562A437A.1040709@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3lcZGd9BuhuYXNfi" Content-Disposition: inline In-Reply-To: <562A437A.1040709@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 28/39] blockdev: Add blockdev-open-tray List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Alberto Garcia , qemu-block@nongnu.org, John Snow , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi --3lcZGd9BuhuYXNfi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 23.10.2015 um 16:26 hat Max Reitz geschrieben: > On 23.10.2015 15:26, Kevin Wolf wrote: > > Am 19.10.2015 um 17:53 hat Max Reitz geschrieben: > >> Signed-off-by: Max Reitz > >> --- > >> blockdev.c | 49 +++++++++++++++++++++++++++++++++++++++++++= ++++++ > >> qapi/block-core.json | 23 +++++++++++++++++++++++ > >> qmp-commands.hx | 39 +++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 111 insertions(+) > >=20 > >> + bs =3D blk_bs(blk); > >> + if (bs) { > >> + aio_context =3D bdrv_get_aio_context(bs); > >> + aio_context_acquire(aio_context); > >> + > >> + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { > >> + goto out; > >> + } > >=20 > > Is this blocker really for protecting against opening the tray? I think > > the BDS shouldn't care about whether the guest can access it. So it's > > probably more about preventing a bdrv_close() from happening, i.e. it > > would be relevant only for the blockdev-remove-medium command. >=20 > I don't think so. I intended blockdev-open-tray to be what it is for > real hardware: Opening the tray severs all the links between the medium > and software trying to access the drive. blockdev-remove-medium should > never fail if the tray is already open, since it would never fail in > real life. Comparison with real hardware works only so far. Real hardware doesn't have block jobs and will therefore never set the eject blocker. As I said, though, it's mostly protecting against bdrv_close(). Now that we don't call that any more, we don't strictly need the blocker any more in order to keep block jobs happy. However, we still need to prevent that the connection between BB and BDS is severed in case the old BDS was created implicitly and therefore would disappear from query-block while the image is still open and in use, which we don't want. This touches blockdev-del land more than op blockers, though... I think the eject op blocker can go. With your check, you prevent the user from opening the tray using QMP and then they can't get into a bad state with blockdev-remove-medium because without an opened tray that would fail. However, they could still eject the medium from within the guest and then use blockdev-remove-medium, which will get them in the bad state that we wanted to prevent. > By the way, that's the reason why I generally preferred > blk_is_available() over blk_is_inserted() in this series. I actually think this use is too restrictive in many cases (and in this patch opening the tray is pointlessly forbidden), but I didn't comment on it because we can fix it whenever someone needs more. Kevin --3lcZGd9BuhuYXNfi Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJWKkf4AAoJEH8JsnLIjy/Wp8QQAMcF+ULzaq7WBH4LB3xkcQ+a VSldZM95Phd2l2vE3umu4DjUSnto1irQrW+eETEpmGyf5SChLuYA/i7yBe7JxCQm YM5wo0IWETwm1asM/dacctx5UbKprTgRHhldoR5NrEdKHs1B895b2r2uUG4c0BJd qkqBV8O5kV1/EveSmM/AL8w2bozdg759OAdk+ghGspXZeJBaX1u+Ewy65akxLhqG 4Q5l2IvcLCSQzAahx4jBoDstq1qRufzskIDvnk0x+N/CXI25oMXk63PZr4s2Gdlc ZcOQv1ggtDzMjvfnd8mAaNJrXZ5zoeUiuXhHo7og2NFhc3Uugcl/LXNVMsmvWqgF Txk5ppTf9IZuUlnIh0n+0OyvyzEdiP503u61l6ldQTME2IOc0/Fx9ZJaiMKlgldU 2sPe6EsYyRYW+3Gemd1Q62KZGL3B/4aljg6sSrENh1Plfhs7MZcNlMFDyj17ymXr hdEtquMrGNMfD6sRRrED7Bym4/fM5fwGvzCS6PXJSh+jcw6yd8OTI00GeGA5jy38 Jwn1O3qTZMOtXKv19yoLngE4U52D4sq033oEdCuEgIbFdMekTny3Se00CEazAvVE yEHIZ7k1bTev6oto3T/squkvtFXJhKLc/RdTHGRFC5x45d1kdairRluxv/3kUzs6 Sr7CowcZu/mcURldjBRu =EBne -----END PGP SIGNATURE----- --3lcZGd9BuhuYXNfi--