| Message ID | 20160601082943.10364-1-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Jun 01, 2016 at 04:29:43PM +0800, Qu Wenruo wrote: > New convert doesn't insert holes for superblock migration range. > > Unlike old design, which only relocate 4K(superblock size) to other > places. > In new design, to make sure convert can handle different page size and > align chunk bytenr, we relocate the whole 64K range. This looks like a potential backward compatibility problem. In the unlikely scenario, mixing old and new convert to do conversion and rollback. Did you try that? > And if there is only a 4K used block inside 64K superblock migration range, it > will make converted fs has discontinuous file extents. > > This patch will fix it by inserting needed holes to avoid such > discontinuous error. > > Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > David, I'm very sorry, all these recently exposed bugs are related to > superblock migration range, and will only be triggered by special build > ext* images(Needs special space range be allocated in ext*) > > I'll try my best to create the minimal trigger e2image dump, our current > image is too large for test cases. Well, given the amount of last minute fixes I'm now hesitant to do 4.6 release with this patchset. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
At 06/01/2016 09:49 PM, David Sterba wrote: > On Wed, Jun 01, 2016 at 04:29:43PM +0800, Qu Wenruo wrote: >> New convert doesn't insert holes for superblock migration range. >> >> Unlike old design, which only relocate 4K(superblock size) to other >> places. >> In new design, to make sure convert can handle different page size and >> align chunk bytenr, we relocate the whole 64K range. > > This looks like a potential backward compatibility problem. In the > unlikely scenario, mixing old and new convert to do conversion and > rollback. Did you try that? Tried, old convert + new rollback is fine. Although for new convert + old rollback, it may fail to rollback some converted case. But I didn't consider new convert + old rollback will be a normal use case. For old converted fs, it's pretty strict for may_rollback(). Almost all ext2 image file extent must be 1:1 mapped, except the first 1M and backup superblocks. While for new convert, it exception range is larger, 0~1M is the same, while for backup superblocks, its range is extended from 4K to 64K. So new convert is compatible with old convert and is able to rollback old converted fs. Although old convert can't rollback new converted fs. > >> And if there is only a 4K used block inside 64K superblock migration range, it >> will make converted fs has discontinuous file extents. >> >> This patch will fix it by inserting needed holes to avoid such >> discontinuous error. >> >> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >> --- >> David, I'm very sorry, all these recently exposed bugs are related to >> superblock migration range, and will only be triggered by special build >> ext* images(Needs special space range be allocated in ext*) >> >> I'll try my best to create the minimal trigger e2image dump, our current >> image is too large for test cases. > > Well, given the amount of last minute fixes I'm now hesitant to do 4.6 > release with this patchset. I'm really sorry for that. Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 02, 2016 at 08:38:03AM +0800, Qu Wenruo wrote: > > > At 06/01/2016 09:49 PM, David Sterba wrote: > > On Wed, Jun 01, 2016 at 04:29:43PM +0800, Qu Wenruo wrote: > >> New convert doesn't insert holes for superblock migration range. > >> > >> Unlike old design, which only relocate 4K(superblock size) to other > >> places. > >> In new design, to make sure convert can handle different page size and > >> align chunk bytenr, we relocate the whole 64K range. > > > > This looks like a potential backward compatibility problem. In the > > unlikely scenario, mixing old and new convert to do conversion and > > rollback. Did you try that? > > Tried, old convert + new rollback is fine. > Although for new convert + old rollback, it may fail to rollback some > converted case. > > But I didn't consider new convert + old rollback will be a normal use case. It's not meant to be a normal use case, but such thing can happen so I want to know how it behaves. If old convert rollback fails on new image, then it's ok, ie. we want to prevent accidental damage. > For old converted fs, it's pretty strict for may_rollback(). > Almost all ext2 image file extent must be 1:1 mapped, except the first > 1M and backup superblocks. > > While for new convert, it exception range is larger, 0~1M is the same, > while for backup superblocks, its range is extended from 4K to 64K. > > So new convert is compatible with old convert and is able to rollback > old converted fs. This could possibly happen so good that it works in that direction. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/btrfs-convert.c b/btrfs-convert.c index 42d646c..25c0d17 100644 --- a/btrfs-convert.c +++ b/btrfs-convert.c @@ -1375,6 +1375,8 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans, { u64 cur_off = start; u64 cur_len = len; + u64 hole_start = start; + u64 hole_len; struct cache_extent *cache; struct btrfs_key key; struct extent_buffer *eb; @@ -1426,9 +1428,23 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans, ret = csum_disk_extent(trans, root, key.objectid, key.offset); + /* Don't forget to insert hole */ + hole_len = cur_off - hole_start; + if (hole_len) { + ret = btrfs_record_file_extent(trans, root, ino, inode, + hole_start, 0, hole_len); + if (ret < 0) + break; + } + cur_off += key.offset; + hole_start = cur_off; cur_len = start + len - cur_off; } + /* Last hole */ + if (start + len - hole_start > 0) + ret = btrfs_record_file_extent(trans, root, ino, inode, + hole_start, 0, start + len - hole_start); return ret; }
New convert doesn't insert holes for superblock migration range. Unlike old design, which only relocate 4K(superblock size) to other places. In new design, to make sure convert can handle different page size and align chunk bytenr, we relocate the whole 64K range. And if there is only a 4K used block inside 64K superblock migration range, it will make converted fs has discontinuous file extents. This patch will fix it by inserting needed holes to avoid such discontinuous error. Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- David, I'm very sorry, all these recently exposed bugs are related to superblock migration range, and will only be triggered by special build ext* images(Needs special space range be allocated in ext*) I'll try my best to create the minimal trigger e2image dump, our current image is too large for test cases. Thanks, Qu --- btrfs-convert.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)