From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHIYb-0005D4-Ok for qemu-devel@nongnu.org; Thu, 07 Jan 2016 16:57:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHIYa-0007UP-JL for qemu-devel@nongnu.org; Thu, 07 Jan 2016 16:57:21 -0500 References: <1447164580-31094-1-git-send-email-kwolf@redhat.com> <1447164580-31094-11-git-send-email-kwolf@redhat.com> <568EBE85.2040608@redhat.com> <568EC721.3000009@redhat.com> From: Max Reitz Message-ID: <568EDF35.4080203@redhat.com> Date: Thu, 7 Jan 2016 22:57:09 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="x38JapwdIRuEorRdX46eb6UjI0fBViOOn" Subject: Re: [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Kevin Wolf , Peter Crosthwaite , QEMU Developers , Qemu-block This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --x38JapwdIRuEorRdX46eb6UjI0fBViOOn Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07.01.2016 22:42, Peter Maydell wrote: > On 7 January 2016 at 20:14, Max Reitz wrote: >> On 07.01.2016 20:56, Peter Maydell wrote: >>> It looks like sd.c is the only one which implements a change_media_cb= >>> but no is_tray_open, but it would be nice if we could implement this >>> in the default blk_dev_is_tray_open() method rather than in the >>> sd and floppy models (ie if I don't implement tray-open then the >>> tray is closed if there's a medium attached, and the block backend >>> ought to know if there's a medium attached itself already). >> >> That would be nice, but there's a difference between "there's a medium= >> attached" (tray can be both open and closed) and "the medium is >> accessible by the guest" (tray must be closed). The BlockBackend does >> not know this difference, only the guest devices does. >> >> It gets told of when to open/close the tray by invocation of the >> change_media_cb() (the @load parameter set to false or true, >> respectively), and we could track this state in the BlockBackend inste= ad >> of in the SDState. But that looks like the wrong place to me. >=20 > Well, previously sd.c didn't need to have any state for this > to all work right (or indeed care about implementing a fake > tray status for a device that doesn't have a tray), so it seems > odd that we need to invent some extra state for it to work now. Before, it took the state from the block layer by calling blk_is_inserted(). Works fine as long we only have the high-level operations change and eject. Stops to work once we introduce lower-level operations (open-tray, remove-medium, insert-medium, close-tray). Why do we need the low-level operations? Mainly because they integrate much better into the model of a more freely configurable block layer (there aren't many options one can give to the 'change' operation; now you can use blockdev-add and the lower-level operations afterwards). Why did I reimplement 'change' and 'eject' using these new operations? Because otherwise we'd have those operations doing one kind of thing, and the lower-level ones doing something completely different. I'd find that bad. >> Right now, sd.c completely ignores the @load parameter of >> change_media_cb(), which seems wrong; this means that "opening the tra= y" >> keeps the medium accessible for the guest. In the past that was fine, >> because eject closed the associated BDS (the medium) right afterwards,= >> so it actually wasn't accessible any more. The new >> blockdev-open-tray no longer does that, so now we need to respect the >> value of @load and no longer rely on blk_is_inserted() alone any more.= >=20 > I think blockdev-open-tray should probably not work for SD cards > (or for floppies?), because saying "open the tray" on a device > without a tray is meaningless. Depends on what you think it is. For me, blockdev-open-tray generally means "pushing the eject button". For actual tray devices, this means that the tray may open (or the OS may be asked to do so). For floppy disk drives, this means the floppy disk will be ejected. For SD slots (where there often is no dedicated button, but one can push the SD card further in for it to get released), this means the SD card will be ejected. Note that this is different from the 'eject' command which models a drive where the user pushes the eject button and suddenly the medium bursts into flames/is dropped on the floor/just disappears. Therefore, I find the 'eject' command quite misnamed, but having named the new command 'blockdev-eject-medium' wouldn't have made things any better. Max --x38JapwdIRuEorRdX46eb6UjI0fBViOOn 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 iQEcBAEBCAAGBQJWjt81AAoJEDuxQgLoOKyti/cIAIUyZ/EUzONzJNN2eofmL30D pEEBkimqlEBTibB3Z91H8QQEYiWvc5hSSA3fXCOtbD9GEuWy3JCX1nbOOEicr2N0 s4HdnhAEo+sCOKy0qv8qCR4Zzue8+7nHtjTXukL0fWg55mSO7pPYETNY/0TnVb1t Uq2tu7AfjthiSetRlsdWnaPbEiM+mmTjfVzdNbRl2osJPPgCvdvSwnYHR+OTpgmC QiC5FhypU5Zq71kyisRQR2qWUuR4q8l7Z7Yw50lQjzyMmZ6wfaoVXG3g0N35/bcs Zf7v+bbPmkwEwk5/lPyFQBQp5sHo00bkT8ohhkxU/lqZJV+ZGuEVoVvXcaGpq9Y= =9kST -----END PGP SIGNATURE----- --x38JapwdIRuEorRdX46eb6UjI0fBViOOn--