Re: [RFC][PATCH 1/1] ext4: Undelete Feature for Ext4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 19 Mar 2014, Lubos Uhliarik wrote:

> Date: Wed, 19 Mar 2014 14:59:04 +0100
> From: Lubos Uhliarik <uhliarik@xxxxxxxxx>
> To: Andreas Dilger <adilger@xxxxxxxxx>
> Cc: linux-ext4@xxxxxxxxxxxxxxx, vojnar@xxxxxxxxxxxx, lczerner@xxxxxxxxxx
> Subject: Re: [RFC][PATCH 1/1] ext4: Undelete Feature for Ext4
> 
> Hi Andreas,
> 
> maybe you didn't notice, but I sent comments in email with subject
> "[RFC][PATCH 0/1] ext4: Undelete Feature for Ext4". Maybe, I should
> mention this in [PATCH 1/1]. In this mail is also link to git repository
> with undelete app (uses ext2fslibs), which demonstrates how to undelete
> file with changes in kernel.

Hi Lubos,

you should definitely put some description into PATCH 1/1 as well -
that's what will eventually end up in the git repository and it
should contain description so that people know what it is all about.

Having e2fsprogs debugfs undelete command is actually a very good
idea. But I think this should be very easy to do with your current
program, or in addition to what you currently have since you're
already using ext2fs library.

Thanks!
-Lukas

> 
> I hope, it will help you.
> 
> Regards,
> 
> Lubos
> 
> Andreas Dilger píše v Út 18. 03. 2014 v 12:23 -0600:
> > On Mar 18, 2014, at 9:09 AM, Lubos Uhliarik <uhliarik@xxxxxxxxx> wrote:
> > > This patch should make undelete process for deleted files under ext4 fs easier.
> > 
> > Hi Lubos,
> > thanks for the interesting patch.  This does seem like an improvement.
> > It would be good to add more description to your commit comment, since
> > it should explain what is actually being done in your patch (e.g. store
> > the extent count and depth in the ext4_extent_header eh_generation field,
> > ...).
> > 
> > It would also be good to include a matching patch for e2fsprogs debugfs
> > in the "undelete" command.  I think one obstacle is the fact that inodes
> > have their blocks zeroed out by the kernel during truncation.  It would
> > be necessary to change the kernel slightly to avoid writing out i_blocks
> > with zero if the inode is being deleted.  Otherwise, this change is not
> > as useful as it could be.
> > 
> > Cheers, Andreas
> > 
> > > Signed-off-by: Lubos Uhliarik <uhliarik@xxxxxxxxx>
> > > ---
> > > fs/ext4/ext4_extents.h | 14 +++++++++++
> > > fs/ext4/extents.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++-----
> > > 2 files changed, 75 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> > > index 5074fe2..22cf2cd 100644
> > > --- a/fs/ext4/ext4_extents.h
> > > +++ b/fs/ext4/ext4_extents.h
> > > @@ -251,6 +251,20 @@ static inline void ext4_ext_store_pblock(struct ext4_extent *ex,
> > > 				      0xffff);
> > > }
> > > 
> > > +static inline void ext4_ext_store_entries(struct ext4_extent_header *eh,
> > > +					  __u16 eh_entries){
> > > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0x0000ffff);
> > > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_entries) << 16)
> > > +				& 0xffff0000);
> > > +}
> > > +
> > > +static inline void ext4_ext_store_depth(struct ext4_extent_header *eh,
> > > +					 __u16 eh_depth){
> > > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0xffff0000);
> > > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_depth))
> > > +				& 0x0000ffff);
> > > +}
> > > +
> > > /*
> > >  * ext4_idx_store_pblock:
> > >  * stores a large physical block number into an index struct,
> > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > > index 74bc2d5..f0f3615 100644
> > > --- a/fs/ext4/extents.c
> > > +++ b/fs/ext4/extents.c
> > > @@ -2565,6 +2565,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 	ext4_lblk_t ex_ee_block;
> > > 	unsigned short ex_ee_len;
> > > 	unsigned uninitialized = 0;
> > > +	unsigned short ex_ee_entries;
> > > 	struct ext4_extent *ex;
> > > 	ext4_fsblk_t pblk;
> > > 
> > > @@ -2584,6 +2585,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 
> > > 	ex_ee_block = le32_to_cpu(ex->ee_block);
> > > 	ex_ee_len = ext4_ext_get_actual_len(ex);
> > > +	ex_ee_entries = le16_to_cpu(eh->eh_entries);
> > > 
> > > 	trace_ext4_ext_rm_leaf(inode, start, ex, *partial_cluster);
> > > 
> > > @@ -2662,11 +2664,9 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 		if (err)
> > > 			goto out;
> > > 
> > > -		if (num == 0)
> > > -			/* this extent is removed; mark slot entirely unused */
> > > -			ext4_ext_store_pblock(ex, 0);
> > > +		if (num != 0)
> > > +			ex->ee_len = cpu_to_le16(num);
> > > 
> > > -		ex->ee_len = cpu_to_le16(num);
> > > 		/*
> > > 		 * Do not mark uninitialized if all the blocks in the
> > > 		 * extent have been removed.
> > > @@ -2726,8 +2726,19 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 
> > > 	/* if this leaf is free, then we should
> > > 	 * remove it from index block above */
> > > -	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
> > > +	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
> > > +		err = ext4_ext_get_access(handle, inode, path + depth);
> > > +		if (err)
> > > +			goto out;
> > > +
> > > +		ext4_ext_store_entries(path[depth].p_hdr, ex_ee_entries);
> > > +
> > > +		err = ext4_ext_dirty(handle, inode, path + depth);
> > > +		if (err)
> > > +			goto out;
> > > +
> > > 		err = ext4_ext_rm_idx(handle, inode, path, depth);
> > > +	}
> > > 
> > > out:
> > > 	return err;
> > > @@ -2760,6 +2771,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> > > 	struct super_block *sb = inode->i_sb;
> > > 	int depth = ext_depth(inode);
> > > 	struct ext4_ext_path *path = NULL;
> > > +	unsigned short *entries_path = NULL;
> > > 	long long partial_cluster = 0;
> > > 	handle_t *handle;
> > > 	int i = 0, err = 0;
> > > @@ -2864,6 +2876,18 @@ again:
> > > 	}
> > > 	err = 0;
> > > 
> > > +	if (!inode->i_nlink) {
> > > +		entries_path = kzalloc(sizeof(short) * (depth + 1),
> > > +				       GFP_NOFS);
> > > +		if (entries_path == NULL) {
> > > +			kfree(path);
> > > +			ext4_journal_stop(handle);
> > > +			return -ENOMEM;
> > > +		}
> > > +
> > > +		entries_path[0] = le16_to_cpu(ext_inode_hdr(inode)->eh_entries);
> > > +	}
> > > +
> > > 	while (i >= 0 && err == 0) {
> > > 		if (i == depth) {
> > > 			/* this is leaf block */
> > > @@ -2890,6 +2914,10 @@ again:
> > > 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
> > > 				  path[i].p_hdr,
> > > 				  le16_to_cpu(path[i].p_hdr->eh_entries));
> > > +			if (entries_path != NULL) {
> > > +				entries_path[i] = le16_to_cpu(
> > > +					path[i].p_hdr->eh_entries);
> > > +			}
> > > 		} else {
> > > 			/* we were already here, see at next index */
> > > 			path[i].p_idx--;
> > > @@ -2928,10 +2956,24 @@ again:
> > > 		} else {
> > > 			/* we finished processing this index, go up */
> > > 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
> > > +				if (entries_path != NULL) {
> > > +					err = ext4_ext_get_access(handle,
> > > +						inode, path);
> > > +					if (err == 0) {
> > > +						ext4_ext_store_entries(
> > > +							path[i].p_hdr,
> > > +							(entries_path[i]));
> > > +						err = ext4_ext_dirty(handle,
> > > +							inode, path);
> > > +					}
> > > +				}
> > > +
> > > 				/* index is empty, remove it;
> > > 				 * handle must be already prepared by the
> > > 				 * truncatei_leaf() */
> > > -				err = ext4_ext_rm_idx(handle, inode, path, i);
> > > +				if (err == 0)
> > > +					err = ext4_ext_rm_idx(handle, inode,
> > > +						path, i);
> > > 			}
> > > 			/* root level has p_bh == NULL, brelse() eats this */
> > > 			brelse(path[i].p_bh);
> > > @@ -2964,6 +3006,17 @@ again:
> > > 		 */
> > > 		err = ext4_ext_get_access(handle, inode, path);
> > > 		if (err == 0) {
> > > +			/* Store depth and entries to eh_generation to make
> > > +			 * recovering deleted files easier. */
> > > +			if (entries_path != NULL) {
> > > +				ext4_ext_store_entries(ext_inode_hdr(inode),
> > > +					(entries_path[0]));
> > > +
> > > +				ext4_ext_store_depth(ext_inode_hdr(inode),
> > > +					le16_to_cpu(
> > > +					ext_inode_hdr(inode)->eh_depth));
> > > +			}
> > > +
> > > 			ext_inode_hdr(inode)->eh_depth = 0;
> > > 			ext_inode_hdr(inode)->eh_max =
> > > 				cpu_to_le16(ext4_ext_space_root(inode, 0));
> > > @@ -2973,6 +3026,8 @@ again:
> > > out:
> > > 	ext4_ext_drop_refs(path);
> > > 	kfree(path);
> > > +	kfree(entries_path);
> > > +
> > > 	if (err == -EAGAIN) {
> > > 		path = NULL;
> > > 		goto again;
> > > -- 
> > > 1.8.3.2
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > Cheers, Andreas
> > 
> > 
> > 
> > 
> > 
> 
> 
> 

[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux