[PATCH] generic/020: fix really long attr test failure for ceph

[PATCH] generic/020: fix really long attr test failure for ceph

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
itself will set the security.selinux extended attribute to MDS.
And it will also eat some space of the total size.

Fixes: https://tracker.ceph.com/issues/58742
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 tests/generic/020 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/generic/020 b/tests/generic/020
index be5cecad..594535b5 100755
--- a/tests/generic/020
+++ b/tests/generic/020
@@ -150,9 +150,11 @@ _attr_get_maxval_size()
 		# it imposes a maximum size for the full set of xattrs
 		# names+values, which by default is 64K.  Compute the maximum
 		# taking into account the already existing attributes
-		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
+		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
 			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
-		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
+		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
+			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
+		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))
 		;;
 	*)
 		# Assume max ~1 block of attrs
-- 
2.31.1

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Darrick J. Wong]

On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
> itself will set the security.selinux extended attribute to MDS.
> And it will also eat some space of the total size.
> 
> Fixes: https://tracker.ceph.com/issues/58742
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  tests/generic/020 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/020 b/tests/generic/020
> index be5cecad..594535b5 100755
> --- a/tests/generic/020
> +++ b/tests/generic/020
> @@ -150,9 +150,11 @@ _attr_get_maxval_size()
>  		# it imposes a maximum size for the full set of xattrs
>  		# names+values, which by default is 64K.  Compute the maximum
>  		# taking into account the already existing attributes
> -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>  			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
> -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
> +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
> +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
> +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))

If this is a ceph bug, then why is the change being applied to the
section for FSTYP=ext* ?  Why not create a case statement for ceph?

--D

>  		;;
>  	*)
>  		# Assume max ~1 block of attrs
> -- 
> 2.31.1
> 

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Xiubo Li]

On 18/02/2023 01:09, Darrick J. Wong wrote:
> On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
>> itself will set the security.selinux extended attribute to MDS.
>> And it will also eat some space of the total size.
>>
>> Fixes: https://tracker.ceph.com/issues/58742
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>   tests/generic/020 | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/generic/020 b/tests/generic/020
>> index be5cecad..594535b5 100755
>> --- a/tests/generic/020
>> +++ b/tests/generic/020
>> @@ -150,9 +150,11 @@ _attr_get_maxval_size()
>>   		# it imposes a maximum size for the full set of xattrs
>>   		# names+values, which by default is 64K.  Compute the maximum
>>   		# taking into account the already existing attributes
>> -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>> +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>   			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
>> -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
>> +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
>> +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
>> +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))
> If this is a ceph bug, then why is the change being applied to the
> section for FSTYP=ext* ?  Why not create a case statement for ceph?

Hi Darrick,

The above change is already in the "ceph)" section:

143         nfs)
144                 # NFS doesn't provide a way to find out the 
max_attrval_size for
145                 # the underlying filesystem, so just use the lowest 
value above.
146                 max_attrval_size=1024
147                 ;;
148         ceph)
149                 # CephFS does not have a maximum value for 
attributes.  Instead,
150                 # it imposes a maximum size for the full set of xattrs
151                 # names+values, which by default is 64K. Compute the 
maximum
152                 # taking into account the already existing attributes
153                 size=$(getfattr --dump -e hex $filename 2>/dev/null | \
154                         awk -F "=0x" '/^user/ {len += length($1) + 
length($2) / 2} END {print len}')
155                 selinux_size=$(getfattr -n 'security.selinux' --dump 
-e hex $filename 2>/dev/null | \
156                         awk -F "=0x" '/^security/ {len += length($1) 
+ length($2) / 2} END {print len}')
157                 max_attrval_size=$((65536 - $size - $selinux_size - 
$max_attrval_namelen))
158                 ;;
159         *)
160                 # Assume max ~1 block of attrs
161                 BLOCK_SIZE=`_get_block_size $TEST_DIR`
162                 # leave a little overhead
163                 let max_attrval_size=$BLOCK_SIZE-256

I didn't find the ext* section in _attr_get_maxval_size(). Did I miss 
something here ?

I have double checked it again by pulling the latest source code, no any 
change about this since my last pull yesterday.

Thanks

- Xiubo


> --D
>
>>   		;;
>>   	*)
>>   		# Assume max ~1 block of attrs
>> -- 
>> 2.31.1
>>
-- 
Best Regards,

Xiubo Li (李秀波)

Email: xiubli@redhat.com/xiubli@ibm.com
Slack: @Xiubo Li

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Zorro Lang]

On Fri, Feb 17, 2023 at 09:09:11AM -0800, Darrick J. Wong wrote:
> On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
> > From: Xiubo Li <xiubli@redhat.com>
> > 
> > If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
> > itself will set the security.selinux extended attribute to MDS.
> > And it will also eat some space of the total size.
> > 
> > Fixes: https://tracker.ceph.com/issues/58742
> > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > ---
> >  tests/generic/020 | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/generic/020 b/tests/generic/020
> > index be5cecad..594535b5 100755
> > --- a/tests/generic/020
> > +++ b/tests/generic/020
> > @@ -150,9 +150,11 @@ _attr_get_maxval_size()
> >  		# it imposes a maximum size for the full set of xattrs
> >  		# names+values, which by default is 64K.  Compute the maximum
> >  		# taking into account the already existing attributes
> > -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> > +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> >  			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
> > -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
> > +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
> > +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
> > +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))
> 
> If this is a ceph bug, then why is the change being applied to the
> section for FSTYP=ext* ?  Why not create a case statement for ceph?

Hi Darrick,

Looks like this change is in ceph section [1], did you hit any errors when
you merge it?

Thanks,
Zorro

[1]
_attr_get_maxval_size()
{
        local max_attrval_namelen="$1"
        local filename="$2"

        # Set max attr value size in bytes based on fs type
        case "$FSTYP" in
        ...
        ...
        ceph)
                # CephFS does not have a maximum value for attributes.  Instead,
                # it imposes a maximum size for the full set of xattrs
                # names+values, which by default is 64K.  Compute the maximum
                # taking into account the already existing attributes
====>           max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
                        awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
====>           max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))



> 
> --D
> 
> >  		;;
> >  	*)
> >  		# Assume max ~1 block of attrs
> > -- 
> > 2.31.1
> > 
> 

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Darrick J. Wong]

On Sat, Feb 18, 2023 at 02:04:36PM +0800, Zorro Lang wrote:
> On Fri, Feb 17, 2023 at 09:09:11AM -0800, Darrick J. Wong wrote:
> > On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
> > > From: Xiubo Li <xiubli@redhat.com>
> > > 
> > > If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
> > > itself will set the security.selinux extended attribute to MDS.
> > > And it will also eat some space of the total size.
> > > 
> > > Fixes: https://tracker.ceph.com/issues/58742
> > > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > > ---
> > >  tests/generic/020 | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/generic/020 b/tests/generic/020
> > > index be5cecad..594535b5 100755
> > > --- a/tests/generic/020
> > > +++ b/tests/generic/020
> > > @@ -150,9 +150,11 @@ _attr_get_maxval_size()
> > >  		# it imposes a maximum size for the full set of xattrs
> > >  		# names+values, which by default is 64K.  Compute the maximum
> > >  		# taking into account the already existing attributes
> > > -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> > > +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> > >  			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
> > > -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
> > > +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
> > > +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
> > > +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))
> > 
> > If this is a ceph bug, then why is the change being applied to the
> > section for FSTYP=ext* ?  Why not create a case statement for ceph?
> 
> Hi Darrick,
> 
> Looks like this change is in ceph section [1], did you hit any errors when
> you merge it?

ahahaa, diff tried to merge that hunk into _attr_get_max and not
_attr_get_maxval_size, and I didn't notice.  Question withdrawn
with apologies. :/

--D

> Thanks,
> Zorro
> 
> [1]
> _attr_get_maxval_size()
> {
>         local max_attrval_namelen="$1"
>         local filename="$2"
> 
>         # Set max attr value size in bytes based on fs type
>         case "$FSTYP" in
>         ...
>         ...
>         ceph)
>                 # CephFS does not have a maximum value for attributes.  Instead,
>                 # it imposes a maximum size for the full set of xattrs
>                 # names+values, which by default is 64K.  Compute the maximum
>                 # taking into account the already existing attributes
> ====>           max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>                         awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
> ====>           max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
> 
> 
> 
> > 
> > --D
> > 
> > >  		;;
> > >  	*)
> > >  		# Assume max ~1 block of attrs
> > > -- 
> > > 2.31.1
> > > 
> > 

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Zorro Lang]

On Tue, Feb 21, 2023 at 11:22:02AM -0800, Darrick J. Wong wrote:
> On Sat, Feb 18, 2023 at 02:04:36PM +0800, Zorro Lang wrote:
> > On Fri, Feb 17, 2023 at 09:09:11AM -0800, Darrick J. Wong wrote:
> > > On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
> > > > From: Xiubo Li <xiubli@redhat.com>
> > > > 
> > > > If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
> > > > itself will set the security.selinux extended attribute to MDS.
> > > > And it will also eat some space of the total size.
> > > > 
> > > > Fixes: https://tracker.ceph.com/issues/58742
> > > > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > > > ---
> > > >  tests/generic/020 | 6 ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tests/generic/020 b/tests/generic/020
> > > > index be5cecad..594535b5 100755
> > > > --- a/tests/generic/020
> > > > +++ b/tests/generic/020
> > > > @@ -150,9 +150,11 @@ _attr_get_maxval_size()
> > > >  		# it imposes a maximum size for the full set of xattrs
> > > >  		# names+values, which by default is 64K.  Compute the maximum
> > > >  		# taking into account the already existing attributes
> > > > -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> > > > +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> > > >  			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
> > > > -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
> > > > +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
> > > > +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
> > > > +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))

The max_attrval_size isn't a local variable, due to we need it to be global.
But the "size" and "selinux_size" look like not global variable, so better
to be *local*.

> > > 
> > > If this is a ceph bug, then why is the change being applied to the
> > > section for FSTYP=ext* ?  Why not create a case statement for ceph?
> > 
> > Hi Darrick,
> > 
> > Looks like this change is in ceph section [1], did you hit any errors when
> > you merge it?
> 
> ahahaa, diff tried to merge that hunk into _attr_get_max and not
> _attr_get_maxval_size, and I didn't notice.  Question withdrawn
> with apologies. :/

Never mind. If there's not objection from you or ceph list, I'll merge this
patch after it changes as above :)

Thanks,
Zorro

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > [1]
> > _attr_get_maxval_size()
> > {
> >         local max_attrval_namelen="$1"
> >         local filename="$2"
> > 
> >         # Set max attr value size in bytes based on fs type
> >         case "$FSTYP" in
> >         ...
> >         ...
> >         ceph)
> >                 # CephFS does not have a maximum value for attributes.  Instead,
> >                 # it imposes a maximum size for the full set of xattrs
> >                 # names+values, which by default is 64K.  Compute the maximum
> >                 # taking into account the already existing attributes
> > ====>           max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
> >                         awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
> > ====>           max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
> > 
> > 
> > 
> > > 
> > > --D
> > > 
> > > >  		;;
> > > >  	*)
> > > >  		# Assume max ~1 block of attrs
> > > > -- 
> > > > 2.31.1
> > > > 
> > > 
> 

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Xiubo Li]

On 22/02/2023 22:15, Zorro Lang wrote:
> On Tue, Feb 21, 2023 at 11:22:02AM -0800, Darrick J. Wong wrote:
>> On Sat, Feb 18, 2023 at 02:04:36PM +0800, Zorro Lang wrote:
>>> On Fri, Feb 17, 2023 at 09:09:11AM -0800, Darrick J. Wong wrote:
>>>> On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
>>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>>
>>>>> If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
>>>>> itself will set the security.selinux extended attribute to MDS.
>>>>> And it will also eat some space of the total size.
>>>>>
>>>>> Fixes: https://tracker.ceph.com/issues/58742
>>>>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>>>>> ---
>>>>>   tests/generic/020 | 6 ++++--
>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/tests/generic/020 b/tests/generic/020
>>>>> index be5cecad..594535b5 100755
>>>>> --- a/tests/generic/020
>>>>> +++ b/tests/generic/020
>>>>> @@ -150,9 +150,11 @@ _attr_get_maxval_size()
>>>>>   		# it imposes a maximum size for the full set of xattrs
>>>>>   		# names+values, which by default is 64K.  Compute the maximum
>>>>>   		# taking into account the already existing attributes
>>>>> -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>>>> +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>>>>   			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
>>>>> -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
>>>>> +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
>>>>> +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
>>>>> +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))
> The max_attrval_size isn't a local variable, due to we need it to be global.
> But the "size" and "selinux_size" look like not global variable, so better
> to be *local*.
>
>>>> If this is a ceph bug, then why is the change being applied to the
>>>> section for FSTYP=ext* ?  Why not create a case statement for ceph?
>>> Hi Darrick,
>>>
>>> Looks like this change is in ceph section [1], did you hit any errors when
>>> you merge it?
>> ahahaa, diff tried to merge that hunk into _attr_get_max and not
>> _attr_get_maxval_size, and I didn't notice.  Question withdrawn
>> with apologies. :/
> Never mind. If there's not objection from you or ceph list, I'll merge this
> patch after it changes as above :)

Sure. Thanks Zorro.

- Xiubo


> Thanks,
> Zorro
>
>> --D
>>
>>> Thanks,
>>> Zorro
>>>
>>> [1]
>>> _attr_get_maxval_size()
>>> {
>>>          local max_attrval_namelen="$1"
>>>          local filename="$2"
>>>
>>>          # Set max attr value size in bytes based on fs type
>>>          case "$FSTYP" in
>>>          ...
>>>          ...
>>>          ceph)
>>>                  # CephFS does not have a maximum value for attributes.  Instead,
>>>                  # it imposes a maximum size for the full set of xattrs
>>>                  # names+values, which by default is 64K.  Compute the maximum
>>>                  # taking into account the already existing attributes
>>> ====>           max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>>                          awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
>>> ====>           max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
>>>
>>>
>>>
>>>> --D
>>>>
>>>>>   		;;
>>>>>   	*)
>>>>>   		# Assume max ~1 block of attrs
>>>>> -- 
>>>>> 2.31.1
>>>>>
-- 
Best Regards,

Xiubo Li (李秀波)

Email: xiubli@redhat.com/xiubli@ibm.com
Slack: @Xiubo Li

Re: [PATCH] generic/020: fix really long attr test failure for ceph

[Xiubo Li]

On 22/02/2023 22:15, Zorro Lang wrote:
> On Tue, Feb 21, 2023 at 11:22:02AM -0800, Darrick J. Wong wrote:
>> On Sat, Feb 18, 2023 at 02:04:36PM +0800, Zorro Lang wrote:
>>> On Fri, Feb 17, 2023 at 09:09:11AM -0800, Darrick J. Wong wrote:
>>>> On Fri, Feb 17, 2023 at 08:45:58PM +0800, xiubli@redhat.com wrote:
>>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>>
>>>>> If the CONFIG_CEPH_FS_SECURITY_LABEL is enabled the kernel ceph
>>>>> itself will set the security.selinux extended attribute to MDS.
>>>>> And it will also eat some space of the total size.
>>>>>
>>>>> Fixes: https://tracker.ceph.com/issues/58742
>>>>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>>>>> ---
>>>>>   tests/generic/020 | 6 ++++--
>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/tests/generic/020 b/tests/generic/020
>>>>> index be5cecad..594535b5 100755
>>>>> --- a/tests/generic/020
>>>>> +++ b/tests/generic/020
>>>>> @@ -150,9 +150,11 @@ _attr_get_maxval_size()
>>>>>   		# it imposes a maximum size for the full set of xattrs
>>>>>   		# names+values, which by default is 64K.  Compute the maximum
>>>>>   		# taking into account the already existing attributes
>>>>> -		max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>>>> +		size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>>>>   			awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
>>>>> -		max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
>>>>> +		selinux_size=$(getfattr -n 'security.selinux' --dump -e hex $filename 2>/dev/null | \
>>>>> +			awk -F "=0x" '/^security/ {len += length($1) + length($2) / 2} END {print len}')
>>>>> +		max_attrval_size=$((65536 - $size - $selinux_size - $max_attrval_namelen))
> The max_attrval_size isn't a local variable, due to we need it to be global.
> But the "size" and "selinux_size" look like not global variable, so better
> to be *local*.
>
Sorry I missed this.

And now fixed it and sent out the V2.

Thanks

- Xiubo


>>>> If this is a ceph bug, then why is the change being applied to the
>>>> section for FSTYP=ext* ?  Why not create a case statement for ceph?
>>> Hi Darrick,
>>>
>>> Looks like this change is in ceph section [1], did you hit any errors when
>>> you merge it?
>> ahahaa, diff tried to merge that hunk into _attr_get_max and not
>> _attr_get_maxval_size, and I didn't notice.  Question withdrawn
>> with apologies. :/
> Never mind. If there's not objection from you or ceph list, I'll merge this
> patch after it changes as above :)
>
> Thanks,
> Zorro
>
>> --D
>>
>>> Thanks,
>>> Zorro
>>>
>>> [1]
>>> _attr_get_maxval_size()
>>> {
>>>          local max_attrval_namelen="$1"
>>>          local filename="$2"
>>>
>>>          # Set max attr value size in bytes based on fs type
>>>          case "$FSTYP" in
>>>          ...
>>>          ...
>>>          ceph)
>>>                  # CephFS does not have a maximum value for attributes.  Instead,
>>>                  # it imposes a maximum size for the full set of xattrs
>>>                  # names+values, which by default is 64K.  Compute the maximum
>>>                  # taking into account the already existing attributes
>>> ====>           max_attrval_size=$(getfattr --dump -e hex $filename 2>/dev/null | \
>>>                          awk -F "=0x" '/^user/ {len += length($1) + length($2) / 2} END {print len}')
>>> ====>           max_attrval_size=$((65536 - $max_attrval_size - $max_attrval_namelen))
>>>
>>>
>>>
>>>> --D
>>>>
>>>>>   		;;
>>>>>   	*)
>>>>>   		# Assume max ~1 block of attrs
>>>>> -- 
>>>>> 2.31.1
>>>>>
-- 
Best Regards,

Xiubo Li (李秀波)

Email: xiubli@redhat.com/xiubli@ibm.com
Slack: @Xiubo Li