2014-11-14 18:05:28

by Henrique Martins

[permalink] [raw]
Subject: [PATCH 1/1] nfsd/exportfs: allow empty exports file

Attaching patch to nfs/exportfs to allow nfsd to start when
/etc/exports is empty, which broke with previous patch
(bugzilla 1115179).

Files changed:
- in export.c/export_read:
counts the number of good (resolvable) and bad
(unresolvable) volume entries and generates a (x)log
L_ERROR if no resolvable entries are exported AND there
are some unresolvable entries.

Built and tested on a Fedora 20 (fully updated) system.

(And yes, if I need to add the patch inline, my mailer may
screw up the indentation, blanks vs tabs.)

Signed-off-by: Henrique Martins <linux@xxxxxxxxxx>
---

diff -upN nfs-utils-1.3.0/support/export/export.c.orig
nfs-utils-1.3.0/support/export/export.c
--- nfs-utils-1.3.0/support/export/export.c.orig
2014-11-14 08:46:58.284175535 -0800
+++ nfs-utils-1.3.0/support/export/export.c 2014-11-14
08:47:52.079349910 -0800
@@ -76,7 +76,8 @@ export_read(char *fname)
struct exportent *eep;
nfs_export *exp;

- int volumes = 0;
+ int good = 0;
+ int bad = 0;

setexportent(fname, "r");
while ((eep = getexportent(0,1)) != NULL) {
@@ -84,13 +85,15 @@ export_read(char *fname)
if (!exp) {
exp = export_create(eep, 0);
if (exp)
- volumes++;
+ good++;
+ else
+ bad++;
}
else
warn_duplicated_exports(exp, eep);
}
endexportent();
- if (volumes == 0)
+ if (good == 0 && bad > 0)
xlog(L_ERROR, "No file systems exported!");
}


2014-11-17 13:09:44

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/1] nfsd/exportfs: allow empty exports file



On 11/14/2014 01:05 PM, Henrique Martins wrote:
> Attaching patch to nfs/exportfs to allow nfsd to start when
> /etc/exports is empty, which broke with previous patch
> (bugzilla 1115179).
>
> Files changed:
> - in export.c/export_read:
> counts the number of good (resolvable) and bad
> (unresolvable) volume entries and generates a (x)log
> L_ERROR if no resolvable entries are exported AND there
> are some unresolvable entries.
>
> Built and tested on a Fedora 20 (fully updated) system.
>
> (And yes, if I need to add the patch inline, my mailer may
> screw up the indentation, blanks vs tabs.)
>
> Signed-off-by: Henrique Martins <linux@xxxxxxxxxx>
> ---
>
> diff -upN nfs-utils-1.3.0/support/export/export.c.orig
> nfs-utils-1.3.0/support/export/export.c
> --- nfs-utils-1.3.0/support/export/export.c.orig
> 2014-11-14 08:46:58.284175535 -0800
> +++ nfs-utils-1.3.0/support/export/export.c 2014-11-14
> 08:47:52.079349910 -0800
> @@ -76,7 +76,8 @@ export_read(char *fname)
> struct exportent *eep;
> nfs_export *exp;
>
> - int volumes = 0;
> + int good = 0;
> + int bad = 0;
>
> setexportent(fname, "r");
> while ((eep = getexportent(0,1)) != NULL) {
> @@ -84,13 +85,15 @@ export_read(char *fname)
> if (!exp) {
> exp = export_create(eep, 0);
> if (exp)
> - volumes++;
> + good++;
> + else
> + bad++;
> }
> else
> warn_duplicated_exports(exp, eep);
> }
> endexportent();
> - if (volumes == 0)
> + if (good == 0 && bad > 0)
> xlog(L_ERROR, "No file systems exported!");
> }
>
The presidence has been set that having an empty export file
is not a problem. So I would rather change that xlog to be
a L_WARNING and only log it when the verbose is set.
Something similar to:

diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index bdea12b..92fb9eb 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
*
*/
void
-export_read(char *fname)
+export_read(char *fname, int verbose)
{
struct exportent *eep;
nfs_export *exp;
@@ -90,8 +90,8 @@ export_read(char *fname)
warn_duplicated_exports(exp, eep);
}
endexportent();
- if (volumes == 0)
- xlog(L_ERROR, "No file systems exported!");
+ if (volumes == 0 && verbose)
+ xlog(L_WARNING, "No file systems exported!");
}


steved.

2014-11-17 18:46:14

by Henrique Martins

[permalink] [raw]
Subject: Re: [PATCH 1/1] nfsd/exportfs: allow empty exports file

steved> The presidence has been set that having an empty
steved> export file is not a problem. So I would rather
steved> change that xlog to be a L_WARNING and only log it
steved> when the verbose is set.

The situation, PRECEDENCE, when I got involved was:
no entries in exports:
nothing logged, nfsd started
all good entries in exports
nothing logged, nfsd started
mixture of bad and good entries in exports
bad entries logged, error logged, nfsd aborted
all bad entries in exports
bad entries logged, error logged, nfsd aborted

My first patch overlooked that first condition, and made the
situation like below, changed from PRECEDENCE uppercased:
no entries in exports:
EMPTY LOGGED, NFSD ABORTED
all good entries in exports
nothing logged, nfsd started
mixture of bad and good entries in exports
bad entries logged, NFSD STARTED
all bad entries in exports
bad entries logged, error logged, nfsd aborted

Current patch makes it, changes from PRECEDENCE uppercased:
no entries in exports:
nothing logged, nfsd started
all good entries in exports
nothing logged, nfsd started
mixture of bad and good entries in exports
bad entries logged, NFSD STARTED
all bad entries in exports
bad entries logged, error logged, nfsd aborted

Your patch makes it, changes from PRECEDENCE uppercased:
no entries in exports:
WARNING LOGGED, nfsd started
all good entries in exports
nothing logged, nfsd started
mixture of bad and good entries in exports
bad entries logged, NFSD STARTED
all bad entries in exports
bad entries logged, NFSD STARTED

Your pick,

-- Henrique

2014-11-17 19:09:34

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/1] nfsd/exportfs: allow empty exports file

Please take a look at what I committed today to
see it it works in your environment. We'll work
on it if it does not...

steved.

On 11/17/2014 10:53 AM, Henrique Martins wrote:
> steved> The presidence has been set that having an empty
> steved> export file is not a problem. So I would rather
> steved> change that xlog to be a L_WARNING and only log it
> steved> when the verbose is set.
>
> The situation, PRECEDENCE, when I got involved was:
> no entries in exports:
> nothing logged, nfsd started
> all good entries in exports
> nothing logged, nfsd started
> mixture of bad and good entries in exports
> bad entries logged, error logged, nfsd aborted
> all bad entries in exports
> bad entries logged, error logged, nfsd aborted
>
> My first patch overlooked that first condition, and made the
> situation like below, changed from PRECEDENCE uppercased:
> no entries in exports:
> EMPTY LOGGED, NFSD ABORTED
> all good entries in exports
> nothing logged, nfsd started
> mixture of bad and good entries in exports
> bad entries logged, NFSD STARTED
> all bad entries in exports
> bad entries logged, error logged, nfsd aborted
>
> Current patch makes it, changes from PRECEDENCE uppercased:
> no entries in exports:
> nothing logged, nfsd started
> all good entries in exports
> nothing logged, nfsd started
> mixture of bad and good entries in exports
> bad entries logged, NFSD STARTED
> all bad entries in exports
> bad entries logged, error logged, nfsd aborted
>
> Your patch makes it, changes from PRECEDENCE uppercased:
> no entries in exports:
> WARNING LOGGED, nfsd started
> all good entries in exports
> nothing logged, nfsd started
> mixture of bad and good entries in exports
> bad entries logged, NFSD STARTED
> all bad entries in exports
> bad entries logged, NFSD STARTED
>
> Your pick,
>
> -- Henrique
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-11-17 20:27:37

by Henrique Martins

[permalink] [raw]
Subject: Re: [PATCH 1/1] nfsd/exportfs: allow empty exports file

> Please take a look at what I committed today to
> see it it works in your environment. We'll work
> on it if it does not...

Committed to?

-- Henrique

2014-11-17 21:31:20

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/1] nfsd/exportfs: allow empty exports file



On 11/17/2014 03:27 PM, Henrique Martins wrote:
>> Please take a look at what I committed today to
>> see it it works in your environment. We'll work
>> on it if it does not...
>
> Committed to?
git://linux-nfs.org/nfs-utils

commit e725def62c73b4aa269fefc4c0d96abb41927fcb
Author: Steve Dickson <[email protected]>
Date: Mon Nov 17 13:17:20 2014 -0500

exportfs: Do not fail on empty exports file.

Commit 076dd80 introduced a regression that causes
exportfs to fail when there is an empty /etc/exports
file. A empty /etc/exports file is valid and should
not cause exportfs to fail.

Signed-off-by: Steve Dickson <[email protected]>

steved.