Libguestfs - Sep 2018 - [PATCH] sparsify: Fix test-virt-sparsify-in-place-fstrim-unsupported.sh

Since Linux commit f663b5b38fff trimming vfat is now supported by
Linux.  This broke the test which assumed it was not supported.  Use
another filesystem (minix) which does not support trimming instead.

Thanks: Daniel P. Berrangé and Pino Toscano.
---
 ...virt-sparsify-in-place-fstrim-unsupported.sh | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
b/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
index ff7dab3fa..e2b2e9c20 100755
--- a/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
+++ b/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
@@ -21,13 +21,11 @@
 #
 # https://bugzilla.redhat.com/show_bug.cgi?id=1364347
 #
-# This test assumes that the kernel vfat driver does not support
+# This test assumes that the kernel minix driver does not support
 # fstrim.  It might become supported in a future kernel version in
 # which case we could use a different filesystem for this test, or
 # delete the test if we are confident that all common filesystems are
 # supported.
-#
-# The reason why vfat is significant is because UEFI guests use it.
 
 set -e
 set -x
@@ -41,8 +39,13 @@ img=test-virt-sparsify-in-place-fstrim-unsupported.img
 log=test-virt-sparsify-in-place-fstrim-unsupported.log
 rm -f $img $log
 
-# Create a test filesystem with a single vfat filesystem.
-guestfish -N $img=fs:vfat exit
+# Create a test filesystem with a single minix filesystem.  If this
+# fails, skip the test under the assumption that (like with Enterprise
+# Linux systems) Minix is not supported.
+guestfish -N $img=fs:minix exit || {
+    echo $0: skipping the test because Minix filesystems are not supported
+    exit 77
+}
 
 # This should warn.
 virt-sparsify --in-place $img |& tee $log
@@ -58,8 +61,8 @@ if grep "warning:.*fstrim.*not supported" $log; then
     exit 1
 fi
 
-# Create a test filesystem with vfat and ext4 filesystems.
-guestfish -N $img=bootroot:vfat:ext4 exit
+# Create a test filesystem with minix and ext4 filesystems.
+guestfish -N $img=bootroot:minix:ext4 exit
 
 # This should warn.
 virt-sparsify --in-place $img |& tee $log
-- 
2.18.0

On Wed, Sep 05, 2018 at 05:30:39PM +0100, Richard W.M. Jones
wrote:
> Since Linux commit f663b5b38fff trimming vfat is now supported by
> Linux.  This broke the test which assumed it was not supported.  Use
> another filesystem (minix) which does not support trimming instead.
> 
> Thanks: Daniel P. Berrangé and Pino Toscano.
> ---
>  ...virt-sparsify-in-place-fstrim-unsupported.sh | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wednesday, 5 September 2018 18:30:39 CEST Richard W.M. Jones
wrote:
> Since Linux commit f663b5b38fff trimming vfat is now supported by
> Linux.  This broke the test which assumed it was not supported.  Use
> another filesystem (minix) which does not support trimming instead.
> 
> Thanks: Daniel P. Berrangé and Pino Toscano.
> ---
>  ...virt-sparsify-in-place-fstrim-unsupported.sh | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
b/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
> index ff7dab3fa..e2b2e9c20 100755
> --- a/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
> +++ b/sparsify/test-virt-sparsify-in-place-fstrim-unsupported.sh
> @@ -21,13 +21,11 @@
>  #
>  # https://bugzilla.redhat.com/show_bug.cgi?id=1364347
>  #
> -# This test assumes that the kernel vfat driver does not support
> +# This test assumes that the kernel minix driver does not support
>  # fstrim.  It might become supported in a future kernel version in
>  # which case we could use a different filesystem for this test, or
>  # delete the test if we are confident that all common filesystems are
>  # supported.
Spot-on comment...
> -# Create a test filesystem with a single vfat filesystem.
> -guestfish -N $img=fs:vfat exit
> +# Create a test filesystem with a single minix filesystem.  If this
> +# fails, skip the test under the assumption that (like with Enterprise
> +# Linux systems) Minix is not supported.
> +guestfish -N $img=fs:minix exit || {
> +    echo $0: skipping the test because Minix filesystems are not supported
> +    exit 77
> +}
IMHO a better test would be using "filesystem_available minix",
although
that means spawning the appliance once more.  In case of genuine
failure in libguestfs/guestfish, the above check will mark the test as
skipped, which is not the ideal situation IMHO.

-- 
Pino Toscano