| Message ID | 1606124332-22100-1-git-send-email-kaixuxia@tencent.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: show the proper user quota options | expand |
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) >
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 >
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
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 >>
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
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 >
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 >>
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)