Re: [patch] autofs for OpenBSD
On 27/03/18(Tue) 13:02, Tomohiro Kusumi wrote: > 2018-03-27 0:57 GMT+09:00 Martin Pieuchot : > > On 25/03/18(Sun) 03:53, Tomohiro Kusumi wrote: > [...] > > - 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 Having it part of autofs(1) would make more sense to me. You can leave exfat code around, hopefully we'll get support for it at some point ;) > > - 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 is simple for the developer to have two programs not for the user. Having separated processes for doing separate actions is a good thing and it will help pledging them. However it doesn't make sense to have to start 2 daemons to do one thing: using autofs. We have many privilege separated daemons that fork and do multiple things. You could look at them. I'm quite sure other BSDs would also welcome such change :) It reduce the number of scripts and keep most of the logic in one place. > >> [...] > >> 3. The "-media" map is currently unsupported due to > >>etc/autofs/special_media not being functional. The same script in > >>FreeBSD uses GEOM sysctl, whereas there seems to be no easy way to > >>achieve the same result on OpenBSD. See the bottom part of > >>etc/autofs/special_media for details. I'd apply if this is doable > >>on OpenBSD, which is to list all possible block devices and > >>partitions for fstyp to check filesystem type. > > > > That's a crucial point. If that's for removable media, how does the > > deamon know when a USB key for example, is inserted? Can't you use > > /dev/hotplug and disklabel(8)? > > The requirement here is to get the list of block devices rather than > catching hardware events, but if the kernel doesn't provide a way to > retrieve such information, it boils down to low level things like > hotplug, searching dmesg, etc. I don't understand when do you have to get such list? But what you're describing seems to be: $ sysctl hw.disknames hw.disknames=sd0:72519550243ad632 $ disklabel 72519550243ad632 # /dev/rsd0c: type: SCSI disk: SCSI disk label: SR CRYPTO duid: 72519550243ad631 flags: bytes/sector: 512 sectors/track: 63 tracks/cylinder: 255 sectors/cylinder: 16065 cylinders: 31129 total sectors: 500102858 boundstart: 64 boundend: 500087385 drivedata: 0 16 partitions: #size offset fstype [fsize bsize cpg] a: 2097152 64 4.2BSD 2048 16384 1 # / b: 16614720 2097216swap# none c:5001028580 unused d: 8388608 18711936 4.2BSD 2048 16384 1 # /tmp e: 8387040 27100544 4.2BSD 2048 16384 1 # /var f: 4194304 67670016 4.2BSD 2048 16384 1 # /usr g: 2097152 71864320 4.2BSD 2048 16384 1 # /usr/X11R6 h: 20971520 73961472 4.2BSD 2048 16384 1 # /usr/local i: 8397088 94932992 4.2BSD 2048 16384 1 # /usr/src j: 4209024103330080 4.2BSD 2048 16384 1 # /usr/xenocara k: 2088448107539104 4.2BSD 2048 16384 1 # /usr/www l: 8385920109627552 4.2BSD 2048 16384 1 # /usr/ports m: 12578912118013472 4.2BSD 2048 16384 12958 # /usr/obj n: 83875360130592384 4.2BSD 2048 16384 1 # /usr/hack o: 20964800214467744 4.2BSD 2048 16384 12958 # /cvs p:264654816235432544 4.2BSD 2048 16384 1 # /home
Re: [patch] autofs for OpenBSD
Hi Thanks for the comments. 2018-03-27 0:57 GMT+09:00 Martin Pieuchot : > 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 change the RB_* macros to RBT_* macros in the kernel code? OK. > > - Why didn't you implemented the set/restore sigmask? Did you encounter > any problem? I couldn't find a way to do this. I think I saw NFS doing something with signal mask though not the exact thing I was looking for. This isn't a must have featu
Re: [patch] autofs for OpenBSD
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 setup can replace an amd(8) one? > 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. 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. - Since it's MI code it should be tested on more than amd64, sparc64 and a 32bit/gcc arch would be great. - Wouldn't it be better to put the configuration file examples to /etc/examples/autofs? - 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. - You can update the OpenBSD release for the man pages to 6.4 - 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? - Since file(1) on OpenBSD is privilege separated and pledge(2)'d do we really need fstyp(8)? What does it buy us? - Why do we need two daemons? Can we merge the functionalities of autounmountd(8) into automountd(8)? - It would be nice if our common log.c would be used instead of adding a new one. - Could you change the RB_* macros to RBT_* macros in the kernel code? - Why didn't you implemented the set/restore sigmask? Did you encounter any problem? - Do we really need a custom task queue? Can't we use the system task queue for sending timeouts? - Instead of doing atomic_add_int_nv(..., -1) you can use atomic_dec_int_nv(). - APRINTF() is not used and can go, __inline -> inline, I'd keep kernel headers in autofs_*.c rather than "autofs.h". - I fear there's a missing reference that can lead to a use-after free. You're giving a request reference to your timeout routing without accounting it. So when timeout_del() is called the task could be spinning on the lock. - Could you add VLOCKSWORK to v_flag if VFSLCKDEBUG is defined? It might help us find possible locking issues. - Did you try running the code with a WITNESS kernel? Are you sure there's no locking ordering issues between `am_lock' and `an_vn_lock'? > [...] > 3. The "-media" map is currently unsupported due to >etc/autofs/special_media not being functional. The same script in >FreeBSD uses GEOM sysctl, whereas there seems to be no easy way to >achieve the same result on OpenBSD. See the bottom part of >etc/autofs/special_media for details. I'd apply if this is doable >on OpenBSD, which is to list all possible block devices and >partitions for fstyp to check filesystem type. That's a crucial point. If that's for removable media, how does the deamon know when a USB key for example, is inserted? Can't you use /dev/hotplug and disklabel(8)?
[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 added
