Re: [PATCH v14 06/13] iotests: add #310 to test bottom node in COR driver

From: Max Reitz <mreitz@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: fam@euphon.net, kwolf@redhat.com, qemu-devel@nongnu.org,
	armbru@redhat.com, stefanha@redhat.com,
	andrey.shinkevich@virtuozzo.com, den@openvz.org,
	jsnow@redhat.com
Subject: Re: [PATCH v14 06/13] iotests: add #310 to test bottom node in COR driver
Date: Fri, 11 Dec 2020 14:24:06 +0100	[thread overview]
Message-ID: <c124518a-1499-6fbd-c47a-fa279d56d326@redhat.com> (raw)
In-Reply-To: <31cc63f1-1e55-22fd-edff-2a815f690f11@virtuozzo.com>

On 11.12.20 14:10, Vladimir Sementsov-Ogievskiy wrote:
> 11.12.2020 15:49, Max Reitz wrote:
>> On 04.12.20 23:07, Vladimir Sementsov-Ogievskiy wrote:
>>> From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>>
>>> The test case #310 is similar to #216 by Max Reitz. The difference is
>>> that the test #310 involves a bottom node to the COR filter driver.
>>>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/310     | 114 +++++++++++++++++++++++++++++++++++++
>>>   tests/qemu-iotests/310.out |  15 +++++
>>>   tests/qemu-iotests/group   |   1 +
>>>   3 files changed, 130 insertions(+)
>>>   create mode 100755 tests/qemu-iotests/310
>>>   create mode 100644 tests/qemu-iotests/310.out
>>>
>>> diff --git a/tests/qemu-iotests/310 b/tests/qemu-iotests/310
>>> new file mode 100755
>>> index 0000000000..c8b34cd887
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/310
>>> @@ -0,0 +1,114 @@
>>> +#!/usr/bin/env python3
>>> +#
>>> +# Copy-on-read tests using a COR filter with a bottom node
>>> +#
>>> +# Copyright (C) 2018 Red Hat, Inc.
>>> +# Copyright (c) 2020 Virtuozzo International GmbH
>>> +#
>>> +# 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 iotests
>>> +from iotests import log, qemu_img, qemu_io_silent
>>> +
>>> +# Need backing file support
>>> +iotests.script_initialize(supported_fmts=['qcow2', 'qcow', 'qed', 
>>> 'vmdk'],
>>> +                          supported_platforms=['linux'])
>>> +
>>> +log('')
>>> +log('=== Copy-on-read across nodes ===')
>>> +log('')
>>> +
>>> +# This test is similar to the 216 one by Max Reitz <mreitz@redhat.com>
>>> +# The difference is that this test case involves a bottom node to the
>>> +# COR filter driver.
>>> +
>>> +with iotests.FilePath('base.img') as base_img_path, \
>>> +     iotests.FilePath('mid.img') as mid_img_path, \
>>> +     iotests.FilePath('top.img') as top_img_path, \
>>> +     iotests.VM() as vm:
>>> +
>>> +    log('--- Setting up images ---')
>>> +    log('')
>>> +
>>> +    assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, 
>>> '64M') == 0
>>> +    assert qemu_io_silent(base_img_path, '-c', 'write -P 1 0M 1M') == 0
>>> +    assert qemu_io_silent(base_img_path, '-c', 'write -P 1 3M 1M') == 0
>>> +    assert qemu_img('create', '-f', iotests.imgfmt, '-b', 
>>> base_img_path,
>>> +                    '-F', iotests.imgfmt, mid_img_path) == 0
>>> +    assert qemu_io_silent(mid_img_path,  '-c', 'write -P 3 2M 1M') == 0
>>> +    assert qemu_io_silent(mid_img_path,  '-c', 'write -P 3 4M 1M') == 0
>>> +    assert qemu_img('create', '-f', iotests.imgfmt, '-b', mid_img_path,
>>> +                    '-F', iotests.imgfmt, top_img_path) == 0
>>> +    assert qemu_io_silent(top_img_path,  '-c', 'write -P 2 1M 1M') == 0
>>> +
>>> +#      0 1 2 3 4
>>> +# top    2
>>> +# mid      3   3
>>> +# base 1     1
>>> +
>>> +    log('Done')
>>> +
>>> +    log('')
>>> +    log('--- Doing COR ---')
>>> +    log('')
>>> +
>>> +    vm.launch()
>>> +
>>> +    log(vm.qmp('blockdev-add',
>>> +               node_name='node0',
>>> +               driver='copy-on-read',
>>> +               bottom='node2',
>>> +               file={
>>> +                   'driver': iotests.imgfmt,
>>> +                   'file': {
>>> +                       'driver': 'file',
>>> +                       'filename': top_img_path
>>> +                   },
>>> +                   'backing': {
>>> +                       'node-name': 'node2',
>>> +                       'driver': iotests.imgfmt,
>>> +                       'file': {
>>> +                           'driver': 'file',
>>> +                           'filename': mid_img_path
>>> +                       },
>>> +                       'backing': {
>>> +                           'driver': iotests.imgfmt,
>>> +                           'file': {
>>> +                               'driver': 'file',
>>> +                               'filename': base_img_path
>>> +                           }
>>> +                       },
>>> +                   }
>>> +               }))
>>> +
>>> +    # Trigger COR
>>> +    log(vm.qmp('human-monitor-command',
>>> +               command_line='qemu-io node0 "read 0 5M"'))
>>> +
>>> +    vm.shutdown()
>>> +
>>> +    log('')
>>> +    log('--- Checking COR result ---')
>>> +    log('')
>>> +
>>> +    assert qemu_io_silent(base_img_path, '-c', 'discard 0 4M') == 0
>>> +    assert qemu_io_silent(mid_img_path, '-c', 'discard 0M 5M') == 0
>>
>> The data discard leaves behind is undefined, so this may not result in 
>> zeroes.  (In fact, the test does fail for me with vmdk, qed, and 
>> qcow.)  'write -z' would work better, although perhaps you 
>> intentionally chose discard to just drop the data from the backing 
>> images.
>>
>> In that case, you could also recreate the middle image, so it’s empty 
>> then – the only problem with that is that it’ll break VMDK because it 
>> stores this reference to its backing image, and if the backing image 
>> is changed, you’ll get EINVAL when falling back to it...
>>
>> (The same goes for overwriting any data in the backing image, though, 
>> be it with discard, write -z, or write -P 0.  So I suppose VMDK just 
>> won’t work with this test.)
>>
> 
> I think the goal is just to be sure the following reads read from the 
> top and check exactly that COR works.
> 
> So we can just use 'write -z'.. Or, we can changed the backing file of 
> top_img to nothing instead. Can qemu-img do it?

Yes, with rebase -u -b ''.  (I think I tested that, and that too didn’t 
work for all formats, though...?)

Max