Skip to content

Commit

Permalink
utils: rm_offset_get_from_fd: Don't combine separate extents
Browse files Browse the repository at this point in the history
rmlint previously ignored holes between physically adjacent extents.
This could cause files to be detected as reflinked that weren't, if one
file had a complete extent and the other file had two pieces of it
separated by a hole.

Check for logical gaps as well as physical gaps to fix this problem.

Fixes sahib#530
  • Loading branch information
cebtenzzre committed Feb 24, 2023
1 parent b64f89f commit 50c3142
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 14 deletions.
10 changes: 6 additions & 4 deletions lib/utilities.c
Original file line number Diff line number Diff line change
Expand Up @@ -1024,9 +1024,9 @@ RmOff rm_offset_get_from_fd(int fd, RmOff file_offset, RmOff *file_offset_next,
RmOff result = 0;
bool done = FALSE;
bool first = TRUE;
struct fiemap_extent fm_last;

/* used for detecting contiguous extents */
unsigned long expected = 0;
memset(&fm_last, 0, sizeof(fm_last));

fsync(fd);

Expand Down Expand Up @@ -1061,7 +1061,9 @@ RmOff rm_offset_get_from_fd(int fd, RmOff file_offset, RmOff *file_offset_next,
first = FALSE;
} else {
/* check if subsequent extents are contiguous */
if(fm_ext.fe_physical != expected) {
unsigned long expected_dense = fm_last.fe_physical + fm_last.fe_length;
unsigned long expected = fm_last.fe_physical + fm_ext.fe_logical - fm_last.fe_logical;
if(fm_ext.fe_physical != expected || fm_ext.fe_physical != expected_dense) {
/* current extent is not contiguous with previous, so we can stop */
g_free(fm);
break;
Expand All @@ -1083,7 +1085,7 @@ RmOff rm_offset_get_from_fd(int fd, RmOff file_offset, RmOff *file_offset_next,

/* move offsets in preparation for reading next extent */
file_offset = fm_ext.fe_logical + fm_ext.fe_length;
expected = fm_ext.fe_physical + fm_ext.fe_length;
fm_last = fm_ext;
}

g_free(fm);
Expand Down
39 changes: 29 additions & 10 deletions tests/test_mains/test_is_reflink.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,26 +142,45 @@ def _copy_file_range(src, dst, count, offset_src, offset_dst):
raise RuntimeError('copy_file_range only copied {} bytes (expected {})'.format(bytes_copied, count))


# GitHub issue #611: Make sure holes can be detected when the physical offsets and logical
# extent ends are otherwise the same.
@needs_reflink_fs
@with_setup(usual_setup_func, usual_teardown_func)
def test_hole_before_extent():
def kb(n):
return 1024 * n


def _hole_testcase_inner(extents):
path_a = os.path.join(TESTDIR_NAME, 'a')
path_b = os.path.join(TESTDIR_NAME, 'b')

def kb(n): return 1024 * n

_run_dd_urandom(path_a, '8K', 2)
_run_dd_urandom(path_a, kb(16) // extents, extents)
with open(path_b, 'wb') as f:
os.truncate(f.fileno(), kb(16)) # same-sized file with no extents

with open(path_a, 'rb') as fsrc, open(path_b, 'wb') as fdst:
infd, outfd = fsrc.fileno(), fdst.fileno()
yield infd, outfd

# expect RM_LINK_NONE
check_is_reflink_status(1, path_a, path_b)


# GitHub issue #611: Make sure holes can be detected when the physical offsets and logical
# extent ends are otherwise the same.
@needs_reflink_fs
@with_setup(usual_setup_func, usual_teardown_func)
def test_hole_before_extent():
for infd, outfd in _hole_testcase_inner(extents=2):
# copy first half of first extent with 4K offset
_copy_file_range(infd, outfd, kb(4), kb(0), kb(4))
# copy second extent
_copy_file_range(infd, outfd, kb(8), kb(8), kb(8))

# expect RM_LINK_NONE
check_is_reflink_status(1, path_a, path_b)

# GitHub issue #530: Make sure physically adjacent extents aren't merged if there is a
# hole between them logically.
@needs_reflink_fs
@with_setup(usual_setup_func, usual_teardown_func)
def test_hole_between_extents():
for infd, outfd in _hole_testcase_inner(extents=1):
# copy first extent
_copy_file_range(infd, outfd, kb(8), kb(0), kb(0))
# copy first half of second extent with 4K offset
_copy_file_range(infd, outfd, kb(4), kb(8), kb(12))

0 comments on commit 50c3142

Please sign in to comment.