Re: [PATCH qemu 4/4] iotests: add test for bitmap mirror

2020-10-13 Thread Max Reitz 
On 02.10.20 10:23, Fabian Grünbichler wrote:
> On October 1, 2020 7:31 pm, Max Reitz wrote:
>> On 22.09.20 11:14, Fabian Grünbichler wrote:
>>> heavily based on/practically forked off iotest 257 for bitmap backups,
>>> but:
>>>
>>> - no writes to filter node 'mirror-top' between completion and
>>> finalization, as those seem to deadlock?
>>> - no inclusion of not-yet-available full/top sync modes in combination
>>> with bitmaps
>>> - extra set of reference/test mirrors to verify that writes in parallel
>>> with active mirror work
>>>
>>> intentionally keeping copyright and ownership of original test case to
>>> honor provenance.
>>>
>>> Signed-off-by: Fabian Grünbichler 
>>> ---
>>>  tests/qemu-iotests/306 |  546 +++
>>>  tests/qemu-iotests/306.out | 2846 
>>>  tests/qemu-iotests/group   |1 +
>>>  3 files changed, 3393 insertions(+)
>>>  create mode 100755 tests/qemu-iotests/306
>>>  create mode 100644 tests/qemu-iotests/306.out
>>>
>>> diff --git a/tests/qemu-iotests/306 b/tests/qemu-iotests/306
>>> new file mode 100755
>>> index 00..1bb8bd4138
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/306
>>
>> [...]
>>
>>> +def test_bitmap_sync(bsync_mode, msync_mode='bitmap', failure=None):
>>> +"""
>>> +Test bitmap mirror routines.
>>> +
>>> +:param bsync_mode: Is the Bitmap Sync mode, and can be any of:
>>> +- on-success: This is the "incremental" style mode. Bitmaps are
>>> +  synchronized to what was copied out only on success.
>>> +  (Partial images must be discarded.)
>>> +- never:  This is the "differential" style mode.
>>> +  Bitmaps are never synchronized.
>>> +- always: This is a "best effort" style mode.
>>> +  Bitmaps are always synchronized, regardless of 
>>> failure.
>>> +  (Partial images must be kept.)
>>> +
>>> +:param msync_mode: The mirror sync mode to use for the first mirror.
>>> +   Can be any one of:
>>> +- bitmap: mirrors based on bitmap manifest.
>>> +- full:   Full mirrors.
>>> +- top:Full mirrors of the top layer only.
>>> +
>>> +:param failure: Is the (optional) failure mode, and can be any of:
>>> +- None: No failure. Test the normative path. Default.
>>> +- simulated:Cancel the job right before it completes.
>>> +This also tests writes "during" the job.
>>> +- intermediate: This tests a job that fails mid-process and 
>>> produces
>>> +an incomplete mirror. Testing limitations prevent
>>> +testing competing writes.
>>> +"""
>>> +with iotests.FilePath('img', 'bsync1', 'bsync2', 'bsync3',
>>> +'fmirror0', 'fmirror1', 'fmirror2', 
>>> 'fmirror3') as \
>>
>> The indentation is off now.
>>
>>> +(img_path, bsync1, bsync2, bsync3,
>>> + fmirror0, fmirror1, fmirror2, fmirror3), \
>>> + iotests.VM() as vm:
>> Hm.  On tmpfs, this test fails for me:
>>
>> ($ TEST_DIR=/tmp/iotest ./check -qcow2 306)
>>
>> @@ -170,7 +170,7 @@
>>  "drive0": [
>>{
>>  "busy": false,
>> -"count": 262144,
>> +"count": 458752,
>>  "granularity": 65536,
>>  "persistent": false,
>>  "recording": true,
>> @@ -464,7 +464,7 @@
>>  "drive0": [
>>{
>>  "busy": false,
>> -"count": 262144,
>> +"count": 458752,
>>  "granularity": 65536,
>>  "persistent": false,
>>  "recording": true,
>> @@ -760,7 +760,7 @@
>>  "drive0": [
>>{
>>  "busy": false,
>> -"count": 262144,
>> +"count": 393216,
>>  "granularity": 65536,
>>  "persistent": false,
>>  "recording": true,
>> @@ -1056,7 +1056,7 @@
>>  "drive0": [
>>{
>>  "busy": false,
>> -"count": 262144,
>> +"count": 458752,
>>  "granularity": 65536,
>>  "persistent": false,
>>  "recording": true,
>> @@ -1350,7 +1350,7 @@
>>  "drive0": [
>>{
>>  "busy": false,
>> -"count": 262144,
>> +"count": 458752,
>>  "granularity": 65536,
>>  "persistent": false,
>>  "recording": true,
>> @@ -2236,7 +2236,7 @@
>>  "drive0": [
>>{
>>  "busy": false,
>> -"count": 262144,
>> +"count": 458752,
>>  "granularity": 65536,
>>  "persistent": false,
>>  "recording": true,
>>
>> Can you see the same?
> 
> yes, but also only on tmpfs. ext4, xfs, ZFS all work fine.. the numbers 
> for tmpfs vary between runs, each wrong count is sometimes 393216 (256k 
> expected + 128k extra), sometimes 458752 (+192k). it's always the dirty 
> bitmap used by the mirror job which is 'wrong', not the passed-in sync 
> bitmap

Re: [PATCH qemu 4/4] iotests: add test for bitmap mirror

2020-10-02 Thread Fabian Grünbichler 
On October 1, 2020 7:31 pm, Max Reitz wrote:
> On 22.09.20 11:14, Fabian Grünbichler wrote:
>> heavily based on/practically forked off iotest 257 for bitmap backups,
>> but:
>> 
>> - no writes to filter node 'mirror-top' between completion and
>> finalization, as those seem to deadlock?
>> - no inclusion of not-yet-available full/top sync modes in combination
>> with bitmaps
>> - extra set of reference/test mirrors to verify that writes in parallel
>> with active mirror work
>> 
>> intentionally keeping copyright and ownership of original test case to
>> honor provenance.
>> 
>> Signed-off-by: Fabian Grünbichler 
>> ---
>>  tests/qemu-iotests/306 |  546 +++
>>  tests/qemu-iotests/306.out | 2846 
>>  tests/qemu-iotests/group   |1 +
>>  3 files changed, 3393 insertions(+)
>>  create mode 100755 tests/qemu-iotests/306
>>  create mode 100644 tests/qemu-iotests/306.out
>> 
>> diff --git a/tests/qemu-iotests/306 b/tests/qemu-iotests/306
>> new file mode 100755
>> index 00..1bb8bd4138
>> --- /dev/null
>> +++ b/tests/qemu-iotests/306
> 
> [...]
> 
>> +def test_bitmap_sync(bsync_mode, msync_mode='bitmap', failure=None):
>> +"""
>> +Test bitmap mirror routines.
>> +
>> +:param bsync_mode: Is the Bitmap Sync mode, and can be any of:
>> +- on-success: This is the "incremental" style mode. Bitmaps are
>> +  synchronized to what was copied out only on success.
>> +  (Partial images must be discarded.)
>> +- never:  This is the "differential" style mode.
>> +  Bitmaps are never synchronized.
>> +- always: This is a "best effort" style mode.
>> +  Bitmaps are always synchronized, regardless of 
>> failure.
>> +  (Partial images must be kept.)
>> +
>> +:param msync_mode: The mirror sync mode to use for the first mirror.
>> +   Can be any one of:
>> +- bitmap: mirrors based on bitmap manifest.
>> +- full:   Full mirrors.
>> +- top:Full mirrors of the top layer only.
>> +
>> +:param failure: Is the (optional) failure mode, and can be any of:
>> +- None: No failure. Test the normative path. Default.
>> +- simulated:Cancel the job right before it completes.
>> +This also tests writes "during" the job.
>> +- intermediate: This tests a job that fails mid-process and produces
>> +an incomplete mirror. Testing limitations prevent
>> +testing competing writes.
>> +"""
>> +with iotests.FilePath('img', 'bsync1', 'bsync2', 'bsync3',
>> +'fmirror0', 'fmirror1', 'fmirror2', 'fmirror3') 
>> as \
> 
> The indentation is off now.
> 
>> +(img_path, bsync1, bsync2, bsync3,
>> + fmirror0, fmirror1, fmirror2, fmirror3), \
>> + iotests.VM() as vm:
> Hm.  On tmpfs, this test fails for me:
> 
> ($ TEST_DIR=/tmp/iotest ./check -qcow2 306)
> 
> @@ -170,7 +170,7 @@
>  "drive0": [
>{
>  "busy": false,
> -"count": 262144,
> +"count": 458752,
>  "granularity": 65536,
>  "persistent": false,
>  "recording": true,
> @@ -464,7 +464,7 @@
>  "drive0": [
>{
>  "busy": false,
> -"count": 262144,
> +"count": 458752,
>  "granularity": 65536,
>  "persistent": false,
>  "recording": true,
> @@ -760,7 +760,7 @@
>  "drive0": [
>{
>  "busy": false,
> -"count": 262144,
> +"count": 393216,
>  "granularity": 65536,
>  "persistent": false,
>  "recording": true,
> @@ -1056,7 +1056,7 @@
>  "drive0": [
>{
>  "busy": false,
> -"count": 262144,
> +"count": 458752,
>  "granularity": 65536,
>  "persistent": false,
>  "recording": true,
> @@ -1350,7 +1350,7 @@
>  "drive0": [
>{
>  "busy": false,
> -"count": 262144,
> +"count": 458752,
>  "granularity": 65536,
>  "persistent": false,
>  "recording": true,
> @@ -2236,7 +2236,7 @@
>  "drive0": [
>{
>  "busy": false,
> -"count": 262144,
> +"count": 458752,
>  "granularity": 65536,
>  "persistent": false,
>  "recording": true,
> 
> Can you see the same?

yes, but also only on tmpfs. ext4, xfs, ZFS all work fine.. the numbers 
for tmpfs vary between runs, each wrong count is sometimes 393216 (256k 
expected + 128k extra), sometimes 458752 (+192k). it's always the dirty 
bitmap used by the mirror job which is 'wrong', not the passed-in sync 
bitmap which verifies correctly. the final mirror results also seem 
correct.

for the first diff hunk (bitmap + never + simulated), we did

- reference mirror #0
- add sync bitmap 'bitmap

Re: [PATCH qemu 4/4] iotests: add test for bitmap mirror

2020-10-01 Thread Max Reitz 
On 22.09.20 11:14, Fabian Grünbichler wrote:
> heavily based on/practically forked off iotest 257 for bitmap backups,
> but:
> 
> - no writes to filter node 'mirror-top' between completion and
> finalization, as those seem to deadlock?
> - no inclusion of not-yet-available full/top sync modes in combination
> with bitmaps
> - extra set of reference/test mirrors to verify that writes in parallel
> with active mirror work
> 
> intentionally keeping copyright and ownership of original test case to
> honor provenance.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
>  tests/qemu-iotests/306 |  546 +++
>  tests/qemu-iotests/306.out | 2846 
>  tests/qemu-iotests/group   |1 +
>  3 files changed, 3393 insertions(+)
>  create mode 100755 tests/qemu-iotests/306
>  create mode 100644 tests/qemu-iotests/306.out
> 
> diff --git a/tests/qemu-iotests/306 b/tests/qemu-iotests/306
> new file mode 100755
> index 00..1bb8bd4138
> --- /dev/null
> +++ b/tests/qemu-iotests/306

[...]

> +def test_bitmap_sync(bsync_mode, msync_mode='bitmap', failure=None):
> +"""
> +Test bitmap mirror routines.
> +
> +:param bsync_mode: Is the Bitmap Sync mode, and can be any of:
> +- on-success: This is the "incremental" style mode. Bitmaps are
> +  synchronized to what was copied out only on success.
> +  (Partial images must be discarded.)
> +- never:  This is the "differential" style mode.
> +  Bitmaps are never synchronized.
> +- always: This is a "best effort" style mode.
> +  Bitmaps are always synchronized, regardless of failure.
> +  (Partial images must be kept.)
> +
> +:param msync_mode: The mirror sync mode to use for the first mirror.
> +   Can be any one of:
> +- bitmap: mirrors based on bitmap manifest.
> +- full:   Full mirrors.
> +- top:Full mirrors of the top layer only.
> +
> +:param failure: Is the (optional) failure mode, and can be any of:
> +- None: No failure. Test the normative path. Default.
> +- simulated:Cancel the job right before it completes.
> +This also tests writes "during" the job.
> +- intermediate: This tests a job that fails mid-process and produces
> +an incomplete mirror. Testing limitations prevent
> +testing competing writes.
> +"""
> +with iotests.FilePath('img', 'bsync1', 'bsync2', 'bsync3',
> +'fmirror0', 'fmirror1', 'fmirror2', 'fmirror3') 
> as \

The indentation is off now.

> +(img_path, bsync1, bsync2, bsync3,
> + fmirror0, fmirror1, fmirror2, fmirror3), \
> + iotests.VM() as vm:
Hm.  On tmpfs, this test fails for me:

($ TEST_DIR=/tmp/iotest ./check -qcow2 306)

@@ -170,7 +170,7 @@
 "drive0": [
   {
 "busy": false,
-"count": 262144,
+"count": 458752,
 "granularity": 65536,
 "persistent": false,
 "recording": true,
@@ -464,7 +464,7 @@
 "drive0": [
   {
 "busy": false,
-"count": 262144,
+"count": 458752,
 "granularity": 65536,
 "persistent": false,
 "recording": true,
@@ -760,7 +760,7 @@
 "drive0": [
   {
 "busy": false,
-"count": 262144,
+"count": 393216,
 "granularity": 65536,
 "persistent": false,
 "recording": true,
@@ -1056,7 +1056,7 @@
 "drive0": [
   {
 "busy": false,
-"count": 262144,
+"count": 458752,
 "granularity": 65536,
 "persistent": false,
 "recording": true,
@@ -1350,7 +1350,7 @@
 "drive0": [
   {
 "busy": false,
-"count": 262144,
+"count": 458752,
 "granularity": 65536,
 "persistent": false,
 "recording": true,
@@ -2236,7 +2236,7 @@
 "drive0": [
   {
 "busy": false,
-"count": 262144,
+"count": 458752,
 "granularity": 65536,
 "persistent": false,
 "recording": true,

Can you see the same?

Max



signature.asc
Description: OpenPGP digital signature

[PATCH qemu 4/4] iotests: add test for bitmap mirror

2020-09-22 Thread Fabian Grünbichler 
heavily based on/practically forked off iotest 257 for bitmap backups,
but:

- no writes to filter node 'mirror-top' between completion and
finalization, as those seem to deadlock?
- no inclusion of not-yet-available full/top sync modes in combination
with bitmaps
- extra set of reference/test mirrors to verify that writes in parallel
with active mirror work

intentionally keeping copyright and ownership of original test case to
honor provenance.

Signed-off-by: Fabian Grünbichler 
---
 tests/qemu-iotests/306 |  546 +++
 tests/qemu-iotests/306.out | 2846 
 tests/qemu-iotests/group   |1 +
 3 files changed, 3393 insertions(+)
 create mode 100755 tests/qemu-iotests/306
 create mode 100644 tests/qemu-iotests/306.out

diff --git a/tests/qemu-iotests/306 b/tests/qemu-iotests/306
new file mode 100755
index 00..1bb8bd4138
--- /dev/null
+++ b/tests/qemu-iotests/306
@@ -0,0 +1,546 @@
+#!/usr/bin/env python3
+#
+# Test bitmap-sync mirrors (incremental, differential, and partials)
+#
+# Copyright (c) 2019 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 .
+#
+# owner=js...@redhat.com
+
+import math
+import os
+
+import iotests
+from iotests import log, qemu_img
+
+SIZE = 64 * 1024 * 1024
+GRANULARITY = 64 * 1024
+
+
+class Pattern:
+def __init__(self, byte, offset, size=GRANULARITY):
+self.byte = byte
+self.offset = offset
+self.size = size
+
+def bits(self, granularity):
+lower = self.offset // granularity
+upper = (self.offset + self.size - 1) // granularity
+return set(range(lower, upper + 1))
+
+
+class PatternGroup:
+"""Grouping of Pattern objects. Initialize with an iterable of Patterns."""
+def __init__(self, patterns):
+self.patterns = patterns
+
+def bits(self, granularity):
+"""Calculate the unique bits dirtied by this pattern grouping"""
+res = set()
+for pattern in self.patterns:
+res |= pattern.bits(granularity)
+return res
+
+
+GROUPS = [
+PatternGroup([
+# Batch 0: 4 clusters
+Pattern('0x49', 0x000),
+Pattern('0x6c', 0x010),   # 1M
+Pattern('0x6f', 0x200),   # 32M
+Pattern('0x76', 0x3ff)]), # 64M - 64K
+PatternGroup([
+# Batch 1: 6 clusters (3 new)
+Pattern('0x65', 0x000),   # Full overwrite
+Pattern('0x77', 0x00f8000),   # Partial-left (1M-32K)
+Pattern('0x72', 0x2008000),   # Partial-right (32M+32K)
+Pattern('0x69', 0x3fe)]), # Adjacent-left (64M - 128K)
+PatternGroup([
+# Batch 2: 7 clusters (3 new)
+Pattern('0x74', 0x001),   # Adjacent-right
+Pattern('0x69', 0x00e8000),   # Partial-left  (1M-96K)
+Pattern('0x6e', 0x2018000),   # Partial-right (32M+96K)
+Pattern('0x67', 0x3fe,
+2*GRANULARITY)]), # Overwrite [(64M-128K)-64M)
+PatternGroup([
+# Batch 3: 8 clusters (5 new)
+# Carefully chosen such that nothing re-dirties the one cluster
+# that copies out successfully before failure in Group #1.
+Pattern('0xaa', 0x001,
+3*GRANULARITY),   # Overwrite and 2x Adjacent-right
+Pattern('0xbb', 0x00d8000),   # Partial-left (1M-160K)
+Pattern('0xcc', 0x2028000),   # Partial-right (32M+160K)
+Pattern('0xdd', 0x3fc)]), # New; leaving a gap to the right
+]
+
+
+class EmulatedBitmap:
+def __init__(self, granularity=GRANULARITY):
+self._bits = set()
+self.granularity = granularity
+
+def dirty_bits(self, bits):
+self._bits |= set(bits)
+
+def dirty_group(self, n):
+self.dirty_bits(GROUPS[n].bits(self.granularity))
+
+def clear(self):
+self._bits = set()
+
+def clear_bits(self, bits):
+self._bits -= set(bits)
+
+def clear_bit(self, bit):
+self.clear_bits({bit})
+
+def clear_group(self, n):
+self.clear_bits(GROUPS[n].bits(self.granularity))
+
+@property
+def first_bit(self):
+return sorted(self.bits)[0]
+
+@property
+def bits(self):
+return self._bits
+
+@property
+def count(self):
+return len(self.bits)
+
+def compare(self, qmp_bitmap):
+"""
+Print a nice human-readable message checking that a bitmap as rep