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

Bugs about backup and crashrecovery #318

Open
rstech209 opened this issue Dec 3, 2021 · 0 comments
Open

Bugs about backup and crashrecovery #318

rstech209 opened this issue Dec 3, 2021 · 0 comments

Comments

@rstech209
Copy link

Bugs related to the management of backups (crashrecovery):
1- The information displayed in "recovery dirs" section by 'psd p' is not always correct.
2- Searching for the most recent backup (between the $BACK_OVFS and $BACKUP directories) will not work on some international versions of 'coreutils'
3- Backups (crashrecovery dirs) are not cleaned as expected in case of multi-profiles.

Detailed information:

Point 1: For example, with multi-profiles in the /home/user/.mozilla/firefox directory, crashrecovery dir created for one of the profiles will be displayed in the "recovery dirs" section of all profiles (Firefox).

Relevant function: parse()
Using the pattern "crashrecovery" (with the 'find' command) for all directories, regardless of the profile name is not sufficient.
On the other hand, the function will also explore all the subdirectories of $DIR to find the "crashrecovery" pattern. This can lead to hazardous deletions if the profile contains a directory with "crashrecovery" in the name...
The following change fixes the problem:

        while IFS= read -d '' -r backup; do
          CRASHArr=("${CRASHArr[@]}" "$backup")
        done < <(find "${DIR%/*}" -maxdepth 1 -type d -name "${item##*/}*crashrecovery*" -print0 | sort -r -z)

Point 2: The following syntax BACKUP_TIME=$(stat "$BACKUP" | grep Change.... used in function ungraceful_state_check() is not compatible with an internationalized version of the 'stat' command ("Change" being translated).
Rather than putting LANG = C; in front of the relevant command lines, the whole code can be simplified :

          BACKUP_TIME=$(stat --format=%Z "$BACKUP")
          BACK_OVFS_TIME=$(stat --format=%Z "$BACK_OVFS")

          [[ $BACK_OVFS_TIME -ge $BACKUP_TIME ]] &&
            TARGETTOKEEP="$BACK_OVFS" ||
            TARGETTOKEEP="$BACKUP"

Point 3 basically comes from the enforce() function.

  • No loading of env variables from the list of browsers (load_env_for not called)

  • No initialization loop of the variable $DIR (as is the case in the cleanup() function)

  • For the cleanup() function, same problem as in point 1 concerning the use of the "crashrecovery" pattern.

Otherwise, this point has already been addressed by the htower : Fix backups cleanup (PR #297)
The main idea is to pass the number of backups to keep as an argument of the cleanup() function.
Finally, this eliminates the need for the enforce() function by calling ' cleanup "$BACKUP_LIMIT" '.

By reshaping the code proposed by htower and adding the modification of the "crashrecovery" pattern processing (with the 'find' command)... It works !

I also observed that the creation of the crashrecovery dir is done by the ungraceful_state_check() function but the enforcement of the limit (max number of recovery directories set by "$BACKUP LIMIT") is only performed when executing a 'sync', more exactly the first invocation of psd resync by systemd psd-resync.service.
This leads to an ambiguous display after invoking 'psd p'.

I think enforce() (rather cleanup "$BACKUP_LIMIT" in the above solution) should be systematically invoked after calling the ungraceful_state_check() function (psd sync|resync|startup|parse)

Other fixes:
In ungraceful_state_check() function:
Message :

echo "Unexpected state detected: we have $BACKUP, but $DIR already exists. Trying move $DIR to $DIR" "$DIR-old-profile-$NOW"

Rather :

echo "Unexpected state detected: we have $BACKUP, but $DIR already exists. Trying move $DIR to $DIR-old-profile-$NOW"

In do_sync() function, forgotten variable :
echo "BACKUP_LIMIT=\"$BACKUP_LIMIT\""

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant