Re: [Qemu-devel] [PATCH 16/17] iotests: add dirty bitmap migration test

[Max Reitz]

On 03.02.2017 12:08, Vladimir Sementsov-Ogievskiy wrote:
> 03.02.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> 02.02.2017 02:12, Max Reitz wrote:
>>> On 22.11.2016 18:54, Vladimir Sementsov-Ogievskiy wrote:
>>>> The test starts two vms (vm_a, vm_b), create dirty bitmap in
>>>> the first one, do several writes to corresponding device and
>>>> then migrate vm_a to vm_b with dirty bitmaps.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>> ---
>>>>   tests/qemu-iotests/169     | 86
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>>   tests/qemu-iotests/169.out |  5 +++
>>>>   tests/qemu-iotests/group   |  1 +
>>>>   3 files changed, 92 insertions(+)
>>>>   create mode 100755 tests/qemu-iotests/169
>>>>   create mode 100644 tests/qemu-iotests/169.out
>>>>
>>>> diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
>>>> new file mode 100755
>>>> index 0000000..58b1ab1
>>>> --- /dev/null
>>>> +++ b/tests/qemu-iotests/169
>>>> @@ -0,0 +1,86 @@
>>>> +#!/usr/bin/env python
>>>> +#
>>>> +# Tests for dirty bitmaps migration.
>>>> +#
>>>> +# Copyright (C) Vladimir Sementsov-Ogievskiy 2015-2016
>>>> +#
>>>> +# 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/>.
>>>> +#
>>>> +
>>>> +import os
>>>> +import iotests
>>>> +import time
>>>> +from iotests import qemu_img
>>>> +
>>>> +disk_a = os.path.join(iotests.test_dir, 'disk_a')
>>>> +disk_b = os.path.join(iotests.test_dir, 'disk_b')
>>>> +fifo = os.path.join(iotests.test_dir, 'mig_fifo')
>>>> +
>>>> +class TestDirtyBitmapMigration(iotests.QMPTestCase):
>>>> +
>>>> +    def setUp(self):
>>>> +        size = 0x400000000 # 1G
>>> I can already see someone on macOS complaining that this test eats their
>>> hard drive when running it for raw images... But we can still amend it
>>> once somebody complains. O:-)
>>>
>>> I think 1 GB is not overly large, but it's reasonable to use such a
>>> rather large value for this test.
>>>
>>>> +        os.mkfifo(fifo)
>>>> +        qemu_img('create', '-f', iotests.imgfmt, disk_a, str(size))
>>>> +        qemu_img('create', '-f', iotests.imgfmt, disk_b, str(size))
>>>> +        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a)
>>>> +        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk_b)
>>>> +        self.vm_b.add_incoming_migration("exec: cat " + fifo)
>>> I guess this will break if the path contains white space. Is there
>>> something we can do about that?
>>>
>>> (You don't need to protect against quotes or something, just white
>>> space.)
>>>
>>>> +        self.vm_a.launch()
>>>> +        self.vm_b.launch()
>>>> +
>>>> +    def tearDown(self):
>>>> +        self.vm_a.shutdown()
>>>> +        self.vm_b.shutdown()
>>>> +        os.remove(disk_a)
>>>> +        os.remove(disk_b)
>>>> +        os.remove(fifo)
>>>> +
>>>> +    def test_migration(self):
>>>> +        granularity = 512
>>>> +        regions = [
>>>> +            { 'start': 0,           'count': 0x100000 },
>>>> +            { 'start': 0x100000000, 'count': 0x200000  },
>>>> +            { 'start': 0x399900000, 'count': 0x100000  }
>>>> +            ]
>>>> +
>>>> +        result = self.vm_a.qmp('block-dirty-bitmap-add',
>>>> node='drive0',
>>>> +                               name='bitmap', granularity=granularity)
>>>> +        self.assert_qmp(result, 'return', {});
>>>> +
>>>> +        for r in regions:
>>>> +          self.vm_a.hmp_qemu_io('drive0',
>>> This line isn't aligned to four spaces.
>>>
>>> With that fixed, and if you deem fixing the white space issue too
>>> complicated:
>>
>> may be, just add quotes, like
>>
>> self.vm_b.add_incoming_migration("exec: cat '" + fifo + "'")
>>
>>
>> ?
> 
> looks like ./check doesn't work with spaces anyway:
> 
> address@hidden qemu-iotests]# ./check -qcow2 -nocache 169
> QEMU          -- "/work/src/post
> copy/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64"
> -nodefaults -machine accel=qtest
> QEMU_IMG      -- "/work/src/post copy/tests/qemu-iotests/../../qemu-img"
> QEMU_IO       -- "/work/src/post copy/tests/qemu-iotests/../../qemu-io" 
> -f qcow2 --cache none
> QEMU_NBD      -- "/work/src/post copy/tests/qemu-iotests/../../qemu-nbd"
> IMGFMT        -- qcow2 (compat=1.1)
> IMGPROTO      -- file
> PLATFORM      -- Linux/x86_64 kvm 3.10.0-327.36.1.vz7.20.9
> TEST_DIR      -- /work/src/post copy/tests/qemu-iotests/scratch
> SOCKET_SCM_HELPER -- /work/src/post
> copy/tests/qemu-iotests/socket_scm_helper
> 
> 169 33s ..../check: line 306: $tmp.out: ambiguous redirect
>  [failed, exit status 1] - output mismatch (see 169.out.bad)
> mv: target '169.out.bad' is not a directory
> diff: 169.out.bad: No such file or directory
> ./check: line 158: [: too many arguments
> ./check: line 171: [: /work/src/post: binary operator expected
> Failures: 169
> Failed 1 of 1 tests
> address@hidden qemu-iotests]# pwd
> /work/src/post copy/tests/qemu-iotests

Hm, too bad. I remember someone once fixing everything, but apparently
it broke again...

Nonetheless, I'd rather not introduce new issues into newly added code.
Perhaps some day someone can once again fix all of the current issues...

Max

signature.asc
Description: OpenPGP digital signature