Patchwork block: Change bdrv_eject() not to drop the image

login
register
mail settings
Submitter Markus Armbruster
Date July 27, 2010, 12:02 p.m.
Message ID <m3fwz5i152.fsf@blackfin.pond.sub.org>
Download mbox | patch
Permalink /patch/59996/
State New
Headers show

Comments

Markus Armbruster - July 27, 2010, 12:02 p.m.
bdrv_eject() gets called when a device model opens or closes the tray.

If the block driver implements method bdrv_eject(), that method gets
called.  Drivers host_cdrom implements it, and it opens and closes the
physical tray, and nothing else.  When a device model opens, then
closes the tray, media changes only if the user actively changes the
physical media while the tray is open.  This is matches how physical
hardware behaves.

If the block driver doesn't implement method bdrv_eject(), we do
something quite different: opening the tray severs the connection to
the image by calling bdrv_close(), and closing the tray does nothing.
When the device model opens, then closes the tray, media is gone,
unless the user actively inserts another one while the tray is open,
with a suitable change command in the monitor.  This isn't how
physical hardware behaves.  Rather inconvenient when programs
"helpfully" eject media to give you a chance to change it.  The way
bdrv_eject() behaves here turns that chance into a must, which is not
what these programs or their users expect.

Change the default action not to call bdrv_close().  Instead, note the
tray status in new BlockDriverState member tray_open.  Use it in
bdrv_is_inserted().

Arguably, the device models should keep track of tray status
themselves.  But this is less invasive.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c     |    7 ++++---
 block_int.h |    1 +
 2 files changed, 5 insertions(+), 3 deletions(-)
Kevin Wolf - July 28, 2010, 10:52 a.m.
Am 27.07.2010 14:02, schrieb Markus Armbruster:
> bdrv_eject() gets called when a device model opens or closes the tray.
> 
> If the block driver implements method bdrv_eject(), that method gets
> called.  Drivers host_cdrom implements it, and it opens and closes the
> physical tray, and nothing else.  When a device model opens, then
> closes the tray, media changes only if the user actively changes the
> physical media while the tray is open.  This is matches how physical
> hardware behaves.
> 
> If the block driver doesn't implement method bdrv_eject(), we do
> something quite different: opening the tray severs the connection to
> the image by calling bdrv_close(), and closing the tray does nothing.
> When the device model opens, then closes the tray, media is gone,
> unless the user actively inserts another one while the tray is open,
> with a suitable change command in the monitor.  This isn't how
> physical hardware behaves.  Rather inconvenient when programs
> "helpfully" eject media to give you a chance to change it.  The way
> bdrv_eject() behaves here turns that chance into a must, which is not
> what these programs or their users expect.
> 
> Change the default action not to call bdrv_close().  Instead, note the
> tray status in new BlockDriverState member tray_open.  Use it in
> bdrv_is_inserted().
> 
> Arguably, the device models should keep track of tray status
> themselves.  But this is less invasive.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Thanks, applied to the block branch.

Kevin

Patch

diff --git a/block.c b/block.c
index f837876..2f0ab28 100644
--- a/block.c
+++ b/block.c
@@ -2502,7 +2502,7 @@  int bdrv_is_inserted(BlockDriverState *bs)
     if (!drv)
         return 0;
     if (!drv->bdrv_is_inserted)
-        return 1;
+        return !bs->tray_open;
     ret = drv->bdrv_is_inserted(bs);
     return ret;
 }
@@ -2544,10 +2544,11 @@  int bdrv_eject(BlockDriverState *bs, int eject_flag)
         ret = drv->bdrv_eject(bs, eject_flag);
     }
     if (ret == -ENOTSUP) {
-        if (eject_flag)
-            bdrv_close(bs);
         ret = 0;
     }
+    if (ret >= 0) {
+        bs->tray_open = eject_flag;
+    }
 
     return ret;
 }
diff --git a/block_int.h b/block_int.h
index 96ff4cf..90e5029 100644
--- a/block_int.h
+++ b/block_int.h
@@ -141,6 +141,7 @@  struct BlockDriverState {
     int open_flags; /* flags used to open the file, re-used for re-open */
     int removable; /* if true, the media can be removed */
     int locked;    /* if true, the media cannot temporarily be ejected */
+    int tray_open; /* if true, the virtual tray is open */
     int encrypted; /* if true, the media is encrypted */
     int valid_key; /* if true, a valid encryption key has been set */
     int sg;        /* if true, the device is a /dev/sg* */