* [PATCH] xfstests: unmount scratch mnt in test 307 @ 2013-05-03 20:11 Josef Bacik 2013-05-03 20:15 ` Eric Sandeen 0 siblings, 1 reply; 4+ messages in thread From: Josef Bacik @ 2013-05-03 20:11 UTC (permalink / raw) To: xfs, linux-btrfs So if you have a mount command that doesn't use /etc/mtab then it will spit out a different device for the mounted device. So say we have SCRATCH_DEV_POOL="/dev/sda /dev/sdb /dev/sdc" we will turn this into SCRATCH_DEV="/dev/sda" SCRATCH_DEV_POOL="/dev/sdb /dev/sdc" and then when you mkfs this you do _scratch_mkfs $SCRATCH_DEV_POOL which turns into this mkfs.btrfs /dev/sdb /dev/sdc /dev/sda becuase we do mkfs $* $SCRATCH_DEV Then btrfs will always show the lowest devid in /proc/mounts to maintain consistency, so even though we do mount /dev/sda $SCRATCH_MNT, you will see /dev/sdb as the mounted device in /proc/mounts. So then say the next test wants to just use $SCRATCH_DEV, it will do _require_scratchdev which will check to see if $SCRATCH_DEV is mounted, which it will look like it is not because /proc/mounts shows /dev/sdb instead of /dev/sda, and so it won't umount $SCRATCH_MNT, and then that test will fail because we can't mkfs the device because it is busy. I reproduced this on a box that doesn't use /etc/mtab by doing ./check btrfs/307 generic/015 and 015 would fail. With this patch it passes now. Thanks, Signed-off-by: Josef Bacik <jbacik@fusionio.com> --- tests/btrfs/307 | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tests/btrfs/307 b/tests/btrfs/307 index 87314c6..15157b3 100644 --- a/tests/btrfs/307 +++ b/tests/btrfs/307 @@ -35,6 +35,7 @@ _cleanup() { cd / rm -f $tmp.* + umount $SCRATCH_MNT } # get standard environment, filters and checks -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfstests: unmount scratch mnt in test 307 2013-05-03 20:11 [PATCH] xfstests: unmount scratch mnt in test 307 Josef Bacik @ 2013-05-03 20:15 ` Eric Sandeen 2013-05-03 23:27 ` Dave Chinner 0 siblings, 1 reply; 4+ messages in thread From: Eric Sandeen @ 2013-05-03 20:15 UTC (permalink / raw) To: Josef Bacik; +Cc: xfs, linux-btrfs On 5/3/13 3:11 PM, Josef Bacik wrote: > So if you have a mount command that doesn't use /etc/mtab then it will spit out > a different device for the mounted device. So say we have > > SCRATCH_DEV_POOL="/dev/sda /dev/sdb /dev/sdc" > > we will turn this into > > SCRATCH_DEV="/dev/sda" > SCRATCH_DEV_POOL="/dev/sdb /dev/sdc" > > and then when you mkfs this you do _scratch_mkfs $SCRATCH_DEV_POOL which turns > into this > > mkfs.btrfs /dev/sdb /dev/sdc /dev/sda > > becuase we do > > mkfs $* $SCRATCH_DEV > > Then btrfs will always show the lowest devid in /proc/mounts to maintain > consistency, so even though we do mount /dev/sda $SCRATCH_MNT, you will see > /dev/sdb as the mounted device in /proc/mounts. So then say the next test wants > to just use $SCRATCH_DEV, it will do _require_scratchdev which will check to see > if $SCRATCH_DEV is mounted, which it will look like it is not because > /proc/mounts shows /dev/sdb instead of /dev/sda, and so it won't umount > $SCRATCH_MNT, and then that test will fail because we can't mkfs the device > because it is busy. I reproduced this on a box that doesn't use /etc/mtab by > doing > > ./check btrfs/307 generic/015 > > and 015 would fail. With this patch it passes now. Thanks, > > Signed-off-by: Josef Bacik <jbacik@fusionio.com> > --- > tests/btrfs/307 | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/tests/btrfs/307 b/tests/btrfs/307 > index 87314c6..15157b3 100644 > --- a/tests/btrfs/307 > +++ b/tests/btrfs/307 > @@ -35,6 +35,7 @@ _cleanup() > { > cd / > rm -f $tmp.* > + umount $SCRATCH_MNT > } > > # get standard environment, filters and checks > This seems fine for this particular test. Is it really a hard requirement that each test unmount SCRATCH_[DEV|MNT] if it used it? If so, fine... the README does indicate this. But I wonder if we can make it a little more foolproof by updating _require_scratch to handle this situation more gracefully? -Eric ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfstests: unmount scratch mnt in test 307 2013-05-03 20:15 ` Eric Sandeen @ 2013-05-03 23:27 ` Dave Chinner 2013-06-26 15:50 ` [BULK] " Josef Bacik 0 siblings, 1 reply; 4+ messages in thread From: Dave Chinner @ 2013-05-03 23:27 UTC (permalink / raw) To: Eric Sandeen; +Cc: Josef Bacik, linux-btrfs, xfs On Fri, May 03, 2013 at 03:15:01PM -0500, Eric Sandeen wrote: > On 5/3/13 3:11 PM, Josef Bacik wrote: > > So if you have a mount command that doesn't use /etc/mtab then it will spit out > > a different device for the mounted device. So say we have > > > > SCRATCH_DEV_POOL="/dev/sda /dev/sdb /dev/sdc" > > > > we will turn this into > > > > SCRATCH_DEV="/dev/sda" > > SCRATCH_DEV_POOL="/dev/sdb /dev/sdc" > > > > and then when you mkfs this you do _scratch_mkfs $SCRATCH_DEV_POOL which turns > > into this > > > > mkfs.btrfs /dev/sdb /dev/sdc /dev/sda > > > > becuase we do > > > > mkfs $* $SCRATCH_DEV > > > > Then btrfs will always show the lowest devid in /proc/mounts to maintain > > consistency, so even though we do mount /dev/sda $SCRATCH_MNT, you will see > > /dev/sdb as the mounted device in /proc/mounts. So then say the next test wants > > to just use $SCRATCH_DEV, it will do _require_scratchdev which will check to see > > if $SCRATCH_DEV is mounted, which it will look like it is not because > > /proc/mounts shows /dev/sdb instead of /dev/sda, and so it won't umount > > $SCRATCH_MNT, and then that test will fail because we can't mkfs the device > > because it is busy. I reproduced this on a box that doesn't use /etc/mtab by > > doing > > > > ./check btrfs/307 generic/015 > > > > and 015 would fail. With this patch it passes now. Thanks, > > > > Signed-off-by: Josef Bacik <jbacik@fusionio.com> > > --- > > tests/btrfs/307 | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/tests/btrfs/307 b/tests/btrfs/307 > > index 87314c6..15157b3 100644 > > --- a/tests/btrfs/307 > > +++ b/tests/btrfs/307 > > @@ -35,6 +35,7 @@ _cleanup() > > { > > cd / > > rm -f $tmp.* > > + umount $SCRATCH_MNT > > } > > > > # get standard environment, filters and checks > > > > This seems fine for this particular test. > > Is it really a hard requirement that each test unmount SCRATCH_[DEV|MNT] if it used it? > If so, fine... the README does indicate this. > > But I wonder if we can make it a little more foolproof by updating _require_scratch > to handle this situation more gracefully? It already tries to unmount $SCRATCH_DEV, and will through an error if it's not mounted on $SCRATCH_MNT. I guess the opposite checks are necessary in this case i.e. check that SCRATCH_MNT is not mounted, and through an error if it's not SCRATCH_DEV that is mounted there... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BULK] Re: [PATCH] xfstests: unmount scratch mnt in test 307 2013-05-03 23:27 ` Dave Chinner @ 2013-06-26 15:50 ` Josef Bacik 0 siblings, 0 replies; 4+ messages in thread From: Josef Bacik @ 2013-06-26 15:50 UTC (permalink / raw) To: Dave Chinner Cc: Eric Sandeen, Josef Bacik, linux-btrfs@vger.kernel.org, xfs@oss.sgi.com On Fri, May 03, 2013 at 07:27:21PM -0400, Dave Chinner wrote: > On Fri, May 03, 2013 at 03:15:01PM -0500, Eric Sandeen wrote: > > On 5/3/13 3:11 PM, Josef Bacik wrote: > > > So if you have a mount command that doesn't use /etc/mtab then it will spit out > > > a different device for the mounted device. So say we have > > > > > > SCRATCH_DEV_POOL="/dev/sda /dev/sdb /dev/sdc" > > > > > > we will turn this into > > > > > > SCRATCH_DEV="/dev/sda" > > > SCRATCH_DEV_POOL="/dev/sdb /dev/sdc" > > > > > > and then when you mkfs this you do _scratch_mkfs $SCRATCH_DEV_POOL which turns > > > into this > > > > > > mkfs.btrfs /dev/sdb /dev/sdc /dev/sda > > > > > > becuase we do > > > > > > mkfs $* $SCRATCH_DEV > > > > > > Then btrfs will always show the lowest devid in /proc/mounts to maintain > > > consistency, so even though we do mount /dev/sda $SCRATCH_MNT, you will see > > > /dev/sdb as the mounted device in /proc/mounts. So then say the next test wants > > > to just use $SCRATCH_DEV, it will do _require_scratchdev which will check to see > > > if $SCRATCH_DEV is mounted, which it will look like it is not because > > > /proc/mounts shows /dev/sdb instead of /dev/sda, and so it won't umount > > > $SCRATCH_MNT, and then that test will fail because we can't mkfs the device > > > because it is busy. I reproduced this on a box that doesn't use /etc/mtab by > > > doing > > > > > > ./check btrfs/307 generic/015 > > > > > > and 015 would fail. With this patch it passes now. Thanks, > > > > > > Signed-off-by: Josef Bacik <jbacik@fusionio.com> > > > --- > > > tests/btrfs/307 | 1 + > > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > > > diff --git a/tests/btrfs/307 b/tests/btrfs/307 > > > index 87314c6..15157b3 100644 > > > --- a/tests/btrfs/307 > > > +++ b/tests/btrfs/307 > > > @@ -35,6 +35,7 @@ _cleanup() > > > { > > > cd / > > > rm -f $tmp.* > > > + umount $SCRATCH_MNT > > > } > > > > > > # get standard environment, filters and checks > > > > > > > This seems fine for this particular test. > > > > Is it really a hard requirement that each test unmount SCRATCH_[DEV|MNT] if it used it? > > If so, fine... the README does indicate this. > > > > But I wonder if we can make it a little more foolproof by updating _require_scratch > > to handle this situation more gracefully? > > It already tries to unmount $SCRATCH_DEV, and will through an error > if it's not mounted on $SCRATCH_MNT. I guess the opposite checks are > necessary in this case i.e. check that SCRATCH_MNT is not mounted, > and through an error if it's not SCRATCH_DEV that is mounted > there... > Just realized this never went in and I had forgotten to address y'alls comments, so I've sent another patch that accomplishes what you asked for and fixes my problem. Thanks, Josef ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-26 15:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-03 20:11 [PATCH] xfstests: unmount scratch mnt in test 307 Josef Bacik 2013-05-03 20:15 ` Eric Sandeen 2013-05-03 23:27 ` Dave Chinner 2013-06-26 15:50 ` [BULK] " Josef Bacik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).