[Qemu-devel] [PATCH v3 00/27] Block layer cleanup & fixes

[Qemu-devel] [PATCH v3 00/27] Block layer cleanup & fixes

[Markus Armbruster]

This patch series looks bigger than it is.  All the patches are small
and hopefully easy to review.

Objectives:

* Push BlockDriverState members locked, tray_open, media_changed into
  device models, where they belong.  Kevin picked the patches pushing
  media_changed from v2, so that part's gone already.

* BlockDriverState member removable is a confusing mess, replace it.

* Improve eject -f.

Also clean up minor messes as they get in the way.

It is based on Kevin's block branch.

Part I: Move tray state to device models
PATCH 01-05 IDE tray open/closed
PATCH 06-07 SCSI tray open/closed
PATCH 08-09 Kill BlockDriverState tray_open
PATCH 10-11 IDE & SCSI track tray lock
PATCH 12-14 Kill BlockDriverState locked
PATCH 15-16 IDE & SCSI tray bug fixes
PATCH 17    IDE migrate tray state

Part II: Miscellaneous
PATCH 18-19 Replace BlockDriverState removable
PATCH 20    Cover tray open/closed in info block
PATCH 21-25 Reduce unclean use of block_int.h
PATCH 26-27 Improve eject -f

Naturally, I want all parts applied.  But I did my best to make
applying only a prefix workable, too.

Review invited from:

* Kevin, Christoph and Amit reviewed previous versions.

* Hannes ACKed the SCSI stuff in v2.

* Luiz reviewed the patches that affect QMP's query-block.  I renamed
  response member "ejected" to "tray-open" since then.

* Paolo commented PATCH 17 `ide/atapi: Preserve tray state on
  migration'.

* Stefano reviewed v1 of PATCH 18 (affects "-drive if=xen").

Testing

* Linux installs from CD to empty disk, then boots fine from disk.

* For both IDE and SCSI:

  - info block reports tray state correctly

  - Guest locking the tray stops eject (without -f) and change

  - eject -f; change works even while tray is locked by guest

  - Reading /dev/sr0 with tray open behaves as before: IDE closes the
    tray and reads (matches bare metal), SCSI reports no medium

  - Tray state is migrated correctly (tested with savevm/loadvm)

* Guest still notices CD media change (IDE only, SCSI doesn't work
  before or after my patches because GESN isn't implemented)

* Migrating ide-cd to older version works when tray is closed and
  unlocked, else fails (tested with savevm/loadvm)


v3:

* Rebased to block branch cfc606da
  - Old PATCH 01-05,25,28-34,40 already there, drop
  - a couple of simple conflicts in hw/scsi-disk.c

* Drop old PATCH v2 27 scsi-disk: Preserve tray state on migration,
  because it doesn't make much sense without working SCSI migration.

* Drop old PATCH v2 22 ide/atapi: Avoid physical/virtual tray state
  mismatch, because it has a bug, how to best fix it isn't obvious,
  and it's not essential to this series.  Drop related PATCH v2 20,24,
  too.  I plan to revisit them later.

* Clean up `ide: Use a table to declare which drive kinds accept each
  command' a bit (Blue & Kevin)

* Hannes's advice:
  - Rename some SCSISense variables

* Kevin's advice:
  - Add comments to explain MMC-5 jargon loej
  - Make bdrv_lock_medium() parameter locked bool.

v2:

* Rebased to block branch; non-trivial conflicts:
  - Old PATCH 01-02,06-09 already there, drop
  - `block: Attach non-qdev devices as well':
    - cover new pci_piix3_xen_ide_unplug()
    - hw/nand has been qdefivied, drop hunk
    - onenand_init() changed, rewrite hunk
  - pci_piix3_xen_ide_unplug() needs new PATCH 33.

* Drop old PATCH 18 `scsi-disk: Reject CD-specific SCSI commands to
  disks' because Hannes wants to do it differently, and it's not
  essential to this series.

* Christoph's advice:
  - Rework `ide: Update command code definitions as per ACS-2'
  - Add comment to `ide: Fix ATA command READ to set ATAPI signature
    for CD-ROM'
  - Squash `ide/atapi: Track tray open/close state' and `ide/atapi:
    Switch from BlockDriverState's tray_open to own'
  - Squash `ide/atapi: Track tray locked state' and `ide/atapi: Switch
    from BlockDriverState's locked to own tray_locked'
  - Squash `scsi-disk: Track tray locked state' and `scsi-disk: Switch
    from BlockDriverState's locked to own tray_locked'
  - Drop `block: Move BlockDriverAIOCB & friends from block_int.h to
    block.h'

* Luiz's advice:
  - Change query-block to always include "ejected" for removable
    devices.  Requires moving `block: Show whether the guest ejected
    the medium in info block', which causes a bunch of conflicts.

* A few cosmetic improvements


Markus Armbruster (27):
  ide: Fix ATA command READ to set ATAPI signature for CD-ROM
  ide: Use a table to declare which drive kinds accept each command
  ide: Reject ATA commands specific to drive kinds
  ide/atapi: Clean up misleading name in cmd_start_stop_unit()
  ide/atapi: Track tray open/close state
  scsi-disk: Factor out scsi_disk_emulate_start_stop()
  scsi-disk: Track tray open/close state
  block: Revert entanglement of bdrv_is_inserted() with tray status
  block: Drop tray status tracking, no longer used
  ide/atapi: Track tray locked state
  scsi-disk: Track tray locked state
  block: Leave enforcing tray lock to device models
  block: Drop medium lock tracking, ask device models instead
  block: Rename bdrv_set_locked() to bdrv_lock_medium()
  ide/atapi: Don't fail eject when tray is already open
  scsi-disk: Fix START_STOP to fail when it can't eject
  ide/atapi: Preserve tray state on migration
  block: Clean up remaining users of "removable"
  block: Drop BlockDriverState member removable
  block: Show whether the virtual tray is open in info block
  block: Move BlockConf & friends from block_int.h to block.h
  hw: Trim superfluous #include "block_int.h"
  block: New bdrv_set_buffer_alignment()
  block: Reset buffer alignment on detach
  nbd: Clean up use of block_int.h
  block: New change_media_cb() parameter load
  ide/atapi scsi-disk: Make monitor eject -f, then change work

 block.c             |  104 ++++++++++++++++++---------------
 block.h             |   63 ++++++++++++++++++--
 block/nbd.c         |    1 +
 block/raw-posix.c   |    8 +-
 block/raw.c         |    6 +-
 block_int.h         |   40 +------------
 blockdev.c          |   10 +--
 hw/fdc.c            |    4 +-
 hw/ide/atapi.c      |   58 +++++++++----------
 hw/ide/cmd646.c     |    1 -
 hw/ide/core.c       |  160 +++++++++++++++++++++++++++++++++++++++++---------
 hw/ide/ich.c        |    1 -
 hw/ide/internal.h   |    3 +-
 hw/ide/isa.c        |    1 -
 hw/ide/macio.c      |    1 -
 hw/ide/microdrive.c |    1 -
 hw/ide/mmio.c       |    1 -
 hw/ide/pci.c        |    1 -
 hw/ide/via.c        |    1 -
 hw/lsi53c895a.c     |    1 -
 hw/scsi-bus.c       |   10 +++
 hw/scsi-disk.c      |   69 +++++++++++++++++++---
 hw/scsi-generic.c   |    1 -
 hw/scsi.h           |    5 +-
 hw/sd.c             |    2 +-
 hw/virtio-blk.c     |    3 +-
 hw/virtio.h         |    2 +-
 nbd.c               |    1 +
 nbd.h               |    2 -
 qmp-commands.hx     |    2 +
 trace-events        |    1 +
 31 files changed, 367 insertions(+), 197 deletions(-)

-- 
1.7.6

[Qemu-devel] [PATCH v3 01/27] ide: Fix ATA command READ to set ATAPI signature for CD-ROM

[Markus Armbruster]

Must set the ATAPI device signature, see ATA4 8.27.5.2 Outputs for
PACKET Command feature set devices, and ACS-2 7.36.6 Outputs for
PACKET feature set devices.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1806e00..def0126 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -983,8 +983,10 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 	lba48 = 1;
     case WIN_READ:
     case WIN_READ_ONCE:
-        if (!s->bs)
+        if (s->drive_kind == IDE_CD) {
+            ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */
             goto abort_cmd;
+        }
 	ide_cmd_lba48_transform(s, lba48);
         s->req_nb_sectors = 1;
         ide_sector_read(s);
-- 
1.7.6

[Qemu-devel] [PATCH v3 02/27] ide: Use a table to declare which drive kinds accept each command

[Markus Armbruster]

No functional change.

It would be nice to have handler functions in the table, like commit
e1a064f9 did for ATAPI.  Left for another day.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c |  105 +++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 80 insertions(+), 25 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index def0126..65ce289 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -901,6 +901,78 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
+#define HD_OK (1u << IDE_HD)
+#define CD_OK (1u << IDE_CD)
+#define CFA_OK (1u << IDE_CFATA)
+#define HD_CFA_OK (HD_OK | CFA_OK)
+#define ALL_OK (HD_OK | CD_OK | CFA_OK)
+
+/* See ACS-2 T13/2015-D Table B.2 Command codes */
+static const uint8_t ide_cmd_table[0x100] = {
+    /* NOP not implemented, mandatory for CD */
+    [CFA_REQ_EXT_ERROR_CODE]            = CFA_OK,
+    [WIN_DSM]                           = ALL_OK,
+    [WIN_DEVICE_RESET]                  = CD_OK,
+    [WIN_RECAL]                         = ALL_OK,
+    [WIN_READ]                          = ALL_OK,
+    [WIN_READ_ONCE]                     = ALL_OK,
+    [WIN_READ_EXT]                      = ALL_OK,
+    [WIN_READDMA_EXT]                   = ALL_OK,
+    [WIN_READ_NATIVE_MAX_EXT]           = ALL_OK,
+    [WIN_MULTREAD_EXT]                  = ALL_OK,
+    [WIN_WRITE]                         = ALL_OK,
+    [WIN_WRITE_ONCE]                    = ALL_OK,
+    [WIN_WRITE_EXT]                     = ALL_OK,
+    [WIN_WRITEDMA_EXT]                  = ALL_OK,
+    [CFA_WRITE_SECT_WO_ERASE]           = ALL_OK,
+    [WIN_MULTWRITE_EXT]                 = ALL_OK,
+    [WIN_WRITE_VERIFY]                  = ALL_OK,
+    [WIN_VERIFY]                        = ALL_OK,
+    [WIN_VERIFY_ONCE]                   = ALL_OK,
+    [WIN_VERIFY_EXT]                    = ALL_OK,
+    [WIN_SEEK]                          = HD_CFA_OK,
+    [CFA_TRANSLATE_SECTOR]              = CFA_OK,
+    [WIN_DIAGNOSE]                      = ALL_OK,
+    [WIN_SPECIFY]                       = ALL_OK,
+    [WIN_STANDBYNOW2]                   = ALL_OK,
+    [WIN_IDLEIMMEDIATE2]                = ALL_OK,
+    [WIN_STANDBY2]                      = ALL_OK,
+    [WIN_SETIDLE2]                      = ALL_OK,
+    [WIN_CHECKPOWERMODE2]               = ALL_OK,
+    [WIN_SLEEPNOW2]                     = ALL_OK,
+    [WIN_PACKETCMD]                     = CD_OK,
+    [WIN_PIDENTIFY]                     = CD_OK,
+    [WIN_SMART]                         = HD_CFA_OK,
+    [CFA_ACCESS_METADATA_STORAGE]       = CFA_OK,
+    [CFA_ERASE_SECTORS]                 = CFA_OK,
+    [WIN_MULTREAD]                      = ALL_OK,
+    [WIN_MULTWRITE]                     = ALL_OK,
+    [WIN_SETMULT]                       = ALL_OK,
+    [WIN_READDMA]                       = ALL_OK,
+    [WIN_READDMA_ONCE]                  = ALL_OK,
+    [WIN_WRITEDMA]                      = ALL_OK,
+    [WIN_WRITEDMA_ONCE]                 = ALL_OK,
+    [CFA_WRITE_MULTI_WO_ERASE]          = ALL_OK,
+    [WIN_STANDBYNOW1]                   = ALL_OK,
+    [WIN_IDLEIMMEDIATE]                 = ALL_OK,
+    [WIN_STANDBY]                       = ALL_OK,
+    [WIN_SETIDLE1]                      = ALL_OK,
+    [WIN_CHECKPOWERMODE1]               = ALL_OK,
+    [WIN_SLEEPNOW1]                     = ALL_OK,
+    [WIN_FLUSH_CACHE]                   = ALL_OK,
+    [WIN_FLUSH_CACHE_EXT]               = ALL_OK,
+    [WIN_IDENTIFY]                      = ALL_OK,
+    [WIN_SETFEATURES]                   = ALL_OK,
+    [IBM_SENSE_CONDITION]               = CFA_OK,
+    [CFA_WEAR_LEVEL]                    = CFA_OK,
+    [WIN_READ_NATIVE_MAX]               = ALL_OK,
+};
+
+static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
+{
+    return cmd <= ARRAY_SIZE(ide_cmd_table)
+        && (ide_cmd_table[cmd] & (1u << s->drive_kind));
+}
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val)
 {
@@ -920,6 +992,10 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
     if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
         return;
 
+    if (!ide_cmd_permitted(s, val)) {
+        goto abort_cmd;
+    }
+
     switch(val) {
     case WIN_DSM:
         switch (s->feature) {
@@ -1140,21 +1216,15 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_set_irq(s->bus);
         break;
     case WIN_SEEK:
-        if(s->drive_kind == IDE_CD)
-            goto abort_cmd;
         /* XXX: Check that seek is within bounds */
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
         break;
         /* ATAPI commands */
     case WIN_PIDENTIFY:
-        if (s->drive_kind == IDE_CD) {
-            ide_atapi_identify(s);
-            s->status = READY_STAT | SEEK_STAT;
-            ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
-        } else {
-            ide_abort_command(s);
-        }
+        ide_atapi_identify(s);
+        s->status = READY_STAT | SEEK_STAT;
+        ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
         ide_set_irq(s->bus);
         break;
     case WIN_DIAGNOSE:
@@ -1171,15 +1241,11 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_set_irq(s->bus);
         break;
     case WIN_DEVICE_RESET:
-        if (s->drive_kind != IDE_CD)
-            goto abort_cmd;
         ide_set_signature(s);
         s->status = 0x00; /* NOTE: READY is _not_ set */
         s->error = 0x01;
         break;
     case WIN_PACKETCMD:
-        if (s->drive_kind != IDE_CD)
-            goto abort_cmd;
         /* overlapping commands not supported */
         if (s->feature & 0x02)
             goto abort_cmd;
@@ -1191,16 +1257,12 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         break;
     /* CF-ATA commands */
     case CFA_REQ_EXT_ERROR_CODE:
-        if (s->drive_kind != IDE_CFATA)
-            goto abort_cmd;
         s->error = 0x09;    /* miscellaneous error */
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
         break;
     case CFA_ERASE_SECTORS:
     case CFA_WEAR_LEVEL:
-        if (s->drive_kind != IDE_CFATA)
-            goto abort_cmd;
         if (val == CFA_WEAR_LEVEL)
             s->nsector = 0;
         if (val == CFA_ERASE_SECTORS)
@@ -1210,8 +1272,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_set_irq(s->bus);
         break;
     case CFA_TRANSLATE_SECTOR:
-        if (s->drive_kind != IDE_CFATA)
-            goto abort_cmd;
         s->error = 0x00;
         s->status = READY_STAT | SEEK_STAT;
         memset(s->io_buffer, 0, 0x200);
@@ -1230,8 +1290,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_set_irq(s->bus);
         break;
     case CFA_ACCESS_METADATA_STORAGE:
-        if (s->drive_kind != IDE_CFATA)
-            goto abort_cmd;
         switch (s->feature) {
         case 0x02:	/* Inquiry Metadata Storage */
             ide_cfata_metadata_inquiry(s);
@@ -1250,8 +1308,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_set_irq(s->bus);
         break;
     case IBM_SENSE_CONDITION:
-        if (s->drive_kind != IDE_CFATA)
-            goto abort_cmd;
         switch (s->feature) {
         case 0x01:  /* sense temperature in device */
             s->nsector = 0x50;      /* +20 C */
@@ -1264,8 +1320,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         break;
 
     case WIN_SMART:
-	if (s->drive_kind == IDE_CD)
-		goto abort_cmd;
 	if (s->hcyl != 0xc2 || s->lcyl != 0x4f)
 		goto abort_cmd;
 	if (!s->smart_enabled && s->feature != SMART_ENABLE)
@@ -1420,6 +1474,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 	}
 	break;
     default:
+        /* should not be reachable */
     abort_cmd:
         ide_abort_command(s);
         ide_set_irq(s->bus);
-- 
1.7.6

[Qemu-devel] [PATCH v3 03/27] ide: Reject ATA commands specific to drive kinds

[Markus Armbruster]

ACS-2 Table B.2 explicitly prohibits ATAPI devices from implementing
WIN_RECAL, WIN_READ_EXT, WIN_READDMA_EXT, WIN_READ_NATIVE_MAX,
WIN_MULTREAD_EXT, WIN_WRITE, WIN_WRITE_ONCE, WIN_WRITE_EXT,
WIN_WRITEDMA_EXT, WIN_MULTWRITE_EXT, WIN_WRITE_VERIFY, WIN_VERIFY,
WIN_VERIFY_ONCE, WIN_VERIFY_EXT, WIN_SPECIFY, WIN_MULTREAD,
WIN_MULTWRITE, WIN_SETMULT, WIN_READDMA, WIN_READDMA_ONCE,
WIN_WRITEDMA, WIN_WRITEDMA_ONCE, WIN_FLUSH_CACHE_EXT.  Restrict them
to IDE_HD and IDE_CFATA.

Same for CFA_WRITE_SECT_WO_ERASE, CFA_WRITE_MULTI_WO_ERASE.  Restrict
them to IDE_CFATA, like the other CFA_ commands.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c |   50 +++++++++++++++++++++++++-------------------------
 1 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 65ce289..34b7b83 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -913,27 +913,27 @@ static const uint8_t ide_cmd_table[0x100] = {
     [CFA_REQ_EXT_ERROR_CODE]            = CFA_OK,
     [WIN_DSM]                           = ALL_OK,
     [WIN_DEVICE_RESET]                  = CD_OK,
-    [WIN_RECAL]                         = ALL_OK,
+    [WIN_RECAL]                         = HD_CFA_OK,
     [WIN_READ]                          = ALL_OK,
     [WIN_READ_ONCE]                     = ALL_OK,
-    [WIN_READ_EXT]                      = ALL_OK,
-    [WIN_READDMA_EXT]                   = ALL_OK,
-    [WIN_READ_NATIVE_MAX_EXT]           = ALL_OK,
-    [WIN_MULTREAD_EXT]                  = ALL_OK,
-    [WIN_WRITE]                         = ALL_OK,
-    [WIN_WRITE_ONCE]                    = ALL_OK,
-    [WIN_WRITE_EXT]                     = ALL_OK,
-    [WIN_WRITEDMA_EXT]                  = ALL_OK,
-    [CFA_WRITE_SECT_WO_ERASE]           = ALL_OK,
-    [WIN_MULTWRITE_EXT]                 = ALL_OK,
-    [WIN_WRITE_VERIFY]                  = ALL_OK,
-    [WIN_VERIFY]                        = ALL_OK,
-    [WIN_VERIFY_ONCE]                   = ALL_OK,
-    [WIN_VERIFY_EXT]                    = ALL_OK,
+    [WIN_READ_EXT]                      = HD_CFA_OK,
+    [WIN_READDMA_EXT]                   = HD_CFA_OK,
+    [WIN_READ_NATIVE_MAX_EXT]           = HD_CFA_OK,
+    [WIN_MULTREAD_EXT]                  = HD_CFA_OK,
+    [WIN_WRITE]                         = HD_CFA_OK,
+    [WIN_WRITE_ONCE]                    = HD_CFA_OK,
+    [WIN_WRITE_EXT]                     = HD_CFA_OK,
+    [WIN_WRITEDMA_EXT]                  = HD_CFA_OK,
+    [CFA_WRITE_SECT_WO_ERASE]           = CFA_OK,
+    [WIN_MULTWRITE_EXT]                 = HD_CFA_OK,
+    [WIN_WRITE_VERIFY]                  = HD_CFA_OK,
+    [WIN_VERIFY]                        = HD_CFA_OK,
+    [WIN_VERIFY_ONCE]                   = HD_CFA_OK,
+    [WIN_VERIFY_EXT]                    = HD_CFA_OK,
     [WIN_SEEK]                          = HD_CFA_OK,
     [CFA_TRANSLATE_SECTOR]              = CFA_OK,
     [WIN_DIAGNOSE]                      = ALL_OK,
-    [WIN_SPECIFY]                       = ALL_OK,
+    [WIN_SPECIFY]                       = HD_CFA_OK,
     [WIN_STANDBYNOW2]                   = ALL_OK,
     [WIN_IDLEIMMEDIATE2]                = ALL_OK,
     [WIN_STANDBY2]                      = ALL_OK,
@@ -945,14 +945,14 @@ static const uint8_t ide_cmd_table[0x100] = {
     [WIN_SMART]                         = HD_CFA_OK,
     [CFA_ACCESS_METADATA_STORAGE]       = CFA_OK,
     [CFA_ERASE_SECTORS]                 = CFA_OK,
-    [WIN_MULTREAD]                      = ALL_OK,
-    [WIN_MULTWRITE]                     = ALL_OK,
-    [WIN_SETMULT]                       = ALL_OK,
-    [WIN_READDMA]                       = ALL_OK,
-    [WIN_READDMA_ONCE]                  = ALL_OK,
-    [WIN_WRITEDMA]                      = ALL_OK,
-    [WIN_WRITEDMA_ONCE]                 = ALL_OK,
-    [CFA_WRITE_MULTI_WO_ERASE]          = ALL_OK,
+    [WIN_MULTREAD]                      = HD_CFA_OK,
+    [WIN_MULTWRITE]                     = HD_CFA_OK,
+    [WIN_SETMULT]                       = HD_CFA_OK,
+    [WIN_READDMA]                       = HD_CFA_OK,
+    [WIN_READDMA_ONCE]                  = HD_CFA_OK,
+    [WIN_WRITEDMA]                      = HD_CFA_OK,
+    [WIN_WRITEDMA_ONCE]                 = HD_CFA_OK,
+    [CFA_WRITE_MULTI_WO_ERASE]          = CFA_OK,
     [WIN_STANDBYNOW1]                   = ALL_OK,
     [WIN_IDLEIMMEDIATE]                 = ALL_OK,
     [WIN_STANDBY]                       = ALL_OK,
@@ -960,7 +960,7 @@ static const uint8_t ide_cmd_table[0x100] = {
     [WIN_CHECKPOWERMODE1]               = ALL_OK,
     [WIN_SLEEPNOW1]                     = ALL_OK,
     [WIN_FLUSH_CACHE]                   = ALL_OK,
-    [WIN_FLUSH_CACHE_EXT]               = ALL_OK,
+    [WIN_FLUSH_CACHE_EXT]               = HD_CFA_OK,
     [WIN_IDENTIFY]                      = ALL_OK,
     [WIN_SETFEATURES]                   = ALL_OK,
     [IBM_SENSE_CONDITION]               = CFA_OK,
-- 
1.7.6

[Qemu-devel] [PATCH v3 04/27] ide/atapi: Clean up misleading name in cmd_start_stop_unit()

[Markus Armbruster]

"eject" is misleading; it means "eject" when start is clear, but
"load" when start is set.  Rename to loej, because that's how MMC-5
calls it, in section 6.40.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/atapi.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f38d289..cb0cdac 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -903,11 +903,11 @@ static void cmd_seek(IDEState *s, uint8_t* buf)
 
 static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
 {
-    int start, eject, sense, err = 0;
-    start = buf[4] & 1;
-    eject = (buf[4] >> 1) & 1;
+    int sense, err = 0;
+    bool start = buf[4] & 1;
+    bool loej = buf[4] & 2;     /* load on start, eject on !start */
 
-    if (eject) {
+    if (loej) {
         err = bdrv_eject(s->bs, !start);
     }
 
-- 
1.7.6

[Qemu-devel] [PATCH v3 05/27] ide/atapi: Track tray open/close state

[Markus Armbruster]

We already track it in BlockDriverState since commit 4be9762a.  As
discussed in that commit's message, we should track it in the device
device models instead, because it's device state.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/atapi.c    |    6 +++++-
 hw/ide/internal.h |    1 +
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index cb0cdac..713b1fd 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -521,7 +521,7 @@ static unsigned int event_status_media(IDEState *s,
     uint8_t event_code, media_status;
 
     media_status = 0;
-    if (s->bs->tray_open) {
+    if (s->tray_open) {
         media_status = MS_TRAY_OPEN;
     } else if (bdrv_is_inserted(s->bs)) {
         media_status = MS_MEDIA_PRESENT;
@@ -926,6 +926,10 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
         ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
         break;
     }
+
+    if (loej && !err) {
+        s->tray_open = !start;
+    }
 }
 
 static void cmd_mechanism_status(IDEState *s, uint8_t* buf)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 1117852..8a9052e 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -442,6 +442,7 @@ struct IDEState {
     struct unreported_events events;
     uint8_t sense_key;
     uint8_t asc;
+    bool tray_open;
     uint8_t cdrom_changed;
     int packet_transfer_size;
     int elementary_transfer_size;
-- 
1.7.6

[Qemu-devel] [PATCH v3 06/27] scsi-disk: Factor out scsi_disk_emulate_start_stop()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-disk.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 9724d0f..c8ad2e7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -814,6 +814,18 @@ static int scsi_disk_emulate_read_toc(SCSIRequest *req, uint8_t *outbuf)
     return toclen;
 }
 
+static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
+{
+    SCSIRequest *req = &r->req;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+    bool start = req->cmd.buf[4] & 1;
+    bool loej = req->cmd.buf[4] & 2; /* load on start, eject on !start */
+
+    if (s->qdev.type == TYPE_ROM && loej) {
+        bdrv_eject(s->bs, !start);
+    }
+}
+
 static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
 {
     SCSIRequest *req = &r->req;
@@ -859,10 +871,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
             goto illegal_request;
         break;
     case START_STOP:
-        if (s->qdev.type == TYPE_ROM && (req->cmd.buf[4] & 2)) {
-            /* load/eject medium */
-            bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
-        }
+        scsi_disk_emulate_start_stop(r);
         break;
     case ALLOW_MEDIUM_REMOVAL:
         bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
-- 
1.7.6

[Qemu-devel] [PATCH v3 07/27] scsi-disk: Track tray open/close state

[Markus Armbruster]

We already track it in BlockDriverState since commit 4be9762a.  As
discussed in that commit's message, we should track it in the device
device models instead, because it's device state.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-disk.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c8ad2e7..f18ddd7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -72,6 +72,7 @@ struct SCSIDiskState
     QEMUBH *bh;
     char *version;
     char *serial;
+    bool tray_open;
 };
 
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -823,6 +824,7 @@ static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
 
     if (s->qdev.type == TYPE_ROM && loej) {
         bdrv_eject(s->bs, !start);
+        s->tray_open = !start;
     }
 }
 
-- 
1.7.6

[Qemu-devel] [PATCH v3 08/27] block: Revert entanglement of bdrv_is_inserted() with tray status

[Markus Armbruster]

Commit 4be9762a changed bdrv_is_inserted() to fail when the tray is
open.  Unfortunately, there are two different kinds of users, with
conflicting needs.

1. Device models using bdrv_eject(), currently ide-cd and scsi-cd.
They expect bdrv_is_inserted() to reflect the tray status.  Commit
4be9762a makes them happy.

2. Code that wants to know whether a BlockDriverState has media, such
as find_image_format(), bdrv_flush_all().  Commit 4be9762a makes them
unhappy.  In particular, it breaks flush on VM stop for media ejected
by the guest.

Revert the change to bdrv_is_inserted().  Check the tray status in the
device models instead.

Note on IDE: Since only ATAPI devices have a tray, and they don't
accept ATA commands since the recent commit "ide: Reject ATA commands
specific to drive kinds", checking in atapi.c suffices.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c        |    7 +++----
 hw/ide/atapi.c |   15 ++++++++-------
 hw/scsi-disk.c |   10 ++++++++--
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index a8c789a..6fe8add 100644
--- a/block.c
+++ b/block.c
@@ -3026,13 +3026,12 @@ static int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs)
 int bdrv_is_inserted(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
-    int ret;
+
     if (!drv)
         return 0;
     if (!drv->bdrv_is_inserted)
-        return !bs->tray_open;
-    ret = drv->bdrv_is_inserted(bs);
-    return ret;
+        return 1;
+    return drv->bdrv_is_inserted(bs);
 }
 
 /**
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 713b1fd..0943f66 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -73,7 +73,7 @@ static void lba_to_msf(uint8_t *buf, int lba)
 
 static inline int media_present(IDEState *s)
 {
-    return (s->nb_sectors > 0);
+    return !s->tray_open && s->nb_sectors > 0;
 }
 
 /* XXX: DVDs that could fit on a CD will be reported as a CD */
@@ -1077,20 +1077,21 @@ static const struct {
     [ 0x03 ] = { cmd_request_sense,                 ALLOW_UA },
     [ 0x12 ] = { cmd_inquiry,                       ALLOW_UA },
     [ 0x1a ] = { cmd_mode_sense, /* (6) */          0 },
-    [ 0x1b ] = { cmd_start_stop_unit,               0 },
+    [ 0x1b ] = { cmd_start_stop_unit,               0 }, /* [1] */
     [ 0x1e ] = { cmd_prevent_allow_medium_removal,  0 },
     [ 0x25 ] = { cmd_read_cdvd_capacity,            CHECK_READY },
-    [ 0x28 ] = { cmd_read, /* (10) */               0 },
+    [ 0x28 ] = { cmd_read, /* (10) */               CHECK_READY },
     [ 0x2b ] = { cmd_seek,                          CHECK_READY },
     [ 0x43 ] = { cmd_read_toc_pma_atip,             CHECK_READY },
     [ 0x46 ] = { cmd_get_configuration,             ALLOW_UA },
     [ 0x4a ] = { cmd_get_event_status_notification, ALLOW_UA },
     [ 0x5a ] = { cmd_mode_sense, /* (10) */         0 },
-    [ 0xa8 ] = { cmd_read, /* (12) */               0 },
-    [ 0xad ] = { cmd_read_dvd_structure,            0 },
+    [ 0xa8 ] = { cmd_read, /* (12) */               CHECK_READY },
+    [ 0xad ] = { cmd_read_dvd_structure,            CHECK_READY },
     [ 0xbb ] = { cmd_set_speed,                     0 },
     [ 0xbd ] = { cmd_mechanism_status,              0 },
-    [ 0xbe ] = { cmd_read_cd,                       0 },
+    [ 0xbe ] = { cmd_read_cd,                       CHECK_READY },
+    /* [1] handler detects and reports not ready condition itself */
 };
 
 void ide_atapi_cmd(IDEState *s)
@@ -1126,7 +1127,7 @@ void ide_atapi_cmd(IDEState *s)
      * GET_EVENT_STATUS_NOTIFICATION to detect such tray open/close
      * states rely on this behavior.
      */
-    if (bdrv_is_inserted(s->bs) && s->cdrom_changed) {
+    if (!s->tray_open && bdrv_is_inserted(s->bs) && s->cdrom_changed) {
         ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
 
         s->cdrom_changed = 0;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f18ddd7..f35ada4 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -183,6 +183,9 @@ static void scsi_read_data(SCSIRequest *req)
     if (n > SCSI_DMA_BUF_SIZE / 512)
         n = SCSI_DMA_BUF_SIZE / 512;
 
+    if (s->tray_open) {
+        scsi_read_complete(r, -ENOMEDIUM);
+    }
     r->iov.iov_len = n * 512;
     qemu_iovec_init_external(&r->qiov, &r->iov, 1);
 
@@ -281,6 +284,9 @@ static void scsi_write_data(SCSIRequest *req)
 
     n = r->iov.iov_len / 512;
     if (n) {
+        if (s->tray_open) {
+            scsi_write_complete(r, -ENOMEDIUM);
+        }
         qemu_iovec_init_external(&r->qiov, &r->iov, 1);
 
         bdrv_acct_start(s->bs, &r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_WRITE);
@@ -837,7 +843,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
 
     switch (req->cmd.buf[0]) {
     case TEST_UNIT_READY:
-        if (!bdrv_is_inserted(s->bs))
+        if (s->tray_open || !bdrv_is_inserted(s->bs))
             goto not_ready;
         break;
     case INQUIRY:
@@ -957,7 +963,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
     return buflen;
 
 not_ready:
-    if (!bdrv_is_inserted(s->bs)) {
+    if (s->tray_open || !bdrv_is_inserted(s->bs)) {
         scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
     } else {
         scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
-- 
1.7.6

[Qemu-devel] [PATCH v3 09/27] block: Drop tray status tracking, no longer used

[Markus Armbruster]

Commit 4be9762a is now completely redone.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c     |    1 -
 block_int.h |    1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 6fe8add..8c86251 100644
--- a/block.c
+++ b/block.c
@@ -3062,7 +3062,6 @@ int bdrv_eject(BlockDriverState *bs, int eject_flag)
     if (drv && drv->bdrv_eject) {
         drv->bdrv_eject(bs, eject_flag);
     }
-    bs->tray_open = eject_flag;
     return 0;
 }
 
diff --git a/block_int.h b/block_int.h
index 5dc0074..b63c57b 100644
--- a/block_int.h
+++ b/block_int.h
@@ -157,7 +157,6 @@ 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* */
-- 
1.7.6

[Qemu-devel] [PATCH v3 10/27] ide/atapi: Track tray locked state

[Markus Armbruster]

We already track it in BlockDriverState.  Just like tray open/close
state, we should track it in the device models instead, because it's
device state.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/atapi.c    |    4 +++-
 hw/ide/internal.h |    1 +
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 0943f66..d9db6de 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -788,8 +788,9 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
             buf[12] = 0x71;
             buf[13] = 3 << 5;
             buf[14] = (1 << 0) | (1 << 3) | (1 << 5);
-            if (bdrv_is_locked(s->bs))
+            if (s->tray_locked) {
                 buf[6] |= 1 << 1;
+            }
             buf[15] = 0x00;
             cpu_to_ube16(&buf[16], 706);
             buf[18] = 0;
@@ -831,6 +832,7 @@ static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
 
 static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
 {
+    s->tray_locked = buf[4] & 1;
     bdrv_set_locked(s->bs, buf[4] & 1);
     ide_atapi_cmd_ok(s);
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 8a9052e..663db39 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -443,6 +443,7 @@ struct IDEState {
     uint8_t sense_key;
     uint8_t asc;
     bool tray_open;
+    bool tray_locked;
     uint8_t cdrom_changed;
     int packet_transfer_size;
     int elementary_transfer_size;
-- 
1.7.6

[Qemu-devel] [PATCH v3 11/27] scsi-disk: Track tray locked state

[Markus Armbruster]

We already track it in BlockDriverState.  Just like tray open/close
state, we should track it in the device models instead, because it's
device state.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-disk.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f35ada4..e7358e3 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -73,6 +73,7 @@ struct SCSIDiskState
     char *version;
     char *serial;
     bool tray_open;
+    bool tray_locked;
 };
 
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -671,7 +672,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         p[5] = 0xff; /* CD DA, DA accurate, RW supported,
                         RW corrected, C2 errors, ISRC,
                         UPC, Bar code */
-        p[6] = 0x2d | (bdrv_is_locked(s->bs)? 2 : 0);
+        p[6] = 0x2d | (s->tray_locked ? 2 : 0);
         /* Locking supported, jumper present, eject, tray */
         p[7] = 0; /* no volume & mute control, no
                      changer */
@@ -882,6 +883,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         scsi_disk_emulate_start_stop(r);
         break;
     case ALLOW_MEDIUM_REMOVAL:
+        s->tray_locked = req->cmd.buf[4] & 1;
         bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
         break;
     case READ_CAPACITY_10:
-- 
1.7.6

[Qemu-devel] [PATCH v3 12/27] block: Leave enforcing tray lock to device models

[Markus Armbruster]

The device model knows best when to accept the guest's eject command.
No need to detour through the block layer.

bdrv_eject() can't fail anymore.  Make it void.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c        |    7 +------
 block.h        |    2 +-
 hw/ide/atapi.c |   29 +++++++++--------------------
 hw/scsi-disk.c |    3 +++
 4 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/block.c b/block.c
index 8c86251..7408fa9 100644
--- a/block.c
+++ b/block.c
@@ -3051,18 +3051,13 @@ int bdrv_media_changed(BlockDriverState *bs)
 /**
  * If eject_flag is TRUE, eject the media. Otherwise, close the tray
  */
-int bdrv_eject(BlockDriverState *bs, int eject_flag)
+void bdrv_eject(BlockDriverState *bs, int eject_flag)
 {
     BlockDriver *drv = bs->drv;
 
-    if (eject_flag && bs->locked) {
-        return -EBUSY;
-    }
-
     if (drv && drv->bdrv_eject) {
         drv->bdrv_eject(bs, eject_flag);
     }
-    return 0;
 }
 
 int bdrv_is_locked(BlockDriverState *bs)
diff --git a/block.h b/block.h
index 8ec409f..5d941e9 100644
--- a/block.h
+++ b/block.h
@@ -208,7 +208,7 @@ int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 int bdrv_is_locked(BlockDriverState *bs);
 void bdrv_set_locked(BlockDriverState *bs, int locked);
-int bdrv_eject(BlockDriverState *bs, int eject_flag);
+void bdrv_eject(BlockDriverState *bs, int eject_flag);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
 BlockDriverState *bdrv_find(const char *name);
 BlockDriverState *bdrv_next(BlockDriverState *bs);
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index d9db6de..afb27c6 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -905,33 +905,22 @@ static void cmd_seek(IDEState *s, uint8_t* buf)
 
 static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
 {
-    int sense, err = 0;
+    int sense;
     bool start = buf[4] & 1;
     bool loej = buf[4] & 2;     /* load on start, eject on !start */
 
     if (loej) {
-        err = bdrv_eject(s->bs, !start);
-    }
-
-    switch (err) {
-    case 0:
-        ide_atapi_cmd_ok(s);
-        break;
-    case -EBUSY:
-        sense = SENSE_NOT_READY;
-        if (bdrv_is_inserted(s->bs)) {
-            sense = SENSE_ILLEGAL_REQUEST;
+        if (!start && s->tray_locked) {
+            sense = bdrv_is_inserted(s->bs)
+                ? SENSE_NOT_READY : SENSE_ILLEGAL_REQUEST;
+            ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
+            return;
         }
-        ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
-        break;
-    default:
-        ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
-        break;
-    }
-
-    if (loej && !err) {
+        bdrv_eject(s->bs, !start);
         s->tray_open = !start;
     }
+
+    ide_atapi_cmd_ok(s);
 }
 
 static void cmd_mechanism_status(IDEState *s, uint8_t* buf)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index e7358e3..65783a7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -830,6 +830,9 @@ static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
     bool loej = req->cmd.buf[4] & 2; /* load on start, eject on !start */
 
     if (s->qdev.type == TYPE_ROM && loej) {
+        if (!start && s->tray_locked) {
+            return;
+        }
         bdrv_eject(s->bs, !start);
         s->tray_open = !start;
     }
-- 
1.7.6

[Qemu-devel] [PATCH v3 13/27] block: Drop medium lock tracking, ask device models instead

[Markus Armbruster]

Requires new BlockDevOps member is_medium_locked().  Implement for IDE
and SCSI CD-ROMs.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c        |   16 +++++++++-------
 block.h        |    7 ++++++-
 block_int.h    |    1 -
 blockdev.c     |    2 +-
 hw/ide/core.c  |    6 ++++++
 hw/scsi-disk.c |   10 ++++++++++
 6 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 7408fa9..1e4be73 100644
--- a/block.c
+++ b/block.c
@@ -818,6 +818,14 @@ static void bdrv_dev_resize_cb(BlockDriverState *bs)
     }
 }
 
+bool bdrv_dev_is_medium_locked(BlockDriverState *bs)
+{
+    if (bs->dev_ops && bs->dev_ops->is_medium_locked) {
+        return bs->dev_ops->is_medium_locked(bs->dev_opaque);
+    }
+    return false;
+}
+
 /*
  * Run consistency checks on an image
  *
@@ -1890,7 +1898,7 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
         bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
                                     "'removable': %i, 'locked': %i }",
                                     bs->device_name, bs->removable,
-                                    bs->locked);
+                                    bdrv_dev_is_medium_locked(bs));
 
         if (bs->drv) {
             QObject *obj;
@@ -3060,11 +3068,6 @@ void bdrv_eject(BlockDriverState *bs, int eject_flag)
     }
 }
 
-int bdrv_is_locked(BlockDriverState *bs)
-{
-    return bs->locked;
-}
-
 /**
  * Lock or unlock the media (if it is locked, the user won't be able
  * to eject it manually).
@@ -3075,7 +3078,6 @@ void bdrv_set_locked(BlockDriverState *bs, int locked)
 
     trace_bdrv_set_locked(bs, locked);
 
-    bs->locked = locked;
     if (drv && drv->bdrv_set_locked) {
         drv->bdrv_set_locked(bs, locked);
     }
diff --git a/block.h b/block.h
index 5d941e9..396ca0e 100644
--- a/block.h
+++ b/block.h
@@ -37,6 +37,11 @@ typedef struct BlockDevOps {
      */
     void (*change_media_cb)(void *opaque);
     /*
+     * Is the virtual medium locked into the device?
+     * Device models implement this only when device has such a lock.
+     */
+    bool (*is_medium_locked)(void *opaque);
+    /*
      * Runs when the size changed (e.g. monitor command block_resize)
      */
     void (*resize_cb)(void *opaque);
@@ -94,6 +99,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev);
 void *bdrv_get_attached_dev(BlockDriverState *bs);
 void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
                       void *opaque);
+bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors);
 int bdrv_write(BlockDriverState *bs, int64_t sector_num,
@@ -206,7 +212,6 @@ int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
-int bdrv_is_locked(BlockDriverState *bs);
 void bdrv_set_locked(BlockDriverState *bs, int locked);
 void bdrv_eject(BlockDriverState *bs, int eject_flag);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
diff --git a/block_int.h b/block_int.h
index b63c57b..4f7ff3b 100644
--- a/block_int.h
+++ b/block_int.h
@@ -156,7 +156,6 @@ struct BlockDriverState {
     int keep_read_only; /* if true, the media was requested to stay read only */
     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 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* */
diff --git a/blockdev.c b/blockdev.c
index 049dda5..3f00b2e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -640,7 +640,7 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
         qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
         return -1;
     }
-    if (!force && bdrv_is_locked(bs)) {
+    if (!force && bdrv_dev_is_medium_locked(bs)) {
         qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
         return -1;
     }
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 34b7b83..b1a73ee 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1795,8 +1795,14 @@ void ide_bus_reset(IDEBus *bus)
     bus->dma->ops->reset(bus->dma);
 }
 
+static bool ide_cd_is_medium_locked(void *opaque)
+{
+    return ((IDEState *)opaque)->tray_locked;
+}
+
 static const BlockDevOps ide_cd_block_ops = {
     .change_media_cb = ide_cd_change_cb,
+    .is_medium_locked = ide_cd_is_medium_locked,
 };
 
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 65783a7..42682d0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1165,6 +1165,15 @@ static void scsi_destroy(SCSIDevice *dev)
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
+static bool scsi_cd_is_medium_locked(void *opaque)
+{
+    return ((SCSIDiskState *)opaque)->tray_locked;
+}
+
+static const BlockDevOps scsi_cd_block_ops = {
+    .is_medium_locked = scsi_cd_is_medium_locked,
+};
+
 static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
@@ -1199,6 +1208,7 @@ static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
     }
 
     if (scsi_type == TYPE_ROM) {
+        bdrv_set_dev_ops(s->bs, &scsi_cd_block_ops, s);
         s->qdev.blocksize = 2048;
     } else if (scsi_type == TYPE_DISK) {
         s->qdev.blocksize = s->qdev.conf.logical_block_size;
-- 
1.7.6

[Qemu-devel] [PATCH v3 14/27] block: Rename bdrv_set_locked() to bdrv_lock_medium()

[Markus Armbruster]

While there, make the locked parameter bool.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c           |    8 ++++----
 block.h           |    2 +-
 block/raw-posix.c |    8 ++++----
 block/raw.c       |    6 +++---
 block_int.h       |    2 +-
 hw/ide/atapi.c    |    2 +-
 hw/scsi-disk.c    |    2 +-
 trace-events      |    1 +
 8 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 1e4be73..7225b15 100644
--- a/block.c
+++ b/block.c
@@ -3072,14 +3072,14 @@ void bdrv_eject(BlockDriverState *bs, int eject_flag)
  * Lock or unlock the media (if it is locked, the user won't be able
  * to eject it manually).
  */
-void bdrv_set_locked(BlockDriverState *bs, int locked)
+void bdrv_lock_medium(BlockDriverState *bs, bool locked)
 {
     BlockDriver *drv = bs->drv;
 
-    trace_bdrv_set_locked(bs, locked);
+    trace_bdrv_lock_medium(bs, locked);
 
-    if (drv && drv->bdrv_set_locked) {
-        drv->bdrv_set_locked(bs, locked);
+    if (drv && drv->bdrv_lock_medium) {
+        drv->bdrv_lock_medium(bs, locked);
     }
 }
 
diff --git a/block.h b/block.h
index 396ca0e..4691090 100644
--- a/block.h
+++ b/block.h
@@ -212,7 +212,7 @@ int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
-void bdrv_set_locked(BlockDriverState *bs, int locked);
+void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 void bdrv_eject(BlockDriverState *bs, int eject_flag);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
 BlockDriverState *bdrv_find(const char *name);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index bcf50b2..a624f56 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1362,7 +1362,7 @@ static void cdrom_eject(BlockDriverState *bs, int eject_flag)
     }
 }
 
-static void cdrom_set_locked(BlockDriverState *bs, int locked)
+static void cdrom_lock_medium(BlockDriverState *bs, bool locked)
 {
     BDRVRawState *s = bs->opaque;
 
@@ -1400,7 +1400,7 @@ static BlockDriver bdrv_host_cdrom = {
     /* removable device support */
     .bdrv_is_inserted   = cdrom_is_inserted,
     .bdrv_eject         = cdrom_eject,
-    .bdrv_set_locked    = cdrom_set_locked,
+    .bdrv_lock_medium   = cdrom_lock_medium,
 
     /* generic scsi device */
     .bdrv_ioctl         = hdev_ioctl,
@@ -1481,7 +1481,7 @@ static void cdrom_eject(BlockDriverState *bs, int eject_flag)
     cdrom_reopen(bs);
 }
 
-static void cdrom_set_locked(BlockDriverState *bs, int locked)
+static void cdrom_lock_medium(BlockDriverState *bs, bool locked)
 {
     BDRVRawState *s = bs->opaque;
 
@@ -1521,7 +1521,7 @@ static BlockDriver bdrv_host_cdrom = {
     /* removable device support */
     .bdrv_is_inserted   = cdrom_is_inserted,
     .bdrv_eject         = cdrom_eject,
-    .bdrv_set_locked    = cdrom_set_locked,
+    .bdrv_lock_medium   = cdrom_lock_medium,
 };
 #endif /* __FreeBSD__ */
 
diff --git a/block/raw.c b/block/raw.c
index f197479..63cf2d3 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -85,9 +85,9 @@ static void raw_eject(BlockDriverState *bs, int eject_flag)
     bdrv_eject(bs->file, eject_flag);
 }
 
-static void raw_set_locked(BlockDriverState *bs, int locked)
+static void raw_lock_medium(BlockDriverState *bs, bool locked)
 {
-    bdrv_set_locked(bs->file, locked);
+    bdrv_lock_medium(bs->file, locked);
 }
 
 static int raw_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
@@ -144,7 +144,7 @@ static BlockDriver bdrv_raw = {
     .bdrv_is_inserted   = raw_is_inserted,
     .bdrv_media_changed = raw_media_changed,
     .bdrv_eject         = raw_eject,
-    .bdrv_set_locked    = raw_set_locked,
+    .bdrv_lock_medium   = raw_lock_medium,
 
     .bdrv_ioctl         = raw_ioctl,
     .bdrv_aio_ioctl     = raw_aio_ioctl,
diff --git a/block_int.h b/block_int.h
index 4f7ff3b..f42af2c 100644
--- a/block_int.h
+++ b/block_int.h
@@ -120,7 +120,7 @@ struct BlockDriver {
     int (*bdrv_is_inserted)(BlockDriverState *bs);
     int (*bdrv_media_changed)(BlockDriverState *bs);
     void (*bdrv_eject)(BlockDriverState *bs, int eject_flag);
-    void (*bdrv_set_locked)(BlockDriverState *bs, int locked);
+    void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
     /* to control generic scsi devices */
     int (*bdrv_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf);
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index afb27c6..06778f3 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -833,7 +833,7 @@ static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
 static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
 {
     s->tray_locked = buf[4] & 1;
-    bdrv_set_locked(s->bs, buf[4] & 1);
+    bdrv_lock_medium(s->bs, buf[4] & 1);
     ide_atapi_cmd_ok(s);
 }
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 42682d0..4e89bb1 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -887,7 +887,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         break;
     case ALLOW_MEDIUM_REMOVAL:
         s->tray_locked = req->cmd.buf[4] & 1;
-        bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
+        bdrv_lock_medium(s->bs, req->cmd.buf[4] & 1);
         break;
     case READ_CAPACITY_10:
         /* The normal LEN field for this command is zero.  */
diff --git a/trace-events b/trace-events
index 08ffedf..1c0b49b 100644
--- a/trace-events
+++ b/trace-events
@@ -62,6 +62,7 @@ bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
 bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p"
 bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
 bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
+bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
 bdrv_set_locked(void *bs, int locked) "bs %p locked %d"
 bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
-- 
1.7.6

[Qemu-devel] [PATCH v3 15/27] ide/atapi: Don't fail eject when tray is already open

[Markus Armbruster]

MMC-5 6.40.2.6 specifies that START STOP UNIT succeeds when the drive
already has the requested state.  cmd_start_stop_unit() fails when
asked to eject while the tray is open and locked.  Fix that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/atapi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 06778f3..3f909c3 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -910,7 +910,7 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
     bool loej = buf[4] & 2;     /* load on start, eject on !start */
 
     if (loej) {
-        if (!start && s->tray_locked) {
+        if (!start && !s->tray_open && s->tray_locked) {
             sense = bdrv_is_inserted(s->bs)
                 ? SENSE_NOT_READY : SENSE_ILLEGAL_REQUEST;
             ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
-- 
1.7.6

[Qemu-devel] [PATCH v3 16/27] scsi-disk: Fix START_STOP to fail when it can't eject

[Markus Armbruster]

Don't fail when tray is already open.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-bus.c  |   10 ++++++++++
 hw/scsi-disk.c |   15 +++++++++++----
 hw/scsi.h      |    4 ++++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 160eaee..79cb29d 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -772,6 +772,11 @@ const struct SCSISense sense_code_NO_MEDIUM = {
     .key = NOT_READY, .asc = 0x3a, .ascq = 0x00
 };
 
+/* LUN not ready, medium removal prevented */
+const struct SCSISense sense_code_NOT_READY_REMOVAL_PREVENTED = {
+    .key = NOT_READY, .asc = 0x53, .ascq = 0x00
+};
+
 /* Hardware error, internal target failure */
 const struct SCSISense sense_code_TARGET_FAILURE = {
     .key = HARDWARE_ERROR, .asc = 0x44, .ascq = 0x00
@@ -807,6 +812,11 @@ const struct SCSISense sense_code_INCOMPATIBLE_MEDIUM = {
     .key = ILLEGAL_REQUEST, .asc = 0x30, .ascq = 0x00
 };
 
+/* Illegal request, medium removal prevented */
+const struct SCSISense sense_code_ILLEGAL_REQ_REMOVAL_PREVENTED = {
+    .key = ILLEGAL_REQUEST, .asc = 0x53, .ascq = 0x00
+};
+
 /* Command aborted, I/O process terminated */
 const struct SCSISense sense_code_IO_ERROR = {
     .key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4e89bb1..1a49217 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -822,7 +822,7 @@ static int scsi_disk_emulate_read_toc(SCSIRequest *req, uint8_t *outbuf)
     return toclen;
 }
 
-static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
+static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
 {
     SCSIRequest *req = &r->req;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
@@ -830,12 +830,17 @@ static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
     bool loej = req->cmd.buf[4] & 2; /* load on start, eject on !start */
 
     if (s->qdev.type == TYPE_ROM && loej) {
-        if (!start && s->tray_locked) {
-            return;
+        if (!start && !s->tray_open && s->tray_locked) {
+            scsi_check_condition(r,
+                                 bdrv_is_inserted(s->bs)
+                                 ? SENSE_CODE(ILLEGAL_REQ_REMOVAL_PREVENTED)
+                                 : SENSE_CODE(NOT_READY_REMOVAL_PREVENTED));
+            return -1;
         }
         bdrv_eject(s->bs, !start);
         s->tray_open = !start;
     }
+    return 0;
 }
 
 static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
@@ -883,7 +888,9 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
             goto illegal_request;
         break;
     case START_STOP:
-        scsi_disk_emulate_start_stop(r);
+        if (scsi_disk_emulate_start_stop(r) < 0) {
+            return -1;
+        }
         break;
     case ALLOW_MEDIUM_REMOVAL:
         s->tray_locked = req->cmd.buf[4] & 1;
diff --git a/hw/scsi.h b/hw/scsi.h
index 98fd689..a28cd68 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -136,6 +136,8 @@ extern const struct SCSISense sense_code_NO_SENSE;
 extern const struct SCSISense sense_code_LUN_NOT_READY;
 /* LUN not ready, Medium not present */
 extern const struct SCSISense sense_code_NO_MEDIUM;
+/* LUN not ready, medium removal prevented */
+extern const struct SCSISense sense_code_NOT_READY_REMOVAL_PREVENTED;
 /* Hardware error, internal target failure */
 extern const struct SCSISense sense_code_TARGET_FAILURE;
 /* Illegal request, invalid command operation code */
@@ -150,6 +152,8 @@ extern const struct SCSISense sense_code_LUN_NOT_SUPPORTED;
 extern const struct SCSISense sense_code_SAVING_PARAMS_NOT_SUPPORTED;
 /* Illegal request, Incompatible format */
 extern const struct SCSISense sense_code_INCOMPATIBLE_FORMAT;
+/* Illegal request, medium removal prevented */
+extern const struct SCSISense sense_code_ILLEGAL_REQ_REMOVAL_PREVENTED;
 /* Command aborted, I/O process terminated */
 extern const struct SCSISense sense_code_IO_ERROR;
 /* Command aborted, I_T Nexus loss occurred */
-- 
1.7.6

[Qemu-devel] [PATCH v3 17/27] ide/atapi: Preserve tray state on migration

[Markus Armbruster]

Use a subsection, so that migration to older version still works,
provided the tray is closed and unlocked.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b1a73ee..30cb7de 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2058,6 +2058,22 @@ static bool ide_drive_pio_state_needed(void *opaque)
         || (s->bus->error_status & BM_STATUS_PIO_RETRY);
 }
 
+static int ide_tray_state_post_load(void *opaque, int version_id)
+{
+    IDEState *s = opaque;
+
+    bdrv_eject(s->bs, s->tray_open);
+    bdrv_lock_medium(s->bs, s->tray_locked);
+    return 0;
+}
+
+static bool ide_tray_state_needed(void *opaque)
+{
+    IDEState *s = opaque;
+
+    return s->tray_open || s->tray_locked;
+}
+
 static bool ide_atapi_gesn_needed(void *opaque)
 {
     IDEState *s = opaque;
@@ -2085,6 +2101,19 @@ static const VMStateDescription vmstate_ide_atapi_gesn_state = {
     }
 };
 
+static const VMStateDescription vmstate_ide_tray_state = {
+    .name = "ide_drive/tray_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .post_load = ide_tray_state_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_BOOL(tray_open, IDEState),
+        VMSTATE_BOOL(tray_locked, IDEState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_ide_drive_pio_state = {
     .name = "ide_drive/pio_state",
     .version_id = 1,
@@ -2139,6 +2168,9 @@ const VMStateDescription vmstate_ide_drive = {
             .vmsd = &vmstate_ide_drive_pio_state,
             .needed = ide_drive_pio_state_needed,
         }, {
+            .vmsd = &vmstate_ide_tray_state,
+            .needed = ide_tray_state_needed,
+        }, {
             .vmsd = &vmstate_ide_atapi_gesn_state,
             .needed = ide_atapi_gesn_needed,
         }, {
-- 
1.7.6

[Qemu-devel] [PATCH v3 18/27] block: Clean up remaining users of "removable"

[Markus Armbruster]

BlockDriverState member removable is a confused mess.  It is true when
an ide-cd, scsi-cd or floppy qdev is attached, or when the
BlockDriverState was created with -drive if={floppy,sd} or -drive
if={ide,scsi,xen,none},media=cdrom ("created removable"), except when
an ide-hd, scsi-hd, scsi-generic or virtio-blk qdev is attached.

Three users remain:

1. eject_device(), via bdrv_is_removable() uses it to determine
   whether a block device can eject media.

2. bdrv_info() is monitor command "info block".  QMP documentation
   says "true if the device is removable, false otherwise".  From the
   monitor user's point of view, the only sensible interpretation of
   "is removable" is "can eject media with monitor commands eject and
   change".

A block device can eject media unless a device is attached that
doesn't support it.  Switch the two users over to new
bdrv_dev_has_removable_media() that returns exactly that.

3. bdrv_getlength() uses to suppress its length cache when media can
   change (see commit 46a4e4e6).  Media change is either monitor
   command change (updates the length cache), monitor command eject
   (doesn't update the length cache, easily fixable), or physical
   media change (invalidates length cache, not so easily fixable).

I'm refraining from improving anything here, because this series is
long enough already.  Instead, I simply switch it over to
bdrv_dev_has_removable_media() as well.

This changes the behavior of the length cache and of monitor commands
eject and change in two cases:

a. drive not created removable, no device attached

   The commit makes the drive removable, and defeats the length cache.

   Example: -drive if=none

b. drive created removable, but the attached drive is non-removable,
   and doesn't call bdrv_set_removable(..., 0) (most devices don't)

   The commit makes the drive non-removable, and enables the length
   cache.

   Example: -drive if=xen,media=cdrom -M xenpv

   The other non-removable devices that don't call
   bdrv_set_removable() can't currently use a drive created removable,
   either because they aren't qdevified, or because they lack a drive
   property.  Won't stay that way.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c        |   18 +++++++++++-------
 block.h        |    3 ++-
 blockdev.c     |    2 +-
 hw/scsi-disk.c |    5 +++++
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 7225b15..fee45bf 100644
--- a/block.c
+++ b/block.c
@@ -802,6 +802,9 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
 {
     bs->dev_ops = ops;
     bs->dev_opaque = opaque;
+    if (bdrv_dev_has_removable_media(bs) && bs == bs_snapshots) {
+        bs_snapshots = NULL;
+    }
 }
 
 static void bdrv_dev_change_media_cb(BlockDriverState *bs)
@@ -811,6 +814,11 @@ static void bdrv_dev_change_media_cb(BlockDriverState *bs)
     }
 }
 
+bool bdrv_dev_has_removable_media(BlockDriverState *bs)
+{
+    return !bs->dev || (bs->dev_ops && bs->dev_ops->change_media_cb);
+}
+
 static void bdrv_dev_resize_cb(BlockDriverState *bs)
 {
     if (bs->dev_ops && bs->dev_ops->resize_cb) {
@@ -1329,7 +1337,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
     if (!drv)
         return -ENOMEDIUM;
 
-    if (bs->growable || bs->removable) {
+    if (bs->growable || bdrv_dev_has_removable_media(bs)) {
         if (drv->bdrv_getlength) {
             return drv->bdrv_getlength(bs);
         }
@@ -1614,11 +1622,6 @@ void bdrv_set_removable(BlockDriverState *bs, int removable)
     }
 }
 
-int bdrv_is_removable(BlockDriverState *bs)
-{
-    return bs->removable;
-}
-
 int bdrv_is_read_only(BlockDriverState *bs)
 {
     return bs->read_only;
@@ -1897,7 +1900,8 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
 
         bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
                                     "'removable': %i, 'locked': %i }",
-                                    bs->device_name, bs->removable,
+                                    bs->device_name,
+                                    bdrv_dev_has_removable_media(bs),
                                     bdrv_dev_is_medium_locked(bs));
 
         if (bs->drv) {
diff --git a/block.h b/block.h
index 4691090..860e30d 100644
--- a/block.h
+++ b/block.h
@@ -34,6 +34,7 @@ typedef struct BlockDevOps {
      * Runs when virtual media changed (monitor commands eject, change)
      * Beware: doesn't run when a host device's physical media
      * changes.  Sure would be useful if it did.
+     * Device models with removable media must implement this callback.
      */
     void (*change_media_cb)(void *opaque);
     /*
@@ -99,6 +100,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev);
 void *bdrv_get_attached_dev(BlockDriverState *bs);
 void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
                       void *opaque);
+bool bdrv_dev_has_removable_media(BlockDriverState *bs);
 bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors);
@@ -206,7 +208,6 @@ 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);
 int bdrv_enable_write_cache(BlockDriverState *bs);
diff --git a/blockdev.c b/blockdev.c
index 3f00b2e..ddf1f8f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -636,7 +636,7 @@ out:
 
 static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 {
-    if (!bdrv_is_removable(bs)) {
+    if (!bdrv_dev_has_removable_media(bs)) {
         qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
         return -1;
     }
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 1a49217..d6e838c 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1172,12 +1172,17 @@ static void scsi_destroy(SCSIDevice *dev)
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
+static void scsi_cd_change_media_cb(void *opaque)
+{
+}
+
 static bool scsi_cd_is_medium_locked(void *opaque)
 {
     return ((SCSIDiskState *)opaque)->tray_locked;
 }
 
 static const BlockDevOps scsi_cd_block_ops = {
+    .change_media_cb = scsi_cd_change_media_cb,
     .is_medium_locked = scsi_cd_is_medium_locked,
 };
 
-- 
1.7.6

[Qemu-devel] [PATCH v3 19/27] block: Drop BlockDriverState member removable

[Markus Armbruster]

It's a confused mess (see previous commit).  No users remain.

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

diff --git a/block.c b/block.c
index fee45bf..d8193a3 100644
--- a/block.c
+++ b/block.c
@@ -1614,14 +1614,6 @@ 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_read_only(BlockDriverState *bs)
 {
     return bs->read_only;
diff --git a/block.h b/block.h
index 860e30d..e4b5530 100644
--- a/block.h
+++ b/block.h
@@ -207,7 +207,6 @@ 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_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index f42af2c..f30563d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -155,7 +155,6 @@ struct BlockDriverState {
     int read_only; /* if true, the media is read only */
     int keep_read_only; /* if true, the media was requested to stay read only */
     int open_flags; /* flags used to open the file, re-used for re-open */
-    int removable; /* if true, the media can be removed */
     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* */
diff --git a/blockdev.c b/blockdev.c
index ddf1f8f..154cc84 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -473,17 +473,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
             }
 	    break;
 	case MEDIA_CDROM:
-            bdrv_set_removable(dinfo->bdrv, 1);
             dinfo->media_cd = 1;
 	    break;
 	}
         break;
     case IF_SD:
-        /* FIXME: This isn't really a floppy, but it's a reasonable
-           approximation.  */
     case IF_FLOPPY:
-        bdrv_set_removable(dinfo->bdrv, 1);
-        break;
     case IF_PFLASH:
     case IF_MTD:
         break;
diff --git a/hw/fdc.c b/hw/fdc.c
index 1d44bbd..edade2b 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1813,7 +1813,6 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
         fd_revalidate(drive);
         if (drive->bs) {
             drive->media_changed = 1;
-            bdrv_set_removable(drive->bs, 1);
             bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive);
         }
     }
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 30cb7de..d4d8f3c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1864,7 +1864,6 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     }
 
     ide_reset(s);
-    bdrv_set_removable(bs, s->drive_kind == IDE_CD);
     return 0;
 }
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d6e838c..0f0cddc 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1233,7 +1233,6 @@ static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
 
     s->qdev.type = scsi_type;
     qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
-    bdrv_set_removable(s->bs, scsi_type == TYPE_ROM);
     add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, ",0");
     return 0;
 }
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index cb5d4f1..5ce01af 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -450,7 +450,6 @@ static int scsi_generic_initfn(SCSIDevice *dev)
         }
     }
     DPRINTF("block size %d\n", s->qdev.blocksize);
-    bdrv_set_removable(s->bs, 0);
     return 0;
 }
 
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 4df23f4..c464bbd 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -599,7 +599,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
     s->qdev = dev;
     register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
-    bdrv_set_removable(s->bs, 0);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
     s->bs->buffer_alignment = conf->logical_block_size;
 
-- 
1.7.6

[Qemu-devel] [PATCH v3 20/27] block: Show whether the virtual tray is open in info block

[Markus Armbruster]

Need to ask the device, so this requires new BlockDevOps member
is_tray_open().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c         |   18 ++++++++++++++++--
 block.h         |    6 ++++++
 hw/ide/core.c   |    6 ++++++
 hw/scsi-disk.c  |    6 ++++++
 qmp-commands.hx |    2 ++
 5 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index d8193a3..febfd67 100644
--- a/block.c
+++ b/block.c
@@ -819,6 +819,14 @@ bool bdrv_dev_has_removable_media(BlockDriverState *bs)
     return !bs->dev || (bs->dev_ops && bs->dev_ops->change_media_cb);
 }
 
+bool bdrv_dev_is_tray_open(BlockDriverState *bs)
+{
+    if (bs->dev_ops && bs->dev_ops->is_tray_open) {
+        return bs->dev_ops->is_tray_open(bs->dev_opaque);
+    }
+    return false;
+}
+
 static void bdrv_dev_resize_cb(BlockDriverState *bs)
 {
     if (bs->dev_ops && bs->dev_ops->resize_cb) {
@@ -1853,8 +1861,9 @@ static void bdrv_print_dict(QObject *obj, void *opaque)
 
     if (qdict_get_bool(bs_dict, "removable")) {
         monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked"));
+        monitor_printf(mon, " tray-open=%d",
+                       qdict_get_bool(bs_dict, "tray-open"));
     }
-
     if (qdict_haskey(bs_dict, "inserted")) {
         QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
 
@@ -1889,16 +1898,21 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
 
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
         QObject *bs_obj;
+        QDict *bs_dict;
 
         bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
                                     "'removable': %i, 'locked': %i }",
                                     bs->device_name,
                                     bdrv_dev_has_removable_media(bs),
                                     bdrv_dev_is_medium_locked(bs));
+        bs_dict = qobject_to_qdict(bs_obj);
 
+        if (bdrv_dev_has_removable_media(bs)) {
+            qdict_put(bs_dict, "tray-open",
+                      qbool_from_int(bdrv_dev_is_tray_open(bs)));
+        }
         if (bs->drv) {
             QObject *obj;
-            QDict *bs_dict = qobject_to_qdict(bs_obj);
 
             obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
                                      "'encrypted': %i }",
diff --git a/block.h b/block.h
index e4b5530..9f6d02c 100644
--- a/block.h
+++ b/block.h
@@ -38,6 +38,11 @@ typedef struct BlockDevOps {
      */
     void (*change_media_cb)(void *opaque);
     /*
+     * Is the virtual tray open?
+     * Device models implement this only when the device has a tray.
+     */
+    bool (*is_tray_open)(void *opaque);
+    /*
      * Is the virtual medium locked into the device?
      * Device models implement this only when device has such a lock.
      */
@@ -101,6 +106,7 @@ void *bdrv_get_attached_dev(BlockDriverState *bs);
 void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
                       void *opaque);
 bool bdrv_dev_has_removable_media(BlockDriverState *bs);
+bool bdrv_dev_is_tray_open(BlockDriverState *bs);
 bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d4d8f3c..84d4c4f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1795,6 +1795,11 @@ void ide_bus_reset(IDEBus *bus)
     bus->dma->ops->reset(bus->dma);
 }
 
+static bool ide_cd_is_tray_open(void *opaque)
+{
+    return ((IDEState *)opaque)->tray_open;
+}
+
 static bool ide_cd_is_medium_locked(void *opaque)
 {
     return ((IDEState *)opaque)->tray_locked;
@@ -1802,6 +1807,7 @@ static bool ide_cd_is_medium_locked(void *opaque)
 
 static const BlockDevOps ide_cd_block_ops = {
     .change_media_cb = ide_cd_change_cb,
+    .is_tray_open = ide_cd_is_tray_open,
     .is_medium_locked = ide_cd_is_medium_locked,
 };
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 0f0cddc..f48ca8b 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1176,6 +1176,11 @@ static void scsi_cd_change_media_cb(void *opaque)
 {
 }
 
+static bool scsi_cd_is_tray_open(void *opaque)
+{
+    return ((SCSIDiskState *)opaque)->tray_open;
+}
+
 static bool scsi_cd_is_medium_locked(void *opaque)
 {
     return ((SCSIDiskState *)opaque)->tray_locked;
@@ -1183,6 +1188,7 @@ static bool scsi_cd_is_medium_locked(void *opaque)
 
 static const BlockDevOps scsi_cd_block_ops = {
     .change_media_cb = scsi_cd_change_media_cb,
+    .is_tray_open = scsi_cd_is_tray_open,
     .is_medium_locked = scsi_cd_is_medium_locked,
 };
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 27cc66e..d1c2c59 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1131,6 +1131,8 @@ Each json-object contain the following:
          - Possible values: "unknown"
 - "removable": true if the device is removable, false otherwise (json-bool)
 - "locked": true if the device is locked, false otherwise (json-bool)
+- "tray-open": only present if removable, true if the device has a tray,
+               and it is open (json-bool)
 - "inserted": only present if the device is inserted, it is a json-object
    containing the following:
          - "file": device file name (json-string)
-- 
1.7.6

[Qemu-devel] [PATCH v3 21/27] block: Move BlockConf & friends from block_int.h to block.h

[Markus Armbruster]

It's convenience stuff for block device models, so block.h isn't the
ideal home either, but better than block_int.h.

Permits moving some #include "block_int.h" from device model .h into
.c.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.h           |   38 ++++++++++++++++++++++++++++++++++++++
 block_int.h       |   35 -----------------------------------
 hw/ide/core.c     |    1 +
 hw/ide/internal.h |    1 -
 hw/scsi-disk.c    |    1 +
 hw/scsi.h         |    1 -
 hw/virtio-blk.c   |    1 +
 hw/virtio.h       |    2 +-
 8 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/block.h b/block.h
index 9f6d02c..6e0c468 100644
--- a/block.h
+++ b/block.h
@@ -350,5 +350,43 @@ typedef enum {
 #define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
 void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
 
+
+/* Convenience for block device models */
+
+typedef struct BlockConf {
+    BlockDriverState *bs;
+    uint16_t physical_block_size;
+    uint16_t logical_block_size;
+    uint16_t min_io_size;
+    uint32_t opt_io_size;
+    int32_t bootindex;
+    uint32_t discard_granularity;
+} BlockConf;
+
+static inline unsigned int get_physical_block_exp(BlockConf *conf)
+{
+    unsigned int exp = 0, size;
+
+    for (size = conf->physical_block_size;
+        size > conf->logical_block_size;
+        size >>= 1) {
+        exp++;
+    }
+
+    return exp;
+}
+
+#define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
+    DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
+    DEFINE_PROP_UINT16("logical_block_size", _state,                    \
+                       _conf.logical_block_size, 512),                  \
+    DEFINE_PROP_UINT16("physical_block_size", _state,                   \
+                       _conf.physical_block_size, 512),                 \
+    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
+    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
+    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
+    DEFINE_PROP_UINT32("discard_granularity", _state, \
+                       _conf.discard_granularity, 0)
+
 #endif
 
diff --git a/block_int.h b/block_int.h
index f30563d..8c3b863 100644
--- a/block_int.h
+++ b/block_int.h
@@ -225,39 +225,4 @@ void qemu_aio_release(void *p);
 int is_windows_drive(const char *filename);
 #endif
 
-typedef struct BlockConf {
-    BlockDriverState *bs;
-    uint16_t physical_block_size;
-    uint16_t logical_block_size;
-    uint16_t min_io_size;
-    uint32_t opt_io_size;
-    int32_t bootindex;
-    uint32_t discard_granularity;
-} BlockConf;
-
-static inline unsigned int get_physical_block_exp(BlockConf *conf)
-{
-    unsigned int exp = 0, size;
-
-    for (size = conf->physical_block_size;
-        size > conf->logical_block_size;
-        size >>= 1) {
-        exp++;
-    }
-
-    return exp;
-}
-
-#define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
-    DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
-    DEFINE_PROP_UINT16("logical_block_size", _state,                    \
-                       _conf.logical_block_size, 512),                  \
-    DEFINE_PROP_UINT16("physical_block_size", _state,                   \
-                       _conf.physical_block_size, 512),                 \
-    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
-    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
-    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
-    DEFINE_PROP_UINT32("discard_granularity", _state, \
-                       _conf.discard_granularity, 0)
-
 #endif /* BLOCK_INT_H */
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 84d4c4f..2f0f6d4 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -30,6 +30,7 @@
 #include "sysemu.h"
 #include "dma.h"
 #include "blockdev.h"
+#include "block_int.h"
 
 #include <hw/ide/internal.h>
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 663db39..233915c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -7,7 +7,6 @@
  * non-internal declarations are in hw/ide.h
  */
 #include <hw/ide.h>
-#include "block_int.h"
 #include "iorange.h"
 #include "dma.h"
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f48ca8b..d44b3b8 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -37,6 +37,7 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 #include "scsi-defs.h"
 #include "sysemu.h"
 #include "blockdev.h"
+#include "block_int.h"
 
 #define SCSI_DMA_BUF_SIZE    131072
 #define SCSI_MAX_INQUIRY_LEN 256
diff --git a/hw/scsi.h b/hw/scsi.h
index a28cd68..e8dcabf 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,7 +3,6 @@
 
 #include "qdev.h"
 #include "block.h"
-#include "block_int.h"
 
 #define MAX_SCSI_DEVS	255
 
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index c464bbd..4876555 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -15,6 +15,7 @@
 #include "qemu-error.h"
 #include "trace.h"
 #include "blockdev.h"
+#include "block_int.h"
 #include "virtio-blk.h"
 #ifdef __linux__
 # include <scsi/sg.h>
diff --git a/hw/virtio.h b/hw/virtio.h
index c129264..4d20d9b 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -18,7 +18,7 @@
 #include "net.h"
 #include "qdev.h"
 #include "sysemu.h"
-#include "block_int.h"
+#include "block.h"
 #include "event_notifier.h"
 #ifdef CONFIG_LINUX
 #include "9p.h"
-- 
1.7.6

[Qemu-devel] [PATCH v3 22/27] hw: Trim superfluous #include "block_int.h"

[Markus Armbruster]

Including it in device models is unclean, including it without a
reason adds insult to injury.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/fdc.c            |    1 -
 hw/ide/cmd646.c     |    1 -
 hw/ide/ich.c        |    1 -
 hw/ide/isa.c        |    1 -
 hw/ide/macio.c      |    1 -
 hw/ide/microdrive.c |    1 -
 hw/ide/mmio.c       |    1 -
 hw/ide/pci.c        |    1 -
 hw/ide/via.c        |    1 -
 hw/lsi53c895a.c     |    1 -
 10 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index edade2b..57eda0c 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -36,7 +36,6 @@
 #include "qdev-addr.h"
 #include "blockdev.h"
 #include "sysemu.h"
-#include "block_int.h"
 
 /********************************************************/
 /* debug Floppy devices */
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 4d91e2c..5fe98b1 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -27,7 +27,6 @@
 #include <hw/pci.h>
 #include <hw/isa.h>
 #include "block.h"
-#include "block_int.h"
 #include "sysemu.h"
 #include "dma.h"
 
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 5278bc4..0327d0e 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -66,7 +66,6 @@
 #include <hw/pci.h>
 #include <hw/isa.h>
 #include "block.h"
-#include "block_int.h"
 #include "dma.h"
 
 #include <hw/ide/pci.h>
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 4ac7453..28b69d2 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -26,7 +26,6 @@
 #include <hw/pc.h>
 #include <hw/isa.h>
 #include "block.h"
-#include "block_int.h"
 #include "dma.h"
 
 #include <hw/ide/internal.h>
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index fdf5d75..c1844cb 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -26,7 +26,6 @@
 #include <hw/ppc_mac.h>
 #include <hw/mac_dbdma.h>
 #include "block.h"
-#include "block_int.h"
 #include "dma.h"
 
 #include <hw/ide/internal.h>
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 91c0e3c..9eee5b5 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -26,7 +26,6 @@
 #include <hw/pc.h>
 #include <hw/pcmcia.h>
 #include "block.h"
-#include "block_int.h"
 #include "dma.h"
 
 #include <hw/ide/internal.h>
diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index 132b751..2ec21b0 100644
--- a/hw/ide/mmio.c
+++ b/hw/ide/mmio.c
@@ -24,7 +24,6 @@
  */
 #include <hw/hw.h>
 #include "block.h"
-#include "block_int.h"
 #include "dma.h"
 
 #include <hw/ide/internal.h>
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index d1a14d7..9fded02 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -27,7 +27,6 @@
 #include <hw/pci.h>
 #include <hw/isa.h>
 #include "block.h"
-#include "block_int.h"
 #include "dma.h"
 
 #include <hw/ide/pci.h>
diff --git a/hw/ide/via.c b/hw/ide/via.c
index c0b9d43..dab8a39 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -28,7 +28,6 @@
 #include <hw/pci.h>
 #include <hw/isa.h>
 #include "block.h"
-#include "block_int.h"
 #include "sysemu.h"
 #include "dma.h"
 
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 1643a63..f5fabad 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -15,7 +15,6 @@
 #include "hw.h"
 #include "pci.h"
 #include "scsi.h"
-#include "block_int.h"
 
 //#define DEBUG_LSI
 //#define DEBUG_LSI_REG
-- 
1.7.6

[Qemu-devel] [PATCH v3 23/27] block: New bdrv_set_buffer_alignment()

[Markus Armbruster]

Device models should be able to set it without an unclean include of
block_int.h.

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

diff --git a/block.c b/block.c
index febfd67..e986986 100644
--- a/block.c
+++ b/block.c
@@ -480,7 +480,6 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
     bs->encrypted = 0;
     bs->valid_key = 0;
     bs->open_flags = flags;
-    /* buffer_alignment defaulted to 512, drivers can change this value */
     bs->buffer_alignment = 512;
 
     pstrcpy(bs->filename, sizeof(bs->filename), filename);
@@ -3115,7 +3114,10 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
     return NULL;
 }
 
-
+void bdrv_set_buffer_alignment(BlockDriverState *bs, int align)
+{
+    bs->buffer_alignment = align;
+}
 
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
 {
diff --git a/block.h b/block.h
index 6e0c468..4b7fa35 100644
--- a/block.h
+++ b/block.h
@@ -269,6 +269,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
                     const char *base_filename, const char *base_fmt,
                     char *options, uint64_t img_size, int flags);
 
+void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
 
 #define BDRV_SECTORS_PER_DIRTY_CHUNK 2048
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2f0f6d4..4fb7e61 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1847,7 +1847,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->smart_selftest_count = 0;
     if (kind == IDE_CD) {
         bdrv_set_dev_ops(bs, &ide_cd_block_ops, s);
-        bs->buffer_alignment = 2048;
+        bdrv_set_buffer_alignment(bs, 2048);
     } else {
         if (!bdrv_is_inserted(s->bs)) {
             error_report("Device needs media, but drive is empty");
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d44b3b8..b115760 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1236,7 +1236,7 @@ static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
         return -1;
     }
     s->cluster_size = s->qdev.blocksize / 512;
-    s->bs->buffer_alignment = s->qdev.blocksize;
+    bdrv_set_buffer_alignment(s->bs, s->qdev.blocksize);
 
     s->qdev.type = scsi_type;
     qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 4876555..ac4b3f1 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -15,7 +15,6 @@
 #include "qemu-error.h"
 #include "trace.h"
 #include "blockdev.h"
-#include "block_int.h"
 #include "virtio-blk.h"
 #ifdef __linux__
 # include <scsi/sg.h>
@@ -601,7 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
     register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
-    s->bs->buffer_alignment = conf->logical_block_size;
+    bdrv_set_buffer_alignment(s->bs, conf->logical_block_size);
 
     add_boot_device_path(conf->bootindex, dev, "/disk@0,0");
 
-- 
1.7.6

[Qemu-devel] [PATCH v3 24/27] block: Reset buffer alignment on detach

[Markus Armbruster]

BlockDriverState member buffer_alignment is initially 512.  The device
model may set them, with bdrv_set_buffer_alignment().  If the device
model gets detached (hot unplug), the device's alignment is left
behind.  Only okay because device hot unplug automatically destroys
the BlockDriverState.  But that's a questionable feature, best not to
rely on it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index e986986..b006e58 100644
--- a/block.c
+++ b/block.c
@@ -788,6 +788,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev)
     bs->dev = NULL;
     bs->dev_ops = NULL;
     bs->dev_opaque = NULL;
+    bs->buffer_alignment = 512;
 }
 
 /* TODO change to return DeviceState * when all users are qdevified */
-- 
1.7.6

[Qemu-devel] [PATCH v3 25/27] nbd: Clean up use of block_int.h

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/nbd.c |    1 +
 nbd.c       |    1 +
 nbd.h       |    2 --
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 55cb2fd..70edd81 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -28,6 +28,7 @@
 
 #include "qemu-common.h"
 #include "nbd.h"
+#include "block_int.h"
 #include "module.h"
 #include "qemu_socket.h"
 
diff --git a/nbd.c b/nbd.c
index e7a585d..6d81cfb 100644
--- a/nbd.c
+++ b/nbd.c
@@ -17,6 +17,7 @@
  */
 
 #include "nbd.h"
+#include "block.h"
 
 #include <errno.h>
 #include <string.h>
diff --git a/nbd.h b/nbd.h
index 96f77fe..4b54dd2 100644
--- a/nbd.h
+++ b/nbd.h
@@ -23,8 +23,6 @@
 
 #include <qemu-common.h>
 
-#include "block_int.h"
-
 struct nbd_request {
     uint32_t magic;
     uint32_t type;
-- 
1.7.6

[Qemu-devel] [PATCH v3 26/27] block: New change_media_cb() parameter load

[Markus Armbruster]

To let device models distinguish between eject and load.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c        |   12 ++++++------
 block.h        |    3 ++-
 hw/fdc.c       |    2 +-
 hw/ide/core.c  |    2 +-
 hw/scsi-disk.c |    2 +-
 hw/sd.c        |    2 +-
 6 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index b006e58..e3fe97f 100644
--- a/block.c
+++ b/block.c
@@ -44,7 +44,7 @@
 #include <windows.h>
 #endif
 
-static void bdrv_dev_change_media_cb(BlockDriverState *bs);
+static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load);
 static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
@@ -688,7 +688,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
     }
 
     if (!bdrv_key_required(bs)) {
-        bdrv_dev_change_media_cb(bs);
+        bdrv_dev_change_media_cb(bs, true);
     }
 
     return 0;
@@ -724,7 +724,7 @@ void bdrv_close(BlockDriverState *bs)
             bdrv_close(bs->file);
         }
 
-        bdrv_dev_change_media_cb(bs);
+        bdrv_dev_change_media_cb(bs, false);
     }
 }
 
@@ -807,10 +807,10 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
     }
 }
 
-static void bdrv_dev_change_media_cb(BlockDriverState *bs)
+static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
 {
     if (bs->dev_ops && bs->dev_ops->change_media_cb) {
-        bs->dev_ops->change_media_cb(bs->dev_opaque);
+        bs->dev_ops->change_media_cb(bs->dev_opaque, load);
     }
 }
 
@@ -1674,7 +1674,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
     } else if (!bs->valid_key) {
         bs->valid_key = 1;
         /* call the change callback now, we skipped it on open */
-        bdrv_dev_change_media_cb(bs);
+        bdrv_dev_change_media_cb(bs, true);
     }
     return ret;
 }
diff --git a/block.h b/block.h
index 4b7fa35..16bfa0a 100644
--- a/block.h
+++ b/block.h
@@ -32,11 +32,12 @@ typedef struct QEMUSnapshotInfo {
 typedef struct BlockDevOps {
     /*
      * Runs when virtual media changed (monitor commands eject, change)
+     * Argument load is true on load and false on eject.
      * Beware: doesn't run when a host device's physical media
      * changes.  Sure would be useful if it did.
      * Device models with removable media must implement this callback.
      */
-    void (*change_media_cb)(void *opaque);
+    void (*change_media_cb)(void *opaque, bool load);
     /*
      * Is the virtual tray open?
      * Device models implement this only when the device has a tray.
diff --git a/hw/fdc.c b/hw/fdc.c
index 57eda0c..433af73 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1777,7 +1777,7 @@ static void fdctrl_result_timer(void *opaque)
     fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
 }
 
-static void fdctrl_change_cb(void *opaque)
+static void fdctrl_change_cb(void *opaque, bool load)
 {
     FDrive *drive = opaque;
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4fb7e61..bc83c46 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -784,7 +784,7 @@ static void ide_cfata_metadata_write(IDEState *s)
 }
 
 /* called when the inserted state of the media has changed */
-static void ide_cd_change_cb(void *opaque)
+static void ide_cd_change_cb(void *opaque, bool load)
 {
     IDEState *s = opaque;
     uint64_t nb_sectors;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b115760..f5f1d82 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1173,7 +1173,7 @@ static void scsi_destroy(SCSIDevice *dev)
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
-static void scsi_cd_change_media_cb(void *opaque)
+static void scsi_cd_change_media_cb(void *opaque, bool load)
 {
 }
 
diff --git a/hw/sd.c b/hw/sd.c
index 1af62b2..10e26ad 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -419,7 +419,7 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
     sd->pwd_len = 0;
 }
 
-static void sd_cardchange(void *opaque)
+static void sd_cardchange(void *opaque, bool load)
 {
     SDState *sd = opaque;
 
-- 
1.7.6

[Qemu-devel] [PATCH v3 27/27] ide/atapi scsi-disk: Make monitor eject -f, then change work

[Markus Armbruster]

change fails while the tray is locked by the guest.  eject -f forces
it open and removes any media.  Unfortunately, the tray closes again
instantly.  Since the lock remains as it is, there is no way to insert
another medium unless the guest voluntarily unlocks.

Fix by leaving the tray open after monitor eject.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c     |    3 ++-
 hw/ide/core.c  |    1 +
 hw/scsi-disk.c |    1 +
 3 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 154cc84..0827bf7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -635,7 +635,8 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
         qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
         return -1;
     }
-    if (!force && bdrv_dev_is_medium_locked(bs)) {
+    if (!force && !bdrv_dev_is_tray_open(bs)
+        && bdrv_dev_is_medium_locked(bs)) {
         qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
         return -1;
     }
diff --git a/hw/ide/core.c b/hw/ide/core.c
index bc83c46..db144aa 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -789,6 +789,7 @@ static void ide_cd_change_cb(void *opaque, bool load)
     IDEState *s = opaque;
     uint64_t nb_sectors;
 
+    s->tray_open = !load;
     bdrv_get_geometry(s->bs, &nb_sectors);
     s->nb_sectors = nb_sectors;
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f5f1d82..4a60820 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1175,6 +1175,7 @@ static void scsi_destroy(SCSIDevice *dev)
 
 static void scsi_cd_change_media_cb(void *opaque, bool load)
 {
+    ((SCSIDiskState *)opaque)->tray_open = !load;
 }
 
 static bool scsi_cd_is_tray_open(void *opaque)
-- 
1.7.6

Re: [Qemu-devel] [PATCH v3 06/27] scsi-disk: Factor out scsi_disk_emulate_start_stop()

[Paolo Bonzini]

On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   hw/scsi-disk.c |   17 +++++++++++++----
>   1 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 9724d0f..c8ad2e7 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -814,6 +814,18 @@ static int scsi_disk_emulate_read_toc(SCSIRequest *req, uint8_t *outbuf)
>       return toclen;
>   }
>
> +static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
> +{
> +    SCSIRequest *req =&r->req;
> +    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
> +    bool start = req->cmd.buf[4]&  1;
> +    bool loej = req->cmd.buf[4]&  2; /* load on start, eject on !start */
> +
> +    if (s->qdev.type == TYPE_ROM&&  loej) {
> +        bdrv_eject(s->bs, !start);
> +    }
> +}
> +
>   static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>   {
>       SCSIRequest *req =&r->req;
> @@ -859,10 +871,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>               goto illegal_request;
>           break;
>       case START_STOP:
> -        if (s->qdev.type == TYPE_ROM&&  (req->cmd.buf[4]&  2)) {
> -            /* load/eject medium */
> -            bdrv_eject(s->bs, !(req->cmd.buf[4]&  1));
> -        }
> +        scsi_disk_emulate_start_stop(r);
>           break;
>       case ALLOW_MEDIUM_REMOVAL:
>           bdrv_set_locked(s->bs, req->cmd.buf[4]&  1);

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v3 07/27] scsi-disk: Track tray open/close state

[Paolo Bonzini]

On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> We already track it in BlockDriverState since commit 4be9762a.  As
> discussed in that commit's message, we should track it in the device
> device models instead, because it's device state.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   hw/scsi-disk.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index c8ad2e7..f18ddd7 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -72,6 +72,7 @@ struct SCSIDiskState
>       QEMUBH *bh;
>       char *version;
>       char *serial;
> +    bool tray_open;
>   };
>
>   static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
> @@ -823,6 +824,7 @@ static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
>
>       if (s->qdev.type == TYPE_ROM&&  loej) {
>           bdrv_eject(s->bs, !start);
> +        s->tray_open = !start;
>       }
>   }
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v3 11/27] scsi-disk: Track tray locked state

[Paolo Bonzini]

On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> We already track it in BlockDriverState.  Just like tray open/close
> state, we should track it in the device models instead, because it's
> device state.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   hw/scsi-disk.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index f35ada4..e7358e3 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -73,6 +73,7 @@ struct SCSIDiskState
>       char *version;
>       char *serial;
>       bool tray_open;
> +    bool tray_locked;
>   };
>
>   static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
> @@ -671,7 +672,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>           p[5] = 0xff; /* CD DA, DA accurate, RW supported,
>                           RW corrected, C2 errors, ISRC,
>                           UPC, Bar code */
> -        p[6] = 0x2d | (bdrv_is_locked(s->bs)? 2 : 0);
> +        p[6] = 0x2d | (s->tray_locked ? 2 : 0);
>           /* Locking supported, jumper present, eject, tray */
>           p[7] = 0; /* no volume&  mute control, no
>                        changer */
> @@ -882,6 +883,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>           scsi_disk_emulate_start_stop(r);
>           break;
>       case ALLOW_MEDIUM_REMOVAL:
> +        s->tray_locked = req->cmd.buf[4]&  1;
>           bdrv_set_locked(s->bs, req->cmd.buf[4]&  1);
>           break;
>       case READ_CAPACITY_10:

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v3 13/27] block: Drop medium lock tracking, ask device models instead

[Paolo Bonzini]

On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> Requires new BlockDevOps member is_medium_locked().  Implement for IDE
> and SCSI CD-ROMs.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   block.c        |   16 +++++++++-------
>   block.h        |    7 ++++++-
>   block_int.h    |    1 -
>   blockdev.c     |    2 +-
>   hw/ide/core.c  |    6 ++++++
>   hw/scsi-disk.c |   10 ++++++++++
>   6 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/block.c b/block.c
> index 7408fa9..1e4be73 100644
> --- a/block.c
> +++ b/block.c
> @@ -818,6 +818,14 @@ static void bdrv_dev_resize_cb(BlockDriverState *bs)
>       }
>   }
>
> +bool bdrv_dev_is_medium_locked(BlockDriverState *bs)
> +{
> +    if (bs->dev_ops&&  bs->dev_ops->is_medium_locked) {
> +        return bs->dev_ops->is_medium_locked(bs->dev_opaque);
> +    }
> +    return false;
> +}
> +
>   /*
>    * Run consistency checks on an image
>    *
> @@ -1890,7 +1898,7 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
>           bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
>                                       "'removable': %i, 'locked': %i }",
>                                       bs->device_name, bs->removable,
> -                                    bs->locked);
> +                                    bdrv_dev_is_medium_locked(bs));
>
>           if (bs->drv) {
>               QObject *obj;
> @@ -3060,11 +3068,6 @@ void bdrv_eject(BlockDriverState *bs, int eject_flag)
>       }
>   }
>
> -int bdrv_is_locked(BlockDriverState *bs)
> -{
> -    return bs->locked;
> -}
> -
>   /**
>    * Lock or unlock the media (if it is locked, the user won't be able
>    * to eject it manually).
> @@ -3075,7 +3078,6 @@ void bdrv_set_locked(BlockDriverState *bs, int locked)
>
>       trace_bdrv_set_locked(bs, locked);
>
> -    bs->locked = locked;
>       if (drv&&  drv->bdrv_set_locked) {
>           drv->bdrv_set_locked(bs, locked);
>       }
> diff --git a/block.h b/block.h
> index 5d941e9..396ca0e 100644
> --- a/block.h
> +++ b/block.h
> @@ -37,6 +37,11 @@ typedef struct BlockDevOps {
>        */
>       void (*change_media_cb)(void *opaque);
>       /*
> +     * Is the virtual medium locked into the device?
> +     * Device models implement this only when device has such a lock.
> +     */
> +    bool (*is_medium_locked)(void *opaque);
> +    /*
>        * Runs when the size changed (e.g. monitor command block_resize)
>        */
>       void (*resize_cb)(void *opaque);
> @@ -94,6 +99,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev);
>   void *bdrv_get_attached_dev(BlockDriverState *bs);
>   void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
>                         void *opaque);
> +bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
>   int bdrv_read(BlockDriverState *bs, int64_t sector_num,
>                 uint8_t *buf, int nb_sectors);
>   int bdrv_write(BlockDriverState *bs, int64_t sector_num,
> @@ -206,7 +212,6 @@ int bdrv_is_sg(BlockDriverState *bs);
>   int bdrv_enable_write_cache(BlockDriverState *bs);
>   int bdrv_is_inserted(BlockDriverState *bs);
>   int bdrv_media_changed(BlockDriverState *bs);
> -int bdrv_is_locked(BlockDriverState *bs);
>   void bdrv_set_locked(BlockDriverState *bs, int locked);
>   void bdrv_eject(BlockDriverState *bs, int eject_flag);
>   void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
> diff --git a/block_int.h b/block_int.h
> index b63c57b..4f7ff3b 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -156,7 +156,6 @@ struct BlockDriverState {
>       int keep_read_only; /* if true, the media was requested to stay read only */
>       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 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* */
> diff --git a/blockdev.c b/blockdev.c
> index 049dda5..3f00b2e 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -640,7 +640,7 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
>           qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
>           return -1;
>       }
> -    if (!force&&  bdrv_is_locked(bs)) {
> +    if (!force&&  bdrv_dev_is_medium_locked(bs)) {
>           qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
>           return -1;
>       }
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 34b7b83..b1a73ee 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1795,8 +1795,14 @@ void ide_bus_reset(IDEBus *bus)
>       bus->dma->ops->reset(bus->dma);
>   }
>
> +static bool ide_cd_is_medium_locked(void *opaque)
> +{
> +    return ((IDEState *)opaque)->tray_locked;
> +}
> +
>   static const BlockDevOps ide_cd_block_ops = {
>       .change_media_cb = ide_cd_change_cb,
> +    .is_medium_locked = ide_cd_is_medium_locked,
>   };
>
>   int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 65783a7..42682d0 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -1165,6 +1165,15 @@ static void scsi_destroy(SCSIDevice *dev)
>       blockdev_mark_auto_del(s->qdev.conf.bs);
>   }
>
> +static bool scsi_cd_is_medium_locked(void *opaque)
> +{
> +    return ((SCSIDiskState *)opaque)->tray_locked;
> +}
> +
> +static const BlockDevOps scsi_cd_block_ops = {
> +    .is_medium_locked = scsi_cd_is_medium_locked,
> +};
> +
>   static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
>   {
>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> @@ -1199,6 +1208,7 @@ static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
>       }
>
>       if (scsi_type == TYPE_ROM) {
> +        bdrv_set_dev_ops(s->bs,&scsi_cd_block_ops, s);
>           s->qdev.blocksize = 2048;
>       } else if (scsi_type == TYPE_DISK) {
>           s->qdev.blocksize = s->qdev.conf.logical_block_size;

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v3 16/27] scsi-disk: Fix START_STOP to fail when it can't eject

[Paolo Bonzini]

On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> Don't fail when tray is already open.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   hw/scsi-bus.c  |   10 ++++++++++
>   hw/scsi-disk.c |   15 +++++++++++----
>   hw/scsi.h      |    4 ++++
>   3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 160eaee..79cb29d 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -772,6 +772,11 @@ const struct SCSISense sense_code_NO_MEDIUM = {
>       .key = NOT_READY, .asc = 0x3a, .ascq = 0x00
>   };
>
> +/* LUN not ready, medium removal prevented */
> +const struct SCSISense sense_code_NOT_READY_REMOVAL_PREVENTED = {
> +    .key = NOT_READY, .asc = 0x53, .ascq = 0x00
> +};
> +
>   /* Hardware error, internal target failure */
>   const struct SCSISense sense_code_TARGET_FAILURE = {
>       .key = HARDWARE_ERROR, .asc = 0x44, .ascq = 0x00
> @@ -807,6 +812,11 @@ const struct SCSISense sense_code_INCOMPATIBLE_MEDIUM = {
>       .key = ILLEGAL_REQUEST, .asc = 0x30, .ascq = 0x00
>   };
>
> +/* Illegal request, medium removal prevented */
> +const struct SCSISense sense_code_ILLEGAL_REQ_REMOVAL_PREVENTED = {
> +    .key = ILLEGAL_REQUEST, .asc = 0x53, .ascq = 0x00
> +};
> +
>   /* Command aborted, I/O process terminated */
>   const struct SCSISense sense_code_IO_ERROR = {
>       .key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 4e89bb1..1a49217 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -822,7 +822,7 @@ static int scsi_disk_emulate_read_toc(SCSIRequest *req, uint8_t *outbuf)
>       return toclen;
>   }
>
> -static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
> +static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
>   {
>       SCSIRequest *req =&r->req;
>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
> @@ -830,12 +830,17 @@ static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
>       bool loej = req->cmd.buf[4]&  2; /* load on start, eject on !start */
>
>       if (s->qdev.type == TYPE_ROM&&  loej) {
> -        if (!start&&  s->tray_locked) {
> -            return;
> +        if (!start&&  !s->tray_open&&  s->tray_locked) {
> +            scsi_check_condition(r,
> +                                 bdrv_is_inserted(s->bs)
> +                                 ? SENSE_CODE(ILLEGAL_REQ_REMOVAL_PREVENTED)
> +                                 : SENSE_CODE(NOT_READY_REMOVAL_PREVENTED));
> +            return -1;
>           }
>           bdrv_eject(s->bs, !start);
>           s->tray_open = !start;
>       }
> +    return 0;
>   }
>
>   static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
> @@ -883,7 +888,9 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>               goto illegal_request;
>           break;
>       case START_STOP:
> -        scsi_disk_emulate_start_stop(r);
> +        if (scsi_disk_emulate_start_stop(r)<  0) {
> +            return -1;
> +        }
>           break;
>       case ALLOW_MEDIUM_REMOVAL:
>           s->tray_locked = req->cmd.buf[4]&  1;
> diff --git a/hw/scsi.h b/hw/scsi.h
> index 98fd689..a28cd68 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -136,6 +136,8 @@ extern const struct SCSISense sense_code_NO_SENSE;
>   extern const struct SCSISense sense_code_LUN_NOT_READY;
>   /* LUN not ready, Medium not present */
>   extern const struct SCSISense sense_code_NO_MEDIUM;
> +/* LUN not ready, medium removal prevented */
> +extern const struct SCSISense sense_code_NOT_READY_REMOVAL_PREVENTED;
>   /* Hardware error, internal target failure */
>   extern const struct SCSISense sense_code_TARGET_FAILURE;
>   /* Illegal request, invalid command operation code */
> @@ -150,6 +152,8 @@ extern const struct SCSISense sense_code_LUN_NOT_SUPPORTED;
>   extern const struct SCSISense sense_code_SAVING_PARAMS_NOT_SUPPORTED;
>   /* Illegal request, Incompatible format */
>   extern const struct SCSISense sense_code_INCOMPATIBLE_FORMAT;
> +/* Illegal request, medium removal prevented */
> +extern const struct SCSISense sense_code_ILLEGAL_REQ_REMOVAL_PREVENTED;
>   /* Command aborted, I/O process terminated */
>   extern const struct SCSISense sense_code_IO_ERROR;
>   /* Command aborted, I_T Nexus loss occurred */

Matches MMC6, paragraph 6.42.3.1.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v3 27/27] ide/atapi scsi-disk: Make monitor eject -f, then change work

[Paolo Bonzini]

On 09/06/2011 06:59 PM, Markus Armbruster wrote:
> change fails while the tray is locked by the guest.  eject -f forces
> it open and removes any media.  Unfortunately, the tray closes again
> instantly.  Since the lock remains as it is, there is no way to insert
> another medium unless the guest voluntarily unlocks.
>
> Fix by leaving the tray open after monitor eject.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   blockdev.c     |    3 ++-
>   hw/ide/core.c  |    1 +
>   hw/scsi-disk.c |    1 +
>   3 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 154cc84..0827bf7 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -635,7 +635,8 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
>           qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
>           return -1;
>       }
> -    if (!force&&  bdrv_dev_is_medium_locked(bs)) {
> +    if (!force&&  !bdrv_dev_is_tray_open(bs)
> +&&  bdrv_dev_is_medium_locked(bs)) {
>           qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
>           return -1;
>       }
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index bc83c46..db144aa 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -789,6 +789,7 @@ static void ide_cd_change_cb(void *opaque, bool load)
>       IDEState *s = opaque;
>       uint64_t nb_sectors;
>
> +    s->tray_open = !load;
>       bdrv_get_geometry(s->bs,&nb_sectors);
>       s->nb_sectors = nb_sectors;
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index f5f1d82..4a60820 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -1175,6 +1175,7 @@ static void scsi_destroy(SCSIDevice *dev)
>
>   static void scsi_cd_change_media_cb(void *opaque, bool load)
>   {
> +    ((SCSIDiskState *)opaque)->tray_open = !load;
>   }
>
>   static bool scsi_cd_is_tray_open(void *opaque)

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v3 17/27] ide/atapi: Preserve tray state on migration

[Paolo Bonzini]

On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> Use a subsection, so that migration to older version still works,
> provided the tray is closed and unlocked.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   hw/ide/core.c |   32 ++++++++++++++++++++++++++++++++
>   1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index b1a73ee..30cb7de 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2058,6 +2058,22 @@ static bool ide_drive_pio_state_needed(void *opaque)
>           || (s->bus->error_status&  BM_STATUS_PIO_RETRY);
>   }
>
> +static int ide_tray_state_post_load(void *opaque, int version_id)
> +{
> +    IDEState *s = opaque;
> +
> +    bdrv_eject(s->bs, s->tray_open);
> +    bdrv_lock_medium(s->bs, s->tray_locked);
> +    return 0;
> +}
> +
> +static bool ide_tray_state_needed(void *opaque)
> +{
> +    IDEState *s = opaque;
> +
> +    return s->tray_open || s->tray_locked;

I wonder if the most common case is this, or rather "tray closed and 
locked".  Perhaps it depends (for Windows it is likely unlocked, for 
Linux locked).  In any case there's time before 1.0 to fix this, so

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCH v3 17/27] ide/atapi: Preserve tray state on migration

[Kevin Wolf]

Am 07.09.2011 09:14, schrieb Paolo Bonzini:
> On 09/06/2011 06:58 PM, Markus Armbruster wrote:
>> Use a subsection, so that migration to older version still works,
>> provided the tray is closed and unlocked.
>>
>> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>> ---
>>   hw/ide/core.c |   32 ++++++++++++++++++++++++++++++++
>>   1 files changed, 32 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index b1a73ee..30cb7de 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2058,6 +2058,22 @@ static bool ide_drive_pio_state_needed(void *opaque)
>>           || (s->bus->error_status&  BM_STATUS_PIO_RETRY);
>>   }
>>
>> +static int ide_tray_state_post_load(void *opaque, int version_id)
>> +{
>> +    IDEState *s = opaque;
>> +
>> +    bdrv_eject(s->bs, s->tray_open);
>> +    bdrv_lock_medium(s->bs, s->tray_locked);
>> +    return 0;
>> +}
>> +
>> +static bool ide_tray_state_needed(void *opaque)
>> +{
>> +    IDEState *s = opaque;
>> +
>> +    return s->tray_open || s->tray_locked;
> 
> I wonder if the most common case is this, or rather "tray closed and 
> locked".  Perhaps it depends (for Windows it is likely unlocked, for 
> Linux locked).  In any case there's time before 1.0 to fix this, so

I would argue that the common case even for Linux is that you don't have
a CD mounted (probably the drive is empty anyway).

Kevin

Re: [Qemu-devel] [PATCH v3 17/27] ide/atapi: Preserve tray state on migration

[Paolo Bonzini]

On 09/07/2011 09:35 AM, Kevin Wolf wrote:
> >  I wonder if the most common case is this, or rather "tray closed and
> >  locked".  Perhaps it depends (for Windows it is likely unlocked, for
> >  Linux locked).  In any case there's time before 1.0 to fix this, so
>
> I would argue that the common case even for Linux is that you don't have
> a CD mounted (probably the drive is empty anyway).

Indeed---I was thinking about automounting by a GUI, but that's not the 
typical server case.

Paolo

Re: [Qemu-devel] [PATCH v3 02/27] ide: Use a table to declare which drive kinds accept each command

[Kevin Wolf]

Am 06.09.2011 18:58, schrieb Markus Armbruster:
> No functional change.
> 
> It would be nice to have handler functions in the table, like commit
> e1a064f9 did for ATAPI.  Left for another day.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/ide/core.c |  105 +++++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 80 insertions(+), 25 deletions(-)

> +    [IBM_SENSE_CONDITION]               = CFA_OK,
> +    [CFA_WEAR_LEVEL]                    = CFA_OK,
> +    [WIN_READ_NATIVE_MAX]               = ALL_OK,
> +};
> +
> +static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
> +{
> +    return cmd <= ARRAY_SIZE(ide_cmd_table)

Shouldn't it be < instead of <= ?

Kevin

Re: [Qemu-devel] [PATCH v3 14/27] block: Rename bdrv_set_locked() to bdrv_lock_medium()

[Kevin Wolf]

Am 06.09.2011 18:58, schrieb Markus Armbruster:
> While there, make the locked parameter bool.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block.c           |    8 ++++----
>  block.h           |    2 +-
>  block/raw-posix.c |    8 ++++----
>  block/raw.c       |    6 +++---
>  block_int.h       |    2 +-
>  hw/ide/atapi.c    |    2 +-
>  hw/scsi-disk.c    |    2 +-
>  trace-events      |    1 +
>  8 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 1e4be73..7225b15 100644
> --- a/block.c
> +++ b/block.c
> @@ -3072,14 +3072,14 @@ void bdrv_eject(BlockDriverState *bs, int eject_flag)
>   * Lock or unlock the media (if it is locked, the user won't be able
>   * to eject it manually).
>   */
> -void bdrv_set_locked(BlockDriverState *bs, int locked)
> +void bdrv_lock_medium(BlockDriverState *bs, bool locked)
>  {
>      BlockDriver *drv = bs->drv;
>  
> -    trace_bdrv_set_locked(bs, locked);
> +    trace_bdrv_lock_medium(bs, locked);
>  
> -    if (drv && drv->bdrv_set_locked) {
> -        drv->bdrv_set_locked(bs, locked);
> +    if (drv && drv->bdrv_lock_medium) {
> +        drv->bdrv_lock_medium(bs, locked);
>      }
>  }
>  
> diff --git a/block.h b/block.h
> index 396ca0e..4691090 100644
> --- a/block.h
> +++ b/block.h
> @@ -212,7 +212,7 @@ int bdrv_is_sg(BlockDriverState *bs);
>  int bdrv_enable_write_cache(BlockDriverState *bs);
>  int bdrv_is_inserted(BlockDriverState *bs);
>  int bdrv_media_changed(BlockDriverState *bs);
> -void bdrv_set_locked(BlockDriverState *bs, int locked);
> +void bdrv_lock_medium(BlockDriverState *bs, bool locked);
>  void bdrv_eject(BlockDriverState *bs, int eject_flag);
>  void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
>  BlockDriverState *bdrv_find(const char *name);
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index bcf50b2..a624f56 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1362,7 +1362,7 @@ static void cdrom_eject(BlockDriverState *bs, int eject_flag)
>      }
>  }
>  
> -static void cdrom_set_locked(BlockDriverState *bs, int locked)
> +static void cdrom_lock_medium(BlockDriverState *bs, bool locked)
>  {
>      BDRVRawState *s = bs->opaque;
>  
> @@ -1400,7 +1400,7 @@ static BlockDriver bdrv_host_cdrom = {
>      /* removable device support */
>      .bdrv_is_inserted   = cdrom_is_inserted,
>      .bdrv_eject         = cdrom_eject,
> -    .bdrv_set_locked    = cdrom_set_locked,
> +    .bdrv_lock_medium   = cdrom_lock_medium,
>  
>      /* generic scsi device */
>      .bdrv_ioctl         = hdev_ioctl,
> @@ -1481,7 +1481,7 @@ static void cdrom_eject(BlockDriverState *bs, int eject_flag)
>      cdrom_reopen(bs);
>  }
>  
> -static void cdrom_set_locked(BlockDriverState *bs, int locked)
> +static void cdrom_lock_medium(BlockDriverState *bs, bool locked)
>  {
>      BDRVRawState *s = bs->opaque;
>  
> @@ -1521,7 +1521,7 @@ static BlockDriver bdrv_host_cdrom = {
>      /* removable device support */
>      .bdrv_is_inserted   = cdrom_is_inserted,
>      .bdrv_eject         = cdrom_eject,
> -    .bdrv_set_locked    = cdrom_set_locked,
> +    .bdrv_lock_medium   = cdrom_lock_medium,
>  };
>  #endif /* __FreeBSD__ */
>  
> diff --git a/block/raw.c b/block/raw.c
> index f197479..63cf2d3 100644
> --- a/block/raw.c
> +++ b/block/raw.c
> @@ -85,9 +85,9 @@ static void raw_eject(BlockDriverState *bs, int eject_flag)
>      bdrv_eject(bs->file, eject_flag);
>  }
>  
> -static void raw_set_locked(BlockDriverState *bs, int locked)
> +static void raw_lock_medium(BlockDriverState *bs, bool locked)
>  {
> -    bdrv_set_locked(bs->file, locked);
> +    bdrv_lock_medium(bs->file, locked);
>  }
>  
>  static int raw_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
> @@ -144,7 +144,7 @@ static BlockDriver bdrv_raw = {
>      .bdrv_is_inserted   = raw_is_inserted,
>      .bdrv_media_changed = raw_media_changed,
>      .bdrv_eject         = raw_eject,
> -    .bdrv_set_locked    = raw_set_locked,
> +    .bdrv_lock_medium   = raw_lock_medium,
>  
>      .bdrv_ioctl         = raw_ioctl,
>      .bdrv_aio_ioctl     = raw_aio_ioctl,
> diff --git a/block_int.h b/block_int.h
> index 4f7ff3b..f42af2c 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -120,7 +120,7 @@ struct BlockDriver {
>      int (*bdrv_is_inserted)(BlockDriverState *bs);
>      int (*bdrv_media_changed)(BlockDriverState *bs);
>      void (*bdrv_eject)(BlockDriverState *bs, int eject_flag);
> -    void (*bdrv_set_locked)(BlockDriverState *bs, int locked);
> +    void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
>  
>      /* to control generic scsi devices */
>      int (*bdrv_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf);
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index afb27c6..06778f3 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -833,7 +833,7 @@ static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
>  static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
>  {
>      s->tray_locked = buf[4] & 1;
> -    bdrv_set_locked(s->bs, buf[4] & 1);
> +    bdrv_lock_medium(s->bs, buf[4] & 1);
>      ide_atapi_cmd_ok(s);
>  }
>  
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 42682d0..4e89bb1 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -887,7 +887,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>          break;
>      case ALLOW_MEDIUM_REMOVAL:
>          s->tray_locked = req->cmd.buf[4] & 1;
> -        bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
> +        bdrv_lock_medium(s->bs, req->cmd.buf[4] & 1);
>          break;
>      case READ_CAPACITY_10:
>          /* The normal LEN field for this command is zero.  */
> diff --git a/trace-events b/trace-events
> index 08ffedf..1c0b49b 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -62,6 +62,7 @@ bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
>  bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p"
>  bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
>  bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
> +bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
>  bdrv_set_locked(void *bs, int locked) "bs %p locked %d"

Do we still need trace_bdrv_set_locked() now? v2 did remove it and I
can't see any comment that suggested that it was still needed.

Kevin

Re: [Qemu-devel] [PATCH v3 02/27] ide: Use a table to declare which drive kinds accept each command

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 06.09.2011 18:58, schrieb Markus Armbruster:
>> No functional change.
>> 
>> It would be nice to have handler functions in the table, like commit
>> e1a064f9 did for ATAPI.  Left for another day.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/ide/core.c |  105 +++++++++++++++++++++++++++++++++++++++++++-------------
>>  1 files changed, 80 insertions(+), 25 deletions(-)
>
>> +    [IBM_SENSE_CONDITION]               = CFA_OK,
>> +    [CFA_WEAR_LEVEL]                    = CFA_OK,
>> +    [WIN_READ_NATIVE_MAX]               = ALL_OK,
>> +};
>> +
>> +static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
>> +{
>> +    return cmd <= ARRAY_SIZE(ide_cmd_table)
>
> Shouldn't it be < instead of <= ?

I plead temporary insanity.  Want a v4, or want to fix it up yourself?

Re: [Qemu-devel] [PATCH v3 14/27] block: Rename bdrv_set_locked() to bdrv_lock_medium()

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 06.09.2011 18:58, schrieb Markus Armbruster:
>> While there, make the locked parameter bool.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  block.c           |    8 ++++----
>>  block.h           |    2 +-
>>  block/raw-posix.c |    8 ++++----
>>  block/raw.c       |    6 +++---
>>  block_int.h       |    2 +-
>>  hw/ide/atapi.c    |    2 +-
>>  hw/scsi-disk.c    |    2 +-
>>  trace-events      |    1 +
>>  8 files changed, 16 insertions(+), 15 deletions(-)
>> 
>> diff --git a/block.c b/block.c
>> index 1e4be73..7225b15 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3072,14 +3072,14 @@ void bdrv_eject(BlockDriverState *bs, int eject_flag)
>>   * Lock or unlock the media (if it is locked, the user won't be able
>>   * to eject it manually).
>>   */
>> -void bdrv_set_locked(BlockDriverState *bs, int locked)
>> +void bdrv_lock_medium(BlockDriverState *bs, bool locked)
>>  {
>>      BlockDriver *drv = bs->drv;
>>  
>> -    trace_bdrv_set_locked(bs, locked);
>> +    trace_bdrv_lock_medium(bs, locked);
>>  
>> -    if (drv && drv->bdrv_set_locked) {
>> -        drv->bdrv_set_locked(bs, locked);
>> +    if (drv && drv->bdrv_lock_medium) {
>> +        drv->bdrv_lock_medium(bs, locked);
>>      }
>>  }
>>  
>> diff --git a/block.h b/block.h
>> index 396ca0e..4691090 100644
>> --- a/block.h
>> +++ b/block.h
>> @@ -212,7 +212,7 @@ int bdrv_is_sg(BlockDriverState *bs);
>>  int bdrv_enable_write_cache(BlockDriverState *bs);
>>  int bdrv_is_inserted(BlockDriverState *bs);
>>  int bdrv_media_changed(BlockDriverState *bs);
>> -void bdrv_set_locked(BlockDriverState *bs, int locked);
>> +void bdrv_lock_medium(BlockDriverState *bs, bool locked);
>>  void bdrv_eject(BlockDriverState *bs, int eject_flag);
>>  void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
>>  BlockDriverState *bdrv_find(const char *name);
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index bcf50b2..a624f56 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -1362,7 +1362,7 @@ static void cdrom_eject(BlockDriverState *bs, int eject_flag)
>>      }
>>  }
>>  
>> -static void cdrom_set_locked(BlockDriverState *bs, int locked)
>> +static void cdrom_lock_medium(BlockDriverState *bs, bool locked)
>>  {
>>      BDRVRawState *s = bs->opaque;
>>  
>> @@ -1400,7 +1400,7 @@ static BlockDriver bdrv_host_cdrom = {
>>      /* removable device support */
>>      .bdrv_is_inserted   = cdrom_is_inserted,
>>      .bdrv_eject         = cdrom_eject,
>> -    .bdrv_set_locked    = cdrom_set_locked,
>> +    .bdrv_lock_medium   = cdrom_lock_medium,
>>  
>>      /* generic scsi device */
>>      .bdrv_ioctl         = hdev_ioctl,
>> @@ -1481,7 +1481,7 @@ static void cdrom_eject(BlockDriverState *bs, int eject_flag)
>>      cdrom_reopen(bs);
>>  }
>>  
>> -static void cdrom_set_locked(BlockDriverState *bs, int locked)
>> +static void cdrom_lock_medium(BlockDriverState *bs, bool locked)
>>  {
>>      BDRVRawState *s = bs->opaque;
>>  
>> @@ -1521,7 +1521,7 @@ static BlockDriver bdrv_host_cdrom = {
>>      /* removable device support */
>>      .bdrv_is_inserted   = cdrom_is_inserted,
>>      .bdrv_eject         = cdrom_eject,
>> -    .bdrv_set_locked    = cdrom_set_locked,
>> +    .bdrv_lock_medium   = cdrom_lock_medium,
>>  };
>>  #endif /* __FreeBSD__ */
>>  
>> diff --git a/block/raw.c b/block/raw.c
>> index f197479..63cf2d3 100644
>> --- a/block/raw.c
>> +++ b/block/raw.c
>> @@ -85,9 +85,9 @@ static void raw_eject(BlockDriverState *bs, int eject_flag)
>>      bdrv_eject(bs->file, eject_flag);
>>  }
>>  
>> -static void raw_set_locked(BlockDriverState *bs, int locked)
>> +static void raw_lock_medium(BlockDriverState *bs, bool locked)
>>  {
>> -    bdrv_set_locked(bs->file, locked);
>> +    bdrv_lock_medium(bs->file, locked);
>>  }
>>  
>>  static int raw_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
>> @@ -144,7 +144,7 @@ static BlockDriver bdrv_raw = {
>>      .bdrv_is_inserted   = raw_is_inserted,
>>      .bdrv_media_changed = raw_media_changed,
>>      .bdrv_eject         = raw_eject,
>> -    .bdrv_set_locked    = raw_set_locked,
>> +    .bdrv_lock_medium   = raw_lock_medium,
>>  
>>      .bdrv_ioctl         = raw_ioctl,
>>      .bdrv_aio_ioctl     = raw_aio_ioctl,
>> diff --git a/block_int.h b/block_int.h
>> index 4f7ff3b..f42af2c 100644
>> --- a/block_int.h
>> +++ b/block_int.h
>> @@ -120,7 +120,7 @@ struct BlockDriver {
>>      int (*bdrv_is_inserted)(BlockDriverState *bs);
>>      int (*bdrv_media_changed)(BlockDriverState *bs);
>>      void (*bdrv_eject)(BlockDriverState *bs, int eject_flag);
>> -    void (*bdrv_set_locked)(BlockDriverState *bs, int locked);
>> +    void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
>>  
>>      /* to control generic scsi devices */
>>      int (*bdrv_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf);
>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>> index afb27c6..06778f3 100644
>> --- a/hw/ide/atapi.c
>> +++ b/hw/ide/atapi.c
>> @@ -833,7 +833,7 @@ static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
>>  static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
>>  {
>>      s->tray_locked = buf[4] & 1;
>> -    bdrv_set_locked(s->bs, buf[4] & 1);
>> +    bdrv_lock_medium(s->bs, buf[4] & 1);
>>      ide_atapi_cmd_ok(s);
>>  }
>>  
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index 42682d0..4e89bb1 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -887,7 +887,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>>          break;
>>      case ALLOW_MEDIUM_REMOVAL:
>>          s->tray_locked = req->cmd.buf[4] & 1;
>> -        bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
>> +        bdrv_lock_medium(s->bs, req->cmd.buf[4] & 1);
>>          break;
>>      case READ_CAPACITY_10:
>>          /* The normal LEN field for this command is zero.  */
>> diff --git a/trace-events b/trace-events
>> index 08ffedf..1c0b49b 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -62,6 +62,7 @@ bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
>>  bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p"
>>  bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
>>  bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
>> +bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
>>  bdrv_set_locked(void *bs, int locked) "bs %p locked %d"
>
> Do we still need trace_bdrv_set_locked() now? v2 did remove it and I
> can't see any comment that suggested that it was still needed.

Merge turd, sorry.  Want a v4, or want to fix it up yourself?

Re: [Qemu-devel] [PATCH v3 00/27] Block layer cleanup & fixes

[Kevin Wolf]

Am 06.09.2011 18:58, schrieb Markus Armbruster:
> This patch series looks bigger than it is.  All the patches are small
> and hopefully easy to review.
> 
> Objectives:
> 
> * Push BlockDriverState members locked, tray_open, media_changed into
>   device models, where they belong.  Kevin picked the patches pushing
>   media_changed from v2, so that part's gone already.
> 
> * BlockDriverState member removable is a confusing mess, replace it.
> 
> * Improve eject -f.
> 
> Also clean up minor messes as they get in the way.
> 
> It is based on Kevin's block branch.
> 
> Part I: Move tray state to device models
> PATCH 01-05 IDE tray open/closed
> PATCH 06-07 SCSI tray open/closed
> PATCH 08-09 Kill BlockDriverState tray_open
> PATCH 10-11 IDE & SCSI track tray lock
> PATCH 12-14 Kill BlockDriverState locked
> PATCH 15-16 IDE & SCSI tray bug fixes
> PATCH 17    IDE migrate tray state
> 
> Part II: Miscellaneous
> PATCH 18-19 Replace BlockDriverState removable
> PATCH 20    Cover tray open/closed in info block
> PATCH 21-25 Reduce unclean use of block_int.h
> PATCH 26-27 Improve eject -f
> 
> Naturally, I want all parts applied.  But I did my best to make
> applying only a prefix workable, too.
> 
> Review invited from:
> 
> * Kevin, Christoph and Amit reviewed previous versions.
> 
> * Hannes ACKed the SCSI stuff in v2.
> 
> * Luiz reviewed the patches that affect QMP's query-block.  I renamed
>   response member "ejected" to "tray-open" since then.
> 
> * Paolo commented PATCH 17 `ide/atapi: Preserve tray state on
>   migration'.
> 
> * Stefano reviewed v1 of PATCH 18 (affects "-drive if=xen").
> 
> Testing
> 
> * Linux installs from CD to empty disk, then boots fine from disk.
> 
> * For both IDE and SCSI:
> 
>   - info block reports tray state correctly
> 
>   - Guest locking the tray stops eject (without -f) and change
> 
>   - eject -f; change works even while tray is locked by guest
> 
>   - Reading /dev/sr0 with tray open behaves as before: IDE closes the
>     tray and reads (matches bare metal), SCSI reports no medium
> 
>   - Tray state is migrated correctly (tested with savevm/loadvm)
> 
> * Guest still notices CD media change (IDE only, SCSI doesn't work
>   before or after my patches because GESN isn't implemented)
> 
> * Migrating ide-cd to older version works when tray is closed and
>   unlocked, else fails (tested with savevm/loadvm)
> 
> 
> v3:
> 
> * Rebased to block branch cfc606da
>   - Old PATCH 01-05,25,28-34,40 already there, drop
>   - a couple of simple conflicts in hw/scsi-disk.c
> 
> * Drop old PATCH v2 27 scsi-disk: Preserve tray state on migration,
>   because it doesn't make much sense without working SCSI migration.
> 
> * Drop old PATCH v2 22 ide/atapi: Avoid physical/virtual tray state
>   mismatch, because it has a bug, how to best fix it isn't obvious,
>   and it's not essential to this series.  Drop related PATCH v2 20,24,
>   too.  I plan to revisit them later.
> 
> * Clean up `ide: Use a table to declare which drive kinds accept each
>   command' a bit (Blue & Kevin)
> 
> * Hannes's advice:
>   - Rename some SCSISense variables
> 
> * Kevin's advice:
>   - Add comments to explain MMC-5 jargon loej
>   - Make bdrv_lock_medium() parameter locked bool.
> 
> v2:
> 
> * Rebased to block branch; non-trivial conflicts:
>   - Old PATCH 01-02,06-09 already there, drop
>   - `block: Attach non-qdev devices as well':
>     - cover new pci_piix3_xen_ide_unplug()
>     - hw/nand has been qdefivied, drop hunk
>     - onenand_init() changed, rewrite hunk
>   - pci_piix3_xen_ide_unplug() needs new PATCH 33.
> 
> * Drop old PATCH 18 `scsi-disk: Reject CD-specific SCSI commands to
>   disks' because Hannes wants to do it differently, and it's not
>   essential to this series.
> 
> * Christoph's advice:
>   - Rework `ide: Update command code definitions as per ACS-2'
>   - Add comment to `ide: Fix ATA command READ to set ATAPI signature
>     for CD-ROM'
>   - Squash `ide/atapi: Track tray open/close state' and `ide/atapi:
>     Switch from BlockDriverState's tray_open to own'
>   - Squash `ide/atapi: Track tray locked state' and `ide/atapi: Switch
>     from BlockDriverState's locked to own tray_locked'
>   - Squash `scsi-disk: Track tray locked state' and `scsi-disk: Switch
>     from BlockDriverState's locked to own tray_locked'
>   - Drop `block: Move BlockDriverAIOCB & friends from block_int.h to
>     block.h'
> 
> * Luiz's advice:
>   - Change query-block to always include "ejected" for removable
>     devices.  Requires moving `block: Show whether the guest ejected
>     the medium in info block', which causes a bunch of conflicts.
> 
> * A few cosmetic improvements
> 
> 
> Markus Armbruster (27):
>   ide: Fix ATA command READ to set ATAPI signature for CD-ROM
>   ide: Use a table to declare which drive kinds accept each command
>   ide: Reject ATA commands specific to drive kinds
>   ide/atapi: Clean up misleading name in cmd_start_stop_unit()
>   ide/atapi: Track tray open/close state
>   scsi-disk: Factor out scsi_disk_emulate_start_stop()
>   scsi-disk: Track tray open/close state
>   block: Revert entanglement of bdrv_is_inserted() with tray status
>   block: Drop tray status tracking, no longer used
>   ide/atapi: Track tray locked state
>   scsi-disk: Track tray locked state
>   block: Leave enforcing tray lock to device models
>   block: Drop medium lock tracking, ask device models instead
>   block: Rename bdrv_set_locked() to bdrv_lock_medium()
>   ide/atapi: Don't fail eject when tray is already open
>   scsi-disk: Fix START_STOP to fail when it can't eject
>   ide/atapi: Preserve tray state on migration
>   block: Clean up remaining users of "removable"
>   block: Drop BlockDriverState member removable
>   block: Show whether the virtual tray is open in info block
>   block: Move BlockConf & friends from block_int.h to block.h
>   hw: Trim superfluous #include "block_int.h"
>   block: New bdrv_set_buffer_alignment()
>   block: Reset buffer alignment on detach
>   nbd: Clean up use of block_int.h
>   block: New change_media_cb() parameter load
>   ide/atapi scsi-disk: Make monitor eject -f, then change work
> 
>  block.c             |  104 ++++++++++++++++++---------------
>  block.h             |   63 ++++++++++++++++++--
>  block/nbd.c         |    1 +
>  block/raw-posix.c   |    8 +-
>  block/raw.c         |    6 +-
>  block_int.h         |   40 +------------
>  blockdev.c          |   10 +--
>  hw/fdc.c            |    4 +-
>  hw/ide/atapi.c      |   58 +++++++++----------
>  hw/ide/cmd646.c     |    1 -
>  hw/ide/core.c       |  160 +++++++++++++++++++++++++++++++++++++++++---------
>  hw/ide/ich.c        |    1 -
>  hw/ide/internal.h   |    3 +-
>  hw/ide/isa.c        |    1 -
>  hw/ide/macio.c      |    1 -
>  hw/ide/microdrive.c |    1 -
>  hw/ide/mmio.c       |    1 -
>  hw/ide/pci.c        |    1 -
>  hw/ide/via.c        |    1 -
>  hw/lsi53c895a.c     |    1 -
>  hw/scsi-bus.c       |   10 +++
>  hw/scsi-disk.c      |   69 +++++++++++++++++++---
>  hw/scsi-generic.c   |    1 -
>  hw/scsi.h           |    5 +-
>  hw/sd.c             |    2 +-
>  hw/virtio-blk.c     |    3 +-
>  hw/virtio.h         |    2 +-
>  nbd.c               |    1 +
>  nbd.h               |    2 -
>  qmp-commands.hx     |    2 +
>  trace-events        |    1 +
>  31 files changed, 367 insertions(+), 197 deletions(-)

Thanks, applied all to the block branch after fixing up patch 2 and 14
as discussed.

Kevin