Skip to content

Conversation

@karelzak
Copy link
Contributor

This pull request is replacement of the obsolete #194.

_Note that after merge systemd will depend on util-linux v2.27 (now -rc1), the package is available in f23 and fedora rawhide._

dnf --enablerepo rawhide update util-linux

is your friend if you use f22 (it's safe to upgrade). Not sure about another distros. The final v2.27 release is planned at the end of the August.

@zonque
Copy link
Member

zonque commented Aug 18, 2015

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.

@zonque
Copy link
Member

zonque commented Sep 7, 2015

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?

@martinpitt
Copy link
Contributor

@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.

@martinpitt
Copy link
Contributor

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?

@zonque
Copy link
Member

zonque commented Sep 8, 2015

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

@martinpitt
Copy link
Contributor

@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)

@martinpitt
Copy link
Contributor

@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.

@zonque
Copy link
Member

zonque commented Sep 8, 2015

@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 :)

@martinpitt
Copy link
Contributor

@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.

@martinpitt
Copy link
Contributor

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.

@zonque
Copy link
Member

zonque commented Sep 8, 2015

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.

@martinpitt
Copy link
Contributor

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?

@zonque
Copy link
Member

zonque commented Sep 8, 2015

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?

@poettering
Copy link
Member

@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.

@zonque
Copy link
Member

zonque commented Sep 8, 2015

@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?

@martinpitt
Copy link
Contributor

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! :-)

@karelzak
Copy link
Contributor Author

karelzak commented Sep 8, 2015

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.)

@zonque
Copy link
Member

zonque commented Sep 9, 2015

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.

@poettering
Copy link
Member

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

@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed needs-discussion 🤔 postponed labels Sep 9, 2015
@karelzak
Copy link
Contributor Author

karelzak commented Sep 9, 2015

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

@karelzak
Copy link
Contributor Author

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.

@mbiebl
Copy link
Contributor

mbiebl commented Sep 11, 2015

2015-09-11 12:06 GMT+02:00 Karel Zak notifications@github.com:

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?

@martinpitt
Copy link
Contributor

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!

@mbiebl
Copy link
Contributor

mbiebl commented Sep 11, 2015

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.

@johannbg
Copy link
Contributor

@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 ).

@mbiebl
Copy link
Contributor

mbiebl commented Sep 11, 2015

@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.

@karelzak
Copy link
Contributor Author

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

The current implementation directly monitor /proc/self/mountinfo and
/run/mount/utab files. It's really not optimal because utab file is
private libmount stuff without any official guaranteed semantic.

The libmount since v2.26 provides API to monitor mount kernel &
userspace changes and since v2.27 the monitor is usable for
non-root users too.

This patch replaces the current implementation with libmount based
solution.

Signed-off-by: Karel Zak <kzak@redhat.com>
@karelzak
Copy link
Contributor Author

Rebased & improved (based on @poettering feedback).

poettering added a commit that referenced this pull request Sep 22, 2015
mount: use libmount to monitor mountinfo & utab
@poettering poettering merged commit 85fade1 into systemd:master Sep 22, 2015
alban added a commit to alban/rkt that referenced this pull request Oct 12, 2015
The src flavor with systemd from git master requires the last version of
libmount.

Symptoms:
> configure: error: *** libmount support required but libraries not found

Semaphore does not have the correct version. This patch installs the
last version as explained on systemd/systemd#986
alban added a commit to alban/rkt that referenced this pull request Oct 12, 2015
The src flavor with systemd from git master requires the last version of
libmount.

Symptoms:
> configure: error: *** libmount support required but libraries not found

Semaphore does not have the correct version. This patch installs the
last version as explained on systemd/systemd#986
alban added a commit to alban/rkt that referenced this pull request Oct 12, 2015
The src flavor with systemd from git master requires the last version of
libmount.

Symptoms:
> configure: error: *** libmount support required but libraries not found

Semaphore does not have the correct version. This patch installs the
last version as explained on systemd/systemd#986
@karelzak karelzak deleted the monitor branch November 2, 2015 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks

Development

Successfully merging this pull request may close these issues.

6 participants