[Qemu-devel] [PULL 00/19] Block patches

[Qemu-devel] [PULL 00/19] Block patches

[Kevin Wolf]

The following changes since commit fd42deeb4cb42f90084046e3ebdb4383953195e3:
  Gerd Hoffmann (1):
        Add exit notifiers.

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git for-anthony

Blue Swirl (1):
      block: fix a warning and possible truncation

Christoph Hellwig (3):
      cow: use pread/pwrite
      cow: stop using mmap
      cow: use qemu block API

Jan Kiszka (1):
      xen: Fix build error due to missing include

Jes Sorensen (1):
      Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

Kevin Wolf (5):
      vpc: Read/write multiple sectors at once
      qcow2: Allow get_refcount to return errors
      qcow2: Allow alloc_clusters_noref to return errors
      qcow2: Return real error code in load_refcount_block
      qcow2: Restore L1 entry on l2_allocate failure

Markus Armbruster (7):
      Fix regression for "-drive file="
      block: Move error actions from DriveInfo to BlockDriverState
      block: Decouple block device "commit all" from DriveInfo
      monitor: Make "commit FOO" complain when FOO doesn't exist
      block: New bdrv_next()
      block: Decouple savevm from DriveInfo
      blockdev: Give drives internal linkage

Miguel Di Ciurcio Filho (1):
      savevm: Really verify if a drive supports snapshots

 block.c                |   49 ++++++++++++++++++-
 block.h                |   11 ++++
 block/cow.c            |  127 ++++++++++++++++++++++++++----------------------
 block/qcow2-cluster.c  |    1 +
 block/qcow2-refcount.c |   70 +++++++++++++++++++++++----
 block/vpc.c            |   41 +++++++++++----
 block_int.h            |    1 +
 blockdev.c             |   39 +++++---------
 blockdev.h             |   12 -----
 hw/fdc.c               |    4 +-
 hw/ide/core.c          |    2 +-
 hw/scsi-disk.c         |    2 +-
 hw/virtio-blk.c        |    3 +-
 hw/xen_backend.h       |    1 +
 qemu-char.c            |    7 +--
 qemu-common.h          |    3 -
 qemu-malloc.c          |    5 --
 savevm.c               |   90 +++++++++++++++++++---------------
 18 files changed, 291 insertions(+), 177 deletions(-)

[Qemu-devel] [PATCH 01/19] vpc: Read/write multiple sectors at once

[Kevin Wolf]

This changes the vpc block driver (for VHD) to read/write multiple sectors at
once instead of doing a request for each single sector.

Before this, running qemu-iotests for VPC took ages, now it's actually quite
reasonable to run it always (down from ~1 hour to 40 seconds for me).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vpc.c |   41 ++++++++++++++++++++++++++++++-----------
 1 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 214e9d1..f1f73e2 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -371,23 +371,33 @@ fail:
 static int vpc_read(BlockDriverState *bs, int64_t sector_num,
                     uint8_t *buf, int nb_sectors)
 {
+    BDRVVPCState *s = bs->opaque;
     int ret;
     int64_t offset;
+    int64_t sectors, sectors_per_block;
 
     while (nb_sectors > 0) {
         offset = get_sector_offset(bs, sector_num, 0);
 
+        sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
+        sectors = sectors_per_block - (sector_num % sectors_per_block);
+        if (sectors > nb_sectors) {
+            sectors = nb_sectors;
+        }
+
         if (offset == -1) {
-            memset(buf, 0, 512);
+            memset(buf, 0, sectors * BDRV_SECTOR_SIZE);
         } else {
-            ret = bdrv_pread(bs->file, offset, buf, 512);
-            if (ret != 512)
+            ret = bdrv_pread(bs->file, offset, buf,
+                sectors * BDRV_SECTOR_SIZE);
+            if (ret != sectors * BDRV_SECTOR_SIZE) {
                 return -1;
+            }
         }
 
-        nb_sectors--;
-        sector_num++;
-        buf += 512;
+        nb_sectors -= sectors;
+        sector_num += sectors;
+        buf += sectors * BDRV_SECTOR_SIZE;
     }
     return 0;
 }
@@ -395,25 +405,34 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,
 static int vpc_write(BlockDriverState *bs, int64_t sector_num,
     const uint8_t *buf, int nb_sectors)
 {
+    BDRVVPCState *s = bs->opaque;
     int64_t offset;
+    int64_t sectors, sectors_per_block;
     int ret;
 
     while (nb_sectors > 0) {
         offset = get_sector_offset(bs, sector_num, 1);
 
+        sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
+        sectors = sectors_per_block - (sector_num % sectors_per_block);
+        if (sectors > nb_sectors) {
+            sectors = nb_sectors;
+        }
+
         if (offset == -1) {
             offset = alloc_block(bs, sector_num);
             if (offset < 0)
                 return -1;
         }
 
-        ret = bdrv_pwrite(bs->file, offset, buf, 512);
-        if (ret != 512)
+        ret = bdrv_pwrite(bs->file, offset, buf, sectors * BDRV_SECTOR_SIZE);
+        if (ret != sectors * BDRV_SECTOR_SIZE) {
             return -1;
+        }
 
-        nb_sectors--;
-        sector_num++;
-        buf += 512;
+        nb_sectors -= sectors;
+        sector_num += sectors;
+        buf += sectors * BDRV_SECTOR_SIZE;
     }
 
     return 0;
-- 
1.6.6.1

[Qemu-devel] [PATCH 02/19] qcow2: Allow get_refcount to return errors

[Kevin Wolf]

get_refcount might need to load a refcount block from disk, so errors may
happen. Return the error code instead of assuming a refcount of 1 and change
the callers to respect error return values.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c |   41 +++++++++++++++++++++++++++++++++++++----
 1 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 22b0b45..ca6b373 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -105,11 +105,17 @@ static int load_refcount_block(BlockDriverState *bs,
     return 0;
 }
 
+/*
+ * Returns the refcount of the cluster given by its index. Any non-negative
+ * return value is the refcount of the cluster, negative values are -errno
+ * and indicate an error.
+ */
 static int get_refcount(BlockDriverState *bs, int64_t cluster_index)
 {
     BDRVQcowState *s = bs->opaque;
     int refcount_table_index, block_index;
     int64_t refcount_block_offset;
+    int ret;
 
     refcount_table_index = cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT);
     if (refcount_table_index >= s->refcount_table_size)
@@ -119,8 +125,10 @@ static int get_refcount(BlockDriverState *bs, int64_t cluster_index)
         return 0;
     if (refcount_block_offset != s->refcount_block_cache_offset) {
         /* better than nothing: return allocated if read error */
-        if (load_refcount_block(bs, refcount_block_offset) < 0)
-            return 1;
+        ret = load_refcount_block(bs, refcount_block_offset);
+        if (ret < 0) {
+            return ret;
+        }
     }
     block_index = cluster_index &
         ((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1);
@@ -538,7 +546,13 @@ fail:
     return ret;
 }
 
-/* addend must be 1 or -1 */
+/*
+ * Increases or decreases the refcount of a given cluster by one.
+ * addend must be 1 or -1.
+ *
+ * If the return value is non-negative, it is the new refcount of the cluster.
+ * If it is negative, it is -errno and indicates an error.
+ */
 static int update_cluster_refcount(BlockDriverState *bs,
                                    int64_t cluster_index,
                                    int addend)
@@ -779,6 +793,10 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                         } else {
                             refcount = get_refcount(bs, offset >> s->cluster_bits);
                         }
+
+                        if (refcount < 0) {
+                            goto fail;
+                        }
                     }
 
                     if (refcount == 1) {
@@ -801,7 +819,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
             } else {
                 refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
             }
-            if (refcount == 1) {
+            if (refcount < 0) {
+                goto fail;
+            } else if (refcount == 1) {
                 l2_offset |= QCOW_OFLAG_COPIED;
             }
             if (l2_offset != old_l2_offset) {
@@ -934,6 +954,10 @@ static int check_refcounts_l2(BlockDriverState *bs,
                     uint64_t entry = offset;
                     offset &= ~QCOW_OFLAG_COPIED;
                     refcount = get_refcount(bs, offset >> s->cluster_bits);
+                    if (refcount < 0) {
+                        fprintf(stderr, "Can't get refcount for offset %"
+                            PRIx64 ": %s\n", entry, strerror(-refcount));
+                    }
                     if ((refcount == 1) != ((entry & QCOW_OFLAG_COPIED) != 0)) {
                         fprintf(stderr, "ERROR OFLAG_COPIED: offset=%"
                             PRIx64 " refcount=%d\n", entry, refcount);
@@ -1011,6 +1035,10 @@ static int check_refcounts_l1(BlockDriverState *bs,
             if (check_copied) {
                 refcount = get_refcount(bs, (l2_offset & ~QCOW_OFLAG_COPIED)
                     >> s->cluster_bits);
+                if (refcount < 0) {
+                    fprintf(stderr, "Can't get refcount for l2_offset %"
+                        PRIx64 ": %s\n", l2_offset, strerror(-refcount));
+                }
                 if ((refcount == 1) != ((l2_offset & QCOW_OFLAG_COPIED) != 0)) {
                     fprintf(stderr, "ERROR OFLAG_COPIED: l2_offset=%" PRIx64
                         " refcount=%d\n", l2_offset, refcount);
@@ -1118,6 +1146,11 @@ int qcow2_check_refcounts(BlockDriverState *bs)
     /* compare ref counts */
     for(i = 0; i < nb_clusters; i++) {
         refcount1 = get_refcount(bs, i);
+        if (refcount1 < 0) {
+            fprintf(stderr, "Can't get refcount for cluster %d: %s\n",
+                i, strerror(-refcount1));
+        }
+
         refcount2 = refcount_table[i];
         if (refcount1 != refcount2) {
             fprintf(stderr, "ERROR cluster %d refcount=%d reference=%d\n",
-- 
1.6.6.1

[Qemu-devel] [PATCH 03/19] qcow2: Allow alloc_clusters_noref to return errors

[Kevin Wolf]

Currently it would consider blocks for which get_refcount fails used. However,
it's unlikely that get_refcount would succeed for the next cluster, so it's not
really helpful. Return an error instead.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ca6b373..51948ae 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -228,7 +228,10 @@ static int64_t alloc_refcount_block(BlockDriverState *bs, int64_t cluster_index)
     }
 
     /* Allocate the refcount block itself and mark it as used */
-    uint64_t new_block = alloc_clusters_noref(bs, s->cluster_size);
+    int64_t new_block = alloc_clusters_noref(bs, s->cluster_size);
+    if (new_block < 0) {
+        return new_block;
+    }
 
 #ifdef DEBUG_ALLOC2
     fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64
@@ -579,14 +582,19 @@ static int update_cluster_refcount(BlockDriverState *bs,
 static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size)
 {
     BDRVQcowState *s = bs->opaque;
-    int i, nb_clusters;
+    int i, nb_clusters, refcount;
 
     nb_clusters = size_to_clusters(s, size);
 retry:
     for(i = 0; i < nb_clusters; i++) {
         int64_t next_cluster_index = s->free_cluster_index++;
-        if (get_refcount(bs, next_cluster_index) != 0)
+        refcount = get_refcount(bs, next_cluster_index);
+
+        if (refcount < 0) {
+            return refcount;
+        } else if (refcount != 0) {
             goto retry;
+        }
     }
 #ifdef DEBUG_ALLOC2
     printf("alloc_clusters: size=%" PRId64 " -> %" PRId64 "\n",
@@ -603,6 +611,10 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size)
 
     BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC);
     offset = alloc_clusters_noref(bs, size);
+    if (offset < 0) {
+        return offset;
+    }
+
     ret = update_refcount(bs, offset, size, 1);
     if (ret < 0) {
         return ret;
-- 
1.6.6.1

[Qemu-devel] [PATCH 04/19] qcow2: Return real error code in load_refcount_block

[Kevin Wolf]

This fixes load_refcount_block which completely ignored the return value of
write_refcount_block and always returned -EIO for bdrv_pwrite failure.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 51948ae..41e1da9 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -93,14 +93,19 @@ static int load_refcount_block(BlockDriverState *bs,
     int ret;
 
     if (cache_refcount_updates) {
-        write_refcount_block(bs);
+        ret = write_refcount_block(bs);
+        if (ret < 0) {
+            return ret;
+        }
     }
 
     BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_LOAD);
     ret = bdrv_pread(bs->file, refcount_block_offset, s->refcount_block_cache,
                      s->cluster_size);
-    if (ret != s->cluster_size)
-        return -EIO;
+    if (ret < 0) {
+        return ret;
+    }
+
     s->refcount_block_cache_offset = refcount_block_offset;
     return 0;
 }
-- 
1.6.6.1

[Qemu-devel] [PATCH 05/19] savevm: Really verify if a drive supports snapshots

[Kevin Wolf]

From: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>

Both bdrv_can_snapshot() and bdrv_has_snapshot() does not work as advertized.

First issue: Their names implies different porpouses, but they do the same thing
and have exactly the same code. Maybe copied and pasted and forgotten?
bdrv_has_snapshot() is called in various places for actually checking if there
is snapshots or not.

Second issue: the way bdrv_can_snapshot() verifies if a block driver supports or
not snapshots does not catch all cases. E.g.: a raw image.

So when do_savevm() is called, first thing it does is to set a global
BlockDriverState to save the VM memory state calling get_bs_snapshots().

static BlockDriverState *get_bs_snapshots(void)
{
    BlockDriverState *bs;
    DriveInfo *dinfo;

    if (bs_snapshots)
        return bs_snapshots;
    QTAILQ_FOREACH(dinfo, &drives, next) {
        bs = dinfo->bdrv;
        if (bdrv_can_snapshot(bs))
            goto ok;
    }
    return NULL;
 ok:
    bs_snapshots = bs;
    return bs;
}

bdrv_can_snapshot() may return a BlockDriverState that does not support
snapshots and do_savevm() goes on.

Later on in do_savevm(), we find:

    QTAILQ_FOREACH(dinfo, &drives, next) {
        bs1 = dinfo->bdrv;
        if (bdrv_has_snapshot(bs1)) {
            /* Write VM state size only to the image that contains the state */
            sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
            ret = bdrv_snapshot_create(bs1, sn);
            if (ret < 0) {
                monitor_printf(mon, "Error while creating snapshot on '%s'\n",
                               bdrv_get_device_name(bs1));
            }
        }
    }

bdrv_has_snapshot(bs1) is not checking if the device does support or has
snapshots as explained above. Only in bdrv_snapshot_create() the device is
actually checked for snapshot support.

So, in cases where the first device supports snapshots, and the second does not,
the snapshot on the first will happen anyways. I believe this is not a good
behavior. It should be an all or nothing process.

This patch addresses these issues by making bdrv_can_snapshot() actually do
what it must do and enforces better tests to avoid errors in the middle of
do_savevm(). bdrv_has_snapshot() is removed and replaced by bdrv_can_snapshot()
where appropriate.

bdrv_can_snapshot() was moved from savevm.c to block.c. It makes more sense to me.

The loadvm_state() function was updated too to enforce that when loading a VM at
least all writable devices must support snapshots too.

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c  |   17 +++++++++++++++++
 block.h  |    1 +
 savevm.c |   58 ++++++++++++++++++++++++++++++++++++----------------------
 3 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index cacf11b..29a6f39 100644
--- a/block.c
+++ b/block.c
@@ -1666,6 +1666,23 @@ void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
 /**************************************************************/
 /* handling of snapshots */
 
+int bdrv_can_snapshot(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv || bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
+        return 0;
+    }
+
+    if (!drv->bdrv_snapshot_create) {
+        if (bs->file != NULL) {
+            return bdrv_can_snapshot(bs->file);
+        }
+        return 0;
+    }
+
+    return 1;
+}
+
 int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info)
 {
diff --git a/block.h b/block.h
index 25744b1..a340ffd 100644
--- a/block.h
+++ b/block.h
@@ -175,6 +175,7 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
+int bdrv_can_snapshot(BlockDriverState *bs);
 int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info);
 int bdrv_snapshot_goto(BlockDriverState *bs,
diff --git a/savevm.c b/savevm.c
index 1173a22..ab58d63 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1575,22 +1575,6 @@ out:
     return ret;
 }
 
-/* device can contain snapshots */
-static int bdrv_can_snapshot(BlockDriverState *bs)
-{
-    return (bs &&
-            !bdrv_is_removable(bs) &&
-            !bdrv_is_read_only(bs));
-}
-
-/* device must be snapshots in order to have a reliable snapshot */
-static int bdrv_has_snapshot(BlockDriverState *bs)
-{
-    return (bs &&
-            !bdrv_is_removable(bs) &&
-            !bdrv_is_read_only(bs));
-}
-
 static BlockDriverState *get_bs_snapshots(void)
 {
     BlockDriverState *bs;
@@ -1600,8 +1584,9 @@ static BlockDriverState *get_bs_snapshots(void)
         return bs_snapshots;
     QTAILQ_FOREACH(dinfo, &drives, next) {
         bs = dinfo->bdrv;
-        if (bdrv_can_snapshot(bs))
+        if (bdrv_can_snapshot(bs)) {
             goto ok;
+        }
     }
     return NULL;
  ok:
@@ -1675,12 +1660,26 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 #endif
     const char *name = qdict_get_try_str(qdict, "name");
 
+    /* Verify if there is a device that doesn't support snapshots and is writable */
+    QTAILQ_FOREACH(dinfo, &drives, next) {
+        bs = dinfo->bdrv;
+
+        if (bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
+            continue;
+        }
+
+        if (!bdrv_can_snapshot(bs)) {
+            monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n",
+                               bdrv_get_device_name(bs));
+            return;
+        }
+    }
+
     bs = get_bs_snapshots();
     if (!bs) {
         monitor_printf(mon, "No block device can accept snapshots\n");
         return;
     }
-
     /* ??? Should this occur after vm_stop?  */
     qemu_aio_flush();
 
@@ -1733,7 +1732,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 
     QTAILQ_FOREACH(dinfo, &drives, next) {
         bs1 = dinfo->bdrv;
-        if (bdrv_has_snapshot(bs1)) {
+        if (bdrv_can_snapshot(bs1)) {
             /* Write VM state size only to the image that contains the state */
             sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
             ret = bdrv_snapshot_create(bs1, sn);
@@ -1757,6 +1756,21 @@ int load_vmstate(const char *name)
     QEMUFile *f;
     int ret;
 
+    /* Verify if there is a device that doesn't support snapshots and is writable */
+    QTAILQ_FOREACH(dinfo, &drives, next) {
+        bs = dinfo->bdrv;
+
+        if (bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
+            continue;
+        }
+
+        if (!bdrv_can_snapshot(bs)) {
+            error_report("Device '%s' is writable but does not support snapshots.",
+                               bdrv_get_device_name(bs));
+            return -ENOTSUP;
+        }
+    }
+
     bs = get_bs_snapshots();
     if (!bs) {
         error_report("No block device supports snapshots");
@@ -1768,7 +1782,7 @@ int load_vmstate(const char *name)
 
     QTAILQ_FOREACH(dinfo, &drives, next) {
         bs1 = dinfo->bdrv;
-        if (bdrv_has_snapshot(bs1)) {
+        if (bdrv_can_snapshot(bs1)) {
             ret = bdrv_snapshot_goto(bs1, name);
             if (ret < 0) {
                 switch(ret) {
@@ -1830,7 +1844,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
 
     QTAILQ_FOREACH(dinfo, &drives, next) {
         bs1 = dinfo->bdrv;
-        if (bdrv_has_snapshot(bs1)) {
+        if (bdrv_can_snapshot(bs1)) {
             ret = bdrv_snapshot_delete(bs1, name);
             if (ret < 0) {
                 if (ret == -ENOTSUP)
@@ -1861,7 +1875,7 @@ void do_info_snapshots(Monitor *mon)
     monitor_printf(mon, "Snapshot devices:");
     QTAILQ_FOREACH(dinfo, &drives, next) {
         bs1 = dinfo->bdrv;
-        if (bdrv_has_snapshot(bs1)) {
+        if (bdrv_can_snapshot(bs1)) {
             if (bs == bs1)
                 monitor_printf(mon, " %s", bdrv_get_device_name(bs1));
         }
-- 
1.6.6.1

[Qemu-devel] [PATCH 06/19] Fix regression for "-drive file="

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Empty file used to create an empty drive (no media).  Since commit
9dfd7c7a, it's an error: "qemu: could not open disk image : No such
file or directory".  Older versions of libvirt can choke on this.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 642ce75..dbeef09 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -462,7 +462,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     case IF_COUNT:
         abort();
     }
-    if (!file) {
+    if (!file || !*file) {
         *fatal_error = 0;
         return NULL;
     }
-- 
1.6.6.1

[Qemu-devel] [PATCH 07/19] qcow2: Restore L1 entry on l2_allocate failure

[Kevin Wolf]

If writing the L1 table to disk failed, we need to restore its old content in
memory to avoid inconsistencies.

Reported-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 03a9f25..5760ad6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -285,6 +285,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table)
     return 0;
 
 fail:
+    s->l1_table[l1_index] = old_l2_offset;
     qcow2_l2_cache_reset(bs);
     return ret;
 }
-- 
1.6.6.1

[Qemu-devel] [PATCH 08/19] cow: use pread/pwrite

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Use pread/pwrite instead of lseek + read/write in preparation of using the
qemu block API.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/cow.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index fde066e..1a48ac0 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -78,7 +78,7 @@ static int cow_open(BlockDriverState *bs, const char *filename, int flags)
     }
     s->fd = fd;
     /* see if it is a cow image */
-    if (read(fd, &cow_header, sizeof(cow_header)) != sizeof(cow_header)) {
+    if (pread(fd, &cow_header, sizeof(cow_header), 0) != sizeof(cow_header)) {
         goto fail;
     }
 
@@ -159,8 +159,8 @@ static int cow_read(BlockDriverState *bs, int64_t sector_num,
 
     while (nb_sectors > 0) {
         if (is_changed(s->cow_bitmap, sector_num, nb_sectors, &n)) {
-            lseek(s->fd, s->cow_sectors_offset + sector_num * 512, SEEK_SET);
-            ret = read(s->fd, buf, n * 512);
+            ret = pread(s->fd, buf, n * 512,
+                        s->cow_sectors_offset + sector_num * 512);
             if (ret != n * 512)
                 return -1;
         } else {
@@ -186,8 +186,8 @@ static int cow_write(BlockDriverState *bs, int64_t sector_num,
     BDRVCowState *s = bs->opaque;
     int ret, i;
 
-    lseek(s->fd, s->cow_sectors_offset + sector_num * 512, SEEK_SET);
-    ret = write(s->fd, buf, nb_sectors * 512);
+    ret = pwrite(s->fd, buf, nb_sectors * 512,
+                 s->cow_sectors_offset + sector_num * 512);
     if (ret != nb_sectors * 512)
         return -1;
     for (i = 0; i < nb_sectors; i++)
-- 
1.6.6.1

[Qemu-devel] [PATCH 09/19] cow: stop using mmap

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

We don't have an equivalent to mmap in the qemu block API, so read and
write the bitmap directly.  At least in the dumb implementation added
in this patch this is a lot less efficient, but it means cow can also
work on windows, and over nbd or curl.  And it fixes qemu-iotests testcase
012 which did not work properly due to issues with read-only mmap access.

In addition we can also get rid of the now unused get_mmap_addr function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/cow.c   |   98 +++++++++++++++++++++++++++++++++++---------------------
 qemu-common.h |    3 --
 qemu-malloc.c |    5 ---
 3 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 1a48ac0..ec9a9f7 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -21,11 +21,9 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-#ifndef _WIN32
 #include "qemu-common.h"
 #include "block_int.h"
 #include "module.h"
-#include <sys/mman.h>
 
 /**************************************************************/
 /* COW block driver using file system holes */
@@ -45,9 +43,6 @@ struct cow_header_v2 {
 
 typedef struct BDRVCowState {
     int fd;
-    uint8_t *cow_bitmap; /* if non NULL, COW mappings are used first */
-    uint8_t *cow_bitmap_addr; /* mmap address of cow_bitmap */
-    int cow_bitmap_size;
     int64_t cow_sectors_offset;
 } BDRVCowState;
 
@@ -68,6 +63,7 @@ static int cow_open(BlockDriverState *bs, const char *filename, int flags)
     BDRVCowState *s = bs->opaque;
     int fd;
     struct cow_header_v2 cow_header;
+    int bitmap_size;
     int64_t size;
 
     fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
@@ -94,61 +90,92 @@ static int cow_open(BlockDriverState *bs, const char *filename, int flags)
     pstrcpy(bs->backing_file, sizeof(bs->backing_file),
             cow_header.backing_file);
 
-    /* mmap the bitmap */
-    s->cow_bitmap_size = ((bs->total_sectors + 7) >> 3) + sizeof(cow_header);
-    s->cow_bitmap_addr = (void *)mmap(get_mmap_addr(s->cow_bitmap_size),
-                                      s->cow_bitmap_size,
-                                      PROT_READ | PROT_WRITE,
-                                      MAP_SHARED, s->fd, 0);
-    if (s->cow_bitmap_addr == MAP_FAILED)
-        goto fail;
-    s->cow_bitmap = s->cow_bitmap_addr + sizeof(cow_header);
-    s->cow_sectors_offset = (s->cow_bitmap_size + 511) & ~511;
+    bitmap_size = ((bs->total_sectors + 7) >> 3) + sizeof(cow_header);
+    s->cow_sectors_offset = (bitmap_size + 511) & ~511;
     return 0;
  fail:
     close(fd);
     return -1;
 }
 
-static inline void cow_set_bit(uint8_t *bitmap, int64_t bitnum)
+/*
+ * XXX(hch): right now these functions are extremly ineffcient.
+ * We should just read the whole bitmap we'll need in one go instead.
+ */
+static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
 {
-    bitmap[bitnum / 8] |= (1 << (bitnum%8));
+    BDRVCowState *s = bs->opaque;
+    uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
+    uint8_t bitmap;
+
+    if (pread(s->fd, &bitmap, sizeof(bitmap), offset) !=
+	    sizeof(bitmap)) {
+       return -errno;
+    }
+
+    bitmap |= (1 << (bitnum % 8));
+
+    if (pwrite(s->fd, &bitmap, sizeof(bitmap), offset) !=
+	    sizeof(bitmap)) {
+       return -errno;
+    }
+    return 0;
 }
 
-static inline int is_bit_set(const uint8_t *bitmap, int64_t bitnum)
+static inline int is_bit_set(BlockDriverState *bs, int64_t bitnum)
 {
-    return !!(bitmap[bitnum / 8] & (1 << (bitnum%8)));
-}
+    BDRVCowState *s = bs->opaque;
+    uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
+    uint8_t bitmap;
 
+    if (pread(s->fd, &bitmap, sizeof(bitmap), offset) !=
+	    sizeof(bitmap)) {
+       return -errno;
+    }
+
+    return !!(bitmap & (1 << (bitnum % 8)));
+}
 
 /* Return true if first block has been changed (ie. current version is
  * in COW file).  Set the number of continuous blocks for which that
  * is true. */
-static inline int is_changed(uint8_t *bitmap,
-                             int64_t sector_num, int nb_sectors,
-                             int *num_same)
+static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num,
+        int nb_sectors, int *num_same)
 {
     int changed;
 
-    if (!bitmap || nb_sectors == 0) {
+    if (nb_sectors == 0) {
 	*num_same = nb_sectors;
 	return 0;
     }
 
-    changed = is_bit_set(bitmap, sector_num);
+    changed = is_bit_set(bs, sector_num);
+    if (changed < 0) {
+        return 0; /* XXX: how to return I/O errors? */
+    }
+
     for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
-	if (is_bit_set(bitmap, sector_num + *num_same) != changed)
+	if (is_bit_set(bs, sector_num + *num_same) != changed)
 	    break;
     }
 
     return changed;
 }
 
-static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num,
-                            int nb_sectors, int *pnum)
+static int cow_update_bitmap(BlockDriverState *bs, int64_t sector_num,
+        int nb_sectors)
 {
-    BDRVCowState *s = bs->opaque;
-    return is_changed(s->cow_bitmap, sector_num, nb_sectors, pnum);
+    int error = 0;
+    int i;
+
+    for (i = 0; i < nb_sectors; i++) {
+        error = cow_set_bit(bs, sector_num + i);
+        if (error) {
+            break;
+        }
+    }
+
+    return error;
 }
 
 static int cow_read(BlockDriverState *bs, int64_t sector_num,
@@ -158,7 +185,7 @@ static int cow_read(BlockDriverState *bs, int64_t sector_num,
     int ret, n;
 
     while (nb_sectors > 0) {
-        if (is_changed(s->cow_bitmap, sector_num, nb_sectors, &n)) {
+        if (cow_is_allocated(bs, sector_num, nb_sectors, &n)) {
             ret = pread(s->fd, buf, n * 512,
                         s->cow_sectors_offset + sector_num * 512);
             if (ret != n * 512)
@@ -184,21 +211,19 @@ static int cow_write(BlockDriverState *bs, int64_t sector_num,
                      const uint8_t *buf, int nb_sectors)
 {
     BDRVCowState *s = bs->opaque;
-    int ret, i;
+    int ret;
 
     ret = pwrite(s->fd, buf, nb_sectors * 512,
                  s->cow_sectors_offset + sector_num * 512);
     if (ret != nb_sectors * 512)
         return -1;
-    for (i = 0; i < nb_sectors; i++)
-        cow_set_bit(s->cow_bitmap, sector_num + i);
-    return 0;
+
+    return cow_update_bitmap(bs, sector_num, nb_sectors);
 }
 
 static void cow_close(BlockDriverState *bs)
 {
     BDRVCowState *s = bs->opaque;
-    munmap((void *)s->cow_bitmap_addr, s->cow_bitmap_size);
     close(s->fd);
 }
 
@@ -308,4 +333,3 @@ static void bdrv_cow_init(void)
 }
 
 block_init(bdrv_cow_init);
-#endif
diff --git a/qemu-common.h b/qemu-common.h
index d133f35..00998c3 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -164,9 +164,6 @@ void qemu_free(void *ptr);
 char *qemu_strdup(const char *str);
 char *qemu_strndup(const char *str, size_t size);
 
-void *get_mmap_addr(unsigned long size);
-
-
 void qemu_mutex_lock_iothread(void);
 void qemu_mutex_unlock_iothread(void);
 
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 1b33e04..36b0b36 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -32,11 +32,6 @@ static void *oom_check(void *ptr)
     return ptr;
 }
 
-void *get_mmap_addr(unsigned long size)
-{
-    return NULL;
-}
-
 void qemu_free(void *ptr)
 {
     free(ptr);
-- 
1.6.6.1

[Qemu-devel] [PATCH 10/19] cow: use qemu block API

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Use bdrv_pwrite to access the backing device instead of pread, and
convert the driver to implementing the bdrv_open method which gives
it an already opened BlockDriverState for the underlying device.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/cow.c |   39 +++++++++++++--------------------------
 1 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index ec9a9f7..d146434 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -42,7 +42,6 @@ struct cow_header_v2 {
 };
 
 typedef struct BDRVCowState {
-    int fd;
     int64_t cow_sectors_offset;
 } BDRVCowState;
 
@@ -58,23 +57,16 @@ static int cow_probe(const uint8_t *buf, int buf_size, const char *filename)
         return 0;
 }
 
-static int cow_open(BlockDriverState *bs, const char *filename, int flags)
+static int cow_open(BlockDriverState *bs, int flags)
 {
     BDRVCowState *s = bs->opaque;
-    int fd;
     struct cow_header_v2 cow_header;
     int bitmap_size;
     int64_t size;
 
-    fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
-    if (fd < 0) {
-        fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
-        if (fd < 0)
-            return -1;
-    }
-    s->fd = fd;
     /* see if it is a cow image */
-    if (pread(fd, &cow_header, sizeof(cow_header), 0) != sizeof(cow_header)) {
+    if (bdrv_pread(bs->file, 0, &cow_header, sizeof(cow_header)) !=
+            sizeof(cow_header)) {
         goto fail;
     }
 
@@ -94,7 +86,6 @@ static int cow_open(BlockDriverState *bs, const char *filename, int flags)
     s->cow_sectors_offset = (bitmap_size + 511) & ~511;
     return 0;
  fail:
-    close(fd);
     return -1;
 }
 
@@ -104,18 +95,17 @@ static int cow_open(BlockDriverState *bs, const char *filename, int flags)
  */
 static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
 {
-    BDRVCowState *s = bs->opaque;
     uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
     uint8_t bitmap;
 
-    if (pread(s->fd, &bitmap, sizeof(bitmap), offset) !=
+    if (bdrv_pread(bs->file, offset, &bitmap, sizeof(bitmap)) !=
 	    sizeof(bitmap)) {
        return -errno;
     }
 
     bitmap |= (1 << (bitnum % 8));
 
-    if (pwrite(s->fd, &bitmap, sizeof(bitmap), offset) !=
+    if (bdrv_pwrite(bs->file, offset, &bitmap, sizeof(bitmap)) !=
 	    sizeof(bitmap)) {
        return -errno;
     }
@@ -124,11 +114,10 @@ static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
 
 static inline int is_bit_set(BlockDriverState *bs, int64_t bitnum)
 {
-    BDRVCowState *s = bs->opaque;
     uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
     uint8_t bitmap;
 
-    if (pread(s->fd, &bitmap, sizeof(bitmap), offset) !=
+    if (bdrv_pread(bs->file, offset, &bitmap, sizeof(bitmap)) !=
 	    sizeof(bitmap)) {
        return -errno;
     }
@@ -186,8 +175,9 @@ static int cow_read(BlockDriverState *bs, int64_t sector_num,
 
     while (nb_sectors > 0) {
         if (cow_is_allocated(bs, sector_num, nb_sectors, &n)) {
-            ret = pread(s->fd, buf, n * 512,
-                        s->cow_sectors_offset + sector_num * 512);
+            ret = bdrv_pread(bs->file,
+                        s->cow_sectors_offset + sector_num * 512,
+                        buf, n * 512);
             if (ret != n * 512)
                 return -1;
         } else {
@@ -213,8 +203,8 @@ static int cow_write(BlockDriverState *bs, int64_t sector_num,
     BDRVCowState *s = bs->opaque;
     int ret;
 
-    ret = pwrite(s->fd, buf, nb_sectors * 512,
-                 s->cow_sectors_offset + sector_num * 512);
+    ret = bdrv_pwrite(bs->file, s->cow_sectors_offset + sector_num * 512,
+                      buf, nb_sectors * 512);
     if (ret != nb_sectors * 512)
         return -1;
 
@@ -223,8 +213,6 @@ static int cow_write(BlockDriverState *bs, int64_t sector_num,
 
 static void cow_close(BlockDriverState *bs)
 {
-    BDRVCowState *s = bs->opaque;
-    close(s->fd);
 }
 
 static int cow_create(const char *filename, QEMUOptionParameter *options)
@@ -294,8 +282,7 @@ exit:
 
 static void cow_flush(BlockDriverState *bs)
 {
-    BDRVCowState *s = bs->opaque;
-    qemu_fdatasync(s->fd);
+    bdrv_flush(bs->file);
 }
 
 static QEMUOptionParameter cow_create_options[] = {
@@ -316,7 +303,7 @@ static BlockDriver bdrv_cow = {
     .format_name	= "cow",
     .instance_size	= sizeof(BDRVCowState),
     .bdrv_probe		= cow_probe,
-    .bdrv_file_open	= cow_open,
+    .bdrv_open		= cow_open,
     .bdrv_read		= cow_read,
     .bdrv_write		= cow_write,
     .bdrv_close		= cow_close,
-- 
1.6.6.1

[Qemu-devel] [PATCH 11/19] block: Move error actions from DriveInfo to BlockDriverState

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

That's where they belong semantically (block device host part), even
though the actions are actually executed by guest device code.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c         |   12 ++++++++++++
 block.h         |    8 ++++++++
 block_int.h     |    1 +
 blockdev.c      |   17 ++---------------
 blockdev.h      |   10 ----------
 hw/ide/core.c   |    2 +-
 hw/scsi-disk.c  |    2 +-
 hw/virtio-blk.c |    3 +--
 8 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/block.c b/block.c
index 29a6f39..e701ec0 100644
--- a/block.c
+++ b/block.c
@@ -1206,6 +1206,18 @@ int bdrv_get_translation_hint(BlockDriverState *bs)
     return bs->translation;
 }
 
+void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
+                       BlockErrorAction on_write_error)
+{
+    bs->on_read_error = on_read_error;
+    bs->on_write_error = on_write_error;
+}
+
+BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read)
+{
+    return is_read ? bs->on_read_error : bs->on_write_error;
+}
+
 int bdrv_is_removable(BlockDriverState *bs)
 {
     return bs->removable;
diff --git a/block.h b/block.h
index a340ffd..d22401f 100644
--- a/block.h
+++ b/block.h
@@ -42,6 +42,11 @@ typedef struct QEMUSnapshotInfo {
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
 typedef enum {
+    BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
+    BLOCK_ERR_STOP_ANY
+} BlockErrorAction;
+
+typedef enum {
     BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
 } BlockMonEventAction;
 
@@ -146,6 +151,9 @@ void bdrv_get_geometry_hint(BlockDriverState *bs,
                             int *pcyls, int *pheads, int *psecs);
 int bdrv_get_type_hint(BlockDriverState *bs);
 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);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 1a7240c..e3bfd19 100644
--- a/block_int.h
+++ b/block_int.h
@@ -182,6 +182,7 @@ struct BlockDriverState {
        drivers. They are not used by the block driver */
     int cyls, heads, secs, translation;
     int type;
+    BlockErrorAction on_read_error, on_write_error;
     char device_name[32];
     unsigned long *dirty_bitmap;
     int64_t dirty_count;
diff --git a/blockdev.c b/blockdev.c
index dbeef09..b5570f4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -90,19 +90,6 @@ const char *drive_get_serial(BlockDriverState *bdrv)
     return "\0";
 }
 
-BlockInterfaceErrorAction drive_get_on_error(
-    BlockDriverState *bdrv, int is_read)
-{
-    DriveInfo *dinfo;
-
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        if (dinfo->bdrv == bdrv)
-            return is_read ? dinfo->on_read_error : dinfo->on_write_error;
-    }
-
-    return is_read ? BLOCK_ERR_REPORT : BLOCK_ERR_STOP_ENOSPC;
-}
-
 static void bdrv_format_print(void *opaque, const char *name)
 {
     fprintf(stderr, " %s", name);
@@ -418,13 +405,13 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->on_read_error = on_read_error;
-    dinfo->on_write_error = on_write_error;
     dinfo->opts = opts;
     if (serial)
         strncpy(dinfo->serial, serial, sizeof(dinfo->serial) - 1);
     QTAILQ_INSERT_TAIL(&drives, dinfo, next);
 
+    bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
+
     switch(type) {
     case IF_IDE:
     case IF_SCSI:
diff --git a/blockdev.h b/blockdev.h
index dfc9de1..9e8a7fc 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -19,11 +19,6 @@ typedef enum {
     IF_COUNT
 } BlockInterfaceType;
 
-typedef enum {
-    BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
-    BLOCK_ERR_STOP_ANY
-} BlockInterfaceErrorAction;
-
 #define BLOCK_SERIAL_STRLEN 20
 
 typedef struct DriveInfo {
@@ -34,8 +29,6 @@ typedef struct DriveInfo {
     int bus;
     int unit;
     QemuOpts *opts;
-    BlockInterfaceErrorAction on_read_error;
-    BlockInterfaceErrorAction on_write_error;
     char serial[BLOCK_SERIAL_STRLEN + 1];
     QTAILQ_ENTRY(DriveInfo) next;
 } DriveInfo;
@@ -51,9 +44,6 @@ extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
 
-extern BlockInterfaceErrorAction drive_get_on_error(
-    BlockDriverState *bdrv, int is_read);
-
 extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
 extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
                              int *fatal_error);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 045d18d..0b3b7c2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -481,7 +481,7 @@ void ide_dma_error(IDEState *s)
 static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
     int is_read = (op & BM_STATUS_RETRY_READ);
-    BlockInterfaceErrorAction action = drive_get_on_error(s->bs, is_read);
+    BlockErrorAction action = bdrv_get_on_error(s->bs, is_read);
 
     if (action == BLOCK_ERR_IGNORE) {
         bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a9bf7d2..2b38984 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -182,7 +182,7 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
 static int scsi_handle_write_error(SCSIDiskReq *r, int error)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-    BlockInterfaceErrorAction action = drive_get_on_error(s->bs, 0);
+    BlockErrorAction action = bdrv_get_on_error(s->bs, 0);
 
     if (action == BLOCK_ERR_IGNORE) {
         bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, 0);
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 4cc94f6..75878eb 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -58,8 +58,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     int is_read)
 {
-    BlockInterfaceErrorAction action =
-        drive_get_on_error(req->dev->bs, is_read);
+    BlockErrorAction action = bdrv_get_on_error(req->dev->bs, is_read);
     VirtIOBlock *s = req->dev;
 
     if (action == BLOCK_ERR_IGNORE) {
-- 
1.6.6.1

[Qemu-devel] [PATCH 12/19] block: Decouple block device "commit all" from DriveInfo

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

do_commit() and mux_proc_byte() iterate over the list of drives
defined with drive_init().  This misses host block devices defined by
other means.  Such means don't exist now, but will be introduced later
in this series.

Change them to use new bdrv_commit_all(), which iterates over all host
block devices.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c     |    9 +++++++++
 block.h     |    1 +
 blockdev.c  |   16 ++++++++--------
 qemu-char.c |    7 +------
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index e701ec0..df2d3a6 100644
--- a/block.c
+++ b/block.c
@@ -788,6 +788,15 @@ ro_cleanup:
     return ret;
 }
 
+void bdrv_commit_all(void)
+{
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        bdrv_commit(bs);
+    }
+}
+
 /*
  * Return values:
  * 0        - success
diff --git a/block.h b/block.h
index d22401f..1f69cbd 100644
--- a/block.h
+++ b/block.h
@@ -85,6 +85,7 @@ int64_t bdrv_getlength(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
 void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs);
 int bdrv_commit(BlockDriverState *bs);
+void bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
     const char *backing_file, const char *backing_fmt);
 void bdrv_register(BlockDriver *bdrv);
diff --git a/blockdev.c b/blockdev.c
index b5570f4..d74cd1d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -486,16 +486,16 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 
 void do_commit(Monitor *mon, const QDict *qdict)
 {
-    int all_devices;
-    DriveInfo *dinfo;
     const char *device = qdict_get_str(qdict, "device");
+    BlockDriverState *bs;
 
-    all_devices = !strcmp(device, "all");
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        if (!all_devices)
-            if (strcmp(bdrv_get_device_name(dinfo->bdrv), device))
-                continue;
-        bdrv_commit(dinfo->bdrv);
+    if (!strcmp(device, "all")) {
+        bdrv_commit_all();
+    } else {
+        bs = bdrv_find(device);
+        if (bs) {
+            bdrv_commit(bs);
+        }
     }
 }
 
diff --git a/qemu-char.c b/qemu-char.c
index 87628ea..9b69d92 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -351,12 +351,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
                  break;
             }
         case 's':
-            {
-                DriveInfo *dinfo;
-                QTAILQ_FOREACH(dinfo, &drives, next) {
-                    bdrv_commit(dinfo->bdrv);
-                }
-            }
+            bdrv_commit_all();
             break;
         case 'b':
             qemu_chr_event(chr, CHR_EVENT_BREAK);
-- 
1.6.6.1

[Qemu-devel] [PATCH 13/19] monitor: Make "commit FOO" complain when FOO doesn't exist

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d74cd1d..f636bc6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -493,9 +493,11 @@ void do_commit(Monitor *mon, const QDict *qdict)
         bdrv_commit_all();
     } else {
         bs = bdrv_find(device);
-        if (bs) {
-            bdrv_commit(bs);
+        if (!bs) {
+            qerror_report(QERR_DEVICE_NOT_FOUND, device);
+            return;
         }
+        bdrv_commit(bs);
     }
 }
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 14/19] block: New bdrv_next()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

This is a more flexible alternative to bdrv_iterate().

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

diff --git a/block.c b/block.c
index df2d3a6..383d59f 100644
--- a/block.c
+++ b/block.c
@@ -1330,6 +1330,14 @@ BlockDriverState *bdrv_find(const char *name)
     return NULL;
 }
 
+BlockDriverState *bdrv_next(BlockDriverState *bs)
+{
+    if (!bs) {
+        return QTAILQ_FIRST(&bdrv_states);
+    }
+    return QTAILQ_NEXT(bs, list);
+}
+
 void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque)
 {
     BlockDriverState *bs;
diff --git a/block.h b/block.h
index 1f69cbd..9df9b38 100644
--- a/block.h
+++ b/block.h
@@ -168,6 +168,7 @@ void bdrv_set_change_cb(BlockDriverState *bs,
                         void (*change_cb)(void *opaque), void *opaque);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
 BlockDriverState *bdrv_find(const char *name);
+BlockDriverState *bdrv_next(BlockDriverState *bs);
 void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs),
                   void *opaque);
 int bdrv_is_encrypted(BlockDriverState *bs);
-- 
1.6.6.1

[Qemu-devel] [PATCH 15/19] block: Decouple savevm from DriveInfo

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

We find snapshots by iterating over the list of drives defined with
drive_init().  This misses host block devices defined by other means.
Such means don't exist now, but will be introduced later in this
series.

Iterate over all host block devices instead, with bdrv_next().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 savevm.c |   40 ++++++++++++++++++----------------------
 1 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/savevm.c b/savevm.c
index ab58d63..20354a8 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1578,12 +1578,13 @@ out:
 static BlockDriverState *get_bs_snapshots(void)
 {
     BlockDriverState *bs;
-    DriveInfo *dinfo;
 
     if (bs_snapshots)
         return bs_snapshots;
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs = dinfo->bdrv;
+    /* FIXME what if bs_snapshots gets hot-unplugged? */
+
+    bs = NULL;
+    while ((bs = bdrv_next(bs))) {
         if (bdrv_can_snapshot(bs)) {
             goto ok;
         }
@@ -1622,12 +1623,11 @@ static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
 static int del_existing_snapshots(Monitor *mon, const char *name)
 {
     BlockDriverState *bs;
-    DriveInfo *dinfo;
     QEMUSnapshotInfo sn1, *snapshot = &sn1;
     int ret;
 
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs = dinfo->bdrv;
+    bs = NULL;
+    while ((bs = bdrv_next(bs))) {
         if (bdrv_can_snapshot(bs) &&
             bdrv_snapshot_find(bs, snapshot, name) >= 0)
         {
@@ -1646,7 +1646,6 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
 
 void do_savevm(Monitor *mon, const QDict *qdict)
 {
-    DriveInfo *dinfo;
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
     int ret;
@@ -1661,8 +1660,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     const char *name = qdict_get_try_str(qdict, "name");
 
     /* Verify if there is a device that doesn't support snapshots and is writable */
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs = dinfo->bdrv;
+    bs = NULL;
+    while ((bs = bdrv_next(bs))) {
 
         if (bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
             continue;
@@ -1730,8 +1729,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 
     /* create the snapshots */
 
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs1 = dinfo->bdrv;
+    bs1 = NULL;
+    while ((bs1 = bdrv_next(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
             /* Write VM state size only to the image that contains the state */
             sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
@@ -1750,15 +1749,14 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 
 int load_vmstate(const char *name)
 {
-    DriveInfo *dinfo;
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn;
     QEMUFile *f;
     int ret;
 
     /* Verify if there is a device that doesn't support snapshots and is writable */
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs = dinfo->bdrv;
+    bs = NULL;
+    while ((bs = bdrv_next(bs))) {
 
         if (bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
             continue;
@@ -1780,8 +1778,8 @@ int load_vmstate(const char *name)
     /* Flush all IO requests so they don't interfere with the new state.  */
     qemu_aio_flush();
 
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs1 = dinfo->bdrv;
+    bs1 = NULL;
+    while ((bs1 = bdrv_next(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
             ret = bdrv_snapshot_goto(bs1, name);
             if (ret < 0) {
@@ -1831,7 +1829,6 @@ int load_vmstate(const char *name)
 
 void do_delvm(Monitor *mon, const QDict *qdict)
 {
-    DriveInfo *dinfo;
     BlockDriverState *bs, *bs1;
     int ret;
     const char *name = qdict_get_str(qdict, "name");
@@ -1842,8 +1839,8 @@ void do_delvm(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs1 = dinfo->bdrv;
+    bs1 = NULL;
+    while ((bs1 = bdrv_next(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
             ret = bdrv_snapshot_delete(bs1, name);
             if (ret < 0) {
@@ -1861,7 +1858,6 @@ void do_delvm(Monitor *mon, const QDict *qdict)
 
 void do_info_snapshots(Monitor *mon)
 {
-    DriveInfo *dinfo;
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo *sn_tab, *sn;
     int nb_sns, i;
@@ -1873,8 +1869,8 @@ void do_info_snapshots(Monitor *mon)
         return;
     }
     monitor_printf(mon, "Snapshot devices:");
-    QTAILQ_FOREACH(dinfo, &drives, next) {
-        bs1 = dinfo->bdrv;
+    bs1 = NULL;
+    while ((bs1 = bdrv_next(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
             if (bs == bs1)
                 monitor_printf(mon, " %s", bdrv_get_device_name(bs1));
-- 
1.6.6.1

[Qemu-devel] [PATCH 16/19] blockdev: Give drives internal linkage

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

This is the list of drives defined with drive_init().  Hide it, so it
doesn't get abused.

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

diff --git a/blockdev.c b/blockdev.c
index f636bc6..b376884 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -15,7 +15,7 @@
 #include "qemu-config.h"
 #include "sysemu.h"
 
-struct drivelist drives = QTAILQ_HEAD_INITIALIZER(drives);
+static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
 
 QemuOpts *drive_add(const char *file, const char *fmt, ...)
 {
diff --git a/blockdev.h b/blockdev.h
index 9e8a7fc..23ea576 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -36,8 +36,6 @@ typedef struct DriveInfo {
 #define MAX_IDE_DEVS	2
 #define MAX_SCSI_DEVS	7
 
-extern QTAILQ_HEAD(drivelist, DriveInfo) drives;
-
 extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
-- 
1.6.6.1

[Qemu-devel] [PATCH 17/19] Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE in hw/fdc.c

Per https://bugs.launchpad.net/qemu/+bug/424453 the correct values
for FD_CMD_SAVE is 0x2e and FD_CMD_RESTORE is 0x4e. Verified against
the Intel 82078 manual which can be found at:
http://wiki.qemu.org/Documentation/HardwareManuals page 22.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/fdc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index b978957..45a876d 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -369,9 +369,9 @@ enum {
     FD_CMD_PART_ID = 0x18,
     FD_CMD_SCAN_LOW_OR_EQUAL = 0x19,
     FD_CMD_SCAN_HIGH_OR_EQUAL = 0x1d,
-    FD_CMD_SAVE = 0x2c,
+    FD_CMD_SAVE = 0x2e,
     FD_CMD_OPTION = 0x33,
-    FD_CMD_RESTORE = 0x4c,
+    FD_CMD_RESTORE = 0x4e,
     FD_CMD_DRIVE_SPECIFICATION_COMMAND = 0x8e,
     FD_CMD_RELATIVE_SEEK_OUT = 0x8f,
     FD_CMD_FORMAT_AND_WRITE = 0xcd,
-- 
1.6.6.1

[Qemu-devel] [PATCH 18/19] block: fix a warning and possible truncation

[Kevin Wolf]

From: Blue Swirl <blauwirbel@gmail.com>

Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
/src/qemu/block.c: In function `bdrv_info_stats_bs':
/src/qemu/block.c:1548: warning: long long int format, long unsigned
int arg (arg 6)

There may be also truncation effects.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 383d59f..3fc2969 100644
--- a/block.c
+++ b/block.c
@@ -1574,7 +1574,8 @@ static QObject* bdrv_info_stats_bs(BlockDriverState *bs)
                              "} }",
                              bs->rd_bytes, bs->wr_bytes,
                              bs->rd_ops, bs->wr_ops,
-                             bs->wr_highest_sector * (long)BDRV_SECTOR_SIZE);
+                             bs->wr_highest_sector *
+                             (uint64_t)BDRV_SECTOR_SIZE);
     dict  = qobject_to_qdict(res);
 
     if (*bs->device_name) {
-- 
1.6.6.1

[Qemu-devel] [PATCH 19/19] xen: Fix build error due to missing include

[Kevin Wolf]

From: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/xen_backend.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/xen_backend.h b/hw/xen_backend.h
index f07f7d4..cc25f9d 100644
--- a/hw/xen_backend.h
+++ b/hw/xen_backend.h
@@ -5,6 +5,7 @@
 #include "sysemu.h"
 #include "net.h"
 #include "block_int.h"
+#include "blockdev.h"
 
 /* ------------------------------------------------------------- */
 
-- 
1.6.6.1

Re: [Qemu-devel] [PULL 00/19] Block patches

[Anthony Liguori]

On 06/15/2010 09:19 AM, Kevin Wolf wrote:
> The following changes since commit fd42deeb4cb42f90084046e3ebdb4383953195e3:
>    Gerd Hoffmann (1):
>          Add exit notifiers.
>
> are available in the git repository at:
>
>    git://repo.or.cz/qemu/kevin.git for-anthony
>    

Merged.  Thanks.

Regards,

Anthony Liguori

> Blue Swirl (1):
>        block: fix a warning and possible truncation
>
> Christoph Hellwig (3):
>        cow: use pread/pwrite
>        cow: stop using mmap
>        cow: use qemu block API
>
> Jan Kiszka (1):
>        xen: Fix build error due to missing include
>
> Jes Sorensen (1):
>        Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE
>
> Kevin Wolf (5):
>        vpc: Read/write multiple sectors at once
>        qcow2: Allow get_refcount to return errors
>        qcow2: Allow alloc_clusters_noref to return errors
>        qcow2: Return real error code in load_refcount_block
>        qcow2: Restore L1 entry on l2_allocate failure
>
> Markus Armbruster (7):
>        Fix regression for "-drive file="
>        block: Move error actions from DriveInfo to BlockDriverState
>        block: Decouple block device "commit all" from DriveInfo
>        monitor: Make "commit FOO" complain when FOO doesn't exist
>        block: New bdrv_next()
>        block: Decouple savevm from DriveInfo
>        blockdev: Give drives internal linkage
>
> Miguel Di Ciurcio Filho (1):
>        savevm: Really verify if a drive supports snapshots
>
>   block.c                |   49 ++++++++++++++++++-
>   block.h                |   11 ++++
>   block/cow.c            |  127 ++++++++++++++++++++++++++----------------------
>   block/qcow2-cluster.c  |    1 +
>   block/qcow2-refcount.c |   70 +++++++++++++++++++++++----
>   block/vpc.c            |   41 +++++++++++----
>   block_int.h            |    1 +
>   blockdev.c             |   39 +++++---------
>   blockdev.h             |   12 -----
>   hw/fdc.c               |    4 +-
>   hw/ide/core.c          |    2 +-
>   hw/scsi-disk.c         |    2 +-
>   hw/virtio-blk.c        |    3 +-
>   hw/xen_backend.h       |    1 +
>   qemu-char.c            |    7 +--
>   qemu-common.h          |    3 -
>   qemu-malloc.c          |    5 --
>   savevm.c               |   90 +++++++++++++++++++---------------
>   18 files changed, 291 insertions(+), 177 deletions(-)
>
>
>    

[Qemu-devel] Re: [PATCH 17/19] Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

[Anthony Liguori]

On 06/15/2010 09:19 AM, Kevin Wolf wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE in hw/fdc.c
>    

FYI, the subject is duplicated here so the commit log looks a bit funky.

Regards,

Anthony Liguori

> Per https://bugs.launchpad.net/qemu/+bug/424453 the correct values
> for FD_CMD_SAVE is 0x2e and FD_CMD_RESTORE is 0x4e. Verified against
> the Intel 82078 manual which can be found at:
> http://wiki.qemu.org/Documentation/HardwareManuals page 22.
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
> ---
>   hw/fdc.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index b978957..45a876d 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -369,9 +369,9 @@ enum {
>       FD_CMD_PART_ID = 0x18,
>       FD_CMD_SCAN_LOW_OR_EQUAL = 0x19,
>       FD_CMD_SCAN_HIGH_OR_EQUAL = 0x1d,
> -    FD_CMD_SAVE = 0x2c,
> +    FD_CMD_SAVE = 0x2e,
>       FD_CMD_OPTION = 0x33,
> -    FD_CMD_RESTORE = 0x4c,
> +    FD_CMD_RESTORE = 0x4e,
>       FD_CMD_DRIVE_SPECIFICATION_COMMAND = 0x8e,
>       FD_CMD_RELATIVE_SEEK_OUT = 0x8f,
>       FD_CMD_FORMAT_AND_WRITE = 0xcd,
>    

[Qemu-devel] Re: [PATCH 17/19] Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

[Kevin Wolf]

Am 15.06.2010 16:28, schrieb Anthony Liguori:
> On 06/15/2010 09:19 AM, Kevin Wolf wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE in hw/fdc.c
>>    
> 
> FYI, the subject is duplicated here so the commit log looks a bit funky.

I agree. There are some more patches by Jes in master that look the
same, so I didn't complain, but it definitely looks a bit strange.

Kevin

[Qemu-devel] Re: [PATCH 17/19] Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

[Jes Sorensen]

On 06/15/10 16:42, Kevin Wolf wrote:
> Am 15.06.2010 16:28, schrieb Anthony Liguori:
>> On 06/15/2010 09:19 AM, Kevin Wolf wrote:
>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>
>>> Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE in hw/fdc.c
>>>    
>>
>> FYI, the subject is duplicated here so the commit log looks a bit funky.
> 
> I agree. There are some more patches by Jes in master that look the
> same, so I didn't complain, but it definitely looks a bit strange.

Hmmm maybe I am messing something up. I started with strange duplicate
lines because I thought git send-email would cut the first line for the
subject line and then just leave the rest in. Probably just me getting
used to do it all in git instead of using quilt.

Cheers,
Jes

[Qemu-devel] Re: [PATCH 17/19] Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

[Kevin Wolf]

Am 15.06.2010 16:47, schrieb Jes Sorensen:
> On 06/15/10 16:42, Kevin Wolf wrote:
>> Am 15.06.2010 16:28, schrieb Anthony Liguori:
>>> On 06/15/2010 09:19 AM, Kevin Wolf wrote:
>>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>>
>>>> Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE in hw/fdc.c
>>>>    
>>>
>>> FYI, the subject is duplicated here so the commit log looks a bit funky.
>>
>> I agree. There are some more patches by Jes in master that look the
>> same, so I didn't complain, but it definitely looks a bit strange.
> 
> Hmmm maybe I am messing something up. I started with strange duplicate
> lines because I thought git send-email would cut the first line for the
> subject line and then just leave the rest in. Probably just me getting
> used to do it all in git instead of using quilt.

It does take the first line for the subject, but when importing your
mail, git am puts the subject back into the first line, so the whole
text should look the same as in your local repository.

Kevin

[Qemu-devel] Re: [PATCH 17/19] Correct definitions for FD_CMD_SAVE and FD_CMD_RESTORE

[Jes Sorensen]

On 06/15/10 16:52, Kevin Wolf wrote:
> Am 15.06.2010 16:47, schrieb Jes Sorensen:
>> Hmmm maybe I am messing something up. I started with strange duplicate
>> lines because I thought git send-email would cut the first line for the
>> subject line and then just leave the rest in. Probably just me getting
>> used to do it all in git instead of using quilt.
> 
> It does take the first line for the subject, but when importing your
> mail, git am puts the subject back into the first line, so the whole
> text should look the same as in your local repository.

I see, I wasn't aware of that ... well I'll try to keep that in mind
next time :)

Thanks
Jes