Re: [PATCH] exportfs: Return non-zero exit value on error

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2309 bytes --]

On Tue, 22 Oct 2013 10:23:14 -0500 Tony Asleson <tasleson@redhat.com> wrote:

> On 10/21/2013 05:25 PM, NeilBrown wrote:
> > On Wed,  2 Oct 2013 18:29:44 -0500 Tony Asleson <tasleson@redhat.com> 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@redhat.com>
> > 
> > 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]