| Message ID | 20200715201253.171356-2-billodo@redhat.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfsprogs: xfs_quota error message and state reporting improvement | expand |
On Wed, Jul 15, 2020 at 03:12:51PM -0500, Bill O'Donnell wrote: > Make the error messages for rudimentary xfs_quota commands > (off, enable, disable) more user friendly, instead of the > terse sys error outputs. > > Signed-off-by: Bill O'Donnell <billodo@redhat.com> Hmmm... where are the underlying ioctls documented, anyways? Can we please get that done? (Note that ENOSYS can also mean that the fs didn't register any quota operations, but xfs always does so meh.) Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > quota/state.c | 32 ++++++++++++++++++++++++++------ > 1 file changed, 26 insertions(+), 6 deletions(-) > > diff --git a/quota/state.c b/quota/state.c > index 8f9718f1..7a595fc6 100644 > --- a/quota/state.c > +++ b/quota/state.c > @@ -306,8 +306,16 @@ enable_enforcement( > return; > } > dir = mount->fs_name; > - if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) > - perror("XFS_QUOTAON"); > + if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) { > + if (errno == EEXIST) > + fprintf(stderr, > + _("Quota enforcement already enabled.\n")); > + else if (errno == EINVAL || errno == ENOSYS) > + fprintf(stderr, > + _("Can't enable enforcement when quota off.\n")); > + else > + perror("XFS_QUOTAON"); > + } > else if (flags & VERBOSE_FLAG) > state_quotafile_mount(stdout, type, mount, flags); > } > @@ -328,8 +336,16 @@ disable_enforcement( > return; > } > dir = mount->fs_name; > - if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) > - perror("XFS_QUOTAOFF"); > + if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) { > + if (errno == EEXIST) > + fprintf(stderr, > + _("Quota enforcement already disabled.\n")); > + else if (errno == EINVAL || errno == ENOSYS) > + fprintf(stderr, > + _("Can't disable enforcement when quota off.\n")); > + else > + perror("XFS_QUOTAOFF"); > + } > else if (flags & VERBOSE_FLAG) > state_quotafile_mount(stdout, type, mount, flags); > } > @@ -350,8 +366,12 @@ quotaoff( > return; > } > dir = mount->fs_name; > - if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) > - perror("XFS_QUOTAOFF"); > + if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) { > + if (errno == EEXIST || errno == ENOSYS) > + fprintf(stderr, _("Quota already off.\n")); > + else > + perror("XFS_QUOTAOFF"); > + } > else if (flags & VERBOSE_FLAG) > state_quotafile_mount(stdout, type, mount, flags); > } > -- > 2.26.2 >
On Wed, Jul 15, 2020 at 03:12:51PM -0500, Bill O'Donnell wrote: > Make the error messages for rudimentary xfs_quota commands > (off, enable, disable) more user friendly, instead of the > terse sys error outputs. > > Signed-off-by: Bill O'Donnell <billodo@redhat.com> I think we should have one helper with the error message instead of duplicating them three times.
On Tue, Jul 21, 2020 at 04:04:04PM +0100, Christoph Hellwig wrote: > On Wed, Jul 15, 2020 at 03:12:51PM -0500, Bill O'Donnell wrote: > > Make the error messages for rudimentary xfs_quota commands > > (off, enable, disable) more user friendly, instead of the > > terse sys error outputs. > > > > Signed-off-by: Bill O'Donnell <billodo@redhat.com> > > I think we should have one helper with the error message > instead of duplicating them three times. > Except that the error messages are different depending on the context, so crafting a helper function that recognizes the context seems to offer diminishing return AFAICT. Thanks- Bill
diff --git a/quota/state.c b/quota/state.c index 8f9718f1..7a595fc6 100644 --- a/quota/state.c +++ b/quota/state.c @@ -306,8 +306,16 @@ enable_enforcement( return; } dir = mount->fs_name; - if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) - perror("XFS_QUOTAON"); + if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) { + if (errno == EEXIST) + fprintf(stderr, + _("Quota enforcement already enabled.\n")); + else if (errno == EINVAL || errno == ENOSYS) + fprintf(stderr, + _("Can't enable enforcement when quota off.\n")); + else + perror("XFS_QUOTAON"); + } else if (flags & VERBOSE_FLAG) state_quotafile_mount(stdout, type, mount, flags); } @@ -328,8 +336,16 @@ disable_enforcement( return; } dir = mount->fs_name; - if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) - perror("XFS_QUOTAOFF"); + if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) { + if (errno == EEXIST) + fprintf(stderr, + _("Quota enforcement already disabled.\n")); + else if (errno == EINVAL || errno == ENOSYS) + fprintf(stderr, + _("Can't disable enforcement when quota off.\n")); + else + perror("XFS_QUOTAOFF"); + } else if (flags & VERBOSE_FLAG) state_quotafile_mount(stdout, type, mount, flags); } @@ -350,8 +366,12 @@ quotaoff( return; } dir = mount->fs_name; - if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) - perror("XFS_QUOTAOFF"); + if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) { + if (errno == EEXIST || errno == ENOSYS) + fprintf(stderr, _("Quota already off.\n")); + else + perror("XFS_QUOTAOFF"); + } else if (flags & VERBOSE_FLAG) state_quotafile_mount(stdout, type, mount, flags); }
Make the error messages for rudimentary xfs_quota commands (off, enable, disable) more user friendly, instead of the terse sys error outputs. Signed-off-by: Bill O'Donnell <billodo@redhat.com> --- quota/state.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)