[Qemu-devel] [PATCH v2 0/4] blockdev-backup: don't check aio_context too early

[Qemu-devel] [PATCH v2 0/4] blockdev-backup: don't check aio_context too early

[John Snow]

See patch one's commit message for justification.

v2: added patch 4, with iotest framework adjustments in patches 2/3.

John Snow (4):
  blockdev-backup: don't check aio_context too early
  iotests.py: do not use infinite waits
  iotests.py: rewrite run_job to be pickier
  iotests: add iotest 250 for testing blockdev-backup across iothread
    contexts

 blockdev.c                    |   4 --
 tests/qemu-iotests/250        | 129 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/250.out    | 119 +++++++++++++++++++++++++++++++
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |  44 ++++++------
 5 files changed, 270 insertions(+), 27 deletions(-)
 create mode 100755 tests/qemu-iotests/250
 create mode 100644 tests/qemu-iotests/250.out

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/4] blockdev-backup: don't check aio_context too early

[John Snow]

in blockdev_backup_prepare, we check to make sure that the target is
associated with a compatible aio context. However, do_blockdev_backup is
called later and has some logic to move the target to a compatible
aio_context. The transaction version will fail certain commands
needlessly early as a result.

Allow blockdev_backup_prepare to simply call do_blockdev_backup, which
will ultimately decide if the contexts are compatible or not.

Note: the transaction version has always disallowed this operation since
its initial commit bd8baecd (2014), whereas the version of
qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to
enforce the aio_context switch instead. It's not clear, and I can't see
from the mailing list archives at the time, why the two functions take a
different approach. It wasn't until later in efd7556708b (2016) that the
standalone version tried to determine if it could set the context or
not.

Reported-by: aihua liang <aliang@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1683498
Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 79fbac8450..a81d88980c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1872,10 +1872,6 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
     }
 
     aio_context = bdrv_get_aio_context(bs);
-    if (aio_context != bdrv_get_aio_context(target)) {
-        error_setg(errp, "Backup between two IO threads is not implemented");
-        return;
-    }
     aio_context_acquire(aio_context);
     state->bs = bs;
 
-- 
2.20.1

[Qemu-devel] [PATCH v2 2/4] iotests.py: do not use infinite waits

[John Snow]

Cap waits to 60 seconds so that iotests can fail gracefully if something
goes wrong.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f811f69135..561f547a97 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -516,7 +516,7 @@ class VM(qtest.QEMUQtestMachine):
             output_list += [key + '=' + obj[key]]
         return ','.join(output_list)
 
-    def get_qmp_events_filtered(self, wait=True):
+    def get_qmp_events_filtered(self, wait=60.0):
         result = []
         for ev in self.get_qmp_events(wait=wait):
             result.append(filter_qmp_event(ev))
@@ -533,10 +533,10 @@ class VM(qtest.QEMUQtestMachine):
         return result
 
     # Returns None on success, and an error string on failure
-    def run_job(self, job, auto_finalize=True, auto_dismiss=False):
+    def run_job(self, job, auto_finalize=True, auto_dismiss=False, wait=60.0):
         error = None
         while True:
-            for ev in self.get_qmp_events_filtered(wait=True):
+            for ev in self.get_qmp_events_filtered(wait=wait):
                 if ev['event'] == 'JOB_STATUS_CHANGE':
                     status = ev['data']['status']
                     if status == 'aborting':
@@ -625,7 +625,7 @@ class QMPTestCase(unittest.TestCase):
         self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
                          self.vm.flatten_qmp_object(reference))
 
-    def cancel_and_wait(self, drive='drive0', force=False, resume=False):
+    def cancel_and_wait(self, drive='drive0', force=False, resume=False, wait=60.0):
         '''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', {})
@@ -636,7 +636,7 @@ class QMPTestCase(unittest.TestCase):
         cancelled = False
         result = None
         while not cancelled:
-            for event in self.vm.get_qmp_events(wait=True):
+            for event in self.vm.get_qmp_events(wait=wait):
                 if event['event'] == 'BLOCK_JOB_COMPLETED' or \
                    event['event'] == 'BLOCK_JOB_CANCELLED':
                     self.assert_qmp(event, 'data/device', drive)
@@ -649,10 +649,10 @@ class QMPTestCase(unittest.TestCase):
         self.assert_no_active_block_jobs()
         return result
 
-    def wait_until_completed(self, drive='drive0', check_offset=True):
+    def wait_until_completed(self, drive='drive0', check_offset=True, wait=60.0):
         '''Wait for a block job to finish, returning the event'''
         while True:
-            for event in self.vm.get_qmp_events(wait=True):
+            for event in self.vm.get_qmp_events(wait=wait):
                 if event['event'] == 'BLOCK_JOB_COMPLETED':
                     self.assert_qmp(event, 'data/device', drive)
                     self.assert_qmp_absent(event, 'data/error')
-- 
2.20.1

[Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier

[John Snow]

Don't pull events out of the queue that don't belong to us;
be choosier so that we can use this method to drive jobs that
were launched by transactions that may have more jobs.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 561f547a97..601c802476 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -536,23 +536,21 @@ class VM(qtest.QEMUQtestMachine):
     def run_job(self, job, auto_finalize=True, auto_dismiss=False, wait=60.0):
         error = None
         while True:
-            for ev in self.get_qmp_events_filtered(wait=wait):
-                if ev['event'] == 'JOB_STATUS_CHANGE':
-                    status = ev['data']['status']
-                    if status == 'aborting':
-                        result = self.qmp('query-jobs')
-                        for j in result['return']:
-                            if j['id'] == job:
-                                error = j['error']
-                                log('Job failed: %s' % (j['error']))
-                    elif status == 'pending' and not auto_finalize:
-                        self.qmp_log('job-finalize', id=job)
-                    elif status == 'concluded' and not auto_dismiss:
-                        self.qmp_log('job-dismiss', id=job)
-                    elif status == 'null':
-                        return error
-                else:
-                    iotests.log(ev)
+            ev = self.event_wait(name='JOB_STATUS_CHANGE',
+                                 match={'data':{'id':job}})
+            status = ev['data']['status']
+            if status == 'aborting':
+                result = self.qmp('query-jobs')
+                for j in result['return']:
+                    if j['id'] == job:
+                        error = j['error']
+                        log('Job failed: %s' % (j['error']))
+            elif status == 'pending' and not auto_finalize:
+                self.qmp_log('job-finalize', id=job)
+            elif status == 'concluded' and not auto_dismiss:
+                self.qmp_log('job-dismiss', id=job)
+            elif status == 'null':
+                return error
 
     def node_info(self, node_name):
         nodes = self.qmp('query-named-block-nodes')
-- 
2.20.1

[Qemu-devel] [PATCH v2 4/4] iotests: add iotest 250 for testing blockdev-backup across iothread contexts

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/250     | 129 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/250.out | 119 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 249 insertions(+)
 create mode 100755 tests/qemu-iotests/250
 create mode 100644 tests/qemu-iotests/250.out

diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
new file mode 100755
index 0000000000..1406b10958
--- /dev/null
+++ b/tests/qemu-iotests/250
@@ -0,0 +1,129 @@
+#!/usr/bin/env python
+#
+# Test incremental/backup across iothread contexts
+#
+# Copyright (c) 2019 John Snow for 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
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# owner=jsnow@redhat.com
+
+import logging
+import os
+import iotests
+from iotests import log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+size = 64 * 1024 * 1024
+
+with iotests.FilePath('img0') as img0_path, \
+     iotests.FilePath('img1') as img1_path, \
+     iotests.FilePath('img0-full') as img0_full_path, \
+     iotests.FilePath('img1-full') as img1_full_path, \
+     iotests.FilePath('img0-incr') as img0_incr_path, \
+     iotests.FilePath('img1-incr') as img1_incr_path, \
+     iotests.VM() as vm:
+
+    def create_target(filepath, name, size):
+        basename = os.path.basename(filepath)
+        nodename = "file_{}".format(basename)
+        log(vm.command('blockdev-create', job_id='job1',
+                       options={
+                           'driver': 'file',
+                           'filename': filepath,
+                           'size': size,
+                       }))
+        vm.run_job('job1')
+        log(vm.command('blockdev-add', driver='file',
+                       node_name=nodename, filename=filepath))
+        log(vm.command('blockdev-create', job_id='job2',
+                       options={
+                           'driver': iotests.imgfmt,
+                           'file': nodename,
+                           'size': size,
+                       }))
+        vm.run_job('job2')
+        log(vm.command('blockdev-add', driver=iotests.imgfmt,
+                       node_name=name,
+                       file=nodename))
+
+    def wait_job(job):
+        vm.run_job(job, auto_dismiss=True)
+        event = vm.event_wait(name='BLOCK_JOB_COMPLETED',
+                              match={'data':{'device':job}})
+        log(iotests.filter_qmp_event(event))
+
+    log('--- Preparing images & VM ---\n')
+    vm.add_object('iothread,id=iothread0')
+    vm.add_object('iothread,id=iothread1')
+    vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
+    vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
+    iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
+    iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
+    vm.add_drive(img0_path, interface='none')
+    vm.add_device('scsi-hd,id=device0,drive=drive0,bus=scsi0.0')
+    vm.add_drive(img1_path, interface='none')
+    vm.add_device('scsi-hd,id=device1,drive=drive1,bus=scsi1.0')
+
+    log('--- Starting VM ---\n')
+    vm.launch()
+
+    log('--- Create Targets & Full Backups ---\n')
+    create_target(img0_full_path, 'img0-full', size)
+    create_target(img1_full_path, 'img1-full', size)
+    ret = vm.qmp_log('transaction', indent=2, actions=[
+        { 'type': 'block-dirty-bitmap-add',
+          'data': { 'node': 'drive0', 'name': 'bitmap0' }},
+        { 'type': 'block-dirty-bitmap-add',
+          'data': { 'node': 'drive1', 'name': 'bitmap1' }},
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive0',
+                    'target': 'img0-full',
+                    'sync': 'full',
+                    'job-id': 'j0' }},
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive1',
+                    'target': 'img1-full',
+                    'sync': 'full',
+                    'job-id': 'j1' }}
+    ])
+    if "error" in ret:
+        raise Exception(ret['error']['desc'])
+    wait_job('j0')
+    wait_job('j1')
+
+    log('\n--- Create Targets & Incremental Backups ---\n')
+    create_target(img0_incr_path, 'img0-incr', size)
+    create_target(img1_incr_path, 'img1-incr', size)
+    ret = vm.qmp_log('transaction', indent=2, actions=[
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive0',
+                    'target': 'img0-incr',
+                    'sync': 'incremental',
+                    'bitmap': 'bitmap0',
+                    'job-id': 'j2' }},
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive1',
+                    'target': 'img1-incr',
+                    'sync': 'incremental',
+                    'bitmap': 'bitmap1',
+                    'job-id': 'j3' }}
+    ])
+    if "error" in ret:
+        raise Exception(ret['error']['desc'])
+    wait_job('j2')
+    wait_job('j3')
+
+    log('\n--- Done ---')
+    vm.shutdown()
diff --git a/tests/qemu-iotests/250.out b/tests/qemu-iotests/250.out
new file mode 100644
index 0000000000..eec38614ec
--- /dev/null
+++ b/tests/qemu-iotests/250.out
@@ -0,0 +1,119 @@
+--- Preparing images & VM ---
+
+--- Starting VM ---
+
+--- Create Targets & Full Backups ---
+
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{
+  "execute": "transaction",
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "name": "bitmap0",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "name": "bitmap1",
+          "node": "drive1"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "device": "drive0",
+          "job-id": "j0",
+          "sync": "full",
+          "target": "img0-full"
+        },
+        "type": "blockdev-backup"
+      },
+      {
+        "data": {
+          "device": "drive1",
+          "job-id": "j1",
+          "sync": "full",
+          "target": "img1-full"
+        },
+        "type": "blockdev-backup"
+      }
+    ]
+  }
+}
+{
+  "return": {}
+}
+{"data": {"device": "j0", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "j1", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+
+--- Create Targets & Incremental Backups ---
+
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{
+  "execute": "transaction",
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "bitmap": "bitmap0",
+          "device": "drive0",
+          "job-id": "j2",
+          "sync": "incremental",
+          "target": "img0-incr"
+        },
+        "type": "blockdev-backup"
+      },
+      {
+        "data": {
+          "bitmap": "bitmap1",
+          "device": "drive1",
+          "job-id": "j3",
+          "sync": "incremental",
+          "target": "img1-incr"
+        },
+        "type": "blockdev-backup"
+      }
+    ]
+  }
+}
+{
+  "return": {}
+}
+{"data": {"device": "j2", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "j3", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+
+--- Done ---
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 7ac9a5ea4a..f533836848 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -249,3 +249,4 @@
 247 rw auto quick
 248 rw auto quick
 249 rw auto quick
+250 rw auto quick
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 0/4] blockdev-backup: don't check aio_context too early

[John Snow]

Happy Friday: ping!

Max: this series corrects some things that were causing some of the
pitfalls that made me nervous about the context I wrote about in 219.

On 5/10/19 3:03 PM, John Snow wrote:
> See patch one's commit message for justification.
> 
> v2: added patch 4, with iotest framework adjustments in patches 2/3.
> 
> John Snow (4):
>   blockdev-backup: don't check aio_context too early
>   iotests.py: do not use infinite waits
>   iotests.py: rewrite run_job to be pickier
>   iotests: add iotest 250 for testing blockdev-backup across iothread
>     contexts
> 
>  blockdev.c                    |   4 --
>  tests/qemu-iotests/250        | 129 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/250.out    | 119 +++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group      |   1 +
>  tests/qemu-iotests/iotests.py |  44 ++++++------
>  5 files changed, 270 insertions(+), 27 deletions(-)
>  create mode 100755 tests/qemu-iotests/250
>  create mode 100644 tests/qemu-iotests/250.out
> 

Re: [Qemu-devel] [PATCH v2 4/4] iotests: add iotest 250 for testing blockdev-backup across iothread contexts

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

On 10.05.19 21:03, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/250     | 129 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/250.out | 119 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 249 insertions(+)
>  create mode 100755 tests/qemu-iotests/250
>  create mode 100644 tests/qemu-iotests/250.out
> 
> diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
> new file mode 100755
> index 0000000000..1406b10958
> --- /dev/null
> +++ b/tests/qemu-iotests/250
> @@ -0,0 +1,129 @@

[...]

> +    def create_target(filepath, name, size):
> +        basename = os.path.basename(filepath)
> +        nodename = "file_{}".format(basename)
> +        log(vm.command('blockdev-create', job_id='job1',
> +                       options={
> +                           'driver': 'file',
> +                           'filename': filepath,
> +                           'size': size,

I think this should be 0.  No complaints apart from that, so I can fix
that up when applying, if you agree.

Max

> +                       }))
> +        vm.run_job('job1')
> +        log(vm.command('blockdev-add', driver='file',
> +                       node_name=nodename, filename=filepath))
> +        log(vm.command('blockdev-create', job_id='job2',
> +                       options={
> +                           'driver': iotests.imgfmt,
> +                           'file': nodename,
> +                           'size': size,
> +                       }))
> +        vm.run_job('job2')
> +        log(vm.command('blockdev-add', driver=iotests.imgfmt,
> +                       node_name=name,
> +                       file=nodename))
> +


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v2 4/4] iotests: add iotest 250 for testing blockdev-backup across iothread contexts

[John Snow]

On 5/17/19 7:18 AM, Max Reitz wrote:
> On 10.05.19 21:03, John Snow wrote:
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  tests/qemu-iotests/250     | 129 +++++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/250.out | 119 ++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/group   |   1 +
>>  3 files changed, 249 insertions(+)
>>  create mode 100755 tests/qemu-iotests/250
>>  create mode 100644 tests/qemu-iotests/250.out
>>
>> diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
>> new file mode 100755
>> index 0000000000..1406b10958
>> --- /dev/null
>> +++ b/tests/qemu-iotests/250
>> @@ -0,0 +1,129 @@
> 
> [...]
> 
>> +    def create_target(filepath, name, size):
>> +        basename = os.path.basename(filepath)
>> +        nodename = "file_{}".format(basename)
>> +        log(vm.command('blockdev-create', job_id='job1',
>> +                       options={
>> +                           'driver': 'file',
>> +                           'filename': filepath,
>> +                           'size': size,
> 
> I think this should be 0.  No complaints apart from that, so I can fix
> that up when applying, if you agree.
> 
> Max
> 

Oh, should it? I guess you're right. At the very least, it's not right
to use the raw logical size here.

Yes, please feel free to amend this.

Thank you, Max!

>> +                       }))
>> +        vm.run_job('job1')
>> +        log(vm.command('blockdev-add', driver='file',
>> +                       node_name=nodename, filename=filepath))
>> +        log(vm.command('blockdev-create', job_id='job2',
>> +                       options={
>> +                           'driver': iotests.imgfmt,
>> +                           'file': nodename,
>> +                           'size': size,
>> +                       }))
>> +        vm.run_job('job2')
>> +        log(vm.command('blockdev-add', driver=iotests.imgfmt,
>> +                       node_name=name,
>> +                       file=nodename))
>> +
> 

Re: [Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

On 10.05.19 21:03, John Snow wrote:
> Don't pull events out of the queue that don't belong to us;
> be choosier so that we can use this method to drive jobs that
> were launched by transactions that may have more jobs.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 32 +++++++++++++++-----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)

There are a couple of conflicts because of concurrent patches to run_job
now.  I resolved those, but then noticed that the tests 245 and 255 no
longer pass; their reference output contains events like
BLOCK_JOB_PENDING and BLOCK_JOB_COMPLETED.

I’m not sure whether we should remove those event from the output.  It
feels weird to me to keep them somewhere in the back log and not show
them in tests that by design have kind of a full QMP log.  On the other
hand, I see that this patch is necessary.  Ideally, I think run_job
should log all events that relate to the job at hand -- but our current
event_wait() matching system doesn’t allow that.

Ideas? :-/

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier

[John Snow]

On 5/23/19 8:39 AM, Max Reitz wrote:
> On 10.05.19 21:03, John Snow wrote:
>> Don't pull events out of the queue that don't belong to us;
>> be choosier so that we can use this method to drive jobs that
>> were launched by transactions that may have more jobs.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  tests/qemu-iotests/iotests.py | 32 +++++++++++++++-----------------
>>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> There are a couple of conflicts because of concurrent patches to run_job
> now.  I resolved those, but then noticed that the tests 245 and 255 no
> longer pass; their reference output contains events like
> BLOCK_JOB_PENDING and BLOCK_JOB_COMPLETED.
> 
> I’m not sure whether we should remove those event from the output.  It
> feels weird to me to keep them somewhere in the back log and not show
> them in tests that by design have kind of a full QMP log.  On the other
> hand, I see that this patch is necessary.  Ideally, I think run_job
> should log all events that relate to the job at hand -- but our current
> event_wait() matching system doesn’t allow that.
> 
> Ideas? :-/
> 

Amend event_wait to be able to wait for multiple events and criteria;
then amend run_job to wait for both legacy and contemporary job events.

Because all block_job_* events are omitted prior to the final transition
to the null state, they can remain optional waits. Whenever they are
present, they will be fully consumed and logged.

Patches comin' up.

> Max
>