| Message ID | 1490165188-488-1-git-send-email-zlang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 22, 2017 at 2:46 AM, Zorro Lang <zlang@redhat.com> wrote: > The format of glusterfs' TEST_DEV or SCRATCH_DEV is XXX:XXX or > XXX:/XXX, but xfstests can't accept the latter now. So change Why can't xfstest accept the latter? > the regular expression from "\w:\w" to ":/?", to accept more > glusterfs device format. > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > > Hi, > > According to the feedback from glusterfs developer: > > "Yes, it would be good to have an optional "/" after the ":". It is not > required, but would probably help when someone runs the tests with the > "hostname:/volume" device format." > The developer also said "We mostly use the format of 192.168.1.1:/testvol, matching the way NFS" If we can enforce the more common format which xfstest already expects for nfs*|ceph) Why would we want to support another optional format and have a special test case for it? > Maybe there's a slash before the gluster volume name. This patch > try to support this format. > > Thanks, > Zorro > > common/rc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/rc b/common/rc > index edfba5d..e1ab2c6 100644 > --- a/common/rc > +++ b/common/rc > @@ -1496,7 +1496,7 @@ _require_scratch_nocheck() > { > case "$FSTYP" in > glusterfs) > - echo $SCRATCH_DEV | grep -q "\w:\w" > /dev/null 2>&1 > + echo $SCRATCH_DEV | egrep -q ":/?" > /dev/null 2>&1 > if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then > _notrun "this test requires a valid \$SCRATCH_DEV" > fi > @@ -1584,7 +1584,7 @@ _require_test() > { > case "$FSTYP" in > glusterfs) > - echo $TEST_DEV | grep -q "\w:\w" > /dev/null 2>&1 > + echo $TEST_DEV | egrep -q ":/?" > /dev/null 2>&1 > if [ -z "$TEST_DEV" -o "$?" != "0" ]; then > _notrun "this test requires a valid \$TEST_DEV" > fi > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 22, 2017 at 06:34:00AM -0400, Amir Goldstein wrote: > On Wed, Mar 22, 2017 at 2:46 AM, Zorro Lang <zlang@redhat.com> wrote: > > The format of glusterfs' TEST_DEV or SCRATCH_DEV is XXX:XXX or > > XXX:/XXX, but xfstests can't accept the latter now. So change > > Why can't xfstest accept the latter? Because I use "\w:\w" in commit "4cbc0a0 fstests: add GlusterFS support". More details please see below. > > > the regular expression from "\w:\w" to ":/?", to accept more > > glusterfs device format. > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > --- > > > > Hi, > > > > According to the feedback from glusterfs developer: > > > > "Yes, it would be good to have an optional "/" after the ":". It is not > > required, but would probably help when someone runs the tests with the > > "hostname:/volume" device format." > > > > The developer also said > "We mostly use the format of 192.168.1.1:/testvol, matching the way NFS" > If we can enforce the more common format which xfstest already expects > for nfs*|ceph) > Why would we want to support another optional format and have a special > test case for it? Niels is the developer of glusterfs' NFS module (sorry, I forgot its project name), he test it likes test NFS: mount -t nfs 192.168.1.1:/XXXX /mnt So he only can use XXXX:/XXX format, because xfstests limit that format to mount NFS. > > > Maybe there's a slash before the gluster volume name. This patch > > try to support this format. > > > > Thanks, > > Zorro > > > > common/rc | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/common/rc b/common/rc > > index edfba5d..e1ab2c6 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -1496,7 +1496,7 @@ _require_scratch_nocheck() > > { > > case "$FSTYP" in > > glusterfs) > > - echo $SCRATCH_DEV | grep -q "\w:\w" > /dev/null 2>&1 > > + echo $SCRATCH_DEV | egrep -q ":/?" > /dev/null 2>&1 As I know, mount a glusrerfs always do like this: mount -t glusterfs 192.169.1.1:testvol /mnt or mount -t glusterfs 192.169.1.1:/testvol /mnt So glusterfs can use XXXX:XXX or XXXX:/XXX format, but if mount a nfs, we only can use XXXX:/XXX format. So that's why NFS use `grep -q ":/"`, but glusterfs use `egrep -q ":/?"` Thanks, Zorro > > if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then > > _notrun "this test requires a valid \$SCRATCH_DEV" > > fi > > @@ -1584,7 +1584,7 @@ _require_test() > > { > > case "$FSTYP" in > > glusterfs) > > - echo $TEST_DEV | grep -q "\w:\w" > /dev/null 2>&1 > > + echo $TEST_DEV | egrep -q ":/?" > /dev/null 2>&1 > > if [ -z "$TEST_DEV" -o "$?" != "0" ]; then > > _notrun "this test requires a valid \$TEST_DEV" > > fi > > -- > > 2.7.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe fstests" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 22, 2017 at 10:06 AM, Zorro Lang <zlang@redhat.com> wrote: > On Wed, Mar 22, 2017 at 06:34:00AM -0400, Amir Goldstein wrote: >> On Wed, Mar 22, 2017 at 2:46 AM, Zorro Lang <zlang@redhat.com> wrote: >> > The format of glusterfs' TEST_DEV or SCRATCH_DEV is XXX:XXX or >> > XXX:/XXX, but xfstests can't accept the latter now. So change >> >> Why can't xfstest accept the latter? > > Because I use "\w:\w" in commit "4cbc0a0 fstests: add GlusterFS support". > More details please see below. > >> >> > the regular expression from "\w:\w" to ":/?", to accept more >> > glusterfs device format. >> > >> > Signed-off-by: Zorro Lang <zlang@redhat.com> >> > --- >> > >> > Hi, >> > >> > According to the feedback from glusterfs developer: >> > >> > "Yes, it would be good to have an optional "/" after the ":". It is not >> > required, but would probably help when someone runs the tests with the >> > "hostname:/volume" device format." >> > >> >> The developer also said >> "We mostly use the format of 192.168.1.1:/testvol, matching the way NFS" >> If we can enforce the more common format which xfstest already expects >> for nfs*|ceph) >> Why would we want to support another optional format and have a special >> test case for it? > > Niels is the developer of glusterfs' NFS module (sorry, I forgot its project > name), he test it likes test NFS: > mount -t nfs 192.168.1.1:/XXXX /mnt > > So he only can use XXXX:/XXX format, because xfstests limit that format > to mount NFS. > >> >> > Maybe there's a slash before the gluster volume name. This patch >> > try to support this format. >> > >> > Thanks, >> > Zorro >> > >> > common/rc | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/common/rc b/common/rc >> > index edfba5d..e1ab2c6 100644 >> > --- a/common/rc >> > +++ b/common/rc >> > @@ -1496,7 +1496,7 @@ _require_scratch_nocheck() >> > { >> > case "$FSTYP" in >> > glusterfs) >> > - echo $SCRATCH_DEV | grep -q "\w:\w" > /dev/null 2>&1 >> > + echo $SCRATCH_DEV | egrep -q ":/?" > /dev/null 2>&1 > > As I know, mount a glusrerfs always do like this: > mount -t glusterfs 192.169.1.1:testvol /mnt > or > mount -t glusterfs 192.169.1.1:/testvol /mnt > > So glusterfs can use XXXX:XXX or XXXX:/XXX format, but if mount a nfs, > we only can use XXXX:/XXX format. > > So that's why NFS use `grep -q ":/"`, but glusterfs use `egrep -q ":/?"` > I understand. What I am saying is that if glusterfs *can* use XXXX:/XXX and a file system tester will dedicate a specific mount for xfstests, so tester can use the NFS format for that mount and we can require that. Is that really a big deal for the tester? If we do that we will have less special cases for format of *_DEV and I think that is a gain. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 22, 2017 at 11:08:27AM -0400, Amir Goldstein wrote: > On Wed, Mar 22, 2017 at 10:06 AM, Zorro Lang <zlang@redhat.com> wrote: > > On Wed, Mar 22, 2017 at 06:34:00AM -0400, Amir Goldstein wrote: > >> On Wed, Mar 22, 2017 at 2:46 AM, Zorro Lang <zlang@redhat.com> wrote: > >> > The format of glusterfs' TEST_DEV or SCRATCH_DEV is XXX:XXX or > >> > XXX:/XXX, but xfstests can't accept the latter now. So change > >> > >> Why can't xfstest accept the latter? > > > > Because I use "\w:\w" in commit "4cbc0a0 fstests: add GlusterFS support". > > More details please see below. > > > >> > >> > the regular expression from "\w:\w" to ":/?", to accept more > >> > glusterfs device format. > >> > > >> > Signed-off-by: Zorro Lang <zlang@redhat.com> > >> > --- > >> > > >> > Hi, > >> > > >> > According to the feedback from glusterfs developer: > >> > > >> > "Yes, it would be good to have an optional "/" after the ":". It is not > >> > required, but would probably help when someone runs the tests with the > >> > "hostname:/volume" device format." > >> > > >> > >> The developer also said > >> "We mostly use the format of 192.168.1.1:/testvol, matching the way NFS" > >> If we can enforce the more common format which xfstest already expects > >> for nfs*|ceph) > >> Why would we want to support another optional format and have a special > >> test case for it? > > > > Niels is the developer of glusterfs' NFS module (sorry, I forgot its project > > name), he test it likes test NFS: > > mount -t nfs 192.168.1.1:/XXXX /mnt > > > > So he only can use XXXX:/XXX format, because xfstests limit that format > > to mount NFS. > > > >> > >> > Maybe there's a slash before the gluster volume name. This patch > >> > try to support this format. > >> > > >> > Thanks, > >> > Zorro > >> > > >> > common/rc | 4 ++-- > >> > 1 file changed, 2 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/common/rc b/common/rc > >> > index edfba5d..e1ab2c6 100644 > >> > --- a/common/rc > >> > +++ b/common/rc > >> > @@ -1496,7 +1496,7 @@ _require_scratch_nocheck() > >> > { > >> > case "$FSTYP" in > >> > glusterfs) > >> > - echo $SCRATCH_DEV | grep -q "\w:\w" > /dev/null 2>&1 > >> > + echo $SCRATCH_DEV | egrep -q ":/?" > /dev/null 2>&1 > > > > As I know, mount a glusrerfs always do like this: > > mount -t glusterfs 192.169.1.1:testvol /mnt > > or > > mount -t glusterfs 192.169.1.1:/testvol /mnt > > > > So glusterfs can use XXXX:XXX or XXXX:/XXX format, but if mount a nfs, > > we only can use XXXX:/XXX format. > > > > So that's why NFS use `grep -q ":/"`, but glusterfs use `egrep -q ":/?"` > > > > I understand. > What I am saying is that if glusterfs *can* use XXXX:/XXX > and a file system tester will dedicate a specific mount for xfstests, > so tester can use the NFS format for that mount and we can require that. > > Is that really a big deal for the tester? > > If we do that we will have less special cases for format of *_DEV > and I think that is a gain. Hah, I already added many things for glusterfs in commit "4cbc0a0 fstests: add GlusterFS support", so I think it doesn't matter to accept both formats for glusterfs too :) Thanks, Zorro -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 22, 2017 at 12:47 PM, Zorro Lang <zlang@redhat.com> wrote: > On Wed, Mar 22, 2017 at 11:08:27AM -0400, Amir Goldstein wrote: >> On Wed, Mar 22, 2017 at 10:06 AM, Zorro Lang <zlang@redhat.com> wrote: >> > On Wed, Mar 22, 2017 at 06:34:00AM -0400, Amir Goldstein wrote: >> >> On Wed, Mar 22, 2017 at 2:46 AM, Zorro Lang <zlang@redhat.com> wrote: >> >> > The format of glusterfs' TEST_DEV or SCRATCH_DEV is XXX:XXX or >> >> > XXX:/XXX, but xfstests can't accept the latter now. So change >> >> >> >> Why can't xfstest accept the latter? >> > >> > Because I use "\w:\w" in commit "4cbc0a0 fstests: add GlusterFS support". >> > More details please see below. >> > >> >> >> >> > the regular expression from "\w:\w" to ":/?", to accept more >> >> > glusterfs device format. >> >> > >> >> > Signed-off-by: Zorro Lang <zlang@redhat.com> >> >> > --- >> >> > >> >> > Hi, >> >> > >> >> > According to the feedback from glusterfs developer: >> >> > >> >> > "Yes, it would be good to have an optional "/" after the ":". It is not >> >> > required, but would probably help when someone runs the tests with the >> >> > "hostname:/volume" device format." >> >> > >> >> >> >> The developer also said >> >> "We mostly use the format of 192.168.1.1:/testvol, matching the way NFS" >> >> If we can enforce the more common format which xfstest already expects >> >> for nfs*|ceph) >> >> Why would we want to support another optional format and have a special >> >> test case for it? >> > >> > Niels is the developer of glusterfs' NFS module (sorry, I forgot its project >> > name), he test it likes test NFS: >> > mount -t nfs 192.168.1.1:/XXXX /mnt >> > >> > So he only can use XXXX:/XXX format, because xfstests limit that format >> > to mount NFS. >> > >> >> >> >> > Maybe there's a slash before the gluster volume name. This patch >> >> > try to support this format. >> >> > >> >> > Thanks, >> >> > Zorro >> >> > >> >> > common/rc | 4 ++-- >> >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> >> > >> >> > diff --git a/common/rc b/common/rc >> >> > index edfba5d..e1ab2c6 100644 >> >> > --- a/common/rc >> >> > +++ b/common/rc >> >> > @@ -1496,7 +1496,7 @@ _require_scratch_nocheck() >> >> > { >> >> > case "$FSTYP" in >> >> > glusterfs) >> >> > - echo $SCRATCH_DEV | grep -q "\w:\w" > /dev/null 2>&1 >> >> > + echo $SCRATCH_DEV | egrep -q ":/?" > /dev/null 2>&1 >> > >> > As I know, mount a glusrerfs always do like this: >> > mount -t glusterfs 192.169.1.1:testvol /mnt >> > or >> > mount -t glusterfs 192.169.1.1:/testvol /mnt >> > >> > So glusterfs can use XXXX:XXX or XXXX:/XXX format, but if mount a nfs, >> > we only can use XXXX:/XXX format. >> > >> > So that's why NFS use `grep -q ":/"`, but glusterfs use `egrep -q ":/?"` >> > >> >> I understand. >> What I am saying is that if glusterfs *can* use XXXX:/XXX >> and a file system tester will dedicate a specific mount for xfstests, >> so tester can use the NFS format for that mount and we can require that. >> >> Is that really a big deal for the tester? >> >> If we do that we will have less special cases for format of *_DEV >> and I think that is a gain. > > Hah, I already added many things for glusterfs in commit "4cbc0a0 > fstests: add GlusterFS support", so I think it doesn't matter to > accept both formats for glusterfs too :) > Sure. fine by me. Was just a suggestion. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/common/rc b/common/rc index edfba5d..e1ab2c6 100644 --- a/common/rc +++ b/common/rc @@ -1496,7 +1496,7 @@ _require_scratch_nocheck() { case "$FSTYP" in glusterfs) - echo $SCRATCH_DEV | grep -q "\w:\w" > /dev/null 2>&1 + echo $SCRATCH_DEV | egrep -q ":/?" > /dev/null 2>&1 if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then _notrun "this test requires a valid \$SCRATCH_DEV" fi @@ -1584,7 +1584,7 @@ _require_test() { case "$FSTYP" in glusterfs) - echo $TEST_DEV | grep -q "\w:\w" > /dev/null 2>&1 + echo $TEST_DEV | egrep -q ":/?" > /dev/null 2>&1 if [ -z "$TEST_DEV" -o "$?" != "0" ]; then _notrun "this test requires a valid \$TEST_DEV" fi
The format of glusterfs' TEST_DEV or SCRATCH_DEV is XXX:XXX or XXX:/XXX, but xfstests can't accept the latter now. So change the regular expression from "\w:\w" to ":/?", to accept more glusterfs device format. Signed-off-by: Zorro Lang <zlang@redhat.com> --- Hi, According to the feedback from glusterfs developer: "Yes, it would be good to have an optional "/" after the ":". It is not required, but would probably help when someone runs the tests with the "hostname:/volume" device format." Maybe there's a slash before the gluster volume name. This patch try to support this format. Thanks, Zorro common/rc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)