On Mon, 2012-05-07 at 10:27 +0400, Michael Tokarev wrote: > On 07.05.2012 04:57, Ian Kent wrote: > > On Sun, 2012-05-06 at 22:47 +0400, Michael Tokarev wrote: > >> Now when kernel has more or less nice interface which works > >> with all variants of userspace to date, maybe it's time to > >> revert the commit which enables different struct size for > >> kernels > 3.3, especially since that commit was initially > >> wrong (since it does not take into account any stable trees > >> and possible backports to other previous versions)? > > > > We will need to use something similar to the patch I already sent you as > > kernel releases don't get unreleased so I will need to cater for it. > > Why cater when kernel does not care anymore? You only may want > to consider versions between 3.3 and 3.3.4 (iirc), but that is > not worth the ugliness I think. That patch for autofs userspace > is not needed anymore, since unpatched autofs works fine with > all previous/older kernels, and since current versions of kernel, > the kernel will work with either size of struct, so that fix in > userspace isn't needed again. So you can just remove and forget > about it, and remove quite some very strange code piece... ;) > > I'm not saying the code does not work, I'm saying that it will > work regardless of the userspace fix, and since the fix is ugly > it might be a good idea to just revert it. > > And you wont be able to compensate for all backports anyway -- > some distro may want to backport this change to some older > kernel (e.g. 2.6.32 - why not for, eg, redhat?) in order to > let both systemd and autofs work there on mixed 32/64bit > environment, -- will you check /etc/redhat.release and > other files too to determine which struct size to use? :) No I won't check those but I want to provide for, if only for documentation purposes, the changes to the upstream kernels. So the function (that's been present for several years) will have an added if clause. It doesn't seem like a big deal to me since the function to get the read length is called only once at program start. Eventually it will be removed but not now when we're so close to a time of change. As far as distros making kernel changes goes then the downstream maintainers can raise issues on the autofs mailing list, again something that isn't done nearly enough, and when it is done it's often not done well, IMHO. > > Current kernel does not care, and without the last userspace > fix the code is at least a bit understandable and it works > either way, that's why I think just reverting it is a good > idea. But it is not my call obviously. Like I say, if only for documentation sake. > > [kernel version] > > Because mount.nfs changed to use the kernel to parse nfs mount options > > based on kernel version. The kernel RPC code works very differently to > > the user space RPC code, since it must wait if a host doesn't respond > > where that isn't so important for user space, which leads to long > > timeout delays. That's the way it should be done, of course, but for an > > interactive application like automount everyone complains about it. > > Basically, automount must RPC ping hosts at some point in the mount > > trigger to mitigate this. > > Aha, I see. So nfs "mounter" also needs to know kernel version to > pass mount info correctly. I've seen this in other places already, > even busybox mount.nfs has kernel version check. Thank you for the > explanation. So indeed, the kernel version gathering code should > stay, no problem with that. It does, and for earlier kernel version it did a bunch of probing of mountd versions and such which isn't done when passing test options to the kernel. While bypassing the probing goes a long way to reducing the number of user space TCP ports consumed it leaves us open the long mount delay problem. > > >> And btw, is there some public autofs userspace git/whatever repository > >> where one can see current state of the package? > > > > git clone git://git.kernel.org/pub/scm/linux/storage/autofs/autofs > > Hm. I found this on http://git.kernel.org/ , but somehow I thought > it is some old clone, since it has last modified 4 weeks ago, and > I thought all this 32/64 issue was later than that... Ohh, time is > flying too fast... :) > > Thank you, I'll use that. > > I asked because apparently I will be one of the maintainers of autofs > userspace in Debian, and today I'm sponsoring first upload of this > package after long inaction of a previous maintainer. See > http://bugs.debian.org/671701 for the current discussion if you're > interested. I was interested at one time, during version 4, some time before work on version 5 started with rigor. I worked through the Debian patches merging what I could and added Debian package infrastructure to the upstream package. The maintainer at the time didn't appreciate that at all and didn't seem interested in co-operative work so I removed the Debian packaging directory, as requested, and have had nothing to do with Debian packaging ever since. Ian -- To unsubscribe from this list: send the line "unsubscribe autofs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
