All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)
@ 2017-09-05  2:11 Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images Fam Zheng
                   ` (12 more replies)
  0 siblings, 13 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

v6: Add license to new file. [Philippe]
    Change tests/.gitignore. [Philippe]

v5: Generate source tar file with a script.
    Fix tmpdir, use pwd.
    Reduce default -j to half cores.

v4: Drop unused imports and parameters. [Cleber]
    Use --exclude-vcs (still no --exclude-vcs-ignores because it's too new). [Philippe]
    Use gtar if available. [Philippe, Kamil]
    /dev/ld1a -> /dev/rld1a for netbsd. [Kamil]
    Only use '-enable-kvm' if /dev/kvm is there. [Kamil]
    Grammar fixes of README. [Stefan]
    Rename image on the server to include version and arch. [Kamil]
    Just ignore *.tmp. [Philippe]

v3: Drop RFC.
    Add Stefan's and Kamil's reviewed-bys.
    Use optparse. [Stefan]
    Drop the VGA patch. [Paolo, Stefan]
    Improve exit/exit code/doc. [Stefan]
    Drop unused line from basevm.py. [Stefan]
    Drop "--target-list" form Makefile.
    More intelligent '-j'.
    Add README. [Stefan]

v2: - Add docstring. [Stefan]
    - Call self._load_io_lod. [Stefan]
    - Use "info usernet" and dynamic ssh_port forwarding. [Stefan]
    - Add image checksum.
    - Use os.rename() and os.makedirs(). [Stefan]
    - Fix NetBSD URL. [Kamil]

Build tests in one 32 bit Linux guest and three BSD images are defined in this
series. This is a more managable way than the manually maintained virtual
machines in patchew. Also, one big advantage of ephemeral VMs over long running
guests is the reduced RAM usage of host, which makes it possible to have one
host test all these BSD variants and probably more.

The BSD guest templates are manually prepared following

https://wiki.qemu.org/Hosts/BSD

as it is not easy to automate. (The ideal approach is like the ubuntu.i386
script, which configures the guest on top of an official released image, fully
automatically.)

Need for help: "gmake check" in the added OpenBSD image fails with -ENOMEM
errors, even if I change "-m 2G" to "-m 8G" when starting VM. Ideas? And there
is a warning from ./configure about OpenBSD going to be unsupported in coming
releases, is it still the case?

Fam

Fam Zheng (12):
  gitignore: Ignore vm test images
  qemu.py: Add "wait()" method
  scripts: Add archive-source.sh
  tests: Add vm test lib
  tests: Add ubuntu.i386 image
  tests: Add FreeBSD image
  tests: Add NetBSD image
  tests: Add OpenBSD image
  Makefile: Add rules to run vm tests
  MAINTAINERS: Add tests/vm entry
  tests: Add README for vm tests
  docker: Use archive-source.py

 .gitignore                    |   1 +
 MAINTAINERS                   |   1 +
 Makefile                      |   2 +
 configure                     |   2 +-
 scripts/archive-source.sh     |  31 +++++
 scripts/qemu.py               |   7 ++
 tests/.gitignore              |   1 +
 tests/docker/Makefile.include |  15 +--
 tests/docker/run              |   8 +-
 tests/vm/Makefile.include     |  42 +++++++
 tests/vm/README               |  63 ++++++++++
 tests/vm/basevm.py            | 276 ++++++++++++++++++++++++++++++++++++++++++
 tests/vm/freebsd              |  42 +++++++
 tests/vm/netbsd               |  42 +++++++
 tests/vm/openbsd              |  43 +++++++
 tests/vm/ubuntu.i386          |  88 ++++++++++++++
 16 files changed, 643 insertions(+), 21 deletions(-)
 create mode 100755 scripts/archive-source.sh
 create mode 100644 tests/vm/Makefile.include
 create mode 100644 tests/vm/README
 create mode 100755 tests/vm/basevm.py
 create mode 100755 tests/vm/freebsd
 create mode 100755 tests/vm/netbsd
 create mode 100755 tests/vm/openbsd
 create mode 100755 tests/vm/ubuntu.i386

-- 
2.13.5

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-08 14:12   ` Alex Bennée
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 02/12] qemu.py: Add "wait()" method Fam Zheng
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 .gitignore       | 1 +
 tests/.gitignore | 1 +
 2 files changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index cf65316863..40acfcb9e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -52,6 +52,7 @@
 /vscclient
 /vhost-user-scsi
 /fsdev/virtfs-proxy-helper
+*.tmp
 *.[1-9]
 *.a
 *.aux
diff --git a/tests/.gitignore b/tests/.gitignore
index fed0189a5a..cf6d99c91e 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -95,3 +95,4 @@ test-filter-mirror
 test-filter-redirector
 *-test
 qapi-schema/*.test.*
+vm/*.img
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 02/12] qemu.py: Add "wait()" method
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-08 14:13   ` Alex Bennée
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh Fam Zheng
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/qemu.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 880e3e8219..153f2d1564 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -143,6 +143,13 @@ class QEMUMachine(object):
             self._post_shutdown()
             raise
 
+    def wait(self):
+        '''Wait for the VM to power off'''
+        self._popen.wait()
+        self._qmp.close()
+        self._load_io_log()
+        self._post_shutdown()
+
     def shutdown(self):
         '''Terminate the VM and clean up'''
         if self.is_running():
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 02/12] qemu.py: Add "wait()" method Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-08 14:42   ` Alex Bennée
  2017-09-08 14:56   ` Peter Maydell
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib Fam Zheng
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100755 scripts/archive-source.sh

diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
new file mode 100755
index 0000000000..3cae7f34d3
--- /dev/null
+++ b/scripts/archive-source.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+#
+# Author: Fam Zheng <famz@redhat.com>
+#
+# Create archive of source tree, including submodules
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+
+set -e
+
+if test $# -lt 1; then
+    echo "Usage: $0 <output>"
+    exit 1
+fi
+
+submodules=$(git submodule foreach --recursive --quiet 'echo $name')
+
+if test -n "$submodules"; then
+    {
+        git ls-files
+        for sm in $submodules; do
+            (cd $sm; git ls-files) | sed "s:^:$sm/:"
+        done
+    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list
+else
+    git ls-files > $1.list
+fi
+
+tar -cf $1 -T $1.list
+rm $1.list
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (2 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-08 15:22   ` Alex Bennée
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 05/12] tests: Add ubuntu.i386 image Fam Zheng
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

This is the common code to implement a "VM test" to

  1) Download and initialize a pre-defined VM that has necessary
  dependencies to build QEMU and SSH access.

  2) Archive $SRC_PATH to a .tar file.

  3) Boot the VM, and pass the source tar file to the guest.

  4) SSH into the VM, untar the source tarball, build from the source.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/basevm.py | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 276 insertions(+)
 create mode 100755 tests/vm/basevm.py

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
new file mode 100755
index 0000000000..9db91d61fa
--- /dev/null
+++ b/tests/vm/basevm.py
@@ -0,0 +1,276 @@
+#!/usr/bin/env python
+#
+# VM testing base class
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import time
+import datetime
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
+from qemu import QEMUMachine
+import subprocess
+import hashlib
+import optparse
+import atexit
+import tempfile
+import shutil
+import multiprocessing
+import traceback
+
+SSH_KEY = """\
+-----BEGIN RSA PRIVATE KEY-----
+MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
+coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
+RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
+7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
+Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
+cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
+7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
+R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
+rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
+cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
+vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
+XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
+j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
+T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
+XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
+JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
+CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
+bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
+eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
+BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
+CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
+D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
+VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
+5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
+-----END RSA PRIVATE KEY-----
+"""
+SSH_PUB_KEY = """\
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn qemu-vm-key
+"""
+
+class BaseVM(object):
+    GUEST_USER = "qemu"
+    GUEST_PASS = "qemupass"
+    ROOT_PASS = "qemupass"
+
+    # The script to run in the guest that builds QEMU
+    BUILD_SCRIPT = ""
+    # The guest name, to be overridden by subclasses
+    name = "#base"
+    def __init__(self, debug=False, vcpus=None):
+        self._guest = None
+        self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", dir=".")
+        atexit.register(shutil.rmtree, self._tmpdir)
+
+        self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
+        open(self._ssh_key_file, "w").write(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)
+
+        self.debug = debug
+        self._stderr = sys.stderr
+        self._devnull = open("/dev/null", "w")
+        if self.debug:
+            self._stdout = sys.stdout
+        else:
+            self._stdout = self._devnull
+        self._args = [ \
+            "-nodefaults", "-m", "2G",
+            "-cpu", "host",
+            "-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
+            "-device", "virtio-net-pci,netdev=vnet",
+            "-vnc", ":0,to=20",
+            "-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
+        if vcpus:
+            self._args += ["-smp", str(vcpus)]
+        if os.access("/dev/kvm", os.R_OK | os.W_OK):
+            self._args += ["-enable-kvm"]
+        else:
+            logging.info("KVM not available, not using -enable-kvm")
+        self._data_args = []
+
+    def _download_with_cache(self, url, sha256sum=None):
+        def check_sha256sum(fname):
+            if not sha256sum:
+                return True
+            checksum = subprocess.check_output(["sha256sum", fname]).split()[0]
+            return sha256sum == checksum
+
+        cache_dir = os.path.expanduser("~/.cache/qemu-vm/download")
+        if not os.path.exists(cache_dir):
+            os.makedirs(cache_dir)
+        fname = os.path.join(cache_dir, hashlib.sha1(url).hexdigest())
+        if os.path.exists(fname) and check_sha256sum(fname):
+            return fname
+        logging.debug("Downloading %s to %s...", url, fname)
+        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"],
+                              stdout=self._stdout, stderr=self._stderr)
+        os.rename(fname + ".download", fname)
+        return fname
+
+    def _ssh_do(self, user, cmd, check, interactive=False):
+        ssh_cmd = ["ssh", "-q",
+                   "-o", "StrictHostKeyChecking=no",
+                   "-o", "UserKnownHostsFile=/dev/null",
+                   "-o", "ConnectTimeout=1",
+                   "-p", self.ssh_port, "-i", self._ssh_key_file]
+        if interactive:
+            ssh_cmd += ['-t']
+        assert not isinstance(cmd, str)
+        ssh_cmd += ["%s@127.0.0.1" % user] + list(cmd)
+        logging.debug("ssh_cmd: %s", " ".join(ssh_cmd))
+        r = subprocess.call(ssh_cmd,
+                            stdin=sys.stdin if interactive else self._devnull,
+                            stdout=sys.stdout if interactive else self._stdout,
+                            stderr=sys.stderr if interactive else self._stderr)
+        if check and r != 0:
+            raise Exception("SSH command failed: %s" % cmd)
+        return r
+
+    def ssh(self, *cmd):
+        return self._ssh_do(self.GUEST_USER, cmd, False)
+
+    def ssh_interactive(self, *cmd):
+        return self._ssh_do(self.GUEST_USER, cmd, False, True)
+
+    def ssh_root(self, *cmd):
+        return self._ssh_do("root", cmd, False)
+
+    def ssh_check(self, *cmd):
+        self._ssh_do(self.GUEST_USER, cmd, True)
+
+    def ssh_root_check(self, *cmd):
+        self._ssh_do("root", cmd, True)
+
+    def build_image(self, img):
+        raise NotImplementedError
+
+    def add_source_dir(self, src_dir):
+        name = "data-" + hashlib.sha1(src_dir).hexdigest()[:5]
+        tarfile = os.path.join(self._tmpdir, name + ".tar")
+        logging.debug("Creating archive %s for src_dir dir: %s", tarfile, src_dir)
+        subprocess.check_call(["./scripts/archive-source.sh", tarfile],
+                              cwd=src_dir, stdin=self._devnull,
+                              stdout=self._stdout, stderr=self._stderr)
+        self._data_args += ["-drive",
+                            "file=%s,if=none,id=%s,cache=writeback,format=raw" % \
+                                    (tarfile, name),
+                            "-device",
+                            "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
+        logging.debug("QEMU args: %s", " ".join(args))
+        guest = QEMUMachine(binary=os.environ.get("QEMU", "qemu-system-x86_64"),
+                            args=args)
+        guest.launch()
+        atexit.register(self.shutdown)
+        self._guest = guest
+        usernet_info = guest.qmp("human-monitor-command",
+                                 command_line="info usernet")
+        self.ssh_port = None
+        for l in usernet_info["return"].splitlines():
+            fields = l.split()
+            if "TCP[HOST_FORWARD]" in fields and "22" in fields:
+                self.ssh_port = l.split()[3]
+        if not self.ssh_port:
+            raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \
+                            usernet_info)
+
+    def wait_ssh(self, seconds=120):
+        starttime = datetime.datetime.now()
+        guest_up = False
+        while (datetime.datetime.now() - starttime).total_seconds() < seconds:
+            if self.ssh("exit 0") == 0:
+                guest_up = True
+                break
+            time.sleep(1)
+        if not guest_up:
+            raise Exception("Timeout while waiting for guest ssh")
+
+    def shutdown(self):
+        self._guest.shutdown()
+
+    def wait(self):
+        self._guest.wait()
+
+    def qmp(self, *args, **kwargs):
+        return self._guest.qmp(*args, **kwargs)
+
+def parse_args(vm_name):
+    parser = optparse.OptionParser(description="""
+    VM test utility.  Exit codes: 0 = success, 1 = command line error, 2 = environment initialization failed, 3 = test command failed""")
+    parser.add_option("--debug", "-D", action="store_true",
+                      help="enable debug output")
+    parser.add_option("--image", "-i", default="%s.img" % vm_name,
+                      help="image file name")
+    parser.add_option("--force", "-f", action="store_true",
+                      help="force build image even if image exists")
+    parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() / 2,
+                      help="number of virtual CPUs")
+    parser.add_option("--build-image", "-b", action="store_true",
+                      help="build image")
+    parser.add_option("--build-qemu",
+                      help="build QEMU from source in guest")
+    parser.add_option("--interactive", "-I", action="store_true",
+                      help="Interactively run command")
+    parser.disable_interspersed_args()
+    return parser.parse_args()
+
+def main(vmcls):
+    try:
+        args, argv = parse_args(vmcls.name)
+        if not argv and not args.build_qemu and not args.build_image:
+            print "Nothing to do?"
+            return 1
+        if args.debug:
+            logging.getLogger().setLevel(logging.DEBUG)
+        vm = vmcls(debug=args.debug, vcpus=args.jobs)
+        if args.build_image:
+            if os.path.exists(args.image) and not args.force:
+                sys.stderr.writelines(["Image file exists: %s\n" % args.image,
+                                      "Use --force option to overwrite\n"])
+                return 1
+            return vm.build_image(args.image)
+        if args.build_qemu:
+            vm.add_source_dir(args.build_qemu)
+            cmd = [vm.BUILD_SCRIPT.format(
+                   configure_opts = " ".join(argv),
+                   jobs=args.jobs)]
+        else:
+            cmd = argv
+        vm.boot(args.image + ",snapshot=on")
+        vm.wait_ssh()
+    except Exception as e:
+        if isinstance(e, SystemExit) and e.code == 0:
+            return 0
+        sys.stderr.write("Failed to prepare guest environment\n")
+        traceback.print_exc()
+        return 2
+
+    if args.interactive:
+        if vm.ssh_interactive(*cmd) == 0:
+            return 0
+        vm.ssh_interactive()
+        return 3
+    else:
+        if vm.ssh(*cmd) != 0:
+            return 3
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 05/12] tests: Add ubuntu.i386 image
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (3 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 06/12] tests: Add FreeBSD image Fam Zheng
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

This adds a 32bit guest.

The official LTS cloud image is downloaded and initialized with
cloud-init.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/ubuntu.i386 | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100755 tests/vm/ubuntu.i386

diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
new file mode 100755
index 0000000000..1a55856d9c
--- /dev/null
+++ b/tests/vm/ubuntu.i386
@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+#
+# Ubuntu i386 image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+import time
+
+class UbuntuX86VM(basevm.BaseVM):
+    name = "ubuntu.i386"
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d);
+        sudo chmod a+r /dev/vdb;
+        tar -xf /dev/vdb;
+        ./configure {configure_opts};
+        make -j{jobs};
+        make check;
+    """
+
+    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")
+        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" % basevm.SSH_PUB_KEY,
+                          "  - name: root\n",
+                          "    ssh-authorized-keys:\n",
+                          "    - %s\n" % basevm.SSH_PUB_KEY])
+        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 build_image(self, img):
+        cimg = self._download_with_cache("https://cloud-images.ubuntu.com/releases/16.04/release/ubuntu-16.04-server-cloudimg-i386-disk1.img")
+        img_tmp = img + ".tmp"
+        subprocess.check_call(["cp", "-f", cimg, img_tmp])
+        subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
+        self.boot(img_tmp, extra_args = ["-cdrom", self._gen_cloud_init_iso()])
+        self.wait_ssh()
+        self.ssh_root_check("touch /etc/cloud/cloud-init.disabled")
+        self.ssh_root_check("apt-get update")
+        self.ssh_root_check("apt-get install -y cloud-initramfs-growroot")
+        # Don't check the status in case the guest hang up too quickly
+        self.ssh_root("sync && reboot")
+        time.sleep(5)
+        self.wait_ssh()
+        # The previous update sometimes doesn't survive a reboot, so do it again
+        self.ssh_root_check("apt-get update")
+        self.ssh_root_check("apt-get build-dep -y qemu")
+        self.ssh_root_check("apt-get install -y libfdt-dev")
+        self.ssh_root("poweroff")
+        self.wait()
+        if os.path.exists(img):
+            os.remove(img)
+        os.rename(img_tmp, img)
+        return 0
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(UbuntuX86VM))
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 06/12] tests: Add FreeBSD image
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (4 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 05/12] tests: Add ubuntu.i386 image Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 07/12] tests: Add NetBSD image Fam Zheng
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/freebsd | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100755 tests/vm/freebsd

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
new file mode 100755
index 0000000000..6840da0bf0
--- /dev/null
+++ b/tests/vm/freebsd
@@ -0,0 +1,42 @@
+#!/usr/bin/env python
+#
+# FreeBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class FreeBSDVM(basevm.BaseVM):
+    name = "freebsd"
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
+        tar -xf /dev/vtbd1;
+        ./configure {configure_opts};
+        gmake -j{jobs};
+        gmake check;
+    """
+
+    def build_image(self, img):
+        cimg = self._download_with_cache("http://download.patchew.org/freebsd-11.1-amd64.img.xz",
+                sha256sum='adcb771549b37bc63826c501f05121a206ed3d9f55f49145908f7e1432d65891')
+        img_tmp_xz = img + ".tmp.xz"
+        img_tmp = img + ".tmp"
+        subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+        subprocess.check_call(["xz", "-df", img_tmp_xz])
+        if os.path.exists(img):
+            os.remove(img)
+        os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(FreeBSDVM))
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 07/12] tests: Add NetBSD image
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (5 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 06/12] tests: Add FreeBSD image Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 08/12] tests: Add OpenBSD image Fam Zheng
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kamil Rytarowski <n54@gmx.com>
---
 tests/vm/netbsd | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100755 tests/vm/netbsd

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
new file mode 100755
index 0000000000..559e89c8a6
--- /dev/null
+++ b/tests/vm/netbsd
@@ -0,0 +1,42 @@
+#!/usr/bin/env python
+#
+# NetBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class NetBSDVM(basevm.BaseVM):
+    name = "netbsd"
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
+        tar -xf /dev/rld1a;
+        ./configure --python=python2.7 {configure_opts};
+        gmake -j{jobs};
+        gmake check;
+    """
+
+    def build_image(self, img):
+        cimg = self._download_with_cache("http://download.patchew.org/netbsd-7.1-amd64.img.xz",
+                                         sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
+        img_tmp_xz = img + ".tmp.xz"
+        img_tmp = img + ".tmp"
+        subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+        subprocess.check_call(["xz", "-df", img_tmp_xz])
+        if os.path.exists(img):
+            os.remove(img)
+        os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(NetBSDVM))
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 08/12] tests: Add OpenBSD image
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (6 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 07/12] tests: Add NetBSD image Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 09/12] Makefile: Add rules to run vm tests Fam Zheng
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/openbsd | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100755 tests/vm/openbsd

diff --git a/tests/vm/openbsd b/tests/vm/openbsd
new file mode 100755
index 0000000000..57b10105f7
--- /dev/null
+++ b/tests/vm/openbsd
@@ -0,0 +1,43 @@
+#!/usr/bin/env python
+#
+# OpenBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class OpenBSDVM(basevm.BaseVM):
+    name = "openbsd"
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
+        tar -xf /dev/rsd1c;
+        ./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 --python=python2.7 {configure_opts};
+        gmake -j{jobs};
+        # XXX: "gmake check" seems to always hang or fail
+        #gmake check;
+    """
+
+    def build_image(self, img):
+        cimg = self._download_with_cache("http://download.patchew.org/openbsd-6.1-amd64.img.xz",
+                sha256sum='8c6cedc483e602cfee5e04f0406c64eb99138495e8ca580bc0293bcf0640c1bf')
+        img_tmp_xz = img + ".tmp.xz"
+        img_tmp = img + ".tmp"
+        subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+        subprocess.check_call(["xz", "-df", img_tmp_xz])
+        if os.path.exists(img):
+            os.remove(img)
+        os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(OpenBSDVM))
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 09/12] Makefile: Add rules to run vm tests
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (7 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 08/12] tests: Add OpenBSD image Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 10/12] MAINTAINERS: Add tests/vm entry Fam Zheng
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile                  |  2 ++
 configure                 |  2 +-
 tests/vm/Makefile.include | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 tests/vm/Makefile.include

diff --git a/Makefile b/Makefile
index 337a1f6f9b..946eb2ce35 100644
--- a/Makefile
+++ b/Makefile
@@ -822,6 +822,7 @@ endif
 -include $(wildcard *.d tests/*.d)
 
 include $(SRC_PATH)/tests/docker/Makefile.include
+include $(SRC_PATH)/tests/vm/Makefile.include
 
 .PHONY: help
 help:
@@ -845,6 +846,7 @@ help:
 	@echo  'Test targets:'
 	@echo  '  check           - Run all tests (check-help for details)'
 	@echo  '  docker          - Help about targets running tests inside Docker containers'
+	@echo  '  vm-test         - Help about targets running tests inside VM'
 	@echo  ''
 	@echo  'Documentation targets:'
 	@echo  '  html info pdf txt'
diff --git a/configure b/configure
index fb7e34a901..09471fbf54 100755
--- a/configure
+++ b/configure
@@ -6563,7 +6563,7 @@ if test "$ccache_cpp2" = "yes"; then
 fi
 
 # build tree in object directory in case the source is not in the current directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
 DIRS="$DIRS docs docs/interop fsdev"
 DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
new file mode 100644
index 0000000000..5daa2a3b73
--- /dev/null
+++ b/tests/vm/Makefile.include
@@ -0,0 +1,42 @@
+# Makefile for VM tests
+
+.PHONY: vm-build-all
+
+IMAGES := ubuntu.i386 freebsd netbsd openbsd
+IMAGE_FILES := $(patsubst %, tests/vm/%.img, $(IMAGES))
+
+.PRECIOUS: $(IMAGE_FILES)
+
+vm-test:
+	@echo "vm-test: Test QEMU in preconfigured virtual machines"
+	@echo
+	@echo "  vm-build-ubuntu.i386            - Build QEMU in ubuntu i386 VM"
+	@echo "  vm-build-freebsd                - Build QEMU in FreeBSD VM"
+	@echo "  vm-build-netbsd                 - Build QEMU in NetBSD VM"
+	@echo "  vm-build-openbsd                - Build QEMU in OpenBSD VM"
+
+vm-build-all: $(addprefix vm-build-, $(IMAGES))
+
+tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
+		$(SRC_PATH)/tests/vm/basevm.py \
+		$(SRC_PATH)/tests/vm/Makefile.include
+	$(call quiet-command, \
+		$< \
+		$(if $(V)$(DEBUG), --debug) \
+		--image "$@" \
+		--force \
+		--build-image $@, \
+		"  VM-IMAGE $*")
+
+
+# Build in VM $(IMAGE)
+vm-build-%: tests/vm/%.img
+	$(call quiet-command, \
+		$(SRC_PATH)/tests/vm/$* \
+		$(if $(V)$(DEBUG), --debug) \
+		$(if $(DEBUG), --interactive) \
+		$(if $(J),--jobs $(J)) \
+		--image "$<" \
+		--build-qemu $(SRC_PATH), \
+		"  VM-BUILD $*")
+
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 10/12] MAINTAINERS: Add tests/vm entry
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (8 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 09/12] Makefile: Add rules to run vm tests Fam Zheng
@ 2017-09-05  2:11 ` Fam Zheng
  2017-09-05  2:12 ` [Qemu-devel] [PATCH v6 11/12] tests: Add README for vm tests Fam Zheng
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5ec945c9af..f6b0fd156e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1897,6 +1897,7 @@ S: Maintained
 F: .travis.yml
 F: .shippable.yml
 F: tests/docker/
+F: tests/vm/
 W: https://travis-ci.org/qemu/qemu
 W: https://app.shippable.com/github/qemu/qemu
 W: http://patchew.org/QEMU/
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 11/12] tests: Add README for vm tests
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (9 preceding siblings ...)
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 10/12] MAINTAINERS: Add tests/vm entry Fam Zheng
@ 2017-09-05  2:12 ` Fam Zheng
  2017-09-05  2:12 ` [Qemu-devel] [PATCH v6 12/12] docker: Use archive-source.py Fam Zheng
  2017-09-08 16:20 ` [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Alex Bennée
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/README | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 tests/vm/README

diff --git a/tests/vm/README b/tests/vm/README
new file mode 100644
index 0000000000..7d2fe4ac8d
--- /dev/null
+++ b/tests/vm/README
@@ -0,0 +1,63 @@
+=== VM test suite to run build in guests ===
+
+== Intro ==
+
+This test suite contains scripts that bootstrap various guest images that have
+necessary packages to build QEMU. The basic usage is documented in Makefile
+help which is displayed with "make vm-test".
+
+== Quick start ==
+
+Run "make vm-test" to list available make targets.
+
+== Manual invocation ==
+
+Each guest script is an executable script with the same command line options.
+For example to work with the netbsd guest, use $QEMU_SRC/tests/vm/netbsd:
+
+    $ cd $QEMU_SRC/tests/vm
+
+    # To bootstrap the image
+    $ ./netbsd --build-image --image /var/tmp/netbsd.img
+    <...>
+
+    # To run an arbitrary command in guest (the output will not be echoed unless
+    # --debug is added)
+    $ ./netbsd --debug --image /var/tmp/netbsd.img uname -a
+
+    # To build QEMU in guest
+    $ ./netbsd --debug --image /var/tmp/netbsd.img --build-qemu $QEMU_SRC
+
+    # To get to an interactive shell
+    $ ./netbsd --interactive --image /var/tmp/netbsd.img sh
+
+== Adding new guests ==
+
+Please look at existing guest scripts for how to add new guests.
+
+Most importantly, create a subclass of BaseVM and implement build_image()
+method and define BUILD_SCRIPT, then finally call basevm.main() from the
+script's main().
+
+  - Usually in build_image(), a template image is downloaded from a predefined
+    URL. BaseVM._download_with_cache() takes care of the cache and the
+    checksum, so consider using it.
+
+  - Once the image is downloaded, users, SSH server and QEMU build deps should
+    be set up:
+
+    * Root password set to BaseVM.ROOT_PASS
+    * User BaseVM.GUEST_USER is created, and password set to BaseVM.GUEST_PASS
+    * SSH service is enabled and started on boot, BaseVM.SSH_PUB_KEY is added
+      to authorized_keys of both root and the normal user
+    * DHCP client service is enabled and started on boot, so that it can
+      automatically configure the virtio-net-pci NIC and communicate with QEMU
+      user net (10.0.2.2)
+    * Necessary packages are installed to untar the source tarball and build
+      QEMU
+
+  - Write a proper BUILD_SCRIPT template, which should be a shell script that
+    untars a raw virtio-blk block device, which is the tarball data blob of the
+    QEMU source tree, then configure/build it. Running "make check" is also
+    recommended.
+
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v6 12/12] docker: Use archive-source.py
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (10 preceding siblings ...)
  2017-09-05  2:12 ` [Qemu-devel] [PATCH v6 11/12] tests: Add README for vm tests Fam Zheng
@ 2017-09-05  2:12 ` Fam Zheng
  2017-09-08 16:20 ` [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Alex Bennée
  12 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-05  2:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: berrange, Alex Bennée, Fam Zheng,
	Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/docker/Makefile.include | 15 ++-------------
 tests/docker/run              |  8 +-------
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index aaab1a4208..7a027d5bd6 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -17,24 +17,13 @@ DOCKER_TOOLS := travis
 TESTS ?= %
 IMAGES ?= %
 
-# Make archive from git repo $1 to tar.gz $2
-make-archive-maybe = $(if $(wildcard $1/*), \
-	$(call quiet-command, \
-		(cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \
-			git archive -1 HEAD --format=tar.gz; \
-		else \
-			git archive -1 $$(git stash create) --format=tar.gz; \
-		fi) > $2, \
-		"ARCHIVE","$(notdir $2)"))
-
 CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.$$$$)
 DOCKER_SRC_COPY := docker-src.$(CUR_TIME)
 
 $(DOCKER_SRC_COPY):
 	@mkdir $@
-	$(call make-archive-maybe, $(SRC_PATH), $@/qemu.tgz)
-	$(call make-archive-maybe, $(SRC_PATH)/dtc, $@/dtc.tgz)
-	$(call make-archive-maybe, $(SRC_PATH)/pixman, $@/pixman.tgz)
+	$(call quiet-command, $(SRC_PATH)/scripts/archive-source.sh $@/qemu.tar, \
+		"GEN", "$@/qemu.tar")
 	$(call quiet-command, cp $(SRC_PATH)/tests/docker/run $@/run, \
 		"COPY","RUNNER")
 
diff --git a/tests/docker/run b/tests/docker/run
index c1e4513bce..9eb9165f76 100755
--- a/tests/docker/run
+++ b/tests/docker/run
@@ -32,13 +32,7 @@ export TEST_DIR=/tmp/qemu-test
 mkdir -p $TEST_DIR/{src,build,install}
 
 # Extract the source tarballs
-tar -C $TEST_DIR/src -xzf $BASE/qemu.tgz
-for p in dtc pixman; do
-    if test -f $BASE/$p.tgz; then
-        tar -C $TEST_DIR/src/$p -xzf $BASE/$p.tgz
-        export FEATURES="$FEATURES $p"
-    fi
-done
+tar -C $TEST_DIR/src -xf $BASE/qemu.tar
 
 if test -n "$SHOW_ENV"; then
     if test -f /packages.txt; then
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images Fam Zheng
@ 2017-09-08 14:12   ` Alex Bennée
  2017-09-08 23:12     ` Fam Zheng
  0 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2017-09-08 14:12 UTC (permalink / raw)
  To: Fam Zheng
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski


Fam Zheng <famz@redhat.com> writes:

> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  .gitignore       | 1 +
>  tests/.gitignore | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/.gitignore b/.gitignore
> index cf65316863..40acfcb9e2 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -52,6 +52,7 @@
>  /vscclient
>  /vhost-user-scsi
>  /fsdev/virtfs-proxy-helper
> +*.tmp

I'm not sure we should be dumping anything in the rootdir, .gitignored
or not.

>  *.[1-9]
>  *.a
>  *.aux
> diff --git a/tests/.gitignore b/tests/.gitignore
> index fed0189a5a..cf6d99c91e 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -95,3 +95,4 @@ test-filter-mirror
>  test-filter-redirector
>  *-test
>  qapi-schema/*.test.*
> +vm/*.img


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 02/12] qemu.py: Add "wait()" method
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 02/12] qemu.py: Add "wait()" method Fam Zheng
@ 2017-09-08 14:13   ` Alex Bennée
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2017-09-08 14:13 UTC (permalink / raw)
  To: Fam Zheng
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski


Fam Zheng <famz@redhat.com> writes:

> Signed-off-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

> ---
>  scripts/qemu.py | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 880e3e8219..153f2d1564 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -143,6 +143,13 @@ class QEMUMachine(object):
>              self._post_shutdown()
>              raise
>
> +    def wait(self):
> +        '''Wait for the VM to power off'''
> +        self._popen.wait()
> +        self._qmp.close()
> +        self._load_io_log()
> +        self._post_shutdown()
> +
>      def shutdown(self):
>          '''Terminate the VM and clean up'''
>          if self.is_running():


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh Fam Zheng
@ 2017-09-08 14:42   ` Alex Bennée
  2017-09-08 23:14     ` Fam Zheng
  2017-09-08 14:56   ` Peter Maydell
  1 sibling, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2017-09-08 14:42 UTC (permalink / raw)
  To: Fam Zheng
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski


Fam Zheng <famz@redhat.com> writes:

> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100755 scripts/archive-source.sh
>
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> new file mode 100755
> index 0000000000..3cae7f34d3
> --- /dev/null
> +++ b/scripts/archive-source.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +#
> +# Author: Fam Zheng <famz@redhat.com>
> +#
> +# Create archive of source tree, including submodules
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
> +
> +set -e
> +
> +if test $# -lt 1; then
> +    echo "Usage: $0 <output>"

Maybe <output tarball> to make it clear what it creates?

> +    exit 1
> +fi
> +
> +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> +
> +if test -n "$submodules"; then
> +    {
> +        git ls-files

Couldn't we do the main git ls-files first and then append the data for
any submodules?

> +        for sm in $submodules; do
> +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
> +        done
> +    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list
> +else
> +    git ls-files > $1.list
> +fi
> +
> +tar -cf $1 -T $1.list
> +rm $1.list


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh Fam Zheng
  2017-09-08 14:42   ` Alex Bennée
@ 2017-09-08 14:56   ` Peter Maydell
  2017-09-08 23:13     ` Fam Zheng
  1 sibling, 1 reply; 29+ messages in thread
From: Peter Maydell @ 2017-09-08 14:56 UTC (permalink / raw)
  To: Fam Zheng
  Cc: QEMU Developers, Daniel P. Berrange, Alex Bennée,
	Philippe Mathieu-Daudé,
	Paolo Bonzini, Stefan Hajnoczi, Cleber Rosa, Eric Blake,
	Kamil Rytarowski

On 5 September 2017 at 03:11, Fam Zheng <famz@redhat.com> wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100755 scripts/archive-source.sh
>
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> new file mode 100755
> index 0000000000..3cae7f34d3
> --- /dev/null
> +++ b/scripts/archive-source.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +#
> +# Author: Fam Zheng <famz@redhat.com>
> +#
> +# Create archive of source tree, including submodules
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.

Why GPL-2-only? We generally prefer GPL-2-or-later for new
files, or something more permissive.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib Fam Zheng
@ 2017-09-08 15:22   ` Alex Bennée
  2017-09-08 23:29     ` Fam Zheng
  0 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2017-09-08 15:22 UTC (permalink / raw)
  To: Fam Zheng
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski


Fam Zheng <famz@redhat.com> writes:

> This is the common code to implement a "VM test" to
>
>   1) Download and initialize a pre-defined VM that has necessary
>   dependencies to build QEMU and SSH access.
>
>   2) Archive $SRC_PATH to a .tar file.
>
>   3) Boot the VM, and pass the source tar file to the guest.
>
>   4) SSH into the VM, untar the source tarball, build from the source.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/vm/basevm.py | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 276 insertions(+)
>  create mode 100755 tests/vm/basevm.py
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> new file mode 100755
> index 0000000000..9db91d61fa
> --- /dev/null
> +++ b/tests/vm/basevm.py
> @@ -0,0 +1,276 @@
> +#!/usr/bin/env python
> +#
> +# VM testing base class
> +#
> +# Copyright (C) 2017 Red Hat Inc.
> +#
> +# Authors:
> +#  Fam Zheng <famz@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import os
> +import sys
> +import logging
> +import time
> +import datetime
> +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
> +from qemu import QEMUMachine
> +import subprocess
> +import hashlib
> +import optparse
> +import atexit
> +import tempfile
> +import shutil
> +import multiprocessing
> +import traceback
> +
> +SSH_KEY = """\
> +-----BEGIN RSA PRIVATE KEY-----
> +MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
> +coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
> ++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
> +RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
> +7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
> +Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
> +cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
> +7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
> +R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
> +rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
> +cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
> +vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
> +XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
> +j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
> +T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
> +XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
> +JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
> +CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
> +bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
> +eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
> +BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
> +CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
> +D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
> +VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
> +5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
> +-----END RSA PRIVATE KEY-----
> +"""
> +SSH_PUB_KEY = """\
> +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn qemu-vm-key
> +"""

I'm not sure we should be embedding the keys in the script. I understand
we need a common key for downloaded images (although it would be better
to post-customise the image after download with the local developer
keys). Perhaps ./tests/testing-keys/id_rsa[.pub]?

> +
> +class BaseVM(object):
> +    GUEST_USER = "qemu"
> +    GUEST_PASS = "qemupass"
> +    ROOT_PASS = "qemupass"
> +
> +    # The script to run in the guest that builds QEMU
> +    BUILD_SCRIPT = ""
> +    # The guest name, to be overridden by subclasses
> +    name = "#base"
> +    def __init__(self, debug=False, vcpus=None):
> +        self._guest = None
> +        self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", dir=".")
> +        atexit.register(shutil.rmtree, self._tmpdir)
> +
> +        self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
> +        open(self._ssh_key_file, "w").write(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)

As above, I think it would be better just to keep copies of the keys in
the tests directory rather than in the python source.

> +
> +        self.debug = debug
> +        self._stderr = sys.stderr
> +        self._devnull = open("/dev/null", "w")

You can use os.devnull as a portable reference.

> +        if self.debug:
> +            self._stdout = sys.stdout
> +        else:
> +            self._stdout = self._devnull
> +        self._args = [ \
> +            "-nodefaults", "-m", "2G",
> +            "-cpu", "host",
> +            "-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",

It doesn't help that howstfwd is poorly documented. Are we trying to map
an ephemeral port to the guests 22?

> +            "-device", "virtio-net-pci,netdev=vnet",
> +            "-vnc", ":0,to=20",

Do we need a GUI?

> +            "-serial", "file:%s" % os.path.join(self._tmpdir,
> "serial.out")]
> +        if vcpus:
> +            self._args += ["-smp", str(vcpus)]
> +        if os.access("/dev/kvm", os.R_OK | os.W_OK):
> +            self._args += ["-enable-kvm"]
> +        else:
> +            logging.info("KVM not available, not using -enable-kvm")
> +        self._data_args = []
> +
> +    def _download_with_cache(self, url, sha256sum=None):
> +        def check_sha256sum(fname):
> +            if not sha256sum:
> +                return True
> +            checksum = subprocess.check_output(["sha256sum", fname]).split()[0]
> +            return sha256sum == checksum
> +
> +        cache_dir = os.path.expanduser("~/.cache/qemu-vm/download")
> +        if not os.path.exists(cache_dir):
> +            os.makedirs(cache_dir)
> +        fname = os.path.join(cache_dir, hashlib.sha1(url).hexdigest())
> +        if os.path.exists(fname) and check_sha256sum(fname):
> +            return fname
> +        logging.debug("Downloading %s to %s...", url, fname)
> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"],
> +                              stdout=self._stdout,
> stderr=self._stderr)

Using wget rather than doing it internally adds a utility requirement.
Is doing a pure python urllib2 fetch too slow for this?

> +        os.rename(fname + ".download", fname)
> +        return fname
> +
> +    def _ssh_do(self, user, cmd, check, interactive=False):
> +        ssh_cmd = ["ssh", "-q",
> +                   "-o", "StrictHostKeyChecking=no",
> +                   "-o", "UserKnownHostsFile=/dev/null",
> +                   "-o", "ConnectTimeout=1",
> +                   "-p", self.ssh_port, "-i", self._ssh_key_file]
> +        if interactive:
> +            ssh_cmd += ['-t']
> +        assert not isinstance(cmd, str)
> +        ssh_cmd += ["%s@127.0.0.1" % user] + list(cmd)
> +        logging.debug("ssh_cmd: %s", " ".join(ssh_cmd))
> +        r = subprocess.call(ssh_cmd,
> +                            stdin=sys.stdin if interactive else self._devnull,
> +                            stdout=sys.stdout if interactive else self._stdout,
> +                            stderr=sys.stderr if interactive else self._stderr)
> +        if check and r != 0:
> +            raise Exception("SSH command failed: %s" % cmd)
> +        return r
> +
> +    def ssh(self, *cmd):
> +        return self._ssh_do(self.GUEST_USER, cmd, False)
> +
> +    def ssh_interactive(self, *cmd):
> +        return self._ssh_do(self.GUEST_USER, cmd, False, True)
> +
> +    def ssh_root(self, *cmd):
> +        return self._ssh_do("root", cmd, False)
> +
> +    def ssh_check(self, *cmd):
> +        self._ssh_do(self.GUEST_USER, cmd, True)
> +
> +    def ssh_root_check(self, *cmd):
> +        self._ssh_do("root", cmd, True)
> +
> +    def build_image(self, img):
> +        raise NotImplementedError
> +
> +    def add_source_dir(self, src_dir):
> +        name = "data-" + hashlib.sha1(src_dir).hexdigest()[:5]
> +        tarfile = os.path.join(self._tmpdir, name + ".tar")
> +        logging.debug("Creating archive %s for src_dir dir: %s", tarfile, src_dir)
> +        subprocess.check_call(["./scripts/archive-source.sh", tarfile],
> +                              cwd=src_dir, stdin=self._devnull,
> +                              stdout=self._stdout, stderr=self._stderr)
> +        self._data_args += ["-drive",
> +                            "file=%s,if=none,id=%s,cache=writeback,format=raw" % \
> +                                    (tarfile, name),
> +                            "-device",
> +                            "virtio-blk,drive=%s,serial=%s,bootindex=1" % (name, name)]
> +
> +    def boot(self, img, extra_args=[]):
> +        args = self._args + [
> +            "-device", "VGA",

Is specifying a display device at boot time really needed (say rather
than further up with the rest of the machine description). Do we even
want a display given use ssh/serial for everything?

> +            "-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
> +            "-device", "virtio-blk,drive=drive0,bootindex=0"]
> +        args += self._data_args + extra_args
> +        logging.debug("QEMU args: %s", " ".join(args))
> +        guest = QEMUMachine(binary=os.environ.get("QEMU", "qemu-system-x86_64"),
> +                            args=args)
> +        guest.launch()
> +        atexit.register(self.shutdown)
> +        self._guest = guest
> +        usernet_info = guest.qmp("human-monitor-command",
> +                                 command_line="info usernet")
> +        self.ssh_port = None
> +        for l in usernet_info["return"].splitlines():
> +            fields = l.split()
> +            if "TCP[HOST_FORWARD]" in fields and "22" in fields:
> +                self.ssh_port = l.split()[3]
> +        if not self.ssh_port:
> +            raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \
> +                            usernet_info)
> +
> +    def wait_ssh(self, seconds=120):
> +        starttime = datetime.datetime.now()
> +        guest_up = False
> +        while (datetime.datetime.now() - starttime).total_seconds() < seconds:
> +            if self.ssh("exit 0") == 0:
> +                guest_up = True
> +                break
> +            time.sleep(1)
> +        if not guest_up:
> +            raise Exception("Timeout while waiting for guest ssh")
> +
> +    def shutdown(self):
> +        self._guest.shutdown()
> +
> +    def wait(self):
> +        self._guest.wait()
> +
> +    def qmp(self, *args, **kwargs):
> +        return self._guest.qmp(*args, **kwargs)
> +
> +def parse_args(vm_name):
> +    parser = optparse.OptionParser(description="""
> +    VM test utility.  Exit codes: 0 = success, 1 = command line error, 2 = environment initialization failed, 3 = test command failed""")
> +    parser.add_option("--debug", "-D", action="store_true",
> +                      help="enable debug output")
> +    parser.add_option("--image", "-i", default="%s.img" % vm_name,
> +                      help="image file name")
> +    parser.add_option("--force", "-f", action="store_true",
> +                      help="force build image even if image exists")
> +    parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() / 2,
> +                      help="number of virtual CPUs")
> +    parser.add_option("--build-image", "-b", action="store_true",
> +                      help="build image")
> +    parser.add_option("--build-qemu",
> +                      help="build QEMU from source in guest")
> +    parser.add_option("--interactive", "-I", action="store_true",
> +                      help="Interactively run command")
> +    parser.disable_interspersed_args()
> +    return parser.parse_args()
> +
> +def main(vmcls):
> +    try:
> +        args, argv = parse_args(vmcls.name)
> +        if not argv and not args.build_qemu and not args.build_image:
> +            print "Nothing to do?"
> +            return 1
> +        if args.debug:
> +            logging.getLogger().setLevel(logging.DEBUG)
> +        vm = vmcls(debug=args.debug, vcpus=args.jobs)
> +        if args.build_image:
> +            if os.path.exists(args.image) and not args.force:
> +                sys.stderr.writelines(["Image file exists: %s\n" % args.image,
> +                                      "Use --force option to overwrite\n"])
> +                return 1
> +            return vm.build_image(args.image)
> +        if args.build_qemu:
> +            vm.add_source_dir(args.build_qemu)
> +            cmd = [vm.BUILD_SCRIPT.format(
> +                   configure_opts = " ".join(argv),
> +                   jobs=args.jobs)]
> +        else:
> +            cmd = argv
> +        vm.boot(args.image + ",snapshot=on")

If this fails it's fairly cryptic:

/home/alex/lsrc/qemu/qemu.git/tests/vm/openbsd  --debug   --image "tests/vm/openbsd.img" --build-qemu /home/alex/lsrc/qemu/qemu.git
DEBUG:root:Creating archive ./vm-test-fxejnB.tmp/data-2de24.tar for src_dir dir: /home/alex/lsrc/qemu/qemu.git
DEBUG:root:QEMU args: -nodefaults -m 2G -cpu host -netdev user,id=vnet,hostfwd=:0.0.0.0:0-:22 -device virtio-net-pci,netdev=vnet -vnc :0,to=20 -serial file:./vm-test-fxejnB.tmp/serial.out -smp 4 -enable-kvm -device VGA -drive file=tests/vm/openbsd.img,snapshot=on,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -drive file=./vm-test-fxejnB.tmp/data-2de24.tar,if=none,id=data-2de24,cache=writeback,format=raw -device virtio-blk,drive=data-2de24,serial=data-2de24,bootindex=1
Failed to prepare guest environment
Traceback (most recent call last):
  File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 260, in main
    vm.boot(args.image + ",snapshot=on")
  File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 184, in boot
    guest.launch()
  File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 151, in launch
    self._post_launch()
  File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 135, in _post_launch
    self._qmp.accept()
  File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 147, in accept
    return self.__negotiate_capabilities()
  File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 60, in __negotiate_capabilities
    raise QMPConnectError
QMPConnectError

I assume QEMU failed to boot and might have given us a message that
would be useful.


> +        vm.wait_ssh()
> +    except Exception as e:
> +        if isinstance(e, SystemExit) and e.code == 0:
> +            return 0
> +        sys.stderr.write("Failed to prepare guest environment\n")
> +        traceback.print_exc()
> +        return 2
> +
> +    if args.interactive:
> +        if vm.ssh_interactive(*cmd) == 0:
> +            return 0
> +        vm.ssh_interactive()
> +        return 3
> +    else:
> +        if vm.ssh(*cmd) != 0:
> +            return 3


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)
  2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
                   ` (11 preceding siblings ...)
  2017-09-05  2:12 ` [Qemu-devel] [PATCH v6 12/12] docker: Use archive-source.py Fam Zheng
@ 2017-09-08 16:20 ` Alex Bennée
  12 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2017-09-08 16:20 UTC (permalink / raw)
  To: Fam Zheng
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski


Fam Zheng <famz@redhat.com> writes:

> v6: Add license to new file. [Philippe]
>     Change tests/.gitignore. [Philippe]
>
> v5: Generate source tar file with a script.
>     Fix tmpdir, use pwd.
>     Reduce default -j to half cores.
>
> v4: Drop unused imports and parameters. [Cleber]
>     Use --exclude-vcs (still no --exclude-vcs-ignores because it's too new). [Philippe]
>     Use gtar if available. [Philippe, Kamil]
>     /dev/ld1a -> /dev/rld1a for netbsd. [Kamil]
>     Only use '-enable-kvm' if /dev/kvm is there. [Kamil]
>     Grammar fixes of README. [Stefan]
>     Rename image on the server to include version and arch. [Kamil]
>     Just ignore *.tmp. [Philippe]
>
> v3: Drop RFC.
>     Add Stefan's and Kamil's reviewed-bys.
>     Use optparse. [Stefan]
>     Drop the VGA patch. [Paolo, Stefan]
>     Improve exit/exit code/doc. [Stefan]
>     Drop unused line from basevm.py. [Stefan]
>     Drop "--target-list" form Makefile.
>     More intelligent '-j'.
>     Add README. [Stefan]
>
> v2: - Add docstring. [Stefan]
>     - Call self._load_io_lod. [Stefan]
>     - Use "info usernet" and dynamic ssh_port forwarding. [Stefan]
>     - Add image checksum.
>     - Use os.rename() and os.makedirs(). [Stefan]
>     - Fix NetBSD URL. [Kamil]
>
> Build tests in one 32 bit Linux guest and three BSD images are defined in this
> series. This is a more managable way than the manually maintained virtual
> machines in patchew. Also, one big advantage of ephemeral VMs over long running
> guests is the reduced RAM usage of host, which makes it possible to have one
> host test all these BSD variants and probably more.
>
> The BSD guest templates are manually prepared following
>
> https://wiki.qemu.org/Hosts/BSD
>
> as it is not easy to automate. (The ideal approach is like the ubuntu.i386
> script, which configures the guest on top of an official released image, fully
> automatically.)
>
> Need for help: "gmake check" in the added OpenBSD image fails with -ENOMEM
> errors, even if I change "-m 2G" to "-m 8G" when starting VM. Ideas? And there
> is a warning from ./configure about OpenBSD going to be unsupported in coming
> releases, is it still the case?

OK I can't boot any of the VMs so I need some more feedback from the
script when things run.

>
> Fam
>
> Fam Zheng (12):
>   gitignore: Ignore vm test images
>   qemu.py: Add "wait()" method
>   scripts: Add archive-source.sh
>   tests: Add vm test lib
>   tests: Add ubuntu.i386 image
>   tests: Add FreeBSD image
>   tests: Add NetBSD image
>   tests: Add OpenBSD image
>   Makefile: Add rules to run vm tests
>   MAINTAINERS: Add tests/vm entry
>   tests: Add README for vm tests
>   docker: Use archive-source.py
>
>  .gitignore                    |   1 +
>  MAINTAINERS                   |   1 +
>  Makefile                      |   2 +
>  configure                     |   2 +-
>  scripts/archive-source.sh     |  31 +++++
>  scripts/qemu.py               |   7 ++
>  tests/.gitignore              |   1 +
>  tests/docker/Makefile.include |  15 +--
>  tests/docker/run              |   8 +-
>  tests/vm/Makefile.include     |  42 +++++++
>  tests/vm/README               |  63 ++++++++++
>  tests/vm/basevm.py            | 276 ++++++++++++++++++++++++++++++++++++++++++
>  tests/vm/freebsd              |  42 +++++++
>  tests/vm/netbsd               |  42 +++++++
>  tests/vm/openbsd              |  43 +++++++
>  tests/vm/ubuntu.i386          |  88 ++++++++++++++
>  16 files changed, 643 insertions(+), 21 deletions(-)
>  create mode 100755 scripts/archive-source.sh
>  create mode 100644 tests/vm/Makefile.include
>  create mode 100644 tests/vm/README
>  create mode 100755 tests/vm/basevm.py
>  create mode 100755 tests/vm/freebsd
>  create mode 100755 tests/vm/netbsd
>  create mode 100755 tests/vm/openbsd
>  create mode 100755 tests/vm/ubuntu.i386


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images
  2017-09-08 14:12   ` Alex Bennée
@ 2017-09-08 23:12     ` Fam Zheng
  0 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-08 23:12 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

On Fri, 09/08 15:12, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  .gitignore       | 1 +
> >  tests/.gitignore | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/.gitignore b/.gitignore
> > index cf65316863..40acfcb9e2 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -52,6 +52,7 @@
> >  /vscclient
> >  /vhost-user-scsi
> >  /fsdev/virtfs-proxy-helper
> > +*.tmp
> 
> I'm not sure we should be dumping anything in the rootdir, .gitignored
> or not.

It used to be /tests/vm/*.tmp but Philippe asked for a generic rule in previous
reviewing. I think it makes sense, we can also unify the docker-src.* pattern by
making the directory names to have a .tmp suffix later.

Fam

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-08 14:56   ` Peter Maydell
@ 2017-09-08 23:13     ` Fam Zheng
  0 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-08 23:13 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Daniel P. Berrange, Alex Bennée,
	Philippe Mathieu-Daudé,
	Paolo Bonzini, Stefan Hajnoczi, Cleber Rosa, Eric Blake,
	Kamil Rytarowski

On Fri, 09/08 15:56, Peter Maydell wrote:
> On 5 September 2017 at 03:11, Fam Zheng <famz@redhat.com> wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100755 scripts/archive-source.sh
> >
> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> > new file mode 100755
> > index 0000000000..3cae7f34d3
> > --- /dev/null
> > +++ b/scripts/archive-source.sh
> > @@ -0,0 +1,31 @@
> > +#!/bin/sh
> > +#
> > +# Author: Fam Zheng <famz@redhat.com>
> > +#
> > +# Create archive of source tree, including submodules
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.
> > +# See the COPYING file in the top-level directory.
> 
> Why GPL-2-only? We generally prefer GPL-2-or-later for new
> files, or something more permissive.

Copied from some other script/* files, apparently. :)

Will make it GPL-2-or-later.

Fam

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-08 14:42   ` Alex Bennée
@ 2017-09-08 23:14     ` Fam Zheng
  2017-09-11 13:43       ` Alex Bennée
  0 siblings, 1 reply; 29+ messages in thread
From: Fam Zheng @ 2017-09-08 23:14 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

On Fri, 09/08 15:42, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100755 scripts/archive-source.sh
> >
> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> > new file mode 100755
> > index 0000000000..3cae7f34d3
> > --- /dev/null
> > +++ b/scripts/archive-source.sh
> > @@ -0,0 +1,31 @@
> > +#!/bin/sh
> > +#
> > +# Author: Fam Zheng <famz@redhat.com>
> > +#
> > +# Create archive of source tree, including submodules
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.
> > +# See the COPYING file in the top-level directory.
> > +
> > +set -e
> > +
> > +if test $# -lt 1; then
> > +    echo "Usage: $0 <output>"
> 
> Maybe <output tarball> to make it clear what it creates?

OK.

> 
> > +    exit 1
> > +fi
> > +
> > +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> > +
> > +if test -n "$submodules"; then
> > +    {
> > +        git ls-files
> 
> Couldn't we do the main git ls-files first and then append the data for
> any submodules?

Isn't that exactly what we are doing now?

Fam

> 
> > +        for sm in $submodules; do
> > +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
> > +        done
> > +    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list
> > +else
> > +    git ls-files > $1.list
> > +fi
> > +
> > +tar -cf $1 -T $1.list
> > +rm $1.list
> 
> 
> --
> Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-08 15:22   ` Alex Bennée
@ 2017-09-08 23:29     ` Fam Zheng
  2017-09-09  1:25       ` Philippe Mathieu-Daudé
  2017-09-11 10:44       ` Alex Bennée
  0 siblings, 2 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-08 23:29 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Peter Maydell, qemu-devel, Philippe Mathieu-Daudé,
	Kamil Rytarowski, stefanha, Cleber Rosa, pbonzini

On Fri, 09/08 16:22, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > This is the common code to implement a "VM test" to
> >
> >   1) Download and initialize a pre-defined VM that has necessary
> >   dependencies to build QEMU and SSH access.
> >
> >   2) Archive $SRC_PATH to a .tar file.
> >
> >   3) Boot the VM, and pass the source tar file to the guest.
> >
> >   4) SSH into the VM, untar the source tarball, build from the source.
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  tests/vm/basevm.py | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 276 insertions(+)
> >  create mode 100755 tests/vm/basevm.py
> >
> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> > new file mode 100755
> > index 0000000000..9db91d61fa
> > --- /dev/null
> > +++ b/tests/vm/basevm.py
> > @@ -0,0 +1,276 @@
> > +#!/usr/bin/env python
> > +#
> > +# VM testing base class
> > +#
> > +# Copyright (C) 2017 Red Hat Inc.
> > +#
> > +# Authors:
> > +#  Fam Zheng <famz@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
> > +# the COPYING file in the top-level directory.
> > +#
> > +
> > +import os
> > +import sys
> > +import logging
> > +import time
> > +import datetime
> > +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
> > +from qemu import QEMUMachine
> > +import subprocess
> > +import hashlib
> > +import optparse
> > +import atexit
> > +import tempfile
> > +import shutil
> > +import multiprocessing
> > +import traceback
> > +
> > +SSH_KEY = """\
> > +-----BEGIN RSA PRIVATE KEY-----
> > +MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
> > +coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
> > ++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
> > +RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
> > +7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
> > +Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
> > +cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
> > +7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
> > +R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
> > +rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
> > +cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
> > +vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
> > +XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
> > +j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
> > +T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
> > +XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
> > +JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
> > +CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
> > +bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
> > +eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
> > +BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
> > +CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
> > +D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
> > +VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
> > +5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
> > +-----END RSA PRIVATE KEY-----
> > +"""
> > +SSH_PUB_KEY = """\
> > +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn qemu-vm-key
> > +"""
> 
> I'm not sure we should be embedding the keys in the script. I understand
> we need a common key for downloaded images (although it would be better
> to post-customise the image after download with the local developer
> keys). Perhaps ./tests/testing-keys/id_rsa[.pub]?

We cannot generate keys or start from local keys because it's hard to inject it
into the *BSD images without ssh access (chicken-and-egg problem). Adding the
local keys might be a good feature, indeed.

> 
> > +
> > +class BaseVM(object):
> > +    GUEST_USER = "qemu"
> > +    GUEST_PASS = "qemupass"
> > +    ROOT_PASS = "qemupass"
> > +
> > +    # The script to run in the guest that builds QEMU
> > +    BUILD_SCRIPT = ""
> > +    # The guest name, to be overridden by subclasses
> > +    name = "#base"
> > +    def __init__(self, debug=False, vcpus=None):
> > +        self._guest = None
> > +        self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", dir=".")
> > +        atexit.register(shutil.rmtree, self._tmpdir)
> > +
> > +        self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
> > +        open(self._ssh_key_file, "w").write(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)
> 
> As above, I think it would be better just to keep copies of the keys in
> the tests directory rather than in the python source.
> 
> > +
> > +        self.debug = debug
> > +        self._stderr = sys.stderr
> > +        self._devnull = open("/dev/null", "w")
> 
> You can use os.devnull as a portable reference.

OK.

> 
> > +        if self.debug:
> > +            self._stdout = sys.stdout
> > +        else:
> > +            self._stdout = self._devnull
> > +        self._args = [ \
> > +            "-nodefaults", "-m", "2G",
> > +            "-cpu", "host",
> > +            "-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
> 
> It doesn't help that howstfwd is poorly documented. Are we trying to map
> an ephemeral port to the guests 22?

Yes, it is a new feature of usernet, you're right the documentation is missing.

> 
> > +            "-device", "virtio-net-pci,netdev=vnet",
> > +            "-vnc", ":0,to=20",
> 
> Do we need a GUI?

I'd like to keep it, it helps when you need to diagnose.

> 
> > +            "-serial", "file:%s" % os.path.join(self._tmpdir,
> > "serial.out")]
> > +        if vcpus:
> > +            self._args += ["-smp", str(vcpus)]
> > +        if os.access("/dev/kvm", os.R_OK | os.W_OK):
> > +            self._args += ["-enable-kvm"]
> > +        else:
> > +            logging.info("KVM not available, not using -enable-kvm")
> > +        self._data_args = []
> > +
> > +    def _download_with_cache(self, url, sha256sum=None):
> > +        def check_sha256sum(fname):
> > +            if not sha256sum:
> > +                return True
> > +            checksum = subprocess.check_output(["sha256sum", fname]).split()[0]
> > +            return sha256sum == checksum
> > +
> > +        cache_dir = os.path.expanduser("~/.cache/qemu-vm/download")
> > +        if not os.path.exists(cache_dir):
> > +            os.makedirs(cache_dir)
> > +        fname = os.path.join(cache_dir, hashlib.sha1(url).hexdigest())
> > +        if os.path.exists(fname) and check_sha256sum(fname):
> > +            return fname
> > +        logging.debug("Downloading %s to %s...", url, fname)
> > +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"],
> > +                              stdout=self._stdout,
> > stderr=self._stderr)
> 
> Using wget rather than doing it internally adds a utility requirement.
> Is doing a pure python urllib2 fetch too slow for this?

I think "-c" is a good feature for downloading big files.

> 
> > +        os.rename(fname + ".download", fname)
> > +        return fname
> > +
> > +    def _ssh_do(self, user, cmd, check, interactive=False):
> > +        ssh_cmd = ["ssh", "-q",
> > +                   "-o", "StrictHostKeyChecking=no",
> > +                   "-o", "UserKnownHostsFile=/dev/null",
> > +                   "-o", "ConnectTimeout=1",
> > +                   "-p", self.ssh_port, "-i", self._ssh_key_file]
> > +        if interactive:
> > +            ssh_cmd += ['-t']
> > +        assert not isinstance(cmd, str)
> > +        ssh_cmd += ["%s@127.0.0.1" % user] + list(cmd)
> > +        logging.debug("ssh_cmd: %s", " ".join(ssh_cmd))
> > +        r = subprocess.call(ssh_cmd,
> > +                            stdin=sys.stdin if interactive else self._devnull,
> > +                            stdout=sys.stdout if interactive else self._stdout,
> > +                            stderr=sys.stderr if interactive else self._stderr)
> > +        if check and r != 0:
> > +            raise Exception("SSH command failed: %s" % cmd)
> > +        return r
> > +
> > +    def ssh(self, *cmd):
> > +        return self._ssh_do(self.GUEST_USER, cmd, False)
> > +
> > +    def ssh_interactive(self, *cmd):
> > +        return self._ssh_do(self.GUEST_USER, cmd, False, True)
> > +
> > +    def ssh_root(self, *cmd):
> > +        return self._ssh_do("root", cmd, False)
> > +
> > +    def ssh_check(self, *cmd):
> > +        self._ssh_do(self.GUEST_USER, cmd, True)
> > +
> > +    def ssh_root_check(self, *cmd):
> > +        self._ssh_do("root", cmd, True)
> > +
> > +    def build_image(self, img):
> > +        raise NotImplementedError
> > +
> > +    def add_source_dir(self, src_dir):
> > +        name = "data-" + hashlib.sha1(src_dir).hexdigest()[:5]
> > +        tarfile = os.path.join(self._tmpdir, name + ".tar")
> > +        logging.debug("Creating archive %s for src_dir dir: %s", tarfile, src_dir)
> > +        subprocess.check_call(["./scripts/archive-source.sh", tarfile],
> > +                              cwd=src_dir, stdin=self._devnull,
> > +                              stdout=self._stdout, stderr=self._stderr)
> > +        self._data_args += ["-drive",
> > +                            "file=%s,if=none,id=%s,cache=writeback,format=raw" % \
> > +                                    (tarfile, name),
> > +                            "-device",
> > +                            "virtio-blk,drive=%s,serial=%s,bootindex=1" % (name, name)]
> > +
> > +    def boot(self, img, extra_args=[]):
> > +        args = self._args + [
> > +            "-device", "VGA",
> 
> Is specifying a display device at boot time really needed (say rather
> than further up with the rest of the machine description). Do we even
> want a display given use ssh/serial for everything?

It makes debug easier, and also I find cloud-init or something in ubuntu
requires a virtual console on the display to make progress.

> 
> > +            "-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
> > +            "-device", "virtio-blk,drive=drive0,bootindex=0"]
> > +        args += self._data_args + extra_args
> > +        logging.debug("QEMU args: %s", " ".join(args))
> > +        guest = QEMUMachine(binary=os.environ.get("QEMU", "qemu-system-x86_64"),
> > +                            args=args)
> > +        guest.launch()
> > +        atexit.register(self.shutdown)
> > +        self._guest = guest
> > +        usernet_info = guest.qmp("human-monitor-command",
> > +                                 command_line="info usernet")
> > +        self.ssh_port = None
> > +        for l in usernet_info["return"].splitlines():
> > +            fields = l.split()
> > +            if "TCP[HOST_FORWARD]" in fields and "22" in fields:
> > +                self.ssh_port = l.split()[3]
> > +        if not self.ssh_port:
> > +            raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \
> > +                            usernet_info)
> > +
> > +    def wait_ssh(self, seconds=120):
> > +        starttime = datetime.datetime.now()
> > +        guest_up = False
> > +        while (datetime.datetime.now() - starttime).total_seconds() < seconds:
> > +            if self.ssh("exit 0") == 0:
> > +                guest_up = True
> > +                break
> > +            time.sleep(1)
> > +        if not guest_up:
> > +            raise Exception("Timeout while waiting for guest ssh")
> > +
> > +    def shutdown(self):
> > +        self._guest.shutdown()
> > +
> > +    def wait(self):
> > +        self._guest.wait()
> > +
> > +    def qmp(self, *args, **kwargs):
> > +        return self._guest.qmp(*args, **kwargs)
> > +
> > +def parse_args(vm_name):
> > +    parser = optparse.OptionParser(description="""
> > +    VM test utility.  Exit codes: 0 = success, 1 = command line error, 2 = environment initialization failed, 3 = test command failed""")
> > +    parser.add_option("--debug", "-D", action="store_true",
> > +                      help="enable debug output")
> > +    parser.add_option("--image", "-i", default="%s.img" % vm_name,
> > +                      help="image file name")
> > +    parser.add_option("--force", "-f", action="store_true",
> > +                      help="force build image even if image exists")
> > +    parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() / 2,
> > +                      help="number of virtual CPUs")
> > +    parser.add_option("--build-image", "-b", action="store_true",
> > +                      help="build image")
> > +    parser.add_option("--build-qemu",
> > +                      help="build QEMU from source in guest")
> > +    parser.add_option("--interactive", "-I", action="store_true",
> > +                      help="Interactively run command")
> > +    parser.disable_interspersed_args()
> > +    return parser.parse_args()
> > +
> > +def main(vmcls):
> > +    try:
> > +        args, argv = parse_args(vmcls.name)
> > +        if not argv and not args.build_qemu and not args.build_image:
> > +            print "Nothing to do?"
> > +            return 1
> > +        if args.debug:
> > +            logging.getLogger().setLevel(logging.DEBUG)
> > +        vm = vmcls(debug=args.debug, vcpus=args.jobs)
> > +        if args.build_image:
> > +            if os.path.exists(args.image) and not args.force:
> > +                sys.stderr.writelines(["Image file exists: %s\n" % args.image,
> > +                                      "Use --force option to overwrite\n"])
> > +                return 1
> > +            return vm.build_image(args.image)
> > +        if args.build_qemu:
> > +            vm.add_source_dir(args.build_qemu)
> > +            cmd = [vm.BUILD_SCRIPT.format(
> > +                   configure_opts = " ".join(argv),
> > +                   jobs=args.jobs)]
> > +        else:
> > +            cmd = argv
> > +        vm.boot(args.image + ",snapshot=on")
> 
> If this fails it's fairly cryptic:
> 
> /home/alex/lsrc/qemu/qemu.git/tests/vm/openbsd  --debug   --image "tests/vm/openbsd.img" --build-qemu /home/alex/lsrc/qemu/qemu.git
> DEBUG:root:Creating archive ./vm-test-fxejnB.tmp/data-2de24.tar for src_dir dir: /home/alex/lsrc/qemu/qemu.git
> DEBUG:root:QEMU args: -nodefaults -m 2G -cpu host -netdev user,id=vnet,hostfwd=:0.0.0.0:0-:22 -device virtio-net-pci,netdev=vnet -vnc :0,to=20 -serial file:./vm-test-fxejnB.tmp/serial.out -smp 4 -enable-kvm -device VGA -drive file=tests/vm/openbsd.img,snapshot=on,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -drive file=./vm-test-fxejnB.tmp/data-2de24.tar,if=none,id=data-2de24,cache=writeback,format=raw -device virtio-blk,drive=data-2de24,serial=data-2de24,bootindex=1
> Failed to prepare guest environment
> Traceback (most recent call last):
>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 260, in main
>     vm.boot(args.image + ",snapshot=on")
>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 184, in boot
>     guest.launch()
>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 151, in launch
>     self._post_launch()
>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 135, in _post_launch
>     self._qmp.accept()
>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 147, in accept
>     return self.__negotiate_capabilities()
>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 60, in __negotiate_capabilities
>     raise QMPConnectError
> QMPConnectError
> 
> I assume QEMU failed to boot and might have given us a message that
> would be useful.

It's a separate issue of qemu.py, I didn't try to fix it here because currently
there are many patches touching that file on the list, there is a high chance of
conflict.

In your case which qemu version are you using? If not master it probably is
failing because of the hostfwd syntax.

> 
> 
> > +        vm.wait_ssh()
> > +    except Exception as e:
> > +        if isinstance(e, SystemExit) and e.code == 0:
> > +            return 0
> > +        sys.stderr.write("Failed to prepare guest environment\n")
> > +        traceback.print_exc()
> > +        return 2
> > +
> > +    if args.interactive:
> > +        if vm.ssh_interactive(*cmd) == 0:
> > +            return 0
> > +        vm.ssh_interactive()
> > +        return 3
> > +    else:
> > +        if vm.ssh(*cmd) != 0:
> > +            return 3
> 
> 
> --
> Alex Bennée
> 

Thanks for the review!

Fam

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-08 23:29     ` Fam Zheng
@ 2017-09-09  1:25       ` Philippe Mathieu-Daudé
  2017-09-09  4:10         ` Fam Zheng
  2017-09-11 10:44       ` Alex Bennée
  1 sibling, 1 reply; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-09-09  1:25 UTC (permalink / raw)
  To: Fam Zheng, Alex Bennée
  Cc: Peter Maydell, qemu-devel, Kamil Rytarowski, stefanha,
	Cleber Rosa, pbonzini

Hi Fam, Alex,

>>
>> If this fails it's fairly cryptic:
>>
>> /home/alex/lsrc/qemu/qemu.git/tests/vm/openbsd  --debug   --image "tests/vm/openbsd.img" --build-qemu /home/alex/lsrc/qemu/qemu.git
>> DEBUG:root:Creating archive ./vm-test-fxejnB.tmp/data-2de24.tar for src_dir dir: /home/alex/lsrc/qemu/qemu.git
>> DEBUG:root:QEMU args: -nodefaults -m 2G -cpu host -netdev user,id=vnet,hostfwd=:0.0.0.0:0-:22 -device virtio-net-pci,netdev=vnet -vnc :0,to=20 -serial file:./vm-test-fxejnB.tmp/serial.out -smp 4 -enable-kvm -device VGA -drive file=tests/vm/openbsd.img,snapshot=on,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -drive file=./vm-test-fxejnB.tmp/data-2de24.tar,if=none,id=data-2de24,cache=writeback,format=raw -device virtio-blk,drive=data-2de24,serial=data-2de24,bootindex=1
>> Failed to prepare guest environment
>> Traceback (most recent call last):
>>    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 260, in main
>>      vm.boot(args.image + ",snapshot=on")
>>    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 184, in boot
>>      guest.launch()
>>    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 151, in launch
>>      self._post_launch()
>>    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 135, in _post_launch
>>      self._qmp.accept()
>>    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 147, in accept
>>      return self.__negotiate_capabilities()
>>    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 60, in __negotiate_capabilities
>>      raise QMPConnectError
>> QMPConnectError
>>
>> I assume QEMU failed to boot and might have given us a message that
>> would be useful.
> 
> It's a separate issue of qemu.py, I didn't try to fix it here because currently
> there are many patches touching that file on the list, there is a high chance of
> conflict.
> 
> In your case which qemu version are you using? If not master it probably is
> failing because of the hostfwd syntax.

very likely.

one simple way to handle this could be having this script catching 
QMPConnectError and restart with older command line format,

but I prefer starting QEMU binding a QMP socket, then initialize the 
options via QMP packets, this should be more portable, this is cleaner.

also I think we don't want to run those images with bleeding edge 
qemu-system, we want a stable QEMU to be able to compile the bleeding 
edge codebase inside a working VM.

another note: currently when a VM fails, it closes and what I've left is 
only the last stderr.
I'd rather see 2 modes:
- run from some CI when only the final return value is useful, the 
stderr being extra value.
- in user-dev mode if the build fails I'd like to have access to the 
terminal, eventually some msg like "this build failed, you can connect 
to this VM copy/pasting the following ssh command: ... or hit ^C to 
gracefully shutdown the VM"
But in this mode I'd rather mount my current workdir via a read-writable 
netshare rather than a one-time temporary read-only tarball, so I could 
modify my repo and just run 'make' inside the VM.

Regards,

Phil.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-09  1:25       ` Philippe Mathieu-Daudé
@ 2017-09-09  4:10         ` Fam Zheng
  0 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-09  4:10 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alex Bennée, Peter Maydell, qemu-devel, Kamil Rytarowski,
	stefanha, Cleber Rosa, pbonzini

On Fri, 09/08 22:25, Philippe Mathieu-Daudé wrote:
> Hi Fam, Alex,
> 
> > > 
> > > If this fails it's fairly cryptic:
> > > 
> > > /home/alex/lsrc/qemu/qemu.git/tests/vm/openbsd  --debug   --image "tests/vm/openbsd.img" --build-qemu /home/alex/lsrc/qemu/qemu.git
> > > DEBUG:root:Creating archive ./vm-test-fxejnB.tmp/data-2de24.tar for src_dir dir: /home/alex/lsrc/qemu/qemu.git
> > > DEBUG:root:QEMU args: -nodefaults -m 2G -cpu host -netdev user,id=vnet,hostfwd=:0.0.0.0:0-:22 -device virtio-net-pci,netdev=vnet -vnc :0,to=20 -serial file:./vm-test-fxejnB.tmp/serial.out -smp 4 -enable-kvm -device VGA -drive file=tests/vm/openbsd.img,snapshot=on,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -drive file=./vm-test-fxejnB.tmp/data-2de24.tar,if=none,id=data-2de24,cache=writeback,format=raw -device virtio-blk,drive=data-2de24,serial=data-2de24,bootindex=1
> > > Failed to prepare guest environment
> > > Traceback (most recent call last):
> > >    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 260, in main
> > >      vm.boot(args.image + ",snapshot=on")
> > >    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 184, in boot
> > >      guest.launch()
> > >    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 151, in launch
> > >      self._post_launch()
> > >    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 135, in _post_launch
> > >      self._qmp.accept()
> > >    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 147, in accept
> > >      return self.__negotiate_capabilities()
> > >    File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 60, in __negotiate_capabilities
> > >      raise QMPConnectError
> > > QMPConnectError
> > > 
> > > I assume QEMU failed to boot and might have given us a message that
> > > would be useful.
> > 
> > It's a separate issue of qemu.py, I didn't try to fix it here because currently
> > there are many patches touching that file on the list, there is a high chance of
> > conflict.
> > 
> > In your case which qemu version are you using? If not master it probably is
> > failing because of the hostfwd syntax.
> 
> very likely.
> 
> one simple way to handle this could be having this script catching
> QMPConnectError and restart with older command line format,

The error reporting definitely needs improving (like saying which version of
QEMU is needed), but I'm not sure about adding fallbacks here and there are
worth the effort, see below.

> 
> but I prefer starting QEMU binding a QMP socket, then initialize the options
> via QMP packets, this should be more portable, this is cleaner.

This is just different, but not necessarily cleaner. When you want to manually
start such a VM, it will be very painful. Nor does "ps | grep | less" etc works.
Command line is simpler and more user friendly in this respect.

> 
> also I think we don't want to run those images with bleeding edge
> qemu-system, we want a stable QEMU to be able to compile the bleeding edge
> codebase inside a working VM.

You are right we are not testing that bleeding edge qemus can run the VM, but we
need the features of the bleeding edge qemu to write simpler test code. I think
this is reasonable for both CI and developers.  So unless there is an
alternative way to do the hostfwd port allocation, I think we can live with this
requirement.

> 
> another note: currently when a VM fails, it closes and what I've left is
> only the last stderr.
> I'd rather see 2 modes:
> - run from some CI when only the final return value is useful, the stderr
> being extra value.
> - in user-dev mode if the build fails I'd like to have access to the
> terminal, eventually some msg like "this build failed, you can connect to
> this VM copy/pasting the following ssh command: ... or hit ^C to gracefully
> shutdown the VM"
> But in this mode I'd rather mount my current workdir via a read-writable
> netshare rather than a one-time temporary read-only tarball, so I could
> modify my repo and just run 'make' inside the VM.

Like the docker tests, this is what DEBUG=1 is for.

Fam

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-08 23:29     ` Fam Zheng
  2017-09-09  1:25       ` Philippe Mathieu-Daudé
@ 2017-09-11 10:44       ` Alex Bennée
  2017-09-12  0:10         ` Fam Zheng
  1 sibling, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2017-09-11 10:44 UTC (permalink / raw)
  To: Fam Zheng
  Cc: Peter Maydell, qemu-devel, Philippe Mathieu-Daudé,
	Kamil Rytarowski, stefanha, Cleber Rosa, pbonzini


Fam Zheng <famz@redhat.com> writes:

> On Fri, 09/08 16:22, Alex Bennée wrote:
>>
>> Fam Zheng <famz@redhat.com> writes:
>>
>> > This is the common code to implement a "VM test" to
>> >
>> >   1) Download and initialize a pre-defined VM that has necessary
>> >   dependencies to build QEMU and SSH access.
>> >
>> >   2) Archive $SRC_PATH to a .tar file.
>> >
>> >   3) Boot the VM, and pass the source tar file to the guest.
>> >
>> >   4) SSH into the VM, untar the source tarball, build from the source.
>> >
>> > Signed-off-by: Fam Zheng <famz@redhat.com>
>> > ---
>> >  tests/vm/basevm.py | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 276 insertions(+)
>> >  create mode 100755 tests/vm/basevm.py
>> >
>> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
>> > new file mode 100755
>> > index 0000000000..9db91d61fa
>> > --- /dev/null
>> > +++ b/tests/vm/basevm.py
>> > @@ -0,0 +1,276 @@
>> > +#!/usr/bin/env python
>> > +#
>> > +# VM testing base class
>> > +#
>> > +# Copyright (C) 2017 Red Hat Inc.
>> > +#
>> > +# Authors:
>> > +#  Fam Zheng <famz@redhat.com>
>> > +#
>> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
>> > +# the COPYING file in the top-level directory.
>> > +#
>> > +
>> > +import os
>> > +import sys
>> > +import logging
>> > +import time
>> > +import datetime
>> > +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
>> > +from qemu import QEMUMachine
>> > +import subprocess
>> > +import hashlib
>> > +import optparse
>> > +import atexit
>> > +import tempfile
>> > +import shutil
>> > +import multiprocessing
>> > +import traceback
>> > +
>> > +SSH_KEY = """\
>> > +-----BEGIN RSA PRIVATE KEY-----
>> > +MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
>> > +coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
>> > ++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
>> > +RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
>> > +7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
>> > +Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
>> > +cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
>> > +7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
>> > +R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
>> > +rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
>> > +cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
>> > +vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
>> > +XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
>> > +j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
>> > +T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
>> > +XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
>> > +JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
>> > +CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
>> > +bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
>> > +eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
>> > +BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
>> > +CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
>> > +D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
>> > +VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
>> > +5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
>> > +-----END RSA PRIVATE KEY-----
>> > +"""
>> > +SSH_PUB_KEY = """\
>> > +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn qemu-vm-key
>> > +"""
>>
>> I'm not sure we should be embedding the keys in the script. I understand
>> we need a common key for downloaded images (although it would be better
>> to post-customise the image after download with the local developer
>> keys). Perhaps ./tests/testing-keys/id_rsa[.pub]?
>
> We cannot generate keys or start from local keys because it's hard to inject it
> into the *BSD images without ssh access (chicken-and-egg problem). Adding the
> local keys might be a good feature, indeed.

Can't libguestfs be used to manipulate the image without booting BSD
itself?

Regardless I still think having the key embedded in the script rather
than in files in the source tree is overly ugly.

>
>>
>> > +
>> > +class BaseVM(object):
>> > +    GUEST_USER = "qemu"
>> > +    GUEST_PASS = "qemupass"
>> > +    ROOT_PASS = "qemupass"
>> > +
>> > +    # The script to run in the guest that builds QEMU
>> > +    BUILD_SCRIPT = ""
>> > +    # The guest name, to be overridden by subclasses
>> > +    name = "#base"
>> > +    def __init__(self, debug=False, vcpus=None):
>> > +        self._guest = None
>> > +        self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", dir=".")
>> > +        atexit.register(shutil.rmtree, self._tmpdir)
>> > +
>> > +        self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
>> > +        open(self._ssh_key_file, "w").write(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)
>>
>> As above, I think it would be better just to keep copies of the keys in
>> the tests directory rather than in the python source.
>>
>> > +
>> > +        self.debug = debug
>> > +        self._stderr = sys.stderr
>> > +        self._devnull = open("/dev/null", "w")
>>
>> You can use os.devnull as a portable reference.
>
> OK.
>
>>
>> > +        if self.debug:
>> > +            self._stdout = sys.stdout
>> > +        else:
>> > +            self._stdout = self._devnull
>> > +        self._args = [ \
>> > +            "-nodefaults", "-m", "2G",
>> > +            "-cpu", "host",
>> > +            "-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
>>
>> It doesn't help that howstfwd is poorly documented. Are we trying to map
>> an ephemeral port to the guests 22?
>
> Yes, it is a new feature of usernet, you're right the documentation is missing.
>
>>
>> > +            "-device", "virtio-net-pci,netdev=vnet",
>> > +            "-vnc", ":0,to=20",
>>
>> Do we need a GUI?
>
> I'd like to keep it, it helps when you need to diagnose.

As long it runs or fails cleanly when run on a on terminal.

>
>>
>> > +            "-serial", "file:%s" % os.path.join(self._tmpdir,
>> > "serial.out")]
>> > +        if vcpus:
>> > +            self._args += ["-smp", str(vcpus)]
>> > +        if os.access("/dev/kvm", os.R_OK | os.W_OK):
>> > +            self._args += ["-enable-kvm"]
>> > +        else:
>> > +            logging.info("KVM not available, not using -enable-kvm")
>> > +        self._data_args = []
>> > +
>> > +    def _download_with_cache(self, url, sha256sum=None):
>> > +        def check_sha256sum(fname):
>> > +            if not sha256sum:
>> > +                return True
>> > +            checksum = subprocess.check_output(["sha256sum", fname]).split()[0]
>> > +            return sha256sum == checksum
>> > +
>> > +        cache_dir = os.path.expanduser("~/.cache/qemu-vm/download")
>> > +        if not os.path.exists(cache_dir):
>> > +            os.makedirs(cache_dir)
>> > +        fname = os.path.join(cache_dir, hashlib.sha1(url).hexdigest())
>> > +        if os.path.exists(fname) and check_sha256sum(fname):
>> > +            return fname
>> > +        logging.debug("Downloading %s to %s...", url, fname)
>> > +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"],
>> > +                              stdout=self._stdout,
>> > stderr=self._stderr)
>>
>> Using wget rather than doing it internally adds a utility requirement.
>> Is doing a pure python urllib2 fetch too slow for this?
>
> I think "-c" is a good feature for downloading big files.

I guess so.

>
>>
>> > +        os.rename(fname + ".download", fname)
>> > +        return fname
>> > +
>> > +    def _ssh_do(self, user, cmd, check, interactive=False):
>> > +        ssh_cmd = ["ssh", "-q",
>> > +                   "-o", "StrictHostKeyChecking=no",
>> > +                   "-o", "UserKnownHostsFile=/dev/null",
>> > +                   "-o", "ConnectTimeout=1",
>> > +                   "-p", self.ssh_port, "-i", self._ssh_key_file]
>> > +        if interactive:
>> > +            ssh_cmd += ['-t']
>> > +        assert not isinstance(cmd, str)
>> > +        ssh_cmd += ["%s@127.0.0.1" % user] + list(cmd)
>> > +        logging.debug("ssh_cmd: %s", " ".join(ssh_cmd))
>> > +        r = subprocess.call(ssh_cmd,
>> > +                            stdin=sys.stdin if interactive else self._devnull,
>> > +                            stdout=sys.stdout if interactive else self._stdout,
>> > +                            stderr=sys.stderr if interactive else self._stderr)
>> > +        if check and r != 0:
>> > +            raise Exception("SSH command failed: %s" % cmd)
>> > +        return r
>> > +
>> > +    def ssh(self, *cmd):
>> > +        return self._ssh_do(self.GUEST_USER, cmd, False)
>> > +
>> > +    def ssh_interactive(self, *cmd):
>> > +        return self._ssh_do(self.GUEST_USER, cmd, False, True)
>> > +
>> > +    def ssh_root(self, *cmd):
>> > +        return self._ssh_do("root", cmd, False)
>> > +
>> > +    def ssh_check(self, *cmd):
>> > +        self._ssh_do(self.GUEST_USER, cmd, True)
>> > +
>> > +    def ssh_root_check(self, *cmd):
>> > +        self._ssh_do("root", cmd, True)
>> > +
>> > +    def build_image(self, img):
>> > +        raise NotImplementedError
>> > +
>> > +    def add_source_dir(self, src_dir):
>> > +        name = "data-" + hashlib.sha1(src_dir).hexdigest()[:5]
>> > +        tarfile = os.path.join(self._tmpdir, name + ".tar")
>> > +        logging.debug("Creating archive %s for src_dir dir: %s", tarfile, src_dir)
>> > +        subprocess.check_call(["./scripts/archive-source.sh", tarfile],
>> > +                              cwd=src_dir, stdin=self._devnull,
>> > +                              stdout=self._stdout, stderr=self._stderr)
>> > +        self._data_args += ["-drive",
>> > +                            "file=%s,if=none,id=%s,cache=writeback,format=raw" % \
>> > +                                    (tarfile, name),
>> > +                            "-device",
>> > +                            "virtio-blk,drive=%s,serial=%s,bootindex=1" % (name, name)]
>> > +
>> > +    def boot(self, img, extra_args=[]):
>> > +        args = self._args + [
>> > +            "-device", "VGA",
>>
>> Is specifying a display device at boot time really needed (say rather
>> than further up with the rest of the machine description). Do we even
>> want a display given use ssh/serial for everything?
>
> It makes debug easier, and also I find cloud-init or something in ubuntu
> requires a virtual console on the display to make progress.
>
>>
>> > +            "-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
>> > +            "-device", "virtio-blk,drive=drive0,bootindex=0"]
>> > +        args += self._data_args + extra_args
>> > +        logging.debug("QEMU args: %s", " ".join(args))
>> > +        guest = QEMUMachine(binary=os.environ.get("QEMU", "qemu-system-x86_64"),
>> > +                            args=args)
>> > +        guest.launch()
>> > +        atexit.register(self.shutdown)
>> > +        self._guest = guest
>> > +        usernet_info = guest.qmp("human-monitor-command",
>> > +                                 command_line="info usernet")
>> > +        self.ssh_port = None
>> > +        for l in usernet_info["return"].splitlines():
>> > +            fields = l.split()
>> > +            if "TCP[HOST_FORWARD]" in fields and "22" in fields:
>> > +                self.ssh_port = l.split()[3]
>> > +        if not self.ssh_port:
>> > +            raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \
>> > +                            usernet_info)
>> > +
>> > +    def wait_ssh(self, seconds=120):
>> > +        starttime = datetime.datetime.now()
>> > +        guest_up = False
>> > +        while (datetime.datetime.now() - starttime).total_seconds() < seconds:
>> > +            if self.ssh("exit 0") == 0:
>> > +                guest_up = True
>> > +                break
>> > +            time.sleep(1)
>> > +        if not guest_up:
>> > +            raise Exception("Timeout while waiting for guest ssh")
>> > +
>> > +    def shutdown(self):
>> > +        self._guest.shutdown()
>> > +
>> > +    def wait(self):
>> > +        self._guest.wait()
>> > +
>> > +    def qmp(self, *args, **kwargs):
>> > +        return self._guest.qmp(*args, **kwargs)
>> > +
>> > +def parse_args(vm_name):
>> > +    parser = optparse.OptionParser(description="""
>> > +    VM test utility.  Exit codes: 0 = success, 1 = command line error, 2 = environment initialization failed, 3 = test command failed""")
>> > +    parser.add_option("--debug", "-D", action="store_true",
>> > +                      help="enable debug output")
>> > +    parser.add_option("--image", "-i", default="%s.img" % vm_name,
>> > +                      help="image file name")
>> > +    parser.add_option("--force", "-f", action="store_true",
>> > +                      help="force build image even if image exists")
>> > +    parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() / 2,
>> > +                      help="number of virtual CPUs")
>> > +    parser.add_option("--build-image", "-b", action="store_true",
>> > +                      help="build image")
>> > +    parser.add_option("--build-qemu",
>> > +                      help="build QEMU from source in guest")
>> > +    parser.add_option("--interactive", "-I", action="store_true",
>> > +                      help="Interactively run command")
>> > +    parser.disable_interspersed_args()
>> > +    return parser.parse_args()
>> > +
>> > +def main(vmcls):
>> > +    try:
>> > +        args, argv = parse_args(vmcls.name)
>> > +        if not argv and not args.build_qemu and not args.build_image:
>> > +            print "Nothing to do?"
>> > +            return 1
>> > +        if args.debug:
>> > +            logging.getLogger().setLevel(logging.DEBUG)
>> > +        vm = vmcls(debug=args.debug, vcpus=args.jobs)
>> > +        if args.build_image:
>> > +            if os.path.exists(args.image) and not args.force:
>> > +                sys.stderr.writelines(["Image file exists: %s\n" % args.image,
>> > +                                      "Use --force option to overwrite\n"])
>> > +                return 1
>> > +            return vm.build_image(args.image)
>> > +        if args.build_qemu:
>> > +            vm.add_source_dir(args.build_qemu)
>> > +            cmd = [vm.BUILD_SCRIPT.format(
>> > +                   configure_opts = " ".join(argv),
>> > +                   jobs=args.jobs)]
>> > +        else:
>> > +            cmd = argv
>> > +        vm.boot(args.image + ",snapshot=on")
>>
>> If this fails it's fairly cryptic:
>>
>> /home/alex/lsrc/qemu/qemu.git/tests/vm/openbsd  --debug   --image "tests/vm/openbsd.img" --build-qemu /home/alex/lsrc/qemu/qemu.git
>> DEBUG:root:Creating archive ./vm-test-fxejnB.tmp/data-2de24.tar for src_dir dir: /home/alex/lsrc/qemu/qemu.git
>> DEBUG:root:QEMU args: -nodefaults -m 2G -cpu host -netdev user,id=vnet,hostfwd=:0.0.0.0:0-:22 -device virtio-net-pci,netdev=vnet -vnc :0,to=20 -serial file:./vm-test-fxejnB.tmp/serial.out -smp 4 -enable-kvm -device VGA -drive file=tests/vm/openbsd.img,snapshot=on,if=none,id=drive0,cache=writeback -device virtio-blk,drive=drive0,bootindex=0 -drive file=./vm-test-fxejnB.tmp/data-2de24.tar,if=none,id=data-2de24,cache=writeback,format=raw -device virtio-blk,drive=data-2de24,serial=data-2de24,bootindex=1
>> Failed to prepare guest environment
>> Traceback (most recent call last):
>>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 260, in main
>>     vm.boot(args.image + ",snapshot=on")
>>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/basevm.py", line 184, in boot
>>     guest.launch()
>>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 151, in launch
>>     self._post_launch()
>>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qemu.py", line 135, in _post_launch
>>     self._qmp.accept()
>>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 147, in accept
>>     return self.__negotiate_capabilities()
>>   File "/home/alex/lsrc/qemu/qemu.git/tests/vm/../../scripts/qmp/qmp.py", line 60, in __negotiate_capabilities
>>     raise QMPConnectError
>> QMPConnectError
>>
>> I assume QEMU failed to boot and might have given us a message that
>> would be useful.
>
> It's a separate issue of qemu.py, I didn't try to fix it here because currently
> there are many patches touching that file on the list, there is a high chance of
> conflict.
>
> In your case which qemu version are you using? If not master it probably is
> failing because of the hostfwd syntax.

Nope, built on master (fcea73709b966a7ded9efa7b106ea50c7fe9025c). As I
say I need a cleaner error message to diagnose why it failed.

>
>>
>>
>> > +        vm.wait_ssh()
>> > +    except Exception as e:
>> > +        if isinstance(e, SystemExit) and e.code == 0:
>> > +            return 0
>> > +        sys.stderr.write("Failed to prepare guest environment\n")
>> > +        traceback.print_exc()
>> > +        return 2
>> > +
>> > +    if args.interactive:
>> > +        if vm.ssh_interactive(*cmd) == 0:
>> > +            return 0
>> > +        vm.ssh_interactive()
>> > +        return 3
>> > +    else:
>> > +        if vm.ssh(*cmd) != 0:
>> > +            return 3
>>
>>
>> --
>> Alex Bennée
>>
>
> Thanks for the review!
>
> Fam


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-08 23:14     ` Fam Zheng
@ 2017-09-11 13:43       ` Alex Bennée
  2017-09-12  0:06         ` Fam Zheng
  0 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2017-09-11 13:43 UTC (permalink / raw)
  To: Fam Zheng
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski


Fam Zheng <famz@redhat.com> writes:

> On Fri, 09/08 15:42, Alex Bennée wrote:
>>
>> Fam Zheng <famz@redhat.com> writes:
>>
>> > Signed-off-by: Fam Zheng <famz@redhat.com>
>> > ---
>> >  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
>> >  1 file changed, 31 insertions(+)
>> >  create mode 100755 scripts/archive-source.sh
>> >
>> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
>> > new file mode 100755
>> > index 0000000000..3cae7f34d3
>> > --- /dev/null
>> > +++ b/scripts/archive-source.sh
>> > @@ -0,0 +1,31 @@
>> > +#!/bin/sh
>> > +#
>> > +# Author: Fam Zheng <famz@redhat.com>
>> > +#
>> > +# Create archive of source tree, including submodules
>> > +#
>> > +# This work is licensed under the terms of the GNU GPL, version 2.
>> > +# See the COPYING file in the top-level directory.
>> > +
>> > +set -e
>> > +
>> > +if test $# -lt 1; then
>> > +    echo "Usage: $0 <output>"
>>
>> Maybe <output tarball> to make it clear what it creates?
>
> OK.
>
>>
>> > +    exit 1
>> > +fi
>> > +
>> > +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
>> > +
>> > +if test -n "$submodules"; then
>> > +    {
>> > +        git ls-files
>>
>> Couldn't we do the main git ls-files first and then append the data for
>> any submodules?
>
> Isn't that exactly what we are doing now?

I mean hoist the git ls-files out of the if so we can avoid repeating
with an else leg. e.g.

  git ls-files > $1.list
  if test -n "$submodules"; then
  {
    .. the rest..
  } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) >> $1.list

>
> Fam
>
>>
>> > +        for sm in $submodules; do
>> > +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
>> > +        done
>> > +    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list
>> > +else
>> > +    git ls-files > $1.list
>> > +fi
>> > +
>> > +tar -cf $1 -T $1.list
>> > +rm $1.list
>>
>>
>> --
>> Alex Bennée


--
Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh
  2017-09-11 13:43       ` Alex Bennée
@ 2017-09-12  0:06         ` Fam Zheng
  0 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-12  0:06 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, berrange, Philippe Mathieu-Daudé,
	pbonzini, stefanha, Cleber Rosa, Peter Maydell, eblake,
	Kamil Rytarowski

On Mon, 09/11 14:43, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > On Fri, 09/08 15:42, Alex Bennée wrote:
> >>
> >> Fam Zheng <famz@redhat.com> writes:
> >>
> >> > Signed-off-by: Fam Zheng <famz@redhat.com>
> >> > ---
> >> >  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
> >> >  1 file changed, 31 insertions(+)
> >> >  create mode 100755 scripts/archive-source.sh
> >> >
> >> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> >> > new file mode 100755
> >> > index 0000000000..3cae7f34d3
> >> > --- /dev/null
> >> > +++ b/scripts/archive-source.sh
> >> > @@ -0,0 +1,31 @@
> >> > +#!/bin/sh
> >> > +#
> >> > +# Author: Fam Zheng <famz@redhat.com>
> >> > +#
> >> > +# Create archive of source tree, including submodules
> >> > +#
> >> > +# This work is licensed under the terms of the GNU GPL, version 2.
> >> > +# See the COPYING file in the top-level directory.
> >> > +
> >> > +set -e
> >> > +
> >> > +if test $# -lt 1; then
> >> > +    echo "Usage: $0 <output>"
> >>
> >> Maybe <output tarball> to make it clear what it creates?
> >
> > OK.
> >
> >>
> >> > +    exit 1
> >> > +fi
> >> > +
> >> > +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> >> > +
> >> > +if test -n "$submodules"; then
> >> > +    {
> >> > +        git ls-files
> >>
> >> Couldn't we do the main git ls-files first and then append the data for
> >> any submodules?
> >
> > Isn't that exactly what we are doing now?
> 
> I mean hoist the git ls-files out of the if so we can avoid repeating
> with an else leg. e.g.
> 
>   git ls-files > $1.list

The output of top "git ls-files" has to be filtered by grep if the submodules
list is non-empty, so we cannot save LoC by hoisting.

Fam

>   if test -n "$submodules"; then
>   {
>     .. the rest..
>   } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) >> $1.list
> 
> >
> > Fam
> >
> >>
> >> > +        for sm in $submodules; do
> >> > +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
> >> > +        done
> >> > +    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list
> >> > +else
> >> > +    git ls-files > $1.list
> >> > +fi
> >> > +
> >> > +tar -cf $1 -T $1.list
> >> > +rm $1.list
> >>
> >>
> >> --
> >> Alex Bennée
> 
> 
> --
> Alex Bennée

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib
  2017-09-11 10:44       ` Alex Bennée
@ 2017-09-12  0:10         ` Fam Zheng
  0 siblings, 0 replies; 29+ messages in thread
From: Fam Zheng @ 2017-09-12  0:10 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Peter Maydell, qemu-devel, Philippe Mathieu-Daudé,
	Kamil Rytarowski, stefanha, Cleber Rosa, pbonzini

On Mon, 09/11 11:44, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > On Fri, 09/08 16:22, Alex Bennée wrote:
> >>
> >> Fam Zheng <famz@redhat.com> writes:
> >>
> >> > This is the common code to implement a "VM test" to
> >> >
> >> >   1) Download and initialize a pre-defined VM that has necessary
> >> >   dependencies to build QEMU and SSH access.
> >> >
> >> >   2) Archive $SRC_PATH to a .tar file.
> >> >
> >> >   3) Boot the VM, and pass the source tar file to the guest.
> >> >
> >> >   4) SSH into the VM, untar the source tarball, build from the source.
> >> >
> >> > Signed-off-by: Fam Zheng <famz@redhat.com>
> >> > ---
> >> >  tests/vm/basevm.py | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 276 insertions(+)
> >> >  create mode 100755 tests/vm/basevm.py
> >> >
> >> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> >> > new file mode 100755
> >> > index 0000000000..9db91d61fa
> >> > --- /dev/null
> >> > +++ b/tests/vm/basevm.py
> >> > @@ -0,0 +1,276 @@
> >> > +#!/usr/bin/env python
> >> > +#
> >> > +# VM testing base class
> >> > +#
> >> > +# Copyright (C) 2017 Red Hat Inc.
> >> > +#
> >> > +# Authors:
> >> > +#  Fam Zheng <famz@redhat.com>
> >> > +#
> >> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
> >> > +# the COPYING file in the top-level directory.
> >> > +#
> >> > +
> >> > +import os
> >> > +import sys
> >> > +import logging
> >> > +import time
> >> > +import datetime
> >> > +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
> >> > +from qemu import QEMUMachine
> >> > +import subprocess
> >> > +import hashlib
> >> > +import optparse
> >> > +import atexit
> >> > +import tempfile
> >> > +import shutil
> >> > +import multiprocessing
> >> > +import traceback
> >> > +
> >> > +SSH_KEY = """\
> >> > +-----BEGIN RSA PRIVATE KEY-----
> >> > +MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
> >> > +coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
> >> > ++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
> >> > +RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
> >> > +7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
> >> > +Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
> >> > +cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
> >> > +7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
> >> > +R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
> >> > +rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
> >> > +cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
> >> > +vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
> >> > +XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
> >> > +j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
> >> > +T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
> >> > +XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
> >> > +JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
> >> > +CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
> >> > +bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
> >> > +eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
> >> > +BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
> >> > +CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
> >> > +D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
> >> > +VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
> >> > +5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
> >> > +-----END RSA PRIVATE KEY-----
> >> > +"""
> >> > +SSH_PUB_KEY = """\
> >> > +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn qemu-vm-key
> >> > +"""
> >>
> >> I'm not sure we should be embedding the keys in the script. I understand
> >> we need a common key for downloaded images (although it would be better
> >> to post-customise the image after download with the local developer
> >> keys). Perhaps ./tests/testing-keys/id_rsa[.pub]?
> >
> > We cannot generate keys or start from local keys because it's hard to inject it
> > into the *BSD images without ssh access (chicken-and-egg problem). Adding the
> > local keys might be a good feature, indeed.
> 
> Can't libguestfs be used to manipulate the image without booting BSD
> itself?

No, not working for BSD.

> 
> Regardless I still think having the key embedded in the script rather
> than in files in the source tree is overly ugly.

OK, I can move it out.

> >>
> >> > +            "-device", "virtio-net-pci,netdev=vnet",
> >> > +            "-vnc", ":0,to=20",
> >>
> >> Do we need a GUI?
> >
> > I'd like to keep it, it helps when you need to diagnose.
> 
> As long it runs or fails cleanly when run on a on terminal.

I believe so.

> > In your case which qemu version are you using? If not master it probably is
> > failing because of the hostfwd syntax.
> 
> Nope, built on master (fcea73709b966a7ded9efa7b106ea50c7fe9025c). As I
> say I need a cleaner error message to diagnose why it failed.

I mean the QEMU binary this script finds in $PATH: it invokes
"qemu-system-x86_64" command, not the built QEMU. This is overrideable by
setting QEMU= env.

Anyway I'll try to improve the error message.

Fam

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2017-09-12  0:10 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05  2:11 [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 01/12] gitignore: Ignore vm test images Fam Zheng
2017-09-08 14:12   ` Alex Bennée
2017-09-08 23:12     ` Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 02/12] qemu.py: Add "wait()" method Fam Zheng
2017-09-08 14:13   ` Alex Bennée
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh Fam Zheng
2017-09-08 14:42   ` Alex Bennée
2017-09-08 23:14     ` Fam Zheng
2017-09-11 13:43       ` Alex Bennée
2017-09-12  0:06         ` Fam Zheng
2017-09-08 14:56   ` Peter Maydell
2017-09-08 23:13     ` Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib Fam Zheng
2017-09-08 15:22   ` Alex Bennée
2017-09-08 23:29     ` Fam Zheng
2017-09-09  1:25       ` Philippe Mathieu-Daudé
2017-09-09  4:10         ` Fam Zheng
2017-09-11 10:44       ` Alex Bennée
2017-09-12  0:10         ` Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 05/12] tests: Add ubuntu.i386 image Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 06/12] tests: Add FreeBSD image Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 07/12] tests: Add NetBSD image Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 08/12] tests: Add OpenBSD image Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 09/12] Makefile: Add rules to run vm tests Fam Zheng
2017-09-05  2:11 ` [Qemu-devel] [PATCH v6 10/12] MAINTAINERS: Add tests/vm entry Fam Zheng
2017-09-05  2:12 ` [Qemu-devel] [PATCH v6 11/12] tests: Add README for vm tests Fam Zheng
2017-09-05  2:12 ` [Qemu-devel] [PATCH v6 12/12] docker: Use archive-source.py Fam Zheng
2017-09-08 16:20 ` [Qemu-devel] [PATCH v6 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux) Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.