diff mbox

fstests: attr: add support for cephfs

Message ID 20170503105413.8314-1-lhenriques@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Luis Henriques May 3, 2017, 10:54 a.m. UTC 
Block size for cephfs is 4M, which makes generic/020 test fail as the
value for MAX_ATTRS and MAX_ATTRVAL_SIZE will be too high.  Restrict these
two variables to sane values for this FSTYP.

Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 common/attr | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eryu Guan May 4, 2017, 3:23 a.m. UTC | #1 
On Wed, May 03, 2017 at 11:54:13AM +0100, Luis Henriques wrote:
> Block size for cephfs is 4M, which makes generic/020 test fail as the
> value for MAX_ATTRS and MAX_ATTRVAL_SIZE will be too high.  Restrict these
> two variables to sane values for this FSTYP.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.com>

Thanks for the patch! I'd like some reviews from ceph developers on the
ceph MAX_ATTRS and MAX_ATTRVAL_SIZE numbers.

Thanks,
Eryu

> ---
>  common/attr | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/common/attr b/common/attr
> index ac139e618b2a..c840ce42a1fa 100644
> --- a/common/attr
> +++ b/common/attr
> @@ -254,26 +254,36 @@ _sort_getfattr_output()
>  }
>  
>  # set maximum total attr space based on fs type
> -if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "pvfs2" ]; then
> +case "$FSTYP" in
> +xfs|udf|pvfs2|ceph)
>  	MAX_ATTRS=1000
> -else # Assume max ~1 block of attrs
> +	;;
> +*)
> +	# Assume max ~1 block of attrs
>  	BLOCK_SIZE=`_get_block_size $TEST_DIR`
>  	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
>  	let MAX_ATTRS=$BLOCK_SIZE/40
> -fi
> +esac
>  
>  export MAX_ATTRS
>  
>  # Set max attr value size based on fs type
> -if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "btrfs" ]; then
> +case "$FSTYP" in
> +xfs|udf|btrfs)
>  	MAX_ATTRVAL_SIZE=64
> -elif [ "$FSTYP" == "pvfs2" ]; then
> +	;;
> +pvfs2)
>  	MAX_ATTRVAL_SIZE=8192
> -else # Assume max ~1 block of attrs
> +	;;
> +ceph)
> +	MAX_ATTRVAL_SIZE=65536
> +	;;
> +*)
> +	# Assume max ~1 block of attrs
>  	BLOCK_SIZE=`_get_block_size $TEST_DIR`
>  	# leave a little overhead
>  	let MAX_ATTRVAL_SIZE=$BLOCK_SIZE-256
> -fi
> +esac
>  
>  export MAX_ATTRVAL_SIZE
>  # make sure this script returns success
> --
> 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 ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yan, Zheng May 4, 2017, 3:44 a.m. UTC | #2 
On Thu, May 4, 2017 at 11:23 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Wed, May 03, 2017 at 11:54:13AM +0100, Luis Henriques wrote:
>> Block size for cephfs is 4M, which makes generic/020 test fail as the
>> value for MAX_ATTRS and MAX_ATTRVAL_SIZE will be too high.  Restrict these
>> two variables to sane values for this FSTYP.
>>
>> Signed-off-by: Luis Henriques <lhenriques@suse.com>
>
> Thanks for the patch! I'd like some reviews from ceph developers on the
> ceph MAX_ATTRS and MAX_ATTRVAL_SIZE numbers.
>

Currently we don't have any limitation on single xattr size and number
of xattrs. But single xattr size is limited to 65536 by kernel.

Regards
Yan, Zheng

> Thanks,
> Eryu
>
>> ---
>>  common/attr | 24 +++++++++++++++++-------
>>  1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/common/attr b/common/attr
>> index ac139e618b2a..c840ce42a1fa 100644
>> --- a/common/attr
>> +++ b/common/attr
>> @@ -254,26 +254,36 @@ _sort_getfattr_output()
>>  }
>>
>>  # set maximum total attr space based on fs type
>> -if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "pvfs2" ]; then
>> +case "$FSTYP" in
>> +xfs|udf|pvfs2|ceph)
>>       MAX_ATTRS=1000
>> -else # Assume max ~1 block of attrs
>> +     ;;
>> +*)
>> +     # Assume max ~1 block of attrs
>>       BLOCK_SIZE=`_get_block_size $TEST_DIR`
>>       # user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
>>       let MAX_ATTRS=$BLOCK_SIZE/40
>> -fi
>> +esac
>>
>>  export MAX_ATTRS
>>
>>  # Set max attr value size based on fs type
>> -if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "btrfs" ]; then
>> +case "$FSTYP" in
>> +xfs|udf|btrfs)
>>       MAX_ATTRVAL_SIZE=64
>> -elif [ "$FSTYP" == "pvfs2" ]; then
>> +     ;;
>> +pvfs2)
>>       MAX_ATTRVAL_SIZE=8192
>> -else # Assume max ~1 block of attrs
>> +     ;;
>> +ceph)
>> +     MAX_ATTRVAL_SIZE=65536
>> +     ;;
>> +*)
>> +     # Assume max ~1 block of attrs
>>       BLOCK_SIZE=`_get_block_size $TEST_DIR`
>>       # leave a little overhead
>>       let MAX_ATTRVAL_SIZE=$BLOCK_SIZE-256
>> -fi
>> +esac
>>
>>  export MAX_ATTRVAL_SIZE
>>  # make sure this script returns success
>> --
>> 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 ceph-devel" 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 ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan May 4, 2017, 5 a.m. UTC | #3 
On Thu, May 04, 2017 at 11:44:46AM +0800, Yan, Zheng wrote:
> On Thu, May 4, 2017 at 11:23 AM, Eryu Guan <eguan@redhat.com> wrote:
> > On Wed, May 03, 2017 at 11:54:13AM +0100, Luis Henriques wrote:
> >> Block size for cephfs is 4M, which makes generic/020 test fail as the
> >> value for MAX_ATTRS and MAX_ATTRVAL_SIZE will be too high.  Restrict these
> >> two variables to sane values for this FSTYP.
> >>
> >> Signed-off-by: Luis Henriques <lhenriques@suse.com>
> >
> > Thanks for the patch! I'd like some reviews from ceph developers on the
> > ceph MAX_ATTRS and MAX_ATTRVAL_SIZE numbers.
> >
> 
> Currently we don't have any limitation on single xattr size and number
> of xattrs. But single xattr size is limited to 65536 by kernel.

Thanks! Can I take it as a Reviewed-by tag?

Eryu
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luis Henriques May 4, 2017, 9:29 a.m. UTC | #4 
On Thu, May 04, 2017 at 11:44:46AM +0800, Yan, Zheng wrote:
> On Thu, May 4, 2017 at 11:23 AM, Eryu Guan <eguan@redhat.com> wrote:
> > On Wed, May 03, 2017 at 11:54:13AM +0100, Luis Henriques wrote:
> >> Block size for cephfs is 4M, which makes generic/020 test fail as the
> >> value for MAX_ATTRS and MAX_ATTRVAL_SIZE will be too high.  Restrict these
> >> two variables to sane values for this FSTYP.
> >>
> >> Signed-off-by: Luis Henriques <lhenriques@suse.com>
> >
> > Thanks for the patch! I'd like some reviews from ceph developers on the
> > ceph MAX_ATTRS and MAX_ATTRVAL_SIZE numbers.
> >
> 
> Currently we don't have any limitation on single xattr size and number
> of xattrs. But single xattr size is limited to 65536 by kernel.

Right, and the same limit is also applied when trying to list (listxattr)
more than XATTR_LIST_MAX bytes.  One of the failures in the generic/020
test was because it was getting a -E2BIG in getfattr.

Cheers,
--
Luís

> 
> Regards
> Yan, Zheng
> 
> > Thanks,
> > Eryu
> >
> >> ---
> >>  common/attr | 24 +++++++++++++++++-------
> >>  1 file changed, 17 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/common/attr b/common/attr
> >> index ac139e618b2a..c840ce42a1fa 100644
> >> --- a/common/attr
> >> +++ b/common/attr
> >> @@ -254,26 +254,36 @@ _sort_getfattr_output()
> >>  }
> >>
> >>  # set maximum total attr space based on fs type
> >> -if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "pvfs2" ]; then
> >> +case "$FSTYP" in
> >> +xfs|udf|pvfs2|ceph)
> >>       MAX_ATTRS=1000
> >> -else # Assume max ~1 block of attrs
> >> +     ;;
> >> +*)
> >> +     # Assume max ~1 block of attrs
> >>       BLOCK_SIZE=`_get_block_size $TEST_DIR`
> >>       # user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
> >>       let MAX_ATTRS=$BLOCK_SIZE/40
> >> -fi
> >> +esac
> >>
> >>  export MAX_ATTRS
> >>
> >>  # Set max attr value size based on fs type
> >> -if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "btrfs" ]; then
> >> +case "$FSTYP" in
> >> +xfs|udf|btrfs)
> >>       MAX_ATTRVAL_SIZE=64
> >> -elif [ "$FSTYP" == "pvfs2" ]; then
> >> +     ;;
> >> +pvfs2)
> >>       MAX_ATTRVAL_SIZE=8192
> >> -else # Assume max ~1 block of attrs
> >> +     ;;
> >> +ceph)
> >> +     MAX_ATTRVAL_SIZE=65536
> >> +     ;;
> >> +*)
> >> +     # Assume max ~1 block of attrs
> >>       BLOCK_SIZE=`_get_block_size $TEST_DIR`
> >>       # leave a little overhead
> >>       let MAX_ATTRVAL_SIZE=$BLOCK_SIZE-256
> >> -fi
> >> +esac
> >>
> >>  export MAX_ATTRVAL_SIZE
> >>  # make sure this script returns success
> >> --
> >> 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 ceph-devel" 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 ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/common/attr b/common/attr
index ac139e618b2a..c840ce42a1fa 100644
--- a/common/attr
+++ b/common/attr
@@ -254,26 +254,36 @@  _sort_getfattr_output()
 }
 
 # set maximum total attr space based on fs type
-if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "pvfs2" ]; then
+case "$FSTYP" in
+xfs|udf|pvfs2|ceph)
 	MAX_ATTRS=1000
-else # Assume max ~1 block of attrs
+	;;
+*)
+	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
 	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
 	let MAX_ATTRS=$BLOCK_SIZE/40
-fi
+esac
 
 export MAX_ATTRS
 
 # Set max attr value size based on fs type
-if [ "$FSTYP" == "xfs" -o "$FSTYP" == "udf" -o "$FSTYP" == "btrfs" ]; then
+case "$FSTYP" in
+xfs|udf|btrfs)
 	MAX_ATTRVAL_SIZE=64
-elif [ "$FSTYP" == "pvfs2" ]; then
+	;;
+pvfs2)
 	MAX_ATTRVAL_SIZE=8192
-else # Assume max ~1 block of attrs
+	;;
+ceph)
+	MAX_ATTRVAL_SIZE=65536
+	;;
+*)
+	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
 	# leave a little overhead
 	let MAX_ATTRVAL_SIZE=$BLOCK_SIZE-256
-fi
+esac
 
 export MAX_ATTRVAL_SIZE
 # make sure this script returns success