Page MenuHomeFreeBSD

growfs: make exit codes more consistent
ClosedPublic

Authored by freebsd_igalic.co on Nov 10 2020, 8:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 5, 2:53 AM
Unknown Object (File)
Fri, Apr 5, 1:58 AM
Unknown Object (File)
Tue, Apr 2, 12:24 PM
Unknown Object (File)
Fri, Mar 22, 3:14 PM
Unknown Object (File)
Fri, Mar 22, 10:00 AM
Unknown Object (File)
Tue, Mar 19, 5:42 PM
Unknown Object (File)
Mar 13 2024, 1:32 AM
Unknown Object (File)
Mar 9 2024, 12:53 PM

Details

Summary

We have overused err(1), so it's hard to distinguish when an error is
very, very serious, and when it's just a user-error, or even harmless.

This patch changes the current behaviour to distinguish between the following three:

1 for usage errors
2 for user-recoverable errors
3 for os-errors, most of which cannot be recovered.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I think this is a nice idea, but we should probably avoid using sysexits(3) for it (see https://reviews.freebsd.org/D27176). Also, it would be nice to update the manual page and explain the errors there (but it could be done in a separate commit probably).

oshogbo added subscribers: jilles, oshogbo.

What was motivation for this? Do you have any use case for that?

This is a delicate topic I think, there are some tools that return different exit codes I think the bsdgrep(1) is such example.
If you have a good motivation for this LGTM and I can commit this.

@jilles Whats your opinion about that?

What was motivation for this? Do you have any use case for that?

the motivation was a patch I was working on for cloud-init, https://github.com/canonical/cloud-init/pull/655 where i had to resort to exception based error handling, because so many,
vastly different error scenarios in growfs exit with 1

at least one of these could barely be called an error scenario at all (the case where growfs has nothing to do. especially when called with -N)

This is a delicate topic I think, there are some tools that return different exit codes I think the bsdgrep(1) is such example.
If you have a good motivation for this LGTM and I can commit this.

after @0mp's comment about https://reviews.freebsd.org/D27176 i would much rather wait for that to land and then revisit this

My suggestion is to change all the exit statuses that were changed to sysexits codes here to 2 instead.

"requested size %s is not larger than the current filesystem size %s" remains at 1 so that it can be checked for.

This results in a pattern much like grep(1) and test(1): the exit status is 0 if the file system has been or would be (with -N) enlarged, 1 if the file system is already at its maximum size and >1 if an error occurred. In practice, the ">1" is always 2, but this allows future expansion; scripts should check for the value 1, not for the value 2.

Please add this to the man page as well.

My suggestion is to change all the exit statuses that were changed to sysexits codes here to 2 instead.

"requested size %s is not larger than the current filesystem size %s" remains at 1 so that it can be checked for.

This results in a pattern much like grep(1) and test(1): the exit status is 0 if the file system has been or would be (with -N) enlarged, 1 if the file system is already at its maximum size and >1 if an error occurred. In practice, the ">1" is always 2, but this allows future expansion; scripts should check for the value 1, not for the value 2.

i can see one other class of error: recoverable error, caused by bogus input. the wrong size, unrecognised extension used on the size,
or heck even, wrong partition (or inability to find the correct device, or supplying a non-UFS.

all of these are recoverable, by correcting the input. however, they are not recoverable in scripts, as they are mostly human errors

OS errors, OTOH, are almost never recoverable without a major operation (running out of RAM or space, etc etc)

Please add this to the man page as well.

that's the plan. as soon as we have something solid.

I'd also like to continue this work in other tools that you find need it

freebsd_igalic.co edited the summary of this revision. (Show Details)

do *not* use sysexits, and document what we've done.

This revision is now accepted and ready to land.Dec 5 2020, 2:44 PM

@oshogbo, do you still want to commit this patch? We may want to set Relnotes: yes to give a heads-up to the users.

emaste added inline comments.
sbin/growfs/growfs.8
93

Do you mean ≥ 1 (or > 0) instead of > 1?

freebsd_igalic.co retitled this revision from growfs: use sysexits in place of err/errc/errx(1) to growfs: make exit codes more consistent.

update patch which was left untouched since 2021

This revision now requires review to proceed.Aug 16 2023, 1:20 PM

Manual page nits fixable on commit if nothing else requires an updated diff.

sbin/growfs/growfs.8
95
96

MW and AHD both give this spelling first, although they also accept "signalled". So change here for consistency with the next line.

97
This revision is now accepted and ready to land.Aug 24 2023, 5:26 PM
This revision now requires review to proceed.Oct 3 2023, 12:41 PM
des requested changes to this revision.Oct 3 2023, 12:46 PM
des added a subscriber: des.
des added inline comments.
sbin/growfs/growfs.8
94
This revision now requires changes to proceed.Oct 3 2023, 12:46 PM

What are the "harmless errors"? If it's harmless it shouldn't be an error and shouldn't exit with nonzero.

freebsd_igalic.co marked an inline comment as done.
des requested changes to this revision.Oct 3 2023, 1:23 PM
des added inline comments.
sbin/growfs/growfs.8
94

As far as I can tell, there are no such cases in the code. If the filesystem is already the requested size we exit with 0. The only case where we exit with 1 is usage().

This revision now requires changes to proceed.Oct 3 2023, 1:23 PM

there are no harmless errors (yet)

This revision is now accepted and ready to land.Oct 3 2023, 2:10 PM

s/signaled with/indicated by/g

also helps with spelling signaled vs signalled

This revision now requires review to proceed.Oct 3 2023, 2:12 PM
This revision is now accepted and ready to land.Oct 3 2023, 2:13 PM

I suggest changing the commit message to:

1 for usage errors
2 for recoverable errors
3 or higher for unrecoverable errors

This revision now requires review to proceed.Oct 4 2023, 10:24 AM
This revision is now accepted and ready to land.Oct 5 2023, 6:24 PM
This revision was automatically updated to reflect the committed changes.