On Tue, 22 Oct 2013 10:23:14 -0500 Tony Asleson <tasleson@xxxxxxxxxx> wrote: > On 10/21/2013 05:25 PM, NeilBrown wrote: > > On Wed, 2 Oct 2013 18:29:44 -0500 Tony Asleson <tasleson@xxxxxxxxxx> wrote: > > > >> To improve error handling when scripting exportfs it's useful > >> to have non-zero exit codes when the requested operation did not > >> succeed. > >> > >> This patch also returns a non-zero exit code if you request to > >> unexport a non-existant share. > >> > >> Signed-off-by: Tony Asleson <tasleson@xxxxxxxxxx> > > > > This seems the have been forgotten, so maybe by replying to it someone will > > notice (hi Steve). > > > > Though I agree with the need for the patch, I don't much like it's shape. > > > > Why change exportfs and unexportfs to return a status? The status is only > > used to set export_errno, and they sometimes set export_errno anyway, so why > > not leave them returning void and just setting export_errno as needed? > > The reason I chose to return values was to make sure requested operation > actually completed requested operation. Unexporting a non-existent > export is not considered an error and returns no indication you did > absolutely nothing. Hi, thanks makes sense - I had missed that (even though you explained it in the patch description :-( ) With your patch, if asked to unexport something that wasn't exported it would not report any error, but would exit with an error status. Is that correct? I think I would rather have a message printed if there is an error. So would something like this (on top of my patch) address you need, or was there something else I missed? Thanks, NeilBrown diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c index 52fc03d..c9e12db 100644 --- a/utils/exportfs/exportfs.c +++ b/utils/exportfs/exportfs.c @@ -351,6 +351,7 @@ unexportfs(char *arg, int verbose) char *path; char *hname = arg; int htype; + int success = 0; if ((path = strchr(arg, ':')) != NULL) *path++ = '\0'; @@ -397,7 +398,10 @@ unexportfs(char *arg, int verbose) #endif exp->m_xtabent = 0; exp->m_mayexport = 0; + success = 1; } + if (!success) + xlog(L_ERROR, "Could not find %s to unexport.\n", arg); freeaddrinfo(ai); }
Attachment:
signature.asc
Description: PGP signature
