(Please Cc me on any followups, I'm not subscribed to the list)
Hi,
I've been doing some work on the Debian packages of nfs-utils lately; as soon
as libgssapi and librpcsecgss goes in we should be ready to push in 1.0.8
(yes, about time :-) ), so I thought it would be time to push some of the
changes from Debian back upstream. (I'm sorry this didn't happen before the
1.0.8 release, but it should at least reach 1.0.9.) In addition, there are
several things that would make the task of maintaining the Debian packages
easier:
- Please just remove the debian/ directory from CVS; it looks like the
previous maintainer preferred to maintain the packaging in there, but now
it is primarily a burden. (The 1.0.7 packaging ended up resorting to ugly
hacks to work around it; the 1.0.8 packaging simply has it removed from
the upstream tarball.)
- It would be helpful if you could run autogen.sh before shipping the
tarballs; AFAIK this is normal practice in most other packages, and it
eliminates the need for us to do it ourselves. (The 1.0.7 packaging ran
autoconf as a part of the build process, but this turned out quite ugly in
a number of places. The 1.0.8 packaging has autogen.sh run before the
Debian diff is created, which is better but creates a huge Debian diff.)
So, over to the patches -- they are all against 1.0.8, and can be found at
http://people.debian.org/~sesse/nfs-utils-patches/ :
- fix_exportfs_with_multiple_matches.diff: Fixes a problem with exportfs -o
and multiple entries of the same type for the same patch that matches
a given client. The entire rationale and problem description can be found
at http://bugs.debian.org/245449 -- this is also in the SourceForge
tracker as #940556
(http://sourceforge.net/tracker/index.php?func=detail&aid=940556&group_id=14&atid=100014).
- fix_anonuid_and_anongid.diff: The current code uses -2 for default
anonymous uid and gid, in the hope that it will be equivalent to 65534.
However, on systems with 32-bit uids and gids, this returns another value
entirely, which gives rise to an unpredictable default uid and gid (which
probably doesn't match what's in passwd and group). This patch fixes the
code and documentation to use 65534 explicitly instead.
- escape_hashes_in_exports.diff: Makes sure any # signs in the printed-out
exports file are escaped (as with quotes, spaces, etc.), so they won't be
treated as a comment when they're read back in again.
- document_sync_option.diff: Document the 'sync' option in the exports(5)
man page -- ATM only the 'async' option is documented, which is not very
symmetric. :-)
- mountd_state_directory.diff: Let the user select (via a new parameter)
the path to the NFS state directory for mountd, to match the statd
functionality. (If you take it in, you might want to remove the part in
the manpage saying it's Debian-specific.)
- fix_nhfsrun_signal.diff: nhfsrun is supposed to be able to be signalled
with SIGUSR1, but the signal trapped is number 30, which is something
else entirely (SIGPWR). This patch simply changes it to say "USR1", which
gets it right no matter what the value is.
Let me know if you have questions about these patches, or really anything
else; I'm not the Debian maintainer, but I seem to be quite involved with the
packaging right now, so I guess I could at least point you the right place. :-)
/* Steinar */
--
Homepage: http://www.sesse.net/
-------------------------------------------------------
All the advantages of Linux Managed Hosting--Without the Cost and Risk!
Fully trained technicians. The highest number of Red Hat certifications in
the hosting industry. Fanatical Support. Click to learn more
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Thu, Jun 01, 2006 at 12:42:47PM -0400, Kevin Coffman wrote:
> There is a similar issue in the svcgssd code. However, while I was
> looking into this, it appeared that the kernel code uses "-2" in
> places as well. It seems like a similar change may be needed in the
> kernel code as well?
Hm, that's possible; the change is a bit less critical for svcgssd, though,
as the default configuration file explicitly seems to specify the user
"nobody" (or am I thinking of the wrong user here?).
/* Steinar */
--
Homepage: http://www.sesse.net/
-------------------------------------------------------
All the advantages of Linux Managed Hosting--Without the Cost and Risk!
Fully trained technicians. The highest number of Red Hat certifications in
the hosting industry. Fanatical Support. Click to learn more
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Thu, Jun 01, 2006 at 05:59:29PM +0200, Steinar H. Gunderson wrote:
> - fix_anonuid_and_anongid.diff: The current code uses -2 for default
> anonymous uid and gid, in the hope that it will be equivalent to 65534.
> However, on systems with 32-bit uids and gids, this returns another value
> entirely, which gives rise to an unpredictable default uid and gid (which
> probably doesn't match what's in passwd and group). This patch fixes the
> code and documentation to use 65534 explicitly instead.
I just noticed this patch is incomplete; you'll also need to make the same
change in utils/exportfs/exportfs.c. The extra patch is trivial, though, but
I missed it the first time. :-)
/* Steinar */
--
Homepage: http://www.sesse.net/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Thu, Jun 01, 2006 at 05:59:29PM +0200, Steinar H. Gunderson wrote:
> So, over to the patches -- they are all against 1.0.8, and can be found at
> http://people.debian.org/~sesse/nfs-utils-patches/ :
I added a new one:
- fix_manpage_errors.diff: Fixes a few errors from man in the idmapd.conf
man page. (I don't know of any other man pages giving errors; "man -l
foo.man >/dev/null" is usually the simplest way to check for these.)
/* Steinar */
--
Homepage: http://www.sesse.net/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On 6/1/06, Steinar H. Gunderson <[email protected]> wrote:
> (Please Cc me on any followups, I'm not subscribed to the list)
>
> Hi,
> - fix_anonuid_and_anongid.diff: The current code uses -2 for default
> anonymous uid and gid, in the hope that it will be equivalent to 65534.
> However, on systems with 32-bit uids and gids, this returns another value
> entirely, which gives rise to an unpredictable default uid and gid (which
> probably doesn't match what's in passwd and group). This patch fixes the
> code and documentation to use 65534 explicitly instead.
There is a similar issue in the svcgssd code. However, while I was
looking into this, it appeared that the kernel code uses "-2" in
places as well. It seems like a similar change may be needed in the
kernel code as well?
The svcgssd code has the added problem that it really wants to use the
per-export anon values if they were specified differently than the
default. Those probably need to be passed up in the upcall, or we
need a way of indicating that the default values should be used in the
downcall.
K.C.
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On 6/1/06, Steinar H. Gunderson <[email protected]> wrote:
> On Thu, Jun 01, 2006 at 12:42:47PM -0400, Kevin Coffman wrote:
> > There is a similar issue in the svcgssd code. However, while I was
> > looking into this, it appeared that the kernel code uses "-2" in
> > places as well. It seems like a similar change may be needed in the
> > kernel code as well?
>
> Hm, that's possible; the change is a bit less critical for svcgssd, though,
> as the default configuration file explicitly seems to specify the user
> "nobody" (or am I thinking of the wrong user here?).
I'm talking about the case where we are unable to find a mapping for a
gss authenticated name to uid/gid and supplementary groups. The 1.0.8
code actually has a failure there, but we really want to default to
the anonuid/anongid instead of failing to create the context. We
could get the mappings for "nobody", but we'd really like to use the
same anonuid/anongid values as used in export.
K.C.
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Thursday June 1, [email protected] wrote:
> (Please Cc me on any followups, I'm not subscribed to the list)
>
> Hi,
>
> I've been doing some work on the Debian packages of nfs-utils lately; as soon
> as libgssapi and librpcsecgss goes in we should be ready to push in 1.0.8
> (yes, about time :-) ), so I thought it would be time to push some of the
> changes from Debian back upstream. (I'm sorry this didn't happen before the
> 1.0.8 release, but it should at least reach 1.0.9.) In addition, there are
> several things that would make the task of maintaining the Debian packages
> easier:
Thanks for this.
>
> - Please just remove the debian/ directory from CVS; it looks like the
> previous maintainer preferred to maintain the packaging in there, but now
> it is primarily a burden. (The 1.0.7 packaging ended up resorting to ugly
> hacks to work around it; the 1.0.8 packaging simply has it removed from
> the upstream tarball.)
I have no problem doing this. debian/ will be gone in 1.0.9.
Note that I've given up on CVS (for now at least) and have decided to
try using 'git' for nfs-utils maintenance. It should appear on
linux-nfs.org
at some stage (once I am happy that I know what I am doing with it).
> - It would be helpful if you could run autogen.sh before shipping the
> tarballs; AFAIK this is normal practice in most other packages, and it
> eliminates the need for us to do it ourselves. (The 1.0.7 packaging ran
> autoconf as a part of the build process, but this turned out quite ugly in
> a number of places. The 1.0.8 packaging has autogen.sh run before the
> Debian diff is created, which is better but creates a huge Debian diff.)
Yes, I'll make sure that happen (I'm still getting used to how all
these auto-tools work!).
>
> So, over to the patches -- they are all against 1.0.8, and can be found at
> http://people.debian.org/~sesse/nfs-utils-patches/ :
>
> - fix_exportfs_with_multiple_matches.diff: Fixes a problem with exportfs -o
> and multiple entries of the same type for the same patch that matches
> a given client. The entire rationale and problem description can be found
> at http://bugs.debian.org/245449 -- this is also in the SourceForge
> tracker as #940556
> (http://sourceforge.net/tracker/index.php?func=detail&aid=940556&group_id=14&atid=100014).
This look sane - thanks.
I wonder if there is a way to turn off the request tracker - no-one
looks at it...
> - fix_anonuid_and_anongid.diff: The current code uses -2 for default
> anonymous uid and gid, in the hope that it will be equivalent to 65534.
> However, on systems with 32-bit uids and gids, this returns another value
> entirely, which gives rise to an unpredictable default uid and gid (which
> probably doesn't match what's in passwd and group). This patch fixes the
> code and documentation to use 65534 explicitly instead.
I've noticing this problem before and always shied away from it.
While the value might be "unpredictable", someone might be depending
on the current behaviour, and changing it could break things for some
people while fixing things for others...
Can you -- or anyone else -- convince me that this is really a good
and safe thing to do? I'm fairly open to being convince, but I don't
feel I'm familiar enough with all the issues and would love to hear
from someone who is.
My particular concern is "What might it break, and how much of a
problem could that be?"
> - escape_hashes_in_exports.diff: Makes sure any # signs in the printed-out
> exports file are escaped (as with quotes, spaces, etc.), so they won't be
> treated as a comment when they're read back in again.
Hmm... I think the real bug here is that a '#' is treated as starting
a comment even if it isn't at the start of a word.
xgetc shouldn't check for '#', xgettok should.
The kernel doesn't currently escape '#' in /proc/fs/nfs/exports so
reading pathnames containing '#' from there will still be a problem.
So I might fix that too..... could that break anything?
> - document_sync_option.diff: Document the 'sync' option in the exports(5)
> man page -- ATM only the 'async' option is documented, which is not very
> symmetric. :-)
Ok, thanks.
> - mountd_state_directory.diff: Let the user select (via a new parameter)
> the path to the NFS state directory for mountd, to match the statd
> functionality. (If you take it in, you might want to remove the part in
> the manpage saying it's Debian-specific.)
Seems pretty pointless ;-) but that won't stop be accepting it...
I bet you didn't test the '-s' usage :-) I've fixed that.
I love getting patches that also update the man page! Now if I can
only train people to update ChangeLog too :-)
> - fix_nhfsrun_signal.diff: nhfsrun is supposed to be able to be signalled
> with SIGUSR1, but the signal trapped is number 30, which is something
> else entirely (SIGPWR). This patch simply changes it to say "USR1", which
> gets it right no matter what the value is.
>
Simple enough, thanks.
> I added a new one:
>
> - fix_manpage_errors.diff: Fixes a few errors from man in the idmapd.conf
> man page. (I don't know of any other man pages giving errors; "man -l
> foo.man >/dev/null" is usually the simplest way to check for these.)
Hmmm. I'm not sure I like removing blank lines just for the same of
some silly warning - blank lines are good for readability.
I have instead replaced them with a single ' which removes the warning
and is almost as good as a blank.
You can see all these patches via
git clone git://linux-nfs.org/~neilb/exports/nfs-utils
I'll try to organise for it to just be
git clone git://linux-nfs.org/nfs-utils
Thanks again,
NeilBrown
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Mon, 5 Jun 2006, Neil Brown wrote:
>
> > - escape_hashes_in_exports.diff: Makes sure any # signs in the printed-out
> > exports file are escaped (as with quotes, spaces, etc.), so they won't be
> > treated as a comment when they're read back in again.
>
> Hmm... I think the real bug here is that a '#' is treated as starting
> a comment even if it isn't at the start of a word.
> xgetc shouldn't check for '#', xgettok should.
> The kernel doesn't currently escape '#' in /proc/fs/nfs/exports so
> reading pathnames containing '#' from there will still be a problem.
>
> So I might fix that too..... could that break anything?
Maybe autofs. I read /proc/mounts a lot and I've had some noise regarding
"#" escaping lately.
I'll check.
Ian
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Mon, Jun 05, 2006 at 01:13:10PM +1000, Neil Brown wrote:
> I've noticing this problem before and always shied away from it.
> While the value might be "unpredictable", someone might be depending
> on the current behaviour, and changing it could break things for some
> people while fixing things for others...
> Can you -- or anyone else -- convince me that this is really a good
> and safe thing to do? I'm fairly open to being convince, but I don't
> feel I'm familiar enough with all the issues and would love to hear
> from someone who is.
> My particular concern is "What might it break, and how much of a
> problem could that be?"
The biggest problem is, IMO: People expect stuff to be done as "nobody" (or,
at least uid 65534, which is mapped to "nobody" on all systems that I know).
"nobody" in Linux (as opposed to, say, Hurd) is not really someone with no
rights, but can own files etc. -- so if you had something owned by "nobody",
or "nobody" was a member of some specific group, you'd expect that to work.
Now it can easily vary between different systems, which is rather bad.
As far as I'm concerned, we've already probably had multiple instances of
this changing (from 65534 to 4294967294, when upgrading) and nobody really
noticed; I don't see the harm of changing it back to it's the same
everywhere.
> Hmm... I think the real bug here is that a '#' is treated as starting
> a comment even if it isn't at the start of a word.
> xgetc shouldn't check for '#', xgettok should.
> The kernel doesn't currently escape '#' in /proc/fs/nfs/exports so
> reading pathnames containing '#' from there will still be a problem.
>
> So I might fix that too..... could that break anything?
Well, I think it's at least inconsistent with the man page as it's written
today; in that case, the man page should be rewritten too. Look at the
original bug report on http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=239230
to see the entire section.
>> - mountd_state_directory.diff: Let the user select (via a new parameter)
>> the path to the NFS state directory for mountd, to match the statd
>> functionality. (If you take it in, you might want to remove the part in
>> the manpage saying it's Debian-specific.)
> Seems pretty pointless ;-) but that won't stop be accepting it...
It is reportedly useful for read-only /var -- people with special clustering
needs complained about the lack of such things, IIRC.
> I love getting patches that also update the man page! Now if I can
> only train people to update ChangeLog too :-)
:-)
> Hmmm. I'm not sure I like removing blank lines just for the same of
> some silly warning - blank lines are good for readability.
> I have instead replaced them with a single ' which removes the warning
> and is almost as good as a blank.
OK, that sounds good too. I don't know why the blanks are disallowed, but
just leaving a man page with known errors wouldn't be too good either. :-)
/* Steinar */
--
Homepage: http://www.sesse.net/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs