linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs: show the proper user quota options
@ 2020-11-23  9:38 xiakaixu1987
  2020-11-23 18:36 ` Eric Sandeen
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: xiakaixu1987 @ 2020-11-23  9:38 UTC (permalink / raw)
  To: linux-xfs; +Cc: darrick.wong, Kaixu Xia

From: Kaixu Xia <kaixuxia@tencent.com>

The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
seems wrong, Fix it and show proper options.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
 fs/xfs/xfs_super.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e3e229e52512..5ebd6cdc44a7 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -199,10 +199,12 @@ xfs_fs_show_options(
 		seq_printf(m, ",swidth=%d",
 				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
 
-	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
-		seq_puts(m, ",usrquota");
-	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
-		seq_puts(m, ",uqnoenforce");
+	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
+		if (mp->m_qflags & XFS_UQUOTA_ENFD)
+			seq_puts(m, ",usrquota");
+		else
+			seq_puts(m, ",uqnoenforce");
+	}
 
 	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
 		if (mp->m_qflags & XFS_PQUOTA_ENFD)
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-11-23  9:38 [PATCH] xfs: show the proper user quota options xiakaixu1987
@ 2020-11-23 18:36 ` Eric Sandeen
  2020-11-24  1:26   ` Eric Sandeen
  2020-11-24  0:30 ` Darrick J. Wong
  2020-12-06 23:12 ` Darrick J. Wong
  2 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2020-11-23 18:36 UTC (permalink / raw)
  To: xiakaixu1987, linux-xfs; +Cc: darrick.wong, Kaixu Xia

On 11/23/20 3:38 AM, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
> seems wrong, Fix it and show proper options.

I'm failing to see the difference in the logic here.  Under the current
code, what combination of flags yields the wrong string, and what does
this patch change in that respect?

Thanks,
-Eric

> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_super.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e3e229e52512..5ebd6cdc44a7 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -199,10 +199,12 @@ xfs_fs_show_options(
>  		seq_printf(m, ",swidth=%d",
>  				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>  
> -	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
> -		seq_puts(m, ",usrquota");
> -	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> -		seq_puts(m, ",uqnoenforce");
> +	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
> +		if (mp->m_qflags & XFS_UQUOTA_ENFD)
> +			seq_puts(m, ",usrquota");
> +		else
> +			seq_puts(m, ",uqnoenforce");
> +	}
>  
>  	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
>  		if (mp->m_qflags & XFS_PQUOTA_ENFD)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-11-23  9:38 [PATCH] xfs: show the proper user quota options xiakaixu1987
  2020-11-23 18:36 ` Eric Sandeen
@ 2020-11-24  0:30 ` Darrick J. Wong
  2020-11-24  8:37   ` kaixuxia
  2020-12-06 23:12 ` Darrick J. Wong
  2 siblings, 1 reply; 8+ messages in thread
From: Darrick J. Wong @ 2020-11-24  0:30 UTC (permalink / raw)
  To: xiakaixu1987; +Cc: linux-xfs, Kaixu Xia

On Mon, Nov 23, 2020 at 05:38:52PM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
> seems wrong, Fix it and show proper options.

This needs a regression test case to make sure that quota mount options
passed in ==> quota options in /proc/mounts, wouldn't you say? ;)

--D

> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_super.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e3e229e52512..5ebd6cdc44a7 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -199,10 +199,12 @@ xfs_fs_show_options(
>  		seq_printf(m, ",swidth=%d",
>  				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>  
> -	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
> -		seq_puts(m, ",usrquota");
> -	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> -		seq_puts(m, ",uqnoenforce");
> +	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
> +		if (mp->m_qflags & XFS_UQUOTA_ENFD)
> +			seq_puts(m, ",usrquota");
> +		else
> +			seq_puts(m, ",uqnoenforce");
> +	}
>  
>  	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
>  		if (mp->m_qflags & XFS_PQUOTA_ENFD)
> -- 
> 2.20.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-11-23 18:36 ` Eric Sandeen
@ 2020-11-24  1:26   ` Eric Sandeen
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2020-11-24  1:26 UTC (permalink / raw)
  To: xiakaixu1987, linux-xfs; +Cc: darrick.wong, Kaixu Xia



On 11/23/20 12:36 PM, Eric Sandeen wrote:
> On 11/23/20 3:38 AM, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
>> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
>> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
>> seems wrong, Fix it and show proper options.
> 
> I'm failing to see the difference in the logic here.  Under the current
> code, what combination of flags yields the wrong string, and what does
> this patch change in that respect?

<djwong pointed out what I missed>

But I guess that just emphasizes the need for a test :)

Thanks,
-Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-11-24  0:30 ` Darrick J. Wong
@ 2020-11-24  8:37   ` kaixuxia
  2020-11-24 16:50     ` Darrick J. Wong
  0 siblings, 1 reply; 8+ messages in thread
From: kaixuxia @ 2020-11-24  8:37 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, Kaixu Xia



On 2020/11/24 8:30, Darrick J. Wong wrote:
> On Mon, Nov 23, 2020 at 05:38:52PM +0800, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
>> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
>> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
>> seems wrong, Fix it and show proper options.
> 
> This needs a regression test case to make sure that quota mount options
> passed in ==> quota options in /proc/mounts, wouldn't you say? ;)

Hi Darrick,

The simple test case as follows:

Before the patch:
 # mount -o uqnoenforce /dev/vdc1 /data1
 # cat /proc/mounts | grep xfs
/dev/vdc1 /data1 xfs rw,relatime,attr2,inode64,logbufs=8,logbsize=32k,usrquota 0 0

After the patch:
 # mount -o uqnoenforce /dev/vdc1 /data1
 # cat /proc/mounts | grep xfs
/dev/vdc1 /data1 xfs rw,relatime,attr2,inode64,logbufs=8,logbsize=32k,uqnoenforce 0 0

I'm not sure if a xfstest case is needed:)

Thanks,
Kaixu

> 
> --D
> 
>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
>> ---
>>  fs/xfs/xfs_super.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>> index e3e229e52512..5ebd6cdc44a7 100644
>> --- a/fs/xfs/xfs_super.c
>> +++ b/fs/xfs/xfs_super.c
>> @@ -199,10 +199,12 @@ xfs_fs_show_options(
>>  		seq_printf(m, ",swidth=%d",
>>  				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>>  
>> -	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
>> -		seq_puts(m, ",usrquota");
>> -	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
>> -		seq_puts(m, ",uqnoenforce");
>> +	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
>> +		if (mp->m_qflags & XFS_UQUOTA_ENFD)
>> +			seq_puts(m, ",usrquota");
>> +		else
>> +			seq_puts(m, ",uqnoenforce");
>> +	}
>>  
>>  	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
>>  		if (mp->m_qflags & XFS_PQUOTA_ENFD)
>> -- 
>> 2.20.0
>>

-- 
kaixuxia

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-11-24  8:37   ` kaixuxia
@ 2020-11-24 16:50     ` Darrick J. Wong
  0 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2020-11-24 16:50 UTC (permalink / raw)
  To: kaixuxia; +Cc: linux-xfs, Kaixu Xia

On Tue, Nov 24, 2020 at 04:37:07PM +0800, kaixuxia wrote:
> 
> 
> On 2020/11/24 8:30, Darrick J. Wong wrote:
> > On Mon, Nov 23, 2020 at 05:38:52PM +0800, xiakaixu1987@gmail.com wrote:
> >> From: Kaixu Xia <kaixuxia@tencent.com>
> >>
> >> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
> >> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
> >> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
> >> seems wrong, Fix it and show proper options.
> > 
> > This needs a regression test case to make sure that quota mount options
> > passed in ==> quota options in /proc/mounts, wouldn't you say? ;)
> 
> Hi Darrick,
> 
> The simple test case as follows:
> 
> Before the patch:
>  # mount -o uqnoenforce /dev/vdc1 /data1
>  # cat /proc/mounts | grep xfs
> /dev/vdc1 /data1 xfs rw,relatime,attr2,inode64,logbufs=8,logbsize=32k,usrquota 0 0
> 
> After the patch:
>  # mount -o uqnoenforce /dev/vdc1 /data1
>  # cat /proc/mounts | grep xfs
> /dev/vdc1 /data1 xfs rw,relatime,attr2,inode64,logbufs=8,logbsize=32k,uqnoenforce 0 0
> 
> I'm not sure if a xfstest case is needed:)

It's been broken for a decade and nobody noticed.

YES IT IS.

--D

> 
> Thanks,
> Kaixu
> 
> > 
> > --D
> > 
> >> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> >> ---
> >>  fs/xfs/xfs_super.c | 10 ++++++----
> >>  1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> >> index e3e229e52512..5ebd6cdc44a7 100644
> >> --- a/fs/xfs/xfs_super.c
> >> +++ b/fs/xfs/xfs_super.c
> >> @@ -199,10 +199,12 @@ xfs_fs_show_options(
> >>  		seq_printf(m, ",swidth=%d",
> >>  				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
> >>  
> >> -	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
> >> -		seq_puts(m, ",usrquota");
> >> -	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> >> -		seq_puts(m, ",uqnoenforce");
> >> +	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
> >> +		if (mp->m_qflags & XFS_UQUOTA_ENFD)
> >> +			seq_puts(m, ",usrquota");
> >> +		else
> >> +			seq_puts(m, ",uqnoenforce");
> >> +	}
> >>  
> >>  	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
> >>  		if (mp->m_qflags & XFS_PQUOTA_ENFD)
> >> -- 
> >> 2.20.0
> >>
> 
> -- 
> kaixuxia

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-11-23  9:38 [PATCH] xfs: show the proper user quota options xiakaixu1987
  2020-11-23 18:36 ` Eric Sandeen
  2020-11-24  0:30 ` Darrick J. Wong
@ 2020-12-06 23:12 ` Darrick J. Wong
  2020-12-07  3:13   ` kaixuxia
  2 siblings, 1 reply; 8+ messages in thread
From: Darrick J. Wong @ 2020-12-06 23:12 UTC (permalink / raw)
  To: xiakaixu1987; +Cc: linux-xfs, Kaixu Xia

On Mon, Nov 23, 2020 at 05:38:52PM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
> seems wrong, Fix it and show proper options.
> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>

FWIW this causes a regression in xfs/513 since mount option uqnoenforce
no longer causes 'usrquota' to be emitted in /proc/mounts.  Do you have
a patch to fix fstests?

--D

> ---
>  fs/xfs/xfs_super.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e3e229e52512..5ebd6cdc44a7 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -199,10 +199,12 @@ xfs_fs_show_options(
>  		seq_printf(m, ",swidth=%d",
>  				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>  
> -	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
> -		seq_puts(m, ",usrquota");
> -	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> -		seq_puts(m, ",uqnoenforce");
> +	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
> +		if (mp->m_qflags & XFS_UQUOTA_ENFD)
> +			seq_puts(m, ",usrquota");
> +		else
> +			seq_puts(m, ",uqnoenforce");
> +	}
>  
>  	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
>  		if (mp->m_qflags & XFS_PQUOTA_ENFD)
> -- 
> 2.20.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: show the proper user quota options
  2020-12-06 23:12 ` Darrick J. Wong
@ 2020-12-07  3:13   ` kaixuxia
  0 siblings, 0 replies; 8+ messages in thread
From: kaixuxia @ 2020-12-07  3:13 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, Kaixu Xia



On 2020/12/7 7:12, Darrick J. Wong wrote:
> On Mon, Nov 23, 2020 at 05:38:52PM +0800, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> The quota option 'usrquota' should be shown if both the XFS_UQUOTA_ACCT
>> and XFS_UQUOTA_ENFD flags are set. The option 'uqnoenforce' should be
>> shown when only the XFS_UQUOTA_ACCT flag is set. The current code logic
>> seems wrong, Fix it and show proper options.
>>
>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> 
> FWIW this causes a regression in xfs/513 since mount option uqnoenforce
> no longer causes 'usrquota' to be emitted in /proc/mounts.  Do you have
> a patch to fix fstests?

Yeah, I'll send the patches to fix the regression in xfs/513 and add the
xfstest case for this bug ASAP:)

Thanks,
Kaixu
> 
> --D
> 
>> ---
>>  fs/xfs/xfs_super.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>> index e3e229e52512..5ebd6cdc44a7 100644
>> --- a/fs/xfs/xfs_super.c
>> +++ b/fs/xfs/xfs_super.c
>> @@ -199,10 +199,12 @@ xfs_fs_show_options(
>>  		seq_printf(m, ",swidth=%d",
>>  				(int)XFS_FSB_TO_BB(mp, mp->m_swidth));
>>  
>> -	if (mp->m_qflags & (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD))
>> -		seq_puts(m, ",usrquota");
>> -	else if (mp->m_qflags & XFS_UQUOTA_ACCT)
>> -		seq_puts(m, ",uqnoenforce");
>> +	if (mp->m_qflags & XFS_UQUOTA_ACCT) {
>> +		if (mp->m_qflags & XFS_UQUOTA_ENFD)
>> +			seq_puts(m, ",usrquota");
>> +		else
>> +			seq_puts(m, ",uqnoenforce");
>> +	}
>>  
>>  	if (mp->m_qflags & XFS_PQUOTA_ACCT) {
>>  		if (mp->m_qflags & XFS_PQUOTA_ENFD)
>> -- 
>> 2.20.0
>>

-- 
kaixuxia

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-07  3:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23  9:38 [PATCH] xfs: show the proper user quota options xiakaixu1987
2020-11-23 18:36 ` Eric Sandeen
2020-11-24  1:26   ` Eric Sandeen
2020-11-24  0:30 ` Darrick J. Wong
2020-11-24  8:37   ` kaixuxia
2020-11-24 16:50     ` Darrick J. Wong
2020-12-06 23:12 ` Darrick J. Wong
2020-12-07  3:13   ` kaixuxia

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).