#1348 PR merged: First draft of TSM backup

Labels: enhancement, fixed / solved / done

schabrolles opened issue at 2017-05-06 15:34:

This is the first draft of TSM backup.

Now, a TSM incremental backup is started when the command rear mkbackup or rear mkbackuponly is executed.

  • TSM client must be installed and properly configured
  • file exclusion is managed by TSM in configuration files.

jsmeix commented at 2017-05-08 08:15:

@schabrolles
many thanks for your contribution to make
TSM support better in ReaR, cf.
https://github.com/rear/rear/issues/823

Just be courageous and enhance the curently rather
poor TSM support in ReaR - it cannot get worse ;-)

gdha commented at 2017-07-04 09:02:

@schabrolles When rear mkbackup triggers a TSM incremental shouldn't we describe this change in behaviour somehow? It was said rear mkbackup is the same as rear mkrescue with commercial backup software. Now at least for TSM rear mkbackup behaves different, no?

gdha commented at 2017-07-04 09:03:

@jsmeix @schlomo what are your thoughts?

gdha commented at 2017-12-22 08:04:

@schabrolles Could you refresh this PR? Or, when it is polished then you may commit this PR.

schabrolles commented at 2018-01-09 15:47:

@jsmeix could you have a quick look before I merge this one.
Thanks.

jsmeix commented at 2018-01-10 10:05:

@schabrolles
could you also have a look at my
https://github.com/rear/rear/pull/1643#issuecomment-354775699

Regarding this pull request here:

To me the code related to the include_list array looks "problematic"
because it seems you use an array as if it was a string.
In general with

for word in foo bar baz ; do arr+=( "$word " ) ; done

each array element gets a trailing blank character:

# for element in "${arr[@]}" ; do echo "'$element'" ; done
'foo '
'bar '
'baz '

Accordingly I think your code should be

# Create TSM friendly backup include list.
for backup_include in $( cat $TMP_DIR/backup-include.txt ) ; do
    include_list+=( "$backup_include" )
done

Then I wonder why you don't use

... dsmc incremental ... "${include_list[@]}" ...

cf. https://github.com/rear/rear/issues/1068

using ${array[@]} without double-quotes is ... problematic

In particular because for me on SLES12 with btrfs I have

BACKUP_PROG_INCLUDE=( '/var/cache/*' '/var/lib/mailman/*' '/var/tmp/*' '/var/lib/pgsql/*' '/usr/local/*' '/opt/*' '/var/lib/libvirt/images/*' '/boot/grub2/i386/*' '/var/opt/*' '/srv/*' '/boot/grub2/x86_64/*' '/var/lib/mariadb/*' '/var/spool/*' '/var/lib/mysql/*' '/tmp/*' '/home/*' '/var/log/*' '/var/lib/named/*' '/var/lib/machines/*' )

and then backup-include.txt contains e.g.

# cat /tmp/rear.PAoykNbFTZqPeEW/tmp/backup-include.txt
/var/cache/*
/var/lib/mailman/*
/var/tmp/*
/var/lib/pgsql/*
/usr/local/*
/opt/*
/var/lib/libvirt/images/*
/boot/grub2/i386/*
/var/opt/*
/srv/*
/boot/grub2/x86_64/*
/var/lib/mariadb/*
/var/spool/*
/var/lib/mysql/*
/tmp/*
/home/*
/var/log/*
/var/lib/named/*
/var/lib/machines/*
/

I wonder if possibly unquoted '*' characters might lead
to unwanted bash evaluation of the include_list array
elements?
I.e. check the log file what the actual "dsmc incremental"
command options are and whether or not they are
as intended.

The new TSM_DSMC_BACKUP_OPTIONS config array variable
should be explained (or at least mentioned) in default.conf

The condition

if test $dsmc_exit_code -lt 12; then

has the implicit assumption all dsmc exit codes >= 12 are errors.
Is this really true or is only dsmc exit code 12 a real error?

I still have the question in my
https://github.com/rear/rear/pull/1348#pullrequestreview-36713346
see also
https://github.com/rear/rear/issues/823
I guess that the include_list array implements that
via BACKUP_PROG_INCLUDE (cf. item 1 above)
but then this behaviour should be explained in
the "BACKUP=TSM stuff" section in default.conf

gdha commented at 2018-01-12 08:19:

@schabrolles I'm OK with this PR.

jsmeix commented at 2018-01-12 15:18:

According to
https://github.com/rear/rear/issues/823#issuecomment-357253860
the current implementation that calls

dsmc incremental ... "${include_list[@]}"

might be in conflict with what the user has set in TSM own files
which tell what to backup and what to exclude.

There should be only one single authoritative source of information
that tells what to include in the backup and what to exclude.

Accordingly I think it works perhaps more fail-safe to remove
the current include_list array code and call only plain

dsmc incremental

to get a TSM backup that contains what TSM own config files tell.

But this is only a blind comment from a non-TSM user.
In the end whatever works reasonably for you is o.k.

schabrolles commented at 2018-01-12 15:27:

@jsmeix , I need to perform additional test to understand how include_list and TSM include file interact.

jsmeix commented at 2018-03-24 13:24:

@schabrolles
do you perhaps find some time to merge it?
A very first initial state is perfectly fine.

schabrolles commented at 2018-04-01 08:09:

@jsmeix,
Let me retest this one on my systems on top of the current master branch to be sure.
I'll merge it next week.


[Export of Github issue for rear/rear.]