PATCH: New implementation of rlFileSave and rlFileRestore functions.

[Robin Hack]

Hello.

I wrote patch for beakerlib which reimplements rlFileBackup and 
rlFileRestore functions.

New functions use tar instead cp.
This patch is more intrusive.

New rlFileSave is able to:
- backup files with less lines of code (attributions preservation is 
delegated to tar)

New rlFileRestore is able to:
- log names of new created files/directories in backuped directory
- remove new created files/direcroties in backuped directory

Performance impacts is unknown for now.

It's first version. Needs heavy testing and review of more bash-skilled 
dudes .

Patch works on 379b446edcc3d1077041e3916d7a7887cfdf0937 git commit.

Have nice day.
Robin Hack

[Roman Rakus]

Some comments inline
On 05/13/2013 10:27 AM, Robin Hack wrote:
> diff -urpN ./beakerlib/src/infrastructure.sh beakerlib-new/src/infrastructure.sh
> --- ./beakerlib/src/infrastructure.sh	2013-05-13 10:23:20.322835505 +0200
> +++ beakerlib-new/src/infrastructure.sh	2013-05-13 10:21:22.806163234 +0200
> @@ -93,6 +93,106 @@ __INTERNAL_Mount(){
>       fi
>   }
>
> +__INTERNAL_Backup() {
> local file="$1"
> local backup="$2"
> # remove / at end of file name
> file=$(echo "${file}" | sed 's//$//')

echo is not safe. You can use:
file="${file%/}"
> # add what we backup to list
> grep -q -x "${file}" ${backup}/backup.txt

missing quotes? "${backup}"/backup.txt
> local res=$?
> if [ $res -ne 0 ]; then

You are not res variable any more, so not need for it. And [[ ]] are better:
   if [[ $? != 0 ]]; then
>    rlLogDebug "rlFileBackup: Add \"$file\" to backup list"
>    if ! echo "$file" >> ${backup}/backup.txt; then

printf '%s\n' "$file"
>        rlLogError "rlFileBackup: Failed! Add \"${file}\" to backup file list failed!"
>        return 5
>    fi
> fi
> local tar_name=$(echo "${file}.tar" | sed 's///##/g' | sed 's/\s/~~/g')
> if tar -S --selinux --xattrs --acls -Ppcvf  "$backup/${tar_name}" "${file}" > ${backup}/${tar_name}.file-list.txt; then
>    rlLogDebug "rlFileBackup: \"$file\" successfully backed up to \"$backup\" as \"${tar_name}\""
>    return 0
> fi
> rlLogError "rlFileBackup: Failed to backup file "${file}" to ${backup} directory."
> return 6
>
> +}
>
> +__INTERNAL_CleanUp() {
> local backup="$1"
> local tar_name="$2"
> local tmp_list="$3"
> local tmp_diff="$4"
> rm -rf "${backup}/${tar_name}.file-list.txt" "${backup}/${tar_name}" "${tmp_list}" "${tmp_diff}"
>
> +}
>
> +__INTERNAL_Restore_impl() {
> local file="$1"
> local backup="$2"
> local tmp_list=$(mktemp)
> # remove / at end of file name
> file=$(echo "${file}" | sed 's//$//')
> find "${file}" ( -type d -printf "%p/\n" -o -print ) | grep -v "^${file}$" > ${tmp_list}
> local tmp_diff=$(mktemp)
> local tar_name=$(echo "${file}.tar" | sed 's///##/g' | sed 's/\s/~~/g')
> grep -Fxv -f "$backup/${tar_name}.file-list.txt" "${tmp_list}" > "${tmp_diff}"
> # just log
> local OLD_IFS=$IFS
> IFS=$(echo -en "\n\b")

IFS=$'\n\b'
or better set IFS only for read command?
> while read f; do

Take care that backslash will be interpreted.
while IFS=$'\n\b' read -r f; do
>    rlLogDebug "rlFileRestore: Removing new file \"$f\""
>    if ! rm -rf "$f"; then
>        rlLogError "Can't remove new file \"$f\""
>    fi
> done < "${tmp_diff}"
> IFS=$OLD_IFS
> if tar SPxpf "${backup}/${tar_name}"; then
>    rlLogDebug "rlFileRestore: Restoring files from ${backup} successful"
>    __INTERNAL_CleanUp $backup $tar_name $tmp_list $tmp_diff

quotes
>    return 0
> fi
> # error fallback
> rlLogDebug "rlFileRestore: Command "tar SPxpf ${backup}/${tar_name}" failed!"
> rlLogError "rlFileRestore: Failed to restore files from "${backup}""
> __INTERNAL_CleanUp $backup $tar_name $tmp_list $tmp_diff

quotes
> return 2
>
> +}
>
> +__INTERNAL_Restore() {
> local backup="$1"
> if [ ! -s ${backup}/backup.txt ]; then

quotes
>    rlLogError "rlFileRestore: Cannot find backup.txt in \"${backup}\""
>    return 2
> fi
> status=0
> local OLD_IFS=$IFS
> IFS=$(echo -en "\n\b")
> while read file; do

same as above
>    __INTERNAL_Restore_impl "$file" "$backup"
>    status=$?
> done < "${backup}/backup.txt"
> IFS=$OLD_IFS
> # cleanup
> if ! rm -rf ${backup}/backup.txt; then
>    rlLogError "rlFileRestore: Can't delete \"${backup}/backup.txt\" file!"
> fi
> return $status
>
> +}
> # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> # rlMount
> # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> @@ -396,13 +496,6 @@ rlFileBackup() {
>       setfacl -m u:root:rwx $BEAKERLIB_DIR &>/dev/null \
>               && acl=true || acl=false
>       for file in "$@"; do
>    # convert relative path to absolute, remove trailing slash
>    file=$(echo "$file" | sed "s|^\([^/]\)|$PWD/\1|" | sed "s|/$||")
>    # follow symlinks in parent dir
>    path=$(dirname "$file")
>    path=$(readlink -n -f $path)
>    file=$path/$(basename $file)
>     # bail out if the file does not exist
>     if ! [ -e "$file" ]; then
>         rlLogError "rlFileBackup: File $file does not exist."
>
> @@ -410,41 +503,9 @@ rlFileBackup() {
>               continue
>           fi
>    # create path
>    if ! mkdir -p "${backup}${path}"; then
>        rlLogError "rlFileBackup: Cannot create ${backup}${path} directory."
>        status=5
>        continue
>    fi
>    # copy files
>    if ! cp -fa "$file" "${backup}${path}"; then
>        rlLogError "rlFileBackup: Failed to copy $file to ${backup}${path}."
>        status=6
>        continue
>    fi
>    # preserve path attributes
>    dir="$path"
>    failed=false
>    while true; do
>        $acl && { getfacl --absolute-names "$dir" | setfacl --set-file=- "${backup}${dir}" || failed=true; }
>        $selinux && { chcon --reference "$dir" "${backup}${dir}" || failed=true; }
>        chown --reference "$dir" "${backup}${dir}" || failed=true
>        chmod --reference "$dir" "${backup}${dir}" || failed=true
>        touch --reference "$dir" "${backup}${dir}" || failed=true
>        [ "$dir" == "/" ] && break
>        dir=$(dirname "$dir")
>    done
>    if $failed; then
>        rlLogError "rlFileBackup: Failed to preserve all attributes for backup path ${backup}${path}."
>        status=7
>        continue
>    fi
>    # everything ok
>    rlLogDebug "rlFileBackup: $file successfully backed up to $backup"
> done
>    __INTERNAL_Backup "$file" "${backup}"
>    status=$?
>
> done
>
> return $status
> }
>
> @@ -523,15 +584,12 @@ rlFileRestore() {
>           fi
>       done
> status=0
>  # restore the files
> if cp -fa "$backup"/* /; then
>    rlLogDebug "rlFileRestore: Restoring files from $backup successful"
> else
>    rlLogError "rlFileRestore: Failed to restore files from $backup"
>    return 2
> fi
> __INTERNAL_Restore "${backup}"
> status=$?
> return 0
> return $status
> }

RR

[Robin Hack]

Hi Roman.

Thanks for contribution.
I fixed mistakes.

Have nice day.

On 05/13/2013 11:00 AM, Roman Rakus wrote:
> Some comments inline
> On 05/13/2013 10:27 AM, Robin Hack wrote:
> > diff -urpN ./beakerlib/src/infrastructure.sh 
> > beakerlib-new/src/infrastructure.sh
> > --- ./beakerlib/src/infrastructure.sh    2013-05-13 
> > 10:23:20.322835505 +0200
> > +++ beakerlib-new/src/infrastructure.sh    2013-05-13 
> > 10:21:22.806163234 +0200
> > @@ -93,6 +93,106 @@ __INTERNAL_Mount(){
> >       fi
> >   }
> >
> > +__INTERNAL_Backup() {
> > local file="$1"
> > local backup="$2"
> > # remove / at end of file name
> > file=$(echo "${file}" | sed 's//$//')
>
> echo is not safe. You can use:
> file="${file%/}"
> > # add what we backup to list
> > grep -q -x "${file}" ${backup}/backup.txt
>
> missing quotes? "${backup}"/backup.txt
> > local res=$?
> > if [ $res -ne 0 ]; then
>
> You are not res variable any more, so not need for it. And [[ ]] are 
> better:
>   if [[ $? != 0 ]]; then
> >    rlLogDebug "rlFileBackup: Add \"$file\" to backup list"
> >    if ! echo "$file" >> ${backup}/backup.txt; then
>
> printf '%s\n' "$file"
> >        rlLogError "rlFileBackup: Failed! Add \"${file}\" to 
> >
> > backup file list failed!"
> >        return 5
> >    fi
> > fi
> > local tar_name=$(echo "${file}.tar" | sed 's///##/g' | sed
> >
> > 's/\s/~~/g')
> > if tar -S --selinux --xattrs --acls -Ppcvf
> >
> > "$backup/${tar_name}" "${file}" > 
> > ${backup}/${tar_name}.file-list.txt; then
> >    rlLogDebug "rlFileBackup: \"$file\" successfully backed up 
> >
> > to "$backup" as "${tar_name}""
> >    return 0
> > fi
> > rlLogError "rlFileBackup: Failed to backup file "${file}" to
> >
> > ${backup} directory."
> > return 6
> >
> > +}
> >
> > +__INTERNAL_CleanUp() {
> > local backup="$1"
> > local tar_name="$2"
> > local tmp_list="$3"
> > local tmp_diff="$4"
> > rm -rf "${backup}/${tar_name}.file-list.txt"
> >
> > "${backup}/${tar_name}" "${tmp_list}" "${tmp_diff}"
> > +}
> >
> > +__INTERNAL_Restore_impl() {
> > local file="$1"
> > local backup="$2"
> > local tmp_list=$(mktemp)
> > # remove / at end of file name
> > file=$(echo "${file}" | sed 's//$//')
> > find "${file}" ( -type d -printf "%p/\n" -o -print ) | grep -v
> >
> > "^${file}$" > ${tmp_list}
> > local tmp_diff=$(mktemp)
> > local tar_name=$(echo "${file}.tar" | sed 's///##/g' | sed
> >
> > 's/\s/~~/g')
> > grep -Fxv -f "$backup/${tar_name}.file-list.txt" "${tmp_list}" >
> >
> > "${tmp_diff}"
> > # just log
> > local OLD_IFS=$IFS
> > IFS=$(echo -en "\n\b")
>
> IFS=$'\n\b'
> or better set IFS only for read command?
> > while read f; do
>
> Take care that backslash will be interpreted.
> while IFS=$'\n\b' read -r f; do
> >    rlLogDebug "rlFileRestore: Removing new file \"$f\""
> >    if ! rm -rf "$f"; then
> >        rlLogError "Can't remove new file \"$f\""
> >    fi
> > done < "${tmp_diff}"
> > IFS=$OLD_IFS
> > if tar SPxpf "${backup}/${tar_name}"; then
> >    rlLogDebug "rlFileRestore: Restoring files from ${backup} 
> >
> > successful"
> >    __INTERNAL_CleanUp $backup $tar_name $tmp_list $tmp_diff
>
> quotes
> >    return 0
> > fi
> > # error fallback
> > rlLogDebug "rlFileRestore: Command "tar SPxpf
> >
> > ${backup}/${tar_name}" failed!"
> > rlLogError "rlFileRestore: Failed to restore files from
> >
> > "${backup}""
> > __INTERNAL_CleanUp $backup $tar_name $tmp_list $tmp_diff
>
> quotes
> > return 2
> >
> > +}
> >
> > +__INTERNAL_Restore() {
> > local backup="$1"
> > if [ ! -s ${backup}/backup.txt ]; then
>
> quotes
> >    rlLogError "rlFileRestore: Cannot find backup.txt in 
> >
> > "${backup}""
> >    return 2
> > fi
> > status=0
> > local OLD_IFS=$IFS
> > IFS=$(echo -en "\n\b")
> > while read file; do
>
> same as above
> >    __INTERNAL_Restore_impl "$file" "$backup"
> >    status=$?
> > done < "${backup}/backup.txt"
> > IFS=$OLD_IFS
> > # cleanup
> > if ! rm -rf ${backup}/backup.txt; then
> >    rlLogError "rlFileRestore: Can't delete 
> >
> > "${backup}/backup.txt" file!"
> > fi
> > return $status
> >
> > +}
> > #
> >   # rlMount
> >   # 
> >
> > @@ -396,13 +496,6 @@ rlFileBackup() {
> >       setfacl -m u:root:rwx $BEAKERLIB_DIR &>/dev/null \
> > && acl=true || acl=false
> >       for file in "$@"; do
> >    # convert relative path to absolute, remove trailing slash
> >    file=$(echo "$file" | sed "s|^\([^/]\)|$PWD/\1|" | sed 
> >
> > "s|/$||")
> >    # follow symlinks in parent dir
> >    path=$(dirname "$file")
> >    path=$(readlink -n -f $path)
> >    file=$path/$(basename $file)
> >     # bail out if the file does not exist
> >     if ! [ -e "$file" ]; then
> >         rlLogError "rlFileBackup: File $file does not exist."
> >
> > @@ -410,41 +503,9 @@ rlFileBackup() {
> >               continue
> >           fi
> >    # create path
> >    if ! mkdir -p "${backup}${path}"; then
> >        rlLogError "rlFileBackup: Cannot create ${backup}${path} 
> >
> > directory."
> >        status=5
> >        continue
> >    fi
> >    # copy files
> >    if ! cp -fa "$file" "${backup}${path}"; then
> >        rlLogError "rlFileBackup: Failed to copy $file to 
> >
> > ${backup}${path}."
> >        status=6
> >        continue
> >    fi
> >    # preserve path attributes
> >    dir="$path"
> >    failed=false
> >    while true; do
> >        $acl && { getfacl --absolute-names "$dir" | setfacl 
> >
> > --set-file=- "${backup}${dir}" || failed=true; }
> >        $selinux && { chcon --reference "$dir" "${backup}${dir}" 
> >
> > || failed=true; }
> >        chown --reference "$dir" "${backup}${dir}" || failed=true
> >        chmod --reference "$dir" "${backup}${dir}" || failed=true
> >        touch --reference "$dir" "${backup}${dir}" || failed=true
> >        [ "$dir" == "/" ] && break
> >        dir=$(dirname "$dir")
> >    done
> >    if $failed; then
> >        rlLogError "rlFileBackup: Failed to preserve all 
> >
> > attributes for backup path ${backup}${path}."
> >        status=7
> >        continue
> >    fi
> >    # everything ok
> >    rlLogDebug "rlFileBackup: $file successfully backed up to 
> >
> > $backup"
> > done
> >    __INTERNAL_Backup "$file" "${backup}"
> >    status=$?
> >
> > done
> >
> > return $status
> > }
> >
> > @@ -523,15 +584,12 @@ rlFileRestore() {
> >           fi
> >       done
> > status=0
> >  # restore the files
> > if cp -fa "$backup"/* /; then
> >    rlLogDebug "rlFileRestore: Restoring files from $backup 
> >
> > successful"
> > else
> >    rlLogError "rlFileRestore: Failed to restore files from 
> >
> > $backup"
> >    return 2
> > fi
> > __INTERNAL_Restore "${backup}"
> > status=$?
> > return 0
> > return $status
> > }
>
> RR
> _______________________________________________
> beakerlib-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/beakerlib-devel