* [PATCH] libext2fs: Make ext2fs_extent_set_bmap() more robust against ENOSPC
@ 2009-07-10 4:23 Theodore Ts'o
2009-07-10 4:45 ` Eric Sandeen
2009-07-10 16:56 ` Eric Sandeen
0 siblings, 2 replies; 3+ messages in thread
From: Theodore Ts'o @ 2009-07-10 4:23 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: Theodore Ts'o
In the case where we ext2fs_extent_set_bmap() is replacing the block
mapping at the beginning of an already-existing extent, insert a new
extent if necessary before shrinking an existing extent, to avoid data
loss if the disk is full.
This mostly addresses the problem described in Red Hat Bugzilla's
statistics are still wrong, but at least the files on the filesystem
are not corrupted. If there is a failure during the
inode_scan_and_fix pass, the simplest thing to do may be to tell the
user to run e2fsck -fy.
Addresses-Red-Hat-Bug: #510379
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
lib/ext2fs/extent.c | 44 ++++++++++++++++++++++++++++++++------------
1 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index 4a4fd2c..2f280ea 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -1270,24 +1270,44 @@ again:
#ifdef DEBUG
printf("(re/un)mapping first block in extent\n");
#endif
+ if (physical) {
+ retval = ext2fs_extent_get(handle,
+ EXT2_EXTENT_PREV_LEAF,
+ &extent);
+ if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
+ extent_uninit = 1;
+ if (retval == EXT2_ET_EXTENT_NO_PREV) {
+ retval = ext2fs_extent_insert(handle,
+ 0, &newextent);
+ } else if (retval)
+ goto done;
+ else if ((logical == extent.e_lblk + extent.e_len) &&
+ (physical == extent.e_pblk + extent.e_len) &&
+ (new_uninit == extent_uninit) &&
+ ((int) extent.e_len < max_len-1)) {
+ extent.e_len++;
+ retval = ext2fs_extent_replace(handle, 0,
+ &extent);
+ } else {
+ retval = ext2fs_extent_insert(handle,
+ EXT2_EXTENT_INSERT_AFTER, &newextent);
+ }
+ if (retval)
+ goto done;
+ }
+ retval = ext2fs_extent_fix_parents(handle);
+ if (retval)
+ goto done;
+ retval = ext2fs_extent_get(handle, EXT2_EXTENT_NEXT_LEAF,
+ &extent);
+ if (retval)
+ goto done;
extent.e_pblk++;
extent.e_lblk++;
extent.e_len--;
retval = ext2fs_extent_replace(handle, 0, &extent);
if (retval)
goto done;
- if (physical) {
- /*
- * We've removed the old block, now rely on
- * the optimized hueristics for adding a new
- * mapping with appropriate merging if necessary.
- */
- goto again;
- } else {
- retval = ext2fs_extent_fix_parents(handle);
- if (retval)
- goto done;
- }
} else {
__u32 orig_length;
--
1.6.3.2.1.gb9f7d.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] libext2fs: Make ext2fs_extent_set_bmap() more robust against ENOSPC
2009-07-10 4:23 [PATCH] libext2fs: Make ext2fs_extent_set_bmap() more robust against ENOSPC Theodore Ts'o
@ 2009-07-10 4:45 ` Eric Sandeen
2009-07-10 16:56 ` Eric Sandeen
1 sibling, 0 replies; 3+ messages in thread
From: Eric Sandeen @ 2009-07-10 4:45 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
Theodore Ts'o wrote:
> In the case where we ext2fs_extent_set_bmap() is replacing the block
> mapping at the beginning of an already-existing extent, insert a new
> extent if necessary before shrinking an existing extent, to avoid data
> loss if the disk is full.
>
> This mostly addresses the problem described in Red Hat Bugzilla's
> statistics are still wrong, but at least the files on the filesystem
> are not corrupted. If there is a failure during the
> inode_scan_and_fix pass, the simplest thing to do may be to tell the
> user to run e2fsck -fy.
>
> Addresses-Red-Hat-Bug: #510379
Thanks Ted, looks reasonable at first glance. I'll review more in depth
tomorrow, but I think you can also remove the again: target, since
nobody's going there anymore w/ this patch.
-Eric
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> lib/ext2fs/extent.c | 44 ++++++++++++++++++++++++++++++++------------
> 1 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
> index 4a4fd2c..2f280ea 100644
> --- a/lib/ext2fs/extent.c
> +++ b/lib/ext2fs/extent.c
> @@ -1270,24 +1270,44 @@ again:
> #ifdef DEBUG
> printf("(re/un)mapping first block in extent\n");
> #endif
> + if (physical) {
> + retval = ext2fs_extent_get(handle,
> + EXT2_EXTENT_PREV_LEAF,
> + &extent);
> + if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
> + extent_uninit = 1;
> + if (retval == EXT2_ET_EXTENT_NO_PREV) {
> + retval = ext2fs_extent_insert(handle,
> + 0, &newextent);
> + } else if (retval)
> + goto done;
> + else if ((logical == extent.e_lblk + extent.e_len) &&
> + (physical == extent.e_pblk + extent.e_len) &&
> + (new_uninit == extent_uninit) &&
> + ((int) extent.e_len < max_len-1)) {
> + extent.e_len++;
> + retval = ext2fs_extent_replace(handle, 0,
> + &extent);
> + } else {
> + retval = ext2fs_extent_insert(handle,
> + EXT2_EXTENT_INSERT_AFTER, &newextent);
> + }
> + if (retval)
> + goto done;
> + }
> + retval = ext2fs_extent_fix_parents(handle);
> + if (retval)
> + goto done;
> + retval = ext2fs_extent_get(handle, EXT2_EXTENT_NEXT_LEAF,
> + &extent);
> + if (retval)
> + goto done;
> extent.e_pblk++;
> extent.e_lblk++;
> extent.e_len--;
> retval = ext2fs_extent_replace(handle, 0, &extent);
> if (retval)
> goto done;
> - if (physical) {
> - /*
> - * We've removed the old block, now rely on
> - * the optimized hueristics for adding a new
> - * mapping with appropriate merging if necessary.
> - */
> - goto again;
> - } else {
> - retval = ext2fs_extent_fix_parents(handle);
> - if (retval)
> - goto done;
> - }
> } else {
> __u32 orig_length;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] libext2fs: Make ext2fs_extent_set_bmap() more robust against ENOSPC
2009-07-10 4:23 [PATCH] libext2fs: Make ext2fs_extent_set_bmap() more robust against ENOSPC Theodore Ts'o
2009-07-10 4:45 ` Eric Sandeen
@ 2009-07-10 16:56 ` Eric Sandeen
1 sibling, 0 replies; 3+ messages in thread
From: Eric Sandeen @ 2009-07-10 16:56 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
Theodore Ts'o wrote:
> In the case where we ext2fs_extent_set_bmap() is replacing the block
> mapping at the beginning of an already-existing extent, insert a new
> extent if necessary before shrinking an existing extent, to avoid data
> loss if the disk is full.
>
> This mostly addresses the problem described in Red Hat Bugzilla's
> statistics are still wrong, but at least the files on the filesystem
> are not corrupted. If there is a failure during the
> inode_scan_and_fix pass, the simplest thing to do may be to tell the
> user to run e2fsck -fy.
>
> Addresses-Red-Hat-Bug: #510379
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> lib/ext2fs/extent.c | 44 ++++++++++++++++++++++++++++++++------------
> 1 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
> index 4a4fd2c..2f280ea 100644
> --- a/lib/ext2fs/extent.c
> +++ b/lib/ext2fs/extent.c
> @@ -1270,24 +1270,44 @@ again:
> #ifdef DEBUG
> printf("(re/un)mapping first block in extent\n");
> #endif
> + if (physical) {
> + retval = ext2fs_extent_get(handle,
> + EXT2_EXTENT_PREV_LEAF,
> + &extent);
> + if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
> + extent_uninit = 1;
> + if (retval == EXT2_ET_EXTENT_NO_PREV) {
> + retval = ext2fs_extent_insert(handle,
> + 0, &newextent);
> + } else if (retval)
> + goto done;
A lot of this is cut & paste from "mapping unmapped logical block" - at
some point we should probably factor out into helpers, but fine for now.
A little comment here about extent merging may be helpful just to guide
the reader; it's a lot of conditionals to eyeball - not so difficult in
the end, but not all that conducive to a quick skim either.
Aside from those nitpicks (and the now-extraneous again: goto target)
this look fine to me, thanks.
-Eric
> + else if ((logical == extent.e_lblk + extent.e_len) &&
> + (physical == extent.e_pblk + extent.e_len) &&
> + (new_uninit == extent_uninit) &&
> + ((int) extent.e_len < max_len-1)) {
> + extent.e_len++;
> + retval = ext2fs_extent_replace(handle, 0,
> + &extent);
> + } else {
> + retval = ext2fs_extent_insert(handle,
> + EXT2_EXTENT_INSERT_AFTER, &newextent);
> + }
> + if (retval)
> + goto done;
> + }
> + retval = ext2fs_extent_fix_parents(handle);
> + if (retval)
> + goto done;
> + retval = ext2fs_extent_get(handle, EXT2_EXTENT_NEXT_LEAF,
> + &extent);
> + if (retval)
> + goto done;
> extent.e_pblk++;
> extent.e_lblk++;
> extent.e_len--;
> retval = ext2fs_extent_replace(handle, 0, &extent);
> if (retval)
> goto done;
> - if (physical) {
> - /*
> - * We've removed the old block, now rely on
> - * the optimized hueristics for adding a new
> - * mapping with appropriate merging if necessary.
> - */
> - goto again;
> - } else {
> - retval = ext2fs_extent_fix_parents(handle);
> - if (retval)
> - goto done;
> - }
> } else {
> __u32 orig_length;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-07-10 16:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-10 4:23 [PATCH] libext2fs: Make ext2fs_extent_set_bmap() more robust against ENOSPC Theodore Ts'o
2009-07-10 4:45 ` Eric Sandeen
2009-07-10 16:56 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.