Re: [Qemu-devel] [PATCH 7/8] qemu-iotests: fix test_stream_partial()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On 04/16/2015 09:12 AM, Alberto Garcia wrote:
> This test is streaming to the top layer using the intermediate image
> as the base. This is a mistake since block-stream never copies data
> from the base image and its backing chain, so this is effectively a
> no-op.
> 
> In addition to fixing the base parameter, this patch also writes some
> data to the intermediate image before the test, so there's anything to

s/anything/something/

> copy and the test is meaningful.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  tests/qemu-iotests/030 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

You know, we should _also_ test a no-op stream, to prove that we handle
it correctly (we've had bugs in the past where 0-length active commit
behaved differently in the events it generated than non-zero length),
and your change is dropping that.

> @@ -93,7 +94,7 @@ class TestSingleDrive(iotests.QMPTestCase):
>      def test_stream_partial(self):
>          self.assert_no_active_block_jobs()
>  
> -        result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
> +        result = self.vm.qmp('block-stream', device='drive0', base=backing_img)

So, I'd actually prefer to keep _both_ of these block-streams (of
course, checking the result between both cases), or even add a second
self.vm.qmp('block-stream', device='drive0', base=backing_img) after the
first one (the first one streams data from mid into active and rebases
active; the second one is a no-op), for maximal coverage of our behavior.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]