Re: [PATCH v12 01/11] iotests: add test for QCOW2 header dump

[Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>]

On 05.08.2020 14:23, Vladimir Sementsov-Ogievskiy wrote:
> 30.07.2020 17:15, Andrey Shinkevich wrote:
>> The simple script creates a QCOW2 image and fills it with some data.
>> Two bitmaps are created as well. Then the script reads the image header
>> with extensions from the disk by running the script qcow2.py and dumps
>> the information to the output. Other entities, such as snapshots, may
>> be added to the test later.
>>
>> Suggested-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/303     | 59 
>> ++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/303.out | 64 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 124 insertions(+)
>>   create mode 100755 tests/qemu-iotests/303
>>   create mode 100644 tests/qemu-iotests/303.out
>>
>> diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
>> new file mode 100755
>> index 0000000..3c7a611
>> --- /dev/null
>> +++ b/tests/qemu-iotests/303
>> @@ -0,0 +1,59 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Test for dumping of qcow2 image metadata
>> +#
>> +# 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
>> +import subprocess
>> +from iotests import qemu_img_create, qemu_io, file_path, log, 
>> filter_qemu_io
>> +
>> +iotests.script_initialize(supported_fmts=['qcow2'])
>> +
>> +disk = file_path('disk')
>> +chunk = 1024 * 1024
>> +
>> +
>> +def create_bitmap(bitmap_number, disabled):
>> +    granularity = 1 << (14 + bitmap_number)
>> +    bitmap_name = 'bitmap-' + str(bitmap_number)
>> +    vm = iotests.VM().add_drive(disk)
>> +    vm.launch()
>> +    vm.qmp_log('block-dirty-bitmap-add', node='drive0', 
>> name=bitmap_name,
>> +               granularity=granularity, persistent=True, 
>> disabled=disabled)
>> +    vm.shutdown()
>> +
>> +
>> +def write_to_disk(offset, size):
>> +    write = f'write {offset} {size}'
>> +    log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
>> +
>> +
>> +def add_bitmap(num, begin, end, disabled):
>> +    log(f'Add bitmap {num}')
>> +    create_bitmap(num, disabled)
>> +    for i in range(begin, end):
>> +        write_to_disk((i-1) * chunk, chunk)
>
> a bit unusual to count chunks starting from "1"..
>
> also, any difference with just
>
> write_to_disk((i-1) * chunk, (end-begin) * chunk)
>
> ?
>

QEMU-IMG limits the maximum number of bytes written with one command by 
... 2M, if I am not wrong.

Andrey


>> +    log('')
>> +
>> +
>> +qemu_img_create('-f', iotests.imgfmt, disk, '10M')
>> +
>> +add_bitmap(1, 1, 7, False)
>> +add_bitmap(2, 7, 9, True)
>> +dump = ['qcow2.py', f'{disk}', 'dump-header']
>
> No reason to put disk into f-string, it's a string anyway: f'{disk}' 
> is equal to just disk.


Thanks


>
>> +subprocess.run(dump)
>
>
> And you may use just
>
>    subprocess.run(['qcow2.py', disk, 'dump-header'])
>
> without additional variable.


Yes, but further adding the '-j' key to the list looks more elegant to me ))

Andrey


>
>
> Still, I'm OK with it as is:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
>