[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 28/39] blockdev: Add blockdev-open-tray
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v7 28/39] blockdev: Add blockdev-open-tray |
Date: |
Fri, 23 Oct 2015 16:45:12 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> >> ---
> >> blockdev.c | 49
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >> qapi/block-core.json | 23 +++++++++++++++++++++++
> >> qmp-commands.hx | 39 +++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 111 insertions(+)
> >
> >> + bs = blk_bs(blk);
> >> + if (bs) {
> >> + aio_context = bdrv_get_aio_context(bs);
> >> + aio_context_acquire(aio_context);
> >> +
> >> + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
> >> + goto out;
> >> + }
> >
> > 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.
>
> 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
pgpVGvyJ_De61.pgp
Description: PGP signature
- [Qemu-devel] [PATCH v7 22/39] block: Add blk_insert_bs(), (continued)
- [Qemu-devel] [PATCH v7 22/39] block: Add blk_insert_bs(), Max Reitz, 2015/10/19
- [Qemu-devel] [PATCH v7 20/39] block: Fail requests to empty BlockBackend, Max Reitz, 2015/10/19
- [Qemu-devel] [PATCH v7 24/39] blockdev: Do not create BDS for empty drive, Max Reitz, 2015/10/19
- [Qemu-devel] [PATCH v7 23/39] block: Prepare for NULL BDS, Max Reitz, 2015/10/19
- [Qemu-devel] [PATCH v7 25/39] blockdev: Pull out blockdev option extraction, Max Reitz, 2015/10/19
- [Qemu-devel] [PATCH v7 28/39] blockdev: Add blockdev-open-tray, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 27/39] block: Add blk_remove_bs(), Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 26/39] blockdev: Allow more options for BB-less BDS tree, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 30/39] blockdev: Add blockdev-remove-medium, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 31/39] blockdev: Add blockdev-insert-medium, Max Reitz, 2015/10/19