[PATCH] debugfs: increase inode reference count after link

[PATCH] debugfs: increase inode reference count after link

[Artem Blagodarenko]

Inode reference count is not increased after debugfs link command.
This leads to inconsistent file system state. fsck can find such
inodes and can fix. There is right code in add_link() that:
1) create link
2) expand directory if needed
3) increase inode referance
This patch uses add_link() function for debug link command.
Automatic directory expanding is useful when many files
are created in the same directory.

Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
---
 debugfs/debugfs.c     |    7 +------
 misc/create_inode.c   |    2 +-
 misc/create_inode.h   |    2 ++
 tests/f_dup4/expect.1 |    6 ------
 4 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 165f924..b40d9e2 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -1440,7 +1440,6 @@ void do_print_working_directory(int argc, char *argv[])
 static void make_link(char *sourcename, char *destname)
 {
     ext2_ino_t    ino;
-    struct ext2_inode inode;
     int        retval;
     ext2_ino_t    dir;
     char        *dest, *cp, *base_name;
@@ -1480,11 +1479,7 @@ static void make_link(char *sourcename, char *destname)
         }
     }

-    if (debugfs_read_inode(ino, &inode, sourcename))
-        return;
-
-    retval = ext2fs_link(current_fs, dir, dest, ino,
-                 ext2_file_type(inode.i_mode));
+    retval = add_link(current_fs, dir, ino, dest);
     if (retval)
         com_err("make_link", retval, 0);
     return;
diff --git a/misc/create_inode.c b/misc/create_inode.c
index ae22ff6..e2d80b4 100644
--- a/misc/create_inode.c
+++ b/misc/create_inode.c
@@ -59,7 +59,7 @@ static int ext2_file_type(unsigned int mode)
 }

 /* Link an inode number to a directory */
-static errcode_t add_link(ext2_filsys fs, ext2_ino_t parent_ino,
+errcode_t add_link(ext2_filsys fs, ext2_ino_t parent_ino,
               ext2_ino_t ino, const char *name)
 {
     struct ext2_inode    inode;
diff --git a/misc/create_inode.h b/misc/create_inode.h
index cf49df2..9c787ab 100644
--- a/misc/create_inode.h
+++ b/misc/create_inode.h
@@ -37,5 +37,7 @@ extern errcode_t do_mkdir_internal(ext2_filsys fs,
ext2_ino_t cwd,
 extern errcode_t do_write_internal(ext2_filsys fs, ext2_ino_t cwd,
                    const char *src, const char *dest,
                    ext2_ino_t root);
+extern errcode_t add_link(ext2_filsys fs, ext2_ino_t parent_ino,
+              ext2_ino_t ino, const char *name);

 #endif /* _CREATE_INODE_H */
diff --git a/tests/f_dup4/expect.1 b/tests/f_dup4/expect.1
index 7d51bb1..9fec126 100644
--- a/tests/f_dup4/expect.1
+++ b/tests/f_dup4/expect.1
@@ -99,12 +99,6 @@ Inode 14 ref count is 4, should be 3.  Fix? yes

 Inode 15 ref count is 0, should be 3.  Fix? yes

-Inode 16 ref count is 1, should be 3.  Fix? yes
-
-Inode 17 ref count is 1, should be 2.  Fix? yes
-
-Inode 19 ref count is 1, should be 3.  Fix? yes

Re: [PATCH] debugfs: increase inode reference count after link

[Theodore Ts'o]

On Fri, Jan 20, 2017 at 07:53:42PM +0300, Artem Blagodarenko wrote:
> Inode reference count is not increased after debugfs link command.
> This leads to inconsistent file system state.

That was deliberate.  Originally the "link" and "unlink" commands were
designed as a low-level command that didn't change the link count.
There are *many* commands in debugfs that can lead to inconsistent
file system state.  In fact, the original raison d'être of debugfs was
to create corrupted file systems to test e2fsck.  :-)

The "rm" command is user-friendly command that I added later which
decrements the link count and deallocates the blocks if the link count
goes to zero.

I never got around to adding a user-friendly 'ln' command which
creates the directory entry and also increments the link count.
That's mainly because no one has really needed or wanted that
functionality.  Or at least, no one has requested it up until now.

If you really want it, probably the best thing to do would be unalias
the "link" and "ln" commands, and make the "ln" command the
user-friendly command that works like the /bin/ln command.

Cheers,

						- Ted

Re: [PATCH] debugfs: increase inode reference count after link

[Artem Blagodarenko]

Hello Theodore,

Thank you for explanation and sorry I misunderstand "link" command. I
don't want to add "link" version that increase reference count. I
added some stings to "expect" file and expect fsck will warn about
this reference count. This is fine for debug purpose.

Best regares,
Artem Blagodarenko.


On Tue, Jan 31, 2017 at 6:48 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Fri, Jan 20, 2017 at 07:53:42PM +0300, Artem Blagodarenko wrote:
>> Inode reference count is not increased after debugfs link command.
>> This leads to inconsistent file system state.
>
> That was deliberate.  Originally the "link" and "unlink" commands were
> designed as a low-level command that didn't change the link count.
> There are *many* commands in debugfs that can lead to inconsistent
> file system state.  In fact, the original raison d'être of debugfs was
> to create corrupted file systems to test e2fsck.  :-)
>
> The "rm" command is user-friendly command that I added later which
> decrements the link count and deallocates the blocks if the link count
> goes to zero.
>
> I never got around to adding a user-friendly 'ln' command which
> creates the directory entry and also increments the link count.
> That's mainly because no one has really needed or wanted that
> functionality.  Or at least, no one has requested it up until now.
>
> If you really want it, probably the best thing to do would be unalias
> the "link" and "ln" commands, and make the "ln" command the
> user-friendly command that works like the /bin/ln command.
>
> Cheers,
>
>                                                 - Ted



-- 
Artem Blagodarenko Ph.D.· SW Developer on my.seagate.com
Seagate Technology, LLC
www.seagate.com