01.08.2019 20:06, Max Reitz wrote:
> On 01.08.19 18:03, Vladimir Sementsov-Ogievskiy wrote:
>> 01.08.2019 18:12, Max Reitz wrote:
>>> Perform two guest writes to not yet backed up areas of an image, where
>>> the former touches an inner area of the latter.
>>>
>>> Before HEAD^, copy offloading broke this in two ways:
>>> (1) The output differs from the reference output (what the source was
>>> before the guest writes).
>>> (2) But you will not see that in the failing output, because the job
>>> offset is reported as being greater than the job length. This is
>>> because one cluster is copied twice, and thus accounted for twice,
>>> but of course the job length does not increase.
>>>
>>> Signed-off-by: Max Reitz <mre...@redhat.com>
>>> ---
>>> tests/qemu-iotests/056 | 34 ++++++++++++++++++++++++++++++++++
>>> tests/qemu-iotests/056.out | 4 ++--
>>> 2 files changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
>>> index f40fc11a09..d7198507f5 100755
>>> --- a/tests/qemu-iotests/056
>>> +++ b/tests/qemu-iotests/056
>>> @@ -133,6 +133,7 @@ class BackupTest(iotests.QMPTestCase):
>>> self.vm = iotests.VM()
>>> self.test_img = img_create('test')
>>> self.dest_img = img_create('dest')
>>> + self.ref_img = img_create('ref')
>>> self.vm.add_drive(self.test_img)
>>> self.vm.launch()
>>>
>>> @@ -140,6 +141,7 @@ class BackupTest(iotests.QMPTestCase):
>>> self.vm.shutdown()
>>> try_remove(self.test_img)
>>> try_remove(self.dest_img)
>>> + try_remove(self.ref_img)
>>>
>>> def hmp_io_writes(self, drive, patterns):
>>> for pattern in patterns:
>>> @@ -177,6 +179,38 @@ class BackupTest(iotests.QMPTestCase):
>>> self.assert_qmp(event, 'data/error', qerror)
>>> return False
>>>
>>> + def test_overlapping_writes(self):
>>> + # Write something to back up
>>> + self.hmp_io_writes('drive0', [('42', '0M', '2M')])
>>> +
>>> + # Create a reference backup
>>> + self.qmp_backup_and_wait(device='drive0', format=iotests.imgfmt,
>>> + sync='full', target=self.ref_img)
>>> +
>>> + # Now to the test backup: We simulate the following guest
>>> + # writes:
>>> + # (1) [1M + 64k, 1M + 128k): Afterwards, everything in that
>>> + # area should be in the target image, and we must not copy
>>> + # it again (because the source image has changed now)
>>> + # (64k is the job's cluster size)
>>> + # (2) [1M, 2M): The backup job must not get overeager. It
>>> + # must copy [1M, 1M + 64k) and [1M + 128k, 2M) separately,
>>> + # but not the area in between.
>>> +
>>> + self.qmp_backup(device='drive0', format=iotests.imgfmt,
>>> sync='full',
>>> + target=self.dest_img, speed=1)
>>> +
>>> + self.hmp_io_writes('drive0', [('23', '%ik' % (1024 + 64), '64k'),
>>> + ('66', '1M', '1M')])
>>> +
>>> + # Let the job complete
>>> + res = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
>>> + self.assert_qmp(res, 'return', {})
>>> + self.qmp_backup_wait('drive0')
>>> +
>>> + self.assertTrue(iotests.compare_images(self.ref_img,
>>> self.dest_img),
>>> + 'target image does not match reference image')
>>> +
>>> def test_dismiss_false(self):
>>> res = self.vm.qmp('query-block-jobs')
>>> self.assert_qmp(res, 'return', [])
>>> diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out
>>> index dae404e278..36376bed87 100644
>>> --- a/tests/qemu-iotests/056.out
>>> +++ b/tests/qemu-iotests/056.out
>>> @@ -1,5 +1,5 @@
>>> -.........
>>> +..........
>>> ----------------------------------------------------------------------
>>> -Ran 9 tests
>>> +Ran 10 tests
>>>
>>> OK
>>>
>>
>> Failed for me:
>> -..........
>> +qemu-img: Could not open
>> '/work/src/qemu/master/tests/qemu-iotests/scratch/dest.qcow2': Failed to get
>> shared "write" lock
>> +Is another process using the image
>> [/work/src/qemu/master/tests/qemu-iotests/scratch/dest.qcow2]?
>> +......F...
>> +======================================================================
>> +FAIL: test_overlapping_writes (__main__.BackupTest)
>> +----------------------------------------------------------------------
>> +Traceback (most recent call last):
>> + File "056", line 212, in test_overlapping_writes
>> + 'target image does not match reference image')
>> +AssertionError: False is not true : target image does not match reference
>> image
>> +
>> ----------------------------------------------------------------------
>> Ran 10 tests
>>
>> -OK
>> +FAILED (failures=1)
>
> Hm. I hoped seeing BLOCK_JOB_COMPLETED would be enough.
The problem is higher: "Failed to get shared "write" lock". Because of this
iotests.compare_images
can't work. So, because of locking, we need to shutdown qemu before starting
qemu-img.
And it's done so in test_complete_top() (I just looked at it as it's the only
other user of compare_images
in 056)
>
>> So, with applied
>>
>> @@ -207,6 +207,7 @@ class BackupTest(iotests.QMPTestCase):
>> res = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
>> self.assert_qmp(res, 'return', {})
>> self.qmp_backup_wait('drive0')
>> + self.vm.shutdown()
>>
>> self.assertTrue(iotests.compare_images(self.ref_img,
>> self.dest_img),
>> 'target image does not match reference image')
>
> I’d personally prefer auto_dismiss=False and then block-job-dismiss.
> Although I can’t give a reason why.
>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>> Tested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>
> In any case, thanks!
>
> Max
>
--
Best regards,
Vladimir
