| Message ID | 1388585425-7051-1-git-send-email-vitoux.pascal@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 01, 2014 at 03:10:25PM +0100, Pascal VITOUX wrote: > --- > cmds-filesystem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cmds-filesystem.c b/cmds-filesystem.c > index 1c1926b..979dbd9 100644 > --- a/cmds-filesystem.c > +++ b/cmds-filesystem.c > @@ -646,7 +646,7 @@ static int defrag_callback(const char *fpath, const struct stat *sb, > int e = 0; > int fd = 0; > > - if (typeflag == FTW_F) { > + if ((typeflag == FTW_F) && S_ISREG(sb->st_mode)) { The ftw(3) documentation states that FTW_F gets the regular files, do you have a reproducer whre this does not work as expected? I think the check could be placed into cmd_defrag, there's a st_mode check for directories already. Defragmenting makes most sense for regular files (though there's the option to defrag portions of the metadata b-trees), so it's fine to handle just directories and regular files in userspace. If you're going to resend the patch, please add 'btrfs-progs: ' string to the subject, write a short changlog and add your Signed-off-by line. thanks, david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 6, 2014 at 4:36 PM, David Sterba <dsterba@suse.cz> wrote: > > On Wed, Jan 01, 2014 at 03:10:25PM +0100, Pascal VITOUX wrote: > > --- > > cmds-filesystem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/cmds-filesystem.c b/cmds-filesystem.c > > index 1c1926b..979dbd9 100644 > > --- a/cmds-filesystem.c > > +++ b/cmds-filesystem.c > > @@ -646,7 +646,7 @@ static int defrag_callback(const char *fpath, const struct stat *sb, > > int e = 0; > > int fd = 0; > > > > - if (typeflag == FTW_F) { > > + if ((typeflag == FTW_F) && S_ISREG(sb->st_mode)) { > > The ftw(3) documentation states that FTW_F gets the regular files, do you have a > reproducer whre this does not work as expected? Yes, this can be tested with the example code provided in ftw(3) and compiled with glibc 2.18. In fact, the ftw(3) page also states in the NOTES part that, under Linux, FTW_F is used for all objects that are not a directory. Some libc versions are specified ( libc4, libc5 and glic 2.0.6 ) but it's still true in glibc 2.18 The ftw's man page is less precise than the libc manual, as you can see here : http://www.gnu.org/software/libc/manual/html_node/Working-with-Directory-Trees.html > I think the check could be placed into cmd_defrag, there's a st_mode > check for directories already. Defragmenting makes most sense for > regular files (though there's the option to defrag portions of the > metadata b-trees), so it's fine to handle just directories and regular > files in userspace. I think so, too. And for a single file defrag the check should return to the user an explicit error message instead of the 'defrag range ioctl not supported' error. > If you're going to resend the patch, please add 'btrfs-progs: ' string > to the subject, write a short changlog and add your Signed-off-by line. > > thanks, > david Ok, thanks for these instructions. I will resend it soon. Pascal. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-filesystem.c b/cmds-filesystem.c index 1c1926b..979dbd9 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -646,7 +646,7 @@ static int defrag_callback(const char *fpath, const struct stat *sb, int e = 0; int fd = 0; - if (typeflag == FTW_F) { + if ((typeflag == FTW_F) && S_ISREG(sb->st_mode)) { if (defrag_global_verbose) printf("%s\n", fpath); fd = open(fpath, O_RDWR);