From c4599d8ea461f9a5cd4fcb2ce50a45eb5d481852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= Date: Thu, 15 Oct 2015 18:39:34 -0400 Subject: [PATCH 1/4] fix cascading failure with the index conversion code this resolves bug #something where the index file could not be converted, completely breaking conversion. it seems that, during some refactoring, the index conversion code was completely dropped. this was missed by the unit tests because the repo can still be opened by the constructor even though the index is invalid, so tests need improvements there. --- borg/upgrader.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/borg/upgrader.py b/borg/upgrader.py index 33f100e38..daf090998 100644 --- a/borg/upgrader.py +++ b/borg/upgrader.py @@ -139,12 +139,14 @@ class AtticRepositoryUpgrader(Repository): `Cache.open()`, edit in place and then `Cache.close()` to make sure we have locking right """ - caches = [] transaction_id = self.get_index_transaction_id() if transaction_id is None: logger.warning('no index file found for repository %s' % self.path) else: - caches += [os.path.join(self.path, 'index.%d' % transaction_id).encode('utf-8')] + cache = os.path.join(self.path, 'index.%d' % transaction_id).encode('utf-8') + logger.info("converting index cache %s" % cache) + if not dryrun: + AtticRepositoryUpgrader.header_replace(cache, b'ATTICIDX', b'BORG_IDX') # copy of attic's get_cache_dir() attic_cache_dir = os.environ.get('ATTIC_CACHE_DIR', From fb68c6ff70acf8592894d83d353cb8f0c7735b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= Date: Thu, 15 Oct 2015 18:42:20 -0400 Subject: [PATCH 2/4] always convert the chunks cache we find it seems it is possible that the chunks files are copied but *not* converted. this may have happened here because the conversion was interrupted, although the specific scenario is still unclear (but it did happen during manual tests here). therefore reproducing this problem seems to be difficult, hence the lack of tests for this specific issue. since we consider the header replacement code to be safe, that we always convert shouldn't pose any additional threat to the existing borg chunk cache. --- borg/upgrader.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/borg/upgrader.py b/borg/upgrader.py index daf090998..6bb5055c7 100644 --- a/borg/upgrader.py +++ b/borg/upgrader.py @@ -166,23 +166,23 @@ class AtticRepositoryUpgrader(Repository): :params path: the basename of the cache file to copy (example: "files" or "chunks") as a string - :returns: the borg file that was created or None if non - was created. + :returns: the borg file that was created or None if no + Attic cache file was found. """ attic_file = os.path.join(attic_cache_dir, path) if os.path.exists(attic_file): borg_file = os.path.join(borg_cache_dir, path) if os.path.exists(borg_file): - logger.warning("borg cache file already exists in %s, skipping conversion of %s" % (borg_file, attic_file)) + logger.warning("borg cache file already exists in %s, not copying from Attic" % (borg_file)) else: logger.info("copying attic cache file from %s to %s" % (attic_file, borg_file)) if not dryrun: shutil.copyfile(attic_file, borg_file) - return borg_file + return borg_file else: logger.warning("no %s cache file found in %s" % (path, attic_file)) - return None + return None # XXX: untested, because generating cache files is a PITA, see # Archiver.do_create() for proof @@ -196,11 +196,10 @@ class AtticRepositoryUpgrader(Repository): # we need to convert the headers of those files, copy first for cache in ['chunks']: - copied = copy_cache_file(cache) - if copied: - logger.info("converting cache %s" % cache) - if not dryrun: - AtticRepositoryUpgrader.header_replace(cache, b'ATTICIDX', b'BORG_IDX') + cache = copy_cache_file(cache) + logger.info("converting cache %s" % cache) + if not dryrun: + AtticRepositoryUpgrader.header_replace(cache, b'ATTICIDX', b'BORG_IDX') class AtticKeyfileKey(KeyfileKey): From 07e7f2dcad2c409282b88f7fa2a2cd2bf840fcee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= Date: Sat, 17 Oct 2015 09:58:40 -0400 Subject: [PATCH 3/4] fixup warning message --- borg/upgrader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borg/upgrader.py b/borg/upgrader.py index 6bb5055c7..dd152496e 100644 --- a/borg/upgrader.py +++ b/borg/upgrader.py @@ -174,7 +174,7 @@ class AtticRepositoryUpgrader(Repository): if os.path.exists(attic_file): borg_file = os.path.join(borg_cache_dir, path) if os.path.exists(borg_file): - logger.warning("borg cache file already exists in %s, not copying from Attic" % (borg_file)) + logger.warning("borg cache file already exists in %s, not copying from Attic", borg_file) else: logger.info("copying attic cache file from %s to %s" % (attic_file, borg_file)) if not dryrun: From ce72051284d78a7d177925b9eb64c55ce33847c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= Date: Sat, 17 Oct 2015 09:59:54 -0400 Subject: [PATCH 4/4] rename cache variable to index for clarity --- borg/upgrader.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/borg/upgrader.py b/borg/upgrader.py index dd152496e..0f05c6982 100644 --- a/borg/upgrader.py +++ b/borg/upgrader.py @@ -143,10 +143,10 @@ class AtticRepositoryUpgrader(Repository): if transaction_id is None: logger.warning('no index file found for repository %s' % self.path) else: - cache = os.path.join(self.path, 'index.%d' % transaction_id).encode('utf-8') - logger.info("converting index cache %s" % cache) + index = os.path.join(self.path, 'index.%d' % transaction_id).encode('utf-8') + logger.info("converting index index %s" % index) if not dryrun: - AtticRepositoryUpgrader.header_replace(cache, b'ATTICIDX', b'BORG_IDX') + AtticRepositoryUpgrader.header_replace(index, b'ATTICIDX', b'BORG_IDX') # copy of attic's get_cache_dir() attic_cache_dir = os.environ.get('ATTIC_CACHE_DIR',