| Message ID | 1397771992-31126-13-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/17/2014 03:59 PM, Max Reitz wrote: > As the length of a mirror block job no longer directly depends on the > size of the block device, drop the related checks from this test. > > As 041 uses the wait_until_completed function from iotests.py, that > check has to be dropped there as well which in turn affects test 055. On > the other hand, a block job's length does not have to be related to the > length of the image file in the first place, so that check was > questionable anyway. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/040 | 3 --- > tests/qemu-iotests/041 | 2 -- > tests/qemu-iotests/iotests.py | 2 -- > 3 files changed, 7 deletions(-) > > diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 > index 734b6a6..437af2b 100755 > --- a/tests/qemu-iotests/040 > +++ b/tests/qemu-iotests/040 > @@ -46,13 +46,10 @@ class ImageCommitTestCase(iotests.QMPTestCase): > if event['event'] == 'BLOCK_JOB_COMPLETED': > self.assert_qmp(event, 'data/type', 'commit') > self.assert_qmp(event, 'data/device', 'drive0') > - self.assert_qmp(event, 'data/offset', self.image_len) > - self.assert_qmp(event, 'data/len', self.image_len) But shouldn't you replace this with an assertion that 'data/offset' and 'data/len' have converged to the same value? > +++ b/tests/qemu-iotests/041 > @@ -46,8 +46,6 @@ class ImageMirroringTestCase(iotests.QMPTestCase): > event = self.cancel_and_wait() > self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED') > self.assert_qmp(event, 'data/type', 'mirror') > - self.assert_qmp(event, 'data/offset', self.image_len) > - self.assert_qmp(event, 'data/len', self.image_len) And again. You can't enforce that they are the same as the image_len, but you SHOULD enforce that they are the same as each other at the completion of the job. > > def complete_and_wait(self, drive='drive0', wait_ready=True): > '''Complete a block job and wait for it to finish''' > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index e4fa9af..0d3ff24 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -265,8 +265,6 @@ class QMPTestCase(unittest.TestCase): > if event['event'] == 'BLOCK_JOB_COMPLETED': > self.assert_qmp(event, 'data/device', drive) > self.assert_qmp_absent(event, 'data/error') > - self.assert_qmp(event, 'data/offset', self.image_len) > - self.assert_qmp(event, 'data/len', self.image_len) and again
On 22.04.2014 17:28, Eric Blake wrote: > On 04/17/2014 03:59 PM, Max Reitz wrote: >> As the length of a mirror block job no longer directly depends on the >> size of the block device, drop the related checks from this test. >> >> As 041 uses the wait_until_completed function from iotests.py, that >> check has to be dropped there as well which in turn affects test 055. On >> the other hand, a block job's length does not have to be related to the >> length of the image file in the first place, so that check was >> questionable anyway. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/040 | 3 --- >> tests/qemu-iotests/041 | 2 -- >> tests/qemu-iotests/iotests.py | 2 -- >> 3 files changed, 7 deletions(-) >> >> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 >> index 734b6a6..437af2b 100755 >> --- a/tests/qemu-iotests/040 >> +++ b/tests/qemu-iotests/040 >> @@ -46,13 +46,10 @@ class ImageCommitTestCase(iotests.QMPTestCase): >> if event['event'] == 'BLOCK_JOB_COMPLETED': >> self.assert_qmp(event, 'data/type', 'commit') >> self.assert_qmp(event, 'data/device', 'drive0') >> - self.assert_qmp(event, 'data/offset', self.image_len) >> - self.assert_qmp(event, 'data/len', self.image_len) > But shouldn't you replace this with an assertion that 'data/offset' and > 'data/len' have converged to the same value? Yes, you're right. Max >> +++ b/tests/qemu-iotests/041 >> @@ -46,8 +46,6 @@ class ImageMirroringTestCase(iotests.QMPTestCase): >> event = self.cancel_and_wait() >> self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED') >> self.assert_qmp(event, 'data/type', 'mirror') >> - self.assert_qmp(event, 'data/offset', self.image_len) >> - self.assert_qmp(event, 'data/len', self.image_len) > And again. You can't enforce that they are the same as the image_len, > but you SHOULD enforce that they are the same as each other at the > completion of the job. > >> >> def complete_and_wait(self, drive='drive0', wait_ready=True): >> '''Complete a block job and wait for it to finish''' >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index e4fa9af..0d3ff24 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -265,8 +265,6 @@ class QMPTestCase(unittest.TestCase): >> if event['event'] == 'BLOCK_JOB_COMPLETED': >> self.assert_qmp(event, 'data/device', drive) >> self.assert_qmp_absent(event, 'data/error') >> - self.assert_qmp(event, 'data/offset', self.image_len) >> - self.assert_qmp(event, 'data/len', self.image_len) > and again >
On Thu, 04/17 23:59, Max Reitz wrote: > As the length of a mirror block job no longer directly depends on the > size of the block device, drop the related checks from this test. > > As 041 uses the wait_until_completed function from iotests.py, that > check has to be dropped there as well which in turn affects test 055. On > the other hand, a block job's length does not have to be related to the > length of the image file in the first place, so that check was > questionable anyway. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/040 | 3 --- > tests/qemu-iotests/041 | 2 -- > tests/qemu-iotests/iotests.py | 2 -- > 3 files changed, 7 deletions(-) > > diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 > index 734b6a6..437af2b 100755 > --- a/tests/qemu-iotests/040 > +++ b/tests/qemu-iotests/040 > @@ -46,13 +46,10 @@ class ImageCommitTestCase(iotests.QMPTestCase): > if event['event'] == 'BLOCK_JOB_COMPLETED': > self.assert_qmp(event, 'data/type', 'commit') > self.assert_qmp(event, 'data/device', 'drive0') > - self.assert_qmp(event, 'data/offset', self.image_len) > - self.assert_qmp(event, 'data/len', self.image_len) > completed = True > elif event['event'] == 'BLOCK_JOB_READY': > self.assert_qmp(event, 'data/type', 'commit') > self.assert_qmp(event, 'data/device', 'drive0') > - self.assert_qmp(event, 'data/len', self.image_len) > self.vm.qmp('block-job-complete', device='drive0') > > self.assert_no_active_block_jobs() > diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 > index ec470b2..8bb7ec3 100755 > --- a/tests/qemu-iotests/041 > +++ b/tests/qemu-iotests/041 > @@ -46,8 +46,6 @@ class ImageMirroringTestCase(iotests.QMPTestCase): > event = self.cancel_and_wait() > self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED') > self.assert_qmp(event, 'data/type', 'mirror') > - self.assert_qmp(event, 'data/offset', self.image_len) > - self.assert_qmp(event, 'data/len', self.image_len) > > def complete_and_wait(self, drive='drive0', wait_ready=True): > '''Complete a block job and wait for it to finish''' > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index e4fa9af..0d3ff24 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -265,8 +265,6 @@ class QMPTestCase(unittest.TestCase): > if event['event'] == 'BLOCK_JOB_COMPLETED': > self.assert_qmp(event, 'data/device', drive) > self.assert_qmp_absent(event, 'data/error') > - self.assert_qmp(event, 'data/offset', self.image_len) > - self.assert_qmp(event, 'data/len', self.image_len) > completed = True > > self.assert_no_active_block_jobs() > -- > 1.9.2 > Since you are touching this code as well, do you mind take this in your next revision? :) https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg00274.html Thanks, Fam
On 23.04.2014 08:50, Fam Zheng wrote: > On Thu, 04/17 23:59, Max Reitz wrote: >> As the length of a mirror block job no longer directly depends on the >> size of the block device, drop the related checks from this test. >> >> As 041 uses the wait_until_completed function from iotests.py, that >> check has to be dropped there as well which in turn affects test 055. On >> the other hand, a block job's length does not have to be related to the >> length of the image file in the first place, so that check was >> questionable anyway. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/040 | 3 --- >> tests/qemu-iotests/041 | 2 -- >> tests/qemu-iotests/iotests.py | 2 -- >> 3 files changed, 7 deletions(-) >> >> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 >> index 734b6a6..437af2b 100755 >> --- a/tests/qemu-iotests/040 >> +++ b/tests/qemu-iotests/040 >> @@ -46,13 +46,10 @@ class ImageCommitTestCase(iotests.QMPTestCase): >> if event['event'] == 'BLOCK_JOB_COMPLETED': >> self.assert_qmp(event, 'data/type', 'commit') >> self.assert_qmp(event, 'data/device', 'drive0') >> - self.assert_qmp(event, 'data/offset', self.image_len) >> - self.assert_qmp(event, 'data/len', self.image_len) >> completed = True >> elif event['event'] == 'BLOCK_JOB_READY': >> self.assert_qmp(event, 'data/type', 'commit') >> self.assert_qmp(event, 'data/device', 'drive0') >> - self.assert_qmp(event, 'data/len', self.image_len) >> self.vm.qmp('block-job-complete', device='drive0') >> >> self.assert_no_active_block_jobs() >> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 >> index ec470b2..8bb7ec3 100755 >> --- a/tests/qemu-iotests/041 >> +++ b/tests/qemu-iotests/041 >> @@ -46,8 +46,6 @@ class ImageMirroringTestCase(iotests.QMPTestCase): >> event = self.cancel_and_wait() >> self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED') >> self.assert_qmp(event, 'data/type', 'mirror') >> - self.assert_qmp(event, 'data/offset', self.image_len) >> - self.assert_qmp(event, 'data/len', self.image_len) >> >> def complete_and_wait(self, drive='drive0', wait_ready=True): >> '''Complete a block job and wait for it to finish''' >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index e4fa9af..0d3ff24 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -265,8 +265,6 @@ class QMPTestCase(unittest.TestCase): >> if event['event'] == 'BLOCK_JOB_COMPLETED': >> self.assert_qmp(event, 'data/device', drive) >> self.assert_qmp_absent(event, 'data/error') >> - self.assert_qmp(event, 'data/offset', self.image_len) >> - self.assert_qmp(event, 'data/len', self.image_len) >> completed = True >> >> self.assert_no_active_block_jobs() >> -- >> 1.9.2 >> > Since you are touching this code as well, do you mind take this in your > next revision? :) > > https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg00274.html Okay, I'll rebase on top of that patch. Max
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index 734b6a6..437af2b 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -46,13 +46,10 @@ class ImageCommitTestCase(iotests.QMPTestCase): if event['event'] == 'BLOCK_JOB_COMPLETED': self.assert_qmp(event, 'data/type', 'commit') self.assert_qmp(event, 'data/device', 'drive0') - self.assert_qmp(event, 'data/offset', self.image_len) - self.assert_qmp(event, 'data/len', self.image_len) completed = True elif event['event'] == 'BLOCK_JOB_READY': self.assert_qmp(event, 'data/type', 'commit') self.assert_qmp(event, 'data/device', 'drive0') - self.assert_qmp(event, 'data/len', self.image_len) self.vm.qmp('block-job-complete', device='drive0') self.assert_no_active_block_jobs() diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index ec470b2..8bb7ec3 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -46,8 +46,6 @@ class ImageMirroringTestCase(iotests.QMPTestCase): event = self.cancel_and_wait() self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED') self.assert_qmp(event, 'data/type', 'mirror') - self.assert_qmp(event, 'data/offset', self.image_len) - self.assert_qmp(event, 'data/len', self.image_len) def complete_and_wait(self, drive='drive0', wait_ready=True): '''Complete a block job and wait for it to finish''' diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index e4fa9af..0d3ff24 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -265,8 +265,6 @@ class QMPTestCase(unittest.TestCase): if event['event'] == 'BLOCK_JOB_COMPLETED': self.assert_qmp(event, 'data/device', drive) self.assert_qmp_absent(event, 'data/error') - self.assert_qmp(event, 'data/offset', self.image_len) - self.assert_qmp(event, 'data/len', self.image_len) completed = True self.assert_no_active_block_jobs()
As the length of a mirror block job no longer directly depends on the size of the block device, drop the related checks from this test. As 041 uses the wait_until_completed function from iotests.py, that check has to be dropped there as well which in turn affects test 055. On the other hand, a block job's length does not have to be related to the length of the image file in the first place, so that check was questionable anyway. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/040 | 3 --- tests/qemu-iotests/041 | 2 -- tests/qemu-iotests/iotests.py | 2 -- 3 files changed, 7 deletions(-)