Skip to content

create: do not wrap repository writes in backup_io("read") - #9854

Merged
ThomasWaldmann merged 1 commit into
borgbackup:1.4-maintfrom
ThomasWaldmann:fix-create-backup-io-repo-write-1.4
Jul 3, 2026
Merged

create: do not wrap repository writes in backup_io("read")#9854
ThomasWaldmann merged 1 commit into
borgbackup:1.4-maintfrom
ThomasWaldmann:fix-create-backup-io-repo-write-1.4

Conversation

@ThomasWaldmann

Copy link
Copy Markdown
Member

What

process_file() ran process_file_chunks() inside with backup_io('read'). That block is meant to guard reading the source file, but the source reads are already guarded individually by backup_io_iter(). The outer wrapper additionally caught add_chunk()'s (and maybe_checkpoint()'s) repository writes, so a repository IO failure — e.g. the repo running out of space — was wrapped into a per-file BackupOSError tagged "read".

Symptom

On an out-of-space repository, borg create emitted misleading per-file warnings:

<path>: read: [Errno 28] No space left on device, slept ..., next: retry: 1 of 9...

i.e. a repository-full condition was reported as if the source file couldn't be read, and the file was pointlessly retried. This contradicts the BackupOSError docstring:

These are non-critical and are only reported (warnings). Any unwrapped IO error is critical and aborts execution (for example repository IO failure).

Note: in 1.4 this is not data loss — the transactional segment repository still rolls the partial transaction back on the eventual failure ("No space left on device, cleaning up partial transaction to free space"), so no corrupt archive is committed. This PR is a correctness/UX fix: the misclassified warning text and needless read-retries of a repository error are wrong regardless. (In borg2/master the same wrapping caused silent data loss because borgstore has no equivalent rollback; fixed separately in #9853.)

Fix

Drop the outer backup_io('read') wrapper around process_file_chunks(). Source reads stay per-file warnings (backup_io_iter is unchanged); repository OSErrors are now left unwrapped and therefore critical, aborting promptly with the correct error.

Audited all 23 with backup_io blocks in archive.py/archiver.py (including indirect repo access via fetch_many, preload, add_item, write_part, maybe_checkpoint): this regular-file backup path was the only one wrapping a repository operation. The stdin/pipe and import-tar process_file_chunks call sites were already unwrapped, and the extract path deliberately keeps the repo read (fetch_many) outside backup_io('write').

Testing

  • testsuite/archive.py (38) and create-related testsuite/archiver.py tests (52) pass.
  • Reproduced on a space-limited macOS ramdisk (source data > free space):
    • Before: create emitted many read: [Errno 28] retry warnings, then rolled back.
    • After: create aborts immediately with the correct critical error, commits no archive, and the repository stays consistent (check ok). Normal backups and unreadable-source-file handling (per-file warning, archive still created) are unchanged.

🤖 Generated with Claude Code

process_file() ran process_file_chunks() inside `with backup_io('read')`.
That block is meant to guard reading the *source* file, but the source reads
are already guarded individually by backup_io_iter(). The outer wrapper also
caught add_chunk()'s (and maybe_checkpoint()'s) *repository* writes, so a
repository IO failure -- e.g. the repo running out of space -- was wrapped
into a per-file BackupOSError tagged "read". Borg then emitted misleading
"<path>: read: [Errno 28] No space left on device" warnings, pointlessly
retried the file, and only treated a critical repository error as a
non-critical per-file one, contrary to the BackupOSError docstring ("Any
unwrapped IO error is critical and aborts execution (for example repository
IO failure)").

In 1.4 the transactional repository still rolls the partial transaction back
on the eventual failure, so this is not data loss here (unlike borg2, where
it silently commits a corrupt archive). But the misclassification -- wrong
warning text and needless read-retries of a repository-full condition -- is
wrong regardless.

Drop the outer backup_io('read') wrapper. Source reads stay per-file
warnings (backup_io_iter is unchanged); repository OSErrors are now left
unwrapped and critical, aborting promptly with the correct error.

Verified on a space-limited macOS ramdisk: before, create emitted many
"read: [Errno 28]" retry warnings then rolled back; after, it aborts
immediately ("No space left on device, cleaning up partial transaction"),
commits no archive, and the repo stays consistent. Normal backups and
unreadable-source-file handling (per-file warning) are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.06%. Comparing base (b09bbed) to head (0c6e8e1).
⚠️ Report is 4 commits behind head on 1.4-maint.

Additional details and impacted files
@@              Coverage Diff              @@
##           1.4-maint    #9854      +/-   ##
=============================================
+ Coverage      82.04%   82.06%   +0.02%     
=============================================
  Files             38       38              
  Lines          11384    11383       -1     
  Branches        1794     1794              
=============================================
+ Hits            9340     9342       +2     
+ Misses          1460     1459       -1     
+ Partials         584      582       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@ThomasWaldmann ThomasWaldmann changed the title create: don't wrap repository writes in backup_io('read') create: do not wrap repository writes in backup_io("read") Jul 3, 2026
@ThomasWaldmann
ThomasWaldmann merged commit 88fcfb9 into borgbackup:1.4-maint Jul 3, 2026
13 of 14 checks passed
@ThomasWaldmann
ThomasWaldmann deleted the fix-create-backup-io-repo-write-1.4 branch July 3, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant