mount: use libmount to monitor mountinfo & utab - #986

I guess we should need to wait for v2.27 final before merging this.

@martinpitt, could you provide a PPA for 14.04? That would be needed for Sempahore to work.

Ok, v2.27 is now final, thanks @karelzak!
@martinpitt, could you make us a PPA for Ubuntu 14.04, so we can feed Semaphore with it?

@zonque: Working on a PPA. The package needs some adjustments to work on 14.04; in particular I need to disable the systemd support to avoid circular build deps, and backport some build tools. But I should have it working today.

I prepared a PPA with that: https://launchpad.net/~pitti/+archive/ubuntu/systemd-semaphore . This is a new one which will only get backports for these upstream semaphore builds, nothing else. (I have different PPAs for packages of systemd itself).

So doing

sudo add-apt-repository ppa:pitti/systemd-semaphore
sudo apt-get update
sudo apt-get -y dist-upgrade

on these boxes should work now. I tested the upgrade and a reboot on a current 14.04 cloud image (which I suppose is very close to what semaphore uses), and it works well enough. @zonque , can you please give this a spin?

@martinpitt for this to work, we need to have a newer version of libmount as well. Could you include that package in the PPA?

@zonque : it's all there -- can you please post apt-cache policy libmount1? (See https://launchpad.net/~pitti/+archive/ubuntu/systemd-semaphore/+packages, expand the "util-linux" line)

@zonque, ah, I see it:

libmount-dev is already the newest version.

I. e. you already have the 14.04 version installed. You did the add-apt-repository call before the apt-get update call, you need to swap it. Otherwise apt won't know about the new source; add-apt-repository only adds the /etc/apt/sources.list.d/ snippet, but doesn't call apt-get update by itself.

@martinpitt yes, sorry, my bad. That was a Semaphore command ordering issue. That's also related to a bug in their UI which I just reported :)

@zonque: Cool, seems this part worked now. The three test failures look suspiciously similar to issue #1132 , that's a bit weird -- I'm fairly sure /sys/fs/cgroup is mounted in your test env as issue #1132 didn't appear there.

Oh wait -- could it be that this just checks out the proposed branch, instead of pulling the proposed commits on top of trunk and building that? That might also explain why it can build the package (ish) but there are conflicts when merging.

Sure, there is no 'trunk' in git ;) Semaphore checks out the branch in question, and builds it, no matter what it is based on. And this branch is based on a patch that is /way/ older than the unified cgroups rework, so that's most definitely unrelated. I'm digging.

Sure, there is no 'trunk' in git ;)

Sorry, I meant "master" of course :-)

I did a manual run of the commands in https://semaphoreci.com/systemd/systemd/branches/pull-request-986/builds/1 in a 14.04 cloud image, and build/tests succeed. Can you do a run on master, to compare?

The problem is that mnt_monitor_enable_userspace() fails with -ENOSYS if /etc/mtab is present. I added a Semaphore command now to purge it, and that makes the tests succeeds.

However, I'm not sure if we can make a non-existing /etc/mtab a hard requirement for all systemd users. As things are right now with this PR applied, PID1 will bail out early if that file exists.

@karelzak could you shed some light on the rationale here?

@zonque we have not supported systems with /etc/mtab as real file since a long long time. We even taint the system if you have one there and warn loudly at boot. Yes, /etc/mtab as symlink or non-existant is a hard requirement of systemd and has been since a long long time.

@poettering ok, makes sense. However, we currently just warn about the fact. With this PR merged, PID1 simply hits an assertion and dies. So we need some better way to catch that.

Anyway, this branch needs rebasing. @karelzak do you wanna to do that?

FTR, I did add the --enable-libmount-force-mountinfo configure option, but apparently that doesn't apply to the assertion in mnt_monitor_enable_userspace()? Thanks @zonque for sorting this out, green is good! :-)

Rebased.

Notes:

• the purpose of the patch is to make systemd sensitive to libmount private (userspace) mount options.

• I cannot imagine system where libmount/mount(8) uses /run/mount/utab, but some another utils (/sbin/mount., df, ..) write or read the classic mtab file.

• libmount monitor does not support /etc/mtab at all, it would be possible to implement it, but I have doubts anyone needs it.

• libmount for backward compatibility supports classic /etc/mtab, this compatibility is enabled when regular mtab is detected. This detection is disabled by --enable-libmount-force-mountinfo to make (for example) systemd based systems more robust (imagine stupid obsolete script with "touch /etc/mtab" during boot). I'll improve the monitor to be also sensitive to --enable-libmount-force-mountinfo.

  You don't want to use /etc/mtab. Kill it ASAP. Really.

The /etc/mtab file is mess on all systems with or without systemd. It provides non-senses about bind mounts, NFS ro/rw flags, it's incompatible with namespaces, etc.

/run/mount/utab is not perfect (for example it's also not compatible with namespaces), but we use it only for very small subset of mounpoints where userspace mount options are specified and on many system it's empty file. (And it's private file, no API like mtab.)

Ok, understood. We should not, however, confront the user with a cryptic error message (Failed to fully start up daemon: Function not implemented) but rather make the already existing check for etc/mtab fatal. This is what #1214 implements.

Looks good otherwise. Might need rebase due to #1218 having been merged.

I'll cleanup & rebase later today. Thanks for review.

Rebased & improved (based on @poettering feedback).

Note I have removed mtab detection from mnt_monitor_enable_userspace() libmount upstream code (the change will be in 2.27.1). It seems better to rely on mtab check during systemd startup bcce718 than try to detect broken system setting in libmount.

2015-09-11 12:06 GMT+02:00 Karel Zak [email protected]:

Rebased & improved (based on @poettering https://github.com/poettering
feedback).

Note I have removed mtab detection from mnt_monitor_enable_userspace()
libmount upstream code (the change will be in 2.27.1). It seems better to
rely on mtab check during systemd startup bcce718
bcce718
than try to detect broken system setting in libmount.

I think it's fine to bump the log message from warning to error, but given
the recent change in libmount, can we please revert the hunk that freezes
PID1.

Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?

can we please revert the hunk that freezes PID1

Seconded -- at least you should get a rescue shell. We've seen all kinds of weird scripts that try to carelessly sed /etc/mtab, or backups restoring it as a file, etc. This is obviously a grave bug, but rendering your system entirely unbootable doesn't give you any way to fix it. Thanks for considering!

As an alternative, we might just as well let systemd turn /etc/mtab into a symlink if it isn't one yet.

We sort of do that already in Debian where we've been shipping a .service unit running really early during boot which converted /etc/mtab into a symlink to /proc/mounts. We've been shipping that for the last two releases and we didn't have any complaints because of that.

Freezing is really user unfriendly. As Martin mentioned, there are enough buggy scripts or old backups out there. And if this renders your server which is 5000km away unbootable without OOB access, then this is not the best user experience.

@mbiebl downstream that still needs this ( I think Arch,Fedora,Mageia,Opensuse and probably more all stop doing that in 2011 ) should be able to carry downstream patch that reverts this and the use case Martin points out should not be applicable since you would not be shipping these changes in core/baseOS components for your stable version ( unless for some reason you suddenly decided shipping updated core/baseOS component in stable and or LTS releases ).

@johannbg you missed the point where I mentioned that in Debian we converted /etc/mtab to a symlink two releases ago, so your complaint is unfounded.
That doesn't prevent buggy scripts or old backups to turn /etc/mtab back into a file.
Rendering your system unbootable because of that doesn't seem reasonable to me.

We want error on regular mtab, if you have regular mtab, then your system is no compatible with systemd. That's all the sorry.

Rebased & improved (based on @poettering feedback).