* [PATCH 2/3] debugfs: Add symlink command
2013-01-04 20:00 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
@ 2013-01-04 20:00 ` Darren Hart
2013-01-04 20:01 ` [PATCH 3/3] libext2fs: Remove obsolete redefinition of EXT2_FT_DIR Darren Hart
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Darren Hart @ 2013-01-04 20:00 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, adilger, sgw, darrick.wong, Darren Hart
Add support for symbolic links using a new symlink command.
Modeled after the do_mkdir() command.
Testing demonstrates both fastlinks and slowlinks work correctly. Very
long target paths fail as the command parsing appears to truncate the
input to somewhere around 256 bytes.
Signed-off-by: Darren Hart <dvhart@infradead.org>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: "Andreas Dilger" <adilger@dilger.ca>
---
debugfs/debug_cmds.ct | 3 +++
debugfs/debugfs.8.in | 3 +++
debugfs/debugfs.c | 43 +++++++++++++++++++++++++++++++++++++++++++
debugfs/debugfs.h | 1 +
debugfs/dump.c | 2 +-
5 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/debugfs/debug_cmds.ct b/debugfs/debug_cmds.ct
index 520933a..95d32df 100644
--- a/debugfs/debug_cmds.ct
+++ b/debugfs/debug_cmds.ct
@@ -160,6 +160,9 @@ request do_bmap, "Calculate the logical->physical block mapping for an inode",
request do_punch, "Punch (or truncate) blocks from an inode by deallocating them",
punch, truncate;
+request do_symlink, "Create a symbolic link",
+ symlink;
+
request do_imap, "Calculate the location of an inode",
imap;
diff --git a/debugfs/debugfs.8.in b/debugfs/debugfs.8.in
index bfcb1a8..c3b49f9 100644
--- a/debugfs/debugfs.8.in
+++ b/debugfs/debugfs.8.in
@@ -469,6 +469,9 @@ is, all of the blocks starting at
.I start_blk
through to the end of the file will be deallocated.
.TP
+.I symlink filespec target
+Make a symbolic link.
+.TP
.I pwd
Print the current working directory.
.TP
diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 341edfa..aa9a5f0 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -2202,6 +2202,49 @@ void do_punch(int argc, char *argv[])
}
#endif /* READ_ONLY */
+void do_symlink(int argc, char *argv[])
+{
+ char *cp;
+ ext2_ino_t parent;
+ char *name, *target;
+ errcode_t retval;
+
+ if (common_args_process(argc, argv, 3, 3, "symlink",
+ "<filename> <target>", CHECK_FS_RW))
+ return;
+
+ cp = strrchr(argv[1], '/');
+ if (cp) {
+ *cp = 0;
+ parent = string_to_inode(argv[1]);
+ if (!parent) {
+ com_err(argv[1], ENOENT, 0);
+ return;
+ }
+ name = cp+1;
+ } else {
+ parent = cwd;
+ name = argv[1];
+ }
+ target = argv[2];
+
+try_again:
+ retval = ext2fs_symlink(current_fs, parent, 0, name, target);
+ if (retval == EXT2_ET_DIR_NO_SPACE) {
+ retval = ext2fs_expand_dir(current_fs, parent);
+ if (retval) {
+ com_err(argv[0], retval, "while expanding directory");
+ return;
+ }
+ goto try_again;
+ }
+ if (retval) {
+ com_err("ext2fs_symlink", retval, 0);
+ return;
+ }
+
+}
+
void do_dump_mmp(int argc EXT2FS_ATTR((unused)), char *argv[])
{
#if CONFIG_MMP
diff --git a/debugfs/debugfs.h b/debugfs/debugfs.h
index efd8d12..bbefdf3 100644
--- a/debugfs/debugfs.h
+++ b/debugfs/debugfs.h
@@ -136,6 +136,7 @@ extern void do_imap(int argc, char **argv);
extern void do_set_current_time(int argc, char **argv);
extern void do_supported_features(int argc, char **argv);
extern void do_punch(int argc, char **argv);
+extern void do_symlink(int argc, char **argv);
extern void do_dump_mmp(int argc, char **argv);
extern void do_set_mmp_value(int argc, char **argv);
diff --git a/debugfs/dump.c b/debugfs/dump.c
index a15a0b7..2d830c9 100644
--- a/debugfs/dump.c
+++ b/debugfs/dump.c
@@ -285,7 +285,7 @@ static void rdump_inode(ext2_ino_t ino, struct ext2_inode *inode,
fix_perms("rdump", inode, -1, fullname);
}
- /* else do nothing (don't dump device files, sockets, fifos, etc.) */
+ /* else do nothing (don't dump device files, sockets, fifos, symlinks, etc.) */
errout:
free(fullname);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/3] libext2fs: Remove obsolete redefinition of EXT2_FT_DIR
2013-01-04 20:00 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
2013-01-04 20:00 ` [PATCH 2/3] debugfs: Add symlink command Darren Hart
@ 2013-01-04 20:01 ` Darren Hart
2013-01-05 20:07 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darrick J. Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Darren Hart @ 2013-01-04 20:01 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, adilger, sgw, darrick.wong, Darren Hart
This was needed a decade or more ago, back when we included
ext2_fs.h from the kernel headers in /usr/include/linux.
Signed-off-by: Darren Hart <dvhart@infradead.org>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: "Andreas Dilger" <adilger@dilger.ca>
---
lib/ext2fs/mkdir.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/lib/ext2fs/mkdir.c b/lib/ext2fs/mkdir.c
index 4a85439..9d69f70 100644
--- a/lib/ext2fs/mkdir.c
+++ b/lib/ext2fs/mkdir.c
@@ -27,10 +27,6 @@
#include "ext2_fs.h"
#include "ext2fs.h"
-#ifndef EXT2_FT_DIR
-#define EXT2_FT_DIR 2
-#endif
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink
2013-01-04 20:00 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
2013-01-04 20:00 ` [PATCH 2/3] debugfs: Add symlink command Darren Hart
2013-01-04 20:01 ` [PATCH 3/3] libext2fs: Remove obsolete redefinition of EXT2_FT_DIR Darren Hart
@ 2013-01-05 20:07 ` Darrick J. Wong
2013-01-05 22:40 ` Theodore Ts'o
2013-01-15 19:17 ` Theodore Ts'o
2013-01-15 19:43 ` Theodore Ts'o
4 siblings, 1 reply; 13+ messages in thread
From: Darrick J. Wong @ 2013-01-05 20:07 UTC (permalink / raw)
To: Darren Hart; +Cc: linux-ext4, tytso, adilger, sgw
On Fri, Jan 04, 2013 at 12:00:58PM -0800, Darren Hart wrote:
> Creating symlinks is a complex affair when accounting for slowlinks.
>
> Create a new function, ext2fs_symlink(), modeled after ext2fs_mkdir().
> Like ext2fs_mkdir(), ext2fs_symlink() takes on the task of allocating a
> new inode and block (for slowlinks), setting up sane default values in
> the inode, copying the target path to either the inode (for fastlinks)
> or to the first block (for slowlinks), and accounting for the inode and
> block stats. Disallow link targets longer than blocksize as the Linux
> kernel prevents this.
>
> It does not attempt to expand the parent directory, instead returning
> EXT2_ET_DIR_NO_SPACE and leaving it to the caller to expand just as
> ext2fs_mkdir() does. Ideally, I think both of these functions should
> make a single attempt to expand the directory.
>
> Note the one remaining FIXME. Do I need to use ext2fs_bmap2() for the
> slowlink without extents? If not, how is the 0 block associated with the
> inode?
>
> libext2fs.a impact (stripped):
> Orig | Patched | Delta
> 274752 | 276580 | 1828
>
> V2 incorporates feedback from Ted and Darrick:
> o Add error codes to the end of the table
> o Remove redundant EXT2_FT_SYMLINK definition
> o Time fields set by write_new_inode
> o Use size=target_len for slow links as well
> o Test for len < blocksize
> o Replace file ops with manual block handling
> o Keep line width under 80 characters
>
> Signed-off-by: Darren Hart <dvhart@infradead.org>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Cc: "Andreas Dilger" <adilger@dilger.ca>
> ---
> lib/ext2fs/Makefile.in | 8 +++
> lib/ext2fs/ext2_err.et.in | 3 +
> lib/ext2fs/symlink.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 172 insertions(+)
> create mode 100644 lib/ext2fs/symlink.c
>
> diff --git a/lib/ext2fs/Makefile.in b/lib/ext2fs/Makefile.in
> index 1a51c51..dc7b0d1 100644
> --- a/lib/ext2fs/Makefile.in
> +++ b/lib/ext2fs/Makefile.in
> @@ -80,6 +80,7 @@ OBJS= $(DEBUGFS_LIB_OBJS) $(RESIZE_LIB_OBJS) $(E2IMAGE_LIB_OBJS) \
> res_gdt.o \
> rw_bitmaps.o \
> swapfs.o \
> + symlink.o \
> tdb.o \
> undo_io.o \
> unix_io.o \
> @@ -155,6 +156,7 @@ SRCS= ext2_err.c \
> $(srcdir)/res_gdt.c \
> $(srcdir)/rw_bitmaps.c \
> $(srcdir)/swapfs.c \
> + $(srcdir)/symlink.c \
> $(srcdir)/tdb.c \
> $(srcdir)/test_io.c \
> $(srcdir)/tst_badblocks.c \
> @@ -893,6 +895,12 @@ swapfs.o: $(srcdir)/swapfs.c $(top_builddir)/lib/config.h \
> $(srcdir)/ext2_fs.h $(srcdir)/ext3_extents.h $(top_srcdir)/lib/et/com_err.h \
> $(srcdir)/ext2_io.h $(top_builddir)/lib/ext2fs/ext2_err.h \
> $(srcdir)/ext2_ext_attr.h $(srcdir)/bitops.h
> +symlink.o: $(srcdir)/symlink.c $(top_builddir)/lib/config.h \
> + $(top_builddir)/lib/dirpaths.h $(srcdir)/ext2_fs.h \
> + $(top_builddir)/lib/ext2fs/ext2_types.h $(srcdir)/ext2fs.h \
> + $(srcdir)/ext2_fs.h $(srcdir)/ext3_extents.h $(top_srcdir)/lib/et/com_err.h \
> + $(srcdir)/ext2_io.h $(top_builddir)/lib/ext2fs/ext2_err.h \
> + $(srcdir)/ext2_ext_attr.h $(srcdir)/bitops.h
> tdb.o: $(srcdir)/tdb.c $(top_builddir)/lib/config.h \
> $(top_builddir)/lib/dirpaths.h $(srcdir)/tdb.h
> test_io.o: $(srcdir)/test_io.c $(top_builddir)/lib/config.h \
> diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in
> index c99a167..d20c6b7 100644
> --- a/lib/ext2fs/ext2_err.et.in
> +++ b/lib/ext2fs/ext2_err.et.in
> @@ -473,4 +473,7 @@ ec EXT2_ET_UNKNOWN_CSUM,
> ec EXT2_ET_MMP_CSUM_INVALID,
> "MMP block checksum does not match MMP block"
>
> +ec EXT2_ET_FILE_EXISTS,
> + "Ext2 file already exists"
> +
> end
> diff --git a/lib/ext2fs/symlink.c b/lib/ext2fs/symlink.c
> new file mode 100644
> index 0000000..3e3a527
> --- /dev/null
> +++ b/lib/ext2fs/symlink.c
> @@ -0,0 +1,161 @@
> +/*
> + * symlink.c --- make a symlink in the filesystem, based on mkdir.c
> + *
> + * Copyright (c) 2012, Intel Corporation.
> + * All Rights Reserved.
> + *
> + * %Begin-Header%
> + * This file may be redistributed under the terms of the GNU Library
> + * General Public License, version 2.
> + * %End-Header%
> + */
> +
> +#include "config.h"
> +#include <stdio.h>
> +#include <string.h>
> +#if HAVE_UNISTD_H
> +#include <unistd.h>
> +#endif
> +#include <fcntl.h>
> +#include <time.h>
> +#if HAVE_SYS_STAT_H
> +#include <sys/stat.h>
> +#endif
> +#if HAVE_SYS_TYPES_H
> +#include <sys/types.h>
> +#endif
> +
> +#include "ext2_fs.h"
> +#include "ext2fs.h"
> +
> +errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
> + const char *name, char *target)
> +{
> + ext2_extent_handle_t handle;
> + errcode_t retval;
> + struct ext2_inode inode;
> + ext2_ino_t scratch_ino;
> + blk64_t blk;
> + int fastlink;
> + int target_len;
> + char *block_buf = 0;
> +
> + EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
> +
> + /* The Linux kernel doesn't allow for links longer than a block */
> + target_len = strlen(target);
> + if (target_len > fs->blocksize) {
> + retval = EXT2_ET_INVALID_ARGUMENT;
> + goto cleanup;
> + }
> +
> + /*
> + * Allocate a data block for slow links
> + */
> + fastlink = (target_len < sizeof(inode.i_block));
> + if (!fastlink) {
> + retval = ext2fs_new_block2(fs, 0, 0, &blk);
> + if (retval)
> + goto cleanup;
> + retval = ext2fs_get_mem(fs->blocksize, &block_buf);
> + if (retval)
> + goto cleanup;
> + }
> +
> + /*
> + * Allocate an inode, if necessary
> + */
> + if (!ino) {
> + retval = ext2fs_new_inode(fs, parent, LINUX_S_IFLNK | 0755,
> + 0, &ino);
> + if (retval)
> + goto cleanup;
> + }
> +
> + /*
> + * Create the inode structure....
> + */
> + memset(&inode, 0, sizeof(struct ext2_inode));
> + inode.i_mode = LINUX_S_IFLNK | 0777;
> + inode.i_uid = inode.i_gid = 0;
> + ext2fs_iblk_set(fs, &inode, 1);
> + inode.i_links_count = 1;
> + inode.i_size = target_len;
> + /* The time fields are set by ext2fs_write_new_inode() */
> +
> + if (fastlink) {
> + /* Fast symlinks, target stored in inode */
> + strcpy((char *)&inode.i_block, target);
> + } else {
> + /* Slow symlinks, target stored in the first block */
> + strcpy(block_buf, target);
> + if (fs->super->s_feature_incompat &
> + EXT3_FEATURE_INCOMPAT_EXTENTS) {
> + /*
> + * The extent bmap is setup after the inode and block
> + * have been written out below.
> + */
> + inode.i_flags |= EXT4_EXTENTS_FL;
Given that you can have at most one data block anyway, does it matter to set
EXTENTS_FL?
Actually, now that I think of it -- any reason not to use the ext2fs_file_*
functions? They'll take care of block allocation, updating the extent tree,
etc, so you don't have to remember how to do that here.
I think you have to ext2fs_write_new_inode() before you can ext2fs_file_open().
Also, what happens to the inode + data block if the ext2fs_link fails? I don't
see any code that explicitly rolls back those allocations, but maybe I missed
something?
--D
> + } else {
> + inode.i_block[0] = blk;
> + }
> + }
> +
> + /*
> + * Write out the inode and inode data block. The inode generation
> + * number is assigned by write_new_inode, which means that the
> + * operations using ino must come after it.
> + */
> + retval = ext2fs_write_new_inode(fs, ino, &inode);
> + if (retval)
> + goto cleanup;
> +
> + if (!fastlink) {
> + retval = io_channel_write_blk(fs->io, blk, 1, block_buf);
> + if (retval)
> + goto cleanup;
> +
> + /* FIXME/VERIFY: Do we need ext2fs_bmap2() here? file ops use
> + * it, mkdir did not */
> +
> + if (fs->super->s_feature_incompat &
> + EXT3_FEATURE_INCOMPAT_EXTENTS) {
> + retval = ext2fs_extent_open2(fs, ino, &inode, &handle);
> + if (retval)
> + goto cleanup;
> + retval = ext2fs_extent_set_bmap(handle, 0, blk, 0);
> + ext2fs_extent_free(handle);
> + if (retval)
> + goto cleanup;
> + }
> + }
> +
> + /*
> + * Link the symlink into the filesystem hierarchy
> + */
> + if (name) {
> + retval = ext2fs_lookup(fs, parent, name, strlen(name), 0,
> + &scratch_ino);
> + if (!retval) {
> + retval = EXT2_ET_FILE_EXISTS;
> + goto cleanup;
> + }
> + if (retval != EXT2_ET_FILE_NOT_FOUND)
> + goto cleanup;
> + retval = ext2fs_link(fs, parent, name, ino, EXT2_FT_SYMLINK);
> + if (retval)
> + goto cleanup;
> + }
> +
> + /*
> + * Update accounting....
> + */
> + if (!fastlink)
> + ext2fs_block_alloc_stats2(fs, blk, +1);
> + ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
> +
> +cleanup:
> + if (block_buf)
> + ext2fs_free_mem(&block_buf);
> + return retval;
> +}
> --
> 1.7.11.7
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink
2013-01-05 20:07 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darrick J. Wong
@ 2013-01-05 22:40 ` Theodore Ts'o
0 siblings, 0 replies; 13+ messages in thread
From: Theodore Ts'o @ 2013-01-05 22:40 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Darren Hart, linux-ext4, adilger, sgw
On Sat, Jan 05, 2013 at 12:07:16PM -0800, Darrick J. Wong wrote:
> Given that you can have at most one data block anyway, does it matter to set
> EXTENTS_FL?
It doesn't matter either way.
> Also, what happens to the inode + data block if the ext2fs_link fails? I don't
> see any code that explicitly rolls back those allocations, but maybe I missed
> something?
It's fine. We don't actually update the block bitmap, nor update the
block group statistics, until the very end, when the proposed code does this:
*/
if (!fastlink)
ext2fs_block_alloc_stats2(fs, blk, +1);
ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
This is one of the reasons why ext2fs_new_block and ext2fs_new_inode
don't actually mark the block and inode as in use. You could argue
they are misnamed; something like ext2fs_find_unused_{block,inode}()
would have been better names, but what I can say? I didn't think of
that back in 1996....
- Ted
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink
2013-01-04 20:00 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
` (2 preceding siblings ...)
2013-01-05 20:07 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darrick J. Wong
@ 2013-01-15 19:17 ` Theodore Ts'o
2013-01-15 19:43 ` Theodore Ts'o
4 siblings, 0 replies; 13+ messages in thread
From: Theodore Ts'o @ 2013-01-15 19:17 UTC (permalink / raw)
To: Darren Hart; +Cc: linux-ext4, adilger, sgw, darrick.wong
On Fri, Jan 04, 2013 at 12:00:58PM -0800, Darren Hart wrote:
Applied, thanks.
> Note the one remaining FIXME. Do I need to use ext2fs_bmap2() for the
> slowlink without extents? If not, how is the 0 block associated with the
> inode?
No, you don't need to use ext2fs_bmap2(). The 0 block was set here in
the else clause:
+ if (fs->super->s_feature_incompat &
+ EXT3_FEATURE_INCOMPAT_EXTENTS) {
+ /*
+ * The extent bmap is setup after the inode and block
+ * have been written out below.
+ */
+ inode.i_flags |= EXT4_EXTENTS_FL;
+ } else {
+ inode.i_block[0] = blk;
+ }
- Ted
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink
2013-01-04 20:00 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
` (3 preceding siblings ...)
2013-01-15 19:17 ` Theodore Ts'o
@ 2013-01-15 19:43 ` Theodore Ts'o
2013-01-15 19:53 ` [PATCH 1/2] debugfs: fix mknod command so that it updates the block group statistics Theodore Ts'o
2013-01-19 5:29 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
4 siblings, 2 replies; 13+ messages in thread
From: Theodore Ts'o @ 2013-01-15 19:43 UTC (permalink / raw)
To: Darren Hart; +Cc: linux-ext4, adilger, sgw, darrick.wong
I'll fix up the this patch before I commit it, but this is a perfect
exhibit about why I request that code submissions come with test
cases. It turns out that there were a couple of problems with
ext2fs_symlink(), that showed up very quickly as soon as I started
writing a test case (where it's important to run e2fsck on the
resulting file system after creating the symlinks --- e2fsck is a
wonderful rep invariant checkers for ext[234] file systems. :-)
*) i_blocks must be set to 0 for fast symlinks
*) The last argument of ext2fs_inode_alloc_stats() indicates whether
the new inode is a directory or not. So when you cut and pasted
the code from ext2fs_mkdir(), it needed to be changed.
*) Zeroing the entire block before setting the symlink in the case
where it needs to use an external data block makes it a lot
easier to write the regression test.
So here's the patch I needed to apply on top of your submission....
- Ted
diff --git a/lib/ext2fs/symlink.c b/lib/ext2fs/symlink.c
index fb8b91b..da6e3a8 100644
--- a/lib/ext2fs/symlink.c
+++ b/lib/ext2fs/symlink.c
@@ -78,7 +78,7 @@ errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
memset(&inode, 0, sizeof(struct ext2_inode));
inode.i_mode = LINUX_S_IFLNK | 0777;
inode.i_uid = inode.i_gid = 0;
- ext2fs_iblk_set(fs, &inode, 1);
+ ext2fs_iblk_set(fs, &inode, fastlink ? 0 : 1);
inode.i_links_count = 1;
inode.i_size = target_len;
/* The time fields are set by ext2fs_write_new_inode() */
@@ -88,6 +88,7 @@ errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
strcpy((char *)&inode.i_block, target);
} else {
/* Slow symlinks, target stored in the first block */
+ memset(block_buf, 0, fs->blocksize);
strcpy(block_buf, target);
if (fs->super->s_feature_incompat &
EXT3_FEATURE_INCOMPAT_EXTENTS) {
@@ -149,7 +150,7 @@ errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
*/
if (!fastlink)
ext2fs_block_alloc_stats2(fs, blk, +1);
- ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
+ ext2fs_inode_alloc_stats2(fs, ino, +1, 0);
cleanup:
if (block_buf)
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 1/2] debugfs: fix mknod command so that it updates the block group statistics
2013-01-15 19:43 ` Theodore Ts'o
@ 2013-01-15 19:53 ` Theodore Ts'o
2013-01-15 19:53 ` [PATCH 2/2] tests: create test for debugfs creating special files Theodore Ts'o
2013-01-19 5:29 ` [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink Darren Hart
1 sibling, 1 reply; 13+ messages in thread
From: Theodore Ts'o @ 2013-01-15 19:53 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: dvhart, Theodore Ts'o
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
debugfs/debugfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 59860e7..2a1525a 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -1747,8 +1747,7 @@ void do_mknod(int argc, char *argv[])
}
if (ext2fs_test_inode_bitmap2(current_fs->inode_map,newfile))
com_err(argv[0], 0, "Warning: inode already set");
- ext2fs_mark_inode_bitmap2(current_fs->inode_map, newfile);
- ext2fs_mark_ib_dirty(current_fs);
+ ext2fs_inode_alloc_stats2(current_fs, newfile, +1, 0);
memset(&inode, 0, sizeof(inode));
inode.i_mode = mode;
inode.i_atime = inode.i_ctime = inode.i_mtime =
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/2] tests: create test for debugfs creating special files
2013-01-15 19:53 ` [PATCH 1/2] debugfs: fix mknod command so that it updates the block group statistics Theodore Ts'o
@ 2013-01-15 19:53 ` Theodore Ts'o
0 siblings, 0 replies; 13+ messages in thread
From: Theodore Ts'o @ 2013-01-15 19:53 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: dvhart, Theodore Ts'o
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
tests/d_special_files/expect | 85 +++++++++++++++++++++++++++++++++++++++
tests/d_special_files/name | 1 +
tests/d_special_files/script | 94 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 180 insertions(+)
create mode 100644 tests/d_special_files/expect
create mode 100644 tests/d_special_files/name
create mode 100644 tests/d_special_files/script
diff --git a/tests/d_special_files/expect b/tests/d_special_files/expect
new file mode 100644
index 0000000..2b2dbfa
--- /dev/null
+++ b/tests/d_special_files/expect
@@ -0,0 +1,85 @@
+debugfs create special files
+mke2fs -Fq -b 1024 test.img 512
+Exit status is 0
+debugfs -R ''stat foo'' -w test.img
+Inode: 12 Type: symlink Mode: 0777 Flags: 0x0
+Generation: 0 Version: 0x00000000
+User: 0 Group: 0 Size: 3
+File ACL: 0 Directory ACL: 0
+Links: 1 Blockcount: 0
+Fragment: Address: 0 Number: 0 Size: 0
+ctime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+atime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+mtime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+Fast_link_dest: bar
+Exit status is 0
+debugfs -R ''stat foo2'' -w test.img
+Inode: 13 Type: symlink Mode: 0777 Flags: 0x0
+Generation: 0 Version: 0x00000000
+User: 0 Group: 0 Size: 80
+File ACL: 0 Directory ACL: 0
+Links: 1 Blockcount: 2
+Fragment: Address: 0 Number: 0 Size: 0
+ctime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+atime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+mtime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+BLOCKS:
+(0):28
+TOTAL: 1
+
+Exit status is 0
+debugfs -R ''block_dump 28'' -w test.img
+0000 3132 3334 3536 3738 3930 3132 3334 3536 1234567890123456
+0020 3738 3930 3132 3334 3536 3738 3930 3132 7890123456789012
+0040 3334 3536 3738 3930 3132 3334 3536 3738 3456789012345678
+0060 3930 3132 3334 3536 3738 3930 3132 3334 9012345678901234
+0100 3536 3738 3930 3132 3334 3536 3738 3930 5678901234567890
+0120 0000 0000 0000 0000 0000 0000 0000 0000 ................
+*
+
+Exit status is 0
+debugfs -R ''stat pipe'' -w test.img
+Inode: 14 Type: FIFO Mode: 0000 Flags: 0x0
+Generation: 0 Version: 0x00000000
+User: 0 Group: 0 Size: 0
+File ACL: 0 Directory ACL: 0
+Links: 1 Blockcount: 0
+Fragment: Address: 0 Number: 0 Size: 0
+ctime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+atime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+mtime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+BLOCKS:
+
+Exit status is 0
+debugfs -R ''stat sda'' -w test.img
+Inode: 15 Type: block special Mode: 0000 Flags: 0x0
+Generation: 0 Version: 0x00000000
+User: 0 Group: 0 Size: 0
+File ACL: 0 Directory ACL: 0
+Links: 1 Blockcount: 0
+Fragment: Address: 0 Number: 0 Size: 0
+ctime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+atime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+mtime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+Device major/minor number: 08:00 (hex 08:00)
+Exit status is 0
+debugfs -R ''stat null'' -w test.img
+Inode: 16 Type: character special Mode: 0000 Flags: 0x0
+Generation: 0 Version: 0x00000000
+User: 0 Group: 0 Size: 0
+File ACL: 0 Directory ACL: 0
+Links: 1 Blockcount: 0
+Fragment: Address: 0 Number: 0 Size: 0
+ctime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+atime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+mtime: 0x50f560e0 -- Tue Jan 15 14:00:00 2013
+Device major/minor number: 01:03 (hex 01:03)
+Exit status is 0
+e2fsck -yf -N test_filesys
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 16/64 files (0.0% non-contiguous), 29/512 blocks
+Exit status is 0
diff --git a/tests/d_special_files/name b/tests/d_special_files/name
new file mode 100644
index 0000000..98d10d2
--- /dev/null
+++ b/tests/d_special_files/name
@@ -0,0 +1 @@
+create special files in debugfs
diff --git a/tests/d_special_files/script b/tests/d_special_files/script
new file mode 100644
index 0000000..85cbb4d
--- /dev/null
+++ b/tests/d_special_files/script
@@ -0,0 +1,94 @@
+if test -x $DEBUGFS_EXE; then
+
+OUT=$test_name.log
+EXP=$test_dir/expect
+VERIFY_FSCK_OPT=-yf
+
+TEST_DATA=$test_name.tmp
+VERIFY_DATA=$test_name.ver.tmp
+
+echo "debugfs create special files" > $OUT
+
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+
+echo "mke2fs -Fq -b 1024 test.img 512" >> $OUT
+
+$MKE2FS -Fq $TMPFILE 512 > /dev/null 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+$DEBUGFS -w $TMPFILE << EOF > /dev/null 2>&1
+set_current_time 201301151400
+set_super_value lastcheck 0
+set_super_value hash_seed null
+set_super_value mkfs_time 0
+symlink foo bar
+symlink foo2 12345678901234567890123456789012345678901234567890123456789012345678901234567890
+mknod pipe p
+mknod sda b 8 0
+mknod null c 1 3
+EOF
+
+echo "debugfs -R ''stat foo'' -w test.img" > $OUT.new
+$DEBUGFS -R "stat foo" -w $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+echo "debugfs -R ''stat foo2'' -w test.img" > $OUT.new
+$DEBUGFS -R "stat foo2" -w $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+echo "debugfs -R ''block_dump 28'' -w test.img" > $OUT.new
+$DEBUGFS -R "block_dump 28" -w $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+echo "debugfs -R ''stat pipe'' -w test.img" > $OUT.new
+$DEBUGFS -R "stat pipe" -w $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+echo "debugfs -R ''stat sda'' -w test.img" > $OUT.new
+$DEBUGFS -R "stat sda" -w $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+echo "debugfs -R ''stat null'' -w test.img" > $OUT.new
+$DEBUGFS -R "stat null" -w $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+echo e2fsck $VERIFY_FSCK_OPT -N test_filesys > $OUT.new
+$FSCK $VERIFY_FSCK_OPT -N test_filesys $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -e '2d' $OUT.new >> $OUT
+
+#
+# Do the verification
+#
+
+rm -f $TMPFILE $OUT.new
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+ echo "$test_name: $test_description: ok"
+ touch $test_name.ok
+else
+ echo "$test_name: $test_description: failed"
+ diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+fi
+
+unset VERIFY_FSCK_OPT NATIVE_FSCK_OPT OUT EXP TEST_DATA VERIFY_DATA
+
+else #if test -x $DEBUGFS_EXE; then
+ echo "$test_name: $test_description: skipped"
+fi
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink
2013-01-15 19:43 ` Theodore Ts'o
2013-01-15 19:53 ` [PATCH 1/2] debugfs: fix mknod command so that it updates the block group statistics Theodore Ts'o
@ 2013-01-19 5:29 ` Darren Hart
1 sibling, 0 replies; 13+ messages in thread
From: Darren Hart @ 2013-01-19 5:29 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, adilger, sgw, darrick.wong
On 01/15/2013 11:43 AM, Theodore Ts'o wrote:
> I'll fix up the this patch before I commit it, but this is a perfect
> exhibit about why I request that code submissions come with test
> cases. It turns out that there were a couple of problems with
> ext2fs_symlink(), that showed up very quickly as soon as I started
> writing a test case (where it's important to run e2fsck on the
> resulting file system after creating the symlinks --- e2fsck is a
> wonderful rep invariant checkers for ext[234] file systems. :-)
>
> *) i_blocks must be set to 0 for fast symlinks
> *) The last argument of ext2fs_inode_alloc_stats() indicates whether
> the new inode is a directory or not. So when you cut and pasted
> the code from ext2fs_mkdir(), it needed to be changed.
> *) Zeroing the entire block before setting the symlink in the case
> where it needs to use an external data block makes it a lot
> easier to write the regression test.
>
> So here's the patch I needed to apply on top of your submission....
>
Ted,
I apologize for not getting to the test case, I meant to, and was still
meaning to. I was pulled away on a number of other tasks, took a family
vacation etc. I figured I was the only one that cared and it would be me
who had to pay the price if things changed while I delayed.
THANK YOU, THANK YOU! I really appreciate you writing the test cases and
fixing up the remaining issues with the patch.
This should now allow us to work with a debugfs script to validate this
tooling to populate filesystem images. Once we have done that, we'll
want to look at pushing some things (like file copy wrappers) from
debugfs into libext2fs and implement the -i (initial dir) option we
discused for mke2fs.
We'll do our validation and then resurface here.
Thanks again!
--
Darren
^ permalink raw reply [flat|nested] 13+ messages in thread