[Qemu-devel] [PATCH 0/4] tests/vm: various trivial fixes

[Qemu-devel] [PATCH 0/4] tests/vm: various trivial fixes

[Philippe Mathieu-Daudé]

Hi,

These are various fixes I added to have the VM tests working for me.

Regards,

Phil.

Philippe Mathieu-Daudé (4):
  tests/vm: Support proxy / corporate firewall
  tests/vm: Add a dependency to qemu-img
  tests/vm: Only use -cpu 'host' if KVM is available
  tests/vm: Add flex and bison to the vm image

 tests/vm/Makefile.include | 3 +++
 tests/vm/basevm.py        | 3 ++-
 tests/vm/ubuntu.i386      | 6 +++++-
 3 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH 1/4] tests/vm: Support proxy / corporate firewall

[Philippe Mathieu-Daudé]

If ftp_proxy/http_proxy/https_proxy standard environment variables
are available, pass them to the vm images.

As per 06cc3551714:
This is required when building behind corporate proxy/firewall, but
also help when using local cache server (ie: apt/yum).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/vm/ubuntu.i386 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index fc319e0e6e..be16ceed50 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -68,6 +68,10 @@ class UbuntuX86VM(basevm.BaseVM):
         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")
+        for k, v in os.environ.iteritems():
+            kl = k.lower()
+            if kl in ['ftp_proxy', 'http_proxy', 'https_proxy']:
+                self.ssh_root_check("echo 'Acquire::{}::Proxy \"{}\";' >> /etc/apt/apt.conf.d/01proxy".format(kl[:-6].upper(), v))
         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
-- 
2.18.0

[Qemu-devel] [PATCH 2/4] tests/vm: Add a dependency to qemu-img

[Philippe Mathieu-Daudé]

Before the first use, the VM image are resized with qemu-img.

Add a dependency to the qemu-img tool to fix:

  $ make vm-build-ubuntu.i386
  Traceback (most recent call last):
    File "source/qemu/tests/vm/basevm.py", line 236, in main
      return vm.build_image(args.image)
    File "tests/vm/ubuntu.i386", line 67, in build_image
      subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
  OSError: [Errno 2] No such file or directory
  tests/vm/Makefile.include:23: recipe for target 'tests/vm/ubuntu.i386.img' failed
  make: *** [tests/vm/ubuntu.i386.img] Error 2

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/vm/Makefile.include | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 5daa2a3b73..e5e8146267 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -17,10 +17,13 @@ vm-test:
 
 vm-build-all: $(addprefix vm-build-, $(IMAGES))
 
+$(IMAGE_FILES): qemu-img$(EXESUF)
+
 tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
 		$(SRC_PATH)/tests/vm/basevm.py \
 		$(SRC_PATH)/tests/vm/Makefile.include
 	$(call quiet-command, \
+		PATH=$(PATH):. \
 		$< \
 		$(if $(V)$(DEBUG), --debug) \
 		--image "$@" \
-- 
2.18.0

[Qemu-devel] [PATCH 3/4] tests/vm: Only use -cpu 'host' if KVM is available

[Philippe Mathieu-Daudé]

If KVM is not available, then use the 'max' cpu.

This fixes:

  ERROR:root:Log:
  ERROR:root:qemu-system-x86_64: CPU model 'host' requires KVM
  Failed to prepare guest environment
  error: [Errno 104] Connection reset by peer
  source/qemu/tests/vm/Makefile.include:25: recipe for target 'tests/vm/ubuntu.i386.img' failed
  make: *** [tests/vm/ubuntu.i386.img] Error 2

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/vm/basevm.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 3643117816..d80cc8ab85 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -65,7 +65,6 @@ class BaseVM(object):
             self._stdout = self._devnull
         self._args = [ \
             "-nodefaults", "-m", "2G",
-            "-cpu", "host",
             "-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22",
             "-device", "virtio-net-pci,netdev=vnet",
             "-vnc", "127.0.0.1:0,to=20",
@@ -73,9 +72,11 @@ class BaseVM(object):
         if vcpus:
             self._args += ["-smp", str(vcpus)]
         if os.access("/dev/kvm", os.R_OK | os.W_OK):
+            self._args += ["-cpu", "host"]
             self._args += ["-enable-kvm"]
         else:
             logging.info("KVM not available, not using -enable-kvm")
+            self._args += ["-cpu", "max"]
         self._data_args = []
 
     def _download_with_cache(self, url, sha256sum=None):
-- 
2.18.0

[Qemu-devel] [PATCH 4/4] tests/vm: Add flex and bison to the vm image

[Philippe Mathieu-Daudé]

Similar to 79f24568e5e70, this fixes the following warnings:

           CHK version_gen.h
           LEX convert-dtsv0-lexer.lex.c
  make[1]: flex: Command not found
           BISON dtc-parser.tab.c
  make[1]: bison: Command not found
           LEX dtc-lexer.lex.c
  make[1]: flex: Command not found

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/vm/ubuntu.i386 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index be16ceed50..88b661b6d8 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -81,7 +81,7 @@ class UbuntuX86VM(basevm.BaseVM):
         # 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_check("apt-get install -y libfdt-dev flex bison")
         self.ssh_root("poweroff")
         self.wait()
         if os.path.exists(img):
-- 
2.18.0

Re: [Qemu-devel] [PATCH 2/4] tests/vm: Add a dependency to qemu-img

[Philippe Mathieu-Daudé]

On 06/28/2018 12:35 PM, Philippe Mathieu-Daudé wrote:
> Before the first use, the VM image are resized with qemu-img.
> 
> Add a dependency to the qemu-img tool to fix:
> 
>   $ make vm-build-ubuntu.i386
>   Traceback (most recent call last):
>     File "source/qemu/tests/vm/basevm.py", line 236, in main
>       return vm.build_image(args.image)
>     File "tests/vm/ubuntu.i386", line 67, in build_image
>       subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
>   OSError: [Errno 2] No such file or directory
>   tests/vm/Makefile.include:23: recipe for target 'tests/vm/ubuntu.i386.img' failed
>   make: *** [tests/vm/ubuntu.i386.img] Error 2
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/vm/Makefile.include | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index 5daa2a3b73..e5e8146267 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -17,10 +17,13 @@ vm-test:
>  
>  vm-build-all: $(addprefix vm-build-, $(IMAGES))
>  
> +$(IMAGE_FILES): qemu-img$(EXESUF)
> +
>  tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
>  		$(SRC_PATH)/tests/vm/basevm.py \
>  		$(SRC_PATH)/tests/vm/Makefile.include
>  	$(call quiet-command, \
> +		PATH=$(PATH):. \

Oh since we want to use the shell PATH and not a make variable, it could
be cleaner to rather use:

+		PATH=$$PATH:. \

>  		$< \
>  		$(if $(V)$(DEBUG), --debug) \
>  		--image "$@" \
> 

Re: [Qemu-devel] [PATCH 1/4] tests/vm: Support proxy / corporate firewall

[Fam Zheng]

On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
> If ftp_proxy/http_proxy/https_proxy standard environment variables
> are available, pass them to the vm images.
> 
> As per 06cc3551714:
> This is required when building behind corporate proxy/firewall, but
> also help when using local cache server (ie: apt/yum).
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/vm/ubuntu.i386 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
> index fc319e0e6e..be16ceed50 100755
> --- a/tests/vm/ubuntu.i386
> +++ b/tests/vm/ubuntu.i386
> @@ -68,6 +68,10 @@ class UbuntuX86VM(basevm.BaseVM):
>          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")
> +        for k, v in os.environ.iteritems():
> +            kl = k.lower()
> +            if kl in ['ftp_proxy', 'http_proxy', 'https_proxy']:
> +                self.ssh_root_check("echo 'Acquire::{}::Proxy \"{}\";' >> /etc/apt/apt.conf.d/01proxy".format(kl[:-6].upper(), v))

Reasonable, but do we want it for other apps and images? How about setting these
env vars to ssh commands?

Fam

>          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
> -- 
> 2.18.0
> 

Re: [Qemu-devel] [PATCH 2/4] tests/vm: Add a dependency to qemu-img

[Fam Zheng]

On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
> Before the first use, the VM image are resized with qemu-img.
> 
> Add a dependency to the qemu-img tool to fix:
> 
>   $ make vm-build-ubuntu.i386
>   Traceback (most recent call last):
>     File "source/qemu/tests/vm/basevm.py", line 236, in main
>       return vm.build_image(args.image)
>     File "tests/vm/ubuntu.i386", line 67, in build_image
>       subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
>   OSError: [Errno 2] No such file or directory
>   tests/vm/Makefile.include:23: recipe for target 'tests/vm/ubuntu.i386.img' failed
>   make: *** [tests/vm/ubuntu.i386.img] Error 2
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/vm/Makefile.include | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index 5daa2a3b73..e5e8146267 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -17,10 +17,13 @@ vm-test:
>  
>  vm-build-all: $(addprefix vm-build-, $(IMAGES))
>  
> +$(IMAGE_FILES): qemu-img$(EXESUF)

No, please don't do this. Let's keep on supporting vm tests from a clean source
tree. Similar to qemu_bin, this can be handled in basevm.py.

Fam

> +
>  tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
>  		$(SRC_PATH)/tests/vm/basevm.py \
>  		$(SRC_PATH)/tests/vm/Makefile.include
>  	$(call quiet-command, \
> +		PATH=$(PATH):. \
>  		$< \
>  		$(if $(V)$(DEBUG), --debug) \
>  		--image "$@" \
> -- 
> 2.18.0
> 

Re: [Qemu-devel] [PATCH 0/4] tests/vm: various trivial fixes

[Fam Zheng]

On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> These are various fixes I added to have the VM tests working for me.

Queued the last two patches. Thanks.

Fam

Re: [Qemu-devel] [PATCH 1/4] tests/vm: Support proxy / corporate firewall

[Philippe Mathieu-Daudé]

Hi Fam,

On 07/02/2018 04:12 AM, Fam Zheng wrote:
> On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
>> If ftp_proxy/http_proxy/https_proxy standard environment variables
>> are available, pass them to the vm images.
>>
>> As per 06cc3551714:
>> This is required when building behind corporate proxy/firewall, but
>> also help when using local cache server (ie: apt/yum).
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/vm/ubuntu.i386 | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
>> index fc319e0e6e..be16ceed50 100755
>> --- a/tests/vm/ubuntu.i386
>> +++ b/tests/vm/ubuntu.i386
>> @@ -68,6 +68,10 @@ class UbuntuX86VM(basevm.BaseVM):
>>          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")
>> +        for k, v in os.environ.iteritems():
>> +            kl = k.lower()
>> +            if kl in ['ftp_proxy', 'http_proxy', 'https_proxy']:
>> +                self.ssh_root_check("echo 'Acquire::{}::Proxy \"{}\";' >> /etc/apt/apt.conf.d/01proxy".format(kl[:-6].upper(), v))
> 
> Reasonable, but do we want it for other apps and images? How about setting these
> env vars to ssh commands?

I see 2 different network usages:

1/ how the guest connect to the outer world, this goes via the firewall.
Here the change only affects apt* based commands (via the apt.conf file).
Do we have other commands requiring network connectivity? If we have,
then yes, we should add the same env vars in the guest.
One case I think of is "git submodule init" calling "git clone".

2/ how the host connect to the guest (ex: SSH).
In this case we use the loopback interface, thus no need to change.

>>          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
>> -- 
>> 2.18.0
>>

Re: [Qemu-devel] [PATCH 2/4] tests/vm: Add a dependency to qemu-img

[Philippe Mathieu-Daudé]

On 07/02/2018 04:18 AM, Fam Zheng wrote:
> On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
>> Before the first use, the VM image are resized with qemu-img.
>>
>> Add a dependency to the qemu-img tool to fix:
>>
>>   $ make vm-build-ubuntu.i386
>>   Traceback (most recent call last):
>>     File "source/qemu/tests/vm/basevm.py", line 236, in main
>>       return vm.build_image(args.image)
>>     File "tests/vm/ubuntu.i386", line 67, in build_image
>>       subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
>>   OSError: [Errno 2] No such file or directory
>>   tests/vm/Makefile.include:23: recipe for target 'tests/vm/ubuntu.i386.img' failed
>>   make: *** [tests/vm/ubuntu.i386.img] Error 2
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/vm/Makefile.include | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
>> index 5daa2a3b73..e5e8146267 100644
>> --- a/tests/vm/Makefile.include
>> +++ b/tests/vm/Makefile.include
>> @@ -17,10 +17,13 @@ vm-test:
>>  
>>  vm-build-all: $(addprefix vm-build-, $(IMAGES))
>>  
>> +$(IMAGE_FILES): qemu-img$(EXESUF)
> 
> No, please don't do this. Let's keep on supporting vm tests from a clean source
> tree. Similar to qemu_bin, this can be handled in basevm.py.

OK.

>> +
>>  tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
>>  		$(SRC_PATH)/tests/vm/basevm.py \
>>  		$(SRC_PATH)/tests/vm/Makefile.include
>>  	$(call quiet-command, \
>> +		PATH=$(PATH):. \
>>  		$< \
>>  		$(if $(V)$(DEBUG), --debug) \
>>  		--image "$@" \
>> -- 
>> 2.18.0
>>
> 

Re: [Qemu-devel] [PATCH 1/4] tests/vm: Support proxy / corporate firewall

[Fam Zheng]

On Mon, 07/02 12:11, Philippe Mathieu-Daudé wrote:
> Hi Fam,
> 
> On 07/02/2018 04:12 AM, Fam Zheng wrote:
> > On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
> >> If ftp_proxy/http_proxy/https_proxy standard environment variables
> >> are available, pass them to the vm images.
> >>
> >> As per 06cc3551714:
> >> This is required when building behind corporate proxy/firewall, but
> >> also help when using local cache server (ie: apt/yum).
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >>  tests/vm/ubuntu.i386 | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
> >> index fc319e0e6e..be16ceed50 100755
> >> --- a/tests/vm/ubuntu.i386
> >> +++ b/tests/vm/ubuntu.i386
> >> @@ -68,6 +68,10 @@ class UbuntuX86VM(basevm.BaseVM):
> >>          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")
> >> +        for k, v in os.environ.iteritems():
> >> +            kl = k.lower()
> >> +            if kl in ['ftp_proxy', 'http_proxy', 'https_proxy']:
> >> +                self.ssh_root_check("echo 'Acquire::{}::Proxy \"{}\";' >> /etc/apt/apt.conf.d/01proxy".format(kl[:-6].upper(), v))
> > 
> > Reasonable, but do we want it for other apps and images? How about setting these
> > env vars to ssh commands?
> 
> I see 2 different network usages:
> 
> 1/ how the guest connect to the outer world, this goes via the firewall.
> Here the change only affects apt* based commands (via the apt.conf file).
> Do we have other commands requiring network connectivity? If we have,
> then yes, we should add the same env vars in the guest.
> One case I think of is "git submodule init" calling "git clone".

Yes, I think this case is what we are looking at here. But this patch is very
specific: it only affects one command in one VM, albeit we don't have many.
Doing this means we'll need to specially open code tests/vm/fedora,
tests/vm/debian, or any other images we introduce later, to be consistent with
ubuntu.i386. It is a poor way to do this, IMO.

What I mean is, can we do it more generically? Like setting the env var in guest
/etc/profile or change BaseVM._ssh_do() to inject env vars:

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 3643117816..94501e7dc7 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -106,7 +106,9 @@ class BaseVM(object):
         if interactive:
             ssh_cmd += ['-t']
         assert not isinstance(cmd, str)
-        ssh_cmd += ["%s@127.0.0.1" % user] + list(cmd)
+        env = ["%s=%s" % (k, v) for k, v in os.environ.items() if k in \
+                ["ftp_proxy", "http_proxy", "https_proxy"]]
+        ssh_cmd += ["%s@127.0.0.1" % user] + env + list(cmd)
         logging.debug("ssh_cmd: %s", " ".join(ssh_cmd))
         r = subprocess.call(ssh_cmd)
         if check and r != 0:

Re: [Qemu-devel] [PATCH 2/4] tests/vm: Add a dependency to qemu-img

[Philippe Mathieu-Daudé]

Hi Fam,

On 07/02/2018 12:19 PM, Philippe Mathieu-Daudé wrote:
> On 07/02/2018 04:18 AM, Fam Zheng wrote:
>> On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
>>> Before the first use, the VM image are resized with qemu-img.
>>>
>>> Add a dependency to the qemu-img tool to fix:
>>>
>>>   $ make vm-build-ubuntu.i386
>>>   Traceback (most recent call last):
>>>     File "source/qemu/tests/vm/basevm.py", line 236, in main
>>>       return vm.build_image(args.image)
>>>     File "tests/vm/ubuntu.i386", line 67, in build_image
>>>       subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
>>>   OSError: [Errno 2] No such file or directory
>>>   tests/vm/Makefile.include:23: recipe for target 'tests/vm/ubuntu.i386.img' failed
>>>   make: *** [tests/vm/ubuntu.i386.img] Error 2
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  tests/vm/Makefile.include | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
>>> index 5daa2a3b73..e5e8146267 100644
>>> --- a/tests/vm/Makefile.include
>>> +++ b/tests/vm/Makefile.include
>>> @@ -17,10 +17,13 @@ vm-test:
>>>  
>>>  vm-build-all: $(addprefix vm-build-, $(IMAGES))
>>>  
>>> +$(IMAGE_FILES): qemu-img$(EXESUF)
>>
>> No, please don't do this. Let's keep on supporting vm tests from a clean source
>> tree. Similar to qemu_bin, this can be handled in basevm.py.
> 
> OK.

The ubuntu.i386 VM is the only one using the qemu-img tool in his
build_image() method implementation. Is it OK to add this check there?
The basevm.build_image() is only an abstract method.

>>> +
>>>  tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
>>>  		$(SRC_PATH)/tests/vm/basevm.py \
>>>  		$(SRC_PATH)/tests/vm/Makefile.include
>>>  	$(call quiet-command, \
>>> +		PATH=$(PATH):. \
>>>  		$< \
>>>  		$(if $(V)$(DEBUG), --debug) \
>>>  		--image "$@" \
>>> -- 
>>> 2.18.0
>>>
>>

Re: [Qemu-devel] [PATCH 2/4] tests/vm: Add a dependency to qemu-img

[Fam Zheng]

On Tue, 07/03 12:56, Philippe Mathieu-Daudé wrote:
> Hi Fam,
> 
> On 07/02/2018 12:19 PM, Philippe Mathieu-Daudé wrote:
> > On 07/02/2018 04:18 AM, Fam Zheng wrote:
> >> On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
> >>> Before the first use, the VM image are resized with qemu-img.
> >>>
> >>> Add a dependency to the qemu-img tool to fix:
> >>>
> >>>   $ make vm-build-ubuntu.i386
> >>>   Traceback (most recent call last):
> >>>     File "source/qemu/tests/vm/basevm.py", line 236, in main
> >>>       return vm.build_image(args.image)
> >>>     File "tests/vm/ubuntu.i386", line 67, in build_image
> >>>       subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
> >>>   OSError: [Errno 2] No such file or directory
> >>>   tests/vm/Makefile.include:23: recipe for target 'tests/vm/ubuntu.i386.img' failed
> >>>   make: *** [tests/vm/ubuntu.i386.img] Error 2
> >>>
> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>> ---
> >>>  tests/vm/Makefile.include | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> >>> index 5daa2a3b73..e5e8146267 100644
> >>> --- a/tests/vm/Makefile.include
> >>> +++ b/tests/vm/Makefile.include
> >>> @@ -17,10 +17,13 @@ vm-test:
> >>>  
> >>>  vm-build-all: $(addprefix vm-build-, $(IMAGES))
> >>>  
> >>> +$(IMAGE_FILES): qemu-img$(EXESUF)
> >>
> >> No, please don't do this. Let's keep on supporting vm tests from a clean source
> >> tree. Similar to qemu_bin, this can be handled in basevm.py.
> > 
> > OK.
> 
> The ubuntu.i386 VM is the only one using the qemu-img tool in his
> build_image() method implementation. Is it OK to add this check there?
> The basevm.build_image() is only an abstract method.

Maybe you can add a BaseVM.qemu_img() method that looks up for available
qemu-img on the system or build dir.

Fam

> 
> >>> +
> >>>  tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
> >>>  		$(SRC_PATH)/tests/vm/basevm.py \
> >>>  		$(SRC_PATH)/tests/vm/Makefile.include
> >>>  	$(call quiet-command, \
> >>> +		PATH=$(PATH):. \
> >>>  		$< \
> >>>  		$(if $(V)$(DEBUG), --debug) \
> >>>  		--image "$@" \
> >>> -- 
> >>> 2.18.0
> >>>
> >>