hunleyd / btrfs-auto-snapshot

BTRFS Automatic Snapshot Service for Linux
GNU General Public License v2.0
16 stars 4 forks source link

Trailing / (slash) missing between mount path and subvolume name #16

Closed a1466d44-d3dc-4c0b-90c7-315b088731d7 closed 4 months ago

a1466d44-d3dc-4c0b-90c7-315b088731d7 commented 1 year ago

Issue with variable definition They are missing a / in between mount path and subvolume name

The error seems to be within the sed statement on line 446 (but I'am not 100% sure)

The issue of my error message ERROR: cannot find real path for '/srv/databackups': No such file or directory Definetly comes from a missing / (slash) between the mount path and the subvolume (backup in this example)

abs_path="$(echo "${subvol}" | sed -r "s!^${mp_subvol}!${mp}!")"

bash -x btrfs-auto-snapshot --keep=1 --label=daily --verbose --dry-run /srv/data
...
+++ btrfs subvolume list -o /srv/data
+++ awk '{print $9}'
++ mp_subvols='backups
db
webroot
.btrfs/btrfs-auto-snap_daily_2023-02-07-1807'
++ IFS=
++ read -r subvol
++ '[' -z backups ']'
++ local abs_path
++ local pattern
++ local matches
+++ echo backups
+++ sed -r 's!^!/srv/data!'
++ abs_path=/srv/databackups
+++ echo /srv/databackups
+++ sed -r 's!^//!/!'
++ abs_path=/srv/databackups
+++ printf '%s\n' /srv/data
++ pattern=/srv/data
+++ echo /srv/data
+++ grep --count -F -f - -x /dev/fd/63
++++ echo /srv/databackups
++ matches=0
++ '[' 0 '!=' 0 ']'
++ local show
++ local sp
++ local no_parent_uuid
++ local is_read_only
+++ btrfs subvolume show /srv/databackups
ERROR: cannot find real path for '/srv/databackups': No such file or directory
...

Software used

$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 11 (bullseye)
Release:        11
Codename:       bullseye
$ btrfs --version
btrfs-progs v5.10.1 
$ uname -a
Linux rockpi 5.15.84-v8+ #1613 SMP PREEMPT Thu Jan 5 12:03:08 GMT 2023 aarch64 GNU/Linux
hunleyd commented 1 year ago

i wonder if this is related to #15? i'll look into it in the near term. thanks for the report!

hunleyd commented 1 year ago

hi @a1466d44-d3dc-4c0b-90c7-315b088731d7 ! can you please share the output of sudo subvolume list / and the contents of /etc/fstab to help me debug this? thx!

a1466d44-d3dc-4c0b-90c7-315b088731d7 commented 1 year ago

Of course /but I think you meant sudo btrfs subvolume list / because:

$ sudo subvolume list /
sudo: subvolume: command not found

;)

$ sudo btrfs subvolume list /
ERROR: not a btrfs filesystem: /
ERROR: can't access '/'

In my case the Voulme is mounted under /srv/data

$ sudo btrfs subvolume list /srv/data 
ID 258 gen 38 top level 5 path backups
ID 259 gen 35 top level 5 path db
ID 260 gen 37 top level 5 path webroot
ID 267 gen 50 top level 5 path .btrfs/btrfs-auto-snap_daily_2023-02-07-1807

and

$ cat /etc/fstab 
proc            /proc           proc    defaults          0       0
PARTUUID=9ae39d54-01  /boot           vfat    defaults          0       2
PARTUUID=9ae39d54-02  /               ext4    defaults,noatime  0       1
# a swapfile is not a swap partition, no line here
#   use  dphys-swapfile swap[on|off]  for that
tmpfs /tmp tmpfs defaults,noatime,nosuid,nodev 0 0
tmpfs /var/tmp tmpfs defaults,noatime,nosuid,nodev 0 0
UUID="1d73c848-3197-4577-bb13-02426ef08624" /srv/data   btrfs   auto,nofail,noatime,rw,user 0   0

Anything else I can help you with?

a1466d44-d3dc-4c0b-90c7-315b088731d7 commented 1 year ago

For me the fix was on line 446, by just adding a trailing slash But I'm not sure if thats korrekt?

My proposal (without any further knowledge of other usecases or propper usage of the variables) would be:

On Line 446 add the check if the variable mp contains a trailing slash and append one if it doesn't

[ "${mp}" != */ ]] && mp="${mp}/"
abs_path="$(echo "${subvol}"   | sed -r "s!^${mp_subvol}!${mp}!")"
hunleyd commented 1 year ago

Sorry for the thinko in my command :)

I'll test to make sure that tweak doesn't impact any of the other ways people do their mounts and let you know when it's committed for testing.

ams-tschoening commented 1 year ago

Adding the code there seems somewhat safe to me, especially as line 447 removes redundant slashes already. Other places of the code remove slashes as well, like in the following example:

snaps_dir="${i%/}/${DEF_SNAPS_DIR}"

So it might even make sense to always end given paths with a slash?

ret_val[paths]="$*"

Looking at my test-VM I seem to have mostly tested with //, so it might simply be safer to add only the concrete change were things break right now.

hunleyd commented 1 year ago

@a1466d44-d3dc-4c0b-90c7-315b088731d7 can you test #18 pls?

a1466d44-d3dc-4c0b-90c7-315b088731d7 commented 1 year ago

Unfortunately, my testsystem is currently not working... I hope to get a replacement in the next week or so. Please stand by (but as I have read it, it looks ok)

hunleyd commented 11 months ago

looking for feedback on #18 still. any chance @a1466d44-d3dc-4c0b-90c7-315b088731d7 or @ams-tschoening can look at it by halloween?

a1466d44-d3dc-4c0b-90c7-315b088731d7 commented 11 months ago

looking for feedback on #18 still. any chance @a1466d44-d3dc-4c0b-90c7-315b088731d7 or @ams-tschoening can look at it by halloween?

I got my hardware replacement, but had no time to test it yet. Unfortunately I can not say more about it than sorry... (children currently consume more an more of my previous spare time :)

hunleyd commented 11 months ago

looking for feedback on #18 still. any chance @a1466d44-d3dc-4c0b-90c7-315b088731d7 or @ams-tschoening can look at it by halloween?

I got my hardware replacement, but had no time to test it yet. Unfortunately I can not say more about it than sorry... (children currently consume more an more of my previous spare time :)

no worries @a1466d44-d3dc-4c0b-90c7-315b088731d7 . i totally understand.