qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v3 4/4] iotests: Add tests for the x-blockdev-del command
Date: Sat, 31 Oct 2015 20:28:35 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 23.10.2015 14:03, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  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 <address@hidden>

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]