Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions scripts/beesd.in
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ readonly bees_bin=$(realpath @LIBEXEC_PREFIX@/bees)

command -v "$bees_bin" &> /dev/null || ERRO "Missing 'bees' agent"

uuid_valid(){
if uuidparse -n -o VARIANT $1 | grep -i -q invalid; then
false
fi
}

help(){
echo "Usage: beesd [options] <btrfs_uuid>"
echo "- - -"
Expand Down Expand Up @@ -62,9 +56,13 @@ for arg in "${ARGUMENTS[@]}"; do
done

for arg in "${NOT_SUPPORTED_ARGS[@]}"; do
if uuid_valid $arg; then
[ ! -z "$UUID" ] && help
UUID=$arg
if [ ! -z "$(blkid --uuid $arg -o value | head -1)" ]; then
UUID="$arg"
INFO "Using UUID $arg"
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
Copy link
Contributor

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

Copy link
Author

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

[ -z "$UUID" ] && help
either way as UUID is never set if we don't find it ?

done

Expand Down
5 changes: 3 additions & 2 deletions scripts/beesd@.service.in
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
[Unit]
Description=Bees (%i)
Documentation=https://github.com/Zygo/bees
After=sysinit.target
#After=sysinit.target
Copy link
Contributor

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.

Copy link
Author

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

After=local-fs.target

[Service]
Type=simple
Expand Down Expand Up @@ -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
Copy link
Contributor

@kakra kakra Jun 7, 2022

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.