HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Yet another bash backup script, using rsync --link-dest

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptyetdestanotherusinglinkbashrsyncbackup

Problem

I’ve written this bash backup script. It uses the --link-dest option of rsync; that way, the user have access to the backed data at any time stamped with relatively affordable data overhead. Any duplicated data should be hard linked; the overhead mostly come from the directory structure.

It’s mostly based on this very nice guide by Mike Rubel and various other contributors, as well as a few answers from Unix SE and other web reference.

The script is meant to be run at regular (typically, hourly) intervals with cron and other scripts are in charge of safely keeping daily/weekly backups.

Of course, I want to minimize the size of the backups. I also want to delete older backups before newer ones. To do so, I build ${backups}, an array of every backup1 sorted by modification date from newest to oldest. Thus ${backups[0]} (if it exists) is the latest complete backup and ${backups[@:$n]} for some integer $n lists every backup but the $n newest (from 0 to $n - 1).

As always with bash scripts, I’m especially afraid of quoting issues, but any remark is welcome.

I in particular quite dislike how I use find with both -mindepth and maxdepth, but couldn’t find any way around it.

Most of the “standard” commands, such as cut, sort or grep, are provided by BusyBox 1.16.1 and may not have every option available on most recent Linux distribution. cut, in particular, does not understand the -d option, hence the ugly tr trick.

```
#!/bin/bash

# Check if we are root (no one else should run this)
# ==================================================

if (( $(id -u) != 0 )); then
echo "/ ! \ Only root can run this script. Backup cancelled." >&2
exit 3
fi

# Functions
# =========

# Given a date (or a placeholder), returns the corresponding hourly backup name
function scheme {
local token="$@"
echo "hourly ${token}"
}

# Parameters
# ==========

# password_file=/etc/backup/passwd # Network yet untested
backup_directory=/path/to/bac

Solution

The script is nicely written. I only have minor suggestions that are barely more than nitpicks.

Function declaration style

Instead of this:

function scheme {


The generally preferred style for declaring functions is this:

scheme() {


Redundant local variable

The local variable token is redundant here:

function scheme {
    local token="$@"
    echo "hourly ${token}"
}


You could simplify to:

echo "hourly $@"


Simplify condition

This condition can be simplified:

if (( ${#backups[@]} > 0 )); then
    latest_backup="${backups[0]}"
else
    latest_backup=""
fi


To just this:

latest_backup="${backups[0]}"


Instead of this:

if [[ "${password_file}" != "" ]]; then


You can omit the != "":

if [[ "${password_file}" ]]; then


Don't repeat yourself

The echo statement is duplicated for the sake of underlining:

echo "Backing up ${source_directory}"
echo "Backing up ${source_directory}" | sed "s/./=/g"


It would be good to create a helper function for this purpose:

print_heading() {
    echo "$@"
    echo "$@" | sed "s/./=/g"
}

Code Snippets

function scheme {
function scheme {
    local token="$@"
    echo "hourly ${token}"
}
echo "hourly $@"
if (( ${#backups[@]} > 0 )); then
    latest_backup="${backups[0]}"
else
    latest_backup=""
fi
latest_backup="${backups[0]}"

Context

StackExchange Code Review Q#126697, answer score: 2

Revisions (0)

No revisions yet.