On 07/01/22 13:01, Richard W.M. Jones wrote:
 This only tests that it doesn't completely fail, which it did
before
 we fixed nbdcopy.  I checked the file sizes manually and with
 compression the resulting file is about half the size.
 ---
  tests/Makefile.am                          |  2 +
  tests/test-v2v-o-local-qcow2-compressed.sh | 51 ++++++++++++++++++++++
  tests/test-v2v-of-option.sh                |  2 +
  3 files changed, 55 insertions(+)
 
 diff --git a/tests/Makefile.am b/tests/Makefile.am
 index ebc433ae5e..fb068624c7 100644
 --- a/tests/Makefile.am
 +++ b/tests/Makefile.am
 @@ -83,6 +83,7 @@ TESTS = \
  	test-v2v-networks-and-bridges.sh \
  	test-v2v-o-glance.sh \
  	test-v2v-o-libvirt.sh \
 +	test-v2v-o-local-qcow2-compressed.sh \
  	test-v2v-o-null.sh \
  	test-v2v-o-openstack.sh \
  	test-v2v-o-qemu.sh \
 @@ -242,6 +243,7 @@ EXTRA_DIST += \
  	test-v2v-networks-and-bridges-expected.xml \
  	test-v2v-o-glance.sh \
  	test-v2v-o-libvirt.sh \
 +	test-v2v-o-local-qcow2-compressed.sh \
  	test-v2v-o-null.sh \
  	test-v2v-o-openstack.sh \
  	test-v2v-o-qemu.sh \
 diff --git a/tests/test-v2v-o-local-qcow2-compressed.sh
b/tests/test-v2v-o-local-qcow2-compressed.sh
 new file mode 100755
 index 0000000000..6ebf92c976
 --- /dev/null
 +++ b/tests/test-v2v-o-local-qcow2-compressed.sh
 @@ -0,0 +1,51 @@
 +#!/bin/bash -
 +# libguestfs virt-v2v test script
 +# Copyright (C) 2014-2022 Red Hat Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program; if not, write to the Free Software
 +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 +
 +# Test -o local -of qcow2 -oo compressed.
 +
 +set -e
 +
 +source ./functions.sh
 +set -e
 +set -x
 +
 +skip_if_skipped
 +requires test -f ../test-data/phony-guests/windows.img
 +
 +# This requires fixed nbdcopy >= 1.13.5.
 +requires nbdcopy --version
 +version="$( nbdcopy --version | head -1 | awk '{print $2}' )"
 +minor="$( echo "$version" | awk -F. '{print $2}' )"
 +release="$( echo "$version" | awk -F. '{print $3}' )"
 +requires test $minor -gt 13 -o \( $minor -eq 13 -a $release -ge 5 \) 
Perhaps simpler:
{
  printf '1.13.5\n'
  nbdcopy --version | sed -n 's/^nbdcopy //p'
} \
| requires sort --sort=version --check=quiet
Another alternative:
nbdcopy --version \
| {
    IFS=' .' read name major minor release
    requires test \( "$major" -gt 1 \) -o \
                  \( "$major" -eq 1 -a "$minor" -gt 13 \) -o
                  \( "$major" -eq 1 -a "$minor" -eq 13 -a
"$release" -ge 5 \)
  }
 +
 +export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
 +
 +d=test-v2v-o-local-qcow2-compressed.d
 +rm -rf $d
 +cleanup_fn rm -rf $d
 +mkdir $d
 +
 +$VG virt-v2v --debug-gc \
 +    -i disk ../test-data/phony-guests/windows.img \
 +    -o local -of qcow2 -oo compressed -os $d
 +
 +# Test the libvirt XML metadata and a disk was created.
 +ls -l $d
 +test -f $d/windows.xml
 +test -f $d/windows-sda
 diff --git a/tests/test-v2v-of-option.sh b/tests/test-v2v-of-option.sh
 index bdfd34180d..6c5f5938c8 100755
 --- a/tests/test-v2v-of-option.sh
 +++ b/tests/test-v2v-of-option.sh
 @@ -42,6 +42,8 @@ $VG virt-v2v --debug-gc \
      -i libvirt -ic "$libvirt_uri" windows \
      -o local -os $d -of qcow2
  
 +ls -l $d
 +
  # Test the disk is qcow2 format.
  if [ "$(guestfish disk-format $d/windows-sda)" != qcow2 ]; then
      echo "$0: test failed: output is not qcow2"
  
The last hunk really confused me. Please add a note to the commit message that the new
test is a modified clone of "test-v2v-of-option.sh", and that because we add
"ls -l $d" to the new test, we retrofit the old one for consistency.
(In theory, it should be a separate, precursor patch... but at least a commit msg note
should guide the reader.)
Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
Thanks,
Laszlo