Re: [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking

[Kevin Wolf]

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 <address@hidden>
> ---
>  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