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. > 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
signature.asc
Description: OpenPGP digital signature