Re: [patch] Proposal: move getmntopts(3) into libutil
On Wed, Feb 27, 2013 at 07:08:35PM +0300, Sergey Kandaurov wrote: > I have put it on freefall for convenience. > http://people.freebsd.org/~pluknet/patches/getmntopts.2.patch Should the synopsis for getmntopts(3) use #include , instead of '"', as well as the example ? For the mntopts.h itself, I suggest to add an include guard, it is required for system header. I would also take an opportunity and wrapped long lines in mntopts.h in the same change. pgpdztsVQqeiG.pgp Description: PGP signature
Re: [patch] Proposal: move getmntopts(3) into libutil
On 26 February 2013 23:11, Craig Rodrigues wrote:
> On Tue, Feb 26, 2013 at 3:39 AM, Sergey Kandaurov wrote:
>>
>> Hi.
>>
>>
>>
>> External mount-like utilities may also have difficulties with building
>> to get getmntopts.c source as this requires /usr/src presence which is
>> in sync with installed world. Look how mount_fusefs from ports compiles:
>> # mount_fusefs needs mntopts.h and getmntopts.c from src/sbin/mount/
>
>
> I have no object in moving getmntopts.c to libutil.
>
> I'll give some history to some of this stuff, to the best of my ability.
> A few years ago, phk@ made a big change by introducing the nmount() system
> call to replace mount().
> Look at all mount-related commits around this time:
> http://lists.freebsd.org/pipermail/cvs-src/2004-December/author.html#36373
>
> This required changing every file system, and for old file systems which
> supported the "classic mount" cmount(),
> in each file system, the cmount() call had to internally call the nmount()
> code.
>
> In addition, phk made a pass to clean up all the userland mount programs to
> use nmount(). There was a lot
> of duplicated ("copy and pasted") code in various mount programs. I helped
> in the effort to clean up some of the userland mount() programs.
>
> pjd@ proposed to make the main /sbin/mount load a shared library for each
> specific file system,
> and each shared library would have file-system specific mount logic. For
> example:
>
> mount -t newfoofs /dev/blah /mnt
>
> would internally do something like:
>
> dlopen("libmount_newfoofs",.);
>
>
> phk@ opposed this approach, saying that it could lead to ABI/API problems,
> library mismatches, etc.
> So we kept the existing approach. I modified /sbin/mount to by default use
> nmount() and only
> for certain file systems, exec an external mount program.
>
> phk's ideas for getmntopts.c was always to keep it as a place where
> "library-like" functions for mounting
> file systems would be kept. To avoid library mismatch problems, it was kept
> has a C file directly compiled
> into the mount programs.
Sure, keeping it directly compiled has its own benefits.
Reading your mail is very educational, thank you.
>
> I have no problems keeping getmntopts.c in a separate library. libutil is
> fine, or even a separate libmntutil (or whatever).
> Just keep in mind the issues that could possibly come up if there is a
> mismatch between
> the userland mount programs, and the library which contains getmntopts.c
>
> Other than that, you proposal is quite reasonable, and I have no issue with
> it.
Although libutil is already used with such binaries like mount and mountd,
library mismatch is a real concern. I will need to think somewhat more.
Thanks for looking at this.
--
wbr,
pluknet
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [patch] Proposal: move getmntopts(3) into libutil
On 26 February 2013 16:20, Konstantin Belousov wrote: > On Tue, Feb 26, 2013 at 02:39:26PM +0300, Sergey Kandaurov wrote: >> Hi. >> >> The functions from sbin/mount/getmntopts.c are used in a bunch of other >> stuff like mount_* utilities which have to suck them in as their own >> functions in quite a hackish way. getmntopts.c copies are compiled in to >> an every utility-consumer instead of being present in one place. Looks >> like getmntopts.c was brought together with mount_ufs.c in 4.4BSD-Lite. >> After that other mount_* were converted to use getmntopts(). > Yes, this is ugly. On the other hand, compiling the functions into > mount binaries makes them not to depend on the yet another library. > It cannot be an argument for rejecting your patch, only a point to > consider. I'm afraid this is the price for the change. No better thoughts. >> The attached patch moves them to the IMHO architecturally more correct >> place: a separate library -lutil. >> sbin/mount/mntopts.h-> include/mntopts.h > I think the mntopts.h should be moved to lib/libutil then, and installed > by libutil Makefile. That's reasonable, thanks. Patch is updated with your correction. I have put it on freefall for convenience. http://people.freebsd.org/~pluknet/patches/getmntopts.2.patch > >> sbin/mount/getmntopts.[3c] -> lib/libutil/getmntopts.[3c] > I assume that the move is done by 'svn mv' to preserve the history. Yes. Unfortunately svn stat cannot nicely represent 'svn mv' ops. -- wbr, pluknet ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [patch] Proposal: move getmntopts(3) into libutil
On Tue, Feb 26, 2013 at 3:39 AM, Sergey Kandaurov wrote:
> Hi.
>
>
> External mount-like utilities may also have difficulties with building
> to get getmntopts.c source as this requires /usr/src presence which is
> in sync with installed world. Look how mount_fusefs from ports compiles:
> # mount_fusefs needs mntopts.h and getmntopts.c from src/sbin/mount/
>
I have no object in moving getmntopts.c to libutil.
I'll give some history to some of this stuff, to the best of my ability.
A few years ago, phk@ made a big change by introducing the nmount() system
call to replace mount().
Look at all mount-related commits around this time:
http://lists.freebsd.org/pipermail/cvs-src/2004-December/author.html#36373
This required changing every file system, and for old file systems which
supported the "classic mount" cmount(),
in each file system, the cmount() call had to internally call the nmount()
code.
In addition, phk made a pass to clean up all the userland mount programs to
use nmount(). There was a lot
of duplicated ("copy and pasted") code in various mount programs. I helped
in the effort to clean up some of the userland mount() programs.
pjd@ proposed to make the main /sbin/mount load a shared library for each
specific file system,
and each shared library would have file-system specific mount logic. For
example:
mount -t newfoofs /dev/blah /mnt
would internally do something like:
dlopen("libmount_newfoofs",.);
phk@ opposed this approach, saying that it could lead to ABI/API problems,
library mismatches, etc.
So we kept the existing approach. I modified /sbin/mount to by default use
nmount() and only
for certain file systems, exec an external mount program.
phk's ideas for getmntopts.c was always to keep it as a place where
"library-like" functions for mounting
file systems would be kept. To avoid library mismatch problems, it was
kept has a C file directly compiled
into the mount programs.
I have no problems keeping getmntopts.c in a separate library. libutil is
fine, or even a separate libmntutil (or whatever).
Just keep in mind the issues that could possibly come up if there is a
mismatch between
the userland mount programs, and the library which contains getmntopts.c
Other than that, you proposal is quite reasonable, and I have no issue with
it.
Thanks!
--
Craig
--
Craig
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [patch] Proposal: move getmntopts(3) into libutil
On 2/26/13 4:20 AM, Konstantin Belousov wrote: On Tue, Feb 26, 2013 at 02:39:26PM +0300, Sergey Kandaurov wrote: Hi. The functions from sbin/mount/getmntopts.c are used in a bunch of other stuff like mount_* utilities which have to suck them in as their own functions in quite a hackish way. getmntopts.c copies are compiled in to an every utility-consumer instead of being present in one place. Looks like getmntopts.c was brought together with mount_ufs.c in 4.4BSD-Lite. After that other mount_* were converted to use getmntopts(). Yes, this is ugly. On the other hand, compiling the functions into mount binaries makes them not to depend on the yet another library. It cannot be an argument for rejecting your patch, only a point to consider. The utilities consuming getmntopts.c as currently present in HEAD: mount_smbfs fsck_ffs growfs mksnap_ffs mount mount_cd9660 mount_ext2fs mount_fusefs mount_hpfs mount_msdosfs mount_nfs mount_nullfs mount_reiserfs mount_std mount_udf mount_unionfs mount_nwfs mount_portalfs mount_smbfs mountd External mount-like utilities may also have difficulties with building to get getmntopts.c source as this requires /usr/src presence which is in sync with installed world. Look how mount_fusefs from ports compiles: # mount_fusefs needs mntopts.h and getmntopts.c from src/sbin/mount/ The attached patch moves them to the IMHO architecturally more correct place: a separate library -lutil. sbin/mount/mntopts.h-> include/mntopts.h I think the mntopts.h should be moved to lib/libutil then, and installed by libutil Makefile. sbin/mount/getmntopts.[3c] -> lib/libutil/getmntopts.[3c] I assume that the move is done by 'svn mv' to preserve the history. Agree with all of Konstantin's review. Looks very good. -Alfred ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [patch] Proposal: move getmntopts(3) into libutil
On Tue, Feb 26, 2013 at 02:39:26PM +0300, Sergey Kandaurov wrote: > Hi. > > The functions from sbin/mount/getmntopts.c are used in a bunch of other > stuff like mount_* utilities which have to suck them in as their own > functions in quite a hackish way. getmntopts.c copies are compiled in to > an every utility-consumer instead of being present in one place. Looks > like getmntopts.c was brought together with mount_ufs.c in 4.4BSD-Lite. > After that other mount_* were converted to use getmntopts(). Yes, this is ugly. On the other hand, compiling the functions into mount binaries makes them not to depend on the yet another library. It cannot be an argument for rejecting your patch, only a point to consider. > > The utilities consuming getmntopts.c as currently present in HEAD: > mount_smbfs > fsck_ffs > growfs > mksnap_ffs > mount > mount_cd9660 > mount_ext2fs > mount_fusefs > mount_hpfs > mount_msdosfs > mount_nfs > mount_nullfs > mount_reiserfs > mount_std > mount_udf > mount_unionfs > mount_nwfs > mount_portalfs > mount_smbfs > mountd > > External mount-like utilities may also have difficulties with building > to get getmntopts.c source as this requires /usr/src presence which is > in sync with installed world. Look how mount_fusefs from ports compiles: > # mount_fusefs needs mntopts.h and getmntopts.c from src/sbin/mount/ > > The attached patch moves them to the IMHO architecturally more correct > place: a separate library -lutil. > sbin/mount/mntopts.h-> include/mntopts.h I think the mntopts.h should be moved to lib/libutil then, and installed by libutil Makefile. > sbin/mount/getmntopts.[3c] -> lib/libutil/getmntopts.[3c] I assume that the move is done by 'svn mv' to preserve the history. pgpAdbjc1IgDi.pgp Description: PGP signature
[patch] Proposal: move getmntopts(3) into libutil
Hi. The functions from sbin/mount/getmntopts.c are used in a bunch of other stuff like mount_* utilities which have to suck them in as their own functions in quite a hackish way. getmntopts.c copies are compiled in to an every utility-consumer instead of being present in one place. Looks like getmntopts.c was brought together with mount_ufs.c in 4.4BSD-Lite. After that other mount_* were converted to use getmntopts(). The utilities consuming getmntopts.c as currently present in HEAD: mount_smbfs fsck_ffs growfs mksnap_ffs mount mount_cd9660 mount_ext2fs mount_fusefs mount_hpfs mount_msdosfs mount_nfs mount_nullfs mount_reiserfs mount_std mount_udf mount_unionfs mount_nwfs mount_portalfs mount_smbfs mountd External mount-like utilities may also have difficulties with building to get getmntopts.c source as this requires /usr/src presence which is in sync with installed world. Look how mount_fusefs from ports compiles: # mount_fusefs needs mntopts.h and getmntopts.c from src/sbin/mount/ The attached patch moves them to the IMHO architecturally more correct place: a separate library -lutil. sbin/mount/mntopts.h-> include/mntopts.h sbin/mount/getmntopts.[3c] -> lib/libutil/getmntopts.[3c] The full list of functions in getmntopts.c: getmntopts() rmslashes() checkpath() build_iovec() build_iovec_argf() This will eventually give them public and documented status. It will also bring back to live its semi-dead man page getmntopts(3) currently disconnected from build, that will force us to update (and use) it which is also a goody. getmntopts.3 was never installed. As a bonus, it will bring us in sync with others BSDs. The attached patch was buildworld-tested and contains only minimal changes; getmntopts(3) updates and other improvements could be made separately. - rearrange files from sbin/mount/ to the new place - update Makefiles's of mount_* to use getmntopts(3) from libutil - #include "mntopts.h" -> #include Well, the include changes should be safe to commit as is. Below is a changelist from svn stat, for convenience 'sake. M contrib/smbfs/mount_smbfs/mount_smbfs.c M include/Makefile A +include/mntopts.h M lib/libutil/Makefile A +lib/libutil/getmntopts.3 A +lib/libutil/getmntopts.c M sbin/fsck_ffs/Makefile M sbin/growfs/Makefile M sbin/mksnap_ffs/Makefile M sbin/mount/Makefile D sbin/mount/getmntopts.3 D sbin/mount/getmntopts.c D sbin/mount/mntopts.h M sbin/mount/mount.c M sbin/mount/mount_fs.c M sbin/mount_cd9660/Makefile M sbin/mount_cd9660/mount_cd9660.c M sbin/mount_ext2fs/Makefile M sbin/mount_ext2fs/mount_ext2fs.c M sbin/mount_fusefs/Makefile M sbin/mount_fusefs/mount_fusefs.c M sbin/mount_hpfs/Makefile M sbin/mount_hpfs/mount_hpfs.c M sbin/mount_msdosfs/Makefile M sbin/mount_msdosfs/mount_msdosfs.c M sbin/mount_nfs/Makefile M sbin/mount_nfs/mount_nfs.c M sbin/mount_ntfs/Makefile M sbin/mount_ntfs/mount_ntfs.c M sbin/mount_nullfs/Makefile M sbin/mount_nullfs/mount_nullfs.c M sbin/mount_reiserfs/Makefile M sbin/mount_reiserfs/mount_reiserfs.c M sbin/mount_std/Makefile M sbin/mount_std/mount_std.c M sbin/mount_udf/Makefile M sbin/mount_udf/mount_udf.c M sbin/mount_unionfs/Makefile M sbin/mount_unionfs/mount_unionfs.c M usr.sbin/mount_nwfs/Makefile M usr.sbin/mount_nwfs/mount_nwfs.c M usr.sbin/mount_portalfs/Makefile M usr.sbin/mount_portalfs/mount_portalfs.c M usr.sbin/mount_smbfs/Makefile M usr.sbin/mountd/Makefile M usr.sbin/mountd/mountd.c -- wbr, pluknet getmntopts.patch Description: Binary data ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
