[Qemu-devel] [PATCH v4 0/4] Use blkdebug to make test deterministic

[Qemu-devel] [PATCH v4 0/4] Use blkdebug to make test deterministic

[Fam Zheng]

This adds "remove_break" command to block, which removes a break point defined
with "break". It is used in iotests.py to pause and resume drive in block job
cases to make the test deterministic.

v4: [01] Added.
    [03] Add common method pair "pause_drive" and "resume_drive".
    [04] Also fix 040, 055.


Fam Zheng (4):
  qemu-iotests: Drop local version of cancel_and_wait from 040
  blkdebug: add "remove_break" command
  qemu-iotest: Add pause_drive and resume_drive methods
  qemu-iotests: Make test case 030, 040 and 055 deterministic

 block.c                       | 13 +++++++++++++
 block/blkdebug.c              | 26 ++++++++++++++++++++++++++
 include/block/block.h         |  1 +
 include/block/block_int.h     |  2 ++
 qemu-io-cmds.c                | 22 ++++++++++++++++++++++
 tests/qemu-iotests/030        | 16 +++++++++++-----
 tests/qemu-iotests/040        | 19 +++----------------
 tests/qemu-iotests/055        | 15 +++++++++++----
 tests/qemu-iotests/iotests.py | 17 ++++++++++++++++-
 9 files changed, 105 insertions(+), 26 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [PATCH v4 1/4] qemu-iotests: Drop local version of cancel_and_wait from 040

[Fam Zheng]

iotests.py already has one.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/040 | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index a2e18c5..0e85136 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -39,21 +39,6 @@ class ImageCommitTestCase(iotests.QMPTestCase):
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return', [])
 
-    def cancel_and_wait(self, drive='drive0'):
-        '''Cancel a block job and wait for it to finish'''
-        result = self.vm.qmp('block-job-cancel', device=drive)
-        self.assert_qmp(result, 'return', {})
-
-        cancelled = False
-        while not cancelled:
-            for event in self.vm.get_qmp_events(wait=True):
-                if event['event'] == 'BLOCK_JOB_CANCELLED':
-                    self.assert_qmp(event, 'data/type', 'commit')
-                    self.assert_qmp(event, 'data/device', drive)
-                    cancelled = True
-
-        self.assert_no_active_commit()
-
 class TestSingleDrive(ImageCommitTestCase):
     image_len = 1 * 1024 * 1024
     test_len = 1 * 1024 * 256
-- 
1.8.4.2

[Qemu-devel] [PATCH v4 2/4] blkdebug: add "remove_break" command

[Fam Zheng]

This adds "remove_break" command which is the reverse of blkdebug
command "break": it removes all breakpoints with given tag and resumes
all the requests.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c                   | 13 +++++++++++++
 block/blkdebug.c          | 26 ++++++++++++++++++++++++++
 include/block/block.h     |  1 +
 include/block/block_int.h |  2 ++
 qemu-io-cmds.c            | 22 ++++++++++++++++++++++
 5 files changed, 64 insertions(+)

diff --git a/block.c b/block.c
index 58efb5b..6bcf945 100644
--- a/block.c
+++ b/block.c
@@ -3412,6 +3412,19 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
     return -ENOTSUP;
 }
 
+int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)
+{
+    while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) {
+        bs = bs->file;
+    }
+
+    if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
+        return bs->drv->bdrv_debug_remove_breakpoint(bs, tag);
+    }
+
+    return -ENOTSUP;
+}
+
 int bdrv_debug_resume(BlockDriverState *bs, const char *tag)
 {
     while (bs && bs->drv && !bs->drv->bdrv_debug_resume) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 16d2b91..23eceea 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -605,6 +605,30 @@ static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
     return -ENOENT;
 }
 
+static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
+                                            const char *tag)
+{
+    BDRVBlkdebugState *s = bs->opaque;
+    BlkdebugSuspendedReq *r;
+    BlkdebugRule *rule, *next;
+    int i;
+
+    for (i = 0; i < BLKDBG_EVENT_MAX; i++) {
+        QLIST_FOREACH_SAFE(rule, &s->rules[i], next, next) {
+            if (rule->action == ACTION_SUSPEND &&
+                !strcmp(rule->options.suspend.tag, tag)) {
+                remove_rule(rule);
+            }
+        }
+    }
+    QLIST_FOREACH(r, &s->suspended_reqs, next) {
+        if (!strcmp(r->tag, tag)) {
+            qemu_coroutine_enter(r->co, NULL);
+            return 0;
+        }
+    }
+    return -ENOENT;
+}
 
 static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char *tag)
 {
@@ -639,6 +663,8 @@ static BlockDriver bdrv_blkdebug = {
 
     .bdrv_debug_event           = blkdebug_debug_event,
     .bdrv_debug_breakpoint      = blkdebug_debug_breakpoint,
+    .bdrv_debug_remove_breakpoint
+                                = blkdebug_debug_remove_breakpoint,
     .bdrv_debug_resume          = blkdebug_debug_resume,
     .bdrv_debug_is_suspended    = blkdebug_debug_is_suspended,
 };
diff --git a/include/block/block.h b/include/block/block.h
index 3560deb..6b17495 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -484,6 +484,7 @@ void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
 
 int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
                            const char *tag);
+int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag);
 int bdrv_debug_resume(BlockDriverState *bs, const char *tag);
 bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag);
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1666066..659dcf4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -218,6 +218,8 @@ struct BlockDriver {
     /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
     int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event,
         const char *tag);
+    int (*bdrv_debug_remove_breakpoint)(BlockDriverState *bs,
+        const char *tag);
     int (*bdrv_debug_resume)(BlockDriverState *bs, const char *tag);
     bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
 
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 667f4e4..4cab57e 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1956,6 +1956,18 @@ static int break_f(BlockDriverState *bs, int argc, char **argv)
     return 0;
 }
 
+static int remove_break_f(BlockDriverState *bs, int argc, char **argv)
+{
+    int ret;
+
+    ret = bdrv_debug_remove_breakpoint(bs, argv[1]);
+    if (ret < 0) {
+        printf("Could not remove breakpoint %s: %s\n", argv[1], strerror(-ret));
+    }
+
+    return 0;
+}
+
 static const cmdinfo_t break_cmd = {
        .name           = "break",
        .argmin         = 2,
@@ -1966,6 +1978,15 @@ static const cmdinfo_t break_cmd = {
                          "request as tag",
 };
 
+static const cmdinfo_t remove_break_cmd = {
+       .name           = "remove_break",
+       .argmin         = 1,
+       .argmax         = 1,
+       .cfunc          = remove_break_f,
+       .args           = "tag",
+       .oneline        = "remove a breakpoint by tag",
+};
+
 static int resume_f(BlockDriverState *bs, int argc, char **argv)
 {
     int ret;
@@ -2126,6 +2147,7 @@ static void __attribute((constructor)) init_qemuio_commands(void)
     qemuio_add_command(&alloc_cmd);
     qemuio_add_command(&map_cmd);
     qemuio_add_command(&break_cmd);
+    qemuio_add_command(&remove_break_cmd);
     qemuio_add_command(&resume_cmd);
     qemuio_add_command(&wait_break_cmd);
     qemuio_add_command(&abort_cmd);
-- 
1.8.4.2

[Qemu-devel] [PATCH v4 3/4] qemu-iotest: Add pause_drive and resume_drive methods

[Fam Zheng]

They wrap blkdebug "break" and "remove_break".

Add optional argument "resume" to cancel_and_wait().

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index fb10ff4..a483865 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -107,6 +107,18 @@ class VM(object):
         self._num_drives += 1
         return self
 
+    def pause_drive(self, drive, event=""):
+        '''Pause drive r/w operations'''
+        if not event:
+            self.pause_drive(drive, "read_aio")
+            self.pause_drive(drive, "write_aio")
+        self.qmp('human-monitor-command',
+                    command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))
+
+    def resume_drive(self, drive):
+        self.qmp('human-monitor-command',
+                    command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))
+
     def hmp_qemu_io(self, drive, cmd):
         '''Write to a given drive using an HMP command'''
         return self.qmp('human-monitor-command',
@@ -222,11 +234,14 @@ class QMPTestCase(unittest.TestCase):
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return', [])
 
-    def cancel_and_wait(self, drive='drive0', force=False):
+    def cancel_and_wait(self, drive='drive0', force=False, resume=""):
         '''Cancel a block job and wait for it to finish, returning the event'''
         result = self.vm.qmp('block-job-cancel', device=drive, force=force)
         self.assert_qmp(result, 'return', {})
 
+        if resume:
+            self.vm.resume_drive(drive)
+
         cancelled = False
         result = None
         while not cancelled:
-- 
1.8.4.2

[Qemu-devel] [PATCH v4 4/4] qemu-iotests: Make test case 030, 040 and 055 deterministic

[Fam Zheng]

Pause the drive and start the block job, so we won't miss the block job.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/030 | 16 +++++++++++-----
 tests/qemu-iotests/040 |  4 +++-
 tests/qemu-iotests/055 | 15 +++++++++++----
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index d0f96ea..6283707 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -34,6 +34,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         iotests.create_image(backing_img, TestSingleDrive.image_len)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
+        qemu_io('-c', 'write -P 0x1 0 512', backing_img)
         self.vm = iotests.VM().add_drive(test_img)
         self.vm.launch()
 
@@ -69,6 +70,7 @@ class TestSingleDrive(iotests.QMPTestCase):
     def test_stream_pause(self):
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('block-stream', device='drive0')
         self.assert_qmp(result, 'return', {})
 
@@ -86,6 +88,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         result = self.vm.qmp('block-job-resume', device='drive0')
         self.assert_qmp(result, 'return', {})
 
+        self.vm.resume_drive('drive0')
         completed = False
         while not completed:
             for event in self.vm.get_qmp_events(wait=True):
@@ -391,7 +394,7 @@ class TestStreamStop(iotests.QMPTestCase):
         qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
         qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
-        self.vm = iotests.VM().add_drive(test_img)
+        self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
         self.vm.launch()
 
     def tearDown(self):
@@ -402,6 +405,7 @@ class TestStreamStop(iotests.QMPTestCase):
     def test_stream_stop(self):
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('block-stream', device='drive0')
         self.assert_qmp(result, 'return', {})
 
@@ -409,7 +413,7 @@ class TestStreamStop(iotests.QMPTestCase):
         events = self.vm.get_qmp_events(wait=False)
         self.assertEqual(events, [], 'unexpected QMP event: %s' % events)
 
-        self.cancel_and_wait()
+        self.cancel_and_wait(resume='drive0')
 
 class TestSetSpeed(iotests.QMPTestCase):
     image_len = 80 * 1024 * 1024 # MB
@@ -419,7 +423,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
         qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
-        self.vm = iotests.VM().add_drive(test_img)
+        self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
         self.vm.launch()
 
     def tearDown(self):
@@ -453,6 +457,7 @@ class TestSetSpeed(iotests.QMPTestCase):
     def test_set_speed(self):
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('block-stream', device='drive0')
         self.assert_qmp(result, 'return', {})
 
@@ -469,7 +474,8 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024)
 
-        self.cancel_and_wait()
+        self.cancel_and_wait(resume='drive0')
+        self.vm.pause_drive('drive0')
 
         # Check setting speed in block-stream works
         result = self.vm.qmp('block-stream', device='drive0', speed=4 * 1024 * 1024)
@@ -479,7 +485,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024)
 
-        self.cancel_and_wait()
+        self.cancel_and_wait(resume='drive0')
 
     def test_set_speed_invalid(self):
         self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 0e85136..fce439f 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -228,6 +228,7 @@ class TestSetSpeed(ImageCommitTestCase):
         qemu_img('create', backing_img, str(TestSetSpeed.image_len))
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
+        qemu_io('-c', 'write -P 0x1 0 512', test_img)
         self.vm = iotests.VM().add_drive(test_img)
         self.vm.launch()
 
@@ -240,6 +241,7 @@ class TestSetSpeed(ImageCommitTestCase):
     def test_set_speed(self):
         self.assert_no_active_commit()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('block-commit', device='drive0', top=mid_img, speed=1024 * 1024)
         self.assert_qmp(result, 'return', {})
 
@@ -248,7 +250,7 @@ class TestSetSpeed(ImageCommitTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 1024 * 1024)
 
-        self.cancel_and_wait()
+        self.cancel_and_wait(resume='drive0')
 
 
 if __name__ == '__main__':
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 44bb025..9bb8e20 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -63,6 +63,7 @@ class TestSingleDrive(iotests.QMPTestCase):
     def test_pause(self):
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('drive-backup', device='drive0',
                              target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
@@ -70,7 +71,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         result = self.vm.qmp('block-job-pause', device='drive0')
         self.assert_qmp(result, 'return', {})
 
-        time.sleep(1)
+        self.vm.resume_drive('drive0')
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
@@ -113,6 +114,7 @@ class TestSetSpeed(iotests.QMPTestCase):
 
     def setUp(self):
         qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSetSpeed.image_len))
+        qemu_io('-c', 'write -P1 0 512', test_img)
         self.vm = iotests.VM().add_drive(test_img)
         self.vm.launch()
 
@@ -124,6 +126,7 @@ class TestSetSpeed(iotests.QMPTestCase):
     def test_set_speed(self):
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('drive-backup', device='drive0',
                              target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
@@ -141,10 +144,11 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024)
 
-        event = self.cancel_and_wait()
+        event = self.cancel_and_wait(resume='drive0')
         self.assert_qmp(event, 'data/type', 'backup')
 
         # Check setting speed in drive-backup works
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('drive-backup', device='drive0',
                              target=target_img, sync='full', speed=4*1024*1024)
         self.assert_qmp(result, 'return', {})
@@ -153,7 +157,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024)
 
-        event = self.cancel_and_wait()
+        event = self.cancel_and_wait(resume='drive0')
         self.assert_qmp(event, 'data/type', 'backup')
 
     def test_set_speed_invalid(self):
@@ -165,6 +169,7 @@ class TestSetSpeed(iotests.QMPTestCase):
 
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('drive-backup', device='drive0',
                              target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
@@ -172,7 +177,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        event = self.cancel_and_wait()
+        event = self.cancel_and_wait(resume='drive0')
         self.assert_qmp(event, 'data/type', 'backup')
 
 class TestSingleTransaction(iotests.QMPTestCase):
@@ -214,6 +219,7 @@ class TestSingleTransaction(iotests.QMPTestCase):
     def test_pause(self):
         self.assert_no_active_block_jobs()
 
+        self.vm.pause_drive('drive0')
         result = self.vm.qmp('transaction', actions=[{
                 'type': 'drive-backup',
                 'data': { 'device': 'drive0',
@@ -226,6 +232,7 @@ class TestSingleTransaction(iotests.QMPTestCase):
         result = self.vm.qmp('block-job-pause', device='drive0')
         self.assert_qmp(result, 'return', {})
 
+        self.vm.resume_drive('drive0')
         time.sleep(1)
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH v4 2/4] blkdebug: add "remove_break" command

[Max Reitz]

On 14.11.2013 10:10, Fam Zheng wrote:
> This adds "remove_break" command which is the reverse of blkdebug
> command "break": it removes all breakpoints with given tag and resumes
> all the requests.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c                   | 13 +++++++++++++
>  block/blkdebug.c          | 26 ++++++++++++++++++++++++++
>  include/block/block.h     |  1 +
>  include/block/block_int.h |  2 ++
>  qemu-io-cmds.c            | 22 ++++++++++++++++++++++
>  5 files changed, 64 insertions(+)
>
> diff --git a/block.c b/block.c
> index 58efb5b..6bcf945 100644
> --- a/block.c
> +++ b/block.c
> @@ -3412,6 +3412,19 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
>      return -ENOTSUP;
>  }
>  
> +int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)
> +{
> +    while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) {
> +        bs = bs->file;
> +    }
> +
> +    if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
> +        return bs->drv->bdrv_debug_remove_breakpoint(bs, tag);
> +    }
> +
> +    return -ENOTSUP;
> +}
> +
>  int bdrv_debug_resume(BlockDriverState *bs, const char *tag)
>  {
>      while (bs && bs->drv && !bs->drv->bdrv_debug_resume) {
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 16d2b91..23eceea 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -605,6 +605,30 @@ static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
>      return -ENOENT;
>  }
>  
> +static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
> +                                            const char *tag)
> +{
> +    BDRVBlkdebugState *s = bs->opaque;
> +    BlkdebugSuspendedReq *r;
> +    BlkdebugRule *rule, *next;
> +    int i;
> +
> +    for (i = 0; i < BLKDBG_EVENT_MAX; i++) {
> +        QLIST_FOREACH_SAFE(rule, &s->rules[i], next, next) {
> +            if (rule->action == ACTION_SUSPEND &&
> +                !strcmp(rule->options.suspend.tag, tag)) {
> +                remove_rule(rule);
> +            }
> +        }
> +    }
> +    QLIST_FOREACH(r, &s->suspended_reqs, next) {
> +        if (!strcmp(r->tag, tag)) {
> +            qemu_coroutine_enter(r->co, NULL);
> +            return 0;

As far as I can see, this does not resume all the request but only the
first one found, or does it?

Max

> +        }
> +    }
> +    return -ENOENT;
> +}
>  
>  static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char *tag)
>  {
> @@ -639,6 +663,8 @@ static BlockDriver bdrv_blkdebug = {
>  
>      .bdrv_debug_event           = blkdebug_debug_event,
>      .bdrv_debug_breakpoint      = blkdebug_debug_breakpoint,
> +    .bdrv_debug_remove_breakpoint
> +                                = blkdebug_debug_remove_breakpoint,
>      .bdrv_debug_resume          = blkdebug_debug_resume,
>      .bdrv_debug_is_suspended    = blkdebug_debug_is_suspended,
>  };
> diff --git a/include/block/block.h b/include/block/block.h
> index 3560deb..6b17495 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -484,6 +484,7 @@ void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
>  
>  int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
>                             const char *tag);
> +int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag);
>  int bdrv_debug_resume(BlockDriverState *bs, const char *tag);
>  bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag);
>  
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 1666066..659dcf4 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -218,6 +218,8 @@ struct BlockDriver {
>      /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
>      int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event,
>          const char *tag);
> +    int (*bdrv_debug_remove_breakpoint)(BlockDriverState *bs,
> +        const char *tag);
>      int (*bdrv_debug_resume)(BlockDriverState *bs, const char *tag);
>      bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
>  
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 667f4e4..4cab57e 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1956,6 +1956,18 @@ static int break_f(BlockDriverState *bs, int argc, char **argv)
>      return 0;
>  }
>  
> +static int remove_break_f(BlockDriverState *bs, int argc, char **argv)
> +{
> +    int ret;
> +
> +    ret = bdrv_debug_remove_breakpoint(bs, argv[1]);
> +    if (ret < 0) {
> +        printf("Could not remove breakpoint %s: %s\n", argv[1], strerror(-ret));
> +    }
> +
> +    return 0;
> +}
> +
>  static const cmdinfo_t break_cmd = {
>         .name           = "break",
>         .argmin         = 2,
> @@ -1966,6 +1978,15 @@ static const cmdinfo_t break_cmd = {
>                           "request as tag",
>  };
>  
> +static const cmdinfo_t remove_break_cmd = {
> +       .name           = "remove_break",
> +       .argmin         = 1,
> +       .argmax         = 1,
> +       .cfunc          = remove_break_f,
> +       .args           = "tag",
> +       .oneline        = "remove a breakpoint by tag",
> +};
> +
>  static int resume_f(BlockDriverState *bs, int argc, char **argv)
>  {
>      int ret;
> @@ -2126,6 +2147,7 @@ static void __attribute((constructor)) init_qemuio_commands(void)
>      qemuio_add_command(&alloc_cmd);
>      qemuio_add_command(&map_cmd);
>      qemuio_add_command(&break_cmd);
> +    qemuio_add_command(&remove_break_cmd);
>      qemuio_add_command(&resume_cmd);
>      qemuio_add_command(&wait_break_cmd);
>      qemuio_add_command(&abort_cmd);

Re: [Qemu-devel] [PATCH v4 3/4] qemu-iotest: Add pause_drive and resume_drive methods

[Max Reitz]

On 14.11.2013 10:10, Fam Zheng wrote:
> They wrap blkdebug "break" and "remove_break".
>
> Add optional argument "resume" to cancel_and_wait().
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index fb10ff4..a483865 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -107,6 +107,18 @@ class VM(object):
>          self._num_drives += 1
>          return self
>  
> +    def pause_drive(self, drive, event=""):
> +        '''Pause drive r/w operations'''
> +        if not event:
> +            self.pause_drive(drive, "read_aio")
> +            self.pause_drive(drive, "write_aio")
> +        self.qmp('human-monitor-command',
> +                    command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))

In the default event="" case (on, say, "drive0"), you'll execute the
three HMP commands 'qemu-io drive0 "break read_aio bp_drive0"', 'qemu-io
drive0 "break write_aio bp_drive0"', 'qemu-io drive0 "break 
bp_drive0"', that is, this code will issue three HMP commands with the
last one having an empty string as the event (which is probably not what
you want and will probably result in a silently ignored error from the
break command).

Also, I'd propose using something like None instead of "" for the
default event.

> +
> +    def resume_drive(self, drive):
> +        self.qmp('human-monitor-command',
> +                    command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))
> +
>      def hmp_qemu_io(self, drive, cmd):
>          '''Write to a given drive using an HMP command'''
>          return self.qmp('human-monitor-command',
> @@ -222,11 +234,14 @@ class QMPTestCase(unittest.TestCase):
>          result = self.vm.qmp('query-block-jobs')
>          self.assert_qmp(result, 'return', [])
>  
> -    def cancel_and_wait(self, drive='drive0', force=False):
> +    def cancel_and_wait(self, drive='drive0', force=False, resume=""):
>          '''Cancel a block job and wait for it to finish, returning the event'''
>          result = self.vm.qmp('block-job-cancel', device=drive, force=force)
>          self.assert_qmp(result, 'return', {})
>  
> +        if resume:
> +            self.vm.resume_drive(drive)
> +

I'd propose using a bool as the resume value (i.e., resume=False as
default). In the next patch, it looks like the parameter should be the
drive name, although it actually doesn't matter at all what string it
contains (as long as it isn't empty). Thus, a boolean value is probably
more appropriate.

Max

>          cancelled = False
>          result = None
>          while not cancelled: