[PATCH 0/8] tests/vm: Add support for aarch64 VMs

[PATCH 0/8] tests/vm: Add support for aarch64 VMs

[Robert Foley]

This patch adds support for 2 aarch64 VMs.  
 - Ubuntu 18.04 aarch64 VM
 - CentOS 8 aarch64 VM

In order to add support for the two new aarch64 VMs, we
generalized and parameterized basevm.py.  
We added a new concept of a configuration, which is really just a 
set of parameters which define how to configure the VM.  
Some examples of parameters are "machine", "memory" and "cpu".
We preserved current default parameters.
Current configuration of pre-existing VMs is supported by default
without need to override default parameters.
 
For example, previously only the 'pc' machine was supported in basevm.py. 
The new aarch64 VMs will override machine to use virt. 
There are a few other examples where we needed to add parameters 
in order to add support for these aarch64 VMs.
In some other cases we added parameters that we thought would be useful
in general, for example username/password, ssh keys, 

In the case of the aarch64 VMs, they override certain parameters
by default.  However, it is also of value to be able to 
dynamically specify other values for these parameters.
Take the case where you create a new VM using vm-build,
but then want to test it using various hardware configurations
such as for example different NUMA topologies. 
Or maybe you want to use a different amount of memory or a different cpu type.

In order to support these use cases we added support
for a configuration .yml file, which allows the user to
specify certain values dynamically such as:
 - machine
 - cpu
 - memory size
 - other qemu args, which allow configuring alternate
   hardware topologies such as NUMA nodes.
 - username, password
 - ssh keys
 For an example of a .yml file, see the included config_example.yml
 
The main use case for using this config.yml file is for where we
are testing/debugging with qemu (vm-build), and need to configure
the VM differently.  However, there is another use case we have
developed, which is a project called lisa-qemu 
(https://github.com/rf972/lisa-qemu).  
This project is an integration between the LISA tool and QEMU.  
This project uses the VMs created by
QEMU's vm-build scripts for use in testing with LISA.  
This use case is similar to the vm-build case in that,
the VM gets created once, and we want to launch the VM with different
configurations (memory, cpu, etc.).
 
As part of developing the scripts for these VMs, we implemented
a few enhancements to help with testing.
For example, we added support for allowing debug mode to
show the ssh output.
We also added support for a new --boot-console option which
will show the boot console as the VM boots up to aid in 
debugging problems during VM boot.

Robert Foley (8):
  tests/vm: Debug mode shows ssh output.
  tests/vm: increased max timeout for vm boot.
  tests/vm: change wait_ssh to optionally wait for root.
  tests/vm: Add configuration to basevm.py
  tests/vm: Added configuration file support
  tests/vm: add --boot-console switch
  tests/vm: Added a new script for ubuntu.aarch64.
  tests/vm: Added a new script for centos.aarch64.

 tests/vm/Makefile.include    |   8 +-
 tests/vm/aarch64vm.py        |  41 +++++++
 tests/vm/basevm.py           | 192 +++++++++++++++++++++++++-----
 tests/vm/centos-8-aarch64.ks |  52 +++++++++
 tests/vm/centos.aarch64      | 218 +++++++++++++++++++++++++++++++++++
 tests/vm/config_example.yml  |  52 +++++++++
 tests/vm/ubuntu.aarch64      | 144 +++++++++++++++++++++++
 7 files changed, 675 insertions(+), 32 deletions(-)
 create mode 100644 tests/vm/aarch64vm.py
 create mode 100644 tests/vm/centos-8-aarch64.ks
 create mode 100755 tests/vm/centos.aarch64
 create mode 100644 tests/vm/config_example.yml
 create mode 100755 tests/vm/ubuntu.aarch64

-- 
2.17.1

[PATCH 1/8] tests/vm: Debug mode shows ssh output.

[Robert Foley]

Add changes to tests/vm/basevm.py so that during debug
 mode we show ssh output.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/basevm.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index ed5dd4f3d0..991115e44b 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -122,11 +122,16 @@ class BaseVM(object):
         return fname
 
     def _ssh_do(self, user, cmd, check):
-        ssh_cmd = ["ssh", "-q", "-t",
+        ssh_cmd = ["ssh",
+                   "-t",
                    "-o", "StrictHostKeyChecking=no",
                    "-o", "UserKnownHostsFile=" + os.devnull,
                    "-o", "ConnectTimeout=1",
                    "-p", self.ssh_port, "-i", self._ssh_key_file]
+        # If not in debug mode, set ssh to quiet mode to
+        # avoid printing the results of commands.
+        if not self.debug:
+            ssh_cmd.append("-q")
         for var in self.envvars:
             ssh_cmd += ['-o', "SendEnv=%s" % var ]
         assert not isinstance(cmd, str)
-- 
2.17.1

Re: [PATCH 1/8] tests/vm: Debug mode shows ssh output.

[Alex Bennée]

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

> Add changes to tests/vm/basevm.py so that during debug
>  mode we show ssh output.

nit: space

>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/vm/basevm.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index ed5dd4f3d0..991115e44b 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -122,11 +122,16 @@ class BaseVM(object):
>          return fname
>  
>      def _ssh_do(self, user, cmd, check):
> -        ssh_cmd = ["ssh", "-q", "-t",
> +        ssh_cmd = ["ssh",
> +                   "-t",
>                     "-o", "StrictHostKeyChecking=no",
>                     "-o", "UserKnownHostsFile=" + os.devnull,
>                     "-o", "ConnectTimeout=1",
>                     "-p", self.ssh_port, "-i", self._ssh_key_file]
> +        # If not in debug mode, set ssh to quiet mode to
> +        # avoid printing the results of commands.
> +        if not self.debug:
> +            ssh_cmd.append("-q")
>          for var in self.envvars:
>              ssh_cmd += ['-o', "SendEnv=%s" % var ]
>          assert not isinstance(cmd, str)


-- 
Alex Bennée

[PATCH 2/8] tests/vm: increased max timeout for vm boot.

[Robert Foley]

Add change to increase timeout waiting for VM to boot.
Needed for some emulation cases where it can take longer
than 5 minutes to boot.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/basevm.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 991115e44b..86908f58ec 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -310,7 +310,7 @@ class BaseVM(object):
     def print_step(self, text):
         sys.stderr.write("### %s ...\n" % text)
 
-    def wait_ssh(self, seconds=300):
+    def wait_ssh(self, seconds=600):
         starttime = datetime.datetime.now()
         endtime = starttime + datetime.timedelta(seconds=seconds)
         guest_up = False
-- 
2.17.1

Re: [PATCH 2/8] tests/vm: increased max timeout for vm boot.

[Philippe Mathieu-Daudé]

Hi Robert,

On 1/24/20 5:53 PM, Robert Foley wrote:
> Add change to increase timeout waiting for VM to boot.
> Needed for some emulation cases where it can take longer
> than 5 minutes to boot.
> 
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> ---
>   tests/vm/basevm.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 991115e44b..86908f58ec 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -310,7 +310,7 @@ class BaseVM(object):
>       def print_step(self, text):
>           sys.stderr.write("### %s ...\n" % text)
>   
> -    def wait_ssh(self, seconds=300):
> +    def wait_ssh(self, seconds=600):
>           starttime = datetime.datetime.now()
>           endtime = starttime + datetime.timedelta(seconds=seconds)
>           guest_up = False
> 

I once suggested "When using TCG, wait longer for a VM to start"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg550610.html

Cleber took some notes about 'kicking a expiring timer' but I can't find 
it. This might be related:
https://trello.com/c/MYdgH4mz/90-delayed-failures

Regards,

Phil.

Re: [PATCH 2/8] tests/vm: increased max timeout for vm boot.

[Robert Foley]

Hi Phil,

> I once suggested "When using TCG, wait longer for a VM to start"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg550610.html
>
Thanks for the pointer.  This increase in timeout under TCG seems just
right for the case we saw.  I will incorporate this into the patch.

Thanks & Regards,
-Rob

On Fri, 24 Jan 2020 at 12:12, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Robert,
>
> On 1/24/20 5:53 PM, Robert Foley wrote:
> > Add change to increase timeout waiting for VM to boot.
> > Needed for some emulation cases where it can take longer
> > than 5 minutes to boot.
> >
> > Signed-off-by: Robert Foley <robert.foley@linaro.org>
> > Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> > ---
> >   tests/vm/basevm.py | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> > index 991115e44b..86908f58ec 100755
> > --- a/tests/vm/basevm.py
> > +++ b/tests/vm/basevm.py
> > @@ -310,7 +310,7 @@ class BaseVM(object):
> >       def print_step(self, text):
> >           sys.stderr.write("### %s ...\n" % text)
> >
> > -    def wait_ssh(self, seconds=300):
> > +    def wait_ssh(self, seconds=600):
> >           starttime = datetime.datetime.now()
> >           endtime = starttime + datetime.timedelta(seconds=seconds)
> >           guest_up = False
> >
>
> I once suggested "When using TCG, wait longer for a VM to start"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg550610.html
>
> Cleber took some notes about 'kicking a expiring timer' but I can't find
> it. This might be related:
> https://trello.com/c/MYdgH4mz/90-delayed-failures
>
> Regards,
>
> Phil.
>

Re: [PATCH 2/8] tests/vm: increased max timeout for vm boot.

[Philippe Mathieu-Daudé]

On 1/24/20 8:00 PM, Robert Foley wrote:
> Hi Phil,
> 
>> I once suggested "When using TCG, wait longer for a VM to start"
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg550610.html
>>
> Thanks for the pointer.  This increase in timeout under TCG seems just
> right for the case we saw.  I will incorporate this into the patch.

Well I'm not sure this is the better way, at the time I posted the 
series (see cover) there was no upstream interest in vmtests with TCG on 
aarch64, so I did not insist.

I'm glad you are improving this area now.

> On Fri, 24 Jan 2020 at 12:12, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Hi Robert,
>>
>> On 1/24/20 5:53 PM, Robert Foley wrote:
>>> Add change to increase timeout waiting for VM to boot.
>>> Needed for some emulation cases where it can take longer
>>> than 5 minutes to boot.
>>>
>>> Signed-off-by: Robert Foley <robert.foley@linaro.org>
>>> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
>>> ---
>>>    tests/vm/basevm.py | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
>>> index 991115e44b..86908f58ec 100755
>>> --- a/tests/vm/basevm.py
>>> +++ b/tests/vm/basevm.py
>>> @@ -310,7 +310,7 @@ class BaseVM(object):
>>>        def print_step(self, text):
>>>            sys.stderr.write("### %s ...\n" % text)
>>>
>>> -    def wait_ssh(self, seconds=300):
>>> +    def wait_ssh(self, seconds=600):
>>>            starttime = datetime.datetime.now()
>>>            endtime = starttime + datetime.timedelta(seconds=seconds)
>>>            guest_up = False
>>>
>>
>> I once suggested "When using TCG, wait longer for a VM to start"
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg550610.html
>>
>> Cleber took some notes about 'kicking a expiring timer' but I can't find
>> it. This might be related:
>> https://trello.com/c/MYdgH4mz/90-delayed-failures
>>
>> Regards,
>>
>> Phil.
>>
> 

[PATCH 3/8] tests/vm: change wait_ssh to optionally wait for root.

[Robert Foley]

Allow wait_ssh to wait for root user to be ready.
This solves the issue where we perform a wait_ssh()
successfully, but the root user is not yet ready
to be logged in.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/basevm.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 86908f58ec..3b4403ddcb 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -310,12 +310,17 @@ class BaseVM(object):
     def print_step(self, text):
         sys.stderr.write("### %s ...\n" % text)
 
-    def wait_ssh(self, seconds=600):
+    def wait_ssh(self, wait_root=False, seconds=600):
         starttime = datetime.datetime.now()
         endtime = starttime + datetime.timedelta(seconds=seconds)
         guest_up = False
         while datetime.datetime.now() < endtime:
-            if self.ssh("exit 0") == 0:
+            if wait_root:
+                if self.ssh("exit 0") == 0 and\
+                   self.ssh_root("exit 0") == 0:
+                    guest_up = True
+                    break
+            elif self.ssh("exit 0") == 0:
                 guest_up = True
                 break
             seconds = (endtime - datetime.datetime.now()).total_seconds()
-- 
2.17.1

Re: [PATCH 3/8] tests/vm: change wait_ssh to optionally wait for root.

[Alex Bennée]

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

> Allow wait_ssh to wait for root user to be ready.
> This solves the issue where we perform a wait_ssh()
> successfully, but the root user is not yet ready
> to be logged in.

So in the case it's the root user we care about...

> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> ---
>  tests/vm/basevm.py | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 86908f58ec..3b4403ddcb 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -310,12 +310,17 @@ class BaseVM(object):
>      def print_step(self, text):
>          sys.stderr.write("### %s ...\n" % text)
>  
> -    def wait_ssh(self, seconds=600):
> +    def wait_ssh(self, wait_root=False, seconds=600):
>          starttime = datetime.datetime.now()
>          endtime = starttime + datetime.timedelta(seconds=seconds)
>          guest_up = False
>          while datetime.datetime.now() < endtime:
> -            if self.ssh("exit 0") == 0:
> +            if wait_root:
> +                if self.ssh("exit 0") == 0 and\
> +                   self.ssh_root("exit 0") == 0:

...why do we need to test both here? 

> +                    guest_up = True
> +                    break
> +            elif self.ssh("exit 0") == 0:

Is this simpler?

    def wait_ssh(self, wait_root=False, seconds=600):
        starttime = datetime.datetime.now()
        endtime = starttime + datetime.timedelta(seconds=seconds)
        guest_up = False
        while datetime.datetime.now() < endtime:
            if wait_root and self.ssh_root("exit 0") == 0:
                guest_up = True
                break
            elif self.ssh("exit 0") == 0:
                guest_up = True
                break
            seconds = (endtime - datetime.datetime.now()).total_seconds()
            logging.debug("%ds before timeout", seconds)
            time.sleep(1)
        if not guest_up:
            raise Exception("Timeout while waiting for guest ssh")


>                  guest_up = True
>                  break
>              seconds = (endtime - datetime.datetime.now()).total_seconds()


-- 
Alex Bennée

Re: [PATCH 3/8] tests/vm: change wait_ssh to optionally wait for root.

[Robert Foley]

On Mon, 27 Jan 2020 at 06:06, Alex Bennée <alex.bennee@linaro.org> wrote:
> > Allow wait_ssh to wait for root user to be ready.
> > This solves the issue where we perform a wait_ssh()
> > successfully, but the root user is not yet ready
> > to be logged in.
>
> So in the case it's the root user we care about...
We care about both the root and guest users.  See below.
> >  tests/vm/basevm.py | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> > index 86908f58ec..3b4403ddcb 100755
> > --- a/tests/vm/basevm.py
> > +++ b/tests/vm/basevm.py
> > @@ -310,12 +310,17 @@ class BaseVM(object):
> >      def print_step(self, text):
> >          sys.stderr.write("### %s ...\n" % text)
> >
> > -    def wait_ssh(self, seconds=600):
> > +    def wait_ssh(self, wait_root=False, seconds=600):
> >          starttime = datetime.datetime.now()
> >          endtime = starttime + datetime.timedelta(seconds=seconds)
> >          guest_up = False
> >          while datetime.datetime.now() < endtime:
> > -            if self.ssh("exit 0") == 0:
> > +            if wait_root:
> > +                if self.ssh("exit 0") == 0 and\
> > +                   self.ssh_root("exit 0") == 0:
>
> ...why do we need to test both here?
We want to make sure the root user is up in
addition to the normal/guest user.  We're trying to add on the root user
since the issue we saw is that the guest user was up, (wait_ssh() completed),
but then when the root user tries to do something we get an error,
since root is not ready yet.

> > +                    guest_up = True
> > +                    break
> > +            elif self.ssh("exit 0") == 0:
>
> Is this simpler?
Certainly simpler.  :)
And simpler seems like the right call here.  But we'll need to call
into wait_ssh() twice,
once with the wait_root option and once without.  But I think this is better
since it makes the code on the caller side more explicit and clear in
that we will
explicitly wait for the guest user and then wait for the root user.

Thanks,
-Rob Foley
>     def wait_ssh(self, wait_root=False, seconds=600):
>         starttime = datetime.datetime.now()
>         endtime = starttime + datetime.timedelta(seconds=seconds)
>         guest_up = False
>         while datetime.datetime.now() < endtime:
>             if wait_root and self.ssh_root("exit 0") == 0:
>                 guest_up = True
>                 break
>             elif self.ssh("exit 0") == 0:
>                 guest_up = True
>                 break
>             seconds = (endtime - datetime.datetime.now()).total_seconds()
>             logging.debug("%ds before timeout", seconds)
>             time.sleep(1)
>         if not guest_up:
>             raise Exception("Timeout while waiting for guest ssh")
>
>
> >                  guest_up = True
> >                  break
> >              seconds = (endtime - datetime.datetime.now()).total_seconds()
>
>
> --
> Alex Bennée

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

[Robert Foley]

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()
+
+# 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"
 
     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'])
 
         self.debug = debug
         self._stderr = sys.stderr
@@ -80,11 +112,14 @@ class BaseVM(object):
             self._stdout = sys.stdout
         else:
             self._stdout = self._devnull
+        netdev = "user,id=vnet,hostfwd=:127.0.0.1:{}-:22"
         self._args = [ \
-            "-nodefaults", "-m", "4G",
-            "-cpu", "max",
-            "-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22" +
-                       (",ipv6=no" if not self.ipv6 else ""),
+            "-nodefaults", "-m", self._config['memory'],
+            "-cpu", self._config['cpu'],
+            "-netdev",
+            netdev.format(self._config['ssh_port']) +
+            (",ipv6=no" if not self.ipv6 else "") +
+            (",dns=" + self._config['dns'] if self._config['dns'] else ""),
             "-device", "virtio-net-pci,netdev=vnet",
             "-vnc", "127.0.0.1:0,to=20"]
         if vcpus and vcpus > 1:
@@ -95,6 +130,25 @@ class BaseVM(object):
             logging.info("KVM not available, not using -enable-kvm")
         self._data_args = []
 
+    def wait_boot(self, wait_string=None):
+        """Wait for the standard string we expect
+           on completion of a normal boot.
+           The user can also choose to override with an
+           alternate string to wait for."""
+        if wait_string is None:
+            if self.login_prompt is None:
+                raise Exception("self.login_prompt not defined")
+            wait_string = self.login_prompt
+        self.console_init()
+        self.console_wait(wait_string)
+
+    def __getattr__(self, name):
+        # Support direct access to config by key.
+        # for example, access self._config['cpu'] by self.cpu
+        if name.lower() in self._config.keys():
+            return self._config[name.lower()]
+        return object.__getattribute__(self, name)
+
     def _download_with_cache(self, url, sha256sum=None, sha512sum=None):
         def check_sha256sum(fname):
             if not sha256sum:
@@ -126,7 +180,8 @@ class BaseVM(object):
                    "-t",
                    "-o", "StrictHostKeyChecking=no",
                    "-o", "UserKnownHostsFile=" + os.devnull,
-                   "-o", "ConnectTimeout=1",
+                   "-o",
+                   "ConnectTimeout={}".format(self._config["ssh_timeout"]),
                    "-p", self.ssh_port, "-i", self._ssh_key_file]
         # If not in debug mode, set ssh to quiet mode to
         # avoid printing the results of commands.
@@ -176,15 +231,15 @@ class BaseVM(object):
                             "virtio-blk,drive=%s,serial=%s,bootindex=1" % (name, name)]
 
     def boot(self, img, extra_args=[]):
-        args = self._args + [
-            "-device", "VGA",
-            "-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
-            "-device", "virtio-blk,drive=drive0,bootindex=0"]
-        args += self._data_args + extra_args
+        boot_dev = BOOT_DEVICE[self._config['boot_dev_type']]
+        boot_params = boot_dev.format(img)
+        args = self._args + boot_params.split(' ')
+        args += self._data_args + extra_args + self._config['extra_args']
+        args += ["-device", "VGA"]
         logging.debug("QEMU args: %s", " ".join(args))
         qemu_bin = os.environ.get("QEMU", "qemu-system-" + self.arch)
         guest = QEMUMachine(binary=qemu_bin, args=args)
-        guest.set_machine('pc')
+        guest.set_machine(self._config['machine'])
         guest.set_console()
         try:
             guest.launch()
@@ -331,7 +386,6 @@ class BaseVM(object):
 
     def shutdown(self):
         self._guest.shutdown()
-
     def wait(self):
         self._guest.wait()
 
@@ -379,15 +433,17 @@ def parse_args(vmcls):
     parser.disable_interspersed_args()
     return parser.parse_args()
 
-def main(vmcls):
+def main(vmcls, config=None):
     try:
+        if config == None:
+            config = {}
         args, argv = parse_args(vmcls)
         if not argv and not args.build_qemu and not args.build_image:
             print("Nothing to do?")
             return 1
         logging.basicConfig(level=(logging.DEBUG if args.debug
                                    else logging.WARN))
-        vm = vmcls(debug=args.debug, vcpus=args.jobs)
+        vm = vmcls(debug=args.debug, vcpus=args.jobs, config=config)
         if args.build_image:
             if os.path.exists(args.image) and not args.force:
                 sys.stderr.writelines(["Image file exists: %s\n" % args.image,
-- 
2.17.1

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

[Alex Bennée]

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

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

[Robert Foley]

On Mon, 27 Jan 2020 at 07:26, Alex Bennée <alex.bennee@linaro.org> wrote:
> > -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?
We now allow the user to override the ssh keys.  Because of this we
need to track
the filename separately from the contents of the file, so that we can
put this default
filename into the DEFAULT_CONFIG below.
We also keep the original SSH_PUB_KEY since it is still
used by some pre-existing VM scripts, and we did not want to break backwards
compatibility for those scripts.
Actually upon further inspection, it looks like we can delete SSH_KEY,
this is no longer needed. :)
>
> > +
> > +# 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?
We don't need these since we moved them up to the new DEFAULT_CONFIG.
These are essentially the default values now since the user
can now override these.
We also handle the cases where these are accessed by existing scripts,
and ensuring backwards compatibility by referencing these values in the
_config (see the code in __getattr__).

This actually brings up a good point that I wanted to mention.
Our initial plan was to leave the existing VM scripts unchanged.
We were thinking that it would also clarify things for a later patch to
simply change references to ROOT_PASS, GUEST_USER/ PASS,
and SSH_PUB_KEY, within the existing VM scripts (centos, openbsd, etc)
to use _config, and then we could get rid of the __getattr__ change entirely.
Actually, we could even put it at the end of this series too.
I think I will add this change to the next version of this patch unless you
think we should keep it separate?
> >
> >      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.
I agree we can clarify this. :) Most of this logic was here previously,
we're just adding the parameterization of the keys.
This is the current flow:
1) copy the key file (config['ssh_key_file']) to a temporary file
(_ssh_pub_key_file)
2) chmod the key file so that ssh is happy with the permissions.
Without this chmod ssh will refuse to use the key file.
It seems to make sense to add a comment here to clarify all this.
It also seems like we could change the name _ssh_pub_key_file to
_ssh_tmp_pub_key_file
to make it more clear it is a temp file.
What do you think, would that be enough to clarify things?

Thanks & Regards,
-Rob
> <snip>

>
> --
> Alex Bennée

[PATCH 5/8] tests/vm: Added configuration file support

[Robert Foley]

Changes to tests/vm/basevm.py to allow accepting a configuration file
as a parameter. Allows for specifying VM options such as
cpu, machine, memory, and arbitrary qemu arguments for specifying options
such as NUMA configuration.
Also added an example config_example.yml.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/basevm.py          | 60 +++++++++++++++++++++++++++++++++++++
 tests/vm/config_example.yml | 52 ++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)
 create mode 100644 tests/vm/config_example.yml

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index ec92c8f105..08a8989ac0 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -31,6 +31,7 @@ import tempfile
 import shutil
 import multiprocessing
 import traceback
+import yaml
 
 SSH_KEY_FILE = os.path.join(os.path.dirname(__file__),
                "..", "keys", "id_rsa")
@@ -396,6 +397,61 @@ class BaseVM(object):
     def qmp(self, *args, **kwargs):
         return self._guest.qmp(*args, **kwargs)
 
+
+def parse_config(config, args):
+    """ Parse yaml config and populate our config structure.
+        The yaml config allows the user to override the
+        defaults for VM parameters.  In many cases these
+        defaults can be overridden without rebuilding the VM."""
+    if args.config:
+        config_file = args.config
+    elif 'QEMU_CONFIG' in os.environ:
+        config_file = os.environ['QEMU_CONFIG']
+    else:
+        return config
+    if not os.path.exists(config_file):
+        raise Exception("config file {} does not exist".format(config_file))
+    with open(config_file) as f:
+        yaml_dict = yaml.safe_load(f)
+    if 'target-conf' in yaml_dict:
+        target_dict = yaml_dict['target-conf']
+        if 'username' in target_dict and target_dict['username'] != 'root':
+            config['guest_user'] = target_dict['username']
+        if 'password' in target_dict:
+            config['root_pass'] = target_dict['password']
+            config['guest_pass'] = target_dict['password']
+        if any (k in target_dict for k in ("ssh_key","ssh_pub_key")) and \
+           not all (k in target_dict for k in ("ssh_key","ssh_pub_key")):
+            missing_key = "ssh_pub_key" \
+              if 'ssh_key' in target_dict else "ssh_key"
+            raise Exception("both ssh_key and ssh_pub_key required. "
+                            "{} key is missing.".format(missing_key))
+        if 'ssh_key' in target_dict:
+            config['ssh_key_file'] = target_dict['ssh_key']
+            if not os.path.exists(config['ssh_key_file']):
+                raise Exception("ssh key file not found.")
+        if 'ssh_pub_key' in target_dict:
+            config['ssh_pub_key_file'] = target_dict['ssh_pub_key']
+            if not os.path.exists(config['ssh_pub_key_file']):
+                raise Exception("ssh pub key file not found.")
+        if 'machine' in target_dict:
+            config['machine'] = target_dict['machine']
+        if 'qemu_args' in target_dict:
+            qemu_args = target_dict['qemu_args']
+            qemu_args = qemu_args.replace('\n', ' ').replace('\r', '')
+            config['extra_args'] = qemu_args.split(' ')
+        if 'memory' in target_dict:
+            config['memory'] = target_dict['memory']
+        if 'dns' in target_dict:
+            config['dns'] = target_dict['dns']
+        if 'cpu' in target_dict:
+            config['cpu'] = target_dict['cpu']
+        if 'ssh_port' in target_dict:
+            config['ssh_port'] = target_dict['ssh_port']
+        if 'install_cmds' in target_dict:
+            config['install_cmds'] = target_dict['install_cmds']
+    return config
+
 def parse_args(vmcls):
 
     def get_default_jobs():
@@ -430,6 +486,9 @@ def parse_args(vmcls):
                       help="Interactively run command")
     parser.add_option("--snapshot", "-s", action="store_true",
                       help="run tests with a snapshot")
+    parser.add_option("--config", "-c", default=None,
+                      help="Provide config yaml for configuration. "\
+                           "See config_example.yaml for example.")
     parser.disable_interspersed_args()
     return parser.parse_args()
 
@@ -441,6 +500,7 @@ def main(vmcls, config=None):
         if not argv and not args.build_qemu and not args.build_image:
             print("Nothing to do?")
             return 1
+        config = parse_config(config, args)
         logging.basicConfig(level=(logging.DEBUG if args.debug
                                    else logging.WARN))
         vm = vmcls(debug=args.debug, vcpus=args.jobs, config=config)
diff --git a/tests/vm/config_example.yml b/tests/vm/config_example.yml
new file mode 100644
index 0000000000..0a1fec3824
--- /dev/null
+++ b/tests/vm/config_example.yml
@@ -0,0 +1,52 @@
+#
+# Example yaml for use by any of the scripts in tests/vm.
+# Can be provided as an environment variable QEMU_CONFIG
+#
+target-conf:
+
+    # If any of the below are not provided, we will just use the qemu defaults.
+
+    # Login username (has to be sudo enabled)
+    #username: qemu
+
+    # Password is used by root and default login user.
+    #password: "qemupass"
+
+    # If one key is provided, both must be provided.
+    #ssh_key: /complete/path/of/your/keyfile/id_rsa
+    #ssh_pub_key: /complete/path/of/your/keyfile/id_rsa.pub
+
+    cpu: max
+    machine: virt,gic_version=3
+    memory: 16G
+
+    # The below is an example for how to configure NUMA topology with
+    # 4 NUMA nodes and 2 different NUMA distances.
+    qemu_args: "-smp cpus=16,sockets=2,cores=8
+                -object memory-backend-ram,size=4G,policy=bind,host-nodes=0,id=ram-node0
+                -object memory-backend-ram,size=4G,policy=bind,host-nodes=0,id=ram-node1
+                -object memory-backend-ram,size=4G,policy=bind,host-nodes=1,id=ram-node2
+                -object memory-backend-ram,size=4G,policy=bind,host-nodes=1,id=ram-node3
+                -numa node,memdev=ram-node0,cpus=0-3,nodeid=0 -numa node,memdev=ram-node1,cpus=4-7,nodeid=1
+                -numa node,memdev=ram-node2,cpus=8-11,nodeid=2 -numa node,memdev=ram-node3,cpus=12-15,nodeid=3
+                -numa dist,src=0,dst=1,val=15 -numa dist,src=2,dst=3,val=15
+                -numa dist,src=0,dst=2,val=20 -numa dist,src=0,dst=3,val=20
+                -numa dist,src=1,dst=2,val=20 -numa dist,src=1,dst=3,val=20"
+
+    # By default we do not set the DNS.
+    # You override the defaults by setting the below.
+    #dns: 1.234.567.89
+
+    # By default we will use a "block" device, but
+    # you can also boot from a "scsi" device.
+    # Just keep in mind your scripts might need to change
+    # As you will have /dev/sda instead of /dev/vda (for block device)
+    #boot_dev_type: "scsi"
+
+    # By default the ssh port is not fixed.
+    # A fixed ssh port makes it easier for automated tests.
+    #ssh_port: 5555
+
+    # To install a different set of packages, provide a command to issue
+    #install_cmds: "apt-get update ; apt-get build-dep -y qemu"
+
-- 
2.17.1

Re: [PATCH 5/8] tests/vm: Added configuration file support

[Alex Bennée]

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

> Changes to tests/vm/basevm.py to allow accepting a configuration file
> as a parameter. Allows for specifying VM options such as
> cpu, machine, memory, and arbitrary qemu arguments for specifying options
> such as NUMA configuration.
> Also added an example config_example.yml.
>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> ---
>  tests/vm/basevm.py          | 60 +++++++++++++++++++++++++++++++++++++
>  tests/vm/config_example.yml | 52 ++++++++++++++++++++++++++++++++
>  2 files changed, 112 insertions(+)
>  create mode 100644 tests/vm/config_example.yml
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index ec92c8f105..08a8989ac0 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -31,6 +31,7 @@ import tempfile
>  import shutil
>  import multiprocessing
>  import traceback
> +import yaml
>  
>  SSH_KEY_FILE = os.path.join(os.path.dirname(__file__),
>                 "..", "keys", "id_rsa")
> @@ -396,6 +397,61 @@ class BaseVM(object):
>      def qmp(self, *args, **kwargs):
>          return self._guest.qmp(*args, **kwargs)
>  
> +
> +def parse_config(config, args):
> +    """ Parse yaml config and populate our config structure.
> +        The yaml config allows the user to override the
> +        defaults for VM parameters.  In many cases these
> +        defaults can be overridden without rebuilding the VM."""
> +    if args.config:
> +        config_file = args.config
> +    elif 'QEMU_CONFIG' in os.environ:
> +        config_file = os.environ['QEMU_CONFIG']
> +    else:
> +        return config
> +    if not os.path.exists(config_file):
> +        raise Exception("config file {} does not exist".format(config_file))
> +    with open(config_file) as f:
> +        yaml_dict = yaml.safe_load(f)
> +    if 'target-conf' in yaml_dict:
> +        target_dict = yaml_dict['target-conf']
> +        if 'username' in target_dict and target_dict['username'] != 'root':
> +            config['guest_user'] = target_dict['username']
> +        if 'password' in target_dict:
> +            config['root_pass'] = target_dict['password']
> +            config['guest_pass'] = target_dict['password']

This seems like impedance matching between two dictionaries. Surely it
would be nicer for the config to be read in fully formed and referable by
the rest of the code. We can also change the internal references.

> +        if any (k in target_dict for k in ("ssh_key","ssh_pub_key")) and \
> +           not all (k in target_dict for k in ("ssh_key","ssh_pub_key")):
> +            missing_key = "ssh_pub_key" \
> +              if 'ssh_key' in target_dict else "ssh_key"
> +            raise Exception("both ssh_key and ssh_pub_key required. "
> +                            "{} key is missing.".format(missing_key))

I guess validation has to be done at some time.. but

> +        if 'ssh_key' in target_dict:
> +            config['ssh_key_file'] = target_dict['ssh_key']
> +            if not os.path.exists(config['ssh_key_file']):
> +                raise Exception("ssh key file not found.")
> +        if 'ssh_pub_key' in target_dict:
> +            config['ssh_pub_key_file'] = target_dict['ssh_pub_key']
> +            if not os.path.exists(config['ssh_pub_key_file']):
> +                raise Exception("ssh pub key file not found.")

here we are both munging dictionaries again before checking the data.
Given we bail with an exception I'm now rethinking if it makes sense to
validate up here. It depends on how many places in the code expect to
use this data.

> +        if 'machine' in target_dict:
> +            config['machine'] = target_dict['machine']
> +        if 'qemu_args' in target_dict:
> +            qemu_args = target_dict['qemu_args']
> +            qemu_args = qemu_args.replace('\n', ' ').replace('\r', '')
> +            config['extra_args'] = qemu_args.split(' ')
> +        if 'memory' in target_dict:
> +            config['memory'] = target_dict['memory']
> +        if 'dns' in target_dict:
> +            config['dns'] = target_dict['dns']
> +        if 'cpu' in target_dict:
> +            config['cpu'] = target_dict['cpu']
> +        if 'ssh_port' in target_dict:
> +            config['ssh_port'] = target_dict['ssh_port']
> +        if 'install_cmds' in target_dict:
> +            config['install_cmds'] = target_dict['install_cmds']
> +    return config
> +
>  def parse_args(vmcls):
>  
>      def get_default_jobs():
> @@ -430,6 +486,9 @@ def parse_args(vmcls):
>                        help="Interactively run command")
>      parser.add_option("--snapshot", "-s", action="store_true",
>                        help="run tests with a snapshot")
> +    parser.add_option("--config", "-c", default=None,
> +                      help="Provide config yaml for configuration. "\
> +                           "See config_example.yaml for example.")
>      parser.disable_interspersed_args()
>      return parser.parse_args()
>  
> @@ -441,6 +500,7 @@ def main(vmcls, config=None):
>          if not argv and not args.build_qemu and not args.build_image:
>              print("Nothing to do?")
>              return 1
> +        config = parse_config(config, args)
>          logging.basicConfig(level=(logging.DEBUG if args.debug
>                                     else logging.WARN))
>          vm = vmcls(debug=args.debug, vcpus=args.jobs, config=config)
> diff --git a/tests/vm/config_example.yml b/tests/vm/config_example.yml
> new file mode 100644
> index 0000000000..0a1fec3824
> --- /dev/null
> +++ b/tests/vm/config_example.yml
> @@ -0,0 +1,52 @@
> +#
> +# Example yaml for use by any of the scripts in tests/vm.
> +# Can be provided as an environment variable QEMU_CONFIG
> +#
> +target-conf:
> +
> +    # If any of the below are not provided, we will just use the qemu defaults.
> +
> +    # Login username (has to be sudo enabled)
> +    #username: qemu
> +
> +    # Password is used by root and default login user.
> +    #password: "qemupass"
> +
> +    # If one key is provided, both must be provided.
> +    #ssh_key: /complete/path/of/your/keyfile/id_rsa
> +    #ssh_pub_key: /complete/path/of/your/keyfile/id_rsa.pub
> +
> +    cpu: max
> +    machine: virt,gic_version=3
> +    memory: 16G
> +
> +    # The below is an example for how to configure NUMA topology with
> +    # 4 NUMA nodes and 2 different NUMA distances.
> +    qemu_args: "-smp cpus=16,sockets=2,cores=8
> +                -object memory-backend-ram,size=4G,policy=bind,host-nodes=0,id=ram-node0
> +                -object memory-backend-ram,size=4G,policy=bind,host-nodes=0,id=ram-node1
> +                -object memory-backend-ram,size=4G,policy=bind,host-nodes=1,id=ram-node2
> +                -object memory-backend-ram,size=4G,policy=bind,host-nodes=1,id=ram-node3
> +                -numa node,memdev=ram-node0,cpus=0-3,nodeid=0 -numa node,memdev=ram-node1,cpus=4-7,nodeid=1
> +                -numa node,memdev=ram-node2,cpus=8-11,nodeid=2 -numa node,memdev=ram-node3,cpus=12-15,nodeid=3
> +                -numa dist,src=0,dst=1,val=15 -numa dist,src=2,dst=3,val=15
> +                -numa dist,src=0,dst=2,val=20 -numa dist,src=0,dst=3,val=20
> +                -numa dist,src=1,dst=2,val=20 -numa dist,src=1,dst=3,val=20"
> +
> +    # By default we do not set the DNS.
> +    # You override the defaults by setting the below.
> +    #dns: 1.234.567.89
> +
> +    # By default we will use a "block" device, but
> +    # you can also boot from a "scsi" device.
> +    # Just keep in mind your scripts might need to change
> +    # As you will have /dev/sda instead of /dev/vda (for block device)
> +    #boot_dev_type: "scsi"
> +
> +    # By default the ssh port is not fixed.
> +    # A fixed ssh port makes it easier for automated tests.
> +    #ssh_port: 5555
> +
> +    # To install a different set of packages, provide a command to issue
> +    #install_cmds: "apt-get update ; apt-get build-dep -y qemu"
> +

Having the example is great. It would be nice to see at least one of the
others converted to a config driven approach as well - is the config
driven approach going to reduce duplication across the various bits of
VM configuring python? Should everything be config driven? Are we in
danger of re-inventing an exiting tooling?

-- 
Alex Bennée

Re: [PATCH 5/8] tests/vm: Added configuration file support

[Robert Foley]

On Mon, 27 Jan 2020 at 07:38, Alex Bennée <alex.bennee@linaro.org> wrote:
> > +        if 'password' in target_dict:
> > +            config['root_pass'] = target_dict['password']
> > +            config['guest_pass'] = target_dict['password']
>
> This seems like impedance matching between two dictionaries. Surely it
> would be nicer for the config to be read in fully formed and referable by
> the rest of the code. We can also change the internal references.

Good point.  Will rework it to avoid this matching.  Basically we can
put the values we
read directly into the config dictionary in one step.
>
> > +        if any (k in target_dict for k in ("ssh_key","ssh_pub_key")) and \
> > +           not all (k in target_dict for k in ("ssh_key","ssh_pub_key")):
> > +            missing_key = "ssh_pub_key" \
> > +              if 'ssh_key' in target_dict else "ssh_key"
> > +            raise Exception("both ssh_key and ssh_pub_key required. "
> > +                            "{} key is missing.".format(missing_key))
>
> I guess validation has to be done at some time.. but
>
> > +        if 'ssh_key' in target_dict:
> > +            config['ssh_key_file'] = target_dict['ssh_key']
> > +            if not os.path.exists(config['ssh_key_file']):
> > +                raise Exception("ssh key file not found.")
> > +        if 'ssh_pub_key' in target_dict:
> > +            config['ssh_pub_key_file'] = target_dict['ssh_pub_key']
> > +            if not os.path.exists(config['ssh_pub_key_file']):
> > +                raise Exception("ssh pub key file not found.")
>
> here we are both munging dictionaries again before checking the data.
> Given we bail with an exception I'm now rethinking if it makes sense to
> validate up here. It depends on how many places in the code expect to
> use this data.

Makes sense.  We will change it to validate in one place just before
we expect to use this data.

> > +    # By default we do not set the DNS.
> > +    # You override the defaults by setting the below.
> > +    #dns: 1.234.567.89
> > +
> > +    # By default we will use a "block" device, but
> > +    # you can also boot from a "scsi" device.
> > +    # Just keep in mind your scripts might need to change
> > +    # As you will have /dev/sda instead of /dev/vda (for block device)
> > +    #boot_dev_type: "scsi"
> > +
> > +    # By default the ssh port is not fixed.
> > +    # A fixed ssh port makes it easier for automated tests.
> > +    #ssh_port: 5555
> > +
> > +    # To install a different set of packages, provide a command to issue
> > +    #install_cmds: "apt-get update ; apt-get build-dep -y qemu"
> > +

>
> Having the example is great. It would be nice to see at least one of the
> others converted to a config driven approach as well

The example we provided was primarily for aarch64, we will add one or
more examples here for the other VMs to
help provide a ready to use template for providing a config file.

> - is the config driven approach going to reduce duplication across the various bits of
> VM configuring python? Should everything be config driven? Are we in
> danger of re-inventing an exiting tooling?

All interesting questions to explore.  Here is my take on this.

One goal we had in mind is to not require a config file for any given
VM.  So in this sense we are
not going in the direction of a config driven approach.
Even the VMs that we added for aarch64 do not require a config file.
The VM scripts will work as is without a config file since the script
itself provides all required defaults.
Our intention was for the config approach to be used to allow
overriding the defaults for any
given VM, to give the flexibility of overriding the parameters.
But on the other hand by not requiring a config file, we make is
simpler and easier to
only override the parameters that the user is interested in.  And also
to limit the cases where
we could generate a non-working VM if the user forgot to provide
certain defaults.

Thanks & Regards,
-Rob

[PATCH 6/8] tests/vm: add --boot-console switch

[Robert Foley]

Added ability to view console during boot via
--boot-console switch.  This helps debug issues that occur
during the boot sequence.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/basevm.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 08a8989ac0..aa8b39beb7 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -489,6 +489,8 @@ def parse_args(vmcls):
     parser.add_option("--config", "-c", default=None,
                       help="Provide config yaml for configuration. "\
                            "See config_example.yaml for example.")
+    parser.add_option("--boot-console", action="store_true",
+                      help="Show console during boot. ")
     parser.disable_interspersed_args()
     return parser.parse_args()
 
@@ -523,6 +525,10 @@ def main(vmcls, config=None):
         if args.snapshot:
             img += ",snapshot=on"
         vm.boot(img)
+        wait_boot = getattr(vm, "wait_boot", None)
+        if args.boot_console and callable(wait_boot):
+            vm.console_init()
+            wait_boot()
         vm.wait_ssh()
     except Exception as e:
         if isinstance(e, SystemExit) and e.code == 0:
-- 
2.17.1

Re: [PATCH 6/8] tests/vm: add --boot-console switch

[Alex Bennée]

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

> Added ability to view console during boot via
> --boot-console switch.  This helps debug issues that occur
> during the boot sequence.
>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> ---
>  tests/vm/basevm.py | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 08a8989ac0..aa8b39beb7 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -489,6 +489,8 @@ def parse_args(vmcls):
>      parser.add_option("--config", "-c", default=None,
>                        help="Provide config yaml for configuration. "\
>                             "See config_example.yaml for example.")
> +    parser.add_option("--boot-console", action="store_true",
> +                      help="Show console during boot. ")
>      parser.disable_interspersed_args()
>      return parser.parse_args()
>  
> @@ -523,6 +525,10 @@ def main(vmcls, config=None):
>          if args.snapshot:
>              img += ",snapshot=on"
>          vm.boot(img)
> +        wait_boot = getattr(vm, "wait_boot", None)

Didn't we add a __getattr method, so we can do self._config['wait_boot']

> +        if args.boot_console and callable(wait_boot):
> +            vm.console_init()
> +            wait_boot()

isn't wait_boot always callable because it's part of the basevm?

>          vm.wait_ssh()
>      except Exception as e:
>          if isinstance(e, SystemExit) and e.code == 0:


-- 
Alex Bennée

Re: [PATCH 6/8] tests/vm: add --boot-console switch

[Robert Foley]

On Mon, 27 Jan 2020 at 07:56, Alex Bennée <alex.bennee@linaro.org> wrote:
> > @@ -523,6 +525,10 @@ def main(vmcls, config=None):
> >          if args.snapshot:
> >              img += ",snapshot=on"
> >          vm.boot(img)
> > +        wait_boot = getattr(vm, "wait_boot", None)
>
> Didn't we add a __getattr method, so we can do self._config['wait_boot']
>
> > +        if args.boot_console and callable(wait_boot):
> > +            vm.console_init()
> > +            wait_boot()
>
> isn't wait_boot always callable because it's part of the basevm?

My bad.  Missed changing this after moving the method into the base class.
Yes, we should change it to something simpler like:
if args.boot_console:
            vm.console_init()
            self.wait_boot()

Thanks & Regards,
-Rob
>
> >          vm.wait_ssh()
> >      except Exception as e:
> >          if isinstance(e, SystemExit) and e.code == 0:
>
>
> --
> Alex Bennée

[PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

ubuntu.aarch64 provides a script to create an Ubuntu 18.04 VM.
Another new file is also added aarch64vm.py, which is a module with
common methods used by aarch64 VMs, such as how to create the
flash images.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/Makefile.include |   7 +-
 tests/vm/aarch64vm.py     |  41 +++++++++++
 tests/vm/ubuntu.aarch64   | 144 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 2 deletions(-)
 create mode 100644 tests/vm/aarch64vm.py
 create mode 100755 tests/vm/ubuntu.aarch64

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 9e7c46a473..966b417ba7 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -2,7 +2,7 @@
 
 .PHONY: vm-build-all vm-clean-all
 
-IMAGES := ubuntu.i386 freebsd netbsd openbsd centos fedora
+IMAGES := ubuntu.i386 freebsd netbsd openbsd centos fedora ubuntu.aarch64
 IMAGES_DIR := $(HOME)/.cache/qemu-vm/images
 IMAGE_FILES := $(patsubst %, $(IMAGES_DIR)/%.img, $(IMAGES))
 
@@ -18,6 +18,7 @@ vm-help vm-test:
 	@echo "  vm-build-openbsd                - Build QEMU in OpenBSD VM"
 	@echo "  vm-build-centos                 - Build QEMU in CentOS VM, with Docker"
 	@echo "  vm-build-fedora                 - Build QEMU in Fedora VM"
+	@echo "  vm-build-ubuntu.aarch64         - Build QEMU in ubuntu aarch64 VM"
 	@echo ""
 	@echo "  vm-build-all                    - Build QEMU in all VMs"
 	@echo "  vm-clean-all                    - Clean up VM images"
@@ -35,6 +36,8 @@ vm-help vm-test:
 	@echo "    V=1				 - Enable verbose ouput on host and guest commands"
 	@echo "    QEMU=/path/to/qemu		 - Change path to QEMU binary"
 	@echo "    QEMU_IMG=/path/to/qemu-img	 - Change path to qemu-img tool"
+	@echo "    QEMU_CONFIG=/path/conf.yml    - Change path to VM configuration .yml file."
+	@echo "                                    See config_example.yml for file format details."
 
 vm-build-all: $(addprefix vm-build-, $(IMAGES))
 
@@ -80,7 +83,7 @@ vm-boot-serial-%: $(IMAGES_DIR)/%.img
 
 vm-boot-ssh-%: $(IMAGES_DIR)/%.img
 	$(call quiet-command, \
-		$(SRC_PATH)/tests/vm/$* \
+		$(PYTHON) $(SRC_PATH)/tests/vm/$* \
 		$(if $(J),--jobs $(J)) \
 		--image "$<" \
 		--interactive \
diff --git a/tests/vm/aarch64vm.py b/tests/vm/aarch64vm.py
new file mode 100644
index 0000000000..43f841571f
--- /dev/null
+++ b/tests/vm/aarch64vm.py
@@ -0,0 +1,41 @@
+#!/usr/bin/env python
+#
+# VM testing aarch64 library
+#
+# Copyright 2020 Linaro
+#
+# Authors:
+#  Robert Foley <robert.foley@linaro.org>
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+#
+import os
+import sys
+import subprocess
+import basevm
+
+
+def create_flash_images():
+    """Creates the appropriate pflash files
+       for an aarch64 VM."""
+    subprocess.check_call(["dd", "if=/dev/zero", "of=flash0.img",
+                           "bs=1M", "count=64"])
+    # A reliable way to get the QEMU EFI image is via an installed package.
+    efi_img = "/usr/share/qemu-efi-aarch64/QEMU_EFI.fd"
+    if not os.path.exists(efi_img):
+        sys.stderr.write("*** {} is missing\n".format(efi_img))
+        sys.stderr.write("*** please install qemu-efi-aarch64 package\n")
+        exit(3)
+    subprocess.check_call(["dd", "if={}".format(efi_img),
+                           "of=flash0.img", "conv=notrunc"])
+    subprocess.check_call(["dd", "if=/dev/zero",
+                           "of=flash1.img", "bs=1M", "count=64"])
+
+def get_pflash_args():
+    """Returns a string that can be used to
+       boot qemu using the appropriate pflash files
+       for aarch64."""
+    pflash_args = "-drive file=flash0.img,format=raw,if=pflash "\
+                  "-drive file=flash1.img,format=raw,if=pflash"
+    return pflash_args.split(" ")
diff --git a/tests/vm/ubuntu.aarch64 b/tests/vm/ubuntu.aarch64
new file mode 100755
index 0000000000..941f7f5166
--- /dev/null
+++ b/tests/vm/ubuntu.aarch64
@@ -0,0 +1,144 @@
+#!/usr/bin/env python
+#
+# Ubuntu aarch64 image
+#
+# Copyright 2020 Linaro
+#
+# Authors:
+#  Robert Foley <robert.foley@linaro.org>
+#  Originally based on ubuntu.i386 Fam Zheng <famz@redhat.com>
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+import time
+import aarch64vm
+
+DEFAULT_CONFIG = {
+    'cpu'          : "max",
+    'machine'      : "virt,gic-version=max",
+    'install_cmds' : "apt-get update,"\
+                     "apt-get build-dep -y qemu,"\
+                     "apt-get install -y libfdt-dev flex bison",
+    # We increase beyond the default time since during boot
+    # it can take some time (many seconds) to log into the VM
+    # especially using softmmu.
+    'ssh_timeout'  : 60,
+}
+
+class UbuntuAarch64VM(basevm.BaseVM):
+    name = "ubuntu.aarch64"
+    arch = "aarch64"
+    image_name = "ubuntu-18.04-server-cloudimg-arm64.img"
+    image_link = "https://cloud-images.ubuntu.com/releases/18.04/release/" + image_name
+    login_prompt = "ubuntu-guest login:"
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d);
+        sudo chmod a+r /dev/vdb;
+        tar --checkpoint=.10 -xf /dev/vdb;
+        ./configure {configure_opts};
+        make --output-sync {target} -j{jobs} {verbose};
+    """
+    def _gen_cloud_init_iso(self):
+        cidir = self._tmpdir
+        mdata = open(os.path.join(cidir, "meta-data"), "w")
+        mdata.writelines(["instance-id: ubuntu-vm-0\n",
+                          "local-hostname: ubuntu-guest\n"])
+        mdata.close()
+        udata = open(os.path.join(cidir, "user-data"), "w")
+        print("guest user:pw {}:{}".format(self.GUEST_USER, self.GUEST_PASS))
+        udata.writelines(["#cloud-config\n",
+                          "chpasswd:\n",
+                          "  list: |\n",
+                          "    root:%s\n" % self.ROOT_PASS,
+                          "    %s:%s\n" % (self.GUEST_USER, self.GUEST_PASS),
+                          "  expire: False\n",
+                          "users:\n",
+                          "  - name: %s\n" % self.GUEST_USER,
+                          "    sudo: ALL=(ALL) NOPASSWD:ALL\n",
+                          "    ssh-authorized-keys:\n",
+                          "    - %s\n" % self.ssh_pub_key,
+                          "  - name: root\n",
+                          "    ssh-authorized-keys:\n",
+                          "    - %s\n" % self.ssh_pub_key,
+                          "locale: en_US.UTF-8\n"])
+        proxy = os.environ.get("http_proxy")
+        if not proxy is None:
+            udata.writelines(["apt:\n",
+                              "  proxy: %s" % proxy])
+        udata.close()
+        subprocess.check_call(["genisoimage", "-output", "cloud-init.iso",
+                               "-volid", "cidata", "-joliet", "-rock",
+                               "user-data", "meta-data"],
+                               cwd=cidir,
+                               stdin=self._devnull, stdout=self._stdout,
+                               stderr=self._stdout)
+
+        return os.path.join(cidir, "cloud-init.iso")
+
+    def boot(self, img, extra_args=None):
+        aarch64vm.create_flash_images()
+        default_args = aarch64vm.get_pflash_args()
+        if extra_args:
+            extra_args.extend(default_args)
+        else:
+            extra_args = default_args
+        # We always add these performance tweaks
+        # because without them, we boot so slowly that we
+        # can time out finding the boot efi device.
+        if os.geteuid() != 0:
+            extra_args.extend(["-accel", "tcg,thread=multi"])
+        if '-smp' not in extra_args and \
+           '-smp' not in self._config['extra_args'] and \
+           '-smp' not in self._args:
+            # Only add if not already there to give caller option to change it.
+            extra_args.extend(["-smp", "8"])
+
+        # We have overridden boot() since aarch64 has additional parameters.
+        # Call down to the base class method.
+        super(UbuntuAarch64VM, self).boot(img, extra_args=extra_args)
+
+    def build_image(self, img):
+        os_img = self._download_with_cache(self.image_link)
+        img_tmp = img + ".tmp"
+        subprocess.check_call(["cp", "-f", os_img, img_tmp])
+        subprocess.check_call(["qemu-img", "resize", img_tmp, "+50G"])
+        ci_img = self._gen_cloud_init_iso()
+
+        self.boot(img_tmp, extra_args = ["-cdrom", ci_img])
+        self.wait_ssh(wait_root=True)
+        # Fix for slow ssh login.
+        self.ssh_root("chmod -x /etc/update-motd.d/*")
+        self.ssh_root("touch /etc/cloud/cloud-init.disabled")
+        # Disable auto upgrades.
+        # We want to keep the VM system state stable.
+        self.ssh_root('sed -ie \'s/"1"/"0"/g\' /etc/apt/apt.conf.d/20auto-upgrades')
+
+        # If the user chooses *not* to do the second phase,
+        # then we will jump right to the graceful shutdown
+        if self._config['install_cmds'] != "":
+            # Don't check the status in case the guest hang up too quickly
+            self.ssh_root("sync && reboot")
+
+            self.wait_ssh(wait_root=True)
+            # The previous update sometimes doesn't survive a reboot, so do it again
+            self.ssh_root("sed -ie s/^#\ deb-src/deb-src/g /etc/apt/sources.list")
+
+            # Issue the install commands.
+            # This can be overriden by the user in the config .yml.
+            install_cmds = self._config['install_cmds'].split(',')
+            for cmd in install_cmds:
+                self.ssh_root(cmd)
+        self.graceful_shutdown()
+        self.wait()
+        os.rename(img_tmp, img)
+        return 0
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(UbuntuAarch64VM, DEFAULT_CONFIG))
-- 
2.17.1

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Alex Bennée]

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

> ubuntu.aarch64 provides a script to create an Ubuntu 18.04 VM.
> Another new file is also added aarch64vm.py, which is a module with
> common methods used by aarch64 VMs, such as how to create the
> flash images.
>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> ---
>  tests/vm/Makefile.include |   7 +-
>  tests/vm/aarch64vm.py     |  41 +++++++++++
>  tests/vm/ubuntu.aarch64   | 144 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 190 insertions(+), 2 deletions(-)
>  create mode 100644 tests/vm/aarch64vm.py
>  create mode 100755 tests/vm/ubuntu.aarch64
>
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index 9e7c46a473..966b417ba7 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -2,7 +2,7 @@
>  
>  .PHONY: vm-build-all vm-clean-all
>  
> -IMAGES := ubuntu.i386 freebsd netbsd openbsd centos fedora
> +IMAGES := ubuntu.i386 freebsd netbsd openbsd centos fedora ubuntu.aarch64
>  IMAGES_DIR := $(HOME)/.cache/qemu-vm/images
>  IMAGE_FILES := $(patsubst %, $(IMAGES_DIR)/%.img, $(IMAGES))
>  
> @@ -18,6 +18,7 @@ vm-help vm-test:
>  	@echo "  vm-build-openbsd                - Build QEMU in OpenBSD VM"
>  	@echo "  vm-build-centos                 - Build QEMU in CentOS VM, with Docker"
>  	@echo "  vm-build-fedora                 - Build QEMU in Fedora VM"
> +	@echo "  vm-build-ubuntu.aarch64         - Build QEMU in ubuntu aarch64 VM"
>  	@echo ""
>  	@echo "  vm-build-all                    - Build QEMU in all VMs"
>  	@echo "  vm-clean-all                    - Clean up VM images"
> @@ -35,6 +36,8 @@ vm-help vm-test:
>  	@echo "    V=1				 - Enable verbose ouput on host and guest commands"
>  	@echo "    QEMU=/path/to/qemu		 - Change path to QEMU binary"
>  	@echo "    QEMU_IMG=/path/to/qemu-img	 - Change path to qemu-img tool"
> +	@echo "    QEMU_CONFIG=/path/conf.yml    - Change path to VM configuration .yml file."
> +	@echo "                                    See config_example.yml for file format details."
>  
>  vm-build-all: $(addprefix vm-build-, $(IMAGES))
>  
> @@ -80,7 +83,7 @@ vm-boot-serial-%: $(IMAGES_DIR)/%.img
>  
>  vm-boot-ssh-%: $(IMAGES_DIR)/%.img
>  	$(call quiet-command, \
> -		$(SRC_PATH)/tests/vm/$* \
> +		$(PYTHON) $(SRC_PATH)/tests/vm/$* \

This seems like it should be in a different patch.

>  		$(if $(J),--jobs $(J)) \
>  		--image "$<" \
>  		--interactive \
> diff --git a/tests/vm/aarch64vm.py b/tests/vm/aarch64vm.py
> new file mode 100644
> index 0000000000..43f841571f
> --- /dev/null
> +++ b/tests/vm/aarch64vm.py
> @@ -0,0 +1,41 @@
> +#!/usr/bin/env python
> +#
> +# VM testing aarch64 library
> +#
> +# Copyright 2020 Linaro
> +#
> +# Authors:
> +#  Robert Foley <robert.foley@linaro.org>
> +#
> +# This code is licensed under the GPL version 2 or later.  See
> +# the COPYING file in the top-level directory.
> +#
> +import os
> +import sys
> +import subprocess
> +import basevm
> +
> +
> +def create_flash_images():
> +    """Creates the appropriate pflash files
> +       for an aarch64 VM."""
> +    subprocess.check_call(["dd", "if=/dev/zero", "of=flash0.img",
> +                           "bs=1M", "count=64"])
> +    # A reliable way to get the QEMU EFI image is via an installed package.
> +    efi_img = "/usr/share/qemu-efi-aarch64/QEMU_EFI.fd"
> +    if not os.path.exists(efi_img):
> +        sys.stderr.write("*** {} is missing\n".format(efi_img))
> +        sys.stderr.write("*** please install qemu-efi-aarch64 package\n")
> +        exit(3)
> +    subprocess.check_call(["dd", "if={}".format(efi_img),
> +                           "of=flash0.img", "conv=notrunc"])
> +    subprocess.check_call(["dd", "if=/dev/zero",
> +                           "of=flash1.img", "bs=1M", "count=64"])
> +
> +def get_pflash_args():
> +    """Returns a string that can be used to
> +       boot qemu using the appropriate pflash files
> +       for aarch64."""
> +    pflash_args = "-drive file=flash0.img,format=raw,if=pflash "\
> +                  "-drive file=flash1.img,format=raw,if=pflash"
> +    return pflash_args.split(" ")
> diff --git a/tests/vm/ubuntu.aarch64 b/tests/vm/ubuntu.aarch64
> new file mode 100755
> index 0000000000..941f7f5166
> --- /dev/null
> +++ b/tests/vm/ubuntu.aarch64
> @@ -0,0 +1,144 @@
> +#!/usr/bin/env python
> +#
> +# Ubuntu aarch64 image
> +#
> +# Copyright 2020 Linaro
> +#
> +# Authors:
> +#  Robert Foley <robert.foley@linaro.org>
> +#  Originally based on ubuntu.i386 Fam Zheng <famz@redhat.com>
> +#
> +# This code is licensed under the GPL version 2 or later.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import os
> +import sys
> +import subprocess
> +import basevm
> +import time
> +import aarch64vm
> +
> +DEFAULT_CONFIG = {
> +    'cpu'          : "max",
> +    'machine'      : "virt,gic-version=max",

According to virt.c:

  Valid values are 2, 3 and host

but max should work on TCG. However we need a more recent QEMU than my
system one for it to work. Otherwise you see:

  DEBUG:qemu.machine:Error launching VM
  DEBUG:qemu.machine:Command: 'qemu-system-aarch64 -display none -vga none -chardev socket,id=mon,path=/var/tmp/qemu-18443-monitor.sock -mon chardev=mon,mode=control -machine
  virt,gic-version=max -chardev socket,id=console,path=/var/tmp/qemu-18443-console.sock,server,nowait -serial chardev:console -nodefaults -m 4G -cpu max -netdev user,id=vnet,hostfwd=:127.0.0.1:0-:22 -device virtio-net-pci,netdev=vnet -vnc 127.0.0.1:0,to=20 -drive file=/home/alex.bennee/.cache/qemu-vm/images/ubuntu.aarch64.img.tmp,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -cdrom /home/alex.bennee/lsrc/qemu.git/builds/all/vm-test-v8jttvl3.tmp/cloud-init.iso -drive file=flash0.img,format=raw,if=pflash -drive file=flash1.img,format=raw,if=pflash -accel tcg,thread=multi -smp 8 -device VGA'
  DEBUG:qemu.machine:Output: 'qemu-system-aarch64: Invalid gic-version value\nValid values are 3, 2, host.\n'
  ERROR:root:Failed to launch QEMU, command line:
  ERROR:root:qemu-system-aarch64 -nodefaults -m 4G -cpu max -netdev user,id=vnet,hostfwd=:127.0.0.1:0-:22 -device virtio-net-pci,netdev=vnet -vnc 127.0.0.1:0,to=20 -drive file=/home/alex.bennee/.cache/qemu-vm/images/ubuntu.aarch64.img.tmp,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -cdrom /home/alex.bennee/lsrc/qemu.git/builds/all/vm-test-v8jttvl3.tmp/cloud-init.iso -drive file=flash0.img,format=raw,if=pflash -drive file=flash1.img,format=raw,if=pflash -accel tcg,thread=multi -smp 8 -device VGA
  ERROR:root:Log:
  ERROR:root:qemu-system-aarch64: Invalid gic-version value
  Valid values are 3, 2, host.

  ERROR:root:QEMU version >= 2.10 is required
  Failed to prepare guest environment
  Traceback (most recent call last):
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/basevm.py", line 514, in main
      return vm.build_image(args.image)
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/ubuntu.aarch64", line 114, in build_image
      self.boot(img_tmp, extra_args = ["-cdrom", ci_img])
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/ubuntu.aarch64", line 105, in boot
      super(UbuntuAarch64VM, self).boot(img, extra_args=extra_args)
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/basevm.py", line 246, in boot
      guest.launch()
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/../../python/qemu/machine.py", line 302, in launch
      self._launch()
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/../../python/qemu/machine.py", line 329, in _launch
      self._post_launch()
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/../../python/qemu/machine.py", line 274, in _post_launch
      self._qmp.accept()
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/../../python/qemu/qmp.py", line 157, in accept
      return self.__negotiate_capabilities()
    File "/home/alex.bennee/lsrc/qemu.git/tests/vm/../../python/qemu/qmp.py", line 73, in __negotiate_capabilities
      raise QMPConnectError
  qemu.qmp.QMPConnectError
  /home/alex.bennee/lsrc/qemu.git/tests/vm/Makefile.include:51: recipe for target '/home/alex.bennee/.cache/qemu-vm/images/ubuntu.aarch64.img' failed
  make: *** [/home/alex.bennee/.cache/qemu-vm/images/ubuntu.aarch64.img] Error 2


> +    'install_cmds' : "apt-get update,"\
> +                     "apt-get build-dep -y qemu,"\
> +                     "apt-get install -y libfdt-dev flex bison",
> +    # We increase beyond the default time since during boot
> +    # it can take some time (many seconds) to log into the VM
> +    # especially using softmmu.
> +    'ssh_timeout'  : 60,
> +}
> +
> +class UbuntuAarch64VM(basevm.BaseVM):
> +    name = "ubuntu.aarch64"
> +    arch = "aarch64"
> +    image_name = "ubuntu-18.04-server-cloudimg-arm64.img"
> +    image_link = "https://cloud-images.ubuntu.com/releases/18.04/release/" + image_name
> +    login_prompt = "ubuntu-guest login:"
> +    BUILD_SCRIPT = """
> +        set -e;
> +        cd $(mktemp -d);
> +        sudo chmod a+r /dev/vdb;
> +        tar --checkpoint=.10 -xf /dev/vdb;
> +        ./configure {configure_opts};
> +        make --output-sync {target} -j{jobs} {verbose};
> +    """
> +    def _gen_cloud_init_iso(self):
> +        cidir = self._tmpdir
> +        mdata = open(os.path.join(cidir, "meta-data"), "w")
> +        mdata.writelines(["instance-id: ubuntu-vm-0\n",
> +                          "local-hostname: ubuntu-guest\n"])
> +        mdata.close()
> +        udata = open(os.path.join(cidir, "user-data"), "w")
> +        print("guest user:pw {}:{}".format(self.GUEST_USER, self.GUEST_PASS))
> +        udata.writelines(["#cloud-config\n",
> +                          "chpasswd:\n",
> +                          "  list: |\n",
> +                          "    root:%s\n" % self.ROOT_PASS,
> +                          "    %s:%s\n" % (self.GUEST_USER, self.GUEST_PASS),
> +                          "  expire: False\n",
> +                          "users:\n",
> +                          "  - name: %s\n" % self.GUEST_USER,
> +                          "    sudo: ALL=(ALL) NOPASSWD:ALL\n",
> +                          "    ssh-authorized-keys:\n",
> +                          "    - %s\n" % self.ssh_pub_key,
> +                          "  - name: root\n",
> +                          "    ssh-authorized-keys:\n",
> +                          "    - %s\n" % self.ssh_pub_key,
> +                          "locale: en_US.UTF-8\n"])
> +        proxy = os.environ.get("http_proxy")
> +        if not proxy is None:
> +            udata.writelines(["apt:\n",
> +                              "  proxy: %s" % proxy])
> +        udata.close()
> +        subprocess.check_call(["genisoimage", "-output", "cloud-init.iso",
> +                               "-volid", "cidata", "-joliet", "-rock",
> +                               "user-data", "meta-data"],
> +                               cwd=cidir,
> +                               stdin=self._devnull, stdout=self._stdout,
> +                               stderr=self._stdout)
> +
> +        return os.path.join(cidir, "cloud-init.iso")

It seems this function is proliferating. It certainly seems common
enough to be basevm functionality.

> +
> +    def boot(self, img, extra_args=None):
> +        aarch64vm.create_flash_images()
> +        default_args = aarch64vm.get_pflash_args()
> +        if extra_args:
> +            extra_args.extend(default_args)
> +        else:
> +            extra_args = default_args
> +        # We always add these performance tweaks
> +        # because without them, we boot so slowly that we
> +        # can time out finding the boot efi device.
> +        if os.geteuid() != 0:
> +            extra_args.extend(["-accel", "tcg,thread=multi"])

Hmmm thread=multi should already be enabled by default where it is safe
to do so. Also what has it to do with euid?

> +        if '-smp' not in extra_args and \
> +           '-smp' not in self._config['extra_args'] and \
> +           '-smp' not in self._args:
> +            # Only add if not already there to give caller option to change it.
> +            extra_args.extend(["-smp", "8"])
> +
> +        # We have overridden boot() since aarch64 has additional parameters.
> +        # Call down to the base class method.
> +        super(UbuntuAarch64VM, self).boot(img, extra_args=extra_args)
> +
> +    def build_image(self, img):
> +        os_img = self._download_with_cache(self.image_link)
> +        img_tmp = img + ".tmp"
> +        subprocess.check_call(["cp", "-f", os_img, img_tmp])
> +        subprocess.check_call(["qemu-img", "resize", img_tmp, "+50G"])
> +        ci_img = self._gen_cloud_init_iso()
> +
> +        self.boot(img_tmp, extra_args = ["-cdrom", ci_img])
> +        self.wait_ssh(wait_root=True)
> +        # Fix for slow ssh login.
> +        self.ssh_root("chmod -x /etc/update-motd.d/*")
> +        self.ssh_root("touch /etc/cloud/cloud-init.disabled")
> +        # Disable auto upgrades.
> +        # We want to keep the VM system state stable.
> +        self.ssh_root('sed -ie \'s/"1"/"0"/g\' /etc/apt/apt.conf.d/20auto-upgrades')
> +
> +        # If the user chooses *not* to do the second phase,
> +        # then we will jump right to the graceful shutdown
> +        if self._config['install_cmds'] != "":
> +            # Don't check the status in case the guest hang up too quickly
> +            self.ssh_root("sync && reboot")
> +
> +            self.wait_ssh(wait_root=True)
> +            # The previous update sometimes doesn't survive a reboot, so do it again
> +            self.ssh_root("sed -ie s/^#\ deb-src/deb-src/g /etc/apt/sources.list")
> +
> +            # Issue the install commands.
> +            # This can be overriden by the user in the config .yml.
> +            install_cmds = self._config['install_cmds'].split(',')
> +            for cmd in install_cmds:
> +                self.ssh_root(cmd)
> +        self.graceful_shutdown()
> +        self.wait()
> +        os.rename(img_tmp, img)
> +        return 0
> +
> +if __name__ == "__main__":
> +    sys.exit(basevm.main(UbuntuAarch64VM, DEFAULT_CONFIG))


-- 
Alex Bennée

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

On Mon, 27 Jan 2020 at 10:01, Alex Bennée <alex.bennee@linaro.org> wrote:
> >  vm-boot-ssh-%: $(IMAGES_DIR)/%.img
> >       $(call quiet-command, \
> > -             $(SRC_PATH)/tests/vm/$* \
> > +             $(PYTHON) $(SRC_PATH)/tests/vm/$* \
>
> This seems like it should be in a different patch.

Good point, will move it to a different patch.

> > +
> > +DEFAULT_CONFIG = {
> > +    'cpu'          : "max",
> > +    'machine'      : "virt,gic-version=max",
>
> According to virt.c:
>
>   Valid values are 2, 3 and host
>
> but max should work on TCG. However we need a more recent QEMU than my
> system one for it to work. Otherwise you see:
>
>   DEBUG:qemu.machine:Error launching VM

Good point.  We were trying to avoid having different values for KVM
vs TCG, which we
could do with the latest QEMU.
We will update this to make sure this works with older versions of QEMU as well.

On my system I have qemu 2.11.1 installed by default.
It seems that we will need the following defaults based on our environment.

For KVM we end up with the below args since max cpu and max
gic-version is not available.
kvm:  -cpu host -machine virt,gic-version=host

For TCG max cpu is also not available: qemu-system-aarch64: unable to
find CPU model 'max',
so we pick cortex-a57.
TCG: -cpu cortex-a57 -machine virt,gic-version=3

I suppose we could check the version of QEMU and use the above
defaults only for earlier versions of QEMU.
This is something we will probably move to aarch64vm.py since it is common.

> > +class UbuntuAarch64VM(basevm.BaseVM):
> > +    name = "ubuntu.aarch64"
> > +    arch = "aarch64"
> > +    image_name = "ubuntu-18.04-server-cloudimg-arm64.img"
> > +    image_link = "https://cloud-images.ubuntu.com/releases/18.04/release/" + image_name
> > +    login_prompt = "ubuntu-guest login:"
> > +    BUILD_SCRIPT = """
> > +        set -e;
> > +        cd $(mktemp -d);
> > +        sudo chmod a+r /dev/vdb;
> > +        tar --checkpoint=.10 -xf /dev/vdb;
> > +        ./configure {configure_opts};
> > +        make --output-sync {target} -j{jobs} {verbose};
> > +    """
> > +    def _gen_cloud_init_iso(self):
__snip__
> > +
> > +        return os.path.join(cidir, "cloud-init.iso")
>
> It seems this function is proliferating. It certainly seems common
> enough to be basevm functionality.

Makes sense.  Will look at making this common to basevm.

>
> > +
> > +    def boot(self, img, extra_args=None):
> > +        aarch64vm.create_flash_images()
> > +        default_args = aarch64vm.get_pflash_args()
> > +        if extra_args:
> > +            extra_args.extend(default_args)
> > +        else:
> > +            extra_args = default_args
> > +        # We always add these performance tweaks
> > +        # because without them, we boot so slowly that we
> > +        # can time out finding the boot efi device.
> > +        if os.geteuid() != 0:
> > +            extra_args.extend(["-accel", "tcg,thread=multi"])
>
> Hmmm thread=multi should already be enabled by default where it is safe
> to do so. Also what has it to do with euid?

OK.  Will look into removing this.
We were trying to check for KVM, to only add this under KVM.
I see now, we need to use kvm_available() instead of euid.

Thanks & Regards,
-Rob


>
> > +        if '-smp' not in extra_args and \
> > +           '-smp' not in self._config['extra_args'] and \
> > +           '-smp' not in self._args:
> > +            # Only add if not already there to give caller option to change it.
> > +            extra_args.extend(["-smp", "8"])
> > +
> > +        # We have overridden boot() since aarch64 has additional parameters.
> > +        # Call down to the base class method.
> > +        super(UbuntuAarch64VM, self).boot(img, extra_args=extra_args)
> > +
> > +    def build_image(self, img):
> > +        os_img = self._download_with_cache(self.image_link)
> > +        img_tmp = img + ".tmp"
> > +        subprocess.check_call(["cp", "-f", os_img, img_tmp])
> > +        subprocess.check_call(["qemu-img", "resize", img_tmp, "+50G"])
> > +        ci_img = self._gen_cloud_init_iso()
> > +
> > +        self.boot(img_tmp, extra_args = ["-cdrom", ci_img])
> > +        self.wait_ssh(wait_root=True)
> > +        # Fix for slow ssh login.
> > +        self.ssh_root("chmod -x /etc/update-motd.d/*")
> > +        self.ssh_root("touch /etc/cloud/cloud-init.disabled")
> > +        # Disable auto upgrades.
> > +        # We want to keep the VM system state stable.
> > +        self.ssh_root('sed -ie \'s/"1"/"0"/g\' /etc/apt/apt.conf.d/20auto-upgrades')
> > +
> > +        # If the user chooses *not* to do the second phase,
> > +        # then we will jump right to the graceful shutdown
> > +        if self._config['install_cmds'] != "":
> > +            # Don't check the status in case the guest hang up too quickly
> > +            self.ssh_root("sync && reboot")
> > +
> > +            self.wait_ssh(wait_root=True)
> > +            # The previous update sometimes doesn't survive a reboot, so do it again
> > +            self.ssh_root("sed -ie s/^#\ deb-src/deb-src/g /etc/apt/sources.list")
> > +
> > +            # Issue the install commands.
> > +            # This can be overriden by the user in the config .yml.
> > +            install_cmds = self._config['install_cmds'].split(',')
> > +            for cmd in install_cmds:
> > +                self.ssh_root(cmd)
> > +        self.graceful_shutdown()
> > +        self.wait()
> > +        os.rename(img_tmp, img)
> > +        return 0
> > +
> > +if __name__ == "__main__":
> > +    sys.exit(basevm.main(UbuntuAarch64VM, DEFAULT_CONFIG))
>
>
> --
> Alex Bennée

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Andrew Jones]

On Mon, Jan 27, 2020 at 11:47:35AM -0500, Robert Foley wrote:
> On Mon, 27 Jan 2020 at 10:01, Alex Bennée <alex.bennee@linaro.org> wrote:
> > >  vm-boot-ssh-%: $(IMAGES_DIR)/%.img
> > >       $(call quiet-command, \
> > > -             $(SRC_PATH)/tests/vm/$* \
> > > +             $(PYTHON) $(SRC_PATH)/tests/vm/$* \
> >
> > This seems like it should be in a different patch.
> 
> Good point, will move it to a different patch.
> 
> > > +
> > > +DEFAULT_CONFIG = {
> > > +    'cpu'          : "max",
> > > +    'machine'      : "virt,gic-version=max",
> >
> > According to virt.c:
> >
> >   Valid values are 2, 3 and host
> >
> > but max should work on TCG. However we need a more recent QEMU than my
> > system one for it to work. Otherwise you see:
> >
> >   DEBUG:qemu.machine:Error launching VM
> 
> Good point.  We were trying to avoid having different values for KVM
> vs TCG, which we
> could do with the latest QEMU.
> We will update this to make sure this works with older versions of QEMU as well.
> 
> On my system I have qemu 2.11.1 installed by default.
> It seems that we will need the following defaults based on our environment.
> 
> For KVM we end up with the below args since max cpu and max
> gic-version is not available.
> kvm:  -cpu host -machine virt,gic-version=host
> 
> For TCG max cpu is also not available: qemu-system-aarch64: unable to
> find CPU model 'max',
> so we pick cortex-a57.
> TCG: -cpu cortex-a57 -machine virt,gic-version=3
> 
> I suppose we could check the version of QEMU and use the above
> defaults only for earlier versions of QEMU.
> This is something we will probably move to aarch64vm.py since it is common.

What versions of QEMU do these tests *have* to support? Because we could
just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
'max' is indeed the nicest selection for using the same command line on
KVM (gicv2 and gicv3 hosts) and TCG.

Thanks,
drew

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

Hi Drew,

On Mon, 27 Jan 2020 at 12:27, Andrew Jones <drjones@redhat.com> wrote:

> >
> > I suppose we could check the version of QEMU and use the above
> > defaults only for earlier versions of QEMU.
> > This is something we will probably move to aarch64vm.py since it is common.
>
> What versions of QEMU do these tests *have* to support? Because we could
> just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
> 'max' is indeed the nicest selection for using the same command line on
> KVM (gicv2 and gicv3 hosts) and TCG.

I believe these test scripts which build/launch the VM have to support
the older version of QEMU since
this is the version of QEMU currently used when these VMs are
launched.  I don't know the history on
this, but it seems intentional that we use one older/different version
of QEMU to launch the VM,
while we test the 'current' build of QEMU inside the VM.
It also seems like a 'nice to have' to automatically support the
latest version where we could
use max as you pointed out.

Thanks & Regards,
-Rob

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Alex Bennée]

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

> Hi Drew,
>
> On Mon, 27 Jan 2020 at 12:27, Andrew Jones <drjones@redhat.com> wrote:
>
>> >
>> > I suppose we could check the version of QEMU and use the above
>> > defaults only for earlier versions of QEMU.
>> > This is something we will probably move to aarch64vm.py since it is common.
>>
>> What versions of QEMU do these tests *have* to support? Because we could
>> just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
>> 'max' is indeed the nicest selection for using the same command line on
>> KVM (gicv2 and gicv3 hosts) and TCG.
>
> I believe these test scripts which build/launch the VM have to support
> the older version of QEMU since
> this is the version of QEMU currently used when these VMs are
> launched.  I don't know the history on
> this, but it seems intentional that we use one older/different version
> of QEMU to launch the VM,

Well we defer to the system QEMU as it should be stable. It can be
overridden with the QEMU environment variable which worked well enough
when we only had VMs of one architecture. Perhaps we needs a new way to
say "use the appropriate QEMU from this build"?

> while we test the 'current' build of QEMU inside the VM.
> It also seems like a 'nice to have' to automatically support the
> latest version where we could
> use max as you pointed out.
>
> Thanks & Regards,
> -Rob


--
Alex Bennée

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

On Mon, 27 Jan 2020 at 15:07, Alex Bennée <alex.bennee@linaro.org> wrote:
> Robert Foley <robert.foley@linaro.org> writes:
>
> > Hi Drew,
> >
> > On Mon, 27 Jan 2020 at 12:27, Andrew Jones <drjones@redhat.com> wrote:
> >
> >> >
> >> > I suppose we could check the version of QEMU and use the above
> >> > defaults only for earlier versions of QEMU.
> >> > This is something we will probably move to aarch64vm.py since it is common.
> >>
> >> What versions of QEMU do these tests *have* to support? Because we could
> >> just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
> >> 'max' is indeed the nicest selection for using the same command line on
> >> KVM (gicv2 and gicv3 hosts) and TCG.
> >
> > I believe these test scripts which build/launch the VM have to support
> > the older version of QEMU since
> > this is the version of QEMU currently used when these VMs are
> > launched.  I don't know the history on
> > this, but it seems intentional that we use one older/different version
> > of QEMU to launch the VM,
>
> Well we defer to the system QEMU as it should be stable. It can be
> overridden with the QEMU environment variable which worked well enough
> when we only had VMs of one architecture. Perhaps we needs a new way to
> say "use the appropriate QEMU from this build"?

Good idea.  This is a pretty common use case and it makes sense to
make it easy to use.
I will add some support for this in my patch series.

Thanks & Regards,
-Rob
>
> > while we test the 'current' build of QEMU inside the VM.
> > It also seems like a 'nice to have' to automatically support the
> > latest version where we could
> > use max as you pointed out.
> >
> > Thanks & Regards,
> > -Rob
>
>
> --
> Alex Bennée

[PATCH 8/8] tests/vm: Added a new script for centos.aarch64.

[Robert Foley]

centos.aarch64 creates a CentOS 8 image.
Also added a new kickstart script used to build the centos.aarch64 image.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
---
 tests/vm/Makefile.include    |   3 +-
 tests/vm/centos-8-aarch64.ks |  52 +++++++++
 tests/vm/centos.aarch64      | 218 +++++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 tests/vm/centos-8-aarch64.ks
 create mode 100755 tests/vm/centos.aarch64

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 966b417ba7..febf82fe16 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -2,7 +2,7 @@
 
 .PHONY: vm-build-all vm-clean-all
 
-IMAGES := ubuntu.i386 freebsd netbsd openbsd centos fedora ubuntu.aarch64
+IMAGES := ubuntu.i386 freebsd netbsd openbsd centos fedora ubuntu.aarch64 centos.aarch64
 IMAGES_DIR := $(HOME)/.cache/qemu-vm/images
 IMAGE_FILES := $(patsubst %, $(IMAGES_DIR)/%.img, $(IMAGES))
 
@@ -19,6 +19,7 @@ vm-help vm-test:
 	@echo "  vm-build-centos                 - Build QEMU in CentOS VM, with Docker"
 	@echo "  vm-build-fedora                 - Build QEMU in Fedora VM"
 	@echo "  vm-build-ubuntu.aarch64         - Build QEMU in ubuntu aarch64 VM"
+	@echo "  vm-build-centos.aarch64         - Build QEMU in CentOS aarch64 VM"
 	@echo ""
 	@echo "  vm-build-all                    - Build QEMU in all VMs"
 	@echo "  vm-clean-all                    - Clean up VM images"
diff --git a/tests/vm/centos-8-aarch64.ks b/tests/vm/centos-8-aarch64.ks
new file mode 100644
index 0000000000..5f6093fefa
--- /dev/null
+++ b/tests/vm/centos-8-aarch64.ks
@@ -0,0 +1,52 @@
+# CentOS aarch64 image kickstart file.
+# This file is used by the CentOS installer to
+# script the generation of the image.
+#
+# Copyright 2020 Linaro
+#
+ignoredisk --only-use=vda
+# System bootloader configuration
+bootloader --append=" crashkernel=auto" --location=mbr --boot-drive=vda
+autopart --type=plain
+# Partition clearing information
+clearpart --linux --initlabel --drives=vda
+# Use text mode install
+text
+repo --name="AppStream" --baseurl=file:///run/install/repo/AppStream
+# Use CDROM installation media
+cdrom
+# Keyboard layouts
+keyboard --vckeymap=us --xlayouts=''
+# System language
+lang en_US.UTF-8
+
+# Network information
+network  --bootproto=dhcp --device=enp0s1 --onboot=off --ipv6=auto --no-activate
+network  --hostname=localhost.localdomain
+# Run the Setup Agent on first boot
+firstboot --enable
+# Do not configure the X Window System
+skipx
+# System services
+services --enabled="chronyd"
+# System timezone
+timezone America/New_York --isUtc
+
+# Shutdown after installation is complete.
+shutdown
+
+%packages
+@^server-product-environment
+kexec-tools
+
+%end
+
+%addon com_redhat_kdump --enable --reserve-mb='auto'
+
+%end
+%anaconda
+pwpolicy root --minlen=6 --minquality=1 --notstrict --nochanges --notempty
+pwpolicy user --minlen=6 --minquality=1 --notstrict --nochanges --emptyok
+pwpolicy luks --minlen=6 --minquality=1 --notstrict --nochanges --notempty
+%end
+
diff --git a/tests/vm/centos.aarch64 b/tests/vm/centos.aarch64
new file mode 100755
index 0000000000..d939c2a900
--- /dev/null
+++ b/tests/vm/centos.aarch64
@@ -0,0 +1,218 @@
+#!/usr/bin/env python
+#
+# Centos aarch64 image
+#
+# Copyright 2020 Linaro
+#
+# Authors:
+#  Robert Foley <robert.foley@linaro.org>
+#  Originally based on ubuntu.aarch64
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+import time
+import traceback
+import aarch64vm
+
+DEFAULT_CONFIG = {
+    'cpu'          : "max",
+    'machine'      : "virt,gic-version=max",
+    'install_cmds' : "yum install -y docker make git python3 gcc, "\
+                     "yum install -y glib2-devel pixman-devel zlib-devel, "\
+                     "yum install -y perl-Test-Harness, "\
+                     "systemctl enable docker",
+    # We increase beyond the default time since during boot
+    # it can take some time (many seconds) to log into the VM.
+    'ssh_timeout'  : 60,
+}
+
+class CentosAarch64VM(basevm.BaseVM):
+    name = "centos.aarch64"
+    arch = "aarch64"
+    login_prompt = "localhost login:"
+    prompt = '[root@localhost ~]#'
+    image_name = "CentOS-8-aarch64-1905-dvd1.iso"
+    image_link = "http://mirrors.usc.edu/pub/linux/distributions/centos/8.0.1905/isos/aarch64/"
+    image_link += image_name
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d);
+        sudo chmod a+r /dev/vdb;
+        tar --checkpoint=.10 -xf /dev/vdb;
+        ./configure {configure_opts};
+        make --output-sync {target} -j{jobs} {verbose};
+    """
+    def set_key_perm(self):
+        """Set permissions properly on certain files to allow
+           ssh access."""
+        self.console_wait_send(self.prompt,
+                               "/usr/sbin/restorecon -R -v /root/.ssh\n")
+        self.console_wait_send(self.prompt,
+                "/usr/sbin/restorecon -R -v "\
+                "/home/{}/.ssh\n".format(self.GUEST_USER))
+
+    def create_kickstart(self):
+        """Generate the kickstart file used to generate the centos image."""
+        # Start with the template for the kickstart.
+        ks_file = "../tests/vm/centos-8-aarch64.ks"
+        subprocess.check_call("cp {} ./ks.cfg".format(ks_file), shell=True)
+        # Append the ssh keys to the kickstart file
+        # as the post processing phase of installation.
+        with open("ks.cfg", "a") as f:
+            # Add in the root pw and guest user.
+            f.write("rootpw --plaintext {}\n".format(self.ROOT_PASS))
+            add_user = "user --groups=wheel --name={} "\
+                       "--password={} --plaintext\n"
+            f.write(add_user.format(self.GUEST_USER, self.GUEST_PASS))
+            # Add the ssh keys.
+            f.write("%post --log=/root/ks-post.log\n")
+            f.write("mkdir -p /root/.ssh\n")
+            addkey = 'echo "{}" >> /root/.ssh/authorized_keys\n'
+            addkey_cmd = addkey.format(self.ssh_pub_key)
+            f.write(addkey_cmd)
+            f.write('mkdir -p /home/{}/.ssh\n'.format(self.GUEST_USER))
+            addkey = 'echo "{}" >> /home/{}/.ssh/authorized_keys\n'
+            addkey_cmd = addkey.format(self.ssh_pub_key, self.GUEST_USER)
+            f.write(addkey_cmd)
+            f.write("%end\n")
+        # Take our kickstart file and create an .iso from it.
+        # The .iso will be provided to qemu as we boot
+        # from the install dvd.
+        # Anaconda will recognize the label "OEMDRV" and will
+        # start the automated installation.
+        gen_iso_img = 'genisoimage -output ks.iso -volid "OEMDRV" ks.cfg'
+        subprocess.check_call(gen_iso_img, shell=True)
+
+    def wait_for_shutdown(self):
+        """We wait for qemu to shutdown the VM and exit.
+           While this happens we display the console view
+           for easier debugging."""
+        # The image creation is essentially done,
+        # so whether or not the wait is successful we want to
+        # wait for qemu to exit (the self.wait()) before we return.
+        try:
+            self.console_wait("reboot: Power down")
+        except Exception as e:
+            sys.stderr.write("Exception hit\n")
+            if isinstance(e, SystemExit) and e.code == 0:
+                return 0
+            traceback.print_exc()
+        finally:
+            self.wait()
+
+    def build_base_image(self, dest_img):
+        """Run through the centos installer to create
+           a base image with name dest_img."""
+        # We create the temp image, and only rename
+        # to destination when we are done.
+        img = dest_img + ".tmp"
+        # Create an empty image.
+        # We will provide this as the install destination.
+        qemu_img_create = "qemu-img create {} 50G".format(img)
+        subprocess.check_call(qemu_img_create, shell=True)
+
+        # Create our kickstart file to be fed to the installer.
+        self.create_kickstart()
+        # Boot the install dvd with the params as our ks.iso
+        os_img = self._download_with_cache(self.image_link)
+        dvd_iso = "centos-8-dvd.iso"
+        subprocess.check_call(["cp", "-f", os_img, dvd_iso])
+        extra_args = "-cdrom ks.iso"
+        extra_args += " -drive file={},if=none,id=drive1,cache=writeback"
+        extra_args += " -device virtio-blk,drive=drive1,bootindex=1"
+        extra_args = extra_args.format(dvd_iso).split(" ")
+        self.boot(img, extra_args=extra_args)
+        self.console_wait_send("change the selection", "\n")
+        # We seem to need to hit esc (chr(27)) twice to abort the
+        # media check, which takes a long time.
+        # Waiting a bit seems to be more reliable before hitting esc.
+        self.console_wait("Checking")
+        time.sleep(5)
+        self.console_wait_send("Checking", chr(27))
+        time.sleep(5)
+        self.console_wait_send("Checking", chr(27))
+        print("Found Checking")
+        self.wait_for_shutdown()
+        os.rename(img, dest_img)
+        print("Done with base image build: {}".format(dest_img))
+
+    def check_create_base_img(self, img_base, img_dest):
+        """Create a base image using the installer.
+           We will use the base image if it exists.
+           This helps cut down on install time in case we
+           need to restart image creation,
+           since the base image creation can take a long time."""
+        if not os.path.exists(img_base):
+            print("Generate new base image: {}".format(img_base))
+            self.build_base_image(img_base);
+        else:
+            print("Use existing base image: {}".format(img_base))
+        # Save a copy of the base image and copy it to dest.
+        # which we will use going forward.
+        subprocess.check_call(["cp", img_base, img_dest])
+
+    def boot(self, img, extra_args=None):
+        aarch64vm.create_flash_images()
+        default_args = aarch64vm.get_pflash_args()
+        if extra_args:
+            extra_args.extend(default_args)
+        else:
+            extra_args = default_args
+        # We always add these performance tweaks
+        # because without them, we boot so slowly that we
+        # can time out finding the boot efi device.
+        if os.geteuid() != 0:
+            extra_args.extend(["-accel", "tcg,thread=multi"])
+        if '-smp' not in extra_args and \
+           '-smp' not in self._config['extra_args'] and \
+           '-smp' not in self._args:
+            # Only add if not already there to give caller option to change it.
+            extra_args.extend(["-smp", "8"])
+        # We have overridden boot() since aarch64 has additional parameters.
+        # Call down to the base class method.
+        super(CentosAarch64VM, self).boot(img, extra_args=extra_args)
+
+    def build_image(self, img):
+        img_tmp = img + ".tmp"
+        self.check_create_base_img(img + ".base", img_tmp)
+
+        # Boot the new image for the first time to finish installation.
+        self.boot(img_tmp)
+        self.console_init()
+        self.console_wait_send(self.login_prompt, "root\n")
+        self.console_wait_send("Password:", "{}\n".format(self.ROOT_PASS))
+
+        self.set_key_perm()
+        self.console_wait_send(self.prompt, "rpm -q centos-release\n")
+        enable_adapter = "sed -i 's/ONBOOT=no/ONBOOT=yes/g'" \
+                         " /etc/sysconfig/network-scripts/ifcfg-enp0s1\n"
+        self.console_wait_send(self.prompt, enable_adapter)
+        self.console_wait_send(self.prompt, "ifup enp0s1\n")
+        self.console_wait_send(self.prompt,
+                               'echo "qemu  ALL=(ALL) NOPASSWD:ALL" | '\
+                               'sudo tee /etc/sudoers.d/qemu\n')
+        self.console_wait(self.prompt)
+
+        # Rest of the commands we issue through ssh.
+        self.wait_ssh(wait_root=True)
+
+        # If the user chooses *not* to do the second phase,
+        # then we will jump right to the graceful shutdown
+        if self._config['install_cmds'] != "":
+            install_cmds = self._config['install_cmds'].split(',')
+            for cmd in install_cmds:
+                self.ssh_root(cmd)
+        self.ssh_root("poweroff")
+        self.wait_for_shutdown()
+        os.rename(img_tmp, img)
+        print("image creation complete: {}".format(img))
+        return 0
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(CentosAarch64VM, DEFAULT_CONFIG))
-- 
2.17.1

Re: [PATCH 0/8] tests/vm: Add support for aarch64 VMs

[Alex Bennée]

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

> This patch adds support for 2 aarch64 VMs.  
>  - Ubuntu 18.04 aarch64 VM
>  - CentOS 8 aarch64 VM
<snip>

Another failure to note - under TCG:

  make vm-build-ubuntu.aarch64 V=1 QEMU=aarch64-softmmu/qemu-system-aarch64

Gives:

Not run: 172 186 192 220
Failures: 001 002 003 004 005 007 008 009 010 011 012 013 017 018 019 020 021 022 024 025 027 029 031 032 033 034 035 036 037 038 039 042 043 046 047 048 049 050 052 053 054 060 061 062 063 066 069 071 072 073 074 079 080 086 089 090 097 098 099 103 104 105 107 108 110 111 114 117 120 126 133 134 137 138 140 141 143 150 154 156 158 159 161 170
174 176 177 179 184 187 190 191 195 214 217 226 229 244 249 251 252 265 267 268
Failed 104 of 104 iotests
/tmp/tmp.EjcqWtvHwd/tests/Makefile.include:840: recipe for target 'check-tests/check-block.sh' failed
make: *** [check-tests/check-block.sh] Error 1
rm tests/qemu-iotests/socket_scm_helper.o
Connection to 127.0.0.1 closed.
DEBUG:QMP:>>> {'execute': 'quit'}
DEBUG:QMP:<<< {'timestamp': {'seconds': 1580134315, 'microseconds': 216297}, 'event': 'NIC_RX_FILTER_CHANGED', 'data': {'path': '/machine/peripheral-anon/device[0]/virtio-backend'}}
DEBUG:QMP:<<< {'return': {}}
/home/alex.bennee/lsrc/qemu.git/tests/vm/Makefile.include:63: recipe for target 'vm-build-ubuntu.aarch64' failed
make: *** [vm-build-ubuntu.aarch64] Error 3

With things like:

--- /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/063.out      2020-01-27 10:54:38.000000000 +0000
+++ /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/063.out.bad  2020-01-28 01:20:28.563789323 +0000
@@ -1,3 +1,4 @@
+bash: warning: setlocale: LC_ALL: cannot change locale (en_GB.UTF-8)
 QA output created by 063
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
 == Testing conversion with -n fails with no target file ==
  TEST    iotest-qcow2: 066 [fail]
QEMU          -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../aarch64-softmmu/qemu-system-aarch64" -nodefaults -display none -machine virt -accel qtest
QEMU_IMG      -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../qemu-img"
QEMU_IO       -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../qemu-io"  --cache writeback -f qcow2
QEMU_NBD      -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../qemu-nbd"
IMGFMT        -- qcow2 (compat=1.1)
IMGPROTO      -- file
PLATFORM      -- Linux/aarch64 ubuntu-guest 4.15.0-74-generic
TEST_DIR      -- /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/scratch
SOCK_DIR      -- /tmp/tmp.BJ9gTNMmv1
SOCKET_SCM_HELPER -- /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/socket_scm_helper

So I suspect a locale issue is breaking things.

-- 
Alex Bennée

Re: [PATCH 0/8] tests/vm: Add support for aarch64 VMs

[Robert Foley]

Thanks for the details on the failure.   I have not been able to
reproduce it yet, but digging into it further.

On Tue, 28 Jan 2020 at 12:52, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Robert Foley <robert.foley@linaro.org> writes:
>
> > This patch adds support for 2 aarch64 VMs.
> >  - Ubuntu 18.04 aarch64 VM
> >  - CentOS 8 aarch64 VM
> <snip>
>
> Another failure to note - under TCG:
>
>   make vm-build-ubuntu.aarch64 V=1 QEMU=aarch64-softmmu/qemu-system-aarch64
>
> Gives:
>
> Not run: 172 186 192 220
> Failures: 001 002 003 004 005 007 008 009 010 011 012 013 017 018 019 020 021 022 024 025 027 029 031 032 033 034 035 036 037 038 039 042 043 046 047 048 049 050 052 053 054 060 061 062 063 066 069 071 072 073 074 079 080 086 089 090 097 098 099 103 104 105 107 108 110 111 114 117 120 126 133 134 137 138 140 141 143 150 154 156 158 159 161 170
> 174 176 177 179 184 187 190 191 195 214 217 226 229 244 249 251 252 265 267 268
> Failed 104 of 104 iotests
> /tmp/tmp.EjcqWtvHwd/tests/Makefile.include:840: recipe for target 'check-tests/check-block.sh' failed
> make: *** [check-tests/check-block.sh] Error 1
> rm tests/qemu-iotests/socket_scm_helper.o
> Connection to 127.0.0.1 closed.
> DEBUG:QMP:>>> {'execute': 'quit'}
> DEBUG:QMP:<<< {'timestamp': {'seconds': 1580134315, 'microseconds': 216297}, 'event': 'NIC_RX_FILTER_CHANGED', 'data': {'path': '/machine/peripheral-anon/device[0]/virtio-backend'}}
> DEBUG:QMP:<<< {'return': {}}
> /home/alex.bennee/lsrc/qemu.git/tests/vm/Makefile.include:63: recipe for target 'vm-build-ubuntu.aarch64' failed
> make: *** [vm-build-ubuntu.aarch64] Error 3
>
> With things like:
>
> --- /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/063.out      2020-01-27 10:54:38.000000000 +0000
> +++ /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/063.out.bad  2020-01-28 01:20:28.563789323 +0000
> @@ -1,3 +1,4 @@
> +bash: warning: setlocale: LC_ALL: cannot change locale (en_GB.UTF-8)
>  QA output created by 063
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
>  == Testing conversion with -n fails with no target file ==
>   TEST    iotest-qcow2: 066 [fail]
> QEMU          -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../aarch64-softmmu/qemu-system-aarch64" -nodefaults -display none -machine virt -accel qtest
> QEMU_IMG      -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../qemu-img"
> QEMU_IO       -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../qemu-io"  --cache writeback -f qcow2
> QEMU_NBD      -- "/tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/../../qemu-nbd"
> IMGFMT        -- qcow2 (compat=1.1)
> IMGPROTO      -- file
> PLATFORM      -- Linux/aarch64 ubuntu-guest 4.15.0-74-generic
> TEST_DIR      -- /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/scratch
> SOCK_DIR      -- /tmp/tmp.BJ9gTNMmv1
> SOCKET_SCM_HELPER -- /tmp/tmp.EjcqWtvHwd/tests/qemu-iotests/socket_scm_helper
>
> So I suspect a locale issue is breaking things.
>
> --
> Alex Bennée