dht: SEEK returns spurious ENXIO during rebalance instead of redirecting to the destination

[ThalesBarretto]

Component

cluster/dht — xlators/cluster/dht/src/dht-inode-read.c (dht_seek_cbk)

Description

During an active file migration (rebalance / remove-brick), a SEEK_DATA or
SEEK_HOLE on an fd that is still open on the source subvolume can return ENXIO
to the application, even though the file's data is intact on the destination.

The DHT FOP callbacks redirect an in-flight operation to the migration destination
when they detect the file is being migrated. The data-path callbacks
(dht_readv_cbk, dht_writev_cbk, dht_file_attr_cbk, …) receive an iatt and
detect migration proactively via IS_DHT_MIGRATION_PHASE2(stbuf) (the source carries
the linkto-file mode during the final phase). dht_seek_cbk receives no iatt —
its only migration signal is the errno — and it only redirects on the "file missing"
errnos:

/* dht-inode-read.c, dht_seek_cbk */
local->op_errno = op_errno;
if ((op_ret == -1) && !dht_inode_missing(op_errno)) {   /* ENOENT || ESTALE only */
    gf_msg_debug(this->name, op_errno, "subvolume %s returned -1", prev->name);
    goto out;
}
if ((op_ret == -1) && ((op_errno == ENXIO) || (op_errno == EOVERFLOW)))
    goto out;                                            /* dead: already left above */
...
local->rebalance.target_op_fn = dht_seek2;
ret = dht_rebalance_complete_check(this, frame);         /* the redirect */

Near the end of dht_migrate_file the source is truncated to 0
(dht-rebalance.c:2109) and later unlinked (:2154); the source's linkto xattr is
set much earlier (:1224). In the window after the truncate, a SEEK_DATA/SEEK_HOLE
that reaches the (now 0-byte) source returns ENXIO — a normal "no data at/after
offset" result at the POSIX layer. That ENXIO is not ENOENT/ESTALE, so
dht_seek_cbk takes the generic goto out and passes it to the client. The migration
redirect (dht_rebalance_complete_check) is never reached.

The explicit if (... ENXIO || EOVERFLOW) goto out; below the generic handler is
dead code — unreachable since it was added (both lines were introduced together in
the original SEEK-at-DHT implementation, PR #3792, which fixed #3373), because the generic
!dht_inode_missing(op_errno) handler already catches those errnos first. The migration
redirect for SEEK was written but never actually reached.

Affected versions: v11.0, v11.1, v11.2 (dht_seek was introduced in v11 by #3792;
dht_seek_cbk has not been modified since). The existing SEEK tests
(tests/basic/distribute/dht_seek_test.t, sparse_file_rebalance.t) exercise
cp --sparse statically or check integrity only after rebalance completes, so none
holds an fd open and issues a SEEK during migration — which is why this slipped through.

Impact

Severity: Low (narrow trigger), but the failure is a silent wrong result, not a
benign retry:

• Only lseek(SEEK_DATA) / lseek(SEEK_HOLE) reach DHT as a SEEK FOP — the FOP's
  gf_seek_what_t has only GF_SEEK_DATA/GF_SEEK_HOLE, and SEEK_SET/CUR/END are
  resolved above the FOP layer — so normal positioning and read/write are unaffected. The
  exposed surface is
  sparse-extent scanners — cp --sparse=always, tar -S, qemu-img convert, and
  similar. These tools treat SEEK_DATA=ENXIO as end-of-data and stop; they do not
  retry, so a spurious ENXIO makes them silently produce a short / all-holes copy.
• On the same fd during the same window, fstat() and read() return the correct size
  and data (they redirect), so the file looks present while SEEK reports "no data" —
  the inconsistency is what misleads the scanner.
• The ENXIO is sticky for a pure SEEK workload: because SEEK does not re-resolve on
  ENXIO, the held fd keeps hitting the truncated source until something else re-resolves
  it (a read/fstat/lookup, or a graph switch). It is not limited to the truncate→unlink
  interval.

The trigger is still narrow: the application must hold an fd opened before migration and
issue a SEEK that lands after the source truncate, during an admin-initiated rebalance.

Reproduction (deterministic)

Reproduced on a 2-brick distribute volume (single-node and 2-node), GlusterFS devel:

1. Create a data file F on brick A; open an fd on it and keep it open.
2. gluster volume remove-brick <brickA> start to migrate F to brick B.
3. While the migration is in the truncate→unlink window (freeze the rebalance daemon at
   dht-rebalance.c:2118 under gdb to make this deterministic), issue SEEK_DATA(fd,0).

Observed on the held fd, during the window:

fstat().st_size = <full size>      # redirected -> correct
pread(fd, ,1, 0) = <real byte>     # redirected -> correct
lseek(fd, 0, SEEK_DATA) = -1 ENXIO # NOT redirected -> spurious
lseek(fd, 0, SEEK_HOLE) = -1 ENXIO

Source brick backend at that instant: F is 0 bytes, a sticky-only regular file
(---------T; permission bits 01000 == DHT_LINKFILE_MODE, full st_mode 0101000),
with trusted.glusterfs.dht.linkto set; the destination brick holds the full file. A pure SEEK_DATA loop on the held fd returns ENXIO repeatedly until a
read/fstat/lookup re-resolves the inode to the destination.

Fix

Exempt ENXIO from the generic error handler so a SEEK during migration falls through to
the redirect, and remove the now-redundant dead check:

if ((op_ret == -1) && !dht_inode_missing(op_errno) && (op_errno != ENXIO)) {
    goto out;
}

After the fix, a window SEEK_DATA on the source falls through to
dht_rebalance_complete_check, which reads the source linkto xattr, re-resolves to the
destination, and dht_seek2 re-dispatches the SEEK there — returning the correct offset.
Verified on the same testbed: the window SEEK returns 0 (data at offset 0) instead of
ENXIO, with no change to non-migrating SEEK behavior.

Only ENXIO is exempted — the old dead check also named EOVERFLOW, but that is not a
migration signal. ENXIO means "offset >= EOF" (data-dependent — the truncated source
raises it, the full destination does not, which is why redirecting helps). EOVERFLOW
means "the seek result does not fit off_t" — subvolume-independent, so a redirect would
just return the same error — and with a 64-bit off_t a SEEK_DATA/SEEK_HOLE cannot
raise it at all (confirmed empirically on the testbed: every offset returns data or ENXIO,
never EOVERFLOW).

Known trade-off: because ENXIO is also the normal EOF signal for
SEEK_DATA/SEEK_HOLE, this adds one syncop_getxattr(linkto) on the source per terminal
ENXIO, even for non-migrating files (the same check already done for ENOENT/ESTALE,
but hit once per file by sparse scanners). A cheaper in-memory gate is not available (SEEK
has no iatt, and the inode migration info is not yet set during the in-progress window),
so the cost is intrinsic to an errno-based fix; it is accepted because the alternative is a
silent wrong result.