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]>
---
lib/e2p/Makefile.in | 6 +++-
lib/e2p/e2p.h | 1 +
lib/e2p/migrate.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
lib/ext2fs/ext2_fs.h | 1 +
misc/chattr.1.in | 5 +++-
misc/chattr.c | 20 +++++++++++++++++-
6 files changed, 80 insertions(+), 5 deletions(-)
diff --git a/lib/e2p/Makefile.in b/lib/e2p/Makefile.in
index c1a45f5..56b6a3a 100644
--- a/lib/e2p/Makefile.in
+++ b/lib/e2p/Makefile.in
@@ -19,7 +19,7 @@ all:: e2p.pc
OBJS= feature.o fgetflags.o fsetflags.o fgetversion.o fsetversion.o \
getflags.o getversion.o hashstr.o iod.o ls.o mntopts.o \
parse_num.o pe.o pf.o ps.o setflags.o setversion.o uuid.o \
- ostype.o percent.o
+ ostype.o percent.o migrate.o
SRCS= $(srcdir)/feature.c $(srcdir)/fgetflags.c \
$(srcdir)/fsetflags.c $(srcdir)/fgetversion.c \
@@ -28,7 +28,7 @@ SRCS= $(srcdir)/feature.c $(srcdir)/fgetflags.c \
$(srcdir)/ls.c $(srcdir)/mntopts.c $(srcdir)/parse_num.c \
$(srcdir)/pe.c $(srcdir)/pf.c $(srcdir)/ps.c \
$(srcdir)/setflags.c $(srcdir)/setversion.c $(srcdir)/uuid.c \
- $(srcdir)/ostype.c $(srcdir)/percent.c
+ $(srcdir)/ostype.c $(srcdir)/percent.c $(srcdir)/migrate.c
HFILES= e2p.h
LIBRARY= libe2p
@@ -162,3 +162,5 @@ ostype.o: $(srcdir)/ostype.c $(srcdir)/e2p.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h
percent.o: $(srcdir)/percent.c $(srcdir)/e2p.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h
+migrate.o: $(srcdir)/migrate.c $(srcdir)/e2p.h \
+ $(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h
diff --git a/lib/e2p/e2p.h b/lib/e2p/e2p.h
index 98c97db..8cbf360 100644
--- a/lib/e2p/e2p.h
+++ b/lib/e2p/e2p.h
@@ -60,3 +60,4 @@ char *e2p_os2string(int os_type);
int e2p_string2os(char *str);
unsigned int e2p_percent(int percent, unsigned int base);
+int ext4_migrate(const char * name);
diff --git a/lib/e2p/migrate.c b/lib/e2p/migrate.c
new file mode 100644
index 0000000..fa5b58c
--- /dev/null
+++ b/lib/e2p/migrate.c
@@ -0,0 +1,52 @@
+/*
+ * Convert a file to extent format
+ *
+ * Copyright IBM Corporation, 2008
+ * Author Aneesh Kumar K.V <[email protected]>
+ *
+ * This file can be redistributed under the terms of the GNU Library General
+ * Public License
+ */
+
+#define _LARGEFILE_SOURCE
+#define _LARGEFILE64_SOURCE
+
+#if HAVE_ERRNO_H
+#include <errno.h>
+#endif
+#if HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#include <fcntl.h>
+#if HAVE_SYS_IOCTL_H
+#include <sys/ioctl.h>
+#endif
+#include "e2p.h"
+
+#ifdef O_LARGEFILE
+#define OPEN_FLAGS (O_RDONLY|O_NONBLOCK|O_LARGEFILE)
+#else
+#define OPEN_FLAGS (O_RDONLY|O_NONBLOCK)
+#endif
+
+int ext4_migrate(const char * name)
+{
+#if HAVE_EXT2_IOCTLS
+ int fd, r, save_errno = 0;
+
+ fd = open (name, OPEN_FLAGS);
+ if (fd == -1)
+ return -1;
+ r = ioctl (fd, EXT4_IOC_MIGRATE, NULL);
+ if (r == -1)
+ save_errno = errno;
+ close (fd);
+ if (save_errno)
+ errno = save_errno;
+ return r;
+#else /* ! HAVE_EXT2_IOCTLS */
+ extern int errno;
+ errno = EOPNOTSUPP;
+ return -1;
+#endif /* ! HAVE_EXT2_IOCTLS */
+}
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index d7d7bdb..2fe6691 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -316,6 +316,7 @@ struct ext4_new_group_input {
#define EXT2_IOC_GROUP_EXTEND _IOW('f', 7, unsigned long)
#define EXT2_IOC_GROUP_ADD _IOW('f', 8,struct ext2_new_group_input)
#define EXT4_IOC_GROUP_ADD _IOW('f', 8,struct ext4_new_group_input)
+#define EXT4_IOC_MIGRATE _IO('f', 7)
/*
* Structure of an inode on the disk
diff --git a/misc/chattr.1.in b/misc/chattr.1.in
index 960f058..7e7f62a 100644
--- a/misc/chattr.1.in
+++ b/misc/chattr.1.in
@@ -5,7 +5,7 @@ chattr \- change file attributes on a Linux second extended file system
.SH SYNOPSIS
.B chattr
[
-.B \-RVf
+.B \-RVfE
]
[
.B \-v
@@ -41,6 +41,9 @@ Be verbose with chattr's output and print the program version.
.B \-f
Suppress most error messages.
.TP
+.B \-E
+Convert the file to extent format
+.TP
.BI \-v " version"
Set the file's version/generation number.
.SH ATTRIBUTES
diff --git a/misc/chattr.c b/misc/chattr.c
index 3d67519..95a3998 100644
--- a/misc/chattr.c
+++ b/misc/chattr.c
@@ -60,6 +60,7 @@ static int add;
static int rem;
static int set;
static int set_version;
+static int conv_to_extents;
static unsigned long version;
@@ -82,7 +83,7 @@ static unsigned long sf;
static void usage(void)
{
fprintf(stderr,
- _("Usage: %s [-RVf] [-+=AacDdijsSu] [-v version] files...\n"),
+ _("Usage: %s [-RVfE] [-+=AacDdijsSu] [-v version] files...\n"),
program_name);
exit(1);
}
@@ -156,6 +157,10 @@ static int decode_arg (int * i, int argc, char ** argv)
set_version = 1;
continue;
}
+ if (*p == 'E') {
+ conv_to_extents = 1;
+ continue;
+ }
if ((fl = get_flag(*p)) == 0)
usage();
rf |= fl;
@@ -245,6 +250,17 @@ static int change_attributes(const char * name)
return -1;
}
}
+ if (conv_to_extents) {
+ if (verbose)
+ printf (_("Converting %s to extent format\n"), name);
+ if (ext4_migrate(name) == -1) {
+ if (!silent)
+ com_err (program_name, errno,
+ _("while converting %s to extent format"),
+ name);
+ return -1;
+ }
+ }
if (S_ISDIR(st.st_mode) && recursive)
return iterate_on_dir (name, chattr_dir_proc, NULL);
return 0;
@@ -306,7 +322,7 @@ int main (int argc, char ** argv)
fputs("Can't both set and unset same flag.\n", stderr);
exit (1);
}
- if (!(add || rem || set || set_version)) {
+ if (!(add || rem || set || set_version || conv_to_extents)) {
fputs(_("Must use '-v', =, - or +\n"), stderr);
exit (1);
}
--
tg: (5bf3f80..) an/chattr (depends on: master)
On Tue, Sep 09, 2008 at 02:42:22PM +0530, Aneesh Kumar K.V wrote:
> 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.
I think this is an awkware interfac. Chattr is supposed to set simple
binary flags and not a front end to complicated filesystem conversions.
On Tue, Sep 09, 2008 at 07:46:47AM -0400, Christoph Hellwig wrote:
> On Tue, Sep 09, 2008 at 02:42:22PM +0530, Aneesh Kumar K.V wrote:
> > 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.
>
> I think this is an awkware interfac. Chattr is supposed to set simple
> binary flags and not a front end to complicated filesystem conversions.
>
Since chattr is used to set inode flags and since migrate can be looked
at as setting extent flags (with the side effect that we change the
inode format) I used chattr interface. Last patch i sent actually added a new
command e4migrate. Let me know if you have any suggestion related to
which command is best suited for setting extent flags.
-aneesh
On Tue, Sep 09, 2008 at 07:46:47AM -0400, Christoph Hellwig wrote:
> On Tue, Sep 09, 2008 at 02:42:22PM +0530, Aneesh Kumar K.V wrote:
> > 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.
>
> I think this is an awkware interfac. Chattr is supposed to set simple
> binary flags and not a front end to complicated filesystem conversions.
The alternate is to create an entire new program (e4migrate) just to
trigger a single ioctl. The reality is this is probably going to more
used by ext4 developers than anybody else, since it's rare that you
would want to convert a single file from using indrect blocks to using
extents. In general, most users/system administrators will want to
convert the entire filesystem; eventually this will probably be done
via some combination with the userspace program to trigger online
defrag, but this was just a stopgap measure to allow us to more easily
exercise the kernel code more than anything else.
So given that this is only to enable extents on a single file, "chattr
+e file" is very much in line with the rest of the chattr interface
for setting other flags.
One of the things which we may want to do to use statfs() and checking
f_type to make sure the file in question is located on an ext2/3/4
filesystem before trying the ioctl, since it is true that a number of
other filesystems have "borrowed" the chattr program and use it for
their own purposes. It's unlikely that the ext4 migration ioctl will
be wired to anything on other filesystems, but it would be a good
safety measure to add just in case.
Regards,
- Ted
On Tue, Sep 09, 2008 at 09:43:47AM -0400, Theodore Tso wrote:
> On Tue, Sep 09, 2008 at 07:46:47AM -0400, Christoph Hellwig wrote:
> > On Tue, Sep 09, 2008 at 02:42:22PM +0530, Aneesh Kumar K.V wrote:
> > > 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.
> >
> > I think this is an awkware interfac. Chattr is supposed to set simple
> > binary flags and not a front end to complicated filesystem conversions.
>
> The alternate is to create an entire new program (e4migrate) just to
> trigger a single ioctl. The reality is this is probably going to more
> used by ext4 developers than anybody else, since it's rare that you
> would want to convert a single file from using indrect blocks to using
> extents. In general, most users/system administrators will want to
> convert the entire filesystem; eventually this will probably be done
> via some combination with the userspace program to trigger online
> defrag, but this was just a stopgap measure to allow us to more easily
> exercise the kernel code more than anything else.
>
> So given that this is only to enable extents on a single file, "chattr
> +e file" is very much in line with the rest of the chattr interface
> for setting other flags.
>
> One of the things which we may want to do to use statfs() and checking
> f_type to make sure the file in question is located on an ext2/3/4
> filesystem before trying the ioctl, since it is true that a number of
> other filesystems have "borrowed" the chattr program and use it for
> their own purposes. It's unlikely that the ext4 migration ioctl will
> be wired to anything on other filesystems, but it would be a good
> safety measure to add just in case.
>
Shouldn't other file system give error when we call an ioctl with
EXT4_IOC_MIGRATE on the fd ?
On ext3 I get the below error
[an/chattr@e2fsprogs-upstream-build]$ ./misc/chattr -E ./misc/e2undo
./misc/chattr: Inappropriate ioctl for device while converting ./misc/e2undo to extent format
-aneesh
On Tue, Sep 09, 2008 at 07:39:29PM +0530, Aneesh Kumar K.V wrote:
>
> Shouldn't other file system give error when we call an ioctl with
> EXT4_IOC_MIGRATE on the fd ?
Only if by some incredible bad luck the ioctl number (which is after
all only at 16 bit number) doesn't happen to do something else random,
like security erase the entire filesystem. :-)
> On ext3 I get the below error
> [an/chattr@e2fsprogs-upstream-build]$ ./misc/chattr -E ./misc/e2undo
> ./misc/chattr: Inappropriate ioctl for device while converting ./misc/e2undo to extent format
>
#1, it really should be +e, since we are turning on the extent flag,
and #2, we should give a much more user-understandable error message
in that case.
- Ted
On Tue, Sep 09, 2008 at 10:39:15AM -0400, Theodore Tso wrote:
> On Tue, Sep 09, 2008 at 07:39:29PM +0530, Aneesh Kumar K.V wrote:
> >
> > Shouldn't other file system give error when we call an ioctl with
> > EXT4_IOC_MIGRATE on the fd ?
>
> Only if by some incredible bad luck the ioctl number (which is after
> all only at 16 bit number) doesn't happen to do something else random,
> like security erase the entire filesystem. :-)
>
> > On ext3 I get the below error
> > [an/chattr@e2fsprogs-upstream-build]$ ./misc/chattr -E ./misc/e2undo
> > ./misc/chattr: Inappropriate ioctl for device while converting ./misc/e2undo to extent format
> >
>
> #1, it really should be +e, since we are turning on the extent flag,
> and #2, we should give a much more user-understandable error message
> in that case.
something like
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]>
---
lib/e2p/e2p.h | 1 +
lib/e2p/fsetflags.c | 10 ++++++++++
lib/ext2fs/ext2_fs.h | 1 +
misc/chattr.1.in | 9 ++++-----
misc/chattr.c | 17 ++++++++++++++++-
5 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/lib/e2p/e2p.h b/lib/e2p/e2p.h
index 98c97db..8cbf360 100644
--- a/lib/e2p/e2p.h
+++ b/lib/e2p/e2p.h
@@ -60,3 +60,4 @@ char *e2p_os2string(int os_type);
int e2p_string2os(char *str);
unsigned int e2p_percent(int percent, unsigned int base);
+int ext4_migrate(const char * name);
diff --git a/lib/e2p/fsetflags.c b/lib/e2p/fsetflags.c
index 62189c9..d1bb9d9 100644
--- a/lib/e2p/fsetflags.c
+++ b/lib/e2p/fsetflags.c
@@ -80,9 +80,19 @@ int fsetflags (const char * name, unsigned long flags)
if (fd == -1)
return -1;
f = (int) flags;
+ if (f & EXT4_EXTENTS_FL) {
+ /* extent flags is set using migrate ioctl */
+ r = ioctl (fd, EXT4_IOC_MIGRATE, NULL);
+ if (r == -1) {
+ save_errno = errno;
+ goto err_out;
+ }
+ f = (int)flags & ~EXT4_EXTENTS_FL;
+ }
r = ioctl (fd, EXT2_IOC_SETFLAGS, &f);
if (r == -1)
save_errno = errno;
+err_out:
close (fd);
if (save_errno)
errno = save_errno;
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index d7d7bdb..2fe6691 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -316,6 +316,7 @@ struct ext4_new_group_input {
#define EXT2_IOC_GROUP_EXTEND _IOW('f', 7, unsigned long)
#define EXT2_IOC_GROUP_ADD _IOW('f', 8,struct ext2_new_group_input)
#define EXT4_IOC_GROUP_ADD _IOW('f', 8,struct ext4_new_group_input)
+#define EXT4_IOC_MIGRATE _IO('f', 7)
/*
* Structure of an inode on the disk
diff --git a/misc/chattr.1.in b/misc/chattr.1.in
index 960f058..1247090 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 +-=[ASacDdIijsTtue].
.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,9 @@ 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. Setting extent flag cause the file to be converted
+to extent format. It may not be unset 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..c9d6d1e 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] [-+=AacDdijsSu] [+e] [-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 }
};
@@ -199,6 +200,20 @@ static int change_attributes(const char * name)
return -1;
}
+ if (rf & EXT4_EXTENTS_FL) {
+ /* only allowed format is +e */
+ com_err (program_name, errno,
+ _("Cannot remove the extent flag on %s"), name);
+ return -1;
+ }
+
+ if (sf & EXT4_EXTENTS_FL) {
+ /* only allowed format is +e */
+ com_err (program_name, errno,
+ _("Extent flag cannot be the only attribute on %s"), name);
+ return -1;
+ }
+
if (set) {
if (verbose) {
printf (_("Flags of %s set as "), name);
--
tg: (5bf3f80..) an/chattr (depends on: master)
On Tue, Sep 09, 2008 at 09:58:14PM +0530, Aneesh Kumar K.V wrote:
> diff --git a/lib/e2p/fsetflags.c b/lib/e2p/fsetflags.c
> index 62189c9..d1bb9d9 100644
> --- a/lib/e2p/fsetflags.c
> +++ b/lib/e2p/fsetflags.c
> @@ -80,9 +80,19 @@ int fsetflags (const char * name, unsigned long flags)
> if (fd == -1)
> return -1;
> f = (int) flags;
> + if (f & EXT4_EXTENTS_FL) {
> + /* extent flags is set using migrate ioctl */
> + r = ioctl (fd, EXT4_IOC_MIGRATE, NULL);
> + if (r == -1) {
> + save_errno = errno;
> + goto err_out;
> + }
> + f = (int)flags & ~EXT4_EXTENTS_FL;
> + }
No, I wouldn't put this in libe2p; it probably should be in the chattr
program. If you do it in chattr, it's much easier to have an
user-friendly error message if the EXT4_IOC_MIGRATE ioctl fails.
Also, it should only call EXT4_IOC_MIGRATE if the file was previously
not supporting extents. Probably a bad idea to unconditionally call
EXT4_IOC_MIGRATE.
- Ted
On Tue, Sep 09, 2008 at 09:43:47AM -0400, Theodore Tso wrote:
> The alternate is to create an entire new program (e4migrate) just to
> trigger a single ioctl. The reality is this is probably going to more
> used by ext4 developers than anybody else, since it's rare that you
> would want to convert a single file from using indrect blocks to using
> extents. In general, most users/system administrators will want to
> convert the entire filesystem; eventually this will probably be done
> via some combination with the userspace program to trigger online
> defrag, but this was just a stopgap measure to allow us to more easily
> exercise the kernel code more than anything else.
>
> So given that this is only to enable extents on a single file, "chattr
> +e file" is very much in line with the rest of the chattr interface
> for setting other flags.
Well, if you think setting a flag is a good interface to convert to
extents, then do it all the way and allow setting the extent
flag in the kernel through FS_IOC_SETFLAGS instead of having an
interface schism.
On Tue, Sep 09, 2008 at 12:50:05PM -0400, Christoph Hellwig wrote:
> Well, if you think setting a flag is a good interface to convert to
> extents, then do it all the way and allow setting the extent
> flag in the kernel through FS_IOC_SETFLAGS instead of having an
> interface schism.
Thanks, that's a good idea. It won't help people on older kernels,
but it probably is the right answer.
Regards,
- Ted