Merge remote-tracking branch 'stefanha/block' into staging

# By Kevin Wolf (22) and Fam Zheng (1)
# Via Stefan Hajnoczi
* stefanha/block: (23 commits)
  vmdk: refuse to open higher version than supported
  block: Always enable discard on the protocol level
  qcow2: Batch discards
  qcow2: Options to enable discard for freed clusters
  qcow2: Add refcount update reason to all callers
  Revert "block: Disable driver-specific options for 1.5"
  ide: Clean up ide_exec_cmd()
  ide: Convert SMART commands to ide_cmd_table handler
  ide: Convert CF-ATA commands to ide_cmd_table handler
  ide: Convert ATAPI commands to ide_cmd_table handler
  ide: Convert SEEK to ide_cmd_table handler
  ide: Convert FLUSH CACHE to ide_cmd_table handler
  ide: Convert SET FEATURES to ide_cmd_table handler
  ide: Convert CHECK POWER MDOE to ide_cmd_table handler
  ide: Convert READ NATIVE MAX ADDRESS to ide_cmd_table handler
  ide: Convert DMA read/write commands to ide_cmd_table handler
  ide: Convert PIO read/write commands to ide_cmd_table handler
  ide: Convert read/write multiple commands to ide_cmd_table handler
  ide: Convert verify commands to ide_cmd_table handler
  ide: Convert cmd_nop commands to ide_cmd_table handler
  ...

Message-id: 1372065035-19601-1-git-send-email-stefanha@redhat.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

block.c

@@ -1045,7 +1045,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     extract_subqdict(options, &file_options, "file.");
 
     ret = bdrv_file_open(&file, filename, file_options,
-                         bdrv_open_flags(bs, flags));
+                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
     if (ret < 0) {
         goto fail;
     }

block/qcow2-cluster.c

@@ -98,14 +98,16 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
         goto fail;
     }
     g_free(s->l1_table);
-    qcow2_free_clusters(bs, s->l1_table_offset, s->l1_size * sizeof(uint64_t));
+    qcow2_free_clusters(bs, s->l1_table_offset, s->l1_size * sizeof(uint64_t),
+                        QCOW2_DISCARD_OTHER);
     s->l1_table_offset = new_l1_table_offset;
     s->l1_table = new_l1_table;
     s->l1_size = new_l1_size;
     return 0;
  fail:
     g_free(new_l1_table);
-    qcow2_free_clusters(bs, new_l1_table_offset, new_l1_size2);
+    qcow2_free_clusters(bs, new_l1_table_offset, new_l1_size2,
+                        QCOW2_DISCARD_OTHER);
     return ret;
 }
 
@@ -548,7 +550,8 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset,
 
         /* Then decrease the refcount of the old table */
         if (l2_offset) {
-            qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t));
+            qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t),
+                                QCOW2_DISCARD_OTHER);
         }
     }
 
@@ -715,10 +718,14 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
     /*
      * If this was a COW, we need to decrease the refcount of the old cluster.
      * Also flush bs->file to get the right order for L2 and refcount update.
+     *
+     * Don't discard clusters that reach a refcount of 0 (e.g. compressed
+     * clusters), the next write will reuse them anyway.
      */
     if (j != 0) {
         for (i = 0; i < j; i++) {
-            qcow2_free_any_clusters(bs, be64_to_cpu(old_cluster[i]), 1);
+            qcow2_free_any_clusters(bs, be64_to_cpu(old_cluster[i]), 1,
+                                    QCOW2_DISCARD_NEVER);
         }
     }
 
@@ -1339,7 +1346,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
         l2_table[l2_index + i] = cpu_to_be64(0);
 
         /* Then decrease the refcount */
-        qcow2_free_any_clusters(bs, old_offset, 1);
+        qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
     }
 
     ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
@@ -1370,18 +1377,25 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
 
     nb_clusters = size_to_clusters(s, end_offset - offset);
 
+    s->cache_discards = true;
+
     /* Each L2 table is handled by its own loop iteration */
     while (nb_clusters > 0) {
         ret = discard_single_l2(bs, offset, nb_clusters);
         if (ret < 0) {
-            return ret;
+            goto fail;
         }
 
         nb_clusters -= ret;
         offset += (ret * s->cluster_size);
     }
 
-    return 0;
+    ret = 0;
+fail:
+    s->cache_discards = false;
+    qcow2_process_discards(bs, ret);
+
+    return ret;
 }
 
 /*
@@ -1415,7 +1429,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
         qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
         if (old_offset & QCOW_OFLAG_COMPRESSED) {
             l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
-            qcow2_free_any_clusters(bs, old_offset, 1);
+            qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
         } else {
             l2_table[l2_index + i] |= cpu_to_be64(QCOW_OFLAG_ZERO);
         }
@@ -1443,15 +1457,22 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors)
     /* Each L2 table is handled by its own loop iteration */
     nb_clusters = size_to_clusters(s, nb_sectors << BDRV_SECTOR_BITS);
 
+    s->cache_discards = true;
+
     while (nb_clusters > 0) {
         ret = zero_single_l2(bs, offset, nb_clusters);
         if (ret < 0) {
-            return ret;
+            goto fail;
         }
 
         nb_clusters -= ret;
         offset += (ret * s->cluster_size);
     }
 
-    return 0;
+    ret = 0;
+fail:
+    s->cache_discards = false;
+    qcow2_process_discards(bs, ret);
+
+    return ret;
 }

block/qcow2-refcount.c

@@ -29,7 +29,7 @@
 static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
                             int64_t offset, int64_t length,
-                            int addend);
+                            int addend, enum qcow2_discard_type type);
 
 
 /*********************************************************/
@@ -235,7 +235,8 @@ static int alloc_refcount_block(BlockDriverState *bs,
     } else {
         /* Described somewhere else. This can recurse at most twice before we
          * arrive at a block that describes itself. */
-        ret = update_refcount(bs, new_block, s->cluster_size, 1);
+        ret = update_refcount(bs, new_block, s->cluster_size, 1,
+                              QCOW2_DISCARD_NEVER);
         if (ret < 0) {
             goto fail_block;
         }
@@ -399,7 +400,8 @@ static int alloc_refcount_block(BlockDriverState *bs,
 
     /* Free old table. Remember, we must not change free_cluster_index */
     uint64_t old_free_cluster_index = s->free_cluster_index;
-    qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t));
+    qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t),
+                        QCOW2_DISCARD_OTHER);
     s->free_cluster_index = old_free_cluster_index;
 
     ret = load_refcount_block(bs, new_block, (void**) refcount_block);
@@ -418,9 +420,77 @@ fail_block:
     return ret;
 }
 
+void qcow2_process_discards(BlockDriverState *bs, int ret)
+{
+    BDRVQcowState *s = bs->opaque;
+    Qcow2DiscardRegion *d, *next;
+
+    QTAILQ_FOREACH_SAFE(d, &s->discards, next, next) {
+        QTAILQ_REMOVE(&s->discards, d, next);
+
+        /* Discard is optional, ignore the return value */
+        if (ret >= 0) {
+            bdrv_discard(bs->file,
+                         d->offset >> BDRV_SECTOR_BITS,
+                         d->bytes >> BDRV_SECTOR_BITS);
+        }
+
+        g_free(d);
+    }
+}
+
+static void update_refcount_discard(BlockDriverState *bs,
+                                    uint64_t offset, uint64_t length)
+{
+    BDRVQcowState *s = bs->opaque;
+    Qcow2DiscardRegion *d, *p, *next;
+
+    QTAILQ_FOREACH(d, &s->discards, next) {
+        uint64_t new_start = MIN(offset, d->offset);
+        uint64_t new_end = MAX(offset + length, d->offset + d->bytes);
+
+        if (new_end - new_start <= length + d->bytes) {
+            /* There can't be any overlap, areas ending up here have no
+             * references any more and therefore shouldn't get freed another
+             * time. */
+            assert(d->bytes + length == new_end - new_start);
+            d->offset = new_start;
+            d->bytes = new_end - new_start;
+            goto found;
+        }
+    }
+
+    d = g_malloc(sizeof(*d));
+    *d = (Qcow2DiscardRegion) {
+        .bs     = bs,
+        .offset = offset,
+        .bytes  = length,
+    };
+    QTAILQ_INSERT_TAIL(&s->discards, d, next);
+
+found:
+    /* Merge discard requests if they are adjacent now */
+    QTAILQ_FOREACH_SAFE(p, &s->discards, next, next) {
+        if (p == d
+            || p->offset > d->offset + d->bytes
+            || d->offset > p->offset + p->bytes)
+        {
+            continue;
+        }
+
+        /* Still no overlap possible */
+        assert(p->offset == d->offset + d->bytes
+            || d->offset == p->offset + p->bytes);
+
+        QTAILQ_REMOVE(&s->discards, p, next);
+        d->offset = MIN(d->offset, p->offset);
+        d->bytes += p->bytes;
+    }
+}
+
 /* XXX: cache several refcount block clusters ? */
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
-    int64_t offset, int64_t length, int addend)
+    int64_t offset, int64_t length, int addend, enum qcow2_discard_type type)
 {
     BDRVQcowState *s = bs->opaque;
     int64_t start, last, cluster_offset;
@@ -486,10 +556,18 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
             s->free_cluster_index = cluster_index;
         }
         refcount_block[block_index] = cpu_to_be16(refcount);
+
+        if (refcount == 0 && s->discard_passthrough[type]) {
+            update_refcount_discard(bs, cluster_offset, s->cluster_size);
+        }
     }
 
     ret = 0;
 fail:
+    if (!s->cache_discards) {
+        qcow2_process_discards(bs, ret);
+    }
+
     /* Write last changed block to disk */
     if (refcount_block) {
         int wret;
@@ -506,7 +584,8 @@ fail:
      */
     if (ret < 0) {
         int dummy;
-        dummy = update_refcount(bs, offset, cluster_offset - offset, -addend);
+        dummy = update_refcount(bs, offset, cluster_offset - offset, -addend,
+                                QCOW2_DISCARD_NEVER);
         (void)dummy;
     }
 
@@ -522,12 +601,14 @@ fail:
  */
 static int update_cluster_refcount(BlockDriverState *bs,
                                    int64_t cluster_index,
-                                   int addend)
+                                   int addend,
+                                   enum qcow2_discard_type type)
 {
     BDRVQcowState *s = bs->opaque;
     int ret;
 
-    ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend);
+    ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend,
+                          type);
     if (ret < 0) {
         return ret;
     }
@@ -579,7 +660,7 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size)
         return offset;
     }
 
-    ret = update_refcount(bs, offset, size, 1);
+    ret = update_refcount(bs, offset, size, 1, QCOW2_DISCARD_NEVER);
     if (ret < 0) {
         return ret;
     }
@@ -611,7 +692,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
     old_free_cluster_index = s->free_cluster_index;
     s->free_cluster_index = cluster_index + i;
 
-    ret = update_refcount(bs, offset, i << s->cluster_bits, 1);
+    ret = update_refcount(bs, offset, i << s->cluster_bits, 1,
+                          QCOW2_DISCARD_NEVER);
     if (ret < 0) {
         return ret;
     }
@@ -649,7 +731,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
         if (free_in_cluster == 0)
             s->free_byte_offset = 0;
         if ((offset & (s->cluster_size - 1)) != 0)
-            update_cluster_refcount(bs, offset >> s->cluster_bits, 1);
+            update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+                                    QCOW2_DISCARD_NEVER);
     } else {
         offset = qcow2_alloc_clusters(bs, s->cluster_size);
         if (offset < 0) {
@@ -659,7 +742,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
         if ((cluster_offset + s->cluster_size) == offset) {
             /* we are lucky: contiguous data */
             offset = s->free_byte_offset;
-            update_cluster_refcount(bs, offset >> s->cluster_bits, 1);
+            update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+                                    QCOW2_DISCARD_NEVER);
             s->free_byte_offset += size;
         } else {
             s->free_byte_offset = offset;
@@ -676,12 +760,13 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 }
 
 void qcow2_free_clusters(BlockDriverState *bs,
-                          int64_t offset, int64_t size)
+                          int64_t offset, int64_t size,
+                          enum qcow2_discard_type type)
 {
     int ret;
 
     BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_FREE);
-    ret = update_refcount(bs, offset, size, -1);
+    ret = update_refcount(bs, offset, size, -1, type);
     if (ret < 0) {
         fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret));
         /* TODO Remember the clusters to free them later and avoid leaking */
@@ -692,8 +777,8 @@ void qcow2_free_clusters(BlockDriverState *bs,
  * Free a cluster using its L2 entry (handles clusters of all types, e.g.
  * normal cluster, compressed cluster, etc.)
  */
-void qcow2_free_any_clusters(BlockDriverState *bs,
+void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
-    uint64_t l2_entry, int nb_clusters)
+                             int nb_clusters, enum qcow2_discard_type type)
 {
     BDRVQcowState *s = bs->opaque;
 
@@ -705,12 +790,12 @@ void qcow2_free_any_clusters(BlockDriverState *bs,
                            s->csize_mask) + 1;
             qcow2_free_clusters(bs,
                 (l2_entry & s->cluster_offset_mask) & ~511,
-                nb_csectors * 512);
+                nb_csectors * 512, type);
         }
         break;
     case QCOW2_CLUSTER_NORMAL:
         qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK,
-                            nb_clusters << s->cluster_bits);
+                            nb_clusters << s->cluster_bits, type);
         break;
     case QCOW2_CLUSTER_UNALLOCATED:
     case QCOW2_CLUSTER_ZERO:
@@ -741,6 +826,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
     l1_table = NULL;
     l1_size2 = l1_size * sizeof(uint64_t);
 
+    s->cache_discards = true;
+
     /* WARNING: qcow2_snapshot_goto relies on this function not using the
      * l1_table_offset when it is the current s->l1_table_offset! Be careful
      * when changing this! */
@@ -785,7 +872,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                             int ret;
                             ret = update_refcount(bs,
                                 (offset & s->cluster_offset_mask) & ~511,
-                                nb_csectors * 512, addend);
+                                nb_csectors * 512, addend,
+                                QCOW2_DISCARD_SNAPSHOT);
                             if (ret < 0) {
                                 goto fail;
                             }
@@ -795,7 +883,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                     } else {
                         uint64_t cluster_index = (offset & L2E_OFFSET_MASK) >> s->cluster_bits;
                         if (addend != 0) {
-                            refcount = update_cluster_refcount(bs, cluster_index, addend);
+                            refcount = update_cluster_refcount(bs, cluster_index, addend,
+                                                               QCOW2_DISCARD_SNAPSHOT);
                         } else {
                             refcount = get_refcount(bs, cluster_index);
                         }
@@ -827,7 +916,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 
 
             if (addend != 0) {
-                refcount = update_cluster_refcount(bs, l2_offset >> s->cluster_bits, addend);
+                refcount = update_cluster_refcount(bs, l2_offset >> s->cluster_bits, addend,
+                                                   QCOW2_DISCARD_SNAPSHOT);
             } else {
                 refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
             }
@@ -850,6 +940,9 @@ fail:
         qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
     }
 
+    s->cache_discards = false;
+    qcow2_process_discards(bs, ret);
+
     /* Update L1 only if it isn't deleted anyway (addend = -1) */
     if (ret == 0 && addend >= 0 && l1_modified) {
         for (i = 0; i < l1_size; i++) {
@@ -1253,7 +1346,8 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
 
             if (num_fixed) {
                 ret = update_refcount(bs, i << s->cluster_bits, 1,
-                                      refcount2 - refcount1);
+                                      refcount2 - refcount1,
+                                      QCOW2_DISCARD_ALWAYS);
                 if (ret >= 0) {
                     (*num_fixed)++;
                     continue;

block/qcow2-snapshot.c

@@ -262,7 +262,8 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
     }
 
     /* free the old snapshot table */
-    qcow2_free_clusters(bs, s->snapshots_offset, s->snapshots_size);
+    qcow2_free_clusters(bs, s->snapshots_offset, s->snapshots_size,
+                        QCOW2_DISCARD_SNAPSHOT);
     s->snapshots_offset = snapshots_offset;
     s->snapshots_size = snapshots_size;
     return 0;
@@ -569,7 +570,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     if (ret < 0) {
         return ret;
     }
-    qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t));
+    qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t),
+                        QCOW2_DISCARD_SNAPSHOT);
 
     /* must update the copied flag on the current cluster offsets */
     ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0);

block/qcow2.c

@@ -295,6 +295,22 @@ static QemuOptsList qcow2_runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Postpone refcount updates",
         },
+        {
+            .name = QCOW2_OPT_DISCARD_REQUEST,
+            .type = QEMU_OPT_BOOL,
+            .help = "Pass guest discard requests to the layer below",
+        },
+        {
+            .name = QCOW2_OPT_DISCARD_SNAPSHOT,
+            .type = QEMU_OPT_BOOL,
+            .help = "Generate discard requests when snapshot related space "
+                    "is freed",
+        },
+        {
+            .name = QCOW2_OPT_DISCARD_OTHER,
+            .type = QEMU_OPT_BOOL,
+            .help = "Generate discard requests when other clusters are freed",
+        },
         { /* end of list */ }
     },
 };
@@ -470,6 +486,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
     }
 
     QLIST_INIT(&s->cluster_allocs);
+    QTAILQ_INIT(&s->discards);
 
     /* read qcow2 extensions */
     if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL)) {
@@ -532,6 +549,16 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
     s->use_lazy_refcounts = qemu_opt_get_bool(opts, QCOW2_OPT_LAZY_REFCOUNTS,
         (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS));
 
+    s->discard_passthrough[QCOW2_DISCARD_NEVER] = false;
+    s->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true;
+    s->discard_passthrough[QCOW2_DISCARD_REQUEST] =
+        qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_REQUEST,
+                          flags & BDRV_O_UNMAP);
+    s->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] =
+        qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_SNAPSHOT, true);
+    s->discard_passthrough[QCOW2_DISCARD_OTHER] =
+        qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);
+
     qemu_opts_del(opts);
 
     if (s->use_lazy_refcounts && s->qcow_version < 3) {
@@ -1196,7 +1223,8 @@ static int preallocate(BlockDriverState *bs)
 
         ret = qcow2_alloc_cluster_link_l2(bs, meta);
         if (ret < 0) {
-            qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_clusters);
+            qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_clusters,
+                                    QCOW2_DISCARD_NEVER);
             return ret;
         }
 

block/qcow2.h

@@ -60,6 +60,9 @@
 
 
 #define QCOW2_OPT_LAZY_REFCOUNTS "lazy_refcounts"
+#define QCOW2_OPT_DISCARD_REQUEST "pass_discard_request"
+#define QCOW2_OPT_DISCARD_SNAPSHOT "pass_discard_snapshot"
+#define QCOW2_OPT_DISCARD_OTHER "pass_discard_other"
 
 typedef struct QCowHeader {
     uint32_t magic;
@@ -129,12 +132,28 @@ enum {
     QCOW2_COMPAT_FEAT_MASK            = QCOW2_COMPAT_LAZY_REFCOUNTS,
 };
 
+enum qcow2_discard_type {
+    QCOW2_DISCARD_NEVER = 0,
+    QCOW2_DISCARD_ALWAYS,
+    QCOW2_DISCARD_REQUEST,
+    QCOW2_DISCARD_SNAPSHOT,
+    QCOW2_DISCARD_OTHER,
+    QCOW2_DISCARD_MAX
+};
+
 typedef struct Qcow2Feature {
     uint8_t type;
     uint8_t bit;
     char    name[46];
 } QEMU_PACKED Qcow2Feature;
 
+typedef struct Qcow2DiscardRegion {
+    BlockDriverState *bs;
+    uint64_t offset;
+    uint64_t bytes;
+    QTAILQ_ENTRY(Qcow2DiscardRegion) next;
+} Qcow2DiscardRegion;
+
 typedef struct BDRVQcowState {
     int cluster_bits;
     int cluster_size;
@@ -178,6 +197,8 @@ typedef struct BDRVQcowState {
     int qcow_version;
     bool use_lazy_refcounts;
 
+    bool discard_passthrough[QCOW2_DISCARD_MAX];
+
     uint64_t incompatible_features;
     uint64_t compatible_features;
     uint64_t autoclear_features;
@@ -185,6 +206,8 @@ typedef struct BDRVQcowState {
     size_t unknown_header_fields_size;
     void* unknown_header_fields;
     QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext;
+    QTAILQ_HEAD (, Qcow2DiscardRegion) discards;
+    bool cache_discards;
 } BDRVQcowState;
 
 /* XXX: use std qcow open function ? */
@@ -349,9 +372,10 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
     int nb_clusters);
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
 void qcow2_free_clusters(BlockDriverState *bs,
-    int64_t offset, int64_t size);
+                          int64_t offset, int64_t size,
-void qcow2_free_any_clusters(BlockDriverState *bs,
+                          enum qcow2_discard_type type);
-    uint64_t cluster_offset, int nb_clusters);
+void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
+                             int nb_clusters, enum qcow2_discard_type type);
 
 int qcow2_update_snapshot_refcount(BlockDriverState *bs,
     int64_t l1_table_offset, int l1_size, int addend);
@@ -359,6 +383,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                           BdrvCheckMode fix);
 
+void qcow2_process_discards(BlockDriverState *bs, int ret);
+
 /* qcow2-cluster.c functions */
 int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
                         bool exact_size);

block/vmdk.c

@@ -561,6 +561,15 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
         header = footer.header;
     }
 
+    if (le32_to_cpu(header.version) >= 3) {
+        char buf[64];
+        snprintf(buf, sizeof(buf), "VMDK version %d",
+                 le32_to_cpu(header.version));
+        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+                bs->device_name, "vmdk", buf);
+        return -ENOTSUP;
+    }
+
     l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
                         * le64_to_cpu(header.granularity);
     if (l1_entry_sectors == 0) {

blockdev.c

@@ -1737,120 +1737,10 @@ QemuOptsList qemu_drive_opts = {
     .name = "drive",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_drive_opts.head),
     .desc = {
-        {
+        /*
-            .name = "bus",
+         * no elements => accept any params
-            .type = QEMU_OPT_NUMBER,
+         * validation will happen later
-            .help = "bus number",
+         */
-        },{
-            .name = "unit",
-            .type = QEMU_OPT_NUMBER,
-            .help = "unit number (i.e. lun for scsi)",
-        },{
-            .name = "if",
-            .type = QEMU_OPT_STRING,
-            .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
-        },{
-            .name = "index",
-            .type = QEMU_OPT_NUMBER,
-            .help = "index number",
-        },{
-            .name = "cyls",
-            .type = QEMU_OPT_NUMBER,
-            .help = "number of cylinders (ide disk geometry)",
-        },{
-            .name = "heads",
-            .type = QEMU_OPT_NUMBER,
-            .help = "number of heads (ide disk geometry)",
-        },{
-            .name = "secs",
-            .type = QEMU_OPT_NUMBER,
-            .help = "number of sectors (ide disk geometry)",
-        },{
-            .name = "trans",
-            .type = QEMU_OPT_STRING,
-            .help = "chs translation (auto, lba. none)",
-        },{
-            .name = "media",
-            .type = QEMU_OPT_STRING,
-            .help = "media type (disk, cdrom)",
-        },{
-            .name = "snapshot",
-            .type = QEMU_OPT_BOOL,
-            .help = "enable/disable snapshot mode",
-        },{
-            .name = "file",
-            .type = QEMU_OPT_STRING,
-            .help = "disk image",
-        },{
-            .name = "discard",
-            .type = QEMU_OPT_STRING,
-            .help = "discard operation (ignore/off, unmap/on)",
-        },{
-            .name = "cache",
-            .type = QEMU_OPT_STRING,
-            .help = "host cache usage (none, writeback, writethrough, "
-                    "directsync, unsafe)",
-        },{
-            .name = "aio",
-            .type = QEMU_OPT_STRING,
-            .help = "host AIO implementation (threads, native)",
-        },{
-            .name = "format",
-            .type = QEMU_OPT_STRING,
-            .help = "disk format (raw, qcow2, ...)",
-        },{
-            .name = "serial",
-            .type = QEMU_OPT_STRING,
-            .help = "disk serial number",
-        },{
-            .name = "rerror",
-            .type = QEMU_OPT_STRING,
-            .help = "read error action",
-        },{
-            .name = "werror",
-            .type = QEMU_OPT_STRING,
-            .help = "write error action",
-        },{
-            .name = "addr",
-            .type = QEMU_OPT_STRING,
-            .help = "pci address (virtio only)",
-        },{
-            .name = "readonly",
-            .type = QEMU_OPT_BOOL,
-            .help = "open drive file as read-only",
-        },{
-            .name = "iops",
-            .type = QEMU_OPT_NUMBER,
-            .help = "limit total I/O operations per second",
-        },{
-            .name = "iops_rd",
-            .type = QEMU_OPT_NUMBER,
-            .help = "limit read operations per second",
-        },{
-            .name = "iops_wr",
-            .type = QEMU_OPT_NUMBER,
-            .help = "limit write operations per second",
-        },{
-            .name = "bps",
-            .type = QEMU_OPT_NUMBER,
-            .help = "limit total bytes per second",
-        },{
-            .name = "bps_rd",
-            .type = QEMU_OPT_NUMBER,
-            .help = "limit read bytes per second",
-        },{
-            .name = "bps_wr",
-            .type = QEMU_OPT_NUMBER,
-            .help = "limit write bytes per second",
-        },{
-            .name = "copy-on-read",
-            .type = QEMU_OPT_BOOL,
-            .help = "copy read data from backing file into image file",
-        },{
-            .name = "boot",
-            .type = QEMU_OPT_BOOL,
-            .help = "(deprecated, ignored)",
-        },
         { /* end of list */ }
     },
 };

hw/ide/core.c

@@ -1004,300 +1004,242 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-#define HD_OK (1u << IDE_HD)
+static bool cmd_nop(IDEState *s, uint8_t cmd)
-#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]                         = HD_CFA_OK,
-    [WIN_READ]                          = ALL_OK,
-    [WIN_READ_ONCE]                     = 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]                       = HD_CFA_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]                      = 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,
-    [WIN_SETIDLE1]                      = ALL_OK,
-    [WIN_CHECKPOWERMODE1]               = ALL_OK,
-    [WIN_SLEEPNOW1]                     = ALL_OK,
-    [WIN_FLUSH_CACHE]                   = ALL_OK,
-    [WIN_FLUSH_CACHE_EXT]               = HD_CFA_OK,
-    [WIN_IDENTIFY]                      = ALL_OK,
-    [WIN_SETFEATURES]                   = ALL_OK,
-    [IBM_SENSE_CONDITION]               = CFA_OK,
-    [CFA_WEAR_LEVEL]                    = HD_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)
+    return true;
-        && (ide_cmd_table[cmd] & (1u << s->drive_kind));
 }
 
-void ide_exec_cmd(IDEBus *bus, uint32_t val)
+static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
 {
-    uint16_t *identify_data;
-    IDEState *s;
-    int n;
-    int lba48 = 0;
-
-#if defined(DEBUG_IDE)
-    printf("ide: CMD=%02x\n", val);
-#endif
-    s = idebus_active_if(bus);
-    /* ignore commands to non existent slave */
-    if (s != bus->ifs && !s->bs)
-        return;
-
-    /* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
-    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) {
     case DSM_TRIM:
-            if (!s->bs) {
+        if (s->bs) {
-                goto abort_cmd;
-            }
             ide_sector_start_dma(s, IDE_DMA_TRIM);
-            break;
+            return false;
-        default:
-            goto abort_cmd;
         }
         break;
-    case WIN_IDENTIFY:
+    }
+
+    ide_abort_command(s);
+    return true;
+}
+
+static bool cmd_identify(IDEState *s, uint8_t cmd)
+{
     if (s->bs && s->drive_kind != IDE_CD) {
-            if (s->drive_kind != IDE_CFATA)
+        if (s->drive_kind != IDE_CFATA) {
             ide_identify(s);
-            else
+        } else {
             ide_cfata_identify(s);
+        }
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
+        ide_set_irq(s->bus);
+        return false;
     } else {
         if (s->drive_kind == IDE_CD) {
             ide_set_signature(s);
         }
         ide_abort_command(s);
     }
-        ide_set_irq(s->bus);
+
-        break;
+    return true;
-    case WIN_SPECIFY:
+}
-    case WIN_RECAL:
+
-        s->error = 0;
+static bool cmd_verify(IDEState *s, uint8_t cmd)
-        s->status = READY_STAT | SEEK_STAT;
+{
-        ide_set_irq(s->bus);
+    bool lba48 = (cmd == WIN_VERIFY_EXT);
-        break;
+
-    case WIN_SETMULT:
+    /* do sector number check ? */
+    ide_cmd_lba48_transform(s, lba48);
+
+    return true;
+}
+
+static bool cmd_set_multiple_mode(IDEState *s, uint8_t cmd)
+{
     if (s->drive_kind == IDE_CFATA && s->nsector == 0) {
         /* Disable Read and Write Multiple */
         s->mult_sectors = 0;
-            s->status = READY_STAT | SEEK_STAT;
     } else if ((s->nsector & 0xff) != 0 &&
         ((s->nsector & 0xff) > MAX_MULT_SECTORS ||
          (s->nsector & (s->nsector - 1)) != 0)) {
         ide_abort_command(s);
     } else {
         s->mult_sectors = s->nsector & 0xff;
-            s->status = READY_STAT | SEEK_STAT;
     }
-        ide_set_irq(s->bus);
-        break;
 
-    case WIN_VERIFY_EXT:
+    return true;
-        lba48 = 1;
+}
-        /* fall through */
+
-    case WIN_VERIFY:
+static bool cmd_read_multiple(IDEState *s, uint8_t cmd)
-    case WIN_VERIFY_ONCE:
+{
-        /* do sector number check ? */
+    bool lba48 = (cmd == WIN_MULTREAD_EXT);
+
+    if (!s->bs || !s->mult_sectors) {
+        ide_abort_command(s);
+        return true;
+    }
+
     ide_cmd_lba48_transform(s, lba48);
-        s->status = READY_STAT | SEEK_STAT;
+    s->req_nb_sectors = s->mult_sectors;
-        ide_set_irq(s->bus);
+    ide_sector_read(s);
-        break;
+    return false;
+}
+
+static bool cmd_write_multiple(IDEState *s, uint8_t cmd)
+{
+    bool lba48 = (cmd == WIN_MULTWRITE_EXT);
+    int n;
+
+    if (!s->bs || !s->mult_sectors) {
+        ide_abort_command(s);
+        return true;
+    }
+
+    ide_cmd_lba48_transform(s, lba48);
+
+    s->req_nb_sectors = s->mult_sectors;
+    n = MIN(s->nsector, s->req_nb_sectors);
+
+    s->status = SEEK_STAT | READY_STAT;
+    ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
+
+    s->media_changed = 1;
+
+    return false;
+}
+
+static bool cmd_read_pio(IDEState *s, uint8_t cmd)
+{
+    bool lba48 = (cmd == WIN_READ_EXT);
 
-    case WIN_READ_EXT:
-        lba48 = 1;
-        /* fall through */
-    case WIN_READ:
-    case WIN_READ_ONCE:
     if (s->drive_kind == IDE_CD) {
         ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */
-            goto abort_cmd;
+        ide_abort_command(s);
+        return true;
     }
+
     if (!s->bs) {
-            goto abort_cmd;
+        ide_abort_command(s);
+        return true;
     }
+
     ide_cmd_lba48_transform(s, lba48);
     s->req_nb_sectors = 1;
     ide_sector_read(s);
-        break;
 
-    case WIN_WRITE_EXT:
+    return false;
-        lba48 = 1;
-        /* fall through */
-    case WIN_WRITE:
-    case WIN_WRITE_ONCE:
-    case CFA_WRITE_SECT_WO_ERASE:
-    case WIN_WRITE_VERIFY:
-        if (!s->bs) {
-            goto abort_cmd;
 }
+
+static bool cmd_write_pio(IDEState *s, uint8_t cmd)
+{
+    bool lba48 = (cmd == WIN_WRITE_EXT);
+
+    if (!s->bs) {
+        ide_abort_command(s);
+        return true;
+    }
+
     ide_cmd_lba48_transform(s, lba48);
-        s->error = 0;
+
-        s->status = SEEK_STAT | READY_STAT;
     s->req_nb_sectors = 1;
+    s->status = SEEK_STAT | READY_STAT;
     ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
+
     s->media_changed = 1;
-        break;
 
-    case WIN_MULTREAD_EXT:
+    return false;
-        lba48 = 1;
-        /* fall through */
-    case WIN_MULTREAD:
-        if (!s->bs) {
-            goto abort_cmd;
 }
-        if (!s->mult_sectors) {
-            goto abort_cmd;
-        }
-	ide_cmd_lba48_transform(s, lba48);
-        s->req_nb_sectors = s->mult_sectors;
-        ide_sector_read(s);
-        break;
 
-    case WIN_MULTWRITE_EXT:
+static bool cmd_read_dma(IDEState *s, uint8_t cmd)
-        lba48 = 1;
+{
-        /* fall through */
+    bool lba48 = (cmd == WIN_READDMA_EXT);
-    case WIN_MULTWRITE:
-    case CFA_WRITE_MULTI_WO_ERASE:
-        if (!s->bs) {
-            goto abort_cmd;
-        }
-        if (!s->mult_sectors) {
-            goto abort_cmd;
-        }
-	ide_cmd_lba48_transform(s, lba48);
-        s->error = 0;
-        s->status = SEEK_STAT | READY_STAT;
-        s->req_nb_sectors = s->mult_sectors;
-        n = s->nsector;
-        if (n > s->req_nb_sectors)
-            n = s->req_nb_sectors;
-        ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
-        s->media_changed = 1;
-        break;
 
-    case WIN_READDMA_EXT:
-        lba48 = 1;
-        /* fall through */
-    case WIN_READDMA:
-    case WIN_READDMA_ONCE:
     if (!s->bs) {
-            goto abort_cmd;
+        ide_abort_command(s);
+        return true;
     }
+
     ide_cmd_lba48_transform(s, lba48);
     ide_sector_start_dma(s, IDE_DMA_READ);
-        break;
 
-    case WIN_WRITEDMA_EXT:
+    return false;
-        lba48 = 1;
-        /* fall through */
-    case WIN_WRITEDMA:
-    case WIN_WRITEDMA_ONCE:
-        if (!s->bs) {
-            goto abort_cmd;
 }
+
+static bool cmd_write_dma(IDEState *s, uint8_t cmd)
+{
+    bool lba48 = (cmd == WIN_WRITEDMA_EXT);
+
+    if (!s->bs) {
+        ide_abort_command(s);
+        return true;
+    }
+
     ide_cmd_lba48_transform(s, lba48);
     ide_sector_start_dma(s, IDE_DMA_WRITE);
-        s->media_changed = 1;
-        break;
 
-    case WIN_READ_NATIVE_MAX_EXT:
+    s->media_changed = 1;
-        lba48 = 1;
+
-        /* fall through */
+    return false;
-    case WIN_READ_NATIVE_MAX:
+}
+
+static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
+{
+    ide_flush_cache(s);
+    return false;
+}
+
+static bool cmd_seek(IDEState *s, uint8_t cmd)
+{
+    /* XXX: Check that seek is within bounds */
+    return true;
+}
+
+static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
+{
+    bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
+
     /* Refuse if no sectors are addressable (e.g. medium not inserted) */
     if (s->nb_sectors == 0) {
-            goto abort_cmd;
+        ide_abort_command(s);
+        return true;
     }
+
     ide_cmd_lba48_transform(s, lba48);
     ide_set_sector(s, s->nb_sectors - 1);
-        s->status = READY_STAT | SEEK_STAT;
-        ide_set_irq(s->bus);
-        break;
 
-    case WIN_CHECKPOWERMODE1:
+    return true;
-    case WIN_CHECKPOWERMODE2:
+}
-        s->error = 0;
+
+static bool cmd_check_power_mode(IDEState *s, uint8_t cmd)
+{
     s->nsector = 0xff; /* device active or idle */
-        s->status = READY_STAT | SEEK_STAT;
+    return true;
-        ide_set_irq(s->bus);
+}
-        break;
+
-    case WIN_SETFEATURES:
+static bool cmd_set_features(IDEState *s, uint8_t cmd)
-        if (!s->bs)
+{
-            goto abort_cmd;
+    uint16_t *identify_data;
+
+    if (!s->bs) {
+        ide_abort_command(s);
+        return true;
+    }
+
     /* XXX: valid for CDROM ? */
     switch (s->feature) {
     case 0x02: /* write cache enable */
         bdrv_set_enable_write_cache(s->bs, true);
         identify_data = (uint16_t *)s->identify_data;
         put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
-            s->status = READY_STAT | SEEK_STAT;
+        return true;
-            ide_set_irq(s->bus);
-            break;
     case 0x82: /* write cache disable */
         bdrv_set_enable_write_cache(s->bs, false);
         identify_data = (uint16_t *)s->identify_data;
         put_le16(identify_data + 85, (1 << 14) | 1);
         ide_flush_cache(s);
-            break;
+        return false;
     case 0xcc: /* reverting to power-on defaults enable */
     case 0x66: /* reverting to power-on defaults disable */
     case 0xaa: /* read look-ahead enable */
@@ -1310,10 +1252,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
     case 0x9a: /* NOP */
     case 0x42: /* enable Automatic Acoustic Mode */
     case 0xc2: /* disable Automatic Acoustic Mode */
-            s->status = READY_STAT | SEEK_STAT;
+        return true;
-            ide_set_irq(s->bus);
+    case 0x03: /* set transfer mode */
-            break;
+        {
-        case 0x03: { /* set transfer mode */
             uint8_t val = s->nsector & 0x07;
             identify_data = (uint16_t *)s->identify_data;
 
@@ -1342,95 +1283,104 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
             default:
                 goto abort_cmd;
             }
-            s->status = READY_STAT | SEEK_STAT;
+            return true;
-            ide_set_irq(s->bus);
-            break;
         }
-        default:
-            goto abort_cmd;
     }
-        break;
+
-    case WIN_FLUSH_CACHE:
+abort_cmd:
-    case WIN_FLUSH_CACHE_EXT:
+    ide_abort_command(s);
-        ide_flush_cache(s);
+    return true;
-        break;
+}
-    case WIN_STANDBY:
+
-    case WIN_STANDBY2:
+
-    case WIN_STANDBYNOW1:
+/*** ATAPI commands ***/
-    case WIN_STANDBYNOW2:
+
-    case WIN_IDLEIMMEDIATE:
+static bool cmd_identify_packet(IDEState *s, uint8_t cmd)
-    case WIN_IDLEIMMEDIATE2:
+{
-    case WIN_SETIDLE1:
-    case WIN_SETIDLE2:
-    case WIN_SLEEPNOW1:
-    case WIN_SLEEPNOW2:
-        s->status = READY_STAT;
-        ide_set_irq(s->bus);
-        break;
-    case WIN_SEEK:
-        /* XXX: Check that seek is within bounds */
-        s->status = READY_STAT | SEEK_STAT;
-        ide_set_irq(s->bus);
-        break;
-        /* ATAPI commands */
-    case WIN_PIDENTIFY:
     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;
+    return false;
-    case WIN_DIAGNOSE:
+}
+
+static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
+{
     ide_set_signature(s);
-        if (s->drive_kind == IDE_CD)
+
+    if (s->drive_kind == IDE_CD) {
         s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
                         * devices to return a clear status register
                         * with READY_STAT *not* set. */
-        else
+    } else {
         s->status = READY_STAT | SEEK_STAT;
-        s->error = 0x01; /* Device 0 passed, Device 1 passed or not
+        /* The bits of the error register are not as usual for this command!
-                          * present.
+         * They are part of the regular output (this is why ERR_STAT isn't set)
-                          */
+         * Device 0 passed, Device 1 passed or not present. */
+        s->error = 0x01;
         ide_set_irq(s->bus);
-        break;
+    }
-    case WIN_DEVICE_RESET:
+
+    return false;
+}
+
+static bool cmd_device_reset(IDEState *s, uint8_t cmd)
+{
     ide_set_signature(s);
     s->status = 0x00; /* NOTE: READY is _not_ set */
     s->error = 0x01;
-        break;
+
-    case WIN_PACKETCMD:
+    return false;
+}
+
+static bool cmd_packet(IDEState *s, uint8_t cmd)
+{
     /* overlapping commands not supported */
-        if (s->feature & 0x02)
+    if (s->feature & 0x02) {
-            goto abort_cmd;
+        ide_abort_command(s);
+        return true;
+    }
+
     s->status = READY_STAT | SEEK_STAT;
     s->atapi_dma = s->feature & 1;
     s->nsector = 1;
     ide_transfer_start(s, s->io_buffer, ATAPI_PACKET_SIZE,
                        ide_atapi_cmd);
-        break;
+    return false;
-    /* CF-ATA commands */
+}
-    case CFA_REQ_EXT_ERROR_CODE:
+
+
+/*** CF-ATA commands ***/
+
+static bool cmd_cfa_req_ext_error_code(IDEState *s, uint8_t cmd)
+{
     s->error = 0x09;    /* miscellaneous error */
     s->status = READY_STAT | SEEK_STAT;
     ide_set_irq(s->bus);
-        break;
+
-    case CFA_ERASE_SECTORS:
+    return false;
-    case CFA_WEAR_LEVEL:
+}
-#if 0
+
-    /* This one has the same ID as CFA_WEAR_LEVEL and is required for
+static bool cmd_cfa_erase_sectors(IDEState *s, uint8_t cmd)
-       Windows 8 to work with AHCI */
+{
-    case WIN_SECURITY_FREEZE_LOCK:
+    /* WIN_SECURITY_FREEZE_LOCK has the same ID as CFA_WEAR_LEVEL and is
-#endif
+     * required for Windows 8 to work with AHCI */
-        if (val == CFA_WEAR_LEVEL)
+
+    if (cmd == CFA_WEAR_LEVEL) {
         s->nsector = 0;
-        if (val == CFA_ERASE_SECTORS)
+    }
+
+    if (cmd == CFA_ERASE_SECTORS) {
         s->media_changed = 1;
-        s->error = 0x00;
+    }
-        s->status = READY_STAT | SEEK_STAT;
+
-        ide_set_irq(s->bus);
+    return true;
-        break;
+}
-    case CFA_TRANSLATE_SECTOR:
+
-        s->error = 0x00;
+static bool cmd_cfa_translate_sector(IDEState *s, uint8_t cmd)
+{
     s->status = READY_STAT | SEEK_STAT;
+
     memset(s->io_buffer, 0, 0x200);
     s->io_buffer[0x00] = s->hcyl;                   /* Cyl MSB */
     s->io_buffer[0x01] = s->lcyl;                   /* Cyl LSB */
@@ -1443,10 +1393,15 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
     s->io_buffer[0x18] = 0x00;                      /* Hot count */
     s->io_buffer[0x19] = 0x00;                      /* Hot count */
     s->io_buffer[0x1a] = 0x01;                      /* Hot count */
+
     ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
     ide_set_irq(s->bus);
-        break;
+
-    case CFA_ACCESS_METADATA_STORAGE:
+    return false;
+}
+
+static bool cmd_cfa_access_metadata_storage(IDEState *s, uint8_t cmd)
+{
     switch (s->feature) {
     case 0x02:  /* Inquiry Metadata Storage */
         ide_cfata_metadata_inquiry(s);
@@ -1458,40 +1413,55 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_cfata_metadata_write(s);
         break;
     default:
-            goto abort_cmd;
+        ide_abort_command(s);
+        return true;
     }
+
     ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
     s->status = 0x00; /* NOTE: READY is _not_ set */
     ide_set_irq(s->bus);
-        break;
+
-    case IBM_SENSE_CONDITION:
+    return false;
+}
+
+static bool cmd_ibm_sense_condition(IDEState *s, uint8_t cmd)
+{
     switch (s->feature) {
     case 0x01:  /* sense temperature in device */
         s->nsector = 0x50;      /* +20 C */
         break;
     default:
+        ide_abort_command(s);
+        return true;
+    }
+
+    return true;
+}
+
+
+/*** SMART commands ***/
+
+static bool cmd_smart(IDEState *s, uint8_t cmd)
+{
+    int n;
+
+    if (s->hcyl != 0xc2 || s->lcyl != 0x4f) {
         goto abort_cmd;
     }
-        s->status = READY_STAT | SEEK_STAT;
-        ide_set_irq(s->bus);
-        break;
 
-    case WIN_SMART:
+    if (!s->smart_enabled && s->feature != SMART_ENABLE) {
-	if (s->hcyl != 0xc2 || s->lcyl != 0x4f)
-		goto abort_cmd;
-	if (!s->smart_enabled && s->feature != SMART_ENABLE)
         goto abort_cmd;
+    }
+
     switch (s->feature) {
     case SMART_DISABLE:
         s->smart_enabled = 0;
-		s->status = READY_STAT | SEEK_STAT;
+        return true;
-		ide_set_irq(s->bus);
+
-		break;
     case SMART_ENABLE:
         s->smart_enabled = 1;
-		s->status = READY_STAT | SEEK_STAT;
+        return true;
-		ide_set_irq(s->bus);
+
-		break;
     case SMART_ATTR_AUTOSAVE:
         switch (s->sector) {
         case 0x00:
@@ -1503,9 +1473,8 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         default:
             goto abort_cmd;
         }
-		s->status = READY_STAT | SEEK_STAT;
+        return true;
-		ide_set_irq(s->bus);
+
-		break;
     case SMART_STATUS:
         if (!s->smart_errors) {
             s->hcyl = 0xc2;
@@ -1514,32 +1483,39 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
             s->hcyl = 0x2c;
             s->lcyl = 0xf4;
         }
-		s->status = READY_STAT | SEEK_STAT;
+        return true;
-		ide_set_irq(s->bus);
+
-		break;
     case SMART_READ_THRESH:
         memset(s->io_buffer, 0, 0x200);
         s->io_buffer[0] = 0x01; /* smart struct version */
+
         for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
             s->io_buffer[2 + 0 + (n * 12)] = smart_attributes[n][0];
             s->io_buffer[2 + 1 + (n * 12)] = smart_attributes[n][11];
         }
-		for (n=0; n<511; n++) /* checksum */
+
+        /* checksum */
+        for (n = 0; n < 511; n++) {
             s->io_buffer[511] += s->io_buffer[n];
+        }
         s->io_buffer[511] = 0x100 - s->io_buffer[511];
+
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
         ide_set_irq(s->bus);
-		break;
+        return false;
+
     case SMART_READ_DATA:
         memset(s->io_buffer, 0, 0x200);
         s->io_buffer[0] = 0x01; /* smart struct version */
+
         for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
             int i;
             for (i = 0; i < 11; i++) {
                 s->io_buffer[2 + i + (n * 12)] = smart_attributes[n][i];
             }
         }
+
         s->io_buffer[362] = 0x02 | (s->smart_autosave ? 0x80 : 0x00);
         if (s->smart_selftest_count == 0) {
             s->io_buffer[363] = 0;
@@ -1560,13 +1536,16 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         s->io_buffer[373] = 0x36; /* minutes for poll ext test */
         s->io_buffer[374] = 0x01; /* minutes for poll conveyance */
 
-		for (n=0; n<511; n++) 
+        for (n = 0; n < 511; n++) {
             s->io_buffer[511] += s->io_buffer[n];
+        }
         s->io_buffer[511] = 0x100 - s->io_buffer[511];
+
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
         ide_set_irq(s->bus);
-		break;
+        return false;
+
     case SMART_READ_LOG:
         switch (s->sector) {
         case 0x01: /* summary smart error log */
@@ -1576,8 +1555,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
             s->io_buffer[452] = s->smart_errors & 0xff;
             s->io_buffer[453] = (s->smart_errors & 0xff00) >> 8;
 
-		for (n=0; n<511; n++)
+            for (n = 0; n < 511; n++) {
                 s->io_buffer[511] += s->io_buffer[n];
+            }
             s->io_buffer[511] = 0x100 - s->io_buffer[511];
             break;
         case 0x06: /* smart self test log */
@@ -1587,11 +1567,14 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
                 s->io_buffer[508] = 0;
             } else {
                 s->io_buffer[508] = s->smart_selftest_count;
-			for (n=2; n<506; n++) 
+                for (n = 2; n < 506; n++)  {
                     s->io_buffer[n] = s->smart_selftest_data[n];
                 }
-		for (n=0; n<511; n++)
+            }
+
+            for (n = 0; n < 511; n++) {
                 s->io_buffer[511] += s->io_buffer[n];
+            }
             s->io_buffer[511] = 0x100 - s->io_buffer[511];
             break;
         default:
@@ -1600,37 +1583,150 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
         ide_set_irq(s->bus);
-		break;
+        return false;
+
     case SMART_EXECUTE_OFFLINE:
         switch (s->sector) {
         case 0: /* off-line routine */
         case 1: /* short self test */
         case 2: /* extended self test */
             s->smart_selftest_count++;
-		if(s->smart_selftest_count > 21)
+            if (s->smart_selftest_count > 21) {
                 s->smart_selftest_count = 0;
+            }
             n = 2 + (s->smart_selftest_count - 1) * 24;
             s->smart_selftest_data[n] = s->sector;
             s->smart_selftest_data[n + 1] = 0x00; /* OK and finished */
             s->smart_selftest_data[n + 2] = 0x34; /* hour count lsb */
             s->smart_selftest_data[n + 3] = 0x12; /* hour count msb */
-		s->status = READY_STAT | SEEK_STAT;
-		ide_set_irq(s->bus);
             break;
         default:
             goto abort_cmd;
         }
-		break;
+        return true;
-	default:
-		goto abort_cmd;
     }
-	break;
+
-    default:
-        /* should not be reachable */
 abort_cmd:
+    ide_abort_command(s);
+    return true;
+}
+
+#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)
+
+/* Set the Disk Seek Completed status bit during completion */
+#define SET_DSC (1u << 8)
+
+/* See ACS-2 T13/2015-D Table B.2 Command codes */
+static const struct {
+    /* Returns true if the completion code should be run */
+    bool (*handler)(IDEState *s, uint8_t cmd);
+    int flags;
+} ide_cmd_table[0x100] = {
+    /* NOP not implemented, mandatory for CD */
+    [CFA_REQ_EXT_ERROR_CODE]      = { cmd_cfa_req_ext_error_code, CFA_OK },
+    [WIN_DSM]                     = { cmd_data_set_management, ALL_OK },
+    [WIN_DEVICE_RESET]            = { cmd_device_reset, CD_OK },
+    [WIN_RECAL]                   = { cmd_nop, HD_CFA_OK | SET_DSC},
+    [WIN_READ]                    = { cmd_read_pio, ALL_OK },
+    [WIN_READ_ONCE]               = { cmd_read_pio, ALL_OK },
+    [WIN_READ_EXT]                = { cmd_read_pio, HD_CFA_OK },
+    [WIN_READDMA_EXT]             = { cmd_read_dma, HD_CFA_OK },
+    [WIN_READ_NATIVE_MAX_EXT]     = { cmd_read_native_max, HD_CFA_OK | SET_DSC },
+    [WIN_MULTREAD_EXT]            = { cmd_read_multiple, HD_CFA_OK },
+    [WIN_WRITE]                   = { cmd_write_pio, HD_CFA_OK },
+    [WIN_WRITE_ONCE]              = { cmd_write_pio, HD_CFA_OK },
+    [WIN_WRITE_EXT]               = { cmd_write_pio, HD_CFA_OK },
+    [WIN_WRITEDMA_EXT]            = { cmd_write_dma, HD_CFA_OK },
+    [CFA_WRITE_SECT_WO_ERASE]     = { cmd_write_pio, CFA_OK },
+    [WIN_MULTWRITE_EXT]           = { cmd_write_multiple, HD_CFA_OK },
+    [WIN_WRITE_VERIFY]            = { cmd_write_pio, HD_CFA_OK },
+    [WIN_VERIFY]                  = { cmd_verify, HD_CFA_OK | SET_DSC },
+    [WIN_VERIFY_ONCE]             = { cmd_verify, HD_CFA_OK | SET_DSC },
+    [WIN_VERIFY_EXT]              = { cmd_verify, HD_CFA_OK | SET_DSC },
+    [WIN_SEEK]                    = { cmd_seek, HD_CFA_OK | SET_DSC },
+    [CFA_TRANSLATE_SECTOR]        = { cmd_cfa_translate_sector, CFA_OK },
+    [WIN_DIAGNOSE]                = { cmd_exec_dev_diagnostic, ALL_OK },
+    [WIN_SPECIFY]                 = { cmd_nop, HD_CFA_OK | SET_DSC },
+    [WIN_STANDBYNOW2]             = { cmd_nop, ALL_OK },
+    [WIN_IDLEIMMEDIATE2]          = { cmd_nop, ALL_OK },
+    [WIN_STANDBY2]                = { cmd_nop, ALL_OK },
+    [WIN_SETIDLE2]                = { cmd_nop, ALL_OK },
+    [WIN_CHECKPOWERMODE2]         = { cmd_check_power_mode, ALL_OK | SET_DSC },
+    [WIN_SLEEPNOW2]               = { cmd_nop, ALL_OK },
+    [WIN_PACKETCMD]               = { cmd_packet, CD_OK },
+    [WIN_PIDENTIFY]               = { cmd_identify_packet, CD_OK },
+    [WIN_SMART]                   = { cmd_smart, HD_CFA_OK | SET_DSC },
+    [CFA_ACCESS_METADATA_STORAGE] = { cmd_cfa_access_metadata_storage, CFA_OK },
+    [CFA_ERASE_SECTORS]           = { cmd_cfa_erase_sectors, CFA_OK | SET_DSC },
+    [WIN_MULTREAD]                = { cmd_read_multiple, HD_CFA_OK },
+    [WIN_MULTWRITE]               = { cmd_write_multiple, HD_CFA_OK },
+    [WIN_SETMULT]                 = { cmd_set_multiple_mode, HD_CFA_OK | SET_DSC },
+    [WIN_READDMA]                 = { cmd_read_dma, HD_CFA_OK },
+    [WIN_READDMA_ONCE]            = { cmd_read_dma, HD_CFA_OK },
+    [WIN_WRITEDMA]                = { cmd_write_dma, HD_CFA_OK },
+    [WIN_WRITEDMA_ONCE]           = { cmd_write_dma, HD_CFA_OK },
+    [CFA_WRITE_MULTI_WO_ERASE]    = { cmd_write_multiple, CFA_OK },
+    [WIN_STANDBYNOW1]             = { cmd_nop, ALL_OK },
+    [WIN_IDLEIMMEDIATE]           = { cmd_nop, ALL_OK },
+    [WIN_STANDBY]                 = { cmd_nop, ALL_OK },
+    [WIN_SETIDLE1]                = { cmd_nop, ALL_OK },
+    [WIN_CHECKPOWERMODE1]         = { cmd_check_power_mode, ALL_OK | SET_DSC },
+    [WIN_SLEEPNOW1]               = { cmd_nop, ALL_OK },
+    [WIN_FLUSH_CACHE]             = { cmd_flush_cache, ALL_OK },
+    [WIN_FLUSH_CACHE_EXT]         = { cmd_flush_cache, HD_CFA_OK },
+    [WIN_IDENTIFY]                = { cmd_identify, ALL_OK },
+    [WIN_SETFEATURES]             = { cmd_set_features, ALL_OK | SET_DSC },
+    [IBM_SENSE_CONDITION]         = { cmd_ibm_sense_condition, CFA_OK | SET_DSC },
+    [CFA_WEAR_LEVEL]              = { cmd_cfa_erase_sectors, HD_CFA_OK | SET_DSC },
+    [WIN_READ_NATIVE_MAX]         = { cmd_read_native_max, ALL_OK | SET_DSC },
+};
+
+static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
+{
+    return cmd < ARRAY_SIZE(ide_cmd_table)
+        && (ide_cmd_table[cmd].flags & (1u << s->drive_kind));
+}
+
+void ide_exec_cmd(IDEBus *bus, uint32_t val)
+{
+    IDEState *s;
+    bool complete;
+
+#if defined(DEBUG_IDE)
+    printf("ide: CMD=%02x\n", val);
+#endif
+    s = idebus_active_if(bus);
+    /* ignore commands to non existent slave */
+    if (s != bus->ifs && !s->bs)
+        return;
+
+    /* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
+    if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
+        return;
+
+    if (!ide_cmd_permitted(s, val)) {
         ide_abort_command(s);
         ide_set_irq(s->bus);
-        break;
+        return;
+    }
+
+    s->status = READY_STAT | BUSY_STAT;
+    s->error = 0;
+
+    complete = ide_cmd_table[val].handler(s, val);
+    if (complete) {
+        s->status &= ~BUSY_STAT;
+        assert(!!s->error == !!(s->status & ERR_STAT));
+
+        if ((ide_cmd_table[val].flags & SET_DSC) && !s->error) {
+            s->status |= SEEK_STAT;
+        }
+
+        ide_set_irq(s->bus);
     }
 }
 

tests/qemu-iotests/group

@@ -57,7 +57,7 @@
 048 img auto quick
 049 rw auto
 050 rw auto backing quick
-#051 rw auto
+051 rw auto
 052 rw auto backing
 053 rw auto
 054 rw auto