[PATCH] xfstests: btrfs/003: stat the dev we're removing to make sure its' really gone

[PATCH] xfstests: btrfs/003: stat the dev we're removing to make sure its' really gone

[Josef Bacik]

I've been periodically failing btrfs/003 because my box sometimes takes a little
longer to unregister the device when we remove it and so the output from btrfs
dev show doesn't match what we are wanting since it still sees the device.  To
fix this just stat and sleep if we still see the device node and only continue
once udev or whatever actually removes the device node so that we don't get
random failures.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 tests/btrfs/003 |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tests/btrfs/003 b/tests/btrfs/003
index 5c88651..dba1a32 100755
--- a/tests/btrfs/003
+++ b/tests/btrfs/003
@@ -145,6 +145,12 @@ _test_replace()
 	_devmgt_remove ${DEVHTL}
 	dev_removed=1
 
+	stat $ds >> $seqres.full 2>&1
+	while [ $? -eq 0 ]; do
+		sleep 1
+		stat $ds >> $seqres.full 2>&1
+	done
+
 	$BTRFS_UTIL_PROG fi show $SCRATCH_DEV | grep "Some devices missing" >> $seqres.full || _fail \
 							"btrfs did not report device missing"
 
-- 
1.7.7.6

Re: [PATCH] xfstests: btrfs/003: stat the dev we're removing to make sure its' really gone

[Eric Sandeen]

On 8/21/13 11:03 AM, Josef Bacik wrote:
> I've been periodically failing btrfs/003 because my box sometimes takes a little
> longer to unregister the device when we remove it and so the output from btrfs
> dev show doesn't match what we are wanting since it still sees the device.  To
> fix this just stat and sleep if we still see the device node and only continue
> once udev or whatever actually removes the device node so that we don't get
> random failures.  Thanks,
> 
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
>  tests/btrfs/003 |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/tests/btrfs/003 b/tests/btrfs/003
> index 5c88651..dba1a32 100755
> --- a/tests/btrfs/003
> +++ b/tests/btrfs/003
> @@ -145,6 +145,12 @@ _test_replace()
>  	_devmgt_remove ${DEVHTL}
>  	dev_removed=1
>  

This should probably go into _devmgt_remove,
and possibly the reverse in _devmgmt_add as well, with
a comment explaining what it's doing?

Otherwise someone else will run into the same problem down the line.

-Eric

> +	stat $ds >> $seqres.full 2>&1
> +	while [ $? -eq 0 ]; do
> +		sleep 1
> +		stat $ds >> $seqres.full 2>&1
> +	done
> +
>  	$BTRFS_UTIL_PROG fi show $SCRATCH_DEV | grep "Some devices missing" >> $seqres.full || _fail \
>  							"btrfs did not report device missing"
>  

Re: [PATCH] xfstests: btrfs/003: stat the dev we're removing to make sure its' really gone

[Josef Bacik]

On Wed, Aug 21, 2013 at 11:44:47AM -0500, Eric Sandeen wrote:
> On 8/21/13 11:03 AM, Josef Bacik wrote:
> > I've been periodically failing btrfs/003 because my box sometimes takes a little
> > longer to unregister the device when we remove it and so the output from btrfs
> > dev show doesn't match what we are wanting since it still sees the device.  To
> > fix this just stat and sleep if we still see the device node and only continue
> > once udev or whatever actually removes the device node so that we don't get
> > random failures.  Thanks,
> > 
> > Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> > ---
> >  tests/btrfs/003 |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tests/btrfs/003 b/tests/btrfs/003
> > index 5c88651..dba1a32 100755
> > --- a/tests/btrfs/003
> > +++ b/tests/btrfs/003
> > @@ -145,6 +145,12 @@ _test_replace()
> >  	_devmgt_remove ${DEVHTL}
> >  	dev_removed=1
> >  
> 
> This should probably go into _devmgt_remove,
> and possibly the reverse in _devmgmt_add as well, with
> a comment explaining what it's doing?
> 
> Otherwise someone else will run into the same problem down the line.
> 

No, the next guy will have to go as much pain and annoyance as I did to make
sure he is worthy of the fix.  Thanks,

Josef

Re: [PATCH] xfstests: btrfs/003: stat the dev we're removing to make sure its' really gone

[Josef Bacik]

On Wed, Aug 21, 2013 at 11:44:47AM -0500, Eric Sandeen wrote:
> On 8/21/13 11:03 AM, Josef Bacik wrote:
> > I've been periodically failing btrfs/003 because my box sometimes takes a little
> > longer to unregister the device when we remove it and so the output from btrfs
> > dev show doesn't match what we are wanting since it still sees the device.  To
> > fix this just stat and sleep if we still see the device node and only continue
> > once udev or whatever actually removes the device node so that we don't get
> > random failures.  Thanks,
> > 
> > Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> > ---
> >  tests/btrfs/003 |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tests/btrfs/003 b/tests/btrfs/003
> > index 5c88651..dba1a32 100755
> > --- a/tests/btrfs/003
> > +++ b/tests/btrfs/003
> > @@ -145,6 +145,12 @@ _test_replace()
> >  	_devmgt_remove ${DEVHTL}
> >  	dev_removed=1
> >  
> 
> This should probably go into _devmgt_remove,
> and possibly the reverse in _devmgmt_add as well, with
> a comment explaining what it's doing?
> 
> Otherwise someone else will run into the same problem down the line.

Ok so I went to do this and realized we only send the formatted thing to the
function, ie '0 0 0' for host/target/lun or whatever the numbers line up to.  We
don't have the actual device node to check at this point, so it needs to be done
on a case by case basis.  I looked at the other tests and all they want is the
device removed for kernel stuff, which happens immediately.  We are the weird
ones checking to make sure btrfs fi show actually notices that we've removed the
device, so it's only really specific to this case and not something I can easily
add to the helper.  Thanks,

Josef