[systemd-devel] [RFC] link against util-linux for fstab parsing

[Dave Reisner]

On Mon, Feb 20, 2012 at 03:31:01PM +0100, Lennart Poettering wrote:
> On Fri, 17.02.12 16:47, Dave Reisner (d at falconindy.com) wrote:
> 
> > Based on the premise that we shouldn't develop a case of NIH, link
> > against a library whose sole purpose in life is parsing tab files.
> 
> Hmmm, using the glibc api setmntent() is hardly NIH, is it?
> 
> I am not strictly against this, but I'd like to have good reasons for
> this. Right now I see on the side against this:
> 
> - adds another dependency
> - code isn't really any shorter than right now
> 
> On the pro side:

Well, it does get shorter if you keep going with it (there's code in
umount that can be replaced). replacing the mountinfo sscanf parsing
with libmount seems like more of a win to me. My working tree is
currently looking like +172/-219 and replaces code in
src/{,u}mount.c and src/cryptsetup/cryptsetup-generator.c

Rewriting remount-api-vfs purely with libmount almost works, but then
doesn't if you want to be able to fork for each mount call. Karel, maybe
I missed something... mnt_context_next_mount() doesn't seem to let you
filter in the way sd's mount_point_is_api() does (filtering on target),
but you can't fork internally for mount(2) if you don't use that call.

Perhaps Karel can be a better salesman than I can. =P

> Dave, Karel, can you fill in any reasons here? If not I think we
> shouldn't apply this patch...
> 
> > Curious if something like this is wanted -- it's 90% complete, but there's no
> > sense in finishing it up if it's not interesting. I'd like to be able to get
> > rid of the fstab_node_to_udev_node() function but that would likely require
> > linking against libblkid in cryptsetup-generator.
> 
> I am not sure about this? What's wrong with fstab_node_to_udev_node()?

Nothing's wrong with it. I was a bit hasty in assuming it could be
ditched -- the value of it is that it always creates a "resolved" path
for LABEL and UUID tags without actually checking for existance. Trying
to resolve LABEL=foo via mnt_resolve_spec() for a crypto volume before
its unlocked of course leads to Bad Things, as I found out in testing.

Was a fun experiment, but if all we can really get out of this is
replacing the parsers, maybe this isn't worth it.

d