diff --git a/lib/backupstore/BackupStoreAccounts.cpp b/lib/backupstore/BackupStoreAccounts.cpp index 856c7a620..8177d760d 100644 --- a/lib/backupstore/BackupStoreAccounts.cpp +++ b/lib/backupstore/BackupStoreAccounts.cpp @@ -107,36 +107,3 @@ bool BackupStoreAccounts::AccountExists(int32_t ID) return mrDatabase.EntryExists(ID); } -void BackupStoreAccounts::LockAccount(int32_t ID, NamedLock& rNamedLock) -{ - const BackupStoreAccountDatabase::Entry &en(mrDatabase.GetEntry(ID)); - std::string rootDir = MakeAccountRootDir(ID, en.GetDiscSet()); - int discSet = en.GetDiscSet(); - - std::string writeLockFilename; - StoreStructure::MakeWriteLockFilename(rootDir, discSet, writeLockFilename); - - bool gotLock = false; - int triesLeft = 8; - do - { - gotLock = rNamedLock.TryAndGetLock(writeLockFilename, - 0600 /* restrictive file permissions */); - - if(!gotLock) - { - --triesLeft; - ::sleep(1); - } - } - while (!gotLock && triesLeft > 0); - - if (!gotLock) - { - THROW_EXCEPTION_MESSAGE(BackupStoreException, - CouldNotLockStoreAccount, "Failed to get exclusive " - "lock on account " << BOX_FORMAT_ACCOUNT(ID)); - } -} - - diff --git a/lib/backupstore/BackupStoreAccounts.h b/lib/backupstore/BackupStoreAccounts.h index 0e3948e15..d982835b4 100644 --- a/lib/backupstore/BackupStoreAccounts.h +++ b/lib/backupstore/BackupStoreAccounts.h @@ -13,14 +13,15 @@ #include #include "BackupStoreAccountDatabase.h" -#include "BackupAccountControl.h" -#include "NamedLock.h" // -------------------------------------------------------------------------- // // Class // Name: BackupStoreAccounts -// Purpose: Account management for backup store server +// Purpose: Account management for backup store server. This +// class now serves very little purpose, and it should +// probably be folded into other classes, such as +// BackupStoreAccountDatabase and RaidBackupFileSystem. // Created: 2003/08/21 // // -------------------------------------------------------------------------- @@ -40,7 +41,6 @@ class BackupStoreAccounts { return MakeAccountRootDir(rEntry.GetID(), rEntry.GetDiscSet()); } - void LockAccount(int32_t ID, NamedLock& rNamedLock); private: static std::string MakeAccountRootDir(int32_t ID, int DiscSet); diff --git a/test/backupstorefix/testbackupstorefix.cpp b/test/backupstorefix/testbackupstorefix.cpp index a560b9816..5955ca780 100644 --- a/test/backupstorefix/testbackupstorefix.cpp +++ b/test/backupstorefix/testbackupstorefix.cpp @@ -723,20 +723,13 @@ int test(int argc, const char *argv[]) BOX_INFO(" === Delete an entry for an object from dir, change that " "object to be a patch, check it's deleted"); { - // Temporarily stop the server, so it doesn't repair the refcount error. Except - // on win32, where hard-killing the server can leave a lockfile in place, - // breaking the rest of the test. -#ifdef WIN32 - // Wait for the server to finish housekeeping first, by getting a lock on - // the account. - std::auto_ptr apAccounts( - BackupStoreAccountDatabase::Read("testfiles/accounts.txt")); - BackupStoreAccounts acc(*apAccounts); - NamedLock lock; - acc.LockAccount(0x1234567, lock); -#else - TEST_THAT(StopServer()); -#endif + // Wait for the server to finish housekeeping (if any) and then lock the account + // before damaging it, to prevent housekeeping from repairing the damage in the + // background. We could just stop the server, but on Windows that can leave the + // account half-cleaned and always leaves a PID file lying around, which breaks + // the rest of the test, so we do it this way on all platforms instead. + RaidBackupFileSystem fs(0x01234567, accountRootDir, 0); // discSet + fs.GetLock(); // Open dir and find entry int64_t delID = getID("Test1/cannes/ict/metegoguered/oats"); @@ -792,9 +785,8 @@ int test(int argc, const char *argv[]) // ERROR: BlocksInCurrentFiles changed from 228 to 226 // ERROR: NumCurrentFiles changed from 114 to 113 // WARNING: Reference count of object 0x44 changed from 1 to 0 -#ifdef WIN32 - lock.ReleaseLock(); -#endif + fs.ReleaseLock(); + TEST_EQUAL(5, check_account_for_errors()); { std::auto_ptr usage = @@ -808,12 +800,6 @@ int test(int argc, const char *argv[]) TEST_EQUAL(usage->GetBlocksInDirectories(), 56); } - // Start the server again, so testbackupstorefix.pl can run bbackupquery which - // connects to it. Except on win32, where we didn't stop it earlier. -#ifndef WIN32 - TEST_THAT(StartServer(daemon_args)); -#endif - // Check TEST_THAT(::system(PERL_EXECUTABLE " testfiles/testbackupstorefix.pl check 1") @@ -995,11 +981,8 @@ int test(int argc, const char *argv[]) // before damaging it, to avoid a race condition where bbstored runs housekeeping // after we disconnect, extremely slowly on AppVeyor, and this causes the second // bbstoreaccounts check command to time out waiting for a lock. - std::auto_ptr apAccounts( - BackupStoreAccountDatabase::Read("testfiles/accounts.txt")); - BackupStoreAccounts acc(*apAccounts); - NamedLock lock; - acc.LockAccount(0x1234567, lock); + RaidBackupFileSystem fs(0x01234567, accountRootDir, 0); // discSet + fs.GetLock(); // File CorruptObject("Test1/foreomizes/stemptinevidate/algoughtnerge", 33, @@ -1035,13 +1018,7 @@ int test(int argc, const char *argv[]) // --------------------------------------------------------- // Stop server - TEST_THAT(KillServer(bbstored_pid)); - - #ifdef WIN32 - TEST_THAT(EMU_UNLINK("testfiles/bbstored.pid") == 0); - #else - TestRemoteProcessMemLeaks("bbstored.memleaks"); - #endif + TEST_THAT(StopServer()); return 0; } diff --git a/test/bbackupd/testbbackupd.cpp b/test/bbackupd/testbbackupd.cpp index 724ec3e3e..8ea20af4a 100644 --- a/test/bbackupd/testbbackupd.cpp +++ b/test/bbackupd/testbbackupd.cpp @@ -2630,8 +2630,8 @@ bool test_store_error_reporting() // Lock scope { - NamedLock writeLock; - acc.LockAccount(0x01234567, writeLock); + RaidBackupFileSystem fs(0x01234567, "backup/01234567/", 0); + fs.GetLock(); TEST_THAT(::rename("testfiles/0_0/backup/01234567/info.rf", "testfiles/0_0/backup/01234567/info.rf.bak") == 0);