diff mbox

[v14,07/20] iotests: 030: Prepare for image locking

Message ID 20170421035606.448-8-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng April 21, 2017, 3:55 a.m. UTC
qemu-img and qemu-io commands when guest is running need "-U" option,
add it.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/030 | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Kevin Wolf April 21, 2017, 1:51 p.m. UTC | #1
Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> qemu-img and qemu-io commands when guest is running need "-U" option,
> add it.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/qemu-iotests/030 | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 0d472d5..5f1dce8 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -63,8 +63,8 @@ class TestSingleDrive(iotests.QMPTestCase):
>      def test_stream_intermediate(self):
>          self.assert_no_active_block_jobs()
>  
> -        self.assertNotEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> -                            qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> +        self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
> +                            qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
>                              'image file map matches backing file before streaming')
>  
>          result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
> @@ -114,7 +114,7 @@ class TestSingleDrive(iotests.QMPTestCase):
>          self.assert_no_active_block_jobs()
>  
>          # The image map is empty before the operation
> -        empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
> +        empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
>  
>          # This is a no-op: no data should ever be copied from the base image
>          result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
> @@ -125,7 +125,7 @@ class TestSingleDrive(iotests.QMPTestCase):
>          self.assert_no_active_block_jobs()
>          self.vm.shutdown()
>  
> -        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> +        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', '-r', test_img),
>                           empty_map, 'image file map changed after a no-op')

This one doesn't seem strictly necessary, as you're only adding -r
without -U. I still think it's a good idea to use -r where we can, but
if we decide to do this, there are more places in this test that could
be updated.

Maybe a separate patch for adding -r without -U to the cases where
qemu-io is run after shutting down the VM?

>      def test_stream_partial(self):
> @@ -197,8 +197,8 @@ class TestParallelOps(iotests.QMPTestCase):
>  
>          # Check that the maps don't match before the streaming operations
>          for i in range(2, self.num_imgs, 2):
> -            self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i]),
> -                                qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i-1]),
> +            self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i]),
> +                                qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i-1]),
>                                  'image file map matches backing file before streaming')
>  
>          # Create all streaming jobs
> @@ -351,8 +351,8 @@ class TestParallelOps(iotests.QMPTestCase):
>      def test_stream_base_node_name(self):
>          self.assert_no_active_block_jobs()
>  
> -        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[4]),
> -                            qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[3]),
> +        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[4]),
> +                            qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[3]),
>                              'image file map matches backing file before streaming')
>  
>          # Error: the base node does not exist
> @@ -422,8 +422,8 @@ class TestQuorum(iotests.QMPTestCase):
>          if not iotests.supports_quorum():
>              return
>  
> -        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
> -                            qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
> +        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
> +                            qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
>                              'image file map matches backing file before streaming')
>  
>          self.assert_no_active_block_jobs()
> @@ -436,8 +436,8 @@ class TestQuorum(iotests.QMPTestCase):
>          self.assert_no_active_block_jobs()
>          self.vm.shutdown()
>  
> -        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
> -                         qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
> +        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
> +                         qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
>                           'image file map does not match backing file after streaming')

These hunks all have -U without -r, which I think we wanted to make an
error.

Kevin
Fam Zheng April 24, 2017, 6:15 a.m. UTC | #2
On Fri, 04/21 15:51, Kevin Wolf wrote:
> Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> > qemu-img and qemu-io commands when guest is running need "-U" option,
> > add it.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  tests/qemu-iotests/030 | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> > index 0d472d5..5f1dce8 100755
> > --- a/tests/qemu-iotests/030
> > +++ b/tests/qemu-iotests/030
> > @@ -63,8 +63,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> >      def test_stream_intermediate(self):
> >          self.assert_no_active_block_jobs()
> >  
> > -        self.assertNotEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> > -                            qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> > +        self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
> > +                            qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
> >                              'image file map matches backing file before streaming')
> >  
> >          result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
> > @@ -114,7 +114,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> >          self.assert_no_active_block_jobs()
> >  
> >          # The image map is empty before the operation
> > -        empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
> > +        empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
> >  
> >          # This is a no-op: no data should ever be copied from the base image
> >          result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
> > @@ -125,7 +125,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> >          self.assert_no_active_block_jobs()
> >          self.vm.shutdown()
> >  
> > -        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> > +        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', '-r', test_img),
> >                           empty_map, 'image file map changed after a no-op')
> 
> This one doesn't seem strictly necessary, as you're only adding -r
> without -U. I still think it's a good idea to use -r where we can, but
> if we decide to do this, there are more places in this test that could
> be updated.
> 
> Maybe a separate patch for adding -r without -U to the cases where
> qemu-io is run after shutting down the VM?

Yes, will split. (In a previous version -U is implied by -r, and the patch was
"add -r to all qemu-io invocations where writing is not needed.)

Fam
diff mbox

Patch

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 0d472d5..5f1dce8 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -63,8 +63,8 @@  class TestSingleDrive(iotests.QMPTestCase):
     def test_stream_intermediate(self):
         self.assert_no_active_block_jobs()
 
-        self.assertNotEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
-                            qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
+        self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
+                            qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
                             'image file map matches backing file before streaming')
 
         result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
@@ -114,7 +114,7 @@  class TestSingleDrive(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         # The image map is empty before the operation
-        empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
+        empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
 
         # This is a no-op: no data should ever be copied from the base image
         result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
@@ -125,7 +125,7 @@  class TestSingleDrive(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
         self.vm.shutdown()
 
-        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
+        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', '-r', test_img),
                          empty_map, 'image file map changed after a no-op')
 
     def test_stream_partial(self):
@@ -197,8 +197,8 @@  class TestParallelOps(iotests.QMPTestCase):
 
         # Check that the maps don't match before the streaming operations
         for i in range(2, self.num_imgs, 2):
-            self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i]),
-                                qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i-1]),
+            self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i]),
+                                qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i-1]),
                                 'image file map matches backing file before streaming')
 
         # Create all streaming jobs
@@ -351,8 +351,8 @@  class TestParallelOps(iotests.QMPTestCase):
     def test_stream_base_node_name(self):
         self.assert_no_active_block_jobs()
 
-        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[4]),
-                            qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[3]),
+        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[4]),
+                            qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[3]),
                             'image file map matches backing file before streaming')
 
         # Error: the base node does not exist
@@ -422,8 +422,8 @@  class TestQuorum(iotests.QMPTestCase):
         if not iotests.supports_quorum():
             return
 
-        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
-                            qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
+        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
+                            qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
                             'image file map matches backing file before streaming')
 
         self.assert_no_active_block_jobs()
@@ -436,8 +436,8 @@  class TestQuorum(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
         self.vm.shutdown()
 
-        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
-                         qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
+        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
+                         qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
                          'image file map does not match backing file after streaming')
 
 class TestSmallerBackingFile(iotests.QMPTestCase):