[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 7/7] iotests: add iotest 236 for testing bitm
Re: [Qemu-devel] [PATCH v2 7/7] iotests: add iotest 236 for testing bitmap merge
Thu, 13 Dec 2018 13:38:32 -0500
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1
On 12/13/18 8:50 AM, Vladimir Sementsov-Ogievskiy wrote:
> 13.12.2018 4:50, John Snow wrote:
>> New interface, new smoke test.
>> Signed-off-by: John Snow <address@hidden>
>> tests/qemu-iotests/236 | 123 +++++++++++++++++
>> tests/qemu-iotests/236.out | 265 +++++++++++++++++++++++++++++++++++++
>> tests/qemu-iotests/group | 1 +
>> 3 files changed, 389 insertions(+)
>> create mode 100755 tests/qemu-iotests/236
>> create mode 100644 tests/qemu-iotests/236.out
>> diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
>> new file mode 100755
>> index 0000000000..3d162e967b
>> --- /dev/null
>> +++ b/tests/qemu-iotests/236
>> @@ -0,0 +1,123 @@
>> +#!/usr/bin/env python
>> +# Test bitmap merges.
>> +# Copyright (c) 2018 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/>.
>> +# address@hidden
>> +import sys
>> +import os
>> +import iotests
>> +from iotests import log
>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
>> +from qemu import QEMUMachine
> unused, with previous line and, therefore, os and sys modules)
Forgive the copy and paste. I'll trim it down.
>> +patterns = [("0x5d", "0", "64k"),
>> + ("0xd5", "1M", "64k"),
>> + ("0xdc", "32M", "64k"),
>> + ("0xcd", "0x3ff0000", "64k")] # 64M - 64K
>> +overwrite = [("0xab", "0", "64k"), # Full overwrite
>> + ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
>> + ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
>> + ("0xea", "0x3fe0000", "64k")] # Adjacent-left (64M - 128K)
>> +with iotests.FilePath('img') as img_path, \
>> + iotests.VM() as vm:
>> + log('--- Preparing image & VM ---\n')
>> + iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
> hm, actually null device is enough here.
>> + vm.add_drive(img_path)
>> + vm.launch()
>> + log('--- Adding preliminary bitmaps A & B ---\n')
>> + vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapA")
>> + vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapB")
>> + # Dirties 4 clusters. count=262144 > + log('')
>> + log('--- Emulating writes ---\n')
>> + for p in patterns:
>> + cmd = "write -P%s %s %s" % p
>> + log(cmd)
>> + log(vm.hmp_qemu_io("drive0", cmd))
>> + vm.qmp_log("query-block", indent=2,
>> + filters=[iotests.filter_generated_node_ids,
>> + iotests.filter_testfiles])
> I'm against. query-block prints a lot of unrelated things, which may change
> version to version (for example, Andrey is now adding bitmap information to
> format-specific info), then, backported test may fail for previous versions
> (or just
> different config) because of something absolutely unrelated to bitmaps.
You have a point. I'm only interested in the bitmap info structures, here.
> I think, it should be shortened to result['device'],
OK, let me look at printing something like
to the log.
> And, I think, any way it would be good to create a separate helper for
> current state of bitmaps (which may output their sha256 too)
Yeah, I was working on a command to do just this but I ran into troubles
with caching the bitmap info for computing it and wound up shelving the
series and I got sidetracked on other issues...
>> + log('')
>> + log('--- Disabling B & Adding C ---\n')
> why not just
> log('\n--- Disabling B & Adding C ---\n')
Just preference, there's no reason.
I have a habit of avoiding pre-pending control characters to string
lines, but don't have an aversion to appending them.
I can change it if it sticks out as too unusual.
> Otherwise, the test looks fine for me
Thanks for looking!
- [Qemu-devel] [PATCH v2 5/7] iotests: add filter_generated_node_ids, (continued)