Re: [PATCH] tests/vm: Add Haiku test based on their vagrant images

[Philippe Mathieu-Daudé]

On 9/25/20 10:49 AM, Philippe Mathieu-Daudé wrote:
> Hi Alexander,
> 
> On 9/6/20 4:41 PM, Alexander von Gluck IV wrote:
>> September 6, 2020 9:35 AM, "Alexander von Gluck IV" <kallisti5@unixzen.com> 
>> wrote:
>>> Signed-off-by: Alexander von Gluck IV <kallisti5@unixzen.com>
>>> ---
>>> tests/keys/vagrant | 27 +++++++++
>>> tests/keys/vagrant.pub | 1 +
>>> tests/vm/basevm.py | 5 +-
>>> tests/vm/haiku.x86_64 | 121 +++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 152 insertions(+), 2 deletions(-)
>>> create mode 100644 tests/keys/vagrant
>>> create mode 100644 tests/keys/vagrant.pub
>>> create mode 100755 tests/vm/haiku.x86_64
>>>
>>> diff --git a/tests/keys/vagrant b/tests/keys/vagrant
>>> new file mode 100644
>>> index 0000000000..7d6a083909
>>> --- /dev/null
>>> +++ b/tests/keys/vagrant
>>> @@ -0,0 +1,27 @@
>>> +-----BEGIN RSA PRIVATE KEY-----
>>> +MIIEogIBAAKCAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzI
>>>
>>> diff --git a/tests/keys/vagrant.pub b/tests/keys/vagrant.pub
>>> new file mode 100644
>>> index 0000000000..18a9c00fd5
>>> --- /dev/null
>>> +++ b/tests/keys/vagrant.pub
>>> @@ -0,0 +1 @@
>>> +ssh-rsa
>>> AAAAB3NzaC1yc2EAAAABIwAAAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzIw+niNltGEFHzD8+v1I2YJ6oX
>>
>> A little background information for context. These are the Vagrant SSH keys 
>> which are packed with every vagrant OS image and allow OS access for 
>> automation.  The python vm tester knowing of these lets it leverage Vagrant 
>> OS images for testing without much work.
> 
> Please add this information as comment in the source files.
> 
>>
>>
>>
>>> --- a/tests/vm/basevm.py
>>> +++ b/tests/vm/basevm.py
>>> @@ -44,6 +44,7 @@ DEFAULT_CONFIG = {
>>> 'machine' : 'pc',
>>> 'guest_user' : "qemu",
>>> 'guest_pass' : "qemupass",
>>> + 'root_user' : "root",
>>> 'root_pass' : "qemupass",
>>> 'ssh_key_file' : SSH_KEY_FILE,
>>> 'ssh_pub_key_file': SSH_PUB_KEY_FILE,
>>> @@ -245,13 +246,13 @@ class BaseVM(object):
>>> return self._ssh_do(self._config["guest_user"], cmd, False)
>>>
>>> def ssh_root(self, *cmd):
>>> - return self._ssh_do("root", cmd, False)
>>> + return self._ssh_do(self._config["root_user"], cmd, False)
>>>
>>> def ssh_check(self, *cmd):
>>> self._ssh_do(self._config["guest_user"], cmd, True)
>>>
>>> def ssh_root_check(self, *cmd):
>>> - self._ssh_do("root", cmd, True)
>>> + self._ssh_do(self._config["root_user"], cmd, True)
>>>
>>> def build_image(self, img):
>>> raise NotImplementedError
>>
>>
>> Haiku's user is UID 0, so essentially our root user isn't named root.
>> This adds the (optional) ability to override the root username.
> 
> Ditto.
> 
>>
>>
>>> diff --git a/tests/vm/haiku.x86_64 b/tests/vm/haiku.x86_64
>>> new file mode 100755
>>> index 0000000000..9777722f51
>>> --- /dev/null
>>> +++ b/tests/vm/haiku.x86_64
>>> @@ -0,0 +1,121 @@
>>> +#!/usr/bin/env python3
>>> +#
>>> +# Haiku VM image
>>> +#
>>> +# Copyright 2020 Haiku, Inc.
>>> +#
>>> +# Authors:
>>> +# Alexander von Gluck IV <kallisti5@unixzen.com>
>>> +#
>>> +# This code is licensed under the GPL version 2 or later. See
>>> +# the COPYING file in the top-level directory.
>>> +#
>>
>>
>> This build script works as expected, transferring the qemu archive over
>> via the virtio block device and building it.
>>
>> More information here (including output of tools):
>> https://bugs.launchpad.net/qemu/+bug/1715203
>>
>> This purpose of this is trying to prevent the need to remove
>> upstream qemu support for Haiku.
>>
>> We have some out-of-tree patches to fix the error seen in our ports, i'll
>> work on upstreaming these.
> 
> Please do, because so far this fail before compiling the 10th file:
> 
> slirp/src/tftp.c: In function 'tftp_read_data':
> slirp/src/tftp.c:113:50: error: 'O_BINARY' undeclared (first use in this
> function); did you mean 'L_INCR'?
>          spt->fd = open(spt->filename, O_RDONLY | O_BINARY);
>                                                   ^~~~~~~~
>                                                   L_INCR
> 
> To avoid using 4GB of temporary storage in my HOMEDIR I had
> to do this change:
> 
> -- >8 --
> --- a/tests/vm/haiku.x86_64
> +++ b/tests/vm/haiku.x86_64
> @@ -93,17 +93,15 @@ class HaikuVM(basevm.BaseVM):
> 
>      def build_image(self, img):
>          self.print_step("Downloading disk image")
> -        cvg = self._download_with_cache(self.link, sha256sum=self.csum)
> -        cgz = cvg + ".tar.gz"
> -        img_tmp = "./box.img"
> +        tarball = self._download_with_cache(self.link, sha256sum=self.csum)
> +
> +        self.print_step("Extracting disk image")
> +
> +        subprocess.check_call(["tar", "xzf", tarball, "./box.img", "-O"],
> +                              stdout=open(img, 'wb'))
> 
>          self.print_step("Preparing disk image")
> -
> -        subprocess.check_call(["cp", "-f", cvg, cgz])
> -        subprocess.check_call(["tar", "xvzf", cgz, img_tmp])
> -        subprocess.check_call(["chmod", "666", img_tmp])
> -
> -        self.boot(img_tmp)
> +        self.boot(img)
> 
>          # Wait for ssh to be available.
>          self.wait_ssh(wait_root=True, cmd="exit 0")
> @@ -112,9 +110,6 @@ class HaikuVM(basevm.BaseVM):
>          self.ssh_root("pkgman install -y %s" % " ".join(self.requirements))
>          self.graceful_shutdown()
> 
> -        if os.path.exists(cgz):
> -            os.remove(cgz)
> -        subprocess.check_call(["mv", img_tmp, img])
>          self.print_step("All done")
> 
>  if __name__ == "__main__":
> ---
> 
> Note that something is not working well in your script because
> the image is extracted/reinstalled each time. This has to be
> done once, then we reuse the image for the builds.

I figured out make was removing the image as a temporary file,
because you didn't added it in the IMAGES variable, so this is
the fix:

-- >8 --
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -4,7 +4,7 @@

 EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)

-IMAGES := freebsd netbsd openbsd centos fedora
+IMAGES := freebsd netbsd openbsd centos fedora haiku.x86_64
 ifneq ($(GENISOIMAGE),)
 IMAGES += ubuntu.i386 centos
 ifneq ($(EFI_AARCH64),)
---

> 
> Also, please address Thomas comment.

Which is:

-- >8 --
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -41,6 +41,7 @@ endif
 else
        @echo "  (install genisoimage to build centos/ubuntu images)"
 endif
+       @echo "  vm-build-haiku.x86_64           - Build QEMU in Haiku VM"
        @echo ""
        @echo "  vm-build-all                    - Build QEMU in all VMs"
        @echo "  vm-clean-all                    - Clean up VM images"
---
> 
> Thanks,
> 
> Phil.
>