[Qemu-devel] [PULL for Kevin 00/16] Block job improvements part 2

[Paolo Bonzini]

Kevin,

the following changes since commit caa1a2eaf344fc8552f30d7c452df6ed292f6336:

  qemu-img rebase: use empty string to rebase without backing file (2012-10-16 
14:53:14 +0200)

are available in the git repository at:

  git://github.com/bonzini/qemu.git blkmirror-job-1.3-part2

for you to fetch changes up to b3929fa13a81380a247904cb518166ed22ebe7b7:

  qemu-iotests: add testcases for mirroring on-source-error/on-target-error 
(2012-10-18 16:27:15 +0200)

This message has the diff from the rebase of v2.  You reviewed 15 more
patches, these are 16 because "block: rename block_job_complete to
block_job_completed" conflicted with Jeff's commit series and thus got
bumped from part 1 to part 2.  I did not manage to find a better name,
and none was suggested either.

The next batch should go up to patch 41, "mirror: allow customizing the
granularity".  Thanks for all your effort.

Paolo

----------------------------------------------------------------
Paolo Bonzini (16):
      block: add bdrv_query_info
      block: add bdrv_query_stats
      block: add bdrv_open_backing_file
      block: introduce new dirty bitmap functionality
      block: export dirty bitmap information in query-block
      block: rename block_job_complete to block_job_completed
      block: add block-job-complete
      block: introduce BLOCK_JOB_READY event
      mirror: introduce mirror job
      qmp: add drive-mirror command
      mirror: implement completion
      qemu-iotests: add mirroring test case
      iostatus: forward block_job_iostatus_reset to block job
      mirror: add support for on-source-error/on-target-error
      qmp: add pull_event function
      qemu-iotests: add testcases for mirroring on-source-error/on-target-error

 QMP/qmp-events.txt            |  20 ++
 QMP/qmp.py                    |  20 ++
 block.c                       | 241 +++++++++++------
 block.h                       |   8 +-
 block/Makefile.objs           |   1 +
 block/commit.c                |   2 +-
 block/mirror.c                | 322 ++++++++++++++++++++++
 block/stream.c                |   4 +-
 block_int.h                   |  24 ++
 blockdev.c                    | 161 ++++++++++-
 blockjob.c                    |  36 ++-
 blockjob.h                    |  41 ++-
 hmp-commands.hx               |  38 ++-
 hmp.c                         |  39 +++
 hmp.h                         |   2 +
 monitor.c                     |   1 +
 monitor.h                     |   1 +
 qapi-schema.json              | 106 +++++++-
 qerror.h                      |   3 +
 qmp-commands.hx               |  53 ++++
 tests/qemu-iotests/041        | 617 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/041.out    |   5 +
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |   4 +
 trace-events                  |   8 +
 25 file modificati, 1643 inserzioni(+), 115 rimozioni(-)
 create mode 100644 block/mirror.c
 create mode 100755 tests/qemu-iotests/041
 create mode 100644 tests/qemu-iotests/041.out

diff --git a/block.c b/block.c
index 5361b6c..3e76322 100644
--- a/block.c
+++ b/block.c
@@ -767,12 +767,6 @@ int bdrv_open_backing_file(BlockDriverState *bs)
         bs->open_flags |= BDRV_O_NO_BACKING;
         return ret;
     }
-    if (bs->is_temporary) {
-        bs->backing_hd->keep_read_only = !(bs->open_flags & BDRV_O_RDWR);
-    } else {
-        /* base images use the same setting as leaf */
-        bs->backing_hd->keep_read_only = bs->keep_read_only;
-    }
     return 0;
 }
 
@@ -2838,7 +2838,8 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
     if (bs->dirty_bitmap) {
         info->has_dirty = true;
         info->dirty = g_malloc0(sizeof(*info->dirty));
-        info->dirty->count = bdrv_get_dirty_count(bs) * 
BDRV_SECTORS_PER_DIRTY_CHUNK;
+        info->dirty->count = bdrv_get_dirty_count(bs) *
+            BDRV_SECTORS_PER_DIRTY_CHUNK * BDRV_SECTOR_SIZE;
     }
 
     if (bs->drv) {
@@ -2855,6 +2856,8 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
             info->inserted->backing_file = g_strdup(bs->backing_file);
         }
 
+        info->inserted->backing_file_depth = bdrv_get_backing_file_depth(bs);
+
         if (bs->io_limits_enabled) {
             info->inserted->bps =
                            bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL];
diff --git a/block/commit.c b/block/commit.c
index 733c914..897af5f 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -160,7 +160,7 @@ exit_restore_reopen:
         bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
     }
 
-    block_job_complete(&s->common, ret);
+    block_job_completed(&s->common, ret);
 }
 
 static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/block/mirror.c b/block/mirror.c
index caec272..d6618a4 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -34,7 +34,7 @@ typedef struct MirrorBlockJob {
     MirrorSyncMode mode;
     BlockdevOnError on_source_error, on_target_error;
     bool synced;
-    bool complete;
+    bool should_complete;
     int64_t sector_num;
     uint8_t *buf;
 } MirrorBlockJob;
@@ -172,7 +172,8 @@ static void coroutine_fn mirror_run(void *opaque)
                     s->synced = true;
                 }
 
-                should_complete = block_job_is_cancelled(&s->common) || 
s->complete;
+                should_complete = s->should_complete ||
+                    block_job_is_cancelled(&s->common);
                 cnt = bdrv_get_dirty_count(bs);
             }
         }
@@ -227,7 +228,10 @@ immediate_exit:
     g_free(s->buf);
     bdrv_set_dirty_tracking(bs, false);
     bdrv_iostatus_disable(s->target);
-    if (s->complete && ret == 0) {
+    if (s->should_complete && ret == 0) {
+        if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
+            bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
+        }
         bdrv_swap(s->target, s->common.bs);
     }
     bdrv_close(s->target);
@@ -271,7 +275,7 @@ static void mirror_complete(BlockJob *job, Error **errp)
         return;
     }
 
-    s->complete = true;
+    s->should_complete = true;
     block_job_resume(job);
 }
 
diff --git a/blockdev.c b/blockdev.c
index b74673c..f37d5eb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1218,6 +1218,11 @@ void qmp_drive_mirror(const char *device, const char 
*target,
         return;
     }
 
+    if (!bdrv_is_inserted(bs)) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+        return;
+    }
+
     if (!has_format) {
         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
     }
@@ -1229,11 +1234,6 @@ void qmp_drive_mirror(const char *device, const char 
*target,
         }
     }
 
-    if (!bdrv_is_inserted(bs)) {
-        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-        return;
-    }
-
     if (bdrv_in_use(bs)) {
         error_set(errp, QERR_DEVICE_IN_USE, device);
         return;
diff --git a/qapi-schema.json b/qapi-schema.json
index 4b299d4..295263b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -651,7 +651,7 @@
 #
 # Block dirty bitmap information.
 #
-# @count: number of dirty sectors according to the dirty bitmap
+# @count: number of dirty bytes according to the dirty bitmap
 #
 # Since: 1.3
 ##
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
old mode 100644
new mode 100755
index ec86c70..6681705
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -2,7 +2,7 @@
 #
 # Tests for image mirroring.
 #
-# Copyright (C) 2012 IBM Corp.
+# Copyright (C) 2012 Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -162,6 +162,20 @@ class TestSingleDrive(ImageMirroringTestCase):
                              target=target_img)
         self.assert_qmp(result, 'return', {})
 
+        self.cancel_and_wait(wait_ready=False)
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/inserted/file', test_img)
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
+    def test_cancel_after_ready(self):
+        self.assert_no_active_mirrors()
+
+        result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
         self.cancel_and_wait()
         result = self.vm.qmp('query-block')
         self.assert_qmp(result, 'return[0]/inserted/file', test_img)
@@ -211,24 +225,21 @@ class TestSingleDrive(ImageMirroringTestCase):
         self.assertTrue(self.compare_images(test_img, target_img),
                         'target image does not match source after mirroring')
 
+    def test_medium_not_found(self):
+        result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
+                             target=target_img)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
     def test_image_not_found(self):
         result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
                              mode='existing', target=target_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        # Avoid failure on os.remove
-        qemu_img('create', '-f', iotests.imgfmt, '-o', 
'cluster_size=%d,backing_file=%s'
-                        % (TestSingleDrive.image_len, test_img), target_img)
-
     def test_device_not_found(self):
         result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
                              target=target_img)
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
-        # Avoid failure on os.remove
-        qemu_img('create', '-f', iotests.imgfmt, '-o', 
'cluster_size=%d,backing_file=%s'
-                        % (TestSingleDrive.image_len, test_img), target_img)
-
 class TestMirrorNoBacking(ImageMirroringTestCase):
     image_len = 2 * 1024 * 1024 # MB