| Message ID | 20200622131319.7717-1-billodo@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] xfsprogs: xfs_quota command error message improvement | expand |
On Mon, Jun 22, 2020 at 08:13:19AM -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> Yay, another sharp edge smoothed down, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > > v2: enable internationalization and capitalize new message strings. > > quota/state.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/quota/state.c b/quota/state.c > index 8f9718f1..7c9fe517 100644 > --- a/quota/state.c > +++ b/quota/state.c > @@ -306,8 +306,15 @@ 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) > + fprintf(stderr, > + _("Can't enable when quotas are off.\n")); > + else > + perror("XFS_QUOTAON"); > + } > else if (flags & VERBOSE_FLAG) > state_quotafile_mount(stdout, type, mount, flags); > } > @@ -328,8 +335,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) > + fprintf(stderr, > + _("Can't disable when quotas are off.\n")); > + else > + perror("XFS_QUOTAOFF"); > + } > else if (flags & VERBOSE_FLAG) > state_quotafile_mount(stdout, type, mount, flags); > } > @@ -350,8 +365,15 @@ 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) > + fprintf(stderr, _("Quota already off.\n")); > + else if (errno == EINVAL) > + fprintf(stderr, > + _("Can't disable when quotas are off.\n")); > + else > + perror("XFS_QUOTAOFF"); > + } > else if (flags & VERBOSE_FLAG) > state_quotafile_mount(stdout, type, mount, flags); > } > -- > 2.26.2 >
On 6/22/20 8:13 AM, Bill O'Donnell wrote: > @@ -350,8 +365,15 @@ 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) > + fprintf(stderr, _("Quota already off.\n")); > + else if (errno == EINVAL) > + fprintf(stderr, > + _("Can't disable when quotas are off.\n")); Is this the right message here? We get here from off_f(), which disables enforcement and accounting, so I'm not sure "can't disable" makes sense if "disable" means "disable enforcement" as it did in disable_enforcement()...? (IOWs, have can you provoke EINVAL? How? Sorry, this just kind of jumps out at me because "can't disable" seems a little out of place in quotaoff() so I want to double check.) Thanks, -Eric
On Tue, Jun 23, 2020 at 05:30:38PM -0500, Eric Sandeen wrote: > On 6/22/20 8:13 AM, Bill O'Donnell wrote: > > @@ -350,8 +365,15 @@ 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) > > + fprintf(stderr, _("Quota already off.\n")); > > + else if (errno == EINVAL) > > + fprintf(stderr, > > + _("Can't disable when quotas are off.\n")); > > Is this the right message here? We get here from off_f(), which disables > enforcement and accounting, so I'm not sure "can't disable" makes sense > if "disable" means "disable enforcement" as it did in disable_enforcement()...? > > (IOWs, have can you provoke EINVAL? How? Sorry, this just kind of jumps out > at me because "can't disable" seems a little out of place in quotaoff() so > I want to double check.) You're right. It was a copy/paste from the disable case. You also have a good point in that I don't think EINVAL is ever provoked for the OFF case. I'll recheck it. Thanks- Bill > > Thanks, > -Eric >
diff --git a/quota/state.c b/quota/state.c index 8f9718f1..7c9fe517 100644 --- a/quota/state.c +++ b/quota/state.c @@ -306,8 +306,15 @@ 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) + fprintf(stderr, + _("Can't enable when quotas are off.\n")); + else + perror("XFS_QUOTAON"); + } else if (flags & VERBOSE_FLAG) state_quotafile_mount(stdout, type, mount, flags); } @@ -328,8 +335,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) + fprintf(stderr, + _("Can't disable when quotas are off.\n")); + else + perror("XFS_QUOTAOFF"); + } else if (flags & VERBOSE_FLAG) state_quotafile_mount(stdout, type, mount, flags); } @@ -350,8 +365,15 @@ 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) + fprintf(stderr, _("Quota already off.\n")); + else if (errno == EINVAL) + fprintf(stderr, + _("Can't disable when quotas are 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> --- v2: enable internationalization and capitalize new message strings. quota/state.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-)