nspawn: convert var-lib-machines.mount to an automount#6095
nspawn: convert var-lib-machines.mount to an automount#6095poettering merged 2 commits intosystemd:masterfrom
Conversation
|
CI fails because packaging needs adjusting (var-lib-machines.mount is no longer enabled) |
poettering
left a comment
poettering
left a comment
There was a problem hiding this comment.
so i figure conceptually we can do this, but this must be conditioned differently as both machined and nspawn use the dir...
| if ENABLE_MACHINED | ||
| MACHINES_TARGET_WANTS += \ | ||
| var-lib-machines.automount | ||
| endif |
There was a problem hiding this comment.
this doesn't look right. nspawn uses the dir too (in fact, it's the dir's primary user if you so will), hence this should be pulled in if nspawn is enabled too.
There was a problem hiding this comment.
Nspawn uses the directory, but it does not manipulate it and does not care how it is mounted. Since #6079 , systemd-nspawn@ has a RequiresMountsFor=/var/lib/machines, which should take care of nspawn.
OTOH, it is machined that creates and manages /var/lib/machines.raw, so if machined is not built, neither should the mount unit, and configuring /var/lib/machines is completely up to the admin.
There was a problem hiding this comment.
I didn't change this as per my reasoning above. Please indicate if you still want this changed.
Makefile.am
Outdated
|
|
||
| dist_systemunit_DATA += \ | ||
| units/var-lib-machines.mount \ | ||
| units/var-lib-machines.automount |
There was a problem hiding this comment.
this part is simply wrong, nspawn uses this too, see other comment.
In fact [email protected] should probably get an explicit RequiresMountsFor= for the container dir it operates on
There was a problem hiding this comment.
Same as other comment, nspawn is a mere user and as such having the mount units when nobody will create the underlying loop device is not useful.
poettering
added
the
reviewed/needs-rework 🔨
(Well, it's more difficult than this. /var is pulled in explicitly from basic.target, hence still part of the early boot even if located on NFS) |
|
So, one thing I don't really like about this is that it somewhat interferes with the user's config if he decides to make /var/lib/machines a mount point of his own (which is a very OK thing to do). So far, by placing an entry in /etc/fstab that overrode the mount unit we ship and all was good. However, this now gets weirdly affected by the automount, as the user's mount unit will now be accompanied by our .automount unit, which however is conditioned out in the usual case... And that's just weird... Dunno if this is a problem IRL, but I am not sure I like it too much... I wonder if it wouldn't be nicer to solve this differently: don't bother with an automount unit at all, but instead make it a normal mount unit, and then:
That way the unit is pulled in late boot only, but we get all the necessary deps in place without any automount unit. Does this make sense? |
Indeed. That's an unfortunate side effect of automounts being a separate unit from the main mount. Alternatively we could have a generator but it sounds overkill.
This makes sense either way, I'll add it.
remote-fs.target sounds a bit unrelated. I initially proposed in #1175 to have the automount hooked up in sysinit.target. If we go automount-less, we could also hook it up there, but without ordering dependencies. This way it would always be brought up, but it could be done late if necessary. I'm not sure what would adding it to machines.target would accomplish.
This is already done since #6079
I think it does. The only question would be where to pull it in. |
It does if you think of it as "this is where remote mounts are pulled in". But if you think about it as "this is the latest where mounts are pulled in" then it starts making sense. That said, I don't think adding this dep actually really makes sense, as mounts marked "auto" are either pulled in by local-fs.target or remote-fs.target anyway, hence there's no need to pull them in explicitly by remote-fs.target a second time. Hence ignore this bit.
So, we document that people can access /var/lib/machines directly too, and drop their stuff there, i.e. that nspawn and machined are happy to use it, but that it isn't private property of either, but anyone is welcome to drop stuff there. Hence I think it would make a certain amount of sense to say that if you start machines.target — which I think should mean something like "please start up the VM/container virtualization subsystem and related stuff now" would also ensure the dir is then available, even if you don't actually have any nspawn instances enabled in it. |
…target /var can be on a remote filesystem, thus hooking it to local-fs.target is not correct. Also, only install the mount unit when machined is enabled, because machined is the one managing the underlying device, and thus makes no sense without machined. Fixes systemd#1175
Since any part of the path could be remote mounted, make sure they are before starting machined
|
Updated as per #6095 (comment) . I decided to use the regular add-wants machinery instead of using |
poettering
removed
the
reviewed/needs-rework 🔨
|
@fsateler @mbiebl @martinpitt any chance you can fix the packaging to adapt to this PR which I just merged so that the CI works again? Thank you! |
|
@poettering: I adjusted the packaging to not care any more into which target(s) it gets installed. I verified that it doesn't break the downstream build, and it should work for upstream now. Unfortunately I can't test it any more on this PR as it's already closed - but e. g. PR #6715 ought to fail still, and I'll retry it to verify. Next time, it'd be great if we could do the downstream adjustments before merging, so that we can use the PR that introduces the change to verify. |
Imported using git-ubuntu import. Changelog parent: 4d48d62 New changelog entries: [ Martin Pitt ] * Adjust var-lib-machines.mount target. Upstream PR systemd#6095 changed the location to {remote-fs,machines}.target.wants, so just install all available ones. [ Dimitri John Ledkov ] * Fix out-of-bounds write in systemd-resolved. CVE-2017-9445 (Closes: #866147, LP: #1695546) [ Michael Biebl ] * Be truly quiet in systemctl -q is-enabled (Closes: #866579) * Improve RLIMIT_NOFILE handling. Use /proc/sys/fs/nr_open to find the current limit of open files compiled into the kernel instead of using a hard-coded value of 65536 for RLIMIT_NOFILE. (Closes: #865449) [ Nicolas Braud-Santoni ] * debian/extra/rules: Use updated U2F ruleset. This ruleset comes from Yubico's libu2f-host. (Closes: #824532)
3 participants
/var can be on a remote filesystem, thus hooking it to local-fs.target is not correct.
Lets convert it to an automount, and have the automount be pulled in by machines.target.
This way, it should be available whenever nspawn machines are enabled.
Fixes #1175
While doing this I noticed that the mount unit was being installed even when machined is disabled. I changed that.