diff mbox

[10/12] block: Fix virtual media change for if=none

Message ID 1277484812-22012-11-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster June 25, 2010, 4:53 p.m. UTC 
BlockDriverState member removable controls whether virtual media
change (monitor commands change, eject) is allowed.  It is set when
the "type hint" is BDRV_TYPE_CDROM or BDRV_TYPE_FLOPPY.

The type hint is only set by drive_init().  It sets BDRV_TYPE_FLOPPY
for if=floppy.  It sets BDRV_TYPE_CDROM for media=cdrom and if=ide,
scsi, xen, or none.

if=ide and if=scsi work, because the type hint makes it a CD-ROM.
if=xen likewise, I think.

For the same reason, if=none works when it's used by ide-drive or
scsi-disk.  For other guest devices, there are problems:

* fdc: you can't change virtual media

    $ qemu [...] -drive if=none,id=foo,... -global isa-fdc.driveA=foo
    QEMU 0.12.50 monitor - type 'help' for more information
    (qemu) eject foo
    Device 'foo' is not removable

  unless you add media=cdrom, but that makes it readonly.

* virtio: if you add media=cdrom, you can change virtual media.  If
  you eject, the guest gets I/O errors.  If you change, the guest sees
  the drive's contents suddenly change.

* scsi-generic: if you add media=cdrom, you can change virtual media.
  I didn't test what that does to the guest or the physical device,
  but it can't be pretty.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c           |    8 ++++++++
 block.h           |    1 +
 hw/fdc.c          |   10 ++++++++--
 hw/ide/core.c     |    1 +
 hw/scsi-disk.c    |    5 ++++-
 hw/scsi-generic.c |    1 +
 hw/virtio-blk.c   |    1 +
 7 files changed, 24 insertions(+), 3 deletions(-)

Comments

Christoph Hellwig June 26, 2010, 10:14 a.m. UTC | #1 
Looks good,


Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox

Patch

diff --git a/block.c b/block.c
index 34055e0..2ae4275 100644
--- a/block.c
+++ b/block.c
@@ -1292,6 +1292,14 @@  BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read)
     return is_read ? bs->on_read_error : bs->on_write_error;
 }
 
+void bdrv_set_removable(BlockDriverState *bs, int removable)
+{
+    bs->removable = removable;
+    if (removable && bs == bs_snapshots) {
+        bs_snapshots = NULL;
+    }
+}
+
 int bdrv_is_removable(BlockDriverState *bs)
 {
     return bs->removable;
diff --git a/block.h b/block.h
index 012c2a1..3d03b3e 100644
--- a/block.h
+++ b/block.h
@@ -162,6 +162,7 @@  int bdrv_get_translation_hint(BlockDriverState *bs);
 void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
                        BlockErrorAction on_write_error);
 BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
+void bdrv_set_removable(BlockDriverState *bs, int removable);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
diff --git a/hw/fdc.c b/hw/fdc.c
index 1496cfa..6c74878 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1847,10 +1847,16 @@  static void fdctrl_result_timer(void *opaque)
 static void fdctrl_connect_drives(FDCtrl *fdctrl)
 {
     unsigned int i;
+    FDrive *drive;
 
     for (i = 0; i < MAX_FD; i++) {
-        fd_init(&fdctrl->drives[i]);
-        fd_revalidate(&fdctrl->drives[i]);
+        drive = &fdctrl->drives[i];
+
+        fd_init(drive);
+        fd_revalidate(drive);
+        if (drive->bs) {
+            bdrv_set_removable(drive->bs, 1);
+        }
     }
 }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index cc4591b..ebdceb5 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2629,6 +2629,7 @@  void ide_init_drive(IDEState *s, BlockDriverState *bs,
         pstrcpy(s->version, sizeof(s->version), QEMU_VERSION);
     }
     ide_reset(s);
+    bdrv_set_removable(bs, s->is_cdrom);
 }
 
 static void ide_init1(IDEBus *bus, int unit)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 9c78979..2211245 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1049,6 +1049,7 @@  static void scsi_destroy(SCSIDevice *dev)
 static int scsi_disk_initfn(SCSIDevice *dev)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+    int is_cd;
     DriveInfo *dinfo;
 
     if (!s->qdev.conf.bs) {
@@ -1056,6 +1057,7 @@  static int scsi_disk_initfn(SCSIDevice *dev)
         return -1;
     }
     s->bs = s->qdev.conf.bs;
+    is_cd = bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM;
 
     if (!s->serial) {
         /* try to fall back to value set with legacy -drive serial=... */
@@ -1072,7 +1074,7 @@  static int scsi_disk_initfn(SCSIDevice *dev)
         return -1;
     }
 
-    if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+    if (is_cd) {
         s->qdev.blocksize = 2048;
     } else {
         s->qdev.blocksize = s->qdev.conf.logical_block_size;
@@ -1081,6 +1083,7 @@  static int scsi_disk_initfn(SCSIDevice *dev)
 
     s->qdev.type = TYPE_DISK;
     qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
+    bdrv_set_removable(s->bs, is_cd);
     return 0;
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 79347f4..3915e78 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -509,6 +509,7 @@  static int scsi_generic_initfn(SCSIDevice *dev)
     DPRINTF("block size %d\n", s->qdev.blocksize);
     s->driver_status = 0;
     memset(s->sensebuf, 0, sizeof(s->sensebuf));
+    bdrv_set_removable(s->bs, 0);
     return 0;
 }
 
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 71c5d21..f0b3ba5 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -500,6 +500,7 @@  VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
     qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
     register_savevm("virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
+    bdrv_set_removable(s->bs, 0);
 
     return &s->vdev;
 }