Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy backup restore log into recreated system (related to pull request 1797) #1803

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented May 8, 2018

@jsmeix jsmeix added the enhancement Adaptions and new features label May 8, 2018
@jsmeix jsmeix added this to the ReaR v2.4 milestone May 8, 2018
@jsmeix jsmeix self-assigned this May 8, 2018
@jsmeix jsmeix requested a review from schabrolles May 8, 2018 13:51
@jsmeix
Copy link
Member Author

jsmeix commented May 8, 2018

FYI:
This is not yet finished.
Currently it is my very first steps so that you can see early what I do
and provide early feedback if I do something plain wrong.

Copy link
Member

@schabrolles schabrolles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove "/" from $fileplace when creating backup_restore_log_file variable

for num in $TSM_RESTORE_FILESPACE_NUMS ; do
filespace="${TSM_FILESPACES[$num]}"
LogUserOutput "Restoring TSM filespace $filespace"
# Create backup restore log file name (a different one for each filespace):
backup_restore_log_file=$VAR_DIR/restore/$backup_restore_log_file_prefix-$filespace-restore.log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue here is that $fileplace are PATH like "/ /home/ /var/lib/". Then your variable looks like:
backup_restore_log_file=TSM-/var/lib/-restore.log. We have to substitute "/" by something like "_" to avoid issues.

I propose something like:

backup_restore_log_file=$VAR_DIR/restore/$backup_restore_log_file_prefix-ROOT${filespace//\//_}-restore.log

This will generate backup_restore_log_file=TSM-ROOT_var_lib_-restore.log

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the information!
I will improve it to get a readable file name.

@jsmeix
Copy link
Member Author

jsmeix commented May 9, 2018

Should be better now but still not finished.

@schabrolles
see my current usr/share/rear/restore/NETFS/default/400_restore_backup.sh
where now both stdout and stderr are redirected into the backup restore log file,
see the comments in the code for the reason why.

Perhaps also for usr/share/rear/restore/TSM/default/400_restore_with_tsm.sh
both stdout and stderr should be redirected into the backup restore log file
if you like it this way?

Copy link
Member

@schabrolles schabrolles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log copy is currently blocked when the restore/layout/recovery sub-directories are already created. The use of mkdir -p should easily solve the problem.

2018-05-10 10:42:25.418151042 Running exit tasks
2018-05-10 10:42:25.419730157 Exit task 'mkdir -p -m 0700 /mnt/local//var/log/rear/recover && cp -p /var/log/rear/rear-rear-sles12-143.log /mnt/local//var/log/rear/recover/rear-rear-sles12-143.log || true'
2018-05-10 10:42:25.423562735 Exit task 'mkdir /mnt/local//var/log/rear/recover/layout && cp -pr /var/lib/rear/layout/* /mnt/local//var/log/rear/recover/layout || true'
mkdir: cannot create directory '/mnt/local//var/log/rear/recover/layout': File exists
2018-05-10 10:42:25.425868407 Exit task 'mkdir /mnt/local//var/log/rear/recover/recovery && cp -pr /var/lib/rear/recovery/* /mnt/local//var/log/rear/recover/recovery || true'
mkdir: cannot create directory '/mnt/local//var/log/rear/recover/recovery': File exists
2018-05-10 10:42:25.428149059 Exit task 'mkdir /mnt/local//var/log/rear/recover/restore && cp -pr /var/lib/rear/restore/* /mnt/local//var/log/rear/recover/restore || true'
mkdir: cannot create directory '/mnt/local//var/log/rear/recover/restore': File exists

# (see AddExitTask in _input-output-functions.sh) the ordering of how AddExitTask is called
# must begin with the to-be-last-run exit task and end with the to-be-first-run exit task:
if test "$( echo $VAR_DIR/restore/* )" ; then
copy_restore_log_exit_task="mkdir $recovery_system_recover_log_dir/restore && cp -pr $VAR_DIR/restore/* $recovery_system_recover_log_dir/restore || true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to use mkdir -p this will avoid to black the copy if the directory is already created.

copy_restore_log_exit_task="mkdir $recovery_system_recover_log_dir/restore && cp -pr $VAR_DIR/restore/* $recovery_system_recover_log_dir/restore || true"
AddExitTask "$copy_restore_log_exit_task"
fi

# Do not copy layout and recovery related files for the 'restoreonly' workflow:
if ! test $WORKFLOW = "restoreonly" ; then
copy_layout_files_exit_task="mkdir $recovery_system_recover_log_dir/layout && cp -pr $VAR_DIR/layout/* $recovery_system_recover_log_dir/layout || true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: mkdir -p

copy_restore_log_exit_task="mkdir $recovery_system_recover_log_dir/restore && cp -pr $VAR_DIR/restore/* $recovery_system_recover_log_dir/restore || true"
AddExitTask "$copy_restore_log_exit_task"
fi

# Do not copy layout and recovery related files for the 'restoreonly' workflow:
if ! test $WORKFLOW = "restoreonly" ; then
copy_layout_files_exit_task="mkdir $recovery_system_recover_log_dir/layout && cp -pr $VAR_DIR/layout/* $recovery_system_recover_log_dir/layout || true"
copy_recovery_files_exit_task="mkdir $recovery_system_recover_log_dir/recovery && cp -pr $VAR_DIR/recovery/* $recovery_system_recover_log_dir/recovery || true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: mkdir -p

@schabrolles
Copy link
Member

@jsmeix
I agree with #1803 (comment). We should also redirect stderr to the log file.

Copy link
Member

@schabrolles schabrolles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid merge conflict, could you also redirect to current TSM log into the $backup_restore_log_file by using -errorlognam=\" $backup_restore_log_file\" dsmc parameter.

# Make sure filespace has a trailing / (for dsmc):
test "${filespace:0-1}" == "/" || filespace="$filespace/"
LogUserOutput "Restoring TSM filespace $filespace"
Log "Running 'LC_ALL=$LANG_RECOVER dsmc restore $filespace $TARGET_FS_ROOT/$filespace -subdir=yes -replace=all -tapeprompt=no ${TSM_DSMC_RESTORE_OPTIONS[@]}'"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add the following options to TSM restore: -errorlogname=\"$backup_restore_log_file\". This will also put all the errors reported by TSM into the current log files.

Log "Running 'LC_ALL=$LANG_RECOVER dsmc restore $filespace $TARGET_FS_ROOT/$filespace -subdir=yes -replace=all -tapeprompt=no ${TSM_DSMC_RESTORE_OPTIONS[@]}'"
# Regarding usage of '0<&6 1>&7 2>&8' see "What to do with stdin, stdout, and stderr" in https://github.com/rear/rear/wiki/Coding-Style
LC_ALL=$LANG_RECOVER dsmc restore "$filespace" "$TARGET_FS_ROOT/$filespace" -subdir=yes -replace=all -tapeprompt=no "${TSM_DSMC_RESTORE_OPTIONS[@]}" 0<&6 1>"$TMP_DIR/TSM-restore.log" 2>&8
LC_ALL=$LANG_RECOVER dsmc restore "$filespace" "$TARGET_FS_ROOT/$filespace" -subdir=yes -replace=all -tapeprompt=no "${TSM_DSMC_RESTORE_OPTIONS[@]}" 0<&6 1>"$backup_restore_log_file" 2>&8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: add the following options to TSM restore: -errorlogname=\"$backup_restore_log_file\"

Copy link
Member Author

@jsmeix jsmeix May 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schabrolles
I have a dim feeling that special care is needed when two or more processes "just write"
into the same file because I think I remember that in this case the processes must
not "just write " but "append " into the same file because otherwise the processes
may overwrite each others outputs (at least if output happens simlutaneously).
Currently I have no time to dig into that issue but hopefully later this week
or perhaps next week...

Cf. my comments in my current
https://github.com/jsmeix/rear/blob/ef20c659cfd57c6ef590e96092ff6828d817b7c8/usr/share/rear/restore/NETFS/default/400_restore_backup.sh
currently starting at line 135 that reads

# To be more on the safe side append to the log file '>>' instead of plain writing to it '>'
# because when a program (bash in this case) is plain writing to the log file it can overwrite
# output of a possibly simultaneously running process that likes to append to the log file
# (e.g. when background processes run that also uses the log file for logging)
# cf. https://github.com/rear/rear/issues/885#issuecomment-310308763
.
.
.
... 1>>$backup_restore_log_file 2>&1

I wonder what exactly -errorlogname=$backup_restore_log_file does:
Does it plain write to the file like 2>$backup_restore_log_file does it
(i.e. with truncating existing file content when opening that file)
or does it append to the file like 2>>$backup_restore_log_file
which would be o.k.?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsmeix, I just made a test and it works perfectly. I can see the errors sent by TSM multiplexed in the logfile.

@jsmeix
Copy link
Member Author

jsmeix commented May 17, 2018

@schabrolles
if you have time for it I would appreciate it if you could test
my latest enhancements regarding TSM backup restore.

@jsmeix
Copy link
Member Author

jsmeix commented May 23, 2018

@schabrolles @gdha
feel free to postpone this one to ReaR 2.5
or "just merge" it into ReaR 2.4 if you like it as is.

I think currently not all relevant places are fixed but
I think the current state may be already sufficient
for 'tar' and TSM backup.

@gdha
Copy link
Member

gdha commented May 28, 2018

@schabrolles You may decide to merge the PR for 2.4 or not.

@schabrolles
Copy link
Member

@gdha,
I will start a test tonight and come back to you tomorrow.

Copy link
Member

@schabrolles schabrolles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me... tested with rhel6 rhel7 sles11 sles12 ubuntu and NETFS backup (tar)
And sles12sp2 + TSM.

I think we can merge this one for 2.4

@gdha gdha merged commit f914bd1 into rear:master May 29, 2018
@gdha
Copy link
Member

gdha commented May 29, 2018

@schabrolles Thank you for your intensive testing!

@jsmeix jsmeix deleted the copy_backup_restore_log_into_recreated_system_related_to_pull_request_1797 branch June 4, 2018 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants