| Message ID | 20200511135825.219437-5-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mirror: Make sure that source and target size match | expand |
On 11.05.20 15:58, Kevin Wolf wrote: > This tests that the mirror job catches situations where the target node > has a different size than the source node. It must also forbid resize > operations when the job is already running. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > Message-Id: <20200507145228.323412-4-kwolf@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/041 | 45 ++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/041.out | 4 ++-- > 2 files changed, 47 insertions(+), 2 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com>
11.05.2020 16:58, Kevin Wolf wrote: > This tests that the mirror job catches situations where the target node > has a different size than the source node. It must also forbid resize > operations when the job is already running. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > Message-Id: <20200507145228.323412-4-kwolf@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/041 | 45 ++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/041.out | 4 ++-- > 2 files changed, 47 insertions(+), 2 deletions(-) > > diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 > index 1812dd8479..601c756117 100755 > --- a/tests/qemu-iotests/041 > +++ b/tests/qemu-iotests/041 > @@ -240,6 +240,49 @@ class TestSingleBlockdev(TestSingleDrive): Hmm, probably resize tests would be good in the TestSingleDrive, to cover drive-mirror too. Still, there shouldn't be any difference.. So, I don't think it worth doing. > target=self.qmp_target) > self.assert_qmp(result, 'error/class', 'GenericError') > > + def do_test_resize(self, device, node): > + def pre_finalize(): [..] > + def do_test_target_size(self, size): > + result = self.vm.qmp('block_resize', node_name=self.qmp_target, > + size=size) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp(self.qmp_cmd, job_id='job0', > + device='drive0', sync='full', auto_dismiss=False, > + target=self.qmp_target) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.run_job('job0') > + self.assertEqual(result, 'Source and target image have different sizes') Hmm, and this proves, that we are not very good with handling early errors. A lot better would be to fail earlier, on blockdev-mirror command. And, as shown by your previous series, backup works better in this case. But again, it's for another series.
Am 13.05.2020 um 13:17 hat Vladimir Sementsov-Ogievskiy geschrieben: > 11.05.2020 16:58, Kevin Wolf wrote: > > This tests that the mirror job catches situations where the target node > > has a different size than the source node. It must also forbid resize > > operations when the job is already running. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > Message-Id: <20200507145228.323412-4-kwolf@redhat.com> > > Reviewed-by: Eric Blake <eblake@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > > --- > > tests/qemu-iotests/041 | 45 ++++++++++++++++++++++++++++++++++++++ > > tests/qemu-iotests/041.out | 4 ++-- > > 2 files changed, 47 insertions(+), 2 deletions(-) > > > > diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 > > index 1812dd8479..601c756117 100755 > > --- a/tests/qemu-iotests/041 > > +++ b/tests/qemu-iotests/041 > > @@ -240,6 +240,49 @@ class TestSingleBlockdev(TestSingleDrive): > > > Hmm, probably resize tests would be good in the TestSingleDrive, to > cover drive-mirror too. Still, there shouldn't be any difference.. So, > I don't think it worth doing. At first, I tried to do a single implementation that works for both, but it wasn't easily possible. I'm not sure any more if it was the only problem, but at least drive-mirror targets don't have a known node-name, so we would have to guess it from query-named-block-nodes. Not impossible, but maybe not worth it... > > target=self.qmp_target) > > self.assert_qmp(result, 'error/class', 'GenericError') > > + def do_test_resize(self, device, node): > > + def pre_finalize(): > > [..] > > > + def do_test_target_size(self, size): > > + result = self.vm.qmp('block_resize', node_name=self.qmp_target, > > + size=size) > > + self.assert_qmp(result, 'return', {}) > > + > > + result = self.vm.qmp(self.qmp_cmd, job_id='job0', > > + device='drive0', sync='full', auto_dismiss=False, > > + target=self.qmp_target) > > + self.assert_qmp(result, 'return', {}) > > + > > + result = self.vm.run_job('job0') > > + self.assertEqual(result, 'Source and target image have different sizes') > > Hmm, and this proves, that we are not very good with handling early > errors. A lot better would be to fail earlier, on blockdev-mirror > command. And, as shown by your previous series, backup works better in > this case. But again, it's for another series. At least it shows that errp does make it to the client. :-) I'm not sure which way is currently better. QMP handlers are still synchronous code that blocks the rest of QEMU, so doing things only inside the coroutine does have some benefits. Maybe with coroutine QMP handlers this will become less of a concern, though. Kevin
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 1812dd8479..601c756117 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -240,6 +240,49 @@ class TestSingleBlockdev(TestSingleDrive): target=self.qmp_target) self.assert_qmp(result, 'error/class', 'GenericError') + def do_test_resize(self, device, node): + def pre_finalize(): + if device: + result = self.vm.qmp('block_resize', device=device, size=65536) + self.assert_qmp(result, 'error/class', 'GenericError') + + result = self.vm.qmp('block_resize', node_name=node, size=65536) + self.assert_qmp(result, 'error/class', 'GenericError') + + result = self.vm.qmp(self.qmp_cmd, job_id='job0', device='drive0', + sync='full', target=self.qmp_target, + auto_finalize=False, auto_dismiss=False) + self.assert_qmp(result, 'return', {}) + + result = self.vm.run_job('job0', auto_finalize=False, + pre_finalize=pre_finalize) + self.assertEqual(result, None) + + def test_source_resize(self): + self.do_test_resize('drive0', 'top') + + def test_target_resize(self): + self.do_test_resize(None, self.qmp_target) + + def do_test_target_size(self, size): + result = self.vm.qmp('block_resize', node_name=self.qmp_target, + size=size) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp(self.qmp_cmd, job_id='job0', + device='drive0', sync='full', auto_dismiss=False, + target=self.qmp_target) + self.assert_qmp(result, 'return', {}) + + result = self.vm.run_job('job0') + self.assertEqual(result, 'Source and target image have different sizes') + + def test_small_target(self): + self.do_test_target_size(self.image_len // 2) + + def test_large_target(self): + self.do_test_target_size(self.image_len * 2) + test_large_cluster = None test_image_not_found = None test_small_buffer2 = None @@ -251,6 +294,8 @@ class TestSingleDriveZeroLength(TestSingleDrive): class TestSingleBlockdevZeroLength(TestSingleBlockdev): image_len = 0 + test_small_target = None + test_large_target = None class TestSingleDriveUnalignedLength(TestSingleDrive): image_len = 1025 * 1024 diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index 877b76fd31..53abe11d73 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -.............................................................................................. +........................................................................................................ ---------------------------------------------------------------------- -Ran 94 tests +Ran 104 tests OK