-
Notifications
You must be signed in to change notification settings - Fork 63
using device mapper name for systemd unit instead #224
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
base: master
Are you sure you want to change the base?
Conversation
kakra
left a comment
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?
| 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
| 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 |
|
|
||
| [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
.oobjects.