From b339cb58f0e0d1c32bbf17a92e03ed49d720ce8b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 24 Aug 2026 18:19:58 +0200 Subject: [PATCH 1/4] check: survive I/O errors while reading repository objects, fixes #3509 An OSError from the store (failing disk, flaky network filesystem, ...) escaped up to the top-level handler, so borg check died with a "Local Exception" traceback and the results collected so far were lost. Reads of repository objects now raise Repository.StoreReadError, keeping the original OSError as __cause__. It is deliberately distinct from ObjectNotFound and IntegrityError: borg never saw the content, so it can tell neither that the object is missing nor that it is corrupt. The repository check reports such an object, goes on and fails at the end, so one run lists everything that is affected. No result is recorded for it, so a later check verifies it again rather than remembering it as corrupt. The archives check reports the affected archive and continues with the next one. Because the data may well be readable again once the underlying problem is fixed, nothing acts on an unreadable object: --repair refuses to repair, the index is not rebuilt from packs that could not be read, and --verify-data leaves those chunks in place instead of deleting them. --- docs/changes.rst | 6 + docs/internals/frontends.rst | 4 + docs/usage/check.rst.inc | 14 ++ src/borg/archive.py | 168 +++++++++++------- src/borg/archiver/check_cmd.py | 14 ++ src/borg/cache.py | 24 ++- src/borg/repository.py | 101 +++++++++-- src/borg/testsuite/archiver/check_cmd_test.py | 118 +++++++++++- 8 files changed, 360 insertions(+), 89 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index d3139fa75a..e1f5b5d18a 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -197,6 +197,12 @@ Fixes: packs, #9443 - --find-lost-archives reuses the archive metadata ids found by --verify-data, #8478 - --repair rebuilds a corrupt index only once, in the archives phase, #10434 + - do not crash with a traceback when a repository object can not be read + (I/O error, e.g. failing disk or flaky network filesystem). The affected object + is reported, the check continues and fails at the end. Such an object is not + recorded as corrupt (a later check verifies it again), ``--repair`` refuses to + repair around it and ``--verify-data`` no longer deletes chunks it could not + read, #3509 - extract --continue: skip complete dirs, count skipped files in --progress, #6857 - create: back up automounted directories, #6652 - security: same local repo dir via another path is not a relocation, #8188 diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 3140b7514a..af3fb1c06e 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -877,6 +877,10 @@ Errors Repository {} needs its key to access the store object {}, but no key was set. Repository.LegacyRepository rc: 29 traceback: no {} looks like a borg 1.x repository, use --from-borg1 to access it (e.g. with borg transfer). + Repository.RepairUnsafe rc: 34 traceback: no + Not repairing: {} repository object(s) could not be read. Fix the underlying problem, then check again. + Repository.StoreReadError rc: 35 traceback: no + Error reading {} from the repository: {}. Check the storage hardware / filesystem. MandatoryFeatureUnsupported rc: 25 traceback: no Unsupported repository feature(s) {}. A newer version of Borg is required to access this repository. diff --git a/docs/usage/check.rst.inc b/docs/usage/check.rst.inc index 8b4378b8e6..aaca31cab0 100644 --- a/docs/usage/check.rst.inc +++ b/docs/usage/check.rst.inc @@ -210,6 +210,20 @@ would report them as lost. After a ``--repair`` that stored or deleted chunks, b packs the repair wrote, makes the chunk index match them and stores it; that always runs to completion, also after a Ctrl-C. +Unreadable repository objects ++++++++++++++++++++++++++++++ + +A repository object that cannot be read at all (an I/O error from a failing disk, a +flaky network filesystem, ...) is different from a corrupt one: Borg never saw its +content, so it cannot tell whether the object is fine. Such an object is reported and +the check continues, so one run lists everything that is affected; the check then +fails. The object is not remembered as corrupt, so a later check verifies it again. + +Because the data may well be readable again once the underlying problem is fixed, +``--repair`` refuses to repair while there are unreadable objects, and +``--verify-data`` leaves chunks it could not read in place instead of deleting them. +Fix the storage hardware, filesystem or network first, then run the check again. + About repair mode +++++++++++++++++ diff --git a/src/borg/archive.py b/src/borg/archive.py index 00a32902eb..af06bf927e 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2607,7 +2607,8 @@ def verify_data(self): verified = 0 # chunks actually verified defect_chunks = [] archive_meta_ids = set() - untrusted_pack_ids = set() # ids of the packs that are missing or hold a defect chunk + untrusted_pack_ids = set() # ids of the packs that are missing, unreadable or hold a defect chunk + unreadable_chunks = 0 # chunks we could not even read, refs #3509 pi = ProgressIndicatorPercent( total=chunks_count, msg="Verifying data %6.2f%%", step=0.01, msgid="check.verify_data" ) @@ -2652,6 +2653,21 @@ def verify_data(self): for missing_id in chunk_ids: logger.debug("chunk %s: pack %s is missing.", bin_to_hex(missing_id), bin_to_hex(pack_id)) pi.show(increase=lost) + except Repository.StoreReadError as err: + # the pack could not be read (I/O error), and like above, get_many() raises before any chunk + # of this pack was read. unreadable, not defect: we did not see the content, so we must not + # conclude it is bad and delete it - it may read fine once the hardware/fs problem is fixed, + # refs #3509. + self.error_found = True + untrusted_pack_ids.add(pack_id) # like a missing pack: its objects may be unreadable + unreadable = len(chunk_ids) + errors += unreadable + unreadable_chunks += unreadable + verified += unreadable # they are not read, but they are accounted for, like a failed read + logger.error("%s, %d chunks could not be verified.", err, unreadable) + for unreadable_id in chunk_ids: + logger.debug("chunk %s: pack %s could not be read.", bin_to_hex(unreadable_id), bin_to_hex(pack_id)) + pi.show(increase=unreadable) # this pack is done: drop it, the cache would else keep PACK_READER_CACHE_SIZE whole packs # of which none is read again, and the retries below must really re-read from the store. self.repository.clear_pack_cache() @@ -2684,6 +2700,11 @@ def verify_data(self): ro_type=ROBJ_DONTCARE, assert_id_place="verify_data", ) + except Repository.StoreReadError as err: + # the retry did not fail on the content, it failed to read at all: keep the + # chunk, the read may well succeed again later, refs #3509. + unreadable_chunks += 1 + logger.error("chunk %s not deleted, could not be re-read: %s", bin_to_hex(defect_chunk), err) except IntegrityErrorBase: # failed twice -> remove this defect chunk. delete rewrites its pack without it, # keeping the other chunks, and removes it from self.chunks, so rebuild_archives @@ -2717,18 +2738,23 @@ def verify_data(self): if not sig_int: # the ids of an interrupted pass are incomplete. self.archive_meta_ids = archive_meta_ids + if unreadable_chunks: + logger.error( + "%d chunk(s) could not be read and were left untouched. This usually means a hardware, " + "filesystem or network problem - fix that first, then run the check again.", + unreadable_chunks, + ) log = logger.error if errors else logger.info if sig_int: log( - "Interrupted cryptographic data integrity verification, " - "verified %d of %d chunks with %d integrity errors.", + "Interrupted cryptographic data integrity verification, verified %d of %d chunks with %d error(s).", verified, chunks_count, errors, ) else: log( - "Finished cryptographic data integrity verification, verified %d chunks with %d integrity errors.", + "Finished cryptographic data integrity verification, verified %d chunks with %d error(s).", verified, errors, ) @@ -3167,71 +3193,81 @@ def check_order(info): analyzed += 1 found_before, self.error_found = self.error_found, False try: - formatted = formatter.format_item(info, jsonline=False) - # the formatter uses defaults for keys like {comment} if it has no archive metadata. - metadata_missing = bool(formatter.used_call_keys) and formatter.archive is None - except (Repository.ObjectNotFound, IntegrityErrorBase): - metadata_missing = True - if metadata_missing: - # keys like {comment} need the archive metadata, which is damaged or missing here. - # use the values from the archive directory entry, they are always available. - formatted = f"{info.name} {OutputTimestamp(info.ts)} {archive_id_hex}" - logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})") - if archive_id not in self.chunks: - logger.error(f"Archive metadata block {archive_id_hex} is missing!") - self.error_found = True - if self.repair: - logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") - self.manifest.archives.delete_by_id(archive_id) + try: + formatted = formatter.format_item(info, jsonline=False) + # the formatter uses defaults for keys like {comment} if it has no archive metadata. + metadata_missing = bool(formatter.used_call_keys) and formatter.archive is None + except (Repository.ObjectNotFound, IntegrityErrorBase): + metadata_missing = True + if metadata_missing: + # keys like {comment} need the archive metadata, which is damaged or missing here. + # use the values from the archive directory entry, they are always available. + formatted = f"{info.name} {OutputTimestamp(info.ts)} {archive_id_hex}" + logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})") + if archive_id not in self.chunks: + logger.error(f"Archive metadata block {archive_id_hex} is missing!") + self.error_found = True + if self.repair: + logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") + self.manifest.archives.delete_by_id(archive_id) + else: + logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") + record_result(archive_id, found_before) + continue + cdata = self.repository.get(archive_id) + try: + _, data = self.repo_objs.parse(archive_id, cdata, ro_type=ROBJ_ARCHIVE_META) + except IntegrityErrorBase as integrity_error: + logger.error(f"Archive metadata block {archive_id_hex} is corrupted: {integrity_error}") + self.error_found = True + if self.repair: + logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") + self.manifest.archives.delete_by_id(archive_id) + else: + logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") + record_result(archive_id, found_before) + continue + archive = self.key.unpack_archive(data) + archive = ArchiveItem(internal_dict=archive) + if archive.version != 2: + raise Exception("Unknown archive metadata version") + items_buffer = ChunkBuffer(self.key) + items_buffer.write_chunk = add_callback + complete = False + for item in robust_iterator(archive): + if sig_int and not self.repair: + # without --repair the archive is only read, so the check also stops within it. + break + if "chunks" in item: + verify_file_chunks(info.name, item) + items_buffer.add(item) else: - logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") - record_result(archive_id, found_before) - continue - cdata = self.repository.get(archive_id) - try: - _, data = self.repo_objs.parse(archive_id, cdata, ro_type=ROBJ_ARCHIVE_META) - except IntegrityErrorBase as integrity_error: - logger.error(f"Archive metadata block {archive_id_hex} is corrupted: {integrity_error}") + complete = True + items_buffer.flush(flush=True) + if self.repair: + archive.item_ptrs = archive_put_items( + items_buffer.chunks, repo_objs=self.repo_objs, add_reference=add_reference + ) + data = self.key.pack_metadata(archive.as_dict()) + new_archive_id = self.key.id_hash(data) + logger.debug(f"archive id old: {bin_to_hex(archive_id)}") + logger.debug(f"archive id new: {bin_to_hex(new_archive_id)}") + cdata = self.repo_objs.format(new_archive_id, {}, data, ro_type=ROBJ_ARCHIVE_META) + add_reference(new_archive_id, len(data), cdata) + self.create_archive_entry(info.name, new_archive_id, info.ts) + if archive_id != new_archive_id: + self.manifest.archives.delete_by_id(archive_id) + archive_id = new_archive_id + record_result(archive_id, found_before, complete=complete) + except Repository.StoreReadError as err: + # some object of this archive could not be read at all (I/O error, refs #3509). + # we do not know what is in it, so we can not tell whether the archive is fine. no + # result is recorded for it, so a later check verifies it again. + logger.error(f"Archive {info.name} {archive_id_hex} could not be checked: {err}") self.error_found = True if self.repair: - logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") - self.manifest.archives.delete_by_id(archive_id) - else: - logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") - record_result(archive_id, found_before) - continue - archive = self.key.unpack_archive(data) - archive = ArchiveItem(internal_dict=archive) - if archive.version != 2: - raise Exception("Unknown archive metadata version") - items_buffer = ChunkBuffer(self.key) - items_buffer.write_chunk = add_callback - complete = False - for item in robust_iterator(archive): - if sig_int and not self.repair: - # without --repair the archive is only read, so the check also stops within it. - break - if "chunks" in item: - verify_file_chunks(info.name, item) - items_buffer.add(item) - else: - complete = True - items_buffer.flush(flush=True) - if self.repair: - archive.item_ptrs = archive_put_items( - items_buffer.chunks, repo_objs=self.repo_objs, add_reference=add_reference - ) - data = self.key.pack_metadata(archive.as_dict()) - new_archive_id = self.key.id_hash(data) - logger.debug(f"archive id old: {bin_to_hex(archive_id)}") - logger.debug(f"archive id new: {bin_to_hex(new_archive_id)}") - cdata = self.repo_objs.format(new_archive_id, {}, data, ro_type=ROBJ_ARCHIVE_META) - add_reference(new_archive_id, len(data), cdata) - self.create_archive_entry(info.name, new_archive_id, info.ts) - if archive_id != new_archive_id: - self.manifest.archives.delete_by_id(archive_id) - archive_id = new_archive_id - record_result(archive_id, found_before, complete=complete) + # rewriting the archive now would drop whatever we could not read from it. + raise finally: pi.finish() report_missing_chunks() diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index f297e7aeed..a7e2c88426 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -300,6 +300,20 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): packs the repair wrote, makes the chunk index match them and stores it; that always runs to completion, also after a Ctrl-C. + Unreadable repository objects + +++++++++++++++++++++++++++++ + + A repository object that cannot be read at all (an I/O error from a failing disk, a + flaky network filesystem, ...) is different from a corrupt one: Borg never saw its + content, so it cannot tell whether the object is fine. Such an object is reported and + the check continues, so one run lists everything that is affected; the check then + fails. The object is not remembered as corrupt, so a later check verifies it again. + + Because the data may well be readable again once the underlying problem is fixed, + ``--repair`` refuses to repair while there are unreadable objects, and + ``--verify-data`` leaves chunks it could not read in place instead of deleting them. + Fix the storage hardware, filesystem or network first, then run the check again. + About repair mode +++++++++++++++++ diff --git a/src/borg/cache.py b/src/borg/cache.py index fafee9c110..2885cb924a 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -907,8 +907,9 @@ def build_chunkindex_from_repo( # read completely, and never write to the repo. # write_immediately: store the index (all of it, deleting all other fragments) before returning it. The # callers hold an exclusive lock (borg compact, borg repo-compress, borg check --repair). - # a corrupt fragment raises CorruptChunkIndexFragment: a corrupt index aborts the command. Only with - # write_immediately (the index is rewritten anyway), the index is rebuilt from the packs instead. + # a corrupt fragment raises CorruptChunkIndexFragment, an unreadable one Repository.StoreReadError: an + # unusable index aborts the command. Only with write_immediately (the index is rewritten anyway), the + # index is rebuilt from the packs instead. # validate: a repo object validator or None, passed to PackReader.iter_headers. With a validator, # the rebuild skips the objects that fail it; without one, a corrupt object header raises CorruptPack. # on_drop: a callable or None, passed to PackReader.iter_headers, called once per byte range the @@ -949,7 +950,7 @@ def build_chunkindex_from_repo( break chunks = ChunkIndex() # we'll merge all fragments into this complete = True - corrupt_fragment = None + unusable_fragment = None # the error of a fragment that is corrupt or could not be read msgid = "cache.merge_chunkindex_fragments" show_percent = progress_wanted() if show_percent: @@ -961,8 +962,9 @@ def build_chunkindex_from_repo( pi.show() try: chunks_to_merge = read_chunkindex_from_repo(repository, hash) - except CorruptChunkIndexFragment as err: - corrupt_fragment = err + except (CorruptChunkIndexFragment, Repository.StoreReadError) as err: + # a corrupt fragment, or one that could not be read (I/O error, refs #3509). + unusable_fragment = err break if chunks_to_merge is None: logger.debug(f"chunk index fragment {hash} vanished, restarting the merge...") @@ -977,13 +979,17 @@ def build_chunkindex_from_repo( pi.show(current=len(hashes)) # finish at 100% finally: pi.finish() - if corrupt_fragment is not None: - # retrying would re-read the same corrupt fragment. abort, unless the index gets rewritten + if unusable_fragment is not None: + # retrying would re-read the same unusable fragment. abort, unless the index gets rewritten # anyway: then rebuild the whole index from the packs. chunks.clear() if not write_immediately: - raise corrupt_fragment - logger.warning(f"{corrupt_fragment.args[0]} is corrupt, rebuilding the chunk index from the packs.") + raise unusable_fragment + if isinstance(unusable_fragment, CorruptChunkIndexFragment): + problem = f"{unusable_fragment.args[0]} is corrupt" + else: + problem = f"chunk index fragment {hash} could not be read: {unusable_fragment}" + logger.warning(f"{problem}, rebuilding the chunk index from the packs.") break if complete: if len(hashes) > 1 and write_immediately: diff --git a/src/borg/repository.py b/src/borg/repository.py index 4ceb987c70..8eb13a9a87 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -434,7 +434,13 @@ def read(self, offset, size): # in-memory pack: return a memoryview into pack_contents. store: range-read bytes. if self.pack_contents is not None: return memoryview(self.pack_contents)[offset : offset + size] - return self.store.load(self.key, offset=offset, size=size) + try: + return self.store.load(self.key, offset=offset, size=size) + except OSError as exc: + # the pack exists, but reading it failed (bad disk, flaky network fs, ...). that is + # neither "object missing" nor "content corrupt", so it gets its own error class: + # callers must not react to it by dropping data, refs #3509. + raise Repository.StoreReadError(f"pack {bin_to_hex(self.pack_id)}", exc) from exc def size(self): """Return the pack size in bytes (a store metadata lookup, unless the pack is in memory or pack_size is set).""" @@ -442,7 +448,10 @@ def size(self): return len(self.pack_contents) if self.pack_size is not None: return self.pack_size - return self.store.info(self.key).size + try: + return self.store.info(self.key).size + except OSError as exc: + raise Repository.StoreReadError(f"pack {bin_to_hex(self.pack_id)}", exc) from exc @staticmethod def _parse_header(hdr_data, offset, pack_size): @@ -1003,6 +1012,18 @@ class DefaultsMissing(Error): exit_mcode = 34 + class RepairUnsafe(Error): + """Not repairing: {} repository object(s) could not be read. Fix the underlying problem, then check again.""" + + exit_mcode = 36 + + class StoreReadError(Error): + """Error reading {} from the repository: {}. Check the storage hardware / filesystem.""" + + exit_mcode = 35 + + # the underlying I/O error (bad disk, flaky network fs, ...) is kept as __cause__, refs #3509. + # Whole packs kept in memory for reads; the least recently used is evicted first. # Memory use is this count times the pack size. PACK_READER_CACHE_SIZE = 3 @@ -1671,12 +1692,19 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=None, validat assert repo_only is not None or not repair def verify(namespace, name): + """Return True if the object is intact, False if it is corrupt, None if it could not be read.""" # name is the store hash of the object's content, so it is intact iff store.hash() matches. key = f"{namespace}/{name}" try: ok = self.store.hash(key, algorithm=STORE_HASH_NAME) == name except StoreObjectNotFound: return True # vanished since store.list(); not an error + except OSError as exc: + # reading failed (bad disk, flaky network fs, ...), so we do not know whether the + # object is intact. report it and go on: one unreadable object must not abort the + # whole check, and the result must not be recorded as "corrupt", refs #3509. + logger.error(f"Store object {key} could not be read: {exc}") + return None if not ok: logger.error(f"Store object {key} is corrupted: content does not match its name (store hash).") return ok @@ -1686,6 +1714,10 @@ def store_list(namespace): return list(self.store.list(namespace)) except StoreObjectNotFound: return [] # namespace does not exist + except OSError as exc: + # without a listing we do not know what to check, so this one is fatal - but it + # ends the check with a clear message instead of a traceback, refs #3509. + raise self.StoreReadError(f"the {namespace}/ listing", exc) from exc partial = bool(max_duration) assert not (repair and partial) @@ -1710,6 +1742,9 @@ def store_list(namespace): salvaged = salvage_lossy = 0 # packs salvaged, and those of them that may have lost chunks reread_intact = 0 # packs that failed the pack check and read intact at the salvage chunks = None # the index read from its fragments; None if it was not read + # objects that could not be read at all (I/O errors, refs #3509) - counted apart from + # corruption: "unreadable" is a different diagnosis and is often transient/fixable. + read_errors = 0 missing_pack_ids = [] # packs referenced by the index but absent from packs/ (refs #9898) stale_entries = 0 # number of index entries removed for missing_pack_ids index_repaired = False @@ -1732,7 +1767,10 @@ def store_list(namespace): self._lock_refresh() index_pi.show(increase=1) index_files += 1 - if not verify("index", info.name): + result = verify("index", info.name) + if result is None: + read_errors += 1 + elif not result: index_errors += 1 if index_infos: index_pi.show(current=len(index_infos)) # finish at 100% @@ -1773,6 +1811,9 @@ def store_list(namespace): ) index_errors += 1 chunks = None + except self.StoreReadError: + # a fragment could not be read (I/O error, refs #3509), the index check above reports it. + chunks = None if chunks is None: logger.warning( "Cannot cross-check packs against the chunk index: the index could not be loaded " @@ -1847,9 +1888,14 @@ def recorded_ts(info): continue pack_files += 1 ok = verify("packs", info.name) - if not ok: - pack_errors += 1 - tracker.record(pack_id, ok) + if ok is None: + # unreadable: we did not learn anything about this pack, so do not record a + # result for it - a later check re-verifies it (unrecorded packs come first). + read_errors += 1 + else: + if not ok: + pack_errors += 1 + tracker.record(pack_id, ok) now = time.monotonic() # a checkpoint rewrites the whole table (41 bytes per pack), so keep the interval long. if now > t_last_checkpoint + 30 * 60: @@ -1866,7 +1912,9 @@ def recorded_ts(info): pack_pi.finish() # salvage before the index rebuild below, which rebuilds only if no pack is left corrupt. try: - if repair and not sig_int and pack_files + pack_skipped == len(pack_infos): + # not with read errors: repairing around unreadable packs could throw away data that reads fine + # again once the underlying problem is fixed, refs #3509 (the repair is refused below). + if repair and not sig_int and read_errors == 0 and pack_files + pack_skipped == len(pack_infos): # a pack skipped by max_age has an intact record, so it is not salvaged anyway and # counts as scanned here; only an interrupted or time-boxed loop must block a salvage. salvaged, salvage_lossy, reread_intact = self._salvage_corrupt_packs( @@ -1879,12 +1927,15 @@ def recorded_ts(info): # this run: sig_int breaks the loop early, so "no corrupt pack" must be paired with "all packs # scanned" (pack_files == len(pack_infos)) to not rebuild from unverified packs. max_age was # set to 0 after the index cross-check for exactly this path, so no pack was skipped. + # read_errors == 0 for the same reason: an unreadable pack was not verified either, and + # rebuilding the index from packs we could not read would drop their chunks, refs #3509. if ( repair and repo_only and index_errors and invalid_pack_names == 0 and not tracker.corrupt_ids() + and read_errors == 0 and not sig_int and pack_files == len(pack_infos) ): @@ -1927,6 +1978,13 @@ def note_drop(): if pack_skipped: summary += f" Reused {pack_skipped} recent pack check result(s)." logger.info(summary) + if read_errors: + logger.error( + f"{read_errors} store object(s) could not be read, so they could not be checked. " + "This usually means a hardware, filesystem or network problem - see the " + '"Data integrity" section of the docs. The repository was not modified; fix the ' + "underlying problem, then run the check again." + ) if missing_pack_ids: # one id per line (the list can be long). logger.error(f"{len(missing_pack_ids)} pack(s) referenced by the index are missing:") @@ -1951,14 +2009,17 @@ def note_drop(): logger.error(f"Found {len(corrupt_ids)} corrupt pack(s):") for pack_id in corrupt_ids: logger.error(f"Corrupt pack: {bin_to_hex(pack_id)}") - # fail if this run found errors, or any pack is recorded corrupt. - problems = objs_errors != 0 or bool(corrupt_ids) + # fail if this run found errors or could not read some objects, or any pack is recorded corrupt. + problems = objs_errors != 0 or read_errors != 0 or bool(corrupt_ids) # On Ctrl-C the check stopped early, so the summary only covers the packs seen so far. done, so_far = ("Interrupted", " so far") if sig_int else ("Finished", "") if not problems: logger.info(f"{done} {mode} repository check, no problems found{so_far}.") elif not repair: logger.error(f"{done} {mode} repository check, errors found{so_far}.") + elif read_errors: + # repair mode, but unreadable objects stop us below - so report like a read-only check. + logger.error(f"{done} {mode} repository check, errors found{so_far}.") elif invalid_pack_names or corrupt_ids: # corrupt packs not salvaged, e.g. packs in which no object authenticates. if repo_only: @@ -2017,6 +2078,10 @@ def note_drop(): if repair: if index_errors and not index_repaired and not index_deferred: return False + if read_errors: + # we do not know what is in those objects, so repairing around them could throw away + # data that reads fine again once the underlying problem is fixed, refs #3509. + raise self.RepairUnsafe(read_errors) if reread_intact: return False return not (repo_only and (invalid_pack_names or corrupt_ids or missing_pack_ids or drops or salvage_lossy)) @@ -2117,7 +2182,11 @@ def _cached_pack_reader(self, pack_id): reader = self._pack_cache.get(pack_id) if reader is None: key = "packs/" + bin_to_hex(pack_id) - reader = PackReader(pack_id=pack_id, pack_contents=self.store.load(key)) + try: + pack_contents = self.store.load(key) + except OSError as exc: + raise self.StoreReadError(f"pack {bin_to_hex(pack_id)}", exc) from exc + reader = PackReader(pack_id=pack_id, pack_contents=pack_contents) self._pack_cache[pack_id] = reader return reader @@ -2142,6 +2211,7 @@ def get_many(self, ids, read_data=True, raise_missing=True): raise self.PackNotFound(id_, entry.pack_id, str(self._location)) from None yield None else: + # the pack is in memory here, so read() only slices it and can not raise OSError. yield reader.read(entry.obj_offset, entry.obj_size) def gather_many(self, ids, raise_missing=True): @@ -2189,6 +2259,10 @@ def _gather_batch(self, ids, raise_missing): if sources: try: data = self.store.gather([source for _, source in sources], namespace="packs") + except OSError as exc: + # reading failed (bad disk, flaky network fs, ...): neither missing nor corrupt, refs #3509. + packs = {pack for _, (pack, _, _) in sources} + raise self.StoreReadError(f"{len(sources)} objects from {len(packs)} pack(s)", exc) from exc except (StoreObjectNotFound, ReadRangeError): # a missing or truncated pack: read the objects one by one, so the ones in intact packs are # still returned and the others get get()'s error handling. @@ -2880,10 +2954,15 @@ def store_list(self, name, *, deleted=False): return list(self.store.list(name, deleted=deleted)) except StoreObjectNotFound: return [] + except OSError as exc: + raise self.StoreReadError(f"the {name}/ listing", exc) from exc def store_load(self, name, *, size=None, offset=0): self._lock_refresh() - return self.store.load(name, size=size, offset=offset) + try: + return self.store.load(name, size=size, offset=offset) + except OSError as exc: + raise self.StoreReadError(name, exc) from exc def store_store(self, name, value): self._lock_refresh() diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index c2d6b50147..3d31a09f4b 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -1,3 +1,4 @@ +import errno import gc import os from pathlib import Path @@ -2029,9 +2030,7 @@ def test_verify_data_reports_a_missing_pack(archivers, request, monkeypatch): assert len(logged) == 2 assert logged[0] == f"pack {bin_to_hex(gone)} is missing, {len(gone_chunks)} chunks are lost." # the lost chunks count as verified and as errors, as if each had been read and failed. - assert logged[-1].endswith( - f"verified {len(repository.chunks)} chunks with {len(gone_chunks)} integrity errors." - ) + assert logged[-1].endswith(f"verified {len(repository.chunks)} chunks with {len(gone_chunks)} error(s).") # each pack was loaded once, the missing one included, and the scan continued past it. packs = {entry.pack_id for _, entry in repository.chunks.iteritems()} assert sorted(loaded) == sorted("packs/" + bin_to_hex(pack_id) for pack_id in packs) @@ -2730,3 +2729,116 @@ def test_check_max_duration_used_up_by_the_repository_check(archiver, monkeypatc assert "Skipping the archive check: the repository check used up --max-duration." in output assert "Starting archive consistency check" not in output assert archive_records(archiver) == {} + + +def make_pack_unreadable(monkeypatch, pack_name): + """Make reading the pack packs/ fail with an OSError, like failing storage does. + + Patches the posixfs backend rather than using file permissions, so it also works when the tests + run as root and does not depend on the platform's permission semantics. Returns a dict whose + "failing" entry switches the failures off again (monkeypatch.undo() must not be used here, it + would also revert the autouse clean_env fixture). + """ + from borgstore.backends.posixfs import PosixFS + + state = {"failing": True} + orig_hash, orig_load = PosixFS.hash, PosixFS.load + + def hits_pack(name): + # the backend gets the name including borgstore's nesting levels, e.g. packs/d0/d0a6... + return state["failing"] and name.rsplit("/", 1)[-1] == pack_name + + def failing_hash(self, name, algorithm="sha256"): + if hits_pack(name): + raise OSError(errno.EIO, "Input/output error", name) + return orig_hash(self, name, algorithm=algorithm) + + def failing_load(self, name, *, size=None, offset=0): + if hits_pack(name): + raise OSError(errno.EIO, "Input/output error", name) + return orig_load(self, name, size=size, offset=offset) + + monkeypatch.setattr(PosixFS, "hash", failing_hash) + monkeypatch.setattr(PosixFS, "load", failing_load) + return state + + +def some_pack_name(archiver): + """Return the name of one of the repository's pack files.""" + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + return sorted(info.name for info in repository.store_list("packs"))[0] + + +def test_check_unreadable_pack(archivers, request, monkeypatch): + # an I/O error while reading a pack must not crash the check with a traceback: it is reported, + # the check goes on and fails at the end, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + cmd(archiver, "check", exit_code=0) + pack_name = some_pack_name(archiver) + make_pack_unreadable(monkeypatch, pack_name) + + output = cmd(archiver, "check", "-v", "--repository-only", exit_code=1) + assert f"Store object packs/{pack_name} could not be read" in output + assert "Input/output error" in output + # the check did not stop at the unreadable pack ... + assert "Finished checking packs." in output + assert "store object(s) could not be read" in output + # ... and it did not claim the pack is corrupt (we never saw its content). + assert "is corrupted" not in output + assert "Corrupt pack" not in output + + +def test_check_unreadable_pack_not_recorded(archivers, request, monkeypatch): + # a pack we could not read gets no result recorded, so a later check verifies it again instead + # of remembering it as corrupt, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + pack_name = some_pack_name(archiver) + state = make_pack_unreadable(monkeypatch, pack_name) + cmd(archiver, "check", "--repository-only", exit_code=1) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + tracker = PackTracker.load(repository) + assert tracker.get(hex_to_bin(pack_name)) is None + assert tracker.corrupt_ids() == [] + # once the pack reads fine again, the check passes without any manual cleanup. + state["failing"] = False + cmd(archiver, "check", exit_code=0) + + +def test_check_repair_refuses_unreadable_pack(archivers, request, monkeypatch): + # --repair must not repair around an unreadable pack: its chunks may well be readable again + # once the underlying problem is fixed, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + pack_name = some_pack_name(archiver) + make_pack_unreadable(monkeypatch, pack_name) + with pytest.raises(Repository.RepairUnsafe): # local (not forked): the Error propagates + cmd(archiver, "check", "--repair") + + +def test_check_verify_data_unreadable_pack_keeps_chunks(archivers, request, monkeypatch): + # --verify-data deletes chunks whose content is defect, but must keep chunks it could not read + # at all, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + pack_name = some_pack_name(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + chunks_before = sorted(chunk_id for chunk_id, _ in repository.chunks.iteritems()) + state = make_pack_unreadable(monkeypatch, pack_name) + + output = cmd(archiver, "check", "--archives-only", "--verify-data", exit_code=1) + assert "could not be read and were left untouched" in output + + state["failing"] = False + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + chunks_after = sorted(chunk_id for chunk_id, _ in repository.chunks.iteritems()) + assert chunks_after == chunks_before # nothing was thrown away From ce8c1d755cafb521e76dc265fd1565df00869c69 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 24 Aug 2026 19:07:35 +0200 Subject: [PATCH 2/4] check: survive an unreadable archive metadata pack, refs #3509 Listing the archives reads each archive's metadata chunk, so an unreadable pack holding one made the archives check die before its per-archive loop even started. The FreeBSD CI job hit this because the pack it picked happened to hold archive metadata. Archives.list() gets an opt-in tolerate_read_errors: with it, an archive whose metadata could not be read becomes a placeholder entry, so the check can report it and go on with the other archives. Only the archives check opts in. Everywhere else the read error propagates, because acting on a partially readable repository is how a transient I/O error turns into data loss: prune would see the placeholder's 1970 timestamp and treat the archive as the oldest one. The --find-lost-archives scan now also skips an unreadable chunk instead of aborting; it only looks for archive metadata there, so skipping one can at worst miss a lost archive. The verify-data test now targets the pack holding file content, so it tests what it means to test regardless of how chunks land in packs. --- docs/changes.rst | 3 +- docs/usage/check.rst.inc | 4 ++ src/borg/archive.py | 23 ++++++-- src/borg/archiver/check_cmd.py | 4 ++ src/borg/manifest.py | 53 ++++++++++++++---- src/borg/testsuite/archiver/check_cmd_test.py | 54 ++++++++++++++++++- src/borg/testsuite/archives_test.py | 19 ++++--- 7 files changed, 137 insertions(+), 23 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index e1f5b5d18a..f2684b737b 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -202,7 +202,8 @@ Fixes: is reported, the check continues and fails at the end. Such an object is not recorded as corrupt (a later check verifies it again), ``--repair`` refuses to repair around it and ``--verify-data`` no longer deletes chunks it could not - read, #3509 + read. Other commands stop with the read error instead of working with a + partially readable repository, #3509 - extract --continue: skip complete dirs, count skipped files in --progress, #6857 - create: back up automounted directories, #6652 - security: same local repo dir via another path is not a relocation, #8188 diff --git a/docs/usage/check.rst.inc b/docs/usage/check.rst.inc index aaca31cab0..4a03085aa8 100644 --- a/docs/usage/check.rst.inc +++ b/docs/usage/check.rst.inc @@ -224,6 +224,10 @@ Because the data may well be readable again once the underlying problem is fixed ``--verify-data`` leaves chunks it could not read in place instead of deleting them. Fix the storage hardware, filesystem or network first, then run the check again. +Only ``borg check`` goes on like this. Other commands stop with the read error rather +than work with a partially readable repository - e.g. ``borg prune`` would otherwise +see an archive whose metadata it could not read as undated and thus as the oldest one. + About repair mode +++++++++++++++++ diff --git a/src/borg/archive.py b/src/borg/archive.py index af06bf927e..23d884f15b 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2844,13 +2844,19 @@ def check_archive_meta(chunk_id): """Load the archive metadata object chunk_id. If it has no archives directory entry, create one (with --repair) or log that it would create one. """ - cdata = self.repository.get(chunk_id) try: + cdata = self.repository.get(chunk_id) meta, data = self.repo_objs.parse(chunk_id, cdata, ro_type=ROBJ_DONTCARE) except IntegrityErrorBase as exc: logger.error("Skipping corrupted chunk: %s", exc) self.error_found = True return + except Repository.StoreReadError as exc: + # unreadable, not corrupt: this scan only looks for archive metadata, so skipping + # such a chunk can at worst miss a lost archive - it never drops data, refs #3509. + logger.error("Skipping unreadable chunk: %s", exc) + self.error_found = True + return if meta["type"] != ROBJ_ARCHIVE_META: return # should never happen try: @@ -2905,6 +2911,11 @@ def check_archive_meta(chunk_id): logger.error("Skipping corrupted chunk: %s", exc) self.error_found = True continue + except Repository.StoreReadError as exc: + # see check_archive_meta. + logger.error("Skipping unreadable chunk: %s", exc) + self.error_found = True + continue if meta["type"] != ROBJ_ARCHIVE_META: continue check_archive_meta(chunk_id) @@ -3149,6 +3160,9 @@ def robust_item_ids(): newest=newest, older=older, newer=newer, + # an archive whose metadata we can not read gets a placeholder entry here, so the + # other archives still get checked; the loop below reports it, refs #3509. + tolerate_read_errors=True, ) if match and not archive_infos: logger.warning("--match-archives %s does not match any archives", match) @@ -3157,7 +3171,7 @@ def robust_item_ids(): if last and len(archive_infos) < last: logger.warning("--last %d archives: only found %d archives", last, len(archive_infos)) else: - archive_infos = self.manifest.archives.list(sort_by=sort_by) + archive_infos = self.manifest.archives.list(sort_by=sort_by, tolerate_read_errors=True) num_archives = len(archive_infos) if deadline is not None: # archives without an ok record first, then the least recently checked ones, so that @@ -3197,10 +3211,11 @@ def check_order(info): formatted = formatter.format_item(info, jsonline=False) # the formatter uses defaults for keys like {comment} if it has no archive metadata. metadata_missing = bool(formatter.used_call_keys) and formatter.archive is None - except (Repository.ObjectNotFound, IntegrityErrorBase): + except (Repository.ObjectNotFound, IntegrityErrorBase, Repository.StoreReadError): metadata_missing = True if metadata_missing: - # keys like {comment} need the archive metadata, which is damaged or missing here. + # keys like {comment} need the archive metadata, which is damaged, missing or + # unreadable here. # use the values from the archive directory entry, they are always available. formatted = f"{info.name} {OutputTimestamp(info.ts)} {archive_id_hex}" logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})") diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index a7e2c88426..2af1c9d363 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -314,6 +314,10 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): ``--verify-data`` leaves chunks it could not read in place instead of deleting them. Fix the storage hardware, filesystem or network first, then run the check again. + Only ``borg check`` goes on like this. Other commands stop with the read error rather + than work with a partially readable repository - e.g. ``borg prune`` would otherwise + see an archive whose metadata it could not read as undated and thus as the oldest one. + About repair mode +++++++++++++++++ diff --git a/src/borg/manifest.py b/src/borg/manifest.py index 6a9697375c..8d0f36badd 100644 --- a/src/borg/manifest.py +++ b/src/borg/manifest.py @@ -195,12 +195,29 @@ def ids(self, *, deleted=False): info = ItemInfo(*info) # RPC does not give us a NamedTuple yield hex_to_bin(info.name) - def _get_archive_meta(self, id: bytes) -> tuple[dict, ArchiveItem | None]: + def _get_archive_meta(self, id: bytes, *, tolerate_read_errors: bool = False) -> tuple[dict, ArchiveItem | None]: # get all metadata directly from the ArchiveItem in the repo, see _parse_archive_meta. from .repository import Repository try: cdata = self.repository.get(id) + except Repository.StoreReadError: + # the archive metadata could not be read at all (I/O error, refs #3509). only borg check + # opts into a placeholder here, so it can go on and check the other archives. everybody + # else must not act on a repository it can not fully read: the placeholder's 1970 + # timestamp would e.g. make prune treat the archive as the oldest one. + if not tolerate_read_errors: + raise + metadata = dict( + id=id, + name="archive-metadata-could-not-be-read", + time="1970-01-01T00:00:00.000000", + exists=False, # we have the pointer, but we could not read the archive item + username="", + hostname="", + tags=(), + ) + return metadata, None except Repository.ObjectNotFound: cdata = None return self._parse_archive_meta(id, cdata) @@ -253,15 +270,28 @@ def _parse_archive_meta(self, id: bytes, cdata) -> tuple[dict, ArchiveItem | Non ) return metadata, archive_item - def _infos(self, *, deleted=False): + def _infos(self, *, deleted=False, tolerate_read_errors=False): # yield (info dict, ArchiveItem) of all archives, reading their metadata objects in batches # (see Repository.gather_many). - ids = list(self.ids(deleted=deleted)) - for id, cdata in zip(ids, self.repository.gather_many(ids, raise_missing=False)): - yield self._parse_archive_meta(id, cdata) + from .repository import Repository - def _info_tuples(self, *, deleted=False): - for info, archive_item in self._infos(deleted=deleted): + ids = list(self.ids(deleted=deleted)) + done = 0 # the number of ids yielded so far + try: + for id, cdata in zip(ids, self.repository.gather_many(ids, raise_missing=False)): + yield self._parse_archive_meta(id, cdata) + done += 1 + except Repository.StoreReadError: + # an archive metadata object could not be read (I/O error, refs #3509), see _get_archive_meta. + if not tolerate_read_errors: + raise + # a batch fails as a whole, so read the remaining objects one by one: only the unreadable + # ones become placeholders. + for id in ids[done:]: + yield self._get_archive_meta(id, tolerate_read_errors=True) + + def _info_tuples(self, *, deleted=False, tolerate_read_errors=False): + for info, archive_item in self._infos(deleted=deleted, tolerate_read_errors=tolerate_read_errors): yield ArchiveInfo( name=info["name"], id=info["id"], @@ -272,8 +302,8 @@ def _info_tuples(self, *, deleted=False): metadata=archive_item, ) - def _matching_info_tuples(self, match_patterns, match_end, *, deleted=False): - archive_infos = list(self._info_tuples(deleted=deleted)) + def _matching_info_tuples(self, match_patterns, match_end, *, deleted=False, tolerate_read_errors=False): + archive_infos = list(self._info_tuples(deleted=deleted, tolerate_read_errors=tolerate_read_errors)) if match_patterns: assert isinstance(match_patterns, list), f"match_pattern is a {type(match_patterns)}" for match in match_patterns: @@ -433,6 +463,7 @@ def list( oldest=None, newest=None, deleted=False, + tolerate_read_errors=False, ): """ Return list of ArchiveInfo instances according to the parameters. @@ -454,7 +485,9 @@ def list( if isinstance(sort_by, (str, bytes)): raise TypeError("sort_by must be a sequence of str") - archive_infos = self._matching_info_tuples(match, match_end, deleted=deleted) + archive_infos = self._matching_info_tuples( + match, match_end, deleted=deleted, tolerate_read_errors=tolerate_read_errors + ) if any([oldest, newest, older, newer]): archive_infos = filter_archives_by_date( diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 3d31a09f4b..52c4c848e1 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -2769,6 +2769,22 @@ def some_pack_name(archiver): return sorted(info.name for info in repository.store_list("packs"))[0] +def pack_name_of(archiver, chunk_id): + """Return the name of the pack file holding chunk_id.""" + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + return bin_to_hex(repository.chunks[chunk_id].pack_id) + + +def file_content_chunk_id(archiver, archive_name="archive1"): + """Return the id of a file content chunk of the given archive.""" + archive, repository = open_archive(archiver.repository_path, archive_name) + with repository: + for item in archive.iter_items(): + if item.path.endswith(src_file): + return item.chunks[-1].id + raise AssertionError(f"{src_file} not found in {archive_name}") + + def test_check_unreadable_pack(archivers, request, monkeypatch): # an I/O error while reading a pack must not crash the check with a traceback: it is reported, # the check goes on and fails at the end, refs #3509. @@ -2830,7 +2846,8 @@ def test_check_verify_data_unreadable_pack_keeps_chunks(archivers, request, monk if archiver.get_kind() != "local": pytest.skip("only works locally, patches objects") check_cmd_setup(archiver) - pack_name = some_pack_name(archiver) + # target the pack holding file content, so --verify-data is what reads it. + pack_name = pack_name_of(archiver, file_content_chunk_id(archiver)) with KeyedRepository(archiver.repository_location, exclusive=True) as repository: chunks_before = sorted(chunk_id for chunk_id, _ in repository.chunks.iteritems()) state = make_pack_unreadable(monkeypatch, pack_name) @@ -2842,3 +2859,38 @@ def test_check_verify_data_unreadable_pack_keeps_chunks(archivers, request, monk with KeyedRepository(archiver.repository_location, exclusive=True) as repository: chunks_after = sorted(chunk_id for chunk_id, _ in repository.chunks.iteritems()) assert chunks_after == chunks_before # nothing was thrown away + + +def test_check_unreadable_archive_metadata_pack(archivers, request, monkeypatch): + # the pack holding an archive's metadata is unreadable: listing the archives must not die, the + # affected archive is reported and the other archives still get checked, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + archive_id = archive.id + pack_name = pack_name_of(archiver, archive_id) + make_pack_unreadable(monkeypatch, pack_name) + + output = cmd(archiver, "check", "--archives-only", exit_code=1) + assert "could not be checked" in output + assert "Input/output error" in output + assert "Archive consistency check complete, problems found." in output + + +def test_unreadable_archive_metadata_pack_does_not_fake_an_archive(archivers, request, monkeypatch): + # outside of borg check, an unreadable archive metadata object must not turn into a placeholder + # entry: acting on a repository we can not fully read (e.g. prune, which would see the + # placeholder's 1970 timestamp) is how transient I/O errors become data loss, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + archive_id = archive.id + make_pack_unreadable(monkeypatch, pack_name_of(archiver, archive_id)) + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "repo-list") diff --git a/src/borg/testsuite/archives_test.py b/src/borg/testsuite/archives_test.py index 563aea50a3..7f4c2dc70f 100644 --- a/src/borg/testsuite/archives_test.py +++ b/src/borg/testsuite/archives_test.py @@ -54,13 +54,15 @@ def _archiveinfo(name, id_, ts=TS, *, username="", hostname="", tags=()): def _stub_matching_info_tuples(infos): ar, _, _ = _archives() - ar._matching_info_tuples = Mock(side_effect=lambda match_patterns, match_end, deleted=False: list(infos)) + ar._matching_info_tuples = Mock( + side_effect=lambda match_patterns, match_end, deleted=False, tolerate_read_errors=False: list(infos) + ) return ar def _stub_info_tuples(infos): ar, _, _ = _archives() - ar._info_tuples = Mock(side_effect=lambda deleted=False: iter(infos)) + ar._info_tuples = Mock(side_effect=lambda deleted=False, tolerate_read_errors=False: iter(infos)) return ar @@ -385,7 +387,9 @@ def test_archive_infos_carry_metadata(): # the ArchiveInfos made by list(), get() and get_by_id() carry the parsed ArchiveItem. archive_item = object() # stands in for the ArchiveItem ar, repo, _ = _archives() - ar._infos = Mock(side_effect=lambda deleted=False: iter([(_archive_meta("a", _id(1)), archive_item)])) + ar._infos = Mock( + side_effect=lambda deleted=False, tolerate_read_errors=False: iter([(_archive_meta("a", _id(1)), archive_item)]) + ) ar._get_archive_meta = Mock(side_effect=lambda id_: (_archive_meta("a", _id(1)), archive_item)) repo.store_list.return_value = [_iteminfo(_id(1))] assert [info.metadata for info in ar.list()] == [archive_item] @@ -501,9 +505,9 @@ def test_list_date_filter(): def test_list_deleted_passes_flag(): ar, _, _ = _archives() - ar._info_tuples = Mock(side_effect=lambda deleted=False: iter([])) + ar._info_tuples = Mock(side_effect=lambda deleted=False, tolerate_read_errors=False: iter([])) ar.list(deleted=True) - ar._info_tuples.assert_called_once_with(deleted=True) + ar._info_tuples.assert_called_once_with(deleted=True, tolerate_read_errors=False) def test_list_match_name(): @@ -600,9 +604,10 @@ def test_get_one_multiple_matches_raises(): def test_get_one_deleted_passes_flag(): i1 = _archiveinfo("a", _id(1)) ar, _, _ = _archives() - ar._info_tuples = Mock(side_effect=lambda deleted=False: iter([i1])) + ar._info_tuples = Mock(side_effect=lambda deleted=False, tolerate_read_errors=False: iter([i1])) ar.get_one(["a"], deleted=True) - ar._info_tuples.assert_called_once_with(deleted=True) + # get_one never opts into tolerance: it must not return a placeholder for an unreadable archive. + ar._info_tuples.assert_called_once_with(deleted=True, tolerate_read_errors=False) def test_list_considering_raises_if_name_set(): From 76fdcb0a0f47fbb6b5dccc0baa6c7d7e0a2b9fd2 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 24 Aug 2026 19:59:15 +0200 Subject: [PATCH 3/4] check: narrow the archive read-error handling, and test it, refs #3509 Instead of one try around the whole per-archive body, catch the read error at the two places that can raise it: reading the archive metadata block, and reading the item metadata stream. Both are precise about what failed, neither can swallow a read error from somewhere unexpected, and the body stays where it was, so the diff is readable. Also add tests for the branches the first two commits left untested: an unreadable namespace listing, an unreadable index object, --repair stopping at an unreadable pack while rebuilding the index, --repair stopping at unreadable archive metadata (with the archive left intact), --find-lost-archives skipping an unreadable chunk, and the one that matters most - a defect chunk whose re-read fails is kept rather than deleted, which is how a flaky disk would otherwise talk borg into throwing data away. The failure injection is now one helper taking a per-read predicate and an "after N reads" counter, so a test can fail one object, one range, one namespace, or only the second read of something. --- src/borg/archive.py | 132 ++++++----- src/borg/testsuite/archiver/check_cmd_test.py | 214 ++++++++++++++++-- 2 files changed, 268 insertions(+), 78 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 23d884f15b..f7f360f09b 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -3207,48 +3207,57 @@ def check_order(info): analyzed += 1 found_before, self.error_found = self.error_found, False try: - try: - formatted = formatter.format_item(info, jsonline=False) - # the formatter uses defaults for keys like {comment} if it has no archive metadata. - metadata_missing = bool(formatter.used_call_keys) and formatter.archive is None - except (Repository.ObjectNotFound, IntegrityErrorBase, Repository.StoreReadError): - metadata_missing = True - if metadata_missing: - # keys like {comment} need the archive metadata, which is damaged, missing or - # unreadable here. - # use the values from the archive directory entry, they are always available. - formatted = f"{info.name} {OutputTimestamp(info.ts)} {archive_id_hex}" - logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})") - if archive_id not in self.chunks: - logger.error(f"Archive metadata block {archive_id_hex} is missing!") - self.error_found = True - if self.repair: - logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") - self.manifest.archives.delete_by_id(archive_id) - else: - logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") - record_result(archive_id, found_before) - continue + formatted = formatter.format_item(info, jsonline=False) + # the formatter uses defaults for keys like {comment} if it has no archive metadata. + metadata_missing = bool(formatter.used_call_keys) and formatter.archive is None + except (Repository.ObjectNotFound, IntegrityErrorBase, Repository.StoreReadError): + metadata_missing = True + if metadata_missing: + # keys like {comment} need the archive metadata, which is damaged, missing or + # unreadable here. + # use the values from the archive directory entry, they are always available. + formatted = f"{info.name} {OutputTimestamp(info.ts)} {archive_id_hex}" + logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})") + if archive_id not in self.chunks: + logger.error(f"Archive metadata block {archive_id_hex} is missing!") + self.error_found = True + if self.repair: + logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") + self.manifest.archives.delete_by_id(archive_id) + else: + logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") + record_result(archive_id, found_before) + continue + try: cdata = self.repository.get(archive_id) - try: - _, data = self.repo_objs.parse(archive_id, cdata, ro_type=ROBJ_ARCHIVE_META) - except IntegrityErrorBase as integrity_error: - logger.error(f"Archive metadata block {archive_id_hex} is corrupted: {integrity_error}") - self.error_found = True - if self.repair: - logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") - self.manifest.archives.delete_by_id(archive_id) - else: - logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") - record_result(archive_id, found_before) - continue - archive = self.key.unpack_archive(data) - archive = ArchiveItem(internal_dict=archive) - if archive.version != 2: - raise Exception("Unknown archive metadata version") - items_buffer = ChunkBuffer(self.key) - items_buffer.write_chunk = add_callback - complete = False + _, data = self.repo_objs.parse(archive_id, cdata, ro_type=ROBJ_ARCHIVE_META) + except Repository.StoreReadError as err: + # unreadable, not corrupt: we did not see the metadata, so we can not tell + # whether this archive is fine - and must not "repair" it away, refs #3509. + # no result is recorded for it, so a later check verifies it again. + logger.error(f"Archive metadata block {archive_id_hex} could not be read: {err}") + self.error_found = True + if self.repair: + raise + continue + except IntegrityErrorBase as integrity_error: + logger.error(f"Archive metadata block {archive_id_hex} is corrupted: {integrity_error}") + self.error_found = True + if self.repair: + logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.") + self.manifest.archives.delete_by_id(archive_id) + else: + logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") + record_result(archive_id, found_before) + continue + archive = self.key.unpack_archive(data) + archive = ArchiveItem(internal_dict=archive) + if archive.version != 2: + raise Exception("Unknown archive metadata version") + items_buffer = ChunkBuffer(self.key) + items_buffer.write_chunk = add_callback + complete = False + try: for item in robust_iterator(archive): if sig_int and not self.repair: # without --repair the archive is only read, so the check also stops within it. @@ -3258,31 +3267,30 @@ def check_order(info): items_buffer.add(item) else: complete = True - items_buffer.flush(flush=True) - if self.repair: - archive.item_ptrs = archive_put_items( - items_buffer.chunks, repo_objs=self.repo_objs, add_reference=add_reference - ) - data = self.key.pack_metadata(archive.as_dict()) - new_archive_id = self.key.id_hash(data) - logger.debug(f"archive id old: {bin_to_hex(archive_id)}") - logger.debug(f"archive id new: {bin_to_hex(new_archive_id)}") - cdata = self.repo_objs.format(new_archive_id, {}, data, ro_type=ROBJ_ARCHIVE_META) - add_reference(new_archive_id, len(data), cdata) - self.create_archive_entry(info.name, new_archive_id, info.ts) - if archive_id != new_archive_id: - self.manifest.archives.delete_by_id(archive_id) - archive_id = new_archive_id - record_result(archive_id, found_before, complete=complete) except Repository.StoreReadError as err: - # some object of this archive could not be read at all (I/O error, refs #3509). - # we do not know what is in it, so we can not tell whether the archive is fine. no - # result is recorded for it, so a later check verifies it again. - logger.error(f"Archive {info.name} {archive_id_hex} could not be checked: {err}") + # part of the item metadata stream could not be read (see above): rewriting the + # archive from what we did read would drop the rest of it, refs #3509. + logger.error(f"Archive {info.name} {archive_id_hex} could not be read fully: {err}") self.error_found = True if self.repair: - # rewriting the archive now would drop whatever we could not read from it. raise + continue + items_buffer.flush(flush=True) + if self.repair: + archive.item_ptrs = archive_put_items( + items_buffer.chunks, repo_objs=self.repo_objs, add_reference=add_reference + ) + data = self.key.pack_metadata(archive.as_dict()) + new_archive_id = self.key.id_hash(data) + logger.debug(f"archive id old: {bin_to_hex(archive_id)}") + logger.debug(f"archive id new: {bin_to_hex(new_archive_id)}") + cdata = self.repo_objs.format(new_archive_id, {}, data, ro_type=ROBJ_ARCHIVE_META) + add_reference(new_archive_id, len(data), cdata) + self.create_archive_entry(info.name, new_archive_id, info.ts) + if archive_id != new_archive_id: + self.manifest.archives.delete_by_id(archive_id) + archive_id = new_archive_id + record_result(archive_id, found_before, complete=complete) finally: pi.finish() report_missing_chunks() diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 52c4c848e1..8503e93722 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -2731,38 +2731,70 @@ def test_check_max_duration_used_up_by_the_repository_check(archiver, monkeypatc assert archive_records(archiver) == {} -def make_pack_unreadable(monkeypatch, pack_name): - """Make reading the pack packs/ fail with an OSError, like failing storage does. +def make_store_reads_fail(monkeypatch, should_fail, *, after=0): + """Make matching posixfs reads fail with an OSError, like failing storage does. + + should_fail(name, offset, size) decides per read; offset/size are None for the operations that + do not take a range (hash, info, list). The first matching reads still succeed, which + models storage that starts failing (or fails only sometimes) rather than being dead from the + start. - Patches the posixfs backend rather than using file permissions, so it also works when the tests - run as root and does not depend on the platform's permission semantics. Returns a dict whose + Patches the backend rather than using file permissions, so this also works when the tests run + as root and does not depend on the platform's permission semantics. Returns a dict whose "failing" entry switches the failures off again (monkeypatch.undo() must not be used here, it would also revert the autouse clean_env fixture). """ from borgstore.backends.posixfs import PosixFS - state = {"failing": True} - orig_hash, orig_load = PosixFS.hash, PosixFS.load + state = {"failing": True, "hits": 0} + orig = {name: getattr(PosixFS, name) for name in ("hash", "load", "info", "list")} - def hits_pack(name): - # the backend gets the name including borgstore's nesting levels, e.g. packs/d0/d0a6... - return state["failing"] and name.rsplit("/", 1)[-1] == pack_name + def check(name, offset=None, size=None): + if not (state["failing"] and should_fail(name, offset, size)): + return + state["hits"] += 1 + if state["hits"] > after: + raise OSError(errno.EIO, "Input/output error", name) def failing_hash(self, name, algorithm="sha256"): - if hits_pack(name): - raise OSError(errno.EIO, "Input/output error", name) - return orig_hash(self, name, algorithm=algorithm) + check(name) + return orig["hash"](self, name, algorithm=algorithm) def failing_load(self, name, *, size=None, offset=0): - if hits_pack(name): - raise OSError(errno.EIO, "Input/output error", name) - return orig_load(self, name, size=size, offset=offset) + check(name, offset, size) + return orig["load"](self, name, size=size, offset=offset) + + def failing_info(self, name): + check(name) + return orig["info"](self, name) + + def failing_list(self, name): + check(name) + return orig["list"](self, name) monkeypatch.setattr(PosixFS, "hash", failing_hash) monkeypatch.setattr(PosixFS, "load", failing_load) + monkeypatch.setattr(PosixFS, "info", failing_info) + monkeypatch.setattr(PosixFS, "list", failing_list) return state +def object_name(name): + """Return the object's own name, without the nesting levels borgstore puts in front of it.""" + # the backend gets the name including those levels, e.g. packs/d0/d0a6... for packs/d0a6... + return name.rsplit("/", 1)[-1] + + +def make_pack_unreadable(monkeypatch, pack_name): + """Make every read of the pack packs/ fail.""" + return make_store_reads_fail(monkeypatch, lambda name, offset, size: object_name(name) == pack_name) + + +def make_namespace_listing_fail(monkeypatch, namespace): + """Make listing the given store namespace fail.""" + return make_store_reads_fail(monkeypatch, lambda name, offset, size: name.split("/")[0] == namespace) + + def some_pack_name(archiver): """Return the name of one of the repository's pack files.""" with KeyedRepository(archiver.repository_location, exclusive=True) as repository: @@ -2839,6 +2871,28 @@ def test_check_repair_refuses_unreadable_pack(archivers, request, monkeypatch): cmd(archiver, "check", "--repair") +def test_check_repair_does_not_salvage_while_a_pack_is_unreadable(archivers, request, monkeypatch): + # --repair salvages corrupt packs, but not while another pack could not be read: the repair is + # refused then, so it must not have changed anything before, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + chunk_id = file_content_chunk_id(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + corrupt_pack_name = bin_to_hex(repository.chunks[chunk_id].pack_id) + corrupt_chunk_on_disk(repository, chunk_id) + unreadable_pack_name = next( + info.name for info in repository.store_list("packs") if info.name != corrupt_pack_name + ) + state = make_pack_unreadable(monkeypatch, unreadable_pack_name) + with pytest.raises(Repository.RepairUnsafe): # local (not forked): the Error propagates + cmd(archiver, "check", "--repair", "--repository-only") + state["failing"] = False + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + assert corrupt_pack_name in {info.name for info in repository.store_list("packs")} # not salvaged + + def test_check_verify_data_unreadable_pack_keeps_chunks(archivers, request, monkeypatch): # --verify-data deletes chunks whose content is defect, but must keep chunks it could not read # at all, refs #3509. @@ -2875,7 +2929,7 @@ def test_check_unreadable_archive_metadata_pack(archivers, request, monkeypatch) make_pack_unreadable(monkeypatch, pack_name) output = cmd(archiver, "check", "--archives-only", exit_code=1) - assert "could not be checked" in output + assert f"Archive metadata block {bin_to_hex(archive_id)} could not be read" in output assert "Input/output error" in output assert "Archive consistency check complete, problems found." in output @@ -2894,3 +2948,131 @@ def test_unreadable_archive_metadata_pack_does_not_fake_an_archive(archivers, re make_pack_unreadable(monkeypatch, pack_name_of(archiver, archive_id)) with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates cmd(archiver, "repo-list") + + +def test_check_unreadable_packs_listing(archivers, request, monkeypatch): + # without a listing we do not know what to check, so this one is fatal - but it must end the + # check with a clear message instead of a traceback, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_namespace_listing_fail(monkeypatch, "packs") + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check", "--repository-only") + + +def test_check_unreadable_index_object(archivers, request, monkeypatch): + # an unreadable index object is reported and the check goes on to the packs. the missing-pack + # cross-check needs the index too, so it is skipped rather than reporting bogus results, #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + index_name = sorted(info.name for info in repository.store_list("index"))[0] + make_store_reads_fail(monkeypatch, lambda name, offset, size: object_name(name) == index_name) + + output = cmd(archiver, "check", "-v", "--repository-only", exit_code=1) + assert f"Store object index/{index_name} could not be read" in output + assert "skipping missing-pack detection" in output + assert "Finished checking packs." in output # the packs were checked anyway + assert "store object(s) could not be read" in output + + +def test_check_repair_unreadable_pack_aborts_index_rebuild(archivers, request, monkeypatch): + # --repair rebuilds the chunk index from the packs' object headers. an unreadable pack stops + # that with a clear error: an index rebuilt without its objects would drop them, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_pack_unreadable(monkeypatch, some_pack_name(archiver)) + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check", "--archives-only", "--repair") + + +def test_verify_data_repair_keeps_a_chunk_that_fails_to_re_read(archivers, request, monkeypatch): + # --verify-data --repair deletes a chunk only after its content failed to verify twice. if the + # second read fails outright, we did not see the content again, so the chunk must stay: this is + # exactly how a flaky disk would otherwise talk borg into throwing data away, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + chunk_id = file_content_chunk_id(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + entry = repository.chunks[chunk_id] + corrupt_chunk_on_disk(repository, chunk_id) + pack_name = bin_to_hex(entry.pack_id) + # fail reads of exactly this object's range (a bad spot in the pack that the first read still got + # through): the first read loads the whole pack and returns the corrupted content, so the chunk + # lands in the defect list, and the re-read that would confirm it (a range read) fails. matching on + # the object's full range leaves the pack's header scan (a short read at the same offset) working, + # so rebuilding the chunk index from the packs still succeeds. + make_store_reads_fail( + monkeypatch, + lambda name, offset, size: ( + object_name(name) == pack_name and offset == entry.obj_offset and size == entry.obj_size + ), + ) + + output = cmd(archiver, "check", "--repair", "--verify-data", "--archives-only", exit_code=0) + assert "not deleted, could not be re-read" in output + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + assert chunk_id in repository.chunks # the chunk is still there + + +def test_find_lost_archives_skips_unreadable_chunk(archivers, request, monkeypatch): + # the --find-lost-archives scan only looks for archive metadata, so skipping an unreadable + # chunk can at worst miss a lost archive - it never drops data, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_pack_unreadable(monkeypatch, pack_name_of(archiver, file_content_chunk_id(archiver))) + + output = cmd(archiver, "check", "--archives-only", "--find-lost-archives", exit_code=1) + assert "Skipping unreadable chunk" in output + + +def test_unreadable_archives_listing_is_not_an_empty_repository(archivers, request, monkeypatch): + # a namespace listing that fails must not look like "the namespace is empty" - that would make + # e.g. repo-list report a repository with no archives at all, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_namespace_listing_fail(monkeypatch, "archives") + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "repo-list") + + +def test_check_repair_stops_at_unreadable_archive_metadata(archivers, request, monkeypatch): + # a bad spot inside an otherwise readable pack: the chunk index still rebuilds from the pack's + # headers, so --repair gets as far as the archive itself - and must stop there rather than + # rewrite the archive around metadata it never read, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + archive_id = archive.id + entry = repository.chunks[archive_id] + pack_name = bin_to_hex(entry.pack_id) + # fail reads of the archive metadata object only; the short header reads at the same offset + # (and every other object in the pack) keep working. + state = make_store_reads_fail( + monkeypatch, + lambda name, offset, size: ( + object_name(name) == pack_name and offset == entry.obj_offset and size == entry.obj_size + ), + ) + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check", "--archives-only", "--repair") + + # the archive is untouched: once it reads again, it is still there and still checks out. + state["failing"] = False + assert "archive1" in cmd(archiver, "repo-list") + cmd(archiver, "check", exit_code=0) From 18f415b61d6e43fe843215dbbed42a7348fbbf5e Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 19 Sep 2026 21:08:38 +0200 Subject: [PATCH 4/4] check: an unreadable repository config is not a missing one, refs #3509 _load_config() carefully tells "the store has no repository config" (a lost config, something that never was a borg 2 repository, or an interrupted repo-create) from "the config is there". A read failure is neither, but it escaped as a raw OSError, so borg died with a traceback on it. It now raises StoreReadError. This replaces the get_manifest() wrapping of the previous revision of this commit: repository version 5 folded the manifest object into config/config, so reading that object is now the place where an I/O error must not be mistaken for "missing". Co-Authored-By: Claude Opus 5 --- src/borg/repository.py | 3 +++ src/borg/testsuite/archiver/check_cmd_test.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/borg/repository.py b/src/borg/repository.py index 8eb13a9a87..0d62bc1f80 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1332,6 +1332,9 @@ def _load_config(self): # the store exists, but has no repository config: a repository that lost its config, something # that never was a borg 2 repository, or the leftover of an interrupted repo-create (see create()). raise _ConfigMissing() from None + except OSError as exc: + # a config we could not read is not a missing one: do not let it look like "no repository", #3509. + raise self.StoreReadError("config/config", exc) from exc except UnicodeDecodeError: raise self.InvalidRepository(str(self._location)) from None config = configparser.ConfigParser(interpolation=None) diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 8503e93722..f0683baed2 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -3076,3 +3076,19 @@ def test_check_repair_stops_at_unreadable_archive_metadata(archivers, request, m state["failing"] = False assert "archive1" in cmd(archiver, "repo-list") cmd(archiver, "check", exit_code=0) + + +def test_unreadable_repo_config_is_not_a_missing_repository(archivers, request, monkeypatch): + # config/config is what makes the store a repository. if it can not be read, that must not look like + # "no repository here" or like a lost config - both are conclusions we did not earn, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + state = make_store_reads_fail(monkeypatch, lambda name, offset, size: name == "config/config") + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check") + + state["failing"] = False + assert "archive1" in cmd(archiver, "repo-list") # the repository was fine all along + cmd(archiver, "check", exit_code=0)