Re: [PATCH 4/8] tests/vm: Add configuration to basevm.py

["Alex Bennée" <alex.bennee@linaro.org>]

Robert Foley <robert.foley@linaro.org> writes:

> Added use of a configuration to tests/vm/basevm.py.
> The configuration provides parameters used to configure a VM.
> This allows for providing alternate configurations to the VM being
> created/launched. cpu, machine, memory, and NUMA configuration are all
> examples of configuration which we might want to vary on the VM being created
> or launched.
> This will for example allow for creating an aarch64 vm.
>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> ---
>  tests/vm/basevm.py | 108 ++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 82 insertions(+), 26 deletions(-)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 3b4403ddcb..ec92c8f105 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -32,15 +32,40 @@ import shutil
>  import multiprocessing
>  import traceback
>  
> -SSH_KEY = open(os.path.join(os.path.dirname(__file__),
> -               "..", "keys", "id_rsa")).read()
> -SSH_PUB_KEY = open(os.path.join(os.path.dirname(__file__),
> -                   "..", "keys", "id_rsa.pub")).read()
> -
> +SSH_KEY_FILE = os.path.join(os.path.dirname(__file__),
> +               "..", "keys", "id_rsa")
> +SSH_PUB_KEY_FILE = os.path.join(os.path.dirname(__file__),
> +                   "..", "keys", "id_rsa.pub")
> +SSH_KEY = open(SSH_KEY_FILE).read()
> +SSH_PUB_KEY = open(SSH_PUB_KEY_FILE).read()

Why are we tracking more information about the keyfile than we used to
now? Is this because it's harder to embed keys over paths in the config? 

> +
> +# This is the standard configuration.
> +# Any or all of these can be overridden by
> +# passing in a config argument to the VM constructor.
> +DEFAULT_CONFIG = {
> +    'cpu'             : "max",
> +    'machine'         : 'pc',
> +    'guest_user'      : "qemu",
> +    'guest_pass'      : "qemupass",
> +    'root_pass'       : "qemupass",
> +    'ssh_key_file'    : SSH_KEY_FILE,
> +    'ssh_pub_key_file': SSH_PUB_KEY_FILE,
> +    'memory'          : "4G",
> +    'extra_args'      : [],
> +    'dns'             : "",
> +    'ssh_port'        : 0,
> +    'install_cmds'    : "",
> +    'boot_dev_type'   : "block",
> +    'ssh_timeout'     : 1,
> +}
> +BOOT_DEVICE = {
> +    'block' :  "-drive file={},if=none,id=drive0,cache=writeback "\
> +               "-device virtio-blk,drive=drive0,bootindex=0",
> +    'scsi'  :  "-device virtio-scsi-device,id=scsi "\
> +               "-drive file={},format=raw,if=none,id=hd0 "\
> +               "-device scsi-hd,drive=hd0,bootindex=0",
> +}
>  class BaseVM(object):
> -    GUEST_USER = "qemu"
> -    GUEST_PASS = "qemupass"
> -    ROOT_PASS = "qemupass"

Don't we need these?

>  
>      envvars = [
>          "https_proxy",
> @@ -59,19 +84,26 @@ class BaseVM(object):
>      poweroff = "poweroff"
>      # enable IPv6 networking
>      ipv6 = True
> -    def __init__(self, debug=False, vcpus=None):
> +    def __init__(self, debug=False, vcpus=None, config=None):
>          self._guest = None
> +        # Allow input config to override defaults.
> +        self._config = DEFAULT_CONFIG.copy()
> +        if config != None:
> +            self._config.update(config)
>          self._tmpdir = os.path.realpath(tempfile.mkdtemp(prefix="vm-test-",
>                                                           suffix=".tmp",
>                                                           dir="."))
>          atexit.register(shutil.rmtree, self._tmpdir)
> -
> +        self._config['ssh_key'] = \
> +            open(self._config['ssh_key_file']).read().rstrip()
> +        self._config['ssh_pub_key'] = \
> +            open(self._config['ssh_pub_key_file']).read().rstrip()
>          self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
> -        open(self._ssh_key_file, "w").write(SSH_KEY)
> +        open(self._ssh_key_file, "w").write(self._config['ssh_key'])
>          subprocess.check_call(["chmod", "600", self._ssh_key_file])
>  
>          self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
> -        open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
> +        open(self._ssh_pub_key_file,
>          "w").write(self._config['ssh_pub_key'])

Read as a block I find this confusing:

        self._config['ssh_key'] = \
            open(self._config['ssh_key_file']).read().rstrip()
        self._config['ssh_pub_key'] = \
            open(self._config['ssh_pub_key_file']).read().rstrip()
        self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
        open(self._ssh_key_file, "w").write(self._config['ssh_key'])
        subprocess.check_call(["chmod", "600", self._ssh_key_file])

        self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
        open(self._ssh_pub_key_file, "w").write(self._config['ssh_pub_key'])

We read config['ssh_key_file'] but write out _ssh_pub_key_file which
doesn't seem related. 

<snip>

-- 
Alex Bennée