diff mbox series

[RFC] ahci-test: Add dependency to qemu-img tool

Message ID 20190125203402.18904-1-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series [RFC] ahci-test: Add dependency to qemu-img tool | expand

Commit Message

Philippe Mathieu-Daudé Jan. 25, 2019, 8:34 p.m. UTC
Since the ahci-test uses qemu-img, add a dependency to build it
before using it.
This fixes:

  $ gmake check-qtest V=1
  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test
  Failed to execute child process "/tmp/qemu-test.19tMRF/qemu-img" (No such file or directory)
  ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC because while this dependency is valid, I don't think this is the
clever way to solve this problem (which is, assuming the host
distribution has the qemu-tools installed).
I guess remember a thread about it (Eric, John?) where it was asked
"What do we want to test, qemu-img or AHCI? Can we trust an unstable
version of a tool to verify a device?"

Slighly related is when vm-tests expect qemu-img available:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08415.html

  $ 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

A better fix would be checking those tools via ./configure...
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

John Snow Jan. 25, 2019, 9:57 p.m. UTC | #1
On 1/25/19 3:34 PM, Philippe Mathieu-Daudé wrote:
> Since the ahci-test uses qemu-img, add a dependency to build it
> before using it.
> This fixes:
> 
>   $ gmake check-qtest V=1
>   QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test
>   Failed to execute child process "/tmp/qemu-test.19tMRF/qemu-img" (No such file or directory)
>   ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> RFC because while this dependency is valid, I don't think this is the
> clever way to solve this problem (which is, assuming the host
> distribution has the qemu-tools installed).
> I guess remember a thread about it (Eric, John?) where it was asked
> "What do we want to test, qemu-img or AHCI? Can we trust an unstable
> version of a tool to verify a device?"
> 

Yeah, these discussions have a great track record of preventing us from
fixing problems.

Here's my two cents: "make check" already exports QTEST_QEMU_IMG as the
one we're building and gets passed to ahci-test -- might as well make
the dependency explicit in that same way.

> Slighly related is when vm-tests expect qemu-img available:
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08415.html
> 
>   $ 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
> 
> A better fix would be checking those tools via ./configure...
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 19b4c0a696..5e03416c81 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -706,7 +706,7 @@ tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
>  tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-spapr-obj-y)
>  tests/fdc-test$(EXESUF): tests/fdc-test.o
>  tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
> -tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
> +tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y) qemu-img$(EXESUF)
>  tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
>  tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
>  tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
> 

Reviewed-by: John Snow <jsnow@redhat.com>
Kevin Wolf Jan. 28, 2019, 4:08 p.m. UTC | #2
Am 25.01.2019 um 22:57 hat John Snow geschrieben:
> On 1/25/19 3:34 PM, Philippe Mathieu-Daudé wrote:
> > Since the ahci-test uses qemu-img, add a dependency to build it
> > before using it.
> > This fixes:
> > 
> >   $ gmake check-qtest V=1
> >   QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test
> >   Failed to execute child process "/tmp/qemu-test.19tMRF/qemu-img" (No such file or directory)
> >   ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > RFC because while this dependency is valid, I don't think this is the
> > clever way to solve this problem (which is, assuming the host
> > distribution has the qemu-tools installed).
> > I guess remember a thread about it (Eric, John?) where it was asked
> > "What do we want to test, qemu-img or AHCI? Can we trust an unstable
> > version of a tool to verify a device?"
> > 
> [...]
> 
> Reviewed-by: John Snow <jsnow@redhat.com>

Are you taking this through your tree, or through which tree should it
be merged?

Kevin
John Snow Jan. 28, 2019, 7:14 p.m. UTC | #3
On 1/28/19 11:08 AM, Kevin Wolf wrote:
> Am 25.01.2019 um 22:57 hat John Snow geschrieben:
>> On 1/25/19 3:34 PM, Philippe Mathieu-Daudé wrote:
>>> Since the ahci-test uses qemu-img, add a dependency to build it
>>> before using it.
>>> This fixes:
>>>
>>>   $ gmake check-qtest V=1
>>>   QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test
>>>   Failed to execute child process "/tmp/qemu-test.19tMRF/qemu-img" (No such file or directory)
>>>   ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> RFC because while this dependency is valid, I don't think this is the
>>> clever way to solve this problem (which is, assuming the host
>>> distribution has the qemu-tools installed).
>>> I guess remember a thread about it (Eric, John?) where it was asked
>>> "What do we want to test, qemu-img or AHCI? Can we trust an unstable
>>> version of a tool to verify a device?"
>>>
>> [...]
>>
>> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> Are you taking this through your tree, or through which tree should it
> be merged?
> 
> Kevin
> 

I can, but it'd be the only patch I have this week. If it's not a
burden, would you mind including it?
Philippe Mathieu-Daudé Jan. 29, 2019, 7:03 a.m. UTC | #4
On 1/28/19 8:14 PM, John Snow wrote:
> On 1/28/19 11:08 AM, Kevin Wolf wrote:
>> Am 25.01.2019 um 22:57 hat John Snow geschrieben:
>>> On 1/25/19 3:34 PM, Philippe Mathieu-Daudé wrote:
>>>> Since the ahci-test uses qemu-img, add a dependency to build it
>>>> before using it.
>>>> This fixes:
>>>>
>>>>   $ gmake check-qtest V=1
>>>>   QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test
>>>>   Failed to execute child process "/tmp/qemu-test.19tMRF/qemu-img" (No such file or directory)
>>>>   ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>> RFC because while this dependency is valid, I don't think this is the
>>>> clever way to solve this problem (which is, assuming the host
>>>> distribution has the qemu-tools installed).
>>>> I guess remember a thread about it (Eric, John?) where it was asked
>>>> "What do we want to test, qemu-img or AHCI? Can we trust an unstable
>>>> version of a tool to verify a device?"
>>>>
>>> [...]
>>>
>>> Reviewed-by: John Snow <jsnow@redhat.com>
>>
>> Are you taking this through your tree, or through which tree should it
>> be merged?
>>
>> Kevin
>>
> 
> I can, but it'd be the only patch I have this week. If it's not a
> burden, would you mind including it?

Actually there are few follow up patches slighly related (not AHCI), so
it indeed makes sense Kevin takes them. I'll respin.
diff mbox series

Patch

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 19b4c0a696..5e03416c81 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -706,7 +706,7 @@  tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
 tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-spapr-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o
 tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
-tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
+tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y) qemu-img$(EXESUF)
 tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
 tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o