-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
using device mapper name for systemd unit instead #224
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fine apart from some nitpicks and oversights.
But this makes me think if we should rather use a systemd-generator in the long term which generates systemd units on the fly for each btrfs partition found and configured in the config file?
@@ -1,7 +1,8 @@ | |||
[Unit] | |||
Description=Bees (%i) | |||
Documentation=https://github.com/Zygo/bees | |||
After=sysinit.target | |||
#After=sysinit.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say, simply remove the commented line. After=local-fs.target
is actually correct here. I'm using the same dependency in my custom crafted unit file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was a mistake perhaps. I don't know why I wanted it before the plymouth
start, perhaps I was testing from the frame-buffer console and it was crashing because my space ran out.
If it wants local-fs.target
that's a dependency of sysinit.target
, then it shouldn't matter if it starts before or after sysinit.target
.
systemctl list-dependencies sysinit.target
sysinit.target
● ├─apparmor.service
● ├─blk-availability.service
● ├─debug-shell.service
● ├─dev-hugepages.mount
● ├─dev-mqueue.mount
● ├─haveged.service
● ├─keyboard-setup.service
● ├─kmod-static-nodes.service
● ├─lvm2-lvmpolld.socket
● ├─lvm2-monitor.service
○ ├─plymouth-read-write.service
● ├─plymouth-start.service
● ├─proc-sys-fs-binfmt_misc.automount
● ├─resolvconf.service
● ├─sys-fs-fuse-connections.mount
● ├─sys-kernel-config.mount
● ├─sys-kernel-debug.mount
● ├─sys-kernel-tracing.mount
○ ├─systemd-ask-password-console.path
○ ├─systemd-binfmt.service
○ ├─systemd-boot-system-token.service
○ ├─systemd-journal-flush.service
● ├─systemd-journald.service
○ ├─systemd-machine-id-commit.service
● ├─systemd-modules-load.service
○ ├─systemd-pstore.service
○ ├─systemd-random-seed.service
○ ├─systemd-repart.service
● ├─systemd-sysctl.service
● ├─systemd-sysusers.service
● ├─systemd-timesyncd.service
● ├─systemd-tmpfiles-setup-dev.service
● ├─systemd-tmpfiles-setup.service
● ├─systemd-udev-trigger.service
● ├─systemd-udevd.service
● ├─systemd-update-utmp.service
● ├─cryptsetup.target
● ├─integritysetup.target
● ├─local-fs.target
● │ ├─-.mount
● │ ├─boot.mount
● │ ├─home.mount
● │ ├─systemd-remount-fs.service
● │ ├─tmp.mount
● │ ├─var-data\x2dcache.mount
● │ └─var.mount
● ├─swap.target
● │ ├─dev-mapper-data\x2d\x2dvg\x2dswap0.swap
● │ └─dev-mapper-data\x2d\x2dvg\x2dswap1.swap
● └─veritysetup.target
fi | ||
if [ ! -z "$(blkid /dev/mapper/$arg -o value | head -1)" ]; then | ||
UUID="$(blkid /dev/mapper/$arg -o value | head -1)" | ||
INFO "Found $UUID for $arg" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This misses the "help" branch condition which has been there previously. Please convert to using elif
and finally end with help
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it going to fall to
Line 69 in fbdfd9b
[ -z "$UUID" ] && help |
@@ -56,4 +57,4 @@ AmbientCapabilities=CAP_DAC_OVERRIDE CAP_DAC_READ_SEARCH CAP_FOWNER CAP_SYS_ADMI | |||
NoNewPrivileges=true | |||
|
|||
[Install] | |||
WantedBy=basic.target | |||
WantedBy=multi-user.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this... While it may fix your problem (and isn't actually incorrect), I rather think that it solves your problem only by accident. After=local-fs.target
should be enough for your lvm setup to become ready, otherwise we may need another After=
line maybe?
Don't get me wrong: I think this change should go into the commit (actually, that's what my custom-crafted unit file also uses). But I think it doesn't reliably solve your problem. I think in case of lvm, there may be another synchronization point missing. What's the output of systemd-analyze critical-chain
?
PS: I wonder why the bees-provided unit file still contains the old broken dependencies. I'm pretty sure this has been discussed before.
Why don't we use the mapper name in the systemd unit alias, the uuid is kinda ugly when I do
systemctl status
.Also, I don't have uuidparse on my system.
I also needed the multi-user.target patch because I use lvm and my btrfs is on top of it. (this should be a separe commit, I can fix it later)
Nevertheless it seems to be working in my build system machine. Thanks for the nice project I use for saving space of my stupid
.o
objects.