Re: [Qemu-block] [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations

[Peter Maydell]

On 7 January 2016 at 20:14, Max Reitz <address@hidden> 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 instead
> of in the SDState. But that looks like the wrong place to me.

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.

> Right now, sd.c completely ignores the @load parameter of
> change_media_cb(), which seems wrong; this means that "opening the tray"
> 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.

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.

thanks
-- PMM