headers
Andreas Dilger | 1 Feb 2007 07:06

Re: [patch 1/2] e2fsprogs: user selectable dup block handling in fsck

On Jan 31, 2007  08:22 -0800, Jim Garlick wrote:
> It also adds a check to make sure only one -E option is passed
> on the command line as -E option parsing is not cumulative.
>
>  <at>  <at>  -633,6 +639,8  <at>  <at>  static errcode_t PRS(int argc, char *arg
>  		case 'E':
> +			if (extended_opts)
> +				fatal_error(ctx, _("-E must only be 
> specified once"));
>  			extended_opts = optarg;

In such cases I've usually just changed the code to do the parsing
as the option is passed.  Otherwise, it isn't possible to "override"
previously-specified options. This sometimes is needed if you have an
alias or script that is passing a bunch of options, and in some rare
cases you don't want the default, e.g.

alias mye2fsck="e2fsck -f -p -E clone=dup"

# mye2fsck -y -E clone=zero /dev/really-broken

Ted, is there a reason that the call to parse_extended_opts() can't
just be moved in place of saving the options in extended_opts?  I
can't see anything in -E (yet) that depends on other options that
might not be set yet.

Also, it looks like that function leaks the duplicated string in "buf",
since that variable goes out of scope without freeing the allocation.

Cheers, Andreas
(Continue reading)

Permalink | Reply | 0 comments |
headers
Andreas Dilger | 1 Feb 2007 07:16

Re: [patch 2/2] e2fsprogs: user selectable dup block handling in fsck

On Jan 31, 2007  08:24 -0800, Jim Garlick wrote:
>  <at>  <at>  -67,7 +68,8  <at>  <at>  static const char *prompt[] = {
>  	N_("Unlink"),		/* 17 */
>  	N_("Clear HTree index"),/* 18 */
>  	N_("Recreate"),		/* 19 */
> -	"",			/* 20 */
> +	N_("Unlink file for lost+found reconnect in pass 3"), /* 20 */
>
> +	/* Unlink file for lost+found reconnect in pass 3? */
> +	{ PR_1D_DISCONNECT_QUESTION,
> +	  "", PROMPT_DISCONNECT, 0 },

Why not use PROMPT_CONNECT ("Connect to lost+found") and then
make the question "File with shared blocks found"?

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
Permalink | Reply | 0 comments |
headers
Andrew Morton | 1 Feb 2007 10:08

Fw: [BUG -mm] ext3_orphan_add() accessing corrupted list on a corrupted ext3fs


Begin forwarded message:

Date: Thu, 1 Feb 2007 16:44:39 +0800
From: Fengguang Wu <fengguang.wu <at> gmail.com>
To: LKML <linux-kernel <at> vger.kernel.org>
Subject: [BUG -mm] ext3_orphan_add() accessing corrupted list on a corrupted ext3fs

I accidentally ran two qemu instances on the same ext3 fs, after that bad
things happened. After exiting the two qemus and running a new one, I got the
following oops:

root ~# ll /etc/mtab
/bin/ls: /etc/mtab: Input/output error
root ~# rm /etc/mtab
[  147.213090] EXT3-fs warning (device hda): ext3_unlink: Deleting nonexistent file (1775838), 0
root ~# halt
[  152.651209] list_add corruption. next->prev should be prev (ffff810007be1a38), but was
ffff81000717e3d8. (next=ffff81000717e3d8).
[  152.652507] ------------[ cut here ]------------
[  152.652900] kernel BUG at lib/list_debug.c:27!
[  152.653283] invalid opcode: 0000 [1] SMP
[  152.653649] last sysfs file: /block/md2/uevent
[  152.654020] CPU 0
[  152.654228] Modules linked in:
[  152.654549] Pid: 1107, comm: zsh Not tainted 2.6.20-rc6-mm3 #1
[  152.655397] RIP: 0010:[<ffffffff8116f558>]  [<ffffffff8116f558>] __list_add+0x48/0xb0
[  152.656139] RSP: 0018:ffff8100062bdd78  EFLAGS: 00000296
[  152.656572] RAX: 0000000000000088 RBX: ffff81000717e3d8 RCX: 0000000000000000
[  152.657140] RDX: ffffffff8101a433 RSI: 0000000000000001 RDI: ffffffff8141fb40
(Continue reading)

Permalink | Reply | 6 comments |
headers
Andreas Dilger | 1 Feb 2007 11:25

Re: Fw: [BUG -mm] ext3_orphan_add() accessing corrupted list on a corrupted ext3fs

I don't have a comment on the actual bug here, but this is another case
where it would be nice to have multi-mount protection built into ext3...
When I last proposed this it was refused on the grounds that an external
HA manager should be doing this job but I don't think that is realistic.

Fengguang Wu <fengguang.wu <at> gmail.com> wrote:
> I accidentally ran two qemu instances on the same ext3 fs, after that bad
> things happened. After exiting the two qemus and running a new one, I got the
> following oops:
> 
> root ~# ll /etc/mtab
> /bin/ls: /etc/mtab: Input/output error
> root ~# rm /etc/mtab
> [  147.213090] EXT3-fs warning (device hda): ext3_unlink: Deleting nonexistent file (1775838), 0
> root ~# halt
> [  152.651209] list_add corruption. next->prev should be prev (ffff810007be1a38), but was
ffff81000717e3d8. (next=ffff81000717e3d8).
> [  152.652507] ------------[ cut here ]------------
> [  152.652900] kernel BUG at lib/list_debug.c:27!
> [  152.653283] invalid opcode: 0000 [1] SMP
> [  152.653649] last sysfs file: /block/md2/uevent
> [  152.654020] CPU 0
> [  152.654228] Modules linked in:
> [  152.654549] Pid: 1107, comm: zsh Not tainted 2.6.20-rc6-mm3 #1
> [  152.655397] RIP: 0010:[<ffffffff8116f558>]  [<ffffffff8116f558>] __list_add+0x48/0xb0
> [  152.656139] RSP: 0018:ffff8100062bdd78  EFLAGS: 00000296
> [  152.656572] RAX: 0000000000000088 RBX: ffff81000717e3d8 RCX: 0000000000000000
> [  152.657140] RDX: ffffffff8101a433 RSI: 0000000000000001 RDI: ffffffff8141fb40
> [  152.657708] RBP: ffff8100062bdd98 R08: 0000000000000002 R09: ffffffff8101a270
> [  152.658275] R10: ffff8100062bdb58 R11: 0000000000000006 R12: ffff810007be1a38
(Continue reading)

Permalink | Reply | 2 comments |
headers
Harry Papaxenopoulos | 1 Feb 2007 12:05
Picon
Favicon

Re: [Resubmit][PATCH 5/5] Secure Deletion and Trash-Bin Support for Ext4


On Wed, 31 Jan 2007, Dave Kleikamp wrote:

> Right now I've only really looked at the jfs patch, since that's what
> I'm the most familiar with.  I'll try to take a look at the rest of them
> later.
>
> I don't have a strong opinion for or against the function and your
> design.  The only potential problem I see in the approach is that
> the .trash directory may conflict with some other use of the same name.
> Since this is primarily vfs function, you'll probably get a wider
> audience on linux-fsdevel.
>
> Have you considered putting ALL of the function in the vfs layer?  It
> looks like this could be done without touching any code in the
> individual file systems.
>

Yes now that you mention it we probably could. The initial idea was to add
this functionality only for Ext4. Only after a suggestion did we move most
of the code to the VFS and have hooks to it through the underlying
file-systems. Nevertheless it probably could be completely moved to the VFS.

> On Wed, 2007-01-31 at 09:55 -0500, Harry Papaxenopoulos wrote:
> > Trash-Bin Functionality for the jfs filesystem:
> >
> > Signed-off-by: Harry Papaxenopoulos <harry <at> cs.sunysb.edu>
> > Signed-off-by: Nikolai Joukov <kolya <at> cs.sunysb.edu>
> > Signed-off-by: Erez Zadok <ezk <at> cs.sunysb.edu>
> >
(Continue reading)

Permalink | Reply | 1 comment |
headers
Dave Kleikamp | 1 Feb 2007 14:19
Picon

Re: [Resubmit][PATCH 5/5] Secure Deletion and Trash-Bin Support for Ext4

On Thu, 2007-02-01 at 06:05 -0500, Harry Papaxenopoulos wrote:
> 
> On Wed, 31 Jan 2007, Dave Kleikamp wrote:

> > why not unconditionally goto out here?  if vfs_trash_entry() was
> > successful, the file was successfully moved to the trashbin directory.
> > There is nothing left to be done, right?  Then there's no need for the
> > trashed flag.
> >
> > In fact, the ifdef'ed code should precede the call to get_UCSname(),
> > since we don't need to allocate dname if we move the file to the
> > trashbin, and we leak the allocation if we jump to out:.
> >
> 
> Well the main reason I don't jump to out is to update the inode's
> change time, otherwise I would have unconditionally jumped.

the rename already updates the change time.
> 
> You're right, I used the incorrect label to jump. Should have been "out1"
> instead of "out" so the allocation is freed. Sorry about that.

I'd rather do the allocation after the new code anyway.  It's not needed
at all if we're moving the file to the trashbin.

Thanks,
Shaggy
--

-- 
David Kleikamp
IBM Linux Technology Center
(Continue reading)

Permalink | Reply | 0 comments |
headers
Eric Sandeen | 1 Feb 2007 17:58
Picon
Favicon
Gravatar

Re: Fw: [BUG -mm] ext3_orphan_add() accessing corrupted list on a corrupted ext3fs

Andrew Morton wrote:
> 
> Begin forwarded message:
> 
> Date: Thu, 1 Feb 2007 16:44:39 +0800
> From: Fengguang Wu <fengguang.wu <at> gmail.com>
> To: LKML <linux-kernel <at> vger.kernel.org>
> Subject: [BUG -mm] ext3_orphan_add() accessing corrupted list on a corrupted ext3fs
> 
> 
> I accidentally ran two qemu instances on the same ext3 fs, after that bad
> things happened. After exiting the two qemus and running a new one, I got the
> following oops:

Is this equivalent to mounting the same SAN block device on 2 different
machines?  And if so how much can the filesystem really be expected to
cope with this?

(remembering to read the rest of his inbox...)

Andreas Dilger wrote:

> I don't have a comment on the actual bug here, but this is another case
> where it would be nice to have multi-mount protection built into ext3...
> When I last proposed this it was refused on the grounds that an external
> HA manager should be doing this job but I don't think that is realistic.

I'm with Andreas on this one, in the era of SANs, iscsi, virtual
machines, and suspended images, it would be nice to prevent multiple
mounts at the fs (or vfs?) level....
(Continue reading)

Permalink | Reply | 0 comments |
headers
Nikolai Joukov | 1 Feb 2007 18:17
Picon
Favicon

Re: [Resubmit][PATCH 5/5] Secure Deletion and Trash-Bin Support for Ext4

> Right now I've only really looked at the jfs patch, since that's what
> I'm the most familiar with.  I'll try to take a look at the rest of them
> later.

Thank you!

> I don't have a strong opinion for or against the function and your
> design.  The only potential problem I see in the approach is that
> the .trash directory may conflict with some other use of the same name.
> Since this is primarily vfs function, you'll probably get a wider
> audience on linux-fsdevel.

Well, I guess lost+found has the same problem but it is not a problem at
all to pick some other (longer) name.

> Have you considered putting ALL of the function in the vfs layer?  It
> looks like this could be done without touching any code in the
> individual file systems.

Unfortunately, we need some file system-specific code to access per-file
secure deletion and per-file trash bit attributes.  These attributes are
supported only by some file systems and in different ways.  We need no
file system-specific code to support trash-bin deletion for whole file
systems.

Nikolai.
Permalink | Reply | 5 comments |
headers
Alex Tomas | 1 Feb 2007 18:28

Re: Fw: [BUG -mm] ext3_orphan_add() accessing corrupted list on a corrupted ext3fs

>>>>> Andreas Dilger (AD) writes:

 AD> I don't have a comment on the actual bug here, but this is another case
 AD> where it would be nice to have multi-mount protection built into ext3...
 AD> When I last proposed this it was refused on the grounds that an external
 AD> HA manager should be doing this job but I don't think that is realistic.

can we use JBD-like approach? export some inode and MMP/whatever
would 'ping' 1st block of the inode.

thanks, Alex
Permalink | Reply | 1 comment |
headers
Dave Kleikamp | 1 Feb 2007 20:32
Picon

Re: [Resubmit][PATCH 5/5] Secure Deletion and Trash-Bin Support for Ext4

On Thu, 2007-02-01 at 12:17 -0500, Nikolai Joukov wrote:

> > I don't have a strong opinion for or against the function and your
> > design.  The only potential problem I see in the approach is that
> > the .trash directory may conflict with some other use of the same name.
> > Since this is primarily vfs function, you'll probably get a wider
> > audience on linux-fsdevel.
> 
> Well, I guess lost+found has the same problem but it is not a problem at
> all to pick some other (longer) name.

Right, I didn't see it as a show-stopper, just something to consider.
> 
> > Have you considered putting ALL of the function in the vfs layer?  It
> > looks like this could be done without touching any code in the
> > individual file systems.
> 
> Unfortunately, we need some file system-specific code to access per-file
> secure deletion and per-file trash bit attributes.  These attributes are
> supported only by some file systems and in different ways.  

Yeah, I did see that.  I wonder adding some inode or file operation just
to query the existence of those attributes (or something more generic)
would be too ugly.

> We need no
> file system-specific code to support trash-bin deletion for whole file
> systems.
> 
> Nikolai.
(Continue reading)

Permalink | Reply | 4 comments |

Gmane