Re: [Qemu-devel] [PATCH v3 4/4] iotests: Add tests for the x-blockdev-del command

[Max Reitz <mreitz@redhat.com>]

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

On 23.10.2015 14:03, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  tests/qemu-iotests/139     | 408 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/139.out |   5 +
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 414 insertions(+)
>  create mode 100644 tests/qemu-iotests/139
>  create mode 100644 tests/qemu-iotests/139.out
> 
> diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
> new file mode 100644
> index 0000000..f38f7e4
> --- /dev/null
> +++ b/tests/qemu-iotests/139
> @@ -0,0 +1,408 @@

[...]

> +
> +    # Add a BlockDriverState that has base_img as its backing file

Not quite; while new_img does have base_img as its backing file,
new_img's format BDS explicitly does not.

> +    def addBlockDriverStateOverlay(self, node):
> +        self.checkBlockDriverState(node, False)
> +        iotests.qemu_img('create', '-f', iotests.imgfmt,
> +                         '-b', base_img, new_img, '1M')
> +        opts = {'driver': iotests.imgfmt,
> +                'node-name': node,
> +                'backing': '',
> +                'file': {'driver': 'file',
> +                         'filename': new_img}}
> +        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
> +        self.assert_qmp(result, 'return', {})
> +        self.checkBlockDriverState(node)

[...]

> +    # Add a BlkDebug node
> +    def addBlkDebug(self, debug, node):
> +        self.checkBlockDriverState(node, False)
> +        self.checkBlockDriverState(debug, False)
> +        image = {'driver': iotests.imgfmt,
> +                 'node-name': node,
> +                 'file': {'driver': 'file',
> +                          'filename': base_img}}
> +        opts = {'driver': 'blkdebug',
> +                'node-name': debug,
> +                'image': image}

Normally, blkdebug is supposed to sit between the format and the
protocol BDS.

(You can either make this a test of "raw" instead of "blkdebug", amend
the blkdebug placement, or simply ignore how it's "normally supposed to
be".)

> +        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
> +        self.assert_qmp(result, 'return', {})
> +        self.checkBlockDriverState(node)
> +        self.checkBlockDriverState(debug)
> +
> +    # Add a BlkVerify node
> +    def addBlkVerify(self, blkverify, test, raw):
> +        self.checkBlockDriverState(test, False)
> +        self.checkBlockDriverState(raw, False)
> +        self.checkBlockDriverState(blkverify, False)
> +        iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M')
> +        node_0 = {'driver': iotests.imgfmt,
> +                  'node-name': test,
> +                  'file': {'driver': 'file',
> +                           'filename': base_img}}
> +        node_1 = {'driver': iotests.imgfmt,
> +                  'node-name': raw,
> +                  'file': {'driver': 'file',
> +                           'filename': new_img}}

And, well, this is normally supposed to be a protocol BDS. The use case
of blkverify is not to be a dumbed-down quorum, but to test the block
driver used for the test node.

(Here, your choices are either to drop this test, or again to ignore how
it's normally supposed to be.)

> +        opts = {'driver': 'blkverify',
> +                'node-name': blkverify,
> +                'test': node_0,
> +                'raw': node_1}
> +        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
> +        self.assert_qmp(result, 'return', {})
> +        self.checkBlockDriverState(test)
> +        self.checkBlockDriverState(raw)
> +        self.checkBlockDriverState(blkverify)

Assuming you choose the "ignore" option for both of the above, and you
do something about the *Overlay comment or I simply accept it as "not
that wrong, and it's just a comment in a test":

Reviewed-by: Max Reitz <mreitz@redhat.com>


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