This patch adds new option, +e to chattr. The +e option
is used to convert the ext3 format (non extent) file
to ext4 (extent) format. This can be used to migrate
the ext3 file system to ext4 file system.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
misc/chattr.c | 58 +++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/misc/chattr.c b/misc/chattr.c
index 3d67519..7807b9e 100644
--- a/misc/chattr.c
+++ b/misc/chattr.c
@@ -82,7 +82,7 @@ static unsigned long sf;
static void usage(void)
{
fprintf(stderr,
- _("Usage: %s [-RVf] [-+=AacDdijsSu] [-v version] files...\n"),
+ _("Usage: %s [-RVf] [-+=AacDdijsSue] [-v version] files...\n"),
program_name);
exit(1);
}
@@ -105,6 +105,7 @@ static const struct flags_char flags_array[] = {
{ EXT2_UNRM_FL, 'u' },
{ EXT2_NOTAIL_FL, 't' },
{ EXT2_TOPDIR_FL, 'T' },
+ { EXT4_EXTENTS_FL, 'e'},
{ 0, 0 }
};
@@ -191,6 +192,7 @@ static int change_attributes(const char * name)
{
unsigned long flags;
STRUCT_STAT st;
+ int extent_file = 0;
if (LSTAT (name, &st) == -1) {
if (!silent)
@@ -199,7 +201,22 @@ static int change_attributes(const char * name)
return -1;
}
+ if (fgetflags (name, &flags) == -1) {
+ if (!silent)
+ com_err (program_name, errno,
+ _("while reading flags on %s"), name);
+ return -1;
+ }
+ if (flags & EXT4_EXTENTS_FL)
+ extent_file = 1;
if (set) {
+ if (extent_file && !(sf & EXT4_EXTENTS_FL)) {
+ if (!silent)
+ com_err(program_name, 0,
+ _("Clearing extent flag not supported on %s"),
+ name);
+ return -1;
+ }
if (verbose) {
printf (_("Flags of %s set as "), name);
print_flags (stdout, sf, 0);
@@ -208,30 +225,31 @@ static int change_attributes(const char * name)
if (fsetflags (name, sf) == -1)
perror (name);
} else {
- if (fgetflags (name, &flags) == -1) {
+ if (rem)
+ flags &= ~rf;
+ if (add)
+ flags |= af;
+ if (extent_file && !(flags & EXT4_EXTENTS_FL)) {
if (!silent)
- com_err (program_name, errno,
- _("while reading flags on %s"), name);
+ com_err(program_name, 0,
+ _("Clearing extent flag not supported on %s"),
+ name);
return -1;
- } else {
- if (rem)
- flags &= ~rf;
- if (add)
- flags |= af;
- if (verbose) {
- printf (_("Flags of %s set as "), name);
- print_flags (stdout, flags, 0);
- printf ("\n");
- }
- if (!S_ISDIR(st.st_mode))
- flags &= ~EXT2_DIRSYNC_FL;
- if (fsetflags (name, flags) == -1) {
- if (!silent)
- com_err(program_name, errno,
+ }
+ if (verbose) {
+ printf (_("Flags of %s set as "), name);
+ print_flags (stdout, flags, 0);
+ printf ("\n");
+ }
+ if (!S_ISDIR(st.st_mode))
+ flags &= ~EXT2_DIRSYNC_FL;
+ if (fsetflags (name, flags) == -1) {
+ if (!silent) {
+ com_err(program_name, errno,
_("while setting flags on %s"),
name);
- return -1;
}
+ return -1;
}
}
if (set_version) {
--
tg: (5bf3f80..) an/chattr3 (depends on: master)
On Sep 12, 2008 14:34 +0530, Aneesh Kumar wrote:
> - _("Usage: %s [-RVf] [-+=AacDdijsSu] [-v version] files...\n"),
> + _("Usage: %s [-RVf] [-+=AacDdijsSue] [-v version] files...\n"),
Please keep options in alphabetical order.
> @@ -105,6 +105,7 @@ static const struct flags_char flags_array[] = {
> { EXT2_UNRM_FL, 'u' },
> { EXT2_NOTAIL_FL, 't' },
> { EXT2_TOPDIR_FL, 'T' },
> + { EXT4_EXTENTS_FL, 'e'},
I'd also prefer to keep these in alphabetical order, which they almost are.
> @@ -199,7 +201,22 @@ static int change_attributes(const char * name)
> if (set) {
> + if (extent_file && !(sf & EXT4_EXTENTS_FL)) {
> + if (!silent)
> + com_err(program_name, 0,
> + _("Clearing extent flag not supported on %s"),
> + name);
> + return -1;
> + }
Why not just try to set this flag and let the kernel decide what is
possible?
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
On Fri, Sep 12, 2008 at 03:17:13AM -0600, Andreas Dilger wrote:
> On Sep 12, 2008 14:34 +0530, Aneesh Kumar wrote:
> > - _("Usage: %s [-RVf] [-+=AacDdijsSu] [-v version] files...\n"),
> > + _("Usage: %s [-RVf] [-+=AacDdijsSue] [-v version] files...\n"),
>
> Please keep options in alphabetical order.
>
> > @@ -105,6 +105,7 @@ static const struct flags_char flags_array[] = {
> > { EXT2_UNRM_FL, 'u' },
> > { EXT2_NOTAIL_FL, 't' },
> > { EXT2_TOPDIR_FL, 'T' },
> > + { EXT4_EXTENTS_FL, 'e'},
>
> I'd also prefer to keep these in alphabetical order, which they almost are.
>
> > @@ -199,7 +201,22 @@ static int change_attributes(const char * name)
> > if (set) {
> > + if (extent_file && !(sf & EXT4_EXTENTS_FL)) {
> > + if (!silent)
> > + com_err(program_name, 0,
> > + _("Clearing extent flag not supported on %s"),
> > + name);
> > + return -1;
> > + }
>
> Why not just try to set this flag and let the kernel decide what is
> possible?
>
The motivation is to give a clear message that we still don't support
clearing extent flags. If we pass it to the kernel we will get
operation not supported error which would confuse the user who does
chattr -de f1
-aneesh
Updated patch
From: Aneesh Kumar K.V <[email protected]>
Subject: [PATCH] Add extent conversion support to chattr
This patch adds new option, +e to chattr. The +e option
is used to convert the ext3 format (non extent) file
to ext4 (extent) format. This can be used to migrate
the ext3 file system to ext4 file system.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
misc/chattr.1.in | 8 ++----
misc/chattr.c | 58 +++++++++++++++++++++++++++++++++++------------------
2 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/misc/chattr.1.in b/misc/chattr.1.in
index 960f058..c49f876 100644
--- a/misc/chattr.1.in
+++ b/misc/chattr.1.in
@@ -19,7 +19,7 @@ chattr \- change file attributes on a Linux second extended file system
.B chattr
changes the file attributes on a Linux second extended file system.
.PP
-The format of a symbolic mode is +-=[ASacDdIijsTtu].
+The format of a symbolic mode is +-=[ASacDdeIijsTtu].
.PP
The operator `+' causes the selected attributes to be added to the
existing attributes of the files; `-' causes them to be removed; and
@@ -74,10 +74,8 @@ although it can be displayed by
.BR lsattr (1).
.PP
The 'e' attribute indicates that the file is using extents for mapping
-the blocks on disk. It may not be set or reset using
-.BR chattr (1),
-although it can be displayed by
-.BR lsattr (1).
+the blocks on disk. It may not be removed using
+.BR chattr (1).
.PP
The 'I' attribute is used by the htree code to indicate that a directory
is being indexed using hashed trees. It may not be set or reset using
diff --git a/misc/chattr.c b/misc/chattr.c
index 3d67519..067d0b6 100644
--- a/misc/chattr.c
+++ b/misc/chattr.c
@@ -82,7 +82,7 @@ static unsigned long sf;
static void usage(void)
{
fprintf(stderr,
- _("Usage: %s [-RVf] [-+=AacDdijsSu] [-v version] files...\n"),
+ _("Usage: %s [-RVf] [-+=AacDdeijsSu] [-v version] files...\n"),
program_name);
exit(1);
}
@@ -99,6 +99,7 @@ static const struct flags_char flags_array[] = {
{ EXT2_APPEND_FL, 'a' },
{ EXT2_COMPR_FL, 'c' },
{ EXT2_NODUMP_FL, 'd' },
+ { EXT4_EXTENTS_FL, 'e'},
{ EXT2_IMMUTABLE_FL, 'i' },
{ EXT3_JOURNAL_DATA_FL, 'j' },
{ EXT2_SECRM_FL, 's' },
@@ -191,6 +192,7 @@ static int change_attributes(const char * name)
{
unsigned long flags;
STRUCT_STAT st;
+ int extent_file = 0;
if (LSTAT (name, &st) == -1) {
if (!silent)
@@ -199,7 +201,22 @@ static int change_attributes(const char * name)
return -1;
}
+ if (fgetflags (name, &flags) == -1) {
+ if (!silent)
+ com_err (program_name, errno,
+ _("while reading flags on %s"), name);
+ return -1;
+ }
+ if (flags & EXT4_EXTENTS_FL)
+ extent_file = 1;
if (set) {
+ if (extent_file && !(sf & EXT4_EXTENTS_FL)) {
+ if (!silent)
+ com_err(program_name, 0,
+ _("Clearing extent flag not supported on %s"),
+ name);
+ return -1;
+ }
if (verbose) {
printf (_("Flags of %s set as "), name);
print_flags (stdout, sf, 0);
@@ -208,30 +225,31 @@ static int change_attributes(const char * name)
if (fsetflags (name, sf) == -1)
perror (name);
} else {
- if (fgetflags (name, &flags) == -1) {
+ if (rem)
+ flags &= ~rf;
+ if (add)
+ flags |= af;
+ if (extent_file && !(flags & EXT4_EXTENTS_FL)) {
if (!silent)
- com_err (program_name, errno,
- _("while reading flags on %s"), name);
+ com_err(program_name, 0,
+ _("Clearing extent flag not supported on %s"),
+ name);
return -1;
- } else {
- if (rem)
- flags &= ~rf;
- if (add)
- flags |= af;
- if (verbose) {
- printf (_("Flags of %s set as "), name);
- print_flags (stdout, flags, 0);
- printf ("\n");
- }
- if (!S_ISDIR(st.st_mode))
- flags &= ~EXT2_DIRSYNC_FL;
- if (fsetflags (name, flags) == -1) {
- if (!silent)
- com_err(program_name, errno,
+ }
+ if (verbose) {
+ printf (_("Flags of %s set as "), name);
+ print_flags (stdout, flags, 0);
+ printf ("\n");
+ }
+ if (!S_ISDIR(st.st_mode))
+ flags &= ~EXT2_DIRSYNC_FL;
+ if (fsetflags (name, flags) == -1) {
+ if (!silent) {
+ com_err(program_name, errno,
_("while setting flags on %s"),
name);
- return -1;
}
+ return -1;
}
}
if (set_version) {
--
tg: (5bf3f80..) an/chattr3 (depends on: master)
On Sep 12, 2008 14:57 +0530, Aneesh Kumar wrote:
> On Fri, Sep 12, 2008 at 03:17:13AM -0600, Andreas Dilger wrote:
> > Why not just try to set this flag and let the kernel decide what is
> > possible?
>
> The motivation is to give a clear message that we still don't support
> clearing extent flags. If we pass it to the kernel we will get
> operation not supported error which would confuse the user who does
> chattr -de f1
The problem is that if the kernel ever does allow converting extents->
block-mapped (or some other mapping that isn't extents), the chattr
tool will cause this to fail even when it shouldn't. Better to leave
the check in a single place (the kernel, which knows best).
I don't think the user would be too confused. You could still check
after the call to the kernel to see if the "e" option is being removed
and print an error if the call failed.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.