Re: [patch] autofs for OpenBSD
Hi Thanks for the comments. 2018-03-27 0:57 GMT+09:00 Martin Pieuchot <m...@openbsd.org>: > Hello, > > On 25/03/18(Sun) 03:53, Tomohiro Kusumi wrote: >> This is the initial patch submission of autofs filesystem support for >> OpenBSD, which was originally written for FreeBSD and later ported to >> DragonFlyBSD and NetBSD. >> >> Autofs is a generic filesystem automounter filesystem, not limited to >> mounting NFS. > > Does that mean amd(8) becomes obsolete? If yes, how does an autofs Yes, basically. FreeBSD still has amd(8), but DragonFly removed it. They can technically coexist though. > setup can replace an amd(8) one? I don't know too much about amd(8), but there is no compatibility. The migration is about replacing amd(8) setup with /etc/auto_master which is not all that difficult. The FreeBSD autofs is aimed to be compatible with Solaris autofs, and https://people.freebsd.org/~trasz/autofs.pdf explains a bit on this. > >> mounting NFS. This OpenBSD port actually started off with the NetBSD >> code rather than FreeBSD though all versions are quite similar except >> for the autofs filesystem code in kernel space. While I currently >> maintain it as an out-of-tree filesystem on GitHub, I'm aiming for >> this to get merged by OpentBSD. > > That's really nice. > >> The initial patch is available from below in three formats. The patch >> applies to the current src as of March 24 (and builds on amd64). >> >> diff -aNur >> https://leaf.dragonflybsd.org/~tkusumi/diff/openbsd/OpenBSD-autofs-v1.patch >> git format-patch >> https://leaf.dragonflybsd.org/~tkusumi/diff/openbsd/0001-OpenBSD-autofs-v1.patch >> GitHub >> https://github.com/kusumi/src/tree/autofs-20180325 > > Please submit the diff inline next time. That allow direct review > without relying on other tools than a mail client. Sure. I didn't inline the patch as it was a big chunk of text. > > I have multiple comments, some of them can be addressed in tree: > > - It's MI code so /dev/autofs should be present on all archs. OK. I'll find MI configuration and move autofs from amd64. > > - Since it's MI code it should be tested on more than amd64, sparc64 > and a 32bit/gcc arch would be great. I only have x86. I can give a try for IA32 on a virtual machine at some point. > > - Wouldn't it be better to put the configuration file examples to > /etc/examples/autofs? /etc/autofs/* aren't examples, but rather features, meaning users are not going to copy these and edit. They just use these scripts, or they add new scripts for new features that they want. In fact, FreeBSD autofs has evolved that way. What users are going to edit is /etc/auto_master and user defined scripts refered to by /etc/auto_master. > > - Could you please reduce the number of sysctl buttons, or even better > remove them completely? `autofs_debug_tunable' is certainly not > needed. I can't say for the others but I'd prefer if we could have > values that work for everybody. All these come from FreeBSD, rather than my preference. I'm fine with removing these entirely if they bother OpenBSD. The default values shouldn't have problems for most cases. > > - You can update the OpenBSD release for the man pages to 6.4 OK. > > - The documentation for EVFILT_FS is misleading. I see that it comes > from DragonFly however using "Currently" implies that something else > will be supported in the future. Is it your plan? How does this > implementation differs from Darwin/Mac OS X? FreeBSD has other events that trigger EVFILT_FS that are not needed by autofs. That's what "currently" implies, but I can drop that. I'm not going to implement the rest anyway. > > - Since file(1) on OpenBSD is privilege separated and pledge(2)'d do we > really need fstyp(8)? What does it buy us? Nothing really. I can make this a part of usr.sbin/autofs rather than as a stand alone command if this bothers OpenBSD. While this command is a generic command, some features are specifically needed by autofs. fstyp(8) also supports multi-volumes filesystem such as HAMMER. By the way, I can drop exfat.c (a copy from below) if OpenBSD does not prefer to have exFAT related code. https://github.com/freebsd/freebsd/blob/master/usr.sbin/fstyp/exfat.c > > - Why do we need two daemons? Can we merge the functionalities of > autounmountd(8) into automountd(8)? automountd(8) blocks at ioctl(2) via /dev/autofs waiting for mount requests. autounmountd(8) blocks waiting for kqueue(2) events. I recommend to have these separated for simplicity. > > - It would be nice if our common log.c would be used instead of adding > a new one. I'll look into log.c. Didn't know such thing exists. > > - Could you c
[patch] autofs for OpenBSD
Hi This is the initial patch submission of autofs filesystem support for OpenBSD, which was originally written for FreeBSD and later ported to DragonFlyBSD and NetBSD. Autofs is a generic filesystem automounter filesystem, not limited to mounting NFS. This OpenBSD port actually started off with the NetBSD code rather than FreeBSD though all versions are quite similar except for the autofs filesystem code in kernel space. While I currently maintain it as an out-of-tree filesystem on GitHub, I'm aiming for this to get merged by OpentBSD. The initial patch is available from below in three formats. The patch applies to the current src as of March 24 (and builds on amd64). diff -aNur https://leaf.dragonflybsd.org/~tkusumi/diff/openbsd/OpenBSD-autofs-v1.patch git format-patch https://leaf.dragonflybsd.org/~tkusumi/diff/openbsd/0001-OpenBSD-autofs-v1.patch GitHub https://github.com/kusumi/src/tree/autofs-20180325 Online documentation and man pages for FreeBSD (which are also part of the patch) would/should work against OpenBSD with a few exceptions. Below links are FreeBSD autofs info you can find on the web. Note that FreeBSD autofs is not a port of Linux autofs which has existed for a long(er) time. Although they are quite similar in terms of features, there is no compatibility in configuration files and such, thus the same for OpenBSD and Linux. https://people.freebsd.org/~trasz/autofs.pdf https://wiki.freebsd.org/Automounter http://freebsdfoundation.blogspot.fi/2015/03/freebsd-from-trenches-using-autofs5-to_13.html --- How to use autofs (currently only enabled on amd64) 1. Build kernel (autofs enabled by default atm in sys/conf/GENERIC) and world. 2. Enable etc/rc.d/automountd and etc/rc.d/autounmountd via rcctl(8). 3. Create a character device /dev/autofs via /dev/MAKEDEV. 4. Write auto mount configuration(s) to /etc/auto_master. FreeBSD documentation should help, though configuration needed to do a simple NFS auto mounting is quite simple (2 lines basically - i.e. 1 line in /etc/auto_master to specify a configuration file for eithre direct or indirect map, and 1 line in that file to specify NFS mount details). 5. Run automount(8) command (no options needed) to mount autofs(5) based on /etc/auto_master contents, and confirm autofs(5) is mounted via mount(8) command. 6. Restart automountd(8) and autounmountd(8). This could be before or after running automount(8) command. 7. Access autofs(5) mount point, and see if auto mounting works. I've only tested with NFS with both direct/indirect mapping, which is probably what majority of people use autofs for, however the basic automounting mechanism is filesystem-agnostic. I'll write down some more details of newly added code. --- Newly added subsystems 1. sys/miscfs/autofs The autofs(5) filesystem catches some of the vnode operations, and notifies the user space daemon automountd(8) that auto mounting requests from user process have arrived. The character device /dev/autofs (with major# 98) is used as a communication channel between autofs(5) and automountd(8) daemon via ioctl(2). Once automountd(8) receives the request from autofs(5) filesystem, it attempts to mount a target filesystem which corresponds to the autofs(5) mount point accessed by the user process, based on what auto_master(5) (/etc/auto_master file) has. The actual mounting is done by a child process forked from automountd(8) using mount(8) and variants. 2. usr.sbin/autofs User space is basically the same as the FreeBSD code as well as DragonFlyBSD and NetBSD ports with minimum changes needed for OpenBSD. The changes mostly come from difference in library/syscall interface, FreeBSD specific syscalls, workaround code specific to OpenBSD, etc. The directory contains followings. * autmount(8) - a command to mount autofs(5) filesystem. * automountd(8) - a daemon to auto mount target filesystems specified in /etc/auto_master. * autounmountd(8) - a daemon to auto unmount the auto mounted filesystems if no longer used when the timer has expired. These executable binaries actually consist of a single inode, i.e. /usr/sbin/automountd and /usr/sbin/autounmountd are hardlinks to /usr/sbin/automount, and they do what they do by checking argv[0] in its main(). 3. etc/auto_master and etc/autofs/* etc/auto_master is a configuration file which describes relations between autofs(5) mount points and target filesystems to auto mount. As mentioned above, automountd(8) mounts filesystems based on this file. The auto_master(5) man page explains details of the format. etc/autofs/* are collection of scripts used by auto_master(5) when pre-defined special mappings are used. These are just copied from FreeBSD unless a script contains FreeBSD specific stuff (e.g. etc/autofs/special_media uses FreeBSD specific sysctl). 4. usr.sbin/fstyp Fstyp(8) is an utility to detect filesystem type of the given block device, and was originally
[patch] sys/tmpfs: fix a comment on conditional to unlock dvp
It's unlocking parent directory vnode because LOCKPARENT is *not* requested. Also fix a typo for "explicitly". Index: sys/tmpfs/tmpfs_vnops.c === RCS file: /cvs/src/sys/tmpfs/tmpfs_vnops.c,v retrieving revision 1.27 diff -u -p -r1.27 tmpfs_vnops.c --- sys/tmpfs/tmpfs_vnops.c 19 Jun 2016 11:54:33 - 1.27 +++ sys/tmpfs/tmpfs_vnops.c 20 Mar 2018 19:07:48 - @@ -289,7 +289,7 @@ done: out: /* * If (1) we succeded, (2) found a distinct vnode to return and (3) were - * either explicitely told to keep the parent locked or are in the + * either explicitly told not to keep the parent locked or are in the * middle of a lookup, unlock the parent vnode. */ if ((error == 0 || error == EJUSTRETURN) && /* (1) */
[patch] sys/tmpfs: unbreak build
Recent commit ("Syncronize filesystems to disk when suspending.") has
added int argument to sync() vfsops, but sys/tmpfs missed that change.
https://leaf.dragonflybsd.org/~tkusumi/diff/openbsd/tmpfs-sync-fix-arg.cvs.patch
Index: sys/tmpfs/tmpfs_vfsops.c
===
RCS file: /cvs/src/sys/tmpfs/tmpfs_vfsops.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 tmpfs_vfsops.c
--- sys/tmpfs/tmpfs_vfsops.c 11 Dec 2017 05:27:40 - 1.14
+++ sys/tmpfs/tmpfs_vfsops.c 7 Mar 2018 07:23:49 -
@@ -65,7 +65,7 @@ int tmpfs_vget(struct mount *, ino_t, st
int tmpfs_fhtovp(struct mount *, struct fid *, struct vnode **);
int tmpfs_vptofh(struct vnode *, struct fid *);
int tmpfs_statfs(struct mount *, struct statfs *, struct proc *);
-int tmpfs_sync(struct mount *, int, struct ucred *, struct proc *);
+int tmpfs_sync(struct mount *, int, int, struct ucred *, struct proc *);
int tmpfs_init(struct vfsconf *);
int
@@ -350,7 +350,8 @@ tmpfs_statfs(struct mount *mp, struct st
}
int
-tmpfs_sync(struct mount *mp, int waitfor, struct ucred *cred, struct proc *p)
+tmpfs_sync(struct mount *mp, int waitfor, int stall, struct ucred *cred,
+struct proc *p)
{
return 0;
