[08/11] qemu-iotests: add no-op streaming test

This patch updates test_stream_partial() to test that the block-stream
operation never copies data from the image specified as base.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/qemu-iotests/030 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

On 24.04.2015 13:40, Alberto Garcia wrote:
> This patch updates test_stream_partial() to test that the block-stream
> operation never copies data from the image specified as base.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>   tests/qemu-iotests/030 | 13 +++++++++++++
>   1 file changed, 13 insertions(+)

I think it would be better to add this as an own test case. But I won't 
oppose adding it as a special case of test_stream_partial either, 
because strictly speaking, streaming nothing is a special case of 
streaming partially.

> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 6e6cb5a..a395a03 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -94,6 +94,19 @@ class TestSingleDrive(iotests.QMPTestCase):
>       def test_stream_partial(self):
>           self.assert_no_active_block_jobs()
>   
> +        # 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)
> +        self.assert_qmp(result, 'return', {})
> +
> +        self.wait_until_completed()
> +
> +        self.assert_no_active_block_jobs()
> +
> +        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> +                            qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> +                            'image file map matches backing file after a no-op')

Well, you haven't really proven anything by doing this, except that the 
mid_img hasn't been streamed completely into test_img... What you want 
to prove is that test_img is empty after streaming, right? This can be 
done by either comparing against an empty image specifically created for 
this single test, or by comparing the qemu_io map output against 
"[                       0]     2048/ 2048 sectors not allocated at 
offset 0 bytes (0)\n", which should be the map output for every empty 1 
MB image (regardless of format, and so on).

Max

> +
> +        # And this is the operation that actually copies data
>           result = self.vm.qmp('block-stream', device='drive0', base=backing_img)
>           self.assert_qmp(result, 'return', {})

On Fri 24 Apr 2015 03:19:54 PM CEST, Max Reitz wrote:

>> This patch updates test_stream_partial() to test that the
>> block-stream operation never copies data from the image specified as
>> base.
>
> I think it would be better to add this as an own test case. But I
> won't oppose adding it as a special case of test_stream_partial
> either, because strictly speaking, streaming nothing is a special case
> of streaming partially.

That was the idea, but of course it can be moved to a new test, it's
also fine with me.

>> +        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
>> +                            qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
>> +                            'image file map matches backing file after a no-op')
>
> Well, you haven't really proven anything by doing this, except that
> the mid_img hasn't been streamed completely into test_img... What you
> want to prove is that test_img is empty after streaming, right?

That's right, but what scenario would that be? A streaming operation
that is completed without errors and leaves partial data on the
destination image?

There's only 512 bytes written in mid_img for this test case. If we
suspect that it can go wrong maybe we should start comparing more things
than just the qemu_io map output...

Berto

On 24.04.2015 15:55, Alberto Garcia wrote:
> On Fri 24 Apr 2015 03:19:54 PM CEST, Max Reitz wrote:
>
>>> This patch updates test_stream_partial() to test that the
>>> block-stream operation never copies data from the image specified as
>>> base.
>> I think it would be better to add this as an own test case. But I
>> won't oppose adding it as a special case of test_stream_partial
>> either, because strictly speaking, streaming nothing is a special case
>> of streaming partially.
> That was the idea, but of course it can be moved to a new test, it's
> also fine with me.
>
>>> +        self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
>>> +                            qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
>>> +                            'image file map matches backing file after a no-op')
>> Well, you haven't really proven anything by doing this, except that
>> the mid_img hasn't been streamed completely into test_img... What you
>> want to prove is that test_img is empty after streaming, right?
> That's right, but what scenario would that be? A streaming operation
> that is completed without errors and leaves partial data on the
> destination image?

That would be a bug. Which is what the tests are for. :-)

Max

> There's only 512 bytes written in mid_img for this test case. If we
> suspect that it can go wrong maybe we should start comparing more things
> than just the qemu_io map output...
>
> Berto