On 2012-12-17 11:53, Russell Coker wrote: > http://etbe.coker.com.au/2012/12/17/using-btrfs/ > > I've written my own scripts to manage snapshot backups on BTRFS and published > them at the above URL.
Hi Russell,
Reading your scripts, I noticed a few of coding practices that I
thought worth commenting on:
I'd write it like this, using 'while read' and quoting "$n" just in
case. I realise these may not cause *you* grief, but giving you're
posting these scripts, I think they should be a little more robust:
| #!/bin/bash
| # Re the for loop, don't do that [1]
| while read -r n
| echo "$n"
| ssh "$n" "btrfs scrub start -B -d /"
| done < <(xm list|cut -f1 -d\ |egrep -v ^Name\|^Domain-0)
btrfs-make-snapshot (untested, but should work, and hopefully you get my
intent):
| #!/bin/bash
| set -e
|
| # usage:
| # btrfs-make-snapshot [-d] minutes|days paths
| # example:
| # btrfs-make-snapshot minutes /home /mail
|
| btrfs()
| {
| if (( debug == 1 ))
| then
| echo btrfs "$@"
| else
| command btrfs "$@"
| fi
| }
|
| # Using capitalised variable names make them look like environment
| # variables and may cause clashes if you're not careful. "/msg greybot
| # !varcaps" for more info
| # Also http://mywiki.wooledge.org/BashFAQ/050 re: storing commands in
| # variables.
| if [ "$1" == "-d" ]; then
| debug=1
| shift
| fi
|
| if [ "$1" == "minutes" ]; then
| date=$(date +%Y-%m-%d:%H:%M)
| else
| date=$(date +%Y-%m-%d)
| fi
| shift
|
| # $* is unsafe. "$@" is almost always what you want.[2]
| for n in "$@" ; do
| btrfs subvol snapshot -r "$n" "$n/backup/$date"
| done
btrfs-remove-snapshots (same deal as above):
| #!/bin/bash
| set -e
|
| # usage:
| # btrfs-remove-snapshots [-d] MINSNAPS DAYSNAPS paths
| # example:
| # btrfs-remove-snapshots 100 100 /home /mail
|
| btrfs()
| {
| if (( debug == 1 ))
| then
| echo btrfs "$@"
| else
| command btrfs "$@"
| fi
| }
|
| if [ "$1" == "-d" ]; then
| debug=1
| shift
| fi
|
| minSnaps=$1
| shift
| # FIXME: Russell, you never use daySnaps. Is that a bug? Should it be
| # in the second loop below? I don't use btrfs so am not sure.
| daySnaps=$1
| shift
|
| for dir in "$@" ; do
| base=$(echo "$dir" | cut -c 2-200)
| while read -r n
| btrfs subvol delete /"$n"
| done < <(btrfs subvol list "$dir"|grep "$base"/backup/.*:|head -n
-"$minSnaps"|sed -e "s/^.* //")
| while read -r n
| btrfs subvol delete /"$n"
| done < <(btrfs subvol list "$dir"|grep "$base"/backup/|grep -v :|head -n
-"$minSnaps"|sed -e "s/^.* //")
| done
That's more than enough ranting and preaching out of me; hope I'm more
enlightening than annoying.
1.
2012-12-17_12:00:11 -!- Irssi: Starting query in Freenode with greybot
2012-12-17_12:00:19 <mattcen> !for$(
2012-12-17_12:00:20 <greybot> Never do this: for x in $(command) or `command`
or $var. for-in is used for iterating arguments,
not (output) strings. Instead, use arrays or a
while-read loop. See
http://mywiki.wooledge.org/BashSheet#Arrays
http://mywiki.wooledge.org/DontReadLinesWithFor
http://mywiki.wooledge.org/BashFAQ/001
2.
2012-12-17_12:18:38 <mattcen> !$*
2012-12-17_12:18:39 <greybot> The difference between $@ and $*: Unquoted
(don't
do this!), none at all: both equal $1 $2 ....
With double quotes, "$@" expands each element as
an argument: "$1" "$2" ..., while "$*" expands
to
all elements merged into one argument:
"$1c$2c..." (where c is the first character of
IFS). You almost always want "$@". The same goes
for arrays: "${myarray[@]}"
--
Regards,
Matthew Cengia
signature.asc
Description: Digital signature
_______________________________________________ luv-main mailing list [email protected] http://lists.luv.asn.au/listinfo/luv-main
