On Fri, 10 Feb 2017 09:57:32 -0500, Dave Reisner wrote:
+ LC_ALL=C btrfs subvolume show "$1" | sed -n 's/^\tSubvolume ID:\s*//p'
This looks like you're parsing human readable output to get the subvol ID. Is this really the only way to get this information?
As far as I can tell, yes.
+) + +# Usage: btrfs_subvolume_list_all $FILEPATH +# +# Given $FILEPATH somewhere on a mounted btrfs filesystem, print the +# ID and full path of every subvolume on the filesystem, one per line +# in the format "$ID $PATH". +btrfs_subvolume_list_all() ( + set -o pipefail + local mountpoint + mountpoint="$(df --output=target "$1" | sed 1d)" || return $?
"return $?" is a long winded way of just "return". The status is implied.
Huh. TIL. Thanks.
+ LC_ALL=C btrfs subvolume list -a "$mountpoint" | sed -r 's|^ID ([0-9]+) .* path (<FS_TREE>/)?(\S*).*|\1 \3|'
You might want to validate that you actually get an integer back here, rather than just assuming the calls succeed. You cannot rely on sed to provide a useful exit status.
I suppose that would be a good idea. The `set -o pipefail` a couple of lines up will catch the cases where the failure is from `btrfs`; the check would be for if btrfs's output format changes. I really should have called it out in a comment and the commit message that that regex is the place where I most expect btrfs to break us in the future. The output format is a space-separated sequence of "key value key value..."; the problem is that both keys and values can contain space, and there's no escaping or indication of when this happens. The reason I tacked `^` in the expression is that there's already a "parent ID" key. The continued success of the regex assumes that there will never be another key or value that contains " path", and that there is no space in the path value. So yeah, I probably should add a check.
+ id="$(btrfs_subvolume_id "$subvolume")" || return $? + all="$(btrfs_subvolume_list_all "$subvolume")" || return $? + path="$(sed -n "s/^$id //p" <<<"$all")"
Rather than injecting unvalidated data into a sed program, I'd suggest using awk:
path=$(awk -v id="$1" '$1 == id { sub($1 FS, ""); print }' <<<"$all")
Sure. Sed was simpler, and I figured that if `btrfs` starts misbehaving, the whole thing is hosed anyway.
+ subvolumes=($(btrfs_subvolume_list "$dir")) || return $?
This is a broken way to read lines into an array, because you're not reading lines at all -- you're reading words.
+ for subvolume in "${subvolumes[@]}"; do + btrfs subvolume delete "$dir/$subvolume" || return $? + done
Yeah, but the lines are guaranteed to be words because of the `(\S*)` in the regexp in `btrfs_subvolume_list_all`. But I should have written it as subvolumes="$(btrfs_subvolume_list "$dir")" || return for read -r subvolume; do btrfs subvolume delete "$dir/$subvolume" || return done <<<"$subvolumes" -- Happy hacking, ~ Luke Shumaker