Libguestfs - Mar 2018 - Re: [PATCH nbdkit 1/2] tests: Remove QEMU_IO / HAVE_QEMU_IO.

On 03/06/2018 06:21 AM, Richard W.M. Jones wrote:
> This is for consistency with qemu-img, socat, ss, etc where we test
> for these binaries at run time.
> ---
>   configure.ac                |  4 ----
>   tests/Makefile.am           |  8 +++-----
>   tests/test-parallel-file.sh | 20 +++++++++++++-------
>   tests/test-parallel-nbd.sh  | 20 +++++++++++++-------
>   4 files changed, 29 insertions(+), 23 deletions(-)
> 
> +++ b/tests/test-parallel-file.sh
> @@ -31,17 +31,23 @@
>   # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   # SUCH DAMAGE.
>   
> -# Makefile sets $QEMU_IO, but it's also nice if the script runs again
> -# standalone afterwards for diagnosing any failures
> -: ${QEMU_IO=qemu-io}
> +# Check file-data was created by Makefile and qemu-io exists.
> +if ! test -f file-data; then
Needs rebasing now that the test no longer relies on file-data.  Also, I 
still appreciate being able to override $QEMU_IO from the command line 
(to point to an alternative version on the fly), so even though I agree 
with your Makefile changes, I disagree with dropping the defaulting of 
QEMU_IO to qemu-io,
>   # Populate file, and sanity check that qemu-io can issue parallel
requests
>   printf '%1024s' . > test-parallel-file.data
> -$QEMU_IO -f raw -c "aio_write -P 1 0 512" -c "aio_write -P
2 512 512" \
> +qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2
512 512" \
as well as disagree with hard-coding only the first qemu-io in $PATH 
here, instead of allowing a command-line override.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Wed, Mar 07, 2018 at 08:39:05AM -0600, Eric Blake
wrote:
> On 03/06/2018 06:21 AM, Richard W.M. Jones wrote:
> >This is for consistency with qemu-img, socat, ss, etc where we test
> >for these binaries at run time.
> >---
> >  configure.ac                |  4 ----
> >  tests/Makefile.am           |  8 +++-----
> >  tests/test-parallel-file.sh | 20 +++++++++++++-------
> >  tests/test-parallel-nbd.sh  | 20 +++++++++++++-------
> >  4 files changed, 29 insertions(+), 23 deletions(-)
> >
> 
> >+++ b/tests/test-parallel-file.sh
> >@@ -31,17 +31,23 @@
> >  # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> >  # SUCH DAMAGE.
> >-# Makefile sets $QEMU_IO, but it's also nice if the script runs
again
> >-# standalone afterwards for diagnosing any failures
> >-: ${QEMU_IO=qemu-io}
> >+# Check file-data was created by Makefile and qemu-io exists.
> >+if ! test -f file-data; then
> 
> Needs rebasing now that the test no longer relies on file-data.
> Also, I still appreciate being able to override $QEMU_IO from the
> command line (to point to an alternative version on the fly), so
> even though I agree with your Makefile changes, I disagree with
> dropping the defaulting of QEMU_IO to qemu-io,
Does setting $PATH not work for this purpose?

Rich.
> >  # Populate file, and sanity check that qemu-io can issue parallel
requests
> >  printf '%1024s' . > test-parallel-file.data
> >-$QEMU_IO -f raw -c "aio_write -P 1 0 512" -c "aio_write
-P 2 512 512" \
> >+qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write
-P 2 512 512" \
> 
> as well as disagree with hard-coding only the first qemu-io in $PATH
> here, instead of allowing a command-line override.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On 03/07/2018 09:39 AM, Richard W.M. Jones wrote:
>> Needs rebasing now that the test no longer relies on file-data.
>> Also, I still appreciate being able to override $QEMU_IO from the
>> command line (to point to an alternative version on the fly), so
>> even though I agree with your Makefile changes, I disagree with
>> dropping the defaulting of QEMU_IO to qemu-io,
> 
> Does setting $PATH not work for this purpose?
> 
> Rich.
> 
>>>   # Populate file, and sanity check that qemu-io can issue parallel
requests
>>>   printf '%1024s' . > test-parallel-file.data
>>> -$QEMU_IO -f raw -c "aio_write -P 1 0 512" -c
"aio_write -P 2 512 512" \
>>> +qemu-io -f raw -c "aio_write -P 1 0 512" -c
"aio_write -P 2 512 512" \
>>
>> as well as disagree with hard-coding only the first qemu-io in $PATH
>> here, instead of allowing a command-line override.
Well, using PATH overrides might work, but it can also be more verbose. 
It's also trickier: if you have two binaries residing in a single 
directory - say qemu-io and qemu-img - but only want the override to 
apply for just one of those binaries, then a direct QEMU_IO override 
would do it at once, but a PATH override would require creating a 
temporary directory, adding a symlink in the temp directory to pick up 
the one binary that will be used as an override, then pointing PATH to 
pick up the temp directory first.

But since you are right that a PATH override is always possible, even if 
more verbose, I won't insist on a QEMU_IO override as it is merely 
syntactic sugar.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org