diff mbox

[v4,1/2] iotests: Use absolute paths for executables

Message ID 20170621133023.27627-2-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Max Reitz June 21, 2017, 1:30 p.m. UTC
A user may specify a relative path for accessing qemu, qemu-img, etc.
through environment variables ($QEMU_PROG and friends) or a symlink.

If a test decides to change its working directory, relative paths will
cease to work, however. Work around this by making all of the paths to
programs that should undergo testing absolute. Besides "realpath", we
also have to use "which" to support programs in $PATH.

As a side effect, this fixes specifying these programs as symlinks for
out-of-tree builds: Before, you would have to create two symlinks, one
in the build and one in the source tree (the first one for common.config
to find, the second one for the iotest to use). Now it is sufficient to
create one in the build tree because common.config will resolve it.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.config | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eric Blake June 29, 2017, 6:52 p.m. UTC | #1
On 06/21/2017 08:30 AM, Max Reitz wrote:
> A user may specify a relative path for accessing qemu, qemu-img, etc.
> through environment variables ($QEMU_PROG and friends) or a symlink.
> 
> If a test decides to change its working directory, relative paths will
> cease to work, however. Work around this by making all of the paths to
> programs that should undergo testing absolute. Besides "realpath", we
> also have to use "which" to support programs in $PATH.

Is this sentence stale...

> 
> As a side effect, this fixes specifying these programs as symlinks for
> out-of-tree builds: Before, you would have to create two symlinks, one
> in the build and one in the source tree (the first one for common.config
> to find, the second one for the iotest to use). Now it is sufficient to
> create one in the build tree because common.config will resolve it.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/common.config | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
> index d1b45f5..c1dc425 100644
> --- a/tests/qemu-iotests/common.config
> +++ b/tests/qemu-iotests/common.config
> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>  fi
>  
> +export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")

...now that you updated per my review to favor 'type' over 'which'?
Otherwise, the R-b stands.
Max Reitz June 30, 2017, 2:46 a.m. UTC | #2
On 2017-06-29 20:52, Eric Blake wrote:
> On 06/21/2017 08:30 AM, Max Reitz wrote:
>> A user may specify a relative path for accessing qemu, qemu-img, etc.
>> through environment variables ($QEMU_PROG and friends) or a symlink.
>>
>> If a test decides to change its working directory, relative paths will
>> cease to work, however. Work around this by making all of the paths to
>> programs that should undergo testing absolute. Besides "realpath", we
>> also have to use "which" to support programs in $PATH.
> 
> Is this sentence stale...

Gnarf. Sure is, yes. Thanks for spotting!

>> As a side effect, this fixes specifying these programs as symlinks for
>> out-of-tree builds: Before, you would have to create two symlinks, one
>> in the build and one in the source tree (the first one for common.config
>> to find, the second one for the iotest to use). Now it is sufficient to
>> create one in the build tree because common.config will resolve it.
>>
>> Reported-by: Kevin Wolf <kwolf@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/common.config | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
>> index d1b45f5..c1dc425 100644
>> --- a/tests/qemu-iotests/common.config
>> +++ b/tests/qemu-iotests/common.config
>> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>>  fi
>>  
>> +export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
> 
> ...now that you updated per my review to favor 'type' over 'which'?
> Otherwise, the R-b stands.

Thanks, will fix and apply then...

...and done, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max
Eric Blake June 30, 2017, 7:41 p.m. UTC | #3
On 06/29/2017 09:46 PM, Max Reitz wrote:

>>> +++ b/tests/qemu-iotests/common.config
>>> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>>>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>>>  fi
>>>  
>>> +export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
>>
>> ...now that you updated per my review to favor 'type' over 'which'?
>> Otherwise, the R-b stands.
> 
> Thanks, will fix and apply then...
> 
> ...and done, applied to my block branch:
> 
> https://github.com/XanClic/qemu/commits/block

Sorry for not noticing sooner, but you'll need to replace v4 (commit
0f0fec82 on your branch) with a fix, because now your branch does the
following on all iotests for me:

068 3s ... - output mismatch (see 068.out.bad)
--- /home/eblake/qemu/tests/qemu-iotests/068.out	2017-06-26
22:02:56.057734882 -0500
+++ 068.out.bad	2017-06-30 14:35:28.720241398 -0500
@@ -1,4 +1,5 @@
 QA output created by 068
+realpath: '': No such file or directory

The culprit? $QEMU_VXHS_PROG is empty for me, which means `set_prog_path
qnio_server` found nothing to use.  You'll have to add in a safety valve
that only calls 'type' if operating on a non-empty path in the first place.
Max Reitz July 2, 2017, 1:35 p.m. UTC | #4
On 2017-06-30 21:41, Eric Blake wrote:
> On 06/29/2017 09:46 PM, Max Reitz wrote:
> 
>>>> +++ b/tests/qemu-iotests/common.config
>>>> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>>>>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>>>>  fi
>>>>  
>>>> +export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
>>>
>>> ...now that you updated per my review to favor 'type' over 'which'?
>>> Otherwise, the R-b stands.
>>
>> Thanks, will fix and apply then...
>>
>> ...and done, applied to my block branch:
>>
>> https://github.com/XanClic/qemu/commits/block
> 
> Sorry for not noticing sooner, but you'll need to replace v4 (commit
> 0f0fec82 on your branch) with a fix, because now your branch does the
> following on all iotests for me:
> 
> 068 3s ... - output mismatch (see 068.out.bad)
> --- /home/eblake/qemu/tests/qemu-iotests/068.out	2017-06-26
> 22:02:56.057734882 -0500
> +++ 068.out.bad	2017-06-30 14:35:28.720241398 -0500
> @@ -1,4 +1,5 @@
>  QA output created by 068
> +realpath: '': No such file or directory

Oops.

> The culprit? $QEMU_VXHS_PROG is empty for me, which means `set_prog_path
> qnio_server` found nothing to use.  You'll have to add in a safety valve
> that only calls 'type' if operating on a non-empty path in the first place.

Sure, will drop the patches and send a v5.

Thanks!

Max
diff mbox

Patch

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index d1b45f5..c1dc425 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -103,6 +103,12 @@  if [ -z "$QEMU_VXHS_PROG" ]; then
     export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
 fi
 
+export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
+export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
+export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
+export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
+export QEMU_VXHS_PROG=$(realpath -- "$(type -p "$QEMU_VXHS_PROG")")
+
 _qemu_wrapper()
 {
     (