summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/backupstore/BackupStoreCheck2.cpp15
-rw-r--r--test/backupstorefix/testbackupstorefix.cpp225
2 files changed, 215 insertions, 25 deletions
diff --git a/lib/backupstore/BackupStoreCheck2.cpp b/lib/backupstore/BackupStoreCheck2.cpp
index 8d633895..19c79304 100644
--- a/lib/backupstore/BackupStoreCheck2.cpp
+++ b/lib/backupstore/BackupStoreCheck2.cpp
@@ -706,7 +706,12 @@ bool BackupStoreDirectory::CheckAndFix()
bool changed = false;
// Check that if a file depends on a new version, that version is in this directory
+ bool restart;
+
+ do
{
+ restart = false;
+
std::vector<Entry*>::iterator i(mEntries.begin());
for(; i != mEntries.end(); ++i)
{
@@ -717,7 +722,7 @@ bool BackupStoreDirectory::CheckAndFix()
if(newerEn == 0)
{
// Depends on something, but it isn't there.
- BOX_TRACE("Entry id " << FMT_i <<
+ BOX_WARNING("Entry id " << FMT_i <<
" removed because depends "
"on newer version " <<
FMT_OID(dependsNewer) <<
@@ -727,11 +732,12 @@ bool BackupStoreDirectory::CheckAndFix()
delete *i;
mEntries.erase(i);
- // Start again at the beginning of the vector, the iterator is now invalid
- i = mEntries.begin();
-
// Mark as changed
changed = true;
+
+ // Start again at the beginning of the vector, the iterator is now invalid
+ restart = true;
+ break;
}
else
{
@@ -753,6 +759,7 @@ bool BackupStoreDirectory::CheckAndFix()
}
}
}
+ while(restart);
// Check that if a file has a dependency marked, it exists, and remove it if it doesn't
{
diff --git a/test/backupstorefix/testbackupstorefix.cpp b/test/backupstorefix/testbackupstorefix.cpp
index 8cc5c8aa..818e836d 100644
--- a/test/backupstorefix/testbackupstorefix.cpp
+++ b/test/backupstorefix/testbackupstorefix.cpp
@@ -201,19 +201,35 @@ void check_dir_dep(BackupStoreDirectory &dir, checkdepinfoen *ck)
void test_dir_fixing()
{
+ // Test that entries pointing to nonexistent entries are removed
{
- MEMLEAKFINDER_NO_LEAKS;
- fnames[0].SetAsClearFilename("x1");
- fnames[1].SetAsClearFilename("x2");
- fnames[2].SetAsClearFilename("x3");
+ BackupStoreDirectory dir;
+ BackupStoreDirectory::Entry* e = dir.AddEntry(fnames[0], 12,
+ 2 /* id */, 1, BackupStoreDirectory::Entry::Flags_File |
+ BackupStoreDirectory::Entry::Flags_OldVersion, 2);
+ e->SetDependsNewer(3);
+
+ TEST_THAT(dir.CheckAndFix() == true);
+ TEST_THAT(dir.CheckAndFix() == false);
+
+ dir_en_check ck[] = {
+ {-1, 0, 0}
+ };
+
+ check_dir(dir, ck);
}
{
BackupStoreDirectory dir;
- dir.AddEntry(fnames[0], 12, 2 /* id */, 1, BackupStoreDirectory::Entry::Flags_File, 2);
- dir.AddEntry(fnames[1], 12, 2 /* id */, 1, BackupStoreDirectory::Entry::Flags_File, 2);
- dir.AddEntry(fnames[0], 12, 3 /* id */, 1, BackupStoreDirectory::Entry::Flags_File, 2);
- dir.AddEntry(fnames[0], 12, 5 /* id */, 1, BackupStoreDirectory::Entry::Flags_File | BackupStoreDirectory::Entry::Flags_OldVersion, 2);
+ dir.AddEntry(fnames[0], 12, 2 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File, 2);
+ dir.AddEntry(fnames[1], 12, 2 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File, 2);
+ dir.AddEntry(fnames[0], 12, 3 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File, 2);
+ dir.AddEntry(fnames[0], 12, 5 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File |
+ BackupStoreDirectory::Entry::Flags_OldVersion, 2);
dir_en_check ck[] = {
{1, 2, BackupStoreDirectory::Entry::Flags_File},
@@ -250,20 +266,26 @@ void test_dir_fixing()
// Test dependency fixing
{
BackupStoreDirectory dir;
- BackupStoreDirectory::Entry *e2
- = dir.AddEntry(fnames[0], 12, 2 /* id */, 1, BackupStoreDirectory::Entry::Flags_File | BackupStoreDirectory::Entry::Flags_OldVersion, 2);
+ BackupStoreDirectory::Entry *e2 = dir.AddEntry(fnames[0], 12,
+ 2 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File |
+ BackupStoreDirectory::Entry::Flags_OldVersion, 2);
TEST_THAT(e2 != 0);
e2->SetDependsNewer(3);
- BackupStoreDirectory::Entry *e3
- = dir.AddEntry(fnames[0], 12, 3 /* id */, 1, BackupStoreDirectory::Entry::Flags_File | BackupStoreDirectory::Entry::Flags_OldVersion, 2);
+ BackupStoreDirectory::Entry *e3 = dir.AddEntry(fnames[0], 12,
+ 3 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File |
+ BackupStoreDirectory::Entry::Flags_OldVersion, 2);
TEST_THAT(e3 != 0);
e3->SetDependsNewer(4); e3->SetDependsOlder(2);
- BackupStoreDirectory::Entry *e4
- = dir.AddEntry(fnames[0], 12, 4 /* id */, 1, BackupStoreDirectory::Entry::Flags_File | BackupStoreDirectory::Entry::Flags_OldVersion, 2);
+ BackupStoreDirectory::Entry *e4 = dir.AddEntry(fnames[0], 12,
+ 4 /* id */, 1,
+ BackupStoreDirectory::Entry::Flags_File |
+ BackupStoreDirectory::Entry::Flags_OldVersion, 2);
TEST_THAT(e4 != 0);
e4->SetDependsNewer(5); e4->SetDependsOlder(3);
- BackupStoreDirectory::Entry *e5
- = dir.AddEntry(fnames[0], 12, 5 /* id */, 1, BackupStoreDirectory::Entry::Flags_File, 2);
+ BackupStoreDirectory::Entry *e5 = dir.AddEntry(fnames[0], 12,
+ 5 /* id */, 1, BackupStoreDirectory::Entry::Flags_File, 2);
TEST_THAT(e5 != 0);
e5->SetDependsOlder(4);
@@ -355,7 +377,167 @@ int test(int argc, const char *argv[])
"create 01234567 0 10000B 20000B") == 0);
TestRemoteProcessMemLeaks("bbstoreaccounts.memleaks");
+ // Run the perl script to create the initial directories
+ TEST_THAT_ABORTONFAIL(::system(PERL_EXECUTABLE
+ " testfiles/testbackupstorefix.pl init") == 0);
+
+ BOX_INFO("Test that an entry pointing to a file that doesn't exist "
+ "is really deleted");
+
+ {
+ // Check that the initial situation matches our expectations.
+ std::string rootDirName;
+ StoreStructure::MakeObjectFilename(1 /* root */, storeRoot,
+ discSetNum, rootDirName,
+ true /* EnsureDirectoryExists */);
+ std::auto_ptr<RaidFileRead> file(RaidFileRead::Open(discSetNum,
+ rootDirName));
+
+ file = RaidFileRead::Open(discSetNum, rootDirName);
+ BackupStoreDirectory dir;
+ dir.ReadFromStream(*file, IOStream::TimeOutInfinite);
+ dir_en_check start_entries[] = {{-1, 0, 0}};
+ check_dir(dir, start_entries);
+ static checkdepinfoen start_deps[] = {{-1, 0, 0}};
+ check_dir_dep(dir, start_deps);
+
+ BackupStoreContext ctx(0x01234567,
+ *(HousekeepingInterface *)NULL,
+ "test" /* rConnectionDetails */);
+ ctx.SetClientHasAccount(storeRoot, discSetNum);
+
+ BackupProtocolLocal client(ctx);
+ client.QueryVersion(BACKUP_STORE_SERVER_VERSION);
+ client.QueryLogin(0x01234567, 0 /* read/write */);
+
+ std::string file_path = "testfiles/TestDir1/cannes/ict/metegoguered/oats";
+ int x1id = fake_upload(client, file_path, 0);
+
+ file = RaidFileRead::Open(discSetNum, rootDirName);
+ dir.ReadFromStream(*file, IOStream::TimeOutInfinite);
+
+ // Everything should be OK at the moment
+ TEST_THAT(dir.CheckAndFix() == false);
+
+ // Check that we've ended up with the right preconditions
+ // for the tests below.
+ dir_en_check before_entries[] = {
+ {0, x1id, BackupStoreDirectory::Entry::Flags_File},
+ {-1, 0, 0}
+ };
+ check_dir(dir, before_entries);
+ static checkdepinfoen before_deps[] = {{x1id, 0, 0}, {-1, 0, 0}};
+ check_dir_dep(dir, before_deps);
+
+ // Now break the reverse dependency by deleting x1 (the file,
+ // not the directory entry)
+ std::string x1FileName;
+ StoreStructure::MakeObjectFilename(x1id, storeRoot, discSetNum,
+ x1FileName, true /* EnsureDirectoryExists */);
+ RaidFileWrite deleteX1(discSetNum, x1FileName);
+ deleteX1.Delete();
+
+ // Check the store, check that the error is detected and
+ // repaired, by removing x1 from the directory.
+ BackupStoreCheck check(storeRoot, discSetNum,
+ 0x01234567 /* AccountID */, true /* FixErrors */,
+ false /* Quiet */);
+ check.Check();
+
+ // Read the directory back in, check that it's empty
+ file = RaidFileRead::Open(discSetNum, rootDirName);
+ dir.ReadFromStream(*file, IOStream::TimeOutInfinite);
+ dir_en_check after_entries[] = {{-1, 0, 0}};
+ check_dir(dir, after_entries);
+ static checkdepinfoen after_deps[] = {{-1, 0, 0}};
+ check_dir_dep(dir, after_deps);
+ }
+
+ BOX_INFO("Test that an entry pointing to another that doesn't exist "
+ "is really deleted");
+
+ {
+ // Check that the initial situation matches our expectations.
+ std::string rootDirName;
+ StoreStructure::MakeObjectFilename(1 /* root */, storeRoot,
+ discSetNum, rootDirName,
+ true /* EnsureDirectoryExists */);
+ std::auto_ptr<RaidFileRead> file(RaidFileRead::Open(discSetNum,
+ rootDirName));
+
+ file = RaidFileRead::Open(discSetNum, rootDirName);
+ BackupStoreDirectory dir;
+ dir.ReadFromStream(*file, IOStream::TimeOutInfinite);
+ dir_en_check start_entries[] = {{-1, 0, 0}};
+ check_dir(dir, start_entries);
+ static checkdepinfoen start_deps[] = {{-1, 0, 0}};
+ check_dir_dep(dir, start_deps);
+
+ BackupStoreContext ctx(0x01234567,
+ *(HousekeepingInterface *)NULL,
+ "test" /* rConnectionDetails */);
+ ctx.SetClientHasAccount(storeRoot, discSetNum);
+
+ BackupProtocolLocal client(ctx);
+ client.QueryVersion(BACKUP_STORE_SERVER_VERSION);
+ client.QueryLogin(0x01234567, 0 /* read/write */);
+
+ std::string file_path = "testfiles/TestDir1/cannes/ict/metegoguered/oats";
+ int x1id = fake_upload(client, file_path, 0);
+
+ // Make a small change to the file
+ FileStream fs(file_path, O_WRONLY | O_APPEND);
+ const char* more = " and more oats!";
+ fs.Write(more, strlen(more));
+ fs.Close();
+
+ int x1aid = fake_upload(client, file_path, x1id);
+ file = RaidFileRead::Open(discSetNum, rootDirName);
+ dir.ReadFromStream(*file, IOStream::TimeOutInfinite);
+
+ // Everything should be OK at the moment
+ TEST_THAT(dir.CheckAndFix() == false);
+
+ // Check that we've ended up with the right preconditions
+ // for the tests below.
+ dir_en_check before_entries[] = {
+ {0, x1id, BackupStoreDirectory::Entry::Flags_File |
+ BackupStoreDirectory::Entry::Flags_OldVersion},
+ {0, x1aid, BackupStoreDirectory::Entry::Flags_File},
+ {-1, 0, 0}
+ };
+ check_dir(dir, before_entries);
+ static checkdepinfoen before_deps[] = {{x1id, x1aid, 0},
+ {x1aid, 0, x1id}, {-1, 0, 0}};
+ check_dir_dep(dir, before_deps);
+
+ // Now break the reverse dependency by deleting x1a (the file,
+ // not the directory entry)
+ std::string x1aFileName;
+ StoreStructure::MakeObjectFilename(x1aid, storeRoot, discSetNum,
+ x1aFileName, true /* EnsureDirectoryExists */);
+ RaidFileWrite deleteX1a(discSetNum, x1aFileName);
+ deleteX1a.Delete();
+
+ // Check the store, check that the error is detected and
+ // repaired, by removing x1 from the directory.
+ BackupStoreCheck check(storeRoot, discSetNum,
+ 0x01234567 /* AccountID */, true /* FixErrors */,
+ false /* Quiet */);
+ check.Check();
+
+ // Read the directory back in, check that it's empty
+ file = RaidFileRead::Open(discSetNum, rootDirName);
+ dir.ReadFromStream(*file, IOStream::TimeOutInfinite);
+ dir_en_check after_entries[] = {{-1, 0, 0}};
+ check_dir(dir, after_entries);
+ static checkdepinfoen after_deps[] = {{-1, 0, 0}};
+ check_dir_dep(dir, after_deps);
+ }
+
// Start the bbstored server
+ BOX_TRACE("Starting bbstored server: " BBSTORED
+ " testfiles/bbstored.conf");
int bbstored_pid = LaunchServer(BBSTORED " testfiles/bbstored.conf",
"testfiles/bbstored.pid");
TEST_THAT(bbstored_pid > 0);
@@ -364,10 +546,6 @@ int test(int argc, const char *argv[])
::sleep(1);
TEST_THAT(ServerIsAlive(bbstored_pid));
- // Run the perl script to create the initial directories
- TEST_THAT_ABORTONFAIL(::system(PERL_EXECUTABLE
- " testfiles/testbackupstorefix.pl init") == 0);
-
std::string cmd = BBACKUPD " " + bbackupd_args +
" testfiles/bbackupd.conf";
int bbackupd_pid = LaunchServer(cmd, "testfiles/bbackupd.pid");
@@ -420,7 +598,12 @@ int test(int argc, const char *argv[])
BackupStoreCheck checker(storeRoot, discSetNum,
0x01234567, true /* FixErrors */, false /* Quiet */);
checker.Check();
- TEST_EQUAL(1, checker.GetNumErrorsFound());
+
+ // Should just be greater than 1 really, we don't know quite
+ // how good the checker is (or will become) at spotting errors!
+ // But this will help us catch changes in checker behaviour,
+ // so it's not a bad thing to test.
+ TEST_EQUAL(5, checker.GetNumErrorsFound());
file = RaidFileRead::Open(discSetNum, fn);
dir.ReadFromStream(*file, IOStream::TimeOutInfinite);