Remove BackupStoreAccounts::LockAccount

Replace with BackupFileSystem::GetLock where used (only in tests at this
point).

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));
-	}
-}
-
-

lib/backupstore/BackupStoreAccounts.h

@@ -13,14 +13,15 @@
 #include <string>
 
 #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);

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<BackupStoreAccountDatabase> 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<BackupProtocolAccountUsage2> 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<BackupStoreAccountDatabase> 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;
 }

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);