Re: [PATCH v1 8/8] migration: Test for RAM and vmstate parts

[Vladimir Sementsov-Ogievskiy]

23.03.2022 13:54, Nikita Lapshin wrote:
> From: Nikita Lapshin <nikita.lapshin@virtuozzo.com>
>
> All other parts works just like existed capabilities.
> Though RAM and vmstate are new so here is new test for
> that parts.
>
> Signed-off-by: Nikita Lapshin <nikita.lapshin@openvz.org>
> ---
>   .../tests/migrate-ram-stream-content-test     | 96 +++++++++++++++++++
>   .../tests/migrate-ram-stream-content-test.out |  5 +
>   2 files changed, 101 insertions(+)
>   create mode 100755 tests/qemu-iotests/tests/migrate-ram-stream-content-test
>   create mode 100644 
> tests/qemu-iotests/tests/migrate-ram-stream-content-test.out
>
> diff --git a/tests/qemu-iotests/tests/migrate-ram-stream-content-test 
> b/tests/qemu-iotests/tests/migrate-ram-stream-content-test
> new file mode 100755
> index 0000000000..2855ca4a64
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/migrate-ram-stream-content-test
> @@ -0,0 +1,96 @@
> +#!/usr/bin/env python3
> +# group: rw migration
> +#
> +# Tests for 'no-ram' and 'ram-only' capabilities
> +#
> +# Copyright (c) 2021 Virtuozzo International GmbH.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import json
> +import subprocess
> +import iotests
> +
> +img = os.path.join(iotests.test_dir, 'disk.img')
> +
> +class TestRamCapabilities(iotests.QMPTestCase):
> +    def setUp(self):
> +        iotests.qemu_img('create', '-f', iotests.imgfmt, img, '10M')
> +        self.vm = iotests.VM()
> +        self.vm.launch()
> +        self.vm.qmp('migrate-set-capabilities', capabilities=[
> +            {
> +                'capability': 'events',
> +                'state': True
> +            }
> +        ])
> +
> +    def tearDown(self):
> +        self.vm.shutdown()
> +        os.remove(img)
> +
> +    def check_ram_only(self, output):
> +        str_json = output.decode()
> +        json_obj = json.loads(str_json)
> +
> +        success = False
> +        for key in json_obj:
> +            self.assertTrue("ram" in key)
> +            success = True
> +        self.assertTrue(success)

I'd write it like this:

self.assertTrue(len(json_obj) > 0)
self.assertTrue(all('ram' in key for key in json_obj))

> +
> +    def run_migration(self, no_ram, tmp_stream):
> +        if no_ram:
> +            output = self.vm.qmp('migrate-set-parameters',
> +                    stream_content_list = ['vmstate'])
> +        else:
> +            self.vm.qmp('migrate-set-parameters',
> +                    stream_content_list = ['ram'])

It would look better if just pass stream_content list argument to the function.

> +
> +        self.vm.qmp('migrate', uri='exec:cat>' + tmp_stream)
> +
> +        while True:
> +            event = self.vm.event_wait('MIGRATION')
> +
> +            if event['data']['status'] == 'completed':
> +                break
> +
> +
> +    def test_no_ram(self):
> +        with iotests.FilePath('tmp_stream') as tmp_stream:
> +            self.run_migration(True, tmp_stream)
> +            output = subprocess.run(
> +                ['../../../scripts/analyze-migration.py', '-f', tmp_stream],
> +                stdout=subprocess.PIPE,
> +                stderr=subprocess.STDOUT,
> +                check=False).stdout
> +
> +            self.assertFalse('ram' in output.decode())
> +
> +    def test_ram_only(self):
> +        with iotests.FilePath('tmp_stream') as tmp_stream:
> +            self.run_migration(False, tmp_stream)
> +            output = subprocess.run(
> +                ['../../../scripts/analyze-migration.py', '-f', tmp_stream,
> +                    '--ram-only'],
> +                stdout=subprocess.PIPE,
> +                stderr=subprocess.STDOUT,
> +                check=False).stdout
> +
> +            self.check_ram_only(output)


Hmm both test cases are mostly the same - may be refactored a bit mor eto not 
duplicate.


> +
> +if __name__ == '__main__':
> +    iotests.main(supported_protocols=['file'])
> diff --git a/tests/qemu-iotests/tests/migrate-ram-stream-content-test.out 
> b/tests/qemu-iotests/tests/migrate-ram-stream-content-test.out
> new file mode 100644
> index 0000000000..fbc63e62f8
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/migrate-ram-stream-content-test.out
> @@ -0,0 +1,5 @@
> +..
> +----------------------------------------------------------------------
> +Ran 2 tests
> +
> +OK

Looks good in general

--
Best regards,
Vladimir