[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 11/11] iotests/257: test traditional sync mod
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 11/11] iotests/257: test traditional sync modes |
Date: |
Wed, 17 Jul 2019 11:50:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 |
On 16.07.19 18:58, John Snow wrote:
>
>
> On 7/16/19 8:04 AM, Max Reitz wrote:
>> On 16.07.19 02:01, John Snow wrote:
>>> Signed-off-by: John Snow <address@hidden>
>>> ---
>>> tests/qemu-iotests/257 | 41 +-
>>> tests/qemu-iotests/257.out | 3089 ++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 3128 insertions(+), 2 deletions(-)
>>
>> This needs a %s/specify Bitmap sync mode/specify bitmap sync mode/.
>>
>>> diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
>>> index 53ab31c92e..c2a72c577a 100755
>>> --- a/tests/qemu-iotests/257
>>> +++ b/tests/qemu-iotests/257
>>
>> [...]
>>
>>> @@ -393,7 +399,7 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap',
>>> failure=None):
>>> # group 1 gets cleared first, then group two gets written.
>>> if ((bsync_mode == 'on-success' and not failure) or
>>> (bsync_mode == 'always')):
>>> - ebitmap.clear_group(1)
>>> + ebitmap.clear()
>>
>> Hmmm... Why?
>>
>
> From an order of operations standpoint, if we are here, we are expecting
> the bitmap to be synchronized. We can clear any existing data it holds,
> and then:
>
>>> ebitmap.dirty_group(2)
>>>
>
> Add new writes that occurred during the job; which only happen here in
> this callback.
>
> (The old code cleared specifically only group 1, the new code is just
> more general. I wound up changing it for a version that didn't make it
> to the list, but this is still correct.)
>
>>> vm.run_job(job, auto_dismiss=True, auto_finalize=False,
>>> @@ -404,8 +410,19 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap',
>>> failure=None):
>>> log('')
>>>
>>> if bsync_mode == 'always' and failure == 'intermediate':
>>> + # TOP treats anything allocated as dirty, expect to see:
>>> + if msync_mode == 'top':
>>> + ebitmap.dirty_group(0)
>>> +
>
> Sorry, this code is definitely in the "cute" category...
>
> If the failure was intermediate, we never call the pre-finalize callback
> above. So we know that the allocated regions of the file are only from
> groups 0 and 1.
>
> So, HERE, we can mark the emulated bitmap's group 0 as dirty, to mimic
> what the copy_bitmap is going to have started the operation with.
>
>>> # We manage to copy one sector (one bit) before the error.
>>> ebitmap.clear_bit(ebitmap.first_bit)
>
> And then right here, we clear the first bit which we did copy out
> successfully. The emulated bitmap is now correct for sync=top.
>
>>> +
>>> + # Full returns all bits set except what was copied/skipped
>>> + if msync_mode == 'full':
>>> + fail_bit = ebitmap.first_bit
>>> + ebitmap.clear()
>>> + ebitmap.dirty_bits(range(fail_bit, SIZE // GRANULARITY))
>>> +
>
> The full mode, though, is special. We cleared the first allocated bit
> just like for sync=top, but we take note of the second bit which is the
> one that caused the injected failure.
>
> For both 'top' and 'full' modes here we're really using the ebitmap as
> an allocation record to inform what the output bitmap is going to look like.
>
>>
>> So sync=top didn‘t copy anything? Is that because it now errors out
>> before getting to copy something?
>>
>
> The ebitmap.clear_bit(ebitmap.first_bit) triggers for top, too. The test
> output should hopefully make sense here.
I... I have no idea how I missed the ebitmap.clear_bit().
So with the test output fixed:
Reviewed-by: Max Reitz <address@hidden>
>> (The rest looks good to me.)
>>
>> Max
>>
>>> ebitmap.compare(get_bitmap(bitmaps, drive0.device, 'bitmap0'))
>>>
>>> # 2 - Writes and Reference Backup
>>
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 09/11] block/backup: teach TOP to never copy unallocated regions, (continued)
[Qemu-devel] [PATCH v2 03/11] iotests/257: Refactor backup helpers, John Snow, 2019/07/15
[Qemu-devel] [PATCH v2 11/11] iotests/257: test traditional sync modes, John Snow, 2019/07/15
Re: [Qemu-devel] [PATCH v2 00/11] bitmaps: allow bitmaps to be used with full and top, John Snow, 2019/07/17