[Qemu-devel] [PATCH] iotests: fix 169

Vladimir Sementsov-Ogievskiy posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180330161040.350271-1-vsementsov@virtuozzo.com
Test checkpatch passed
Test docker-build@min-glib failed
Test docker-mingw@fedora failed
Test docker-quick@centos6 failed
Test s390x passed
tests/qemu-iotests/169 | 44 +++++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 21 deletions(-)
[Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
Use MIGRATION events instead of RESUME. Also, make a TODO: enable
dirty-bitmaps capability for offline case.

This (likely) fixes racy faults at least of the following types:

    - timeout on waiting for RESUME event
    - sha256 mismatch on 136 (138 after this patch)

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---

This patch is a true change for the test anyway. But I don't understand,
why (and do really) it fixes the things. And I'm not sure about do we
really have a bug in bitmap migration or persistence. So, it's up to you,
take it into 2.12... 

It was already discussed, that "STOP" event is bad for tests. What about
"RESUME"? How can we miss it? And sha256 mismatch is really something
strange.

Max, please check, do it fix 169 for you.

 tests/qemu-iotests/169 | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
index 153b10b6e7..5e525ab9d5 100755
--- a/tests/qemu-iotests/169
+++ b/tests/qemu-iotests/169
@@ -31,6 +31,8 @@ disk_a = os.path.join(iotests.test_dir, 'disk_a')
 disk_b = os.path.join(iotests.test_dir, 'disk_b')
 size = '1M'
 mig_file = os.path.join(iotests.test_dir, 'mig_file')
+mig_cmd = 'exec: cat > ' + mig_file
+incoming_cmd = 'exec: cat ' + mig_file
 
 
 class TestDirtyBitmapMigration(iotests.QMPTestCase):
@@ -49,7 +51,6 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
         self.vm_a.launch()
 
         self.vm_b = iotests.VM(path_suffix='b')
-        self.vm_b.add_incoming("exec: cat '" + mig_file + "'")
 
     def add_bitmap(self, vm, granularity, persistent):
         params = {'node': 'drive0',
@@ -86,36 +87,30 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
                    (0xa0201, 0x1000))
 
         should_migrate = migrate_bitmaps or persistent and shared_storage
+        mig_caps = [{'capability': 'events', 'state': True}]
+        if migrate_bitmaps:
+            mig_caps.append({'capability': 'dirty-bitmaps', 'state': True})
 
+        result = self.vm_a.qmp('migrate-set-capabilities',
+                               capabilities=mig_caps)
+        self.assert_qmp(result, 'return', {})
+
+        self.vm_b.add_incoming(incoming_cmd if online else "defer")
         self.vm_b.add_drive(disk_a if shared_storage else disk_b)
 
         if online:
             os.mkfifo(mig_file)
             self.vm_b.launch()
+            result = self.vm_b.qmp('migrate-set-capabilities',
+                                   capabilities=mig_caps)
+            self.assert_qmp(result, 'return', {})
 
         self.add_bitmap(self.vm_a, granularity, persistent)
         for r in regions:
             self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
         sha256 = self.get_bitmap_hash(self.vm_a)
 
-        if migrate_bitmaps:
-            capabilities = [{'capability': 'dirty-bitmaps', 'state': True}]
-
-            result = self.vm_a.qmp('migrate-set-capabilities',
-                                   capabilities=capabilities)
-            self.assert_qmp(result, 'return', {})
-
-            if online:
-                result = self.vm_b.qmp('migrate-set-capabilities',
-                                       capabilities=capabilities)
-                self.assert_qmp(result, 'return', {})
-
-        result = self.vm_a.qmp('migrate-set-capabilities',
-                               capabilities=[{'capability': 'events',
-                                              'state': True}])
-        self.assert_qmp(result, 'return', {})
-
-        result = self.vm_a.qmp('migrate', uri='exec:cat>' + mig_file)
+        result = self.vm_a.qmp('migrate', uri=mig_cmd)
         while True:
             event = self.vm_a.event_wait('MIGRATION')
             if event['data']['status'] == 'completed':
@@ -124,9 +119,16 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
         if not online:
             self.vm_a.shutdown()
             self.vm_b.launch()
-            # TODO enable bitmap capability for vm_b in this case
+            result = self.vm_b.qmp('migrate-set-capabilities',
+                                   capabilities=mig_caps)
+            self.assert_qmp(result, 'return', {})
+            result = self.vm_b.qmp('migrate-incoming', uri=incoming_cmd)
+            self.assert_qmp(result, 'return', {})
 
-        self.vm_b.event_wait("RESUME", timeout=10.0)
+        while True:
+            event = self.vm_b.event_wait('MIGRATION')
+            if event['data']['status'] == 'completed':
+                break
 
         self.check_bitmap(self.vm_b, sha256 if should_migrate else False)
 
-- 
2.11.1


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
30.03.2018 19:10, Vladimir Sementsov-Ogievskiy wrote:
> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
> dirty-bitmaps capability for offline case.
>
> This (likely) fixes racy faults at least of the following types:
>
>      - timeout on waiting for RESUME event
>      - sha256 mismatch on 136 (138 after this patch)

136 line I mean

>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>
> This patch is a true change for the test anyway. But I don't understand,
> why (and do really) it fixes the things. And I'm not sure about do we
> really have a bug in bitmap migration or persistence. So, it's up to you,
> take it into 2.12...
>
> It was already discussed, that "STOP" event is bad for tests. What about
> "RESUME"? How can we miss it? And sha256 mismatch is really something
> strange.
>
> Max, please check, do it fix 169 for you.
>
>   tests/qemu-iotests/169 | 44 +++++++++++++++++++++++---------------------
>   1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
> index 153b10b6e7..5e525ab9d5 100755
> --- a/tests/qemu-iotests/169
> +++ b/tests/qemu-iotests/169
> @@ -31,6 +31,8 @@ disk_a = os.path.join(iotests.test_dir, 'disk_a')
>   disk_b = os.path.join(iotests.test_dir, 'disk_b')
>   size = '1M'
>   mig_file = os.path.join(iotests.test_dir, 'mig_file')
> +mig_cmd = 'exec: cat > ' + mig_file
> +incoming_cmd = 'exec: cat ' + mig_file
>   
>   
>   class TestDirtyBitmapMigration(iotests.QMPTestCase):
> @@ -49,7 +51,6 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
>           self.vm_a.launch()
>   
>           self.vm_b = iotests.VM(path_suffix='b')
> -        self.vm_b.add_incoming("exec: cat '" + mig_file + "'")
>   
>       def add_bitmap(self, vm, granularity, persistent):
>           params = {'node': 'drive0',
> @@ -86,36 +87,30 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
>                      (0xa0201, 0x1000))
>   
>           should_migrate = migrate_bitmaps or persistent and shared_storage
> +        mig_caps = [{'capability': 'events', 'state': True}]
> +        if migrate_bitmaps:
> +            mig_caps.append({'capability': 'dirty-bitmaps', 'state': True})
>   
> +        result = self.vm_a.qmp('migrate-set-capabilities',
> +                               capabilities=mig_caps)
> +        self.assert_qmp(result, 'return', {})
> +
> +        self.vm_b.add_incoming(incoming_cmd if online else "defer")
>           self.vm_b.add_drive(disk_a if shared_storage else disk_b)
>   
>           if online:
>               os.mkfifo(mig_file)
>               self.vm_b.launch()
> +            result = self.vm_b.qmp('migrate-set-capabilities',
> +                                   capabilities=mig_caps)
> +            self.assert_qmp(result, 'return', {})
>   
>           self.add_bitmap(self.vm_a, granularity, persistent)
>           for r in regions:
>               self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
>           sha256 = self.get_bitmap_hash(self.vm_a)
>   
> -        if migrate_bitmaps:
> -            capabilities = [{'capability': 'dirty-bitmaps', 'state': True}]
> -
> -            result = self.vm_a.qmp('migrate-set-capabilities',
> -                                   capabilities=capabilities)
> -            self.assert_qmp(result, 'return', {})
> -
> -            if online:
> -                result = self.vm_b.qmp('migrate-set-capabilities',
> -                                       capabilities=capabilities)
> -                self.assert_qmp(result, 'return', {})
> -
> -        result = self.vm_a.qmp('migrate-set-capabilities',
> -                               capabilities=[{'capability': 'events',
> -                                              'state': True}])
> -        self.assert_qmp(result, 'return', {})
> -
> -        result = self.vm_a.qmp('migrate', uri='exec:cat>' + mig_file)
> +        result = self.vm_a.qmp('migrate', uri=mig_cmd)
>           while True:
>               event = self.vm_a.event_wait('MIGRATION')
>               if event['data']['status'] == 'completed':
> @@ -124,9 +119,16 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
>           if not online:
>               self.vm_a.shutdown()
>               self.vm_b.launch()
> -            # TODO enable bitmap capability for vm_b in this case
> +            result = self.vm_b.qmp('migrate-set-capabilities',
> +                                   capabilities=mig_caps)
> +            self.assert_qmp(result, 'return', {})
> +            result = self.vm_b.qmp('migrate-incoming', uri=incoming_cmd)
> +            self.assert_qmp(result, 'return', {})
>   
> -        self.vm_b.event_wait("RESUME", timeout=10.0)
> +        while True:
> +            event = self.vm_b.event_wait('MIGRATION')
> +            if event['data']['status'] == 'completed':
> +                break
>   
>           self.check_bitmap(self.vm_b, sha256 if should_migrate else False)
>   


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by no-reply@patchew.org 6 years ago
Hi,

This series failed docker-build@min-glib build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180330161040.350271-1-vsementsov@virtuozzo.com
Subject: [Qemu-devel] [PATCH] iotests: fix 169

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
3d3f087137 iotests: fix 169

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-ywza6euw/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   min-glib
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-ywza6euw/src'
  GEN     /var/tmp/patchew-tester-tmp-ywza6euw/src/docker-src.2018-03-31-04.59.05.31413/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-ywza6euw/src/docker-src.2018-03-31-04.59.05.31413/qemu.tar.vroot'...
done.
Checking out files:  41% (2535/6066)   
Checking out files:  42% (2548/6066)   
Checking out files:  43% (2609/6066)   
Checking out files:  44% (2670/6066)   
Checking out files:  45% (2730/6066)   
Checking out files:  45% (2740/6066)   
Checking out files:  46% (2791/6066)   
Checking out files:  47% (2852/6066)   
Checking out files:  48% (2912/6066)   
Checking out files:  49% (2973/6066)   
Checking out files:  50% (3033/6066)   
Checking out files:  51% (3094/6066)   
Checking out files:  52% (3155/6066)   
Checking out files:  53% (3215/6066)   
Checking out files:  54% (3276/6066)   
Checking out files:  55% (3337/6066)   
Checking out files:  56% (3397/6066)   
Checking out files:  57% (3458/6066)   
Checking out files:  58% (3519/6066)   
Checking out files:  59% (3579/6066)   
Checking out files:  60% (3640/6066)   
Checking out files:  61% (3701/6066)   
Checking out files:  62% (3761/6066)   
Checking out files:  62% (3814/6066)   
Checking out files:  63% (3822/6066)   
Checking out files:  63% (3862/6066)   
Checking out files:  64% (3883/6066)   
Checking out files:  65% (3943/6066)   
Checking out files:  66% (4004/6066)   
Checking out files:  67% (4065/6066)   
Checking out files:  68% (4125/6066)   
Checking out files:  69% (4186/6066)   
Checking out files:  70% (4247/6066)   
Checking out files:  71% (4307/6066)   
Checking out files:  72% (4368/6066)   
Checking out files:  73% (4429/6066)   
Checking out files:  74% (4489/6066)   
Checking out files:  75% (4550/6066)   
Checking out files:  76% (4611/6066)   
Checking out files:  77% (4671/6066)   
Checking out files:  78% (4732/6066)   
Checking out files:  79% (4793/6066)   
Checking out files:  79% (4827/6066)   
Checking out files:  80% (4853/6066)   
Checking out files:  81% (4914/6066)   
Checking out files:  82% (4975/6066)   
Checking out files:  83% (5035/6066)   
Checking out files:  84% (5096/6066)   
Checking out files:  85% (5157/6066)   
Checking out files:  86% (5217/6066)   
Checking out files:  87% (5278/6066)   
Checking out files:  88% (5339/6066)   
Checking out files:  89% (5399/6066)   
Checking out files:  90% (5460/6066)   
Checking out files:  90% (5482/6066)   
Checking out files:  91% (5521/6066)   
Checking out files:  92% (5581/6066)   
Checking out files:  93% (5642/6066)   
Checking out files:  94% (5703/6066)   
Checking out files:  95% (5763/6066)   
Checking out files:  96% (5824/6066)   
Checking out files:  97% (5885/6066)   
Checking out files:  98% (5945/6066)   
Checking out files:  99% (6006/6066)   
Checking out files: 100% (6066/6066)   
Checking out files: 100% (6066/6066), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-ywza6euw/src/docker-src.2018-03-31-04.59.05.31413/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-ywza6euw/src/docker-src.2018-03-31-04.59.05.31413/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: /var/tmp/patchew-tester-tmp-ywza6euw/src/docker-src.2018-03-31-04.59.05.31413/qemu.tar: Wrote only 4096 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPY    RUNNER
    RUN test-build in qemu:min-glib 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Environment variables:
HOSTNAME=f6edce847e91
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

/var/tmp/qemu/run: line 52: cd: /tmp/qemu-test/src/tests/docker: No such file or directory
/var/tmp/qemu/run: line 57: /test-build: No such file or directory
/var/tmp/qemu/run: line 57: exec: /test-build: cannot execute: No such file or directory
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=cf35ef1234c111e8a67852540069c830', '-u', '0', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ywza6euw/src/docker-src.2018-03-31-04.59.05.31413:/var/tmp/qemu:z,ro', 'qemu:min-glib', '/var/tmp/qemu/run', 'test-build']' returned non-zero exit status 126
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-ywza6euw/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-build@min-glib] Error 2

real	0m30.307s
user	0m9.427s
sys	0m6.627s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Max Reitz 6 years ago
On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
> dirty-bitmaps capability for offline case.
> 
> This (likely) fixes racy faults at least of the following types:
> 
>     - timeout on waiting for RESUME event
>     - sha256 mismatch on 136 (138 after this patch)
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 
> This patch is a true change for the test anyway. But I don't understand,
> why (and do really) it fixes the things. And I'm not sure about do we
> really have a bug in bitmap migration or persistence. So, it's up to you,
> take it into 2.12... 
> 
> It was already discussed, that "STOP" event is bad for tests. What about
> "RESUME"? How can we miss it? And sha256 mismatch is really something
> strange.
> 
> Max, please check, do it fix 169 for you.
> 
>  tests/qemu-iotests/169 | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)

This makes the test pass (thanks!), but it still leaves behind five cats...

Max

Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by John Snow 6 years ago

On 04/03/2018 12:23 PM, Max Reitz wrote:
> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>> dirty-bitmaps capability for offline case.
>>
>> This (likely) fixes racy faults at least of the following types:
>>
>>     - timeout on waiting for RESUME event
>>     - sha256 mismatch on 136 (138 after this patch)
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>
>> This patch is a true change for the test anyway. But I don't understand,
>> why (and do really) it fixes the things. And I'm not sure about do we
>> really have a bug in bitmap migration or persistence. So, it's up to you,
>> take it into 2.12... 
>>
>> It was already discussed, that "STOP" event is bad for tests. What about
>> "RESUME"? How can we miss it? And sha256 mismatch is really something
>> strange.
>>
>> Max, please check, do it fix 169 for you.
>>
>>  tests/qemu-iotests/169 | 44 +++++++++++++++++++++++---------------------
>>  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> This makes the test pass (thanks!), but it still leaves behind five cats...
> 
> Max
> 
> 

Hmm:

jhuston  14772  0.0  0.0   4296   784 pts/3    S    16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  14796  0.0  0.0   4296   764 pts/3    S    16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  14940  0.0  0.0   4296   788 pts/3    S    16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  14964  0.0  0.0   4296   720 pts/3    S    16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  15052  0.0  0.0   4296   768 pts/3    S    16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file

Why do these get left behind? Nothing to consume the data...?

Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
03.04.2018 23:13, John Snow wrote:
>
> On 04/03/2018 12:23 PM, Max Reitz wrote:
>> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>>> dirty-bitmaps capability for offline case.
>>>
>>> This (likely) fixes racy faults at least of the following types:
>>>
>>>      - timeout on waiting for RESUME event
>>>      - sha256 mismatch on 136 (138 after this patch)
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>
>>> This patch is a true change for the test anyway. But I don't understand,
>>> why (and do really) it fixes the things. And I'm not sure about do we
>>> really have a bug in bitmap migration or persistence. So, it's up to you,
>>> take it into 2.12...
>>>
>>> It was already discussed, that "STOP" event is bad for tests. What about
>>> "RESUME"? How can we miss it? And sha256 mismatch is really something
>>> strange.
>>>
>>> Max, please check, do it fix 169 for you.
>>>
>>>   tests/qemu-iotests/169 | 44 +++++++++++++++++++++++---------------------
>>>   1 file changed, 23 insertions(+), 21 deletions(-)
>> This makes the test pass (thanks!), but it still leaves behind five cats...
>>
>> Max
>>
>>
> Hmm:
>
> jhuston  14772  0.0  0.0   4296   784 pts/3    S    16:12   0:00 cat
> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
> jhuston  14796  0.0  0.0   4296   764 pts/3    S    16:12   0:00 cat
> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
> jhuston  14940  0.0  0.0   4296   788 pts/3    S    16:12   0:00 cat
> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
> jhuston  14964  0.0  0.0   4296   720 pts/3    S    16:12   0:00 cat
> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
> jhuston  15052  0.0  0.0   4296   768 pts/3    S    16:12   0:00 cat
> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>
> Why do these get left behind? Nothing to consume the data...?

aha, understand. it is due to last vm_b.shutdown() and vm_b.launch in 
case of should_migrate. So, at the end of the test I restart vm_b with 
-incoming parameter. But it looks like  a bug anyway, If we start qemu 
with -incoming "exec", should not we kill cat process, if there were no 
migration?

-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
11.04.2018 12:02, Vladimir Sementsov-Ogievskiy wrote:
> 03.04.2018 23:13, John Snow wrote:
>>
>> On 04/03/2018 12:23 PM, Max Reitz wrote:
>>> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>>>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>>>> dirty-bitmaps capability for offline case.
>>>>
>>>> This (likely) fixes racy faults at least of the following types:
>>>>
>>>>      - timeout on waiting for RESUME event
>>>>      - sha256 mismatch on 136 (138 after this patch)
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>
>>>> This patch is a true change for the test anyway. But I don't 
>>>> understand,
>>>> why (and do really) it fixes the things. And I'm not sure about do we
>>>> really have a bug in bitmap migration or persistence. So, it's up 
>>>> to you,
>>>> take it into 2.12...
>>>>
>>>> It was already discussed, that "STOP" event is bad for tests. What 
>>>> about
>>>> "RESUME"? How can we miss it? And sha256 mismatch is really something
>>>> strange.
>>>>
>>>> Max, please check, do it fix 169 for you.
>>>>
>>>>   tests/qemu-iotests/169 | 44 
>>>> +++++++++++++++++++++++---------------------
>>>>   1 file changed, 23 insertions(+), 21 deletions(-)
>>> This makes the test pass (thanks!), but it still leaves behind five 
>>> cats...
>>>
>>> Max
>>>
>>>
>> Hmm:
>>
>> jhuston  14772  0.0  0.0   4296   784 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  14796  0.0  0.0   4296   764 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  14940  0.0  0.0   4296   788 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  14964  0.0  0.0   4296   720 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  15052  0.0  0.0   4296   768 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>
>> Why do these get left behind? Nothing to consume the data...?
>
> aha, understand. it is due to last vm_b.shutdown() and vm_b.launch in 
> case of should_migrate. So, at the end of the test I restart vm_b with 
> -incoming parameter. But it looks like  a bug anyway, If we start qemu 
> with -incoming "exec", should not we kill cat process, if there were 
> no migration?
>

third type of fail, without this patch:

+======================================================================
+ERROR: test__persistent__migbitmap__offline_shared 
(__main__.TestDirtyBitmapMigration)
+methodcaller(name, ...) --> methodcaller object
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "169", line 135, in do_test_migration
+    self.vm_b.launch()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", line 
221, in launch
+    self._launch()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", line 
244, in _launch
+    self._post_launch()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qtest.py", line 
100, in _post_launch
+    super(QEMUQtestMachine, self)._post_launch()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", line 
196, in _post_launch
+    self._qmp.accept()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
line 157, in accept
+    return self.__negotiate_capabilities()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
line 75, in __negotiate_capabilities
+    resp = self.cmd('qmp_capabilities')
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
line 191, in cmd
+    return self.cmd_obj(qmp_cmd)
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
line 174, in cmd_obj
+    resp = self.__json_read()
+  File 
"/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
line 82, in __json_read
+    data = self.__sockfile.readline()
+  File "/usr/lib64/python2.7/socket.py", line 447, in readline
+    data = self._sock.recv(self._rbufsize)
+error: [Errno 104] Connection reset by peer
+


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
11.04.2018 12:36, Vladimir Sementsov-Ogievskiy wrote:
> 11.04.2018 12:02, Vladimir Sementsov-Ogievskiy wrote:
>> 03.04.2018 23:13, John Snow wrote:
>>>
>>> On 04/03/2018 12:23 PM, Max Reitz wrote:
>>>> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>>>>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>>>>> dirty-bitmaps capability for offline case.
>>>>>
>>>>> This (likely) fixes racy faults at least of the following types:
>>>>>
>>>>>      - timeout on waiting for RESUME event
>>>>>      - sha256 mismatch on 136 (138 after this patch)
>>>>>
>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>>>> <vsementsov@virtuozzo.com>
>>>>> ---
>>>>>
>>>>> This patch is a true change for the test anyway. But I don't 
>>>>> understand,
>>>>> why (and do really) it fixes the things. And I'm not sure about do we
>>>>> really have a bug in bitmap migration or persistence. So, it's up 
>>>>> to you,
>>>>> take it into 2.12...
>>>>>
>>>>> It was already discussed, that "STOP" event is bad for tests. What 
>>>>> about
>>>>> "RESUME"? How can we miss it? And sha256 mismatch is really something
>>>>> strange.
>>>>>
>>>>> Max, please check, do it fix 169 for you.
>>>>>
>>>>>   tests/qemu-iotests/169 | 44 
>>>>> +++++++++++++++++++++++---------------------
>>>>>   1 file changed, 23 insertions(+), 21 deletions(-)
>>>> This makes the test pass (thanks!), but it still leaves behind five 
>>>> cats...
>>>>
>>>> Max
>>>>
>>>>
>>> Hmm:
>>>
>>> jhuston  14772  0.0  0.0   4296   784 pts/3    S    16:12 0:00 cat
>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>> jhuston  14796  0.0  0.0   4296   764 pts/3    S    16:12 0:00 cat
>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>> jhuston  14940  0.0  0.0   4296   788 pts/3    S    16:12 0:00 cat
>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>> jhuston  14964  0.0  0.0   4296   720 pts/3    S    16:12 0:00 cat
>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>> jhuston  15052  0.0  0.0   4296   768 pts/3    S    16:12 0:00 cat
>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>>
>>> Why do these get left behind? Nothing to consume the data...?
>>
>> aha, understand. it is due to last vm_b.shutdown() and vm_b.launch in 
>> case of should_migrate. So, at the end of the test I restart vm_b 
>> with -incoming parameter. But it looks like a bug anyway, If we start 
>> qemu with -incoming "exec", should not we kill cat process, if there 
>> were no migration?
>>
>
> third type of fail, without this patch:
>
> +======================================================================
> +ERROR: test__persistent__migbitmap__offline_shared 
> (__main__.TestDirtyBitmapMigration)
> +methodcaller(name, ...) --> methodcaller object
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "169", line 135, in do_test_migration
> +    self.vm_b.launch()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", line 
> 221, in launch
> +    self._launch()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", line 
> 244, in _launch
> +    self._post_launch()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qtest.py", 
> line 100, in _post_launch
> +    super(QEMUQtestMachine, self)._post_launch()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", line 
> 196, in _post_launch
> +    self._qmp.accept()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
> line 157, in accept
> +    return self.__negotiate_capabilities()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
> line 75, in __negotiate_capabilities
> +    resp = self.cmd('qmp_capabilities')
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
> line 191, in cmd
> +    return self.cmd_obj(qmp_cmd)
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
> line 174, in cmd_obj
> +    resp = self.__json_read()
> +  File 
> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
> line 82, in __json_read
> +    data = self.__sockfile.readline()
> +  File "/usr/lib64/python2.7/socket.py", line 447, in readline
> +    data = self._sock.recv(self._rbufsize)
> +error: [Errno 104] Connection reset by peer
> +
>
>

Hmm, first type? I'm now not sure about, did I really see sha256 
mismatch, or something like this (should be error, but found bitmap):

--- /work/src/qemu/up-169/tests/qemu-iotests/169.out    2018-04-11 
15:35:10.055027392 +0300
+++ /work/src/qemu/up-169/tests/qemu-iotests/169.out.bad 2018-04-11 
15:58:09.300450045 +0300
@@ -1,5 +1,20 @@
-........
+F.......
+======================================================================
+FAIL: test__not_persistent__migbitmap__offline 
(__main__.TestDirtyBitmapMigration)
+methodcaller(name, ...) --> methodcaller object
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "169", line 136, in do_test_migration
+    self.check_bitmap(self.vm_b, sha256 if persistent else False)
+  File "169", line 77, in check_bitmap
+    "Dirty bitmap 'bitmap0' not found");
+  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 389, 
in assert_qmp
+    result = self.dictpath(d, path)
+  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 348, 
in dictpath
+    self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
+AssertionError: failed path traversal for "error/desc" in "{u'return': 
{u'sha256': 
u'01d2ebedcb8f549a2547dbf8e231c410e3e747a9479e98909fc936e0035cf8b1'}}"


Max, did you really seed sha256 mismatch or only something like this?

-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
11.04.2018 16:05, Vladimir Sementsov-Ogievskiy wrote:
> 11.04.2018 12:36, Vladimir Sementsov-Ogievskiy wrote:
>> 11.04.2018 12:02, Vladimir Sementsov-Ogievskiy wrote:
>>> 03.04.2018 23:13, John Snow wrote:
>>>>
>>>> On 04/03/2018 12:23 PM, Max Reitz wrote:
>>>>> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>>>>>> dirty-bitmaps capability for offline case.
>>>>>>
>>>>>> This (likely) fixes racy faults at least of the following types:
>>>>>>
>>>>>>      - timeout on waiting for RESUME event
>>>>>>      - sha256 mismatch on 136 (138 after this patch)
>>>>>>
>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>>>>> <vsementsov@virtuozzo.com>
>>>>>> ---
>>>>>>
>>>>>> This patch is a true change for the test anyway. But I don't 
>>>>>> understand,
>>>>>> why (and do really) it fixes the things. And I'm not sure about 
>>>>>> do we
>>>>>> really have a bug in bitmap migration or persistence. So, it's up 
>>>>>> to you,
>>>>>> take it into 2.12...
>>>>>>
>>>>>> It was already discussed, that "STOP" event is bad for tests. 
>>>>>> What about
>>>>>> "RESUME"? How can we miss it? And sha256 mismatch is really 
>>>>>> something
>>>>>> strange.
>>>>>>
>>>>>> Max, please check, do it fix 169 for you.
>>>>>>
>>>>>>   tests/qemu-iotests/169 | 44 
>>>>>> +++++++++++++++++++++++---------------------
>>>>>>   1 file changed, 23 insertions(+), 21 deletions(-)
>>>>> This makes the test pass (thanks!), but it still leaves behind 
>>>>> five cats...
>>>>>
>>>>> Max
>>>>>
>>>>>
>>>> Hmm:
>>>>
>>>> jhuston  14772  0.0  0.0   4296   784 pts/3    S    16:12 0:00 cat
>>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>>> jhuston  14796  0.0  0.0   4296   764 pts/3    S    16:12 0:00 cat
>>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>>> jhuston  14940  0.0  0.0   4296   788 pts/3    S    16:12 0:00 cat
>>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>>> jhuston  14964  0.0  0.0   4296   720 pts/3    S    16:12 0:00 cat
>>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>>> jhuston  15052  0.0  0.0   4296   768 pts/3    S    16:12 0:00 cat
>>>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>>>
>>>> Why do these get left behind? Nothing to consume the data...?
>>>
>>> aha, understand. it is due to last vm_b.shutdown() and vm_b.launch 
>>> in case of should_migrate. So, at the end of the test I restart vm_b 
>>> with -incoming parameter. But it looks like a bug anyway, If we 
>>> start qemu with -incoming "exec", should not we kill cat process, if 
>>> there were no migration?
>>>
>>
>> third type of fail, without this patch:
>>
>> +======================================================================
>> +ERROR: test__persistent__migbitmap__offline_shared 
>> (__main__.TestDirtyBitmapMigration)
>> +methodcaller(name, ...) --> methodcaller object
>> +----------------------------------------------------------------------
>> +Traceback (most recent call last):
>> +  File "169", line 135, in do_test_migration
>> +    self.vm_b.launch()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", 
>> line 221, in launch
>> +    self._launch()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", 
>> line 244, in _launch
>> +    self._post_launch()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qtest.py", 
>> line 100, in _post_launch
>> +    super(QEMUQtestMachine, self)._post_launch()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qemu.py", 
>> line 196, in _post_launch
>> +    self._qmp.accept()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
>> line 157, in accept
>> +    return self.__negotiate_capabilities()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
>> line 75, in __negotiate_capabilities
>> +    resp = self.cmd('qmp_capabilities')
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
>> line 191, in cmd
>> +    return self.cmd_obj(qmp_cmd)
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
>> line 174, in cmd_obj
>> +    resp = self.__json_read()
>> +  File 
>> "/work/src/qemu/up-169/tests/qemu-iotests/../../scripts/qmp/qmp.py", 
>> line 82, in __json_read
>> +    data = self.__sockfile.readline()
>> +  File "/usr/lib64/python2.7/socket.py", line 447, in readline
>> +    data = self._sock.recv(self._rbufsize)
>> +error: [Errno 104] Connection reset by peer
>> +
>>
>>
>
> Hmm, first type? I'm now not sure about, did I really see sha256 
> mismatch, or something like this (should be error, but found bitmap):
>
> --- /work/src/qemu/up-169/tests/qemu-iotests/169.out    2018-04-11 
> 15:35:10.055027392 +0300
> +++ /work/src/qemu/up-169/tests/qemu-iotests/169.out.bad 2018-04-11 
> 15:58:09.300450045 +0300
> @@ -1,5 +1,20 @@
> -........
> +F.......
> +======================================================================
> +FAIL: test__not_persistent__migbitmap__offline 
> (__main__.TestDirtyBitmapMigration)
> +methodcaller(name, ...) --> methodcaller object
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "169", line 136, in do_test_migration
> +    self.check_bitmap(self.vm_b, sha256 if persistent else False)
> +  File "169", line 77, in check_bitmap
> +    "Dirty bitmap 'bitmap0' not found");
> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 
> 389, in assert_qmp
> +    result = self.dictpath(d, path)
> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 
> 348, in dictpath
> +    self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
> +AssertionError: failed path traversal for "error/desc" in 
> "{u'return': {u'sha256': 
> u'01d2ebedcb8f549a2547dbf8e231c410e3e747a9479e98909fc936e0035cf8b1'}}"

this looks absolutely impossible. not-persistent case. Where from we get 
the bitmap after vm stop/start?? Ohh I can imagine. It's offline case, 
and we remember that I forget to drop -incoming for vm b, so it loads 
same incoming migration file.. then, the question is why it don't fail 
always.. Aha! it may be because I don't wait for RESUME.

>
>
> Max, did you really seed sha256 mismatch or only something like this?
>


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Max Reitz 6 years ago
On 2018-04-11 15:05, Vladimir Sementsov-Ogievskiy wrote:

[...]

> Hmm, first type? I'm now not sure about, did I really see sha256
> mismatch, or something like this (should be error, but found bitmap):
> 
> --- /work/src/qemu/up-169/tests/qemu-iotests/169.out    2018-04-11
> 15:35:10.055027392 +0300
> +++ /work/src/qemu/up-169/tests/qemu-iotests/169.out.bad 2018-04-11
> 15:58:09.300450045 +0300
> @@ -1,5 +1,20 @@
> -........
> +F.......
> +======================================================================
> +FAIL: test__not_persistent__migbitmap__offline
> (__main__.TestDirtyBitmapMigration)
> +methodcaller(name, ...) --> methodcaller object
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "169", line 136, in do_test_migration
> +    self.check_bitmap(self.vm_b, sha256 if persistent else False)
> +  File "169", line 77, in check_bitmap
> +    "Dirty bitmap 'bitmap0' not found");
> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 389,
> in assert_qmp
> +    result = self.dictpath(d, path)
> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 348,
> in dictpath
> +    self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
> +AssertionError: failed path traversal for "error/desc" in "{u'return':
> {u'sha256':
> u'01d2ebedcb8f549a2547dbf8e231c410e3e747a9479e98909fc936e0035cf8b1'}}"
> 
> 
> Max, did you really seed sha256 mismatch or only something like this?

I'm pretty sure I did see mismatches.

Max

Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
11.04.2018 19:11, Max Reitz wrote:
> On 2018-04-11 15:05, Vladimir Sementsov-Ogievskiy wrote:
>
> [...]
>
>> Hmm, first type? I'm now not sure about, did I really see sha256
>> mismatch, or something like this (should be error, but found bitmap):
>>
>> --- /work/src/qemu/up-169/tests/qemu-iotests/169.out    2018-04-11
>> 15:35:10.055027392 +0300
>> +++ /work/src/qemu/up-169/tests/qemu-iotests/169.out.bad 2018-04-11
>> 15:58:09.300450045 +0300
>> @@ -1,5 +1,20 @@
>> -........
>> +F.......
>> +======================================================================
>> +FAIL: test__not_persistent__migbitmap__offline
>> (__main__.TestDirtyBitmapMigration)
>> +methodcaller(name, ...) --> methodcaller object
>> +----------------------------------------------------------------------
>> +Traceback (most recent call last):
>> +  File "169", line 136, in do_test_migration
>> +    self.check_bitmap(self.vm_b, sha256 if persistent else False)
>> +  File "169", line 77, in check_bitmap
>> +    "Dirty bitmap 'bitmap0' not found");
>> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 389,
>> in assert_qmp
>> +    result = self.dictpath(d, path)
>> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 348,
>> in dictpath
>> +    self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
>> +AssertionError: failed path traversal for "error/desc" in "{u'return':
>> {u'sha256':
>> u'01d2ebedcb8f549a2547dbf8e231c410e3e747a9479e98909fc936e0035cf8b1'}}"
>>
>>
>> Max, did you really seed sha256 mismatch or only something like this?
> I'm pretty sure I did see mismatches.

hm, may be it possible too, because of not waiting for RESUME in this 
case, we can request sha256 in the intermediate state of bitmap loading

>
> Max


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
12.04.2018 11:34, Vladimir Sementsov-Ogievskiy wrote:
> 11.04.2018 19:11, Max Reitz wrote:
>> On 2018-04-11 15:05, Vladimir Sementsov-Ogievskiy wrote:
>>
>> [...]
>>
>>> Hmm, first type? I'm now not sure about, did I really see sha256
>>> mismatch, or something like this (should be error, but found bitmap):
>>>
>>> --- /work/src/qemu/up-169/tests/qemu-iotests/169.out 2018-04-11
>>> 15:35:10.055027392 +0300
>>> +++ /work/src/qemu/up-169/tests/qemu-iotests/169.out.bad 2018-04-11
>>> 15:58:09.300450045 +0300
>>> @@ -1,5 +1,20 @@
>>> -........
>>> +F.......
>>> +======================================================================
>>> +FAIL: test__not_persistent__migbitmap__offline
>>> (__main__.TestDirtyBitmapMigration)
>>> +methodcaller(name, ...) --> methodcaller object
>>> +----------------------------------------------------------------------
>>> +Traceback (most recent call last):
>>> +  File "169", line 136, in do_test_migration
>>> +    self.check_bitmap(self.vm_b, sha256 if persistent else False)
>>> +  File "169", line 77, in check_bitmap
>>> +    "Dirty bitmap 'bitmap0' not found");
>>> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 
>>> 389,
>>> in assert_qmp
>>> +    result = self.dictpath(d, path)
>>> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line 
>>> 348,
>>> in dictpath
>>> +    self.fail('failed path traversal for "%s" in "%s"' % (path, 
>>> str(d)))
>>> +AssertionError: failed path traversal for "error/desc" in "{u'return':
>>> {u'sha256':
>>> u'01d2ebedcb8f549a2547dbf8e231c410e3e747a9479e98909fc936e0035cf8b1'}}"
>>>
>>>
>>> Max, did you really seed sha256 mismatch or only something like this?
>> I'm pretty sure I did see mismatches.
>
> hm, may be it possible too, because of not waiting for RESUME in this 
> case, we can request sha256 in the intermediate state of bitmap loading

no. if you see mismatch on first check after migration, then it is after 
RESUME event, so it means, migrated broken bitmap, it's a bug..

if you see mismatch on second check - after vm_b stop/start, this means 
that it is a persistent case, so incoming migration should fail, and 
bitmap is loaded from file, but how it can mismatch? persistance bug?


>
>>
>> Max
>
>


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Max Reitz 6 years ago
On 2018-04-12 11:09, Vladimir Sementsov-Ogievskiy wrote:
> 12.04.2018 11:34, Vladimir Sementsov-Ogievskiy wrote:
>> 11.04.2018 19:11, Max Reitz wrote:
>>> On 2018-04-11 15:05, Vladimir Sementsov-Ogievskiy wrote:
>>>
>>> [...]
>>>
>>>> Hmm, first type? I'm now not sure about, did I really see sha256
>>>> mismatch, or something like this (should be error, but found bitmap):
>>>>
>>>> --- /work/src/qemu/up-169/tests/qemu-iotests/169.out 2018-04-11
>>>> 15:35:10.055027392 +0300
>>>> +++ /work/src/qemu/up-169/tests/qemu-iotests/169.out.bad 2018-04-11
>>>> 15:58:09.300450045 +0300
>>>> @@ -1,5 +1,20 @@
>>>> -........
>>>> +F.......
>>>> +======================================================================
>>>> +FAIL: test__not_persistent__migbitmap__offline
>>>> (__main__.TestDirtyBitmapMigration)
>>>> +methodcaller(name, ...) --> methodcaller object
>>>> +----------------------------------------------------------------------
>>>> +Traceback (most recent call last):
>>>> +  File "169", line 136, in do_test_migration
>>>> +    self.check_bitmap(self.vm_b, sha256 if persistent else False)
>>>> +  File "169", line 77, in check_bitmap
>>>> +    "Dirty bitmap 'bitmap0' not found");
>>>> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line
>>>> 389,
>>>> in assert_qmp
>>>> +    result = self.dictpath(d, path)
>>>> +  File "/work/src/qemu/up-169/tests/qemu-iotests/iotests.py", line
>>>> 348,
>>>> in dictpath
>>>> +    self.fail('failed path traversal for "%s" in "%s"' % (path,
>>>> str(d)))
>>>> +AssertionError: failed path traversal for "error/desc" in "{u'return':
>>>> {u'sha256':
>>>> u'01d2ebedcb8f549a2547dbf8e231c410e3e747a9479e98909fc936e0035cf8b1'}}"
>>>>
>>>>
>>>> Max, did you really seed sha256 mismatch or only something like this?
>>> I'm pretty sure I did see mismatches.
>>
>> hm, may be it possible too, because of not waiting for RESUME in this
>> case, we can request sha256 in the intermediate state of bitmap loading
> 
> no. if you see mismatch on first check after migration, then it is after
> RESUME event, so it means, migrated broken bitmap, it's a bug..
> 
> if you see mismatch on second check - after vm_b stop/start, this means
> that it is a persistent case, so incoming migration should fail, and
> bitmap is loaded from file, but how it can mismatch? persistance bug?

Well, I'll tell you if I ever see it again.  If not... Then I guess
we're fine.

Max

Re: [Qemu-devel] [PATCH] iotests: fix 169
Posted by Max Reitz 6 years ago
On 2018-04-11 11:02, Vladimir Sementsov-Ogievskiy wrote:
> 03.04.2018 23:13, John Snow wrote:
>>
>> On 04/03/2018 12:23 PM, Max Reitz wrote:
>>> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>>>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>>>> dirty-bitmaps capability for offline case.
>>>>
>>>> This (likely) fixes racy faults at least of the following types:
>>>>
>>>>      - timeout on waiting for RESUME event
>>>>      - sha256 mismatch on 136 (138 after this patch)
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>
>>>> This patch is a true change for the test anyway. But I don't
>>>> understand,
>>>> why (and do really) it fixes the things. And I'm not sure about do we
>>>> really have a bug in bitmap migration or persistence. So, it's up to
>>>> you,
>>>> take it into 2.12...
>>>>
>>>> It was already discussed, that "STOP" event is bad for tests. What
>>>> about
>>>> "RESUME"? How can we miss it? And sha256 mismatch is really something
>>>> strange.
>>>>
>>>> Max, please check, do it fix 169 for you.
>>>>
>>>>   tests/qemu-iotests/169 | 44
>>>> +++++++++++++++++++++++---------------------
>>>>   1 file changed, 23 insertions(+), 21 deletions(-)
>>> This makes the test pass (thanks!), but it still leaves behind five
>>> cats...
>>>
>>> Max
>>>
>>>
>> Hmm:
>>
>> jhuston  14772  0.0  0.0   4296   784 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  14796  0.0  0.0   4296   764 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  14940  0.0  0.0   4296   788 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  14964  0.0  0.0   4296   720 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>> jhuston  15052  0.0  0.0   4296   768 pts/3    S    16:12   0:00 cat
>> /home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
>>
>> Why do these get left behind? Nothing to consume the data...?
> 
> aha, understand. it is due to last vm_b.shutdown() and vm_b.launch in
> case of should_migrate. So, at the end of the test I restart vm_b with
> -incoming parameter. But it looks like  a bug anyway, If we start qemu
> with -incoming "exec", should not we kill cat process, if there were no
> migration?

I agree, but it's your choice whether you want to fix that bug or just
change the test slightly -- I'm responsible for the iotests, but not for
migration, so I have to admit I don't mind just changing this test to
accomodate. O:-)

It appears that just removing the mig_file before the second
vm_b.launch() is sufficient (and enclosing the removal of that file in
tearDown() in a try/except block).  I suppose the cat process will
complain, but that doesn't stop the test from passing.  If you want to
be really nice, I suppose you could just overwrite the FIFO mig_file
with an empty regular file, but I don't think it's actually necessary...

Max