Dont delete user content #1625
Merged
Dont delete user content #1625
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The remove_files_in_manifest_from_fs function isn't used and was a user of a rather dangerous recursive remove function. Get rid of it to avoid potential confusion. Signed-off-by: William Douglas <[email protected]>
bryteise
force-pushed
the
dont-delete-user-content
branch
from
1547203
to
9c4befb
Compare
fenrus75
reviewed
Apr 4, 2024
src/swupd_lib/target_root.c
Outdated
| @@ -330,6 +333,61 @@ static enum swupd_code install_dir(const char *fullfile_path, const char *target | |||
| return install_dir_using_tar(fullfile_path, target_file); | |||
| } | |||
|
|
|||
| /* Backup target directory to a backup and timestamp prefixed name. */ | |||
| /* This function should only be used after the deletes are processed. */ | |||
| static enum swupd_code move_to_timestamp(const char *filename) | |||
There was a problem hiding this comment.
maybe call it "dirname" not "filename" to indicate we expect it to be a directory?
fenrus75
reviewed
Apr 4, 2024
src/swupd_lib/target_root.c
Outdated
|
|
||
| /* It shouldn't be possible to have an already created backup_dir | ||
| but just in case, try and remove it and error out if unable to. */ | ||
| ret = sys_rm_recursive(backup_dir); |
There was a problem hiding this comment.
shuold we actually check if this is there first? just incase we have some bug... then it only ever happens if it actually exists
There was a problem hiding this comment.
Sure, less to go wrong. I suspect if things are going wrong in this area though we are in a whole different world of hurt that'll show up all over in testing.
fenrus75
approved these changes
Apr 4, 2024
bwarden
reviewed
Apr 4, 2024
Currently sys_rm_recursive was used in any instance of deleting swupd content from the system (update, repair and bundle-remove). This can cause user data loss when unkown files are in directories that swupd is deleting. To prevent this, this patch changes how deleting content in swupd operates. Swupd content removal is now done with sys_rm and the return value is checked in case the removal failed due to a directory that still had files in it. When this specific failure occurs, the directory is added to a new list for reprocessing removals as it is expected once the rest of the deletes on the system occur the failures will go away as the directories will be empty (these deletes are processed in alphabetical reverse order so leaf directories are processed first). If the removal fails again it is presumed the contents of the directory are not files swupd knows about and as such should be kept somewhere else. For handling the retention of user data, directories (with only the content unknown to swupd) are renamed (currently using a .deleted.$timestamp. prefix of the old name) and stored at the same directory level they were previously found with one exception. The exception is for nested deleted content best illustrated with an example: /swupd-dir1/user-file1 /swupd-dir1/swupd-dir2/user-file2 When swupd tries to remove the /swupd-dir1 content, it will store the user files as follows: /.deleted.$timestamp1.swupd-dir1/user-file1 /.deleted.$timestamp1.swupd-dir1/.deleted.$timestamp1.swupd-dir2/user-file2 To demarcate what was part of swupd content vs user content. Signed-off-by: William Douglas <[email protected]>
bryteise
force-pushed
the
dont-delete-user-content
branch
from
9c4befb
to
987ee9e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.