[PATCH] generic/081: clean up snapshot devices correctly

[PATCH] generic/081: clean up snapshot devices correctly

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

I recently updated the userspace on a DAX-capable test VM, and no
g/081 fails to release the SCRATCH_DEV correctly. g/081 fails on dax
capable devices because DM rejects one of the setup operations:

The 081.full output file shows setup failure and then snapshot
creation failure:

  Wiping xfs signature on /dev/pmem1.
  Physical volume "/dev/pmem1" successfully created.
  Volume group "vg_081" successfully created
  Logical volume "base_081" created.
  device-mapper: reload ioctl on  (251:0) failed: Invalid argument
  Failed to suspend logical volume vg_081/base_081.
  Device vg_081-base_081-real (251:1) is used by another device.
  Failed to revert logical volume vg_081/base_081.
  Aborting. Manual intervention required.
Failed to create snapshot
.....

And dmesg tells us the reload ioctl failed because:

[ 2977.522306] device-mapper: ioctl: can't change device type (old=3 vs new=1) after initial table load.

DM table types are:

enum dm_queue_mode {
        DM_TYPE_NONE             = 0,
        DM_TYPE_BIO_BASED        = 1,
        DM_TYPE_REQUEST_BASED    = 2,
        DM_TYPE_DAX_BIO_BASED    = 3,
};

which indicates that the original table is DAX_BIO based (correct,
this is a pmem device) but then one of the setup operations is for
a type that is not DAX capable. Hence it fails and then things go
bad.

But the test has actually created the snapshot device and it's COW
target, so now when the _cleanup() function is called, the vgremove
command fails because the VG is busy (still has snapshot devices
attached to it) and at this point $SCRATCH_DEV is now unusable
without manual cleanup.

Manual cleanup just needs to remove the snapshot and logical base
volumes before trying to remove the vg and pv.  With this change, we
now see the teardown do:

....
Failed to create snapshot
umount: /mnt/test/mnt_081: not mounted.
  Logical volume "snap_081" successfully removed
  Logical volume "base_081" successfully removed
  Volume group "vg_081" successfully removed
  Labels on physical volume "/dev/pmem1" successfully wiped.

And future tests are able to use the SCRATCH_DEV normally.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/generic/081 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/generic/081 b/tests/generic/081
index 8e552074..9f294c11 100755
--- a/tests/generic/081
+++ b/tests/generic/081
@@ -33,6 +33,8 @@ _cleanup()
 	while test -e /dev/mapper/$vgname-$snapname || \
 	      test -e /dev/mapper/$vgname-$lvname; do
 		$UMOUNT_PROG $mnt >> $seqres.full 2>&1
+		$LVM_PROG lvremove -f $vgname/$snapname >>$seqres.full 2>&1
+		$LVM_PROG lvremove -f $vgname/$lvname >>$seqres.full 2>&1
 		$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
 		$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
 		pv_ret=$?
-- 
2.31.1

Re: [PATCH] generic/081: clean up snapshot devices correctly

[Zorro Lang]

On Mon, Aug 30, 2021 at 11:13:24AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> I recently updated the userspace on a DAX-capable test VM, and no
> g/081 fails to release the SCRATCH_DEV correctly. g/081 fails on dax
> capable devices because DM rejects one of the setup operations:
> 
> The 081.full output file shows setup failure and then snapshot
> creation failure:
> 
>   Wiping xfs signature on /dev/pmem1.
>   Physical volume "/dev/pmem1" successfully created.
>   Volume group "vg_081" successfully created
>   Logical volume "base_081" created.
>   device-mapper: reload ioctl on  (251:0) failed: Invalid argument
>   Failed to suspend logical volume vg_081/base_081.
>   Device vg_081-base_081-real (251:1) is used by another device.
>   Failed to revert logical volume vg_081/base_081.
>   Aborting. Manual intervention required.
> Failed to create snapshot
> .....
> 
> And dmesg tells us the reload ioctl failed because:
> 
> [ 2977.522306] device-mapper: ioctl: can't change device type (old=3 vs new=1) after initial table load.
> 
> DM table types are:
> 
> enum dm_queue_mode {
>         DM_TYPE_NONE             = 0,
>         DM_TYPE_BIO_BASED        = 1,
>         DM_TYPE_REQUEST_BASED    = 2,
>         DM_TYPE_DAX_BIO_BASED    = 3,
> };
> 
> which indicates that the original table is DAX_BIO based (correct,
> this is a pmem device) but then one of the setup operations is for
> a type that is not DAX capable. Hence it fails and then things go
> bad.
> 
> But the test has actually created the snapshot device and it's COW
> target, so now when the _cleanup() function is called, the vgremove
> command fails because the VG is busy (still has snapshot devices
> attached to it) and at this point $SCRATCH_DEV is now unusable
> without manual cleanup.
> 
> Manual cleanup just needs to remove the snapshot and logical base
> volumes before trying to remove the vg and pv.  With this change, we
> now see the teardown do:
> 
> ....
> Failed to create snapshot
> umount: /mnt/test/mnt_081: not mounted.
>   Logical volume "snap_081" successfully removed
>   Logical volume "base_081" successfully removed
>   Volume group "vg_081" successfully removed
>   Labels on physical volume "/dev/pmem1" successfully wiped.
> 
> And future tests are able to use the SCRATCH_DEV normally.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  tests/generic/081 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/generic/081 b/tests/generic/081
> index 8e552074..9f294c11 100755
> --- a/tests/generic/081
> +++ b/tests/generic/081
> @@ -33,6 +33,8 @@ _cleanup()
>  	while test -e /dev/mapper/$vgname-$snapname || \
>  	      test -e /dev/mapper/$vgname-$lvname; do
>  		$UMOUNT_PROG $mnt >> $seqres.full 2>&1
> +		$LVM_PROG lvremove -f $vgname/$snapname >>$seqres.full 2>&1
> +		$LVM_PROG lvremove -f $vgname/$lvname >>$seqres.full 2>&1

Make sense to me. Although I don't fail on it, this patch doesn't break
my test.

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  		$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
>  		$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
>  		pv_ret=$?
> -- 
> 2.31.1
>