qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/17] Block patches
@ 2016-01-20 16:24 Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 01/17] block: Fix .bdrv_open flags Kevin Wolf
                   ` (17 more replies)
  0 siblings, 18 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

The following changes since commit 3db34bf64ab4f8797565dd8750003156c32b301d:

  Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-peter' into staging (2016-01-18 17:40:50 +0000)

are available in the git repository at:


  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to e9b155501da2638e4e319af9960d35da1bc8662b:

  iotests: Test that throttle values ranges (2016-01-20 13:37:57 +0100)

----------------------------------------------------------------
Block layer patches

----------------------------------------------------------------
Christian Borntraeger (1):
      block/raw-posix: avoid bogus fixup for cylinders on DASD disks

Eric Blake (1):
      qemu-iotests: Reduce racy output in 028

Fam Zheng (4):
      qemu-img: Speed up comparing empty/zero images
      vmdk: Create streamOptimized as version 3
      blockdev: Error out on negative throttling option values
      iotests: Test that throttle values ranges

Kevin Wolf (10):
      block: Fix .bdrv_open flags
      qcow2: Write feature table only for v3 images
      qcow2: Write full header on image creation
      block: Assert no write requests under BDRV_O_INCOMING
      block: Fix error path in bdrv_invalidate_cache()
      block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVE
      block: Inactivate BDS when migration completes
      qcow2: Implement .bdrv_inactivate
      qcow2: Fix BDRV_O_INACTIVE handling in qcow2_invalidate_cache()
      qcow2: Make image inaccessible after failed qcow2_invalidate_cache()

Peter Maydell (1):
      block: Clean up includes

 block.c                         |  55 ++++++++++++++++----
 block/accounting.c              |   1 +
 block/archipelago.c             |   2 +-
 block/backup.c                  |   4 +-
 block/blkdebug.c                |   1 +
 block/blkverify.c               |   2 +-
 block/block-backend.c           |   1 +
 block/bochs.c                   |   1 +
 block/cloop.c                   |   1 +
 block/commit.c                  |   1 +
 block/curl.c                    |   1 +
 block/dmg.c                     |   1 +
 block/gluster.c                 |   1 +
 block/io.c                      |   3 ++
 block/iscsi.c                   |   2 +-
 block/linux-aio.c               |   1 +
 block/mirror.c                  |   1 +
 block/nbd-client.c              |   1 +
 block/nbd.c                     |   3 +-
 block/nfs.c                     |   2 +-
 block/null.c                    |   1 +
 block/parallels.c               |   1 +
 block/qapi.c                    |   1 +
 block/qcow.c                    |   1 +
 block/qcow2-cache.c             |   3 +-
 block/qcow2-cluster.c           |   1 +
 block/qcow2-refcount.c          |   1 +
 block/qcow2-snapshot.c          |   1 +
 block/qcow2.c                   | 111 ++++++++++++++++++++++++----------------
 block/qed-check.c               |   1 +
 block/qed-cluster.c             |   1 +
 block/qed-gencb.c               |   1 +
 block/qed-l2-cache.c            |   1 +
 block/qed-table.c               |   1 +
 block/qed.c                     |   5 +-
 block/quorum.c                  |   1 +
 block/raw-posix.c               |  10 +---
 block/raw-win32.c               |   1 +
 block/raw_bsd.c                 |   1 +
 block/rbd.c                     |   2 +-
 block/sheepdog.c                |   1 +
 block/snapshot.c                |   1 +
 block/ssh.c                     |   4 +-
 block/stream.c                  |   1 +
 block/throttle-groups.c         |   1 +
 block/vdi.c                     |   1 +
 block/vhdx-endian.c             |   1 +
 block/vhdx-log.c                |   1 +
 block/vhdx.c                    |   1 +
 block/vmdk.c                    |   9 +++-
 block/vpc.c                     |   1 +
 block/vvfat.c                   |   2 +-
 block/win32-aio.c               |   1 +
 block/write-threshold.c         |   1 +
 blockdev.c                      |   3 +-
 hw/block/block.c                |   1 +
 hw/block/cdrom.c                |   1 +
 hw/block/dataplane/virtio-blk.c |   1 +
 hw/block/ecc.c                  |   1 +
 hw/block/fdc.c                  |   1 +
 hw/block/hd-geometry.c          |   1 +
 hw/block/m25p80.c               |   1 +
 hw/block/nvme.c                 |   1 +
 hw/block/onenand.c              |   1 +
 hw/block/pflash_cfi01.c         |   1 +
 hw/block/pflash_cfi02.c         |   1 +
 hw/block/tc58128.c              |   1 +
 hw/block/virtio-blk.c           |   1 +
 hw/block/xen_disk.c             |  12 +----
 include/block/block.h           |   3 +-
 include/block/block_int.h       |   1 +
 include/qemu/throttle.h         |   2 +
 migration/migration.c           |   7 +++
 nbd/server.c                    |   2 +-
 qemu-img.c                      |  47 +++++++++++------
 qemu-io-cmds.c                  |   1 +
 qemu-io.c                       |   5 +-
 qmp.c                           |  12 +++++
 tests/qemu-iotests/028          |   6 ++-
 tests/qemu-iotests/028.out      |   3 --
 tests/qemu-iotests/031.out      |  17 +++---
 tests/qemu-iotests/036          |   2 +
 tests/qemu-iotests/036.out      |   5 ++
 tests/qemu-iotests/051          |  18 +++++++
 tests/qemu-iotests/051.out      |  39 ++++++++++++++
 tests/qemu-iotests/051.pc.out   |  39 ++++++++++++++
 tests/qemu-iotests/061.out      |  35 +++++++------
 util/throttle.c                 |  16 +++---
 88 files changed, 390 insertions(+), 155 deletions(-)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 01/17] block: Fix .bdrv_open flags
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 02/17] block/raw-posix: avoid bogus fixup for cylinders on DASD disks Kevin Wolf
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

bdrv_common_open() modified bs->open_flags after inferring the set of
options to pass to the driver's .bdrv_open callback. This means that the
cache options were correctly set in bs->open_flags (and therefore
correctly displayed in 'info block'), but the image would actually be
opened with the default cache mode instead.

This patch removes the flags parameter to bdrv_common_open() (except for
BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two
names for the same thing is confusing), and moves the assignment of
open_flags down to immediately before calling into the block drivers. In
all other places, bs->open_flags is now used consistently.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 54c37f9..1a13716 100644
--- a/block.c
+++ b/block.c
@@ -905,7 +905,7 @@ static QemuOptsList bdrv_runtime_opts = {
  * Removes all processed options from *options.
  */
 static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
-                            QDict *options, int flags, Error **errp)
+                            QDict *options, Error **errp)
 {
     int ret, open_flags;
     const char *filename;
@@ -943,7 +943,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         goto fail_opts;
     }
 
-    trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
+    trace_bdrv_open_common(bs, filename ?: "", bs->open_flags,
+                           drv->format_name);
 
     node_name = qemu_opt_get(opts, "node-name");
     bdrv_assign_node_name(bs, node_name, &local_err);
@@ -955,8 +956,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
 
     bs->request_alignment = 512;
     bs->zero_beyond_eof = true;
-    open_flags = bdrv_open_flags(bs, flags);
-    bs->read_only = !(open_flags & BDRV_O_RDWR);
+    bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
         error_setg(errp,
@@ -969,7 +969,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     }
 
     assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
-    if (flags & BDRV_O_COPY_ON_READ) {
+    if (bs->open_flags & BDRV_O_COPY_ON_READ) {
         if (!bs->read_only) {
             bdrv_enable_copy_on_read(bs);
         } else {
@@ -994,6 +994,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
 
     /* Open the image, either directly or using a protocol */
+    open_flags = bdrv_open_flags(bs, bs->open_flags);
     if (drv->bdrv_file_open) {
         assert(file == NULL);
         assert(!drv->bdrv_needs_filename || filename != NULL);
@@ -1656,7 +1657,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
     assert(!(flags & BDRV_O_PROTOCOL) || !file);
 
     /* Open the image */
-    ret = bdrv_open_common(bs, file, options, flags, &local_err);
+    ret = bdrv_open_common(bs, file, options, &local_err);
     if (ret < 0) {
         goto fail;
     }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 02/17] block/raw-posix: avoid bogus fixup for cylinders on DASD disks
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 01/17] block: Fix .bdrv_open flags Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 03/17] qemu-img: Speed up comparing empty/zero images Kevin Wolf
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Christian Borntraeger <borntraeger@de.ibm.com>

large volume DASD that have > 64k cylinders do claim to have
0xFFFE cylinders as special value in the old 16 bit field. We
want to pass this "token" along to the guest, instead of
calculating the real number. Otherwise qemu might fail with
"cyls must be between 1 and 65535"

Cc: qemu-stable@nongnu.org
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw-posix.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 076d070..816bdf7 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -779,7 +779,6 @@ static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
 {
     BDRVRawState *s = bs->opaque;
     struct hd_geometry ioctl_geo = {0};
-    uint32_t blksize;
 
     /* If DASD, get its geometry */
     if (check_for_dasd(s->fd) < 0) {
@@ -799,12 +798,6 @@ static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
     }
     geo->heads = ioctl_geo.heads;
     geo->sectors = ioctl_geo.sectors;
-    if (!probe_physical_blocksize(s->fd, &blksize)) {
-        /* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
-        geo->cylinders = bdrv_nb_sectors(bs) / (blksize / BDRV_SECTOR_SIZE)
-                                             / (geo->heads * geo->sectors);
-        return 0;
-    }
     geo->cylinders = ioctl_geo.cylinders;
 
     return 0;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 03/17] qemu-img: Speed up comparing empty/zero images
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 01/17] block: Fix .bdrv_open flags Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 02/17] block/raw-posix: avoid bogus fixup for cylinders on DASD disks Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 04/17] qemu-iotests: Reduce racy output in 028 Kevin Wolf
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Fam Zheng <famz@redhat.com>

Two empty raw files are always compared by actually reading data even if
there is no data, because BDRV_BLOCK_ZERO is considered "allocated" in
bdrv_is_allocated_above().  That is inefficient.

Use bdrv_get_block_status_above() for more information, and skip the
consecutive zero sectors.

This brings a huge speed up in comparing sparse/empty raw images:

    $ qemu-img create a 1G

    $ time ~/build/master/bin/qemu-img compare a a
    Images are identical.

    real    0m6.583s
    user    0m0.191s
    sys     0m6.367s

    $ time qemu-img compare a a
    Images are identical.

    real    0m0.033s
    user    0m0.003s
    sys     0m0.031s

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index a5949e6..6773943 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1071,28 +1071,50 @@ static int img_compare(int argc, char **argv)
     }
 
     for (;;) {
+        int64_t status1, status2;
         nb_sectors = sectors_to_process(total_sectors, sector_num);
         if (nb_sectors <= 0) {
             break;
         }
-        allocated1 = bdrv_is_allocated_above(bs1, NULL, sector_num, nb_sectors,
-                                             &pnum1);
-        if (allocated1 < 0) {
+        status1 = bdrv_get_block_status_above(bs1, NULL, sector_num,
+                                              total_sectors1 - sector_num,
+                                              &pnum1);
+        if (status1 < 0) {
             ret = 3;
             error_report("Sector allocation test failed for %s", filename1);
             goto out;
         }
+        allocated1 = status1 & BDRV_BLOCK_ALLOCATED;
 
-        allocated2 = bdrv_is_allocated_above(bs2, NULL, sector_num, nb_sectors,
-                                             &pnum2);
-        if (allocated2 < 0) {
+        status2 = bdrv_get_block_status_above(bs2, NULL, sector_num,
+                                              total_sectors2 - sector_num,
+                                              &pnum2);
+        if (status2 < 0) {
             ret = 3;
             error_report("Sector allocation test failed for %s", filename2);
             goto out;
         }
-        nb_sectors = MIN(pnum1, pnum2);
+        allocated2 = status2 & BDRV_BLOCK_ALLOCATED;
+        if (pnum1) {
+            nb_sectors = MIN(nb_sectors, pnum1);
+        }
+        if (pnum2) {
+            nb_sectors = MIN(nb_sectors, pnum2);
+        }
 
-        if (allocated1 == allocated2) {
+        if (strict) {
+            if ((status1 & ~BDRV_BLOCK_OFFSET_MASK) !=
+                (status2 & ~BDRV_BLOCK_OFFSET_MASK)) {
+                ret = 1;
+                qprintf(quiet, "Strict mode: Offset %" PRId64
+                        " block status mismatch!\n",
+                        sectors_to_bytes(sector_num));
+                goto out;
+            }
+        }
+        if ((status1 & BDRV_BLOCK_ZERO) && (status2 & BDRV_BLOCK_ZERO)) {
+            nb_sectors = MIN(pnum1, pnum2);
+        } else if (allocated1 == allocated2) {
             if (allocated1) {
                 ret = blk_read(blk1, sector_num, buf1, nb_sectors);
                 if (ret < 0) {
@@ -1120,13 +1142,6 @@ static int img_compare(int argc, char **argv)
                 }
             }
         } else {
-            if (strict) {
-                ret = 1;
-                qprintf(quiet, "Strict mode: Offset %" PRId64
-                        " allocation mismatch!\n",
-                        sectors_to_bytes(sector_num));
-                goto out;
-            }
 
             if (allocated1) {
                 ret = check_empty_sectors(blk1, sector_num, nb_sectors,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 04/17] qemu-iotests: Reduce racy output in 028
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (2 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 03/17] qemu-img: Speed up comparing empty/zero images Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 05/17] block: Clean up includes Kevin Wolf
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Eric Blake <eblake@redhat.com>

On my machine, './check -qcow2 028' was failing about 80% of the
time, due to a race in how many times the repeated attempts
to run 'info block-jobs' could occur before the job was done,
showing up as a failure of fewer '(qemu) ' prompts than in the
expected output.  Silence the output during the repetitions, then
add a final clean command to keep the expected output useful;
once patched, I was finally able to run the test 20 times in a
row with no failures.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/028     | 6 ++++--
 tests/qemu-iotests/028.out | 3 ---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028
index 009510d..4909b9b 100755
--- a/tests/qemu-iotests/028
+++ b/tests/qemu-iotests/028
@@ -114,10 +114,12 @@ h=$QEMU_HANDLE
 QEMU_COMM_TIMEOUT=1
 
 # Silence output since it contains the disk image path and QEMU's readline
-# character echoing makes it very hard to filter the output
+# character echoing makes it very hard to filter the output. Plus, there
+# is no telling how many times the command will repeat before succeeding.
 _send_qemu_cmd $h "drive_backup disk ${TEST_IMG}.copy" "(qemu)" >/dev/null
 _send_qemu_cmd $h "" "Formatting" | _filter_img_create
-qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs"
+qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs" >/dev/null
+_send_qemu_cmd $h "info block-jobs" "No active jobs"
 _send_qemu_cmd $h 'quit' ""
 
 # Base image sectors
diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out
index 279029d..acd2870 100644
--- a/tests/qemu-iotests/028.out
+++ b/tests/qemu-iotests/028.out
@@ -469,10 +469,7 @@ No errors were found on the image.
 block-backup
 
 Formatting 'TEST_DIR/t.IMGFMT.copy', fmt=IMGFMT size=4294968832 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
-(qemu)
 (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-j^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-jo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-job^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-jobs^[[K
-Type backup, device disk: Completed 0 of 4294968832 bytes, speed limit 0 bytes/s
-i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-j^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-jo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-job^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block-jobs^[[K
 No active jobs
 === IO: pattern 195
 read 512/512 bytes at offset 3221194240
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 05/17] block: Clean up includes
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (3 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 04/17] qemu-iotests: Reduce racy output in 028 Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 06/17] qcow2: Write feature table only for v3 images Kevin Wolf
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Peter Maydell <peter.maydell@linaro.org>

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/accounting.c              |  1 +
 block/archipelago.c             |  2 +-
 block/backup.c                  |  4 +---
 block/blkdebug.c                |  1 +
 block/blkverify.c               |  2 +-
 block/block-backend.c           |  1 +
 block/bochs.c                   |  1 +
 block/cloop.c                   |  1 +
 block/commit.c                  |  1 +
 block/curl.c                    |  1 +
 block/dmg.c                     |  1 +
 block/gluster.c                 |  1 +
 block/io.c                      |  1 +
 block/iscsi.c                   |  2 +-
 block/linux-aio.c               |  1 +
 block/mirror.c                  |  1 +
 block/nbd-client.c              |  1 +
 block/nbd.c                     |  3 +--
 block/nfs.c                     |  2 +-
 block/null.c                    |  1 +
 block/parallels.c               |  1 +
 block/qapi.c                    |  1 +
 block/qcow.c                    |  1 +
 block/qcow2-cache.c             |  3 +--
 block/qcow2-cluster.c           |  1 +
 block/qcow2-refcount.c          |  1 +
 block/qcow2-snapshot.c          |  1 +
 block/qcow2.c                   |  1 +
 block/qed-check.c               |  1 +
 block/qed-cluster.c             |  1 +
 block/qed-gencb.c               |  1 +
 block/qed-l2-cache.c            |  1 +
 block/qed-table.c               |  1 +
 block/qed.c                     |  1 +
 block/quorum.c                  |  1 +
 block/raw-posix.c               |  3 +--
 block/raw-win32.c               |  1 +
 block/raw_bsd.c                 |  1 +
 block/rbd.c                     |  2 +-
 block/sheepdog.c                |  1 +
 block/snapshot.c                |  1 +
 block/ssh.c                     |  4 +---
 block/stream.c                  |  1 +
 block/throttle-groups.c         |  1 +
 block/vdi.c                     |  1 +
 block/vhdx-endian.c             |  1 +
 block/vhdx-log.c                |  1 +
 block/vhdx.c                    |  1 +
 block/vmdk.c                    |  1 +
 block/vpc.c                     |  1 +
 block/vvfat.c                   |  2 +-
 block/win32-aio.c               |  1 +
 block/write-threshold.c         |  1 +
 hw/block/block.c                |  1 +
 hw/block/cdrom.c                |  1 +
 hw/block/dataplane/virtio-blk.c |  1 +
 hw/block/ecc.c                  |  1 +
 hw/block/fdc.c                  |  1 +
 hw/block/hd-geometry.c          |  1 +
 hw/block/m25p80.c               |  1 +
 hw/block/nvme.c                 |  1 +
 hw/block/onenand.c              |  1 +
 hw/block/pflash_cfi01.c         |  1 +
 hw/block/pflash_cfi02.c         |  1 +
 hw/block/tc58128.c              |  1 +
 hw/block/virtio-blk.c           |  1 +
 hw/block/xen_disk.c             | 12 +-----------
 qemu-img.c                      |  2 +-
 qemu-io-cmds.c                  |  1 +
 qemu-io.c                       |  5 +----
 70 files changed, 70 insertions(+), 34 deletions(-)

diff --git a/block/accounting.c b/block/accounting.c
index 185025e..3f457c4 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -23,6 +23,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "block/accounting.h"
 #include "block/block_int.h"
 #include "qemu/timer.h"
diff --git a/block/archipelago.c b/block/archipelago.c
index 855655c..0507589 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -50,6 +50,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/error-report.h"
@@ -59,7 +60,6 @@
 #include "qapi/qmp/qjson.h"
 #include "qemu/atomic.h"
 
-#include <inttypes.h>
 #include <xseg/xseg.h>
 #include <xseg/protocol.h>
 
diff --git a/block/backup.c b/block/backup.c
index 705bb77..00cafdb 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -11,9 +11,7 @@
  *
  */
 
-#include <stdio.h>
-#include <errno.h>
-#include <unistd.h>
+#include "qemu/osdep.h"
 
 #include "trace.h"
 #include "block/block.h"
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 86b143d..f85c54b 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/config-file.h"
 #include "block/block_int.h"
diff --git a/block/blkverify.c b/block/blkverify.c
index 1d75449..2a885cc 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -7,7 +7,7 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include <stdarg.h>
+#include "qemu/osdep.h"
 #include "qemu/sockets.h" /* for EINPROGRESS on Windows */
 #include "block/block_int.h"
 #include "qapi/qmp/qdict.h"
diff --git a/block/block-backend.c b/block/block-backend.c
index e813759..efd6146 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -10,6 +10,7 @@
  * or later.  See the COPYING.LIB file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include "sysemu/block-backend.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
diff --git a/block/bochs.c b/block/bochs.c
index 18949b9..8b953bb 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -22,6 +22,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/cloop.c b/block/cloop.c
index 4190ae0..41bdee8 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/commit.c b/block/commit.c
index a5d02aa..446a3ae 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
diff --git a/block/curl.c b/block/curl.c
index 8994182..1507e0a 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "block/block_int.h"
diff --git a/block/dmg.c b/block/dmg.c
index 546a6f5..1018fd1 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/bswap.h"
diff --git a/block/gluster.c b/block/gluster.c
index 0857c14..65077a0 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -7,6 +7,7 @@
  * See the COPYING file in the top-level directory.
  *
  */
+#include "qemu/osdep.h"
 #include <glusterfs/api/glfs.h>
 #include "block/block_int.h"
 #include "qemu/uri.h"
diff --git a/block/io.c b/block/io.c
index 63e3678..707c04b 100644
--- a/block/io.c
+++ b/block/io.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "sysemu/block-backend.h"
 #include "block/blockjob.h"
diff --git a/block/iscsi.c b/block/iscsi.c
index 3acb052..bffd707 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -23,7 +23,7 @@
  * THE SOFTWARE.
  */
 
-#include "config-host.h"
+#include "qemu/osdep.h"
 
 #include <poll.h>
 #include <math.h>
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 88b0520..805757e 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -7,6 +7,7 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/aio.h"
 #include "qemu/queue.h"
diff --git a/block/mirror.c b/block/mirror.c
index f201f2b..e9e151c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "block/blockjob.h"
 #include "block/block_int.h"
diff --git a/block/nbd-client.c b/block/nbd-client.c
index b7fd17a..568c56c 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -26,6 +26,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "nbd-client.h"
 #include "qemu/sockets.h"
 
diff --git a/block/nbd.c b/block/nbd.c
index 416f42b..1a90bc7 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -26,6 +26,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "block/nbd-client.h"
 #include "qemu/uri.h"
 #include "block/block_int.h"
@@ -36,8 +37,6 @@
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"
 
-#include <sys/types.h>
-#include <unistd.h>
 
 #define EN_OPTSTR ":exportname="
 
diff --git a/block/nfs.c b/block/nfs.c
index fd79f89..5eb8c13 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 
-#include "config-host.h"
+#include "qemu/osdep.h"
 
 #include <poll.h>
 #include "qemu-common.h"
diff --git a/block/null.c b/block/null.c
index 7d08323..d90165d 100644
--- a/block/null.c
+++ b/block/null.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include "block/block_int.h"
 
 #define NULL_OPT_LATENCY "latency-ns"
diff --git a/block/parallels.c b/block/parallels.c
index e4a56a5..ee39081 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -27,6 +27,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/qapi.c b/block/qapi.c
index 58d3975..a49c118 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "block/qapi.h"
 #include "block/block_int.h"
 #include "block/throttle-groups.h"
diff --git a/block/qcow.c b/block/qcow.c
index 635085e..afed18f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 86dd7f2..0fe8eda 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -23,7 +23,7 @@
  */
 
 /* Needed for CONFIG_MADVISE */
-#include "config-host.h"
+#include "qemu/osdep.h"
 
 #if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE)
 #include <sys/mman.h>
@@ -31,7 +31,6 @@
 
 #include "block/block_int.h"
 #include "qemu-common.h"
-#include "qemu/osdep.h"
 #include "qcow2.h"
 #include "trace.h"
 
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 34112c3..3e887e9 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include <zlib.h>
 
 #include "qemu-common.h"
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index af493f8..52a0a9f 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "block/qcow2.h"
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index def7201..13f88d1 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "block/qcow2.h"
diff --git a/block/qcow2.c b/block/qcow2.c
index d992e7f..e4e2754 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/qed-check.c b/block/qed-check.c
index 36ecd29..622f308 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qed.h"
 
 typedef struct {
diff --git a/block/qed-cluster.c b/block/qed-cluster.c
index f64b2af..c24e756 100644
--- a/block/qed-cluster.c
+++ b/block/qed-cluster.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qed.h"
 
 /**
diff --git a/block/qed-gencb.c b/block/qed-gencb.c
index b817a8b..faf8ecc 100644
--- a/block/qed-gencb.c
+++ b/block/qed-gencb.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qed.h"
 
 void *gencb_alloc(size_t len, BlockCompletionFunc *cb, void *opaque)
diff --git a/block/qed-l2-cache.c b/block/qed-l2-cache.c
index e9b2aae..5cba794 100644
--- a/block/qed-l2-cache.c
+++ b/block/qed-l2-cache.c
@@ -50,6 +50,7 @@
  * table will be deleted in favor of the existing cache entry.
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "qed.h"
 
diff --git a/block/qed-table.c b/block/qed-table.c
index f4219b8..802945f 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "qemu/sockets.h" /* for EINPROGRESS on Windows */
 #include "qed.h"
diff --git a/block/qed.c b/block/qed.c
index 31f4cc9..093d6e5 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qemu/timer.h"
 #include "trace.h"
 #include "qed.h"
diff --git a/block/quorum.c b/block/quorum.c
index 6793f12..a5ae4b8 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -13,6 +13,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include "block/block_int.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 816bdf7..6df3067 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
@@ -51,8 +52,6 @@
 #include <sys/dkio.h>
 #endif
 #ifdef __linux__
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <sys/ioctl.h>
 #include <sys/param.h>
 #include <linux/cdrom.h>
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 2d0907a..21a6cb8 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/timer.h"
 #include "block/block_int.h"
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 915d6fd..bcaee11 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -26,6 +26,7 @@
  * IN THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "block/block_int.h"
 #include "qemu/option.h"
 
diff --git a/block/rbd.c b/block/rbd.c
index a60a19d..51b64f3 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -11,7 +11,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include <inttypes.h>
+#include "qemu/osdep.h"
 
 #include "qemu-common.h"
 #include "qemu/error-report.h"
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6986be8..ff89298 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -12,6 +12,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/uri.h"
 #include "qemu/error-report.h"
diff --git a/block/snapshot.c b/block/snapshot.c
index 2d86b88..17a27b5 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "block/snapshot.h"
 #include "block/block_int.h"
 #include "qapi/qmp/qerror.h"
diff --git a/block/ssh.c b/block/ssh.c
index af025c0..04deeba 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -22,9 +22,7 @@
  * THE SOFTWARE.
  */
 
-#include <stdio.h>
-#include <stdlib.h>
-#include <stdarg.h>
+#include "qemu/osdep.h"
 
 #include <libssh2.h>
 #include <libssh2_sftp.h>
diff --git a/block/stream.c b/block/stream.c
index 25af7ef..cafaa07 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 13b5baa..4920e09 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -22,6 +22,7 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "block/throttle-groups.h"
 #include "qemu/queue.h"
 #include "qemu/thread.h"
diff --git a/block/vdi.c b/block/vdi.c
index 17f435f..61bcd54 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -49,6 +49,7 @@
  * so this seems to be reasonable.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/vhdx-endian.c b/block/vhdx-endian.c
index 0640d3f..da33cd3 100644
--- a/block/vhdx-endian.c
+++ b/block/vhdx-endian.c
@@ -15,6 +15,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "block/vhdx.h"
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index ab86416..3690761 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -17,6 +17,7 @@
  * See the COPYING.LIB file in the top-level directory.
  *
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/error-report.h"
diff --git a/block/vhdx.c b/block/vhdx.c
index 2fe9a5e..72042e9 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -15,6 +15,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/vmdk.c b/block/vmdk.c
index 2b5cb00..6b8596c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -23,6 +23,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qapi/qmp/qerror.h"
diff --git a/block/vpc.c b/block/vpc.c
index 299d373..d852f96 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -22,6 +22,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
diff --git a/block/vvfat.c b/block/vvfat.c
index b184eca..2ea5a4a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -22,7 +22,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-#include <sys/stat.h>
+#include "qemu/osdep.h"
 #include <dirent.h>
 #include "qemu-common.h"
 #include "block/block_int.h"
diff --git a/block/win32-aio.c b/block/win32-aio.c
index bbf2f01..2d509a9 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/timer.h"
 #include "block/block_int.h"
diff --git a/block/write-threshold.c b/block/write-threshold.c
index 0fe3891..cc2ca71 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -10,6 +10,7 @@
  * See the COPYING.LIB file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include "block/block_int.h"
 #include "qemu/coroutine.h"
 #include "block/write-threshold.h"
diff --git a/hw/block/block.c b/hw/block/block.c
index f7243e5..960df2b 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -7,6 +7,7 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/block-backend.h"
 #include "hw/block/block.h"
diff --git a/hw/block/cdrom.c b/hw/block/cdrom.c
index 4e1019c..da937fe 100644
--- a/hw/block/cdrom.c
+++ b/hw/block/cdrom.c
@@ -25,6 +25,7 @@
 /* ??? Most of the ATAPI emulation is still in ide.c.  It should be moved
    here.  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "hw/scsi/scsi.h"
 
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index b8ce6cd..bc34046 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "trace.h"
 #include "qemu/iov.h"
 #include "qemu/thread.h"
diff --git a/hw/block/ecc.c b/hw/block/ecc.c
index 10bb233..48311d2 100644
--- a/hw/block/ecc.c
+++ b/hw/block/ecc.c
@@ -11,6 +11,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/block/flash.h"
 
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 858f5f7..6711c6a 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -27,6 +27,7 @@
  * way. There are changes in DOR register and DMA is not available.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/block/fdc.h"
 #include "qemu/error-report.h"
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index b187878..6d02192 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -30,6 +30,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "sysemu/block-backend.h"
 #include "hw/block/block.h"
 #include "trace.h"
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index efc43dd..4bbf90d 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -21,6 +21,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 169e4fa..a5fedb2 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -20,6 +20,7 @@
  *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>
  */
 
+#include "qemu/osdep.h"
 #include <hw/block/block.h>
 #include <hw/hw.h>
 #include <hw/pci/msix.h>
diff --git a/hw/block/onenand.c b/hw/block/onenand.c
index 58eff50..9189685 100644
--- a/hw/block/onenand.c
+++ b/hw/block/onenand.c
@@ -18,6 +18,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "hw/hw.h"
 #include "hw/block/flash.h"
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 2ba6c77..a4c4fa1 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -36,6 +36,7 @@
  * It does not implement much more ...
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/block/flash.h"
 #include "sysemu/block-backend.h"
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 074a005..aaa697a 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -35,6 +35,7 @@
  * It does not implement multiple sectors erase
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/block/flash.h"
 #include "qemu/timer.h"
diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c
index 728f1c3..7909d50 100644
--- a/hw/block/tc58128.c
+++ b/hw/block/tc58128.c
@@ -1,3 +1,4 @@
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/sh4/sh.h"
 #include "hw/loader.h"
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 51f867b..11bedff 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/iov.h"
 #include "qemu/error-report.h"
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index a48e726..571f651 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -19,18 +19,8 @@
  *  GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include <stdio.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <string.h>
-#include <unistd.h>
-#include <inttypes.h>
-#include <time.h>
-#include <fcntl.h>
-#include <errno.h>
+#include "qemu/osdep.h"
 #include <sys/ioctl.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <sys/mman.h>
 #include <sys/uio.h>
 
diff --git a/qemu-img.c b/qemu-img.c
index 6773943..33e451c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "qapi-visit.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/qmp/qerror.h"
@@ -28,7 +29,6 @@
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
-#include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/block-backend.h"
 #include "block/block_int.h"
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 18fc2bd..e929d24 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -8,6 +8,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include "qemu-io.h"
 #include "sysemu/block-backend.h"
 #include "block/block.h"
diff --git a/qemu-io.c b/qemu-io.c
index d47228a..d593f19 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -7,10 +7,7 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
-#include <sys/time.h>
-#include <sys/types.h>
-#include <stdarg.h>
-#include <stdio.h>
+#include "qemu/osdep.h"
 #include <getopt.h>
 #include <libgen.h>
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 06/17] qcow2: Write feature table only for v3 images
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (4 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 05/17] block: Clean up includes Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 07/17] qcow2: Write full header on image creation Kevin Wolf
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

Version 2 images don't have feature bits, so writing a feature table to
those images is kind of pointless.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c              | 48 ++++++++++++++++++++++++----------------------
 tests/qemu-iotests/031.out | 12 +-----------
 tests/qemu-iotests/061.out | 15 ---------------
 3 files changed, 26 insertions(+), 49 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index e4e2754..20e4057 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1895,31 +1895,33 @@ int qcow2_update_header(BlockDriverState *bs)
     }
 
     /* Feature table */
-    Qcow2Feature features[] = {
-        {
-            .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
-            .bit  = QCOW2_INCOMPAT_DIRTY_BITNR,
-            .name = "dirty bit",
-        },
-        {
-            .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
-            .bit  = QCOW2_INCOMPAT_CORRUPT_BITNR,
-            .name = "corrupt bit",
-        },
-        {
-            .type = QCOW2_FEAT_TYPE_COMPATIBLE,
-            .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
-            .name = "lazy refcounts",
-        },
-    };
+    if (s->qcow_version >= 3) {
+        Qcow2Feature features[] = {
+            {
+                .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
+                .bit  = QCOW2_INCOMPAT_DIRTY_BITNR,
+                .name = "dirty bit",
+            },
+            {
+                .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
+                .bit  = QCOW2_INCOMPAT_CORRUPT_BITNR,
+                .name = "corrupt bit",
+            },
+            {
+                .type = QCOW2_FEAT_TYPE_COMPATIBLE,
+                .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
+                .name = "lazy refcounts",
+            },
+        };
 
-    ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE,
-                         features, sizeof(features), buflen);
-    if (ret < 0) {
-        goto fail;
+        ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE,
+                             features, sizeof(features), buflen);
+        if (ret < 0) {
+            goto fail;
+        }
+        buf += ret;
+        buflen -= ret;
     }
-    buf += ret;
-    buflen -= ret;
 
     /* Keep unknown header extensions */
     QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
index fce3ce0..f065404 100644
--- a/tests/qemu-iotests/031.out
+++ b/tests/qemu-iotests/031.out
@@ -53,11 +53,6 @@ refcount_order            4
 header_length             72
 
 Header extension:
-magic                     0x6803f857
-length                    144
-data                      <binary>
-
-Header extension:
 magic                     0x12345678
 length                    31
 data                      'This is a test header extension'
@@ -68,7 +63,7 @@ No errors were found on the image.
 
 magic                     0x514649fb
 version                   2
-backing_file_offset       0x128
+backing_file_offset       0x90
 backing_file_size         0x17
 cluster_bits              16
 size                      67108864
@@ -91,11 +86,6 @@ length                    11
 data                      'host_device'
 
 Header extension:
-magic                     0x6803f857
-length                    144
-data                      <binary>
-
-Header extension:
 magic                     0x12345678
 length                    31
 data                      'This is a test header extension'
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 57aae28..d604682 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -43,11 +43,6 @@ autoclear_features        0x0
 refcount_order            4
 header_length             72
 
-Header extension:
-magic                     0x6803f857
-length                    144
-data                      <binary>
-
 read 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 No errors were found on the image.
@@ -105,11 +100,6 @@ autoclear_features        0x0
 refcount_order            4
 header_length             72
 
-Header extension:
-magic                     0x6803f857
-length                    144
-data                      <binary>
-
 read 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 No errors were found on the image.
@@ -155,11 +145,6 @@ autoclear_features        0x0
 refcount_order            4
 header_length             72
 
-Header extension:
-magic                     0x6803f857
-length                    144
-data                      <binary>
-
 No errors were found on the image.
 
 === Testing version upgrade and resize ===
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 07/17] qcow2: Write full header on image creation
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (5 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 06/17] qcow2: Write feature table only for v3 images Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING Kevin Wolf
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

When creating a qcow2 image, we didn't necessarily call
qcow2_update_header(), but could end up with the basic header that
qcow2_create2() created manually. One thing that this basic header
lacks is the feature table. Let's make sure that it's always present.

This requires a few updates to test cases as well.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c              |  7 +++++++
 tests/qemu-iotests/031.out |  5 +++++
 tests/qemu-iotests/036     |  2 ++
 tests/qemu-iotests/036.out |  5 +++++
 tests/qemu-iotests/061.out | 20 ++++++++++++++++++++
 5 files changed, 39 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 20e4057..037afbf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2239,6 +2239,13 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         abort();
     }
 
+    /* Create a full header (including things like feature table) */
+    ret = qcow2_update_header(bs);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not update qcow2 header");
+        goto out;
+    }
+
     /* Okay, now that we have a valid image, let's give it the right size */
     ret = bdrv_truncate(bs, total_size);
     if (ret < 0) {
diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
index f065404..7f5050b 100644
--- a/tests/qemu-iotests/031.out
+++ b/tests/qemu-iotests/031.out
@@ -116,6 +116,11 @@ refcount_order            4
 header_length             104
 
 Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
+Header extension:
 magic                     0x12345678
 length                    31
 data                      'This is a test header extension'
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index 392f1ef..c4cc91b 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -57,6 +57,7 @@ _make_test_img 64M
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63
 
 # Without feature table
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
 $PYTHON qcow2.py "$TEST_IMG" dump-header
 _img_info
 
@@ -73,6 +74,7 @@ $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 62
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63
 
 # Without feature table
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
 _img_info
 
 # With feature table containing bit 63
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index 5616e37..f443635 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -56,6 +56,11 @@ autoclear_features        0x8000000000000000
 refcount_order            4
 header_length             104
 
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
 
 === Repair image ===
 
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index d604682..a03732e 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -24,6 +24,11 @@ autoclear_features        0x0
 refcount_order            4
 header_length             104
 
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
 magic                     0x514649fb
 version                   2
 backing_file_offset       0x0
@@ -76,6 +81,11 @@ autoclear_features        0x0
 refcount_order            4
 header_length             104
 
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
 ERROR cluster 5 refcount=0 reference=1
 ERROR cluster 6 refcount=0 reference=1
 Rebuilding refcount structure
@@ -126,6 +136,11 @@ autoclear_features        0x40000000000
 refcount_order            4
 header_length             104
 
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
 magic                     0x514649fb
 version                   2
 backing_file_offset       0x0
@@ -228,6 +243,11 @@ autoclear_features        0x0
 refcount_order            4
 header_length             104
 
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
 ERROR cluster 5 refcount=0 reference=1
 ERROR cluster 6 refcount=0 reference=1
 Rebuilding refcount structure
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (6 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 07/17] qcow2: Write full header on image creation Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-02-18 16:03   ` Laurent Vivier
  2016-01-20 16:24 ` [Qemu-devel] [PULL 09/17] block: Fix error path in bdrv_invalidate_cache() Kevin Wolf
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

As long as BDRV_O_INCOMING is set, the image file is only opened so we
have a file descriptor for it. We're definitely not supposed to modify
the image, it's still owned by the migration source.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/io.c b/block/io.c
index 707c04b..2372994 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1301,6 +1301,7 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
     if (bs->read_only) {
         return -EPERM;
     }
+    assert(!(bs->open_flags & BDRV_O_INCOMING));
 
     ret = bdrv_check_byte_request(bs, offset, bytes);
     if (ret < 0) {
@@ -2462,6 +2463,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
     } else if (bs->read_only) {
         return -EPERM;
     }
+    assert(!(bs->open_flags & BDRV_O_INCOMING));
 
     /* Do nothing if disabled.  */
     if (!(bs->open_flags & BDRV_O_UNMAP)) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 09/17] block: Fix error path in bdrv_invalidate_cache()
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (7 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:24 ` [Qemu-devel] [PULL 10/17] block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVE Kevin Wolf
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

We can only clear BDRV_O_INCOMING if the caches were actually
invalidated.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block.c b/block.c
index 1a13716..6ac3191 100644
--- a/block.c
+++ b/block.c
@@ -3272,12 +3272,14 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
         bdrv_invalidate_cache(bs->file->bs, &local_err);
     }
     if (local_err) {
+        bs->open_flags |= BDRV_O_INCOMING;
         error_propagate(errp, local_err);
         return;
     }
 
     ret = refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
+        bs->open_flags |= BDRV_O_INCOMING;
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
         return;
     }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 10/17] block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVE
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (8 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 09/17] block: Fix error path in bdrv_invalidate_cache() Kevin Wolf
@ 2016-01-20 16:24 ` Kevin Wolf
  2016-01-20 16:25 ` [Qemu-devel] [PULL 11/17] block: Inactivate BDS when migration completes Kevin Wolf
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

Instead of covering only the state of images on the migration
destination before the migration is completed, the flag will also cover
the state of images on the migration source after completion. This
common state implies that the image is technically still open, but no
writes will happen and any cached contents will be reloaded from disk if
and when the image leaves this state.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c               | 10 +++++-----
 block/io.c            |  4 ++--
 block/qcow2.c         |  6 +++---
 block/qed.c           |  4 ++--
 include/block/block.h |  2 +-
 nbd/server.c          |  2 +-
 6 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index 6ac3191..95b2967 100644
--- a/block.c
+++ b/block.c
@@ -1191,7 +1191,7 @@ static int bdrv_fill_options(QDict **options, const char *filename,
     }
 
     if (runstate_check(RUN_STATE_INMIGRATE)) {
-        *flags |= BDRV_O_INCOMING;
+        *flags |= BDRV_O_INACTIVE;
     }
 
     return 0;
@@ -3261,10 +3261,10 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
         return;
     }
 
-    if (!(bs->open_flags & BDRV_O_INCOMING)) {
+    if (!(bs->open_flags & BDRV_O_INACTIVE)) {
         return;
     }
-    bs->open_flags &= ~BDRV_O_INCOMING;
+    bs->open_flags &= ~BDRV_O_INACTIVE;
 
     if (bs->drv->bdrv_invalidate_cache) {
         bs->drv->bdrv_invalidate_cache(bs, &local_err);
@@ -3272,14 +3272,14 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
         bdrv_invalidate_cache(bs->file->bs, &local_err);
     }
     if (local_err) {
-        bs->open_flags |= BDRV_O_INCOMING;
+        bs->open_flags |= BDRV_O_INACTIVE;
         error_propagate(errp, local_err);
         return;
     }
 
     ret = refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
-        bs->open_flags |= BDRV_O_INCOMING;
+        bs->open_flags |= BDRV_O_INACTIVE;
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
         return;
     }
diff --git a/block/io.c b/block/io.c
index 2372994..5bb353a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1301,7 +1301,7 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
     if (bs->read_only) {
         return -EPERM;
     }
-    assert(!(bs->open_flags & BDRV_O_INCOMING));
+    assert(!(bs->open_flags & BDRV_O_INACTIVE));
 
     ret = bdrv_check_byte_request(bs, offset, bytes);
     if (ret < 0) {
@@ -2463,7 +2463,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
     } else if (bs->read_only) {
         return -EPERM;
     }
-    assert(!(bs->open_flags & BDRV_O_INCOMING));
+    assert(!(bs->open_flags & BDRV_O_INACTIVE));
 
     /* Do nothing if disabled.  */
     if (!(bs->open_flags & BDRV_O_UNMAP)) {
diff --git a/block/qcow2.c b/block/qcow2.c
index 037afbf..340ae8f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1141,7 +1141,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Clear unknown autoclear feature bits */
-    if (!bs->read_only && !(flags & BDRV_O_INCOMING) && s->autoclear_features) {
+    if (!bs->read_only && !(flags & BDRV_O_INACTIVE) && s->autoclear_features) {
         s->autoclear_features = 0;
         ret = qcow2_update_header(bs);
         if (ret < 0) {
@@ -1154,7 +1154,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     qemu_co_mutex_init(&s->lock);
 
     /* Repair image if dirty */
-    if (!(flags & (BDRV_O_CHECK | BDRV_O_INCOMING)) && !bs->read_only &&
+    if (!(flags & (BDRV_O_CHECK | BDRV_O_INACTIVE)) && !bs->read_only &&
         (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) {
         BdrvCheckResult result = {0};
 
@@ -1693,7 +1693,7 @@ static void qcow2_close(BlockDriverState *bs)
     /* else pre-write overlap checks in cache_destroy may crash */
     s->l1_table = NULL;
 
-    if (!(bs->open_flags & BDRV_O_INCOMING)) {
+    if (!(bs->open_flags & BDRV_O_INACTIVE)) {
         int ret1, ret2;
 
         ret1 = qcow2_cache_flush(bs, s->l2_table_cache);
diff --git a/block/qed.c b/block/qed.c
index 093d6e5..0c870cd 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -478,7 +478,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
      * feature is no longer valid.
      */
     if ((s->header.autoclear_features & ~QED_AUTOCLEAR_FEATURE_MASK) != 0 &&
-        !bdrv_is_read_only(bs->file->bs) && !(flags & BDRV_O_INCOMING)) {
+        !bdrv_is_read_only(bs->file->bs) && !(flags & BDRV_O_INACTIVE)) {
         s->header.autoclear_features &= QED_AUTOCLEAR_FEATURE_MASK;
 
         ret = qed_write_header_sync(s);
@@ -506,7 +506,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
          * aid data recovery from an otherwise inconsistent image.
          */
         if (!bdrv_is_read_only(bs->file->bs) &&
-            !(flags & BDRV_O_INCOMING)) {
+            !(flags & BDRV_O_INACTIVE)) {
             BdrvCheckResult result = {0};
 
             ret = qed_check(s, &result, true);
diff --git a/include/block/block.h b/include/block/block.h
index c96923d..2b7d33c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -84,7 +84,7 @@ typedef struct HDGeometry {
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
 #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
 #define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
-#define BDRV_O_INCOMING    0x0800  /* consistency hint for incoming migration */
+#define BDRV_O_INACTIVE    0x0800  /* consistency hint for migration handoff */
 #define BDRV_O_CHECK       0x1000  /* open solely for consistency check */
 #define BDRV_O_ALLOW_RDWR  0x2000  /* allow reopen to change from r/o to r/w */
 #define BDRV_O_UNMAP       0x4000  /* execute guest UNMAP/TRIM operations */
diff --git a/nbd/server.c b/nbd/server.c
index eead339..2265cb0 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -668,7 +668,7 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
     blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
     /*
      * NBD exports are used for non-shared storage migration.  Make sure
-     * that BDRV_O_INCOMING is cleared and the image is ready for write
+     * that BDRV_O_INACTIVE is cleared and the image is ready for write
      * access since the export could be available before migration handover.
      */
     blk_invalidate_cache(blk, NULL);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 11/17] block: Inactivate BDS when migration completes
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (9 preceding siblings ...)
  2016-01-20 16:24 ` [Qemu-devel] [PULL 10/17] block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVE Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-20 16:25 ` [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate Kevin Wolf
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

So far, live migration with shared storage meant that the image is in a
not-really-ready don't-touch-me state on the destination while the
source is still actively using it, but after completing the migration,
the image was fully opened on both sides. This is bad.

This patch adds a block driver callback to inactivate images on the
source before completing the migration. Inactivation means that it goes
to a state as if it was just live migrated to the qemu instance on the
source (i.e. BDRV_O_INACTIVE is set). You're then supposed to continue
either on the source or on the destination, which takes ownership of the
image.

A typical migration looks like this now with respect to disk images:

1. Destination qemu is started, the image is opened with
   BDRV_O_INACTIVE. The image is fully opened on the source.

2. Migration is about to complete. The source flushes the image and
   inactivates it. Now both sides have the image opened with
   BDRV_O_INACTIVE and are expecting the other side to still modify it.

3. One side (the destination on success) continues and calls
   bdrv_invalidate_all() in order to take ownership of the image again.
   This removes BDRV_O_INACTIVE on the resuming side; the flag remains
   set on the other side.

This ensures that the same image isn't written to by both instances
(unless both are resumed, but then you get what you deserve). This is
important because .bdrv_close for non-BDRV_O_INACTIVE images could write
to the image file, which is definitely forbidden while another host is
using the image.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 block.c                   | 34 ++++++++++++++++++++++++++++++++++
 include/block/block.h     |  1 +
 include/block/block_int.h |  1 +
 migration/migration.c     |  7 +++++++
 qmp.c                     | 12 ++++++++++++
 5 files changed, 55 insertions(+)

diff --git a/block.c b/block.c
index 95b2967..5709d3d 100644
--- a/block.c
+++ b/block.c
@@ -3303,6 +3303,40 @@ void bdrv_invalidate_cache_all(Error **errp)
     }
 }
 
+static int bdrv_inactivate(BlockDriverState *bs)
+{
+    int ret;
+
+    if (bs->drv->bdrv_inactivate) {
+        ret = bs->drv->bdrv_inactivate(bs);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    bs->open_flags |= BDRV_O_INACTIVE;
+    return 0;
+}
+
+int bdrv_inactivate_all(void)
+{
+    BlockDriverState *bs;
+    int ret;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
+        AioContext *aio_context = bdrv_get_aio_context(bs);
+
+        aio_context_acquire(aio_context);
+        ret = bdrv_inactivate(bs);
+        aio_context_release(aio_context);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
 /**************************************************************/
 /* removable device support */
 
diff --git a/include/block/block.h b/include/block/block.h
index 2b7d33c..25f36dc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -369,6 +369,7 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 /* Invalidate any cached metadata used by image formats */
 void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp);
 void bdrv_invalidate_cache_all(Error **errp);
+int bdrv_inactivate_all(void);
 
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 256609d..428fa33 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -172,6 +172,7 @@ struct BlockDriver {
      * Invalidate any cached meta-data.
      */
     void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp);
+    int (*bdrv_inactivate)(BlockDriverState *bs);
 
     /*
      * Flushes all data that was already written to the OS all the way down to
diff --git a/migration/migration.c b/migration/migration.c
index bc611e4..aaca451 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1422,7 +1422,11 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
     *old_vm_running = runstate_is_running();
     global_state_store();
     ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+    if (ret < 0) {
+        goto fail;
+    }
 
+    ret = bdrv_inactivate_all();
     if (ret < 0) {
         goto fail;
     }
@@ -1542,6 +1546,9 @@ static void migration_completion(MigrationState *s, int current_active_state,
         if (!ret) {
             ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
             if (ret >= 0) {
+                ret = bdrv_inactivate_all();
+            }
+            if (ret >= 0) {
                 qemu_file_set_rate_limit(s->file, INT64_MAX);
                 qemu_savevm_state_complete_precopy(s->file, false);
             }
diff --git a/qmp.c b/qmp.c
index 3ff6db7..53affe2 100644
--- a/qmp.c
+++ b/qmp.c
@@ -192,6 +192,18 @@ void qmp_cont(Error **errp)
         }
     }
 
+    /* Continuing after completed migration. Images have been inactivated to
+     * allow the destination to take control. Need to get control back now. */
+    if (runstate_check(RUN_STATE_FINISH_MIGRATE) ||
+        runstate_check(RUN_STATE_POSTMIGRATE))
+    {
+        bdrv_invalidate_cache_all(&local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         autostart = 1;
     } else {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (10 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 11/17] block: Inactivate BDS when migration completes Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-22 11:24   ` Paolo Bonzini
  2016-01-20 16:25 ` [Qemu-devel] [PULL 13/17] qcow2: Fix BDRV_O_INACTIVE handling in qcow2_invalidate_cache() Kevin Wolf
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

The callback has to ensure that closing or flushing the image afterwards
wouldn't cause a write access to the image files. This means that just
the caches have to be written out, which is part of the existing
.bdrv_close implementation.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 340ae8f..bfc80ea 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1686,6 +1686,32 @@ fail:
     return ret;
 }
 
+static int qcow2_inactivate(BlockDriverState *bs)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int ret, result = 0;
+
+    ret = qcow2_cache_flush(bs, s->l2_table_cache);
+    if (ret) {
+        result = ret;
+        error_report("Failed to flush the L2 table cache: %s",
+                     strerror(-ret));
+    }
+
+    ret = qcow2_cache_flush(bs, s->refcount_block_cache);
+    if (ret) {
+        result = ret;
+        error_report("Failed to flush the refcount block cache: %s",
+                     strerror(-ret));
+    }
+
+    if (result == 0) {
+        qcow2_mark_clean(bs);
+    }
+
+    return result;
+}
+
 static void qcow2_close(BlockDriverState *bs)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -1694,23 +1720,7 @@ static void qcow2_close(BlockDriverState *bs)
     s->l1_table = NULL;
 
     if (!(bs->open_flags & BDRV_O_INACTIVE)) {
-        int ret1, ret2;
-
-        ret1 = qcow2_cache_flush(bs, s->l2_table_cache);
-        ret2 = qcow2_cache_flush(bs, s->refcount_block_cache);
-
-        if (ret1) {
-            error_report("Failed to flush the L2 table cache: %s",
-                         strerror(-ret1));
-        }
-        if (ret2) {
-            error_report("Failed to flush the refcount block cache: %s",
-                         strerror(-ret2));
-        }
-
-        if (!ret1 && !ret2) {
-            qcow2_mark_clean(bs);
-        }
+        qcow2_inactivate(bs);
     }
 
     cache_clean_timer_del(bs);
@@ -3340,6 +3350,7 @@ BlockDriver bdrv_qcow2 = {
 
     .bdrv_refresh_limits        = qcow2_refresh_limits,
     .bdrv_invalidate_cache      = qcow2_invalidate_cache,
+    .bdrv_inactivate            = qcow2_inactivate,
 
     .create_opts         = &qcow2_create_opts,
     .bdrv_check          = qcow2_check,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 13/17] qcow2: Fix BDRV_O_INACTIVE handling in qcow2_invalidate_cache()
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (11 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-20 16:25 ` [Qemu-devel] [PULL 14/17] qcow2: Make image inaccessible after failed qcow2_invalidate_cache() Kevin Wolf
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

What qcow2_invalidate_cache() should do is close the image with
BDRV_O_INACTIVE set and reopen it with the flag cleared. In fact, it
used to do exactly the opposite: qcow2_close() relied on bs->open_flags,
which is already updated to have cleared BDRV_O_INACTIVE at this point,
whereas qcow2_open() was called with s->flags, which has the flag still
set. Fix this.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index bfc80ea..0f8c485 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1719,7 +1719,7 @@ static void qcow2_close(BlockDriverState *bs)
     /* else pre-write overlap checks in cache_destroy may crash */
     s->l1_table = NULL;
 
-    if (!(bs->open_flags & BDRV_O_INACTIVE)) {
+    if (!(s->flags & BDRV_O_INACTIVE)) {
         qcow2_inactivate(bs);
     }
 
@@ -1770,6 +1770,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
     memset(s, 0, sizeof(BDRVQcow2State));
     options = qdict_clone_shallow(bs->options);
 
+    flags &= ~BDRV_O_INACTIVE;
     ret = qcow2_open(bs, options, flags, &local_err);
     QDECREF(options);
     if (local_err) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 14/17] qcow2: Make image inaccessible after failed qcow2_invalidate_cache()
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (12 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 13/17] qcow2: Fix BDRV_O_INACTIVE handling in qcow2_invalidate_cache() Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-20 16:25 ` [Qemu-devel] [PULL 15/17] vmdk: Create streamOptimized as version 3 Kevin Wolf
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

If qcow2_invalidate_cache() fails, we are in a state where qcow2_close()
has already been completed, but the image hasn't been reopened yet.
Calling into any qcow2 function for an image in this state will cause
crashes.

The real solution would be to get rid of the close/open pair and instead
do an atomic reset of the involved data structures, but this isn't
trivial, so let's just make the image inaccessible for now.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 0f8c485..fd8436c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1764,6 +1764,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
     bdrv_invalidate_cache(bs->file->bs, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
+        bs->drv = NULL;
         return;
     }
 
@@ -1776,9 +1777,11 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
     if (local_err) {
         error_propagate(errp, local_err);
         error_prepend(errp, "Could not reopen qcow2 layer: ");
+        bs->drv = NULL;
         return;
     } else if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not reopen qcow2 layer");
+        bs->drv = NULL;
         return;
     }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 15/17] vmdk: Create streamOptimized as version 3
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (13 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 14/17] qcow2: Make image inaccessible after failed qcow2_invalidate_cache() Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-20 16:25 ` [Qemu-devel] [PULL 16/17] blockdev: Error out on negative throttling option values Kevin Wolf
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Fam Zheng <famz@redhat.com>

VMware products accept only version 3 for streamOptimized, let's bump
the version.

Reported-by: Radoslav Gerganov <rgerganov@vmware.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 6b8596c..698679d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1663,7 +1663,13 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
     }
     magic = cpu_to_be32(VMDK4_MAGIC);
     memset(&header, 0, sizeof(header));
-    header.version = zeroed_grain ? 2 : 1;
+    if (compress) {
+        header.version = 3;
+    } else if (zeroed_grain) {
+        header.version = 2;
+    } else {
+        header.version = 1;
+    }
     header.flags = VMDK4_FLAG_RGD | VMDK4_FLAG_NL_DETECT
                    | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0)
                    | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 16/17] blockdev: Error out on negative throttling option values
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (14 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 15/17] vmdk: Create streamOptimized as version 3 Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-20 16:25 ` [Qemu-devel] [PULL 17/17] iotests: Test that throttle values ranges Kevin Wolf
  2016-01-21 13:42 ` [Qemu-devel] [PULL 00/17] Block patches Peter Maydell
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Fam Zheng <famz@redhat.com>

extract_common_blockdev_options() uses qemu_opt_get_number() to parse
the bps/iops numbers to uint64_t, then converts to double and stores in
ThrottleConfig.  The actual parsing is done by strtoull() in
parse_option_number().  Negative numbers are wrapped to large positive
ones, and stored.

We used to reject negative numbers since 7d81c1413c9, but this regressed
when the option parsing code was changed later. Now fix this again.

This time, define an arbitrary large upper limit (1e15),  and check the
values so both negative and impractically big numbers are caught and
reported.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c              |  3 ++-
 include/qemu/throttle.h |  2 ++
 util/throttle.c         | 16 ++++++----------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1392fff..07cfe25 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -348,7 +348,8 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
     }
 
     if (!throttle_is_valid(cfg)) {
-        error_setg(errp, "bps/iops/maxs values must be 0 or greater");
+        error_setg(errp, "bps/iops/max values must be within [0, %lld]",
+                   THROTTLE_VALUE_MAX);
         return false;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 12faaad..d0c98ed 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -29,6 +29,8 @@
 #include "qemu-common.h"
 #include "qemu/timer.h"
 
+#define THROTTLE_VALUE_MAX 1000000000000000LL
+
 typedef enum {
     THROTTLE_BPS_TOTAL,
     THROTTLE_BPS_READ,
diff --git a/util/throttle.c b/util/throttle.c
index 1113671..af4bc95 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -282,22 +282,18 @@ bool throttle_conflicting(ThrottleConfig *cfg)
  */
 bool throttle_is_valid(ThrottleConfig *cfg)
 {
-    bool invalid = false;
     int i;
 
     for (i = 0; i < BUCKETS_COUNT; i++) {
-        if (cfg->buckets[i].avg < 0) {
-            invalid = true;
+        if (cfg->buckets[i].avg < 0 ||
+            cfg->buckets[i].max < 0 ||
+            cfg->buckets[i].avg > THROTTLE_VALUE_MAX ||
+            cfg->buckets[i].max > THROTTLE_VALUE_MAX) {
+            return false;
         }
     }
 
-    for (i = 0; i < BUCKETS_COUNT; i++) {
-        if (cfg->buckets[i].max < 0) {
-            invalid = true;
-        }
-    }
-
-    return !invalid;
+    return true;
 }
 
 /* check if bps_max/iops_max is used without bps/iops
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PULL 17/17] iotests: Test that throttle values ranges
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (15 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 16/17] blockdev: Error out on negative throttling option values Kevin Wolf
@ 2016-01-20 16:25 ` Kevin Wolf
  2016-01-21 13:42 ` [Qemu-devel] [PULL 00/17] Block patches Peter Maydell
  17 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-01-20 16:25 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/051        | 18 ++++++++++++++++++
 tests/qemu-iotests/051.out    | 39 +++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/051.pc.out | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index d91f80b..7bfe9ff 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -263,6 +263,24 @@ run_qemu -drive file="$TEST_IMG",iops_size=1234,throttling.iops-size=5678
 run_qemu -drive file="$TEST_IMG",readonly=on,read-only=off
 
 echo
+echo === Catching negative/large throttling values ===
+echo
+
+run_qemu -drive file="$TEST_IMG",iops=-1
+run_qemu -drive file="$TEST_IMG",bps=-2
+run_qemu -drive file="$TEST_IMG",bps_rd=-3
+run_qemu -drive file="$TEST_IMG",bps_rd_max=-3
+run_qemu -drive file="$TEST_IMG",throttling.iops-total=-4
+run_qemu -drive file="$TEST_IMG",throttling.bps-total=-5
+# These are accepted
+run_qemu -drive file="$TEST_IMG",bps=0
+run_qemu -drive file="$TEST_IMG",bps=1
+run_qemu -drive file="$TEST_IMG",bps=1000000000000000
+# While these are not
+run_qemu -drive file="$TEST_IMG",bps=1000000000000001
+run_qemu -drive file="$TEST_IMG",bps=9999999999999999
+
+echo
 echo === Parsing protocol from file name ===
 echo
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index bf886ce..0f8a8d3 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -285,6 +285,45 @@ Testing: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off
 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' and its alias 'readonly' can't be used at the same time
 
 
+=== Catching negative/large throttling values ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops=-1
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops=-1: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=-2
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=-2: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd=-3
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd=-3: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=0
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000000
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999: bps/iops/max values must be within [0, 1000000000000000]
+
+
 === Parsing protocol from file name ===
 
 Testing: -hda foo:bar
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index a5dfc33..85fc05d 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -379,6 +379,45 @@ Testing: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off
 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' and its alias 'readonly' can't be used at the same time
 
 
+=== Catching negative/large throttling values ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops=-1
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops=-1: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=-2
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=-2: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd=-3
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd=-3: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=0
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000000
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001: bps/iops/max values must be within [0, 1000000000000000]
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999: bps/iops/max values must be within [0, 1000000000000000]
+
+
 === Parsing protocol from file name ===
 
 Testing: -hda foo:bar
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 00/17] Block patches
  2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
                   ` (16 preceding siblings ...)
  2016-01-20 16:25 ` [Qemu-devel] [PULL 17/17] iotests: Test that throttle values ranges Kevin Wolf
@ 2016-01-21 13:42 ` Peter Maydell
  17 siblings, 0 replies; 26+ messages in thread
From: Peter Maydell @ 2016-01-21 13:42 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block

On 20 January 2016 at 16:24, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit 3db34bf64ab4f8797565dd8750003156c32b301d:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-peter' into staging (2016-01-18 17:40:50 +0000)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to e9b155501da2638e4e319af9960d35da1bc8662b:
>
>   iotests: Test that throttle values ranges (2016-01-20 13:37:57 +0100)
>
> ----------------------------------------------------------------
> Block layer patches
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate
  2016-01-20 16:25 ` [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate Kevin Wolf
@ 2016-01-22 11:24   ` Paolo Bonzini
  0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2016-01-22 11:24 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: qemu-devel



On 20/01/2016 17:25, Kevin Wolf wrote:
> +    ret = qcow2_cache_flush(bs, s->refcount_block_cache);
> +    if (ret) {
> +        result = ret;
> +        error_report("Failed to flush the refcount block cache: %s",
> +                     strerror(-ret));
> +    }
> +
> +    if (result == 0) {
> +        qcow2_mark_clean(bs);

Should you return an error if qcow2_mark_clean does?  (Coverity noticed
that its return value is usually checked).

Paolo

> +    }
> +
> +    return result;

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-01-20 16:24 ` [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING Kevin Wolf
@ 2016-02-18 16:03   ` Laurent Vivier
  2016-02-19  9:17     ` Kevin Wolf
  0 siblings, 1 reply; 26+ messages in thread
From: Laurent Vivier @ 2016-02-18 16:03 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: qemu-devel

Hi,

this commit breaks incoming migration case:

qemu-system-ppc64 XXX -incoming tcp:0:4444
qemu-system-ppc64: .../qemu/block/io.c:1304: bdrv_co_do_pwritev:
Assertion `!(bs->open_flags & 0x0800)' failed.

Without "-incoming", the same command line boots fine.

Are you aware of the problem?

Laurent

On 20/01/2016 17:24, Kevin Wolf wrote:
> As long as BDRV_O_INCOMING is set, the image file is only opened so we
> have a file descriptor for it. We're definitely not supposed to modify
> the image, it's still owned by the migration source.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block/io.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/io.c b/block/io.c
> index 707c04b..2372994 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1301,6 +1301,7 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
>      if (bs->read_only) {
>          return -EPERM;
>      }
> +    assert(!(bs->open_flags & BDRV_O_INCOMING));
>  
>      ret = bdrv_check_byte_request(bs, offset, bytes);
>      if (ret < 0) {
> @@ -2462,6 +2463,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
>      } else if (bs->read_only) {
>          return -EPERM;
>      }
> +    assert(!(bs->open_flags & BDRV_O_INCOMING));
>  
>      /* Do nothing if disabled.  */
>      if (!(bs->open_flags & BDRV_O_UNMAP)) {
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-02-18 16:03   ` Laurent Vivier
@ 2016-02-19  9:17     ` Kevin Wolf
  2016-02-19 10:37       ` Laurent Vivier
  0 siblings, 1 reply; 26+ messages in thread
From: Kevin Wolf @ 2016-02-19  9:17 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: qemu-devel, qemu-block

Am 18.02.2016 um 17:03 hat Laurent Vivier geschrieben:
> Hi,
> 
> this commit breaks incoming migration case:
> 
> qemu-system-ppc64 XXX -incoming tcp:0:4444
> qemu-system-ppc64: .../qemu/block/io.c:1304: bdrv_co_do_pwritev:
> Assertion `!(bs->open_flags & 0x0800)' failed.
> 
> Without "-incoming", the same command line boots fine.
> 
> Are you aware of the problem?

No, and with no other command line options it works for me:

    $ ppc64-softmmu/qemu-system-ppc64 -incoming tcp::4444

I have also tested somewhat more realistic cases on x86 (including
performing actual migrations) before committing the patch, so it's not
generally broken.

Can you please give the full command line and the stack backtrace? I
think you're seeing a bug where some PPC hardware tries to read (or
access anyway) the image before the migration has completed. This is for
obvious reasons wrong (the contents may still change until the VM is
handed over).

Kevin

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-02-19  9:17     ` Kevin Wolf
@ 2016-02-19 10:37       ` Laurent Vivier
  2016-02-19 13:47         ` Kevin Wolf
  0 siblings, 1 reply; 26+ messages in thread
From: Laurent Vivier @ 2016-02-19 10:37 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, qemu-block



On 19/02/2016 10:17, Kevin Wolf wrote:
> Am 18.02.2016 um 17:03 hat Laurent Vivier geschrieben:
>> Hi,
>>
>> this commit breaks incoming migration case:
>>
>> qemu-system-ppc64 XXX -incoming tcp:0:4444
>> qemu-system-ppc64: .../qemu/block/io.c:1304: bdrv_co_do_pwritev:
>> Assertion `!(bs->open_flags & 0x0800)' failed.
>>
>> Without "-incoming", the same command line boots fine.
>>
>> Are you aware of the problem?
> 
> No, and with no other command line options it works for me:
> 
>     $ ppc64-softmmu/qemu-system-ppc64 -incoming tcp::4444
> 
> I have also tested somewhat more realistic cases on x86 (including
> performing actual migrations) before committing the patch, so it's not
> generally broken.
> 
> Can you please give the full command line and the stack backtrace? I
> think you're seeing a bug where some PPC hardware tries to read (or
> access anyway) the image before the migration has completed. This is for
> obvious reasons wrong (the contents may still change until the VM is
> handed over).

It happens before the migration has started, as soon as I start qemu
with "-incoming" parameter.

The simplified command line is:

qemu-system-ppc64
-drive file=rhel7.2-le.qcow2,format=qcow2,snapshot=on
-incoming tcp:0:4444

The problem seems coming from "snapshot=on".

Laurent

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-02-19 10:37       ` Laurent Vivier
@ 2016-02-19 13:47         ` Kevin Wolf
  2016-02-19 14:11           ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Kevin Wolf @ 2016-02-19 13:47 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: qemu-devel, qemu-block

Am 19.02.2016 um 11:37 hat Laurent Vivier geschrieben:
> 
> 
> On 19/02/2016 10:17, Kevin Wolf wrote:
> > Am 18.02.2016 um 17:03 hat Laurent Vivier geschrieben:
> >> Hi,
> >>
> >> this commit breaks incoming migration case:
> >>
> >> qemu-system-ppc64 XXX -incoming tcp:0:4444
> >> qemu-system-ppc64: .../qemu/block/io.c:1304: bdrv_co_do_pwritev:
> >> Assertion `!(bs->open_flags & 0x0800)' failed.
> >>
> >> Without "-incoming", the same command line boots fine.
> >>
> >> Are you aware of the problem?
> > 
> > No, and with no other command line options it works for me:
> > 
> >     $ ppc64-softmmu/qemu-system-ppc64 -incoming tcp::4444
> > 
> > I have also tested somewhat more realistic cases on x86 (including
> > performing actual migrations) before committing the patch, so it's not
> > generally broken.
> > 
> > Can you please give the full command line and the stack backtrace? I
> > think you're seeing a bug where some PPC hardware tries to read (or
> > access anyway) the image before the migration has completed. This is for
> > obvious reasons wrong (the contents may still change until the VM is
> > handed over).
> 
> It happens before the migration has started, as soon as I start qemu
> with "-incoming" parameter.
> 
> The simplified command line is:
> 
> qemu-system-ppc64
> -drive file=rhel7.2-le.qcow2,format=qcow2,snapshot=on
> -incoming tcp:0:4444
> 
> The problem seems coming from "snapshot=on".

Aha. The combination of migration with snapshot=on is obvious nonsense
(because on the destination the disk state would be reverted while the
guest OS thinks the disk is unchanged), so we should probably take a
migration blocker when an image with snapshot=on is opened.

Kevin

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-02-19 13:47         ` Kevin Wolf
@ 2016-02-19 14:11           ` Paolo Bonzini
  2016-02-19 14:17             ` Kevin Wolf
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2016-02-19 14:11 UTC (permalink / raw)
  To: Kevin Wolf, Laurent Vivier; +Cc: qemu-devel, qemu-block

On 19/02/2016 14:47, Kevin Wolf wrote:
> > The problem seems coming from "snapshot=on".
> Aha. The combination of migration with snapshot=on is obvious nonsense
> (because on the destination the disk state would be reverted while the
> guest OS thinks the disk is unchanged),

Not really, on the destination the disk state will not be _further
updated_, but no reversion happens.

Migration with "snapshot=on" makes sense if you are resuming a VM from a
saved checkpoint ("-incoming exec").  You can use it to debug a
migration load, for example, because you want the resumed guest to
always start with the same disk contents.  It's somewhat weird, but it
definitely has its uses.  A blocker would be a pain in the ass...

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING
  2016-02-19 14:11           ` Paolo Bonzini
@ 2016-02-19 14:17             ` Kevin Wolf
  0 siblings, 0 replies; 26+ messages in thread
From: Kevin Wolf @ 2016-02-19 14:17 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Laurent Vivier, qemu-devel, qemu-block

Am 19.02.2016 um 15:11 hat Paolo Bonzini geschrieben:
> On 19/02/2016 14:47, Kevin Wolf wrote:
> > > The problem seems coming from "snapshot=on".
> > Aha. The combination of migration with snapshot=on is obvious nonsense
> > (because on the destination the disk state would be reverted while the
> > guest OS thinks the disk is unchanged),
> 
> Not really, on the destination the disk state will not be _further
> updated_, but no reversion happens.

Right, I started writing a patch when I noticed that that wasn't quite
right and -snapshot on the destination is somewhat unusual, but not in
all cases wrong.

> Migration with "snapshot=on" makes sense if you are resuming a VM from a
> saved checkpoint ("-incoming exec").  You can use it to debug a
> migration load, for example, because you want the resumed guest to
> always start with the same disk contents.  It's somewhat weird, but it
> definitely has its uses.  A blocker would be a pain in the ass...

My other thinko was that a blocker would help. In reality, blockers are
on the source host. They would make a little more sense than forbidding
-snapshot with -incoming, but we don't know how you migrate storage. If
you don't use shared storage but mirroring, then it could be okay again
(but probably even weirder than the other case).

Kevin

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2016-02-19 14:17 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-20 16:24 [Qemu-devel] [PULL 00/17] Block patches Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 01/17] block: Fix .bdrv_open flags Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 02/17] block/raw-posix: avoid bogus fixup for cylinders on DASD disks Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 03/17] qemu-img: Speed up comparing empty/zero images Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 04/17] qemu-iotests: Reduce racy output in 028 Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 05/17] block: Clean up includes Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 06/17] qcow2: Write feature table only for v3 images Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 07/17] qcow2: Write full header on image creation Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 08/17] block: Assert no write requests under BDRV_O_INCOMING Kevin Wolf
2016-02-18 16:03   ` Laurent Vivier
2016-02-19  9:17     ` Kevin Wolf
2016-02-19 10:37       ` Laurent Vivier
2016-02-19 13:47         ` Kevin Wolf
2016-02-19 14:11           ` Paolo Bonzini
2016-02-19 14:17             ` Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 09/17] block: Fix error path in bdrv_invalidate_cache() Kevin Wolf
2016-01-20 16:24 ` [Qemu-devel] [PULL 10/17] block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVE Kevin Wolf
2016-01-20 16:25 ` [Qemu-devel] [PULL 11/17] block: Inactivate BDS when migration completes Kevin Wolf
2016-01-20 16:25 ` [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate Kevin Wolf
2016-01-22 11:24   ` Paolo Bonzini
2016-01-20 16:25 ` [Qemu-devel] [PULL 13/17] qcow2: Fix BDRV_O_INACTIVE handling in qcow2_invalidate_cache() Kevin Wolf
2016-01-20 16:25 ` [Qemu-devel] [PULL 14/17] qcow2: Make image inaccessible after failed qcow2_invalidate_cache() Kevin Wolf
2016-01-20 16:25 ` [Qemu-devel] [PULL 15/17] vmdk: Create streamOptimized as version 3 Kevin Wolf
2016-01-20 16:25 ` [Qemu-devel] [PULL 16/17] blockdev: Error out on negative throttling option values Kevin Wolf
2016-01-20 16:25 ` [Qemu-devel] [PULL 17/17] iotests: Test that throttle values ranges Kevin Wolf
2016-01-21 13:42 ` [Qemu-devel] [PULL 00/17] Block patches Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).