PosixFile.fsBlockSize asks stat for %s, making dd transfers to XFS hosts crawl
- Dominant language
- Go
- Stars
- 53
- Forks
- 33
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 29
Description
## What happens
Uploading a file to a host whose target directory is on XFS runs dd with a
block size of a few dozen bytes. A k0sctl run uploading a k0s airgap image
bundle into `/var/lib/k0s/images` produced:
```
INFO ==> Running phase: Upload files to hosts
INFO [ssh] host:22: uploading bundle_file.tar
```
with the host command:
```
sudo -- dd if=/dev/stdin of=/var/lib/k0s/images/bundle_file.tar bs=29 seek=0 conv=notrunc
```
`bs=29`, so dd copies a multi-hundred-megabyte bundle 29 bytes at a time —
roughly 18 million read/write syscall pairs for a 500 MB bundle. The upload
takes hours instead of seconds.
## Why
`PosixFile.fsBlockSize` in `remotefs/posixfile.go` asks stat for `%s`:
```go
out, err := f.fs.ExecOutput(fmt.Sprintf(
`stat -c "%%s" %[1]s 2> /dev/null || stat -f "%%k" %[1]s`,
shellescape.Quote(path.Dir(f.path))))
```
`%s` is total size in bytes. The intent was `%o`, the optimal I/O block size —
which is exactly what the BSD `%k` in the same fallback expression means.
It stats the *parent directory*, and a directory's size in bytes is
filesystem-specific:
- **ext4** stores directory data in whole blocks, so `stat -c %s` returns 4096
and coincidentally equals `%o`. The bug is invisible.
- **XFS** stores a small directory inline in the inode, so the size is the size
of that inline data.
The 29 is the XFS shortform directory layout for a directory holding exactly
one entry, which is what rig itself has just created:
| part | bytes |
| --- | --- |
| header: `count` + `i8count` + parent inode | 1 + 1 + 4 = 6 |
| entry: `namelen` + `offset` + `name` + `ftype` + inode | 1 + 2 + 15 + 1 + 4 = 23 |
| **total** | **29** |
`bundle_file.tar` is 15 characters.
## Reproducing
On any host with an XFS `/var/lib`:
```
$ stat -c "%s %o" /var/lib/k0s/images
29 4096
```
The first number is what rig uses; the second is what it meant. On ext4 both
read 4096, which is why this has not come up before.
## Also
Two things next to it in the same code path:
- If stat exits 0 but prints something that does not parse as an integer,
`fsBlockSize` leaves `blockSize` at 0 and `ddParams` panics with an integer
divide by zero (`numBytes % 0`).
- `CopyFrom` passes `f.pos` to dd's `seek=`, which dd counts in output blocks
rather than bytes, so a resumed copy writes at `f.pos * bs`. Harmless at
offset 0, which is the ordinary upload.
## Versions
`fsBlockSize` is unchanged from v2.0.0 through v2.2.0 and `main` (a69b40a).
The report above comes from a k0sctl whose bundled rig still passes
`if=/dev/stdin`, so it predates #425, but the defect is the same on current
`main`.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in remotefs/posixfile.go by reading fsBlockSize and its callers, especially ddParams and CopyFrom. Reproduce the differing stat values on XFS and ext4, then verify the transfer block size, malformed stat output, and resumed-copy offset behavior. Done means uploads no longer use the directory's byte size and the reported edge cases do not cause incorrect transfers or panics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100