btrfs-progs: restore symlinks
Subsystems (2)
cover btrfs-progs: restore symlinks
Hopefully this is sufficiently paranoid, tested with PATH_MAX length symlinks, existing files, insufficient permissions, dangling symlinks. I think I got the coding style correct this time, I'll fix and resend if not.
Includes a trivial fix from my metadata patch, the documentation got lost in the merge.
PATCH 1/3 btrfs-progs: restore: document metadata restore.
This was lost in the cleanup of 71a559
--- Documentation/btrfs-restore.asciidoc | 3 +++ 1 file changed, 3 insertions(+)
Documentation/btrfs-restore.asciidoc +3 -0
@@ -29,6 +29,9 @@ get snapshots, btrfs restore skips snapshots in default. -x:: get extended attributes. +-m|--metadata:: +restore owner, mode and times. + -v:: verbose.
Signature · 1 line
2.1.4
PATCH 2/3 btrfs-progs: separate the overwrite check.
In reply to Dan MerillatSymlink restore needs this, but the cut&paste became too complicated. Simplify everything.
--- cmds-restore.c | 53 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 19 deletions(-)
cmds-restore.c +34 -19
@@ -781,6 +781,37 @@ out:return ret; } +/* returns: +* 0 if the file exists and should be skipped. +* 1 if the file does NOT exist +* 2 if the file exists but is OK to overwrite +*/ + +static int overwrite_ok (const char * path) +{ +static int warn =0 ; +struct stat st; +int ret; + +/* don't be fooled by symlinks */ + ret =fstatat (-1 , path_name, &st,AT_SYMLINK_NOFOLLOW ); + +if (!ret) { +if (overwrite) +return 2 ; + +if (verbose || !warn) +printf ("Skipping existing file" +" %s\n" , path); +if (!warn) +printf ("If you wish to overwrite use " +"the -o option to overwrite\n" ); + warn =1 ; +return 0 ; + } +return 1 ; +} +static int search_dir (struct btrfs_root *root,struct btrfs_key *key,const char *output_rootdir,const char *in_dir,const regex_t *mreg)
@@ -897,25 +928,9 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,* files, no symlinks or anything else. */ if (type ==BTRFS_FT_REG_FILE ) { -if (!overwrite) { -static int warn =0 ; -struct stat st; - - ret =stat (path_name, &st); -if (!ret) { - loops =0 ; -if (verbose || !warn) -printf ("Skipping existing file" -" %s\n" , path_name); -if (warn) - goto next; -printf ("If you wish to overwrite use " -"the -o option to overwrite\n" ); - warn =1 ; - goto next; - } - ret =0 ; - } +if (!overwrite_ok (path_name)) + goto next; +if (verbose)printf ("Restoring %s\n" , path_name);if (dry_run)
Signature · 1 line
2.1.4
Re: [PATCH 2/3] btrfs-progs: separate the overwrite check.
Quoted 4 lines · Dan Merillat
cmds-restore.c +4
-0+/* returns: +* 0 if the file exists and should be skipped. +* 1 if the file does NOT exist +* 2 if the file exists but is OK to overwrite
cmds-restore.c out: · line 797
Quoted 10 lines
cmds-restore.c +10
-0+*/ + +static int overwrite_ok (const char * path) +{ +static int warn =0 ; +struct stat st; +int ret; + +/* don't be fooled by symlinks */ + ret =fstatat (-1 , path_name, &st,AT_SYMLINK_NOFOLLOW );
Is the filedescriptor -1 correct? Previously, stat was used that uses AT_FDCWD for the dirfd, which is -100. -1 could be intepreted as a bad filedescriptor (EBADF).
Re: [PATCH 2/3] btrfs-progs: separate the overwrite check.
It's a good question. If path_name is absolute, the file descriptor is ignored. I used -1 (EBADF) instead of AT_FDCWD there so if a non-absolute path gets in there it errors out instead of attempting to use a relative path off the current directory.
I'm not entirely sure if it's the best way, so if anyone else has ideas let me know.
Quoted 19 lines · David Sterba
Quoted 4 lines · Dan Merillat
cmds-restore.c +4
-0+/* returns: +* 0 if the file exists and should be skipped. +* 1 if the file does NOT exist +* 2 if the file exists but is OK to overwrite cmds-restore.c out: · line 797
Quoted 10 lines
cmds-restore.c +10
-0+*/ + +static int overwrite_ok (const char * path) +{ +static int warn =0 ; +struct stat st; +int ret; + +/* don't be fooled by symlinks */ + ret =fstatat (-1 , path_name, &st,AT_SYMLINK_NOFOLLOW );Is the filedescriptor -1 correct? Previously, stat was used that uses
AT_FDCWDfor the dirfd, which is -100. -1 could be intepreted as a bad filedescriptor (EBADF).
Re: [PATCH 2/3] btrfs-progs: separate the overwrite check.
Quoted 2 lines · Dan Merillat
It's a good question. If
path_nameis absolute, the file descriptor is ignored.
AFAICS the paths are just appended to what the user specifies on the commandline, so it's not necessarily an absolute path. However, it still works with -1 as expected. I'll merge the patch as-is (scheduled for 4.0.1) so there's time to test it.
PATCH 3/3 btrfs-progs: optionally restore symlinks.
In reply to Dan MerillatRestore symlinks, optionally with owner/times.
--- Documentation/btrfs-restore.asciidoc | 3 + cmds-restore.c | 140 ++++++++++++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 3 deletions(-)
Documentation/btrfs-restore.asciidoc +3 -0
@@ -32,6 +32,9 @@ get extended attributes. -m|--metadata:: restore owner, mode and times. +-S|--symlinks:: +restore symbolic links as well as normal files. + -v:: verbose.
cmds-restore.c +137 -3
188 lines not on this page · raw message
Signature · 1 line
2.1.4
Dan Merillat posted on Thu, 23 Apr 2015 12:47:29 -0400 as excerpted:
Quoted 7 lines
Hopefully this is sufficiently paranoid, tested with
PATH_MAXlength symlinks, existing files, insufficient permissions, dangling symlinks. I think I got the coding style correct this time, I'll fix and resend if not.Includes a trivial fix from my metadata patch, the documentation got lost in the merge.
Thanks for all this. I've only had to use restore once and hopefully won't be using it again in the near future, but having it restore the metadata and symlinks as well would surely have made the experience easier. There's a lot of people going to benefit from these patches over time as btrfs gains usage and the inevitable breakage happens to some of those filesystems. =:^/
Signature · 3 lines
Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman
At this point I I'm done - after writing the symlink patch I restored everything of importance off my array to a scratch disk, wiped the array and am in the process of copying everything back. I'll keep an eye on this thread if changes need to be made to my patches, but hopefully I won't be needing btrfs restore for a few more years!
Quoted 26 lines · Duncan
Dan Merillat posted on Thu, 23 Apr 2015 12:47:29 -0400 as excerpted:
Quoted 7 lines
Hopefully this is sufficiently paranoid, tested with
PATH_MAXlength symlinks, existing files, insufficient permissions, dangling symlinks. I think I got the coding style correct this time, I'll fix and resend if not.Includes a trivial fix from my metadata patch, the documentation got lost in the merge.
Thanks for all this. I've only had to use restore once and hopefully won't be using it again in the near future, but having it restore the metadata and symlinks as well would surely have made the experience easier. There's a lot of people going to benefit from these patches over time as btrfs gains usage and the inevitable breakage happens to some of those filesystems. =:^/
--
Duncan - List replies preferred. No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master." Richard Stallman--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html