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 <mreitz@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
--
2.21.0
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 <mreitz@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)
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')
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
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 <mreitz@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.
> 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 <vsementsov@virtuozzo.com>
> Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
In any case, thanks!
Max
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 <mreitz@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 <vsementsov@virtuozzo.com>
>> Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> In any case, thanks!
>
> Max
>
--
Best regards,
Vladimir
On 01.08.19 19:25, Vladimir Sementsov-Ogievskiy wrote:
> 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 <mreitz@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)
The destination image shouldn’t be in use by qemu after the block job is
done, though. Therefore, there shouldn’t be a lock issue. That’s what
I meant.
Max
>>> 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 <vsementsov@virtuozzo.com>
>>> Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> In any case, thanks!
>>
>> Max
>>
>
>
01.08.2019 20:35, Max Reitz wrote:
> On 01.08.19 19:25, Vladimir Sementsov-Ogievskiy wrote:
>> 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 <mreitz@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)
>
> The destination image shouldn’t be in use by qemu after the block job is
> done, though. Therefore, there shouldn’t be a lock issue. That’s what
> I meant.
>
Ah, understand. Hmm, then it's strange that I see that error..
Clearly, in job_finalize_single(), job_clean() is called first, which should call backup_clean to unref target,
then job_event_completed() is called...
>
>>>> 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 <vsementsov@virtuozzo.com>
>>>> Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>> In any case, thanks!
>>>
>>> Max
>>>
>>
>>
>
>
--
Best regards,
Vladimir
01.08.2019 20:43, Vladimir Sementsov-Ogievskiy wrote:
> 01.08.2019 20:35, Max Reitz wrote:
>> On 01.08.19 19:25, Vladimir Sementsov-Ogievskiy wrote:
>>> 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 <mreitz@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)
>>
>> The destination image shouldn’t be in use by qemu after the block job is
>> done, though. Therefore, there shouldn’t be a lock issue. That’s what
>> I meant.
>>
>
> Ah, understand. Hmm, then it's strange that I see that error..
>
> Clearly, in job_finalize_single(), job_clean() is called first, which should call backup_clean to unref target,
> then job_event_completed() is called...
>
>
But BlockJob.nodes are removed at very-end from block_job_free, so it seems to be another reference to target,
which is still alive when event is sent.
>
>>
>>>>> 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 <vsementsov@virtuozzo.com>
>>>>> Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>
>>>> In any case, thanks!
>>>>
>>>> Max
>>>>
>>>
>>>
>>
>>
>
>
--
Best regards,
Vladimir
© 2016 - 2026 Red Hat, Inc.