libext2fs: reset handle after inserting new extent
PATCH 1/1 libext2fs: reset handle after inserting new extent
Commit 53422e8a5644e22ea3f6e0efba82a765b72e4308 moved the new extent insertion in ext2fs_extent_set_bmap prior to the modification of the original extent, but the insert function left the handle pointing a the new extent; this left us modifying the -new- extent not the original one, and winding up with a corrupt extent tree something like:
BLOCKS:
(0-1):588791-588792, (0):588791
We need to move back to the previous extent prior to modification, if we inserted a new one.
---
(aside: should there be functions to simply move the handle around, w/o necessarily populating an extent structure?)
(aside2: maybe ext2fs_extent_insert should take a flag saying whether the handle should be moved after the insertion; moving it around to back where we want to be seems a bit tedious)
lib/ext2fs/extent.c +5 -0
@@ -1257,6 +1257,11 @@ again: EXT2_EXTENT_INSERT_AFTER, &newextent);if (retval) goto done; +/* Now pointing at new extent; move back to prev */ + retval =ext2fs_extent_get (handle,EXT2_EXTENT_PREV , + &extent); +if (retval) + goto done; } extent.e_len--; retval =ext2fs_extent_replace (handle,0 , &extent);
Quoted 8 lines · Eric Sandeen
Commit
53422e8a5644e22ea3f6e0efba82a765b72e4308moved the new extent insertion inext2fs_extent_set_bmapprior to the modification of the original extent, but the insert function left the handle pointing a the new extent; this left us modifying the -new- extent not the original one, and winding up with a corrupt extent tree something like:
Quoted 5 lines
BLOCKS:
(0-1):588791-588792, (0):588791We need to move back to the previous extent prior to modification, if we inserted a new one.
Hmm, I just thought of something awful; what if the insert resulted in a node split? Instead of using ext2fs_extent_get(EXT2_EXTENT_PREV), we may need to use ext2fs_extent_goto() to seek to the correct logical block instead.
- Ted
Quoted 15 lines · Theodore Tso
Quoted 8 lines · Eric Sandeen
Commit
53422e8a5644e22ea3f6e0efba82a765b72e4308moved the new extent insertion inext2fs_extent_set_bmapprior to the modification of the original extent, but the insert function left the handle pointing a the new extent; this left us modifying the -new- extent not the original one, and winding up with a corrupt extent tree something like:Quoted 5 lines
BLOCKS:
(0-1):588791-588792, (0):588791We need to move back to the previous extent prior to modification, if we inserted a new one.
Quoted 6 lines
Hmm, I just thought of something awful; what if the insert resulted in a node split? Instead of using
ext2fs_extent_get(EXT2_EXTENT_PREV), we may need to useext2fs_extent_goto()to seek to the correct logical block instead.- Ted
Hm....
so PREV doesn't go to the node for the previous logical block, but to what, the node to the left at this level? Guess I need to read more carefully...
-Eric
PATCH v2 1/1 libext2fs: reset handle after inserting new extent
In reply to Eric SandeenCommit 53422e8a5644e22ea3f6e0efba82a765b72e4308 moved the new extent insertion in ext2fs_extent_set_bmap prior to the modification of the original extent, but the insert function left the handle pointing a the new extent; this left us modifying the -new- extent not the original one, and winding up with a corrupt extent tree something like:
BLOCKS:
(0-1):588791-588792, (0):588791
We need to move back to the previous extent prior to modification, if we inserted a new one.
V2: Use extent_goto vs. extent_get.
---
lib/ext2fs/extent.c +4 -0
@@ -1257,6 +1257,10 @@ again: EXT2_EXTENT_INSERT_AFTER, &newextent);if (retval) goto done; +/* Now pointing at inserted extent; move back to prev */ + retval =ext2fs_extent_goto (handle, logical -1 ); +if (retval) + goto done; } extent.e_len--; retval =ext2fs_extent_replace (handle,0 , &extent);