diff --git a/lib/backupstore/BackupStoreCheck.cpp b/lib/backupstore/BackupStoreCheck.cpp index 73890c6ad..70cea88c4 100644 --- a/lib/backupstore/BackupStoreCheck.cpp +++ b/lib/backupstore/BackupStoreCheck.cpp @@ -125,7 +125,41 @@ void BackupStoreCheck::Check() } BackupStoreAccountDatabase::Entry account(mAccountID, mDiscSetNumber); - mapNewRefs = BackupStoreRefCountDatabase::Create(account); + // If we are read-only, then we should not call GetPotentialRefCountDatabase because + // that does actually change the store: the temporary file would conflict with any other + // process which wants to do the same thing at the same time (e.g. housekeeping), and if + // neither process locks the store, they will break each other. We can still create a + // refcount DB in a temporary directory, and Commit() will not really commit it in that + // case (it will rename it, but still in the temporary directory). + if(mFixErrors) + { + mapNewRefs = BackupStoreRefCountDatabase::Create(account); + } + else + { + std::string temp_file = GetTempDirPath() + "boxbackup_refcount_db_XXXXXX"; + char temp_file_buf[PATH_MAX]; + strncpy(temp_file_buf, temp_file.c_str(), sizeof(temp_file_buf)); +#ifdef WIN32 + if(_mktemp_s(temp_file_buf, sizeof(temp_file_buf)) != 0) + { + THROW_EXCEPTION_MESSAGE(BackupStoreException, FailedToCreateTemporaryFile, + "Failed to get a temporary file name based on " << temp_file); + } +#else + int fd = mkstemp(temp_file_buf); + if(fd == -1) + { + THROW_SYS_FILE_ERROR("Failed to get a temporary file name based on", + temp_file, BackupStoreException, FailedToCreateTemporaryFile); + } + close(fd); +#endif + + BOX_TRACE("Creating temporary refcount DB in a temporary file: " << temp_file_buf); + mapNewRefs = BackupStoreRefCountDatabase::Create(temp_file_buf, + mAccountID, true); // reuse_existing_file + } // Phase 1, check objects if(!mQuiet) diff --git a/lib/backupstore/BackupStoreException.txt b/lib/backupstore/BackupStoreException.txt index 9e4320e8d..15cbbea6f 100644 --- a/lib/backupstore/BackupStoreException.txt +++ b/lib/backupstore/BackupStoreException.txt @@ -77,3 +77,4 @@ InvalidEtagHeader 73 The S3 HTTP response contain a malformed or unexpected ETa FileSystemNotInitialised 74 No BackupFileSystem has been configured for this account ObjectDoesNotExist 75 The specified object ID does not exist in the store, or is not of this type AccountAlreadyExists 76 Tried to create an account that already exists +FailedToCreateTemporaryFile 77 Failed to create a temporary file diff --git a/lib/backupstore/BackupStoreRefCountDatabase.cpp b/lib/backupstore/BackupStoreRefCountDatabase.cpp index 6a80ecf8b..b4b0c895a 100644 --- a/lib/backupstore/BackupStoreRefCountDatabase.cpp +++ b/lib/backupstore/BackupStoreRefCountDatabase.cpp @@ -27,6 +27,9 @@ #define REFCOUNT_MAGIC_VALUE 0x52656643 // RefC #define REFCOUNT_FILENAME "refcount" +typedef BackupStoreRefCountDatabase::refcount_t refcount_t; + + // -------------------------------------------------------------------------- // // Function @@ -35,22 +38,27 @@ // Created: 2003/08/28 // // -------------------------------------------------------------------------- -BackupStoreRefCountDatabase::BackupStoreRefCountDatabase(const - BackupStoreAccountDatabase::Entry& rAccount, bool ReadOnly, - bool Temporary, std::auto_ptr apDatabaseFile) -: mAccount(rAccount), - mFilename(GetFilename(rAccount, Temporary)), +BackupStoreRefCountDatabase::BackupStoreRefCountDatabase( + const std::string& Filename, int64_t AccountID, bool ReadOnly, bool PotentialDB, + bool TemporaryDB, std::auto_ptr apDatabaseFile) +: mAccountID(AccountID), + mFilename(Filename + (PotentialDB ? "X" : "")), + mFinalFilename(TemporaryDB ? "" : Filename), mReadOnly(ReadOnly), mIsModified(false), - mIsTemporaryFile(Temporary), + mIsPotentialDB(PotentialDB), + mIsTemporaryDB(TemporaryDB), mapDatabaseFile(apDatabaseFile) { - ASSERT(!(ReadOnly && Temporary)); // being both doesn't make sense + ASSERT(!(PotentialDB && TemporaryDB)); // being both doesn't make sense + ASSERT(!(ReadOnly && PotentialDB)); // being both doesn't make sense } void BackupStoreRefCountDatabase::Commit() { - if (!mIsTemporaryFile) + ASSERT(!mIsTemporaryDB); + + if (!mIsPotentialDB) { THROW_EXCEPTION_MESSAGE(CommonException, Internal, "Cannot commit a permanent reference count database"); @@ -65,32 +73,30 @@ void BackupStoreRefCountDatabase::Commit() mapDatabaseFile->Close(); mapDatabaseFile.reset(); - std::string Final_Filename = GetFilename(mAccount, false); - #ifdef WIN32 - if(FileExists(Final_Filename) && EMU_UNLINK(Final_Filename.c_str()) != 0) + if(FileExists(mFinalFilename) && EMU_UNLINK(mFinalFilename.c_str()) != 0) { THROW_EMU_FILE_ERROR("Failed to delete old permanent refcount " - "database file", mFilename, CommonException, + "database file", mFinalFilename, CommonException, OSFileError); } #endif - if(rename(mFilename.c_str(), Final_Filename.c_str()) != 0) + if(rename(mFilename.c_str(), mFinalFilename.c_str()) != 0) { THROW_EMU_ERROR("Failed to rename temporary refcount database " "file from " << mFilename << " to " << - Final_Filename, CommonException, OSFileError); + mFinalFilename, CommonException, OSFileError); } - mFilename = Final_Filename; + mFilename = mFinalFilename; mIsModified = false; - mIsTemporaryFile = false; + mIsPotentialDB = false; } void BackupStoreRefCountDatabase::Discard() { - if (!mIsTemporaryFile) + if (!mIsTemporaryDB && !mIsPotentialDB) { THROW_EXCEPTION_MESSAGE(CommonException, Internal, "Cannot discard a permanent reference count database"); @@ -108,13 +114,14 @@ void BackupStoreRefCountDatabase::Discard() if(EMU_UNLINK(mFilename.c_str()) != 0) { - THROW_EMU_FILE_ERROR("Failed to delete temporary refcount " + THROW_EMU_FILE_ERROR("Failed to delete temporary/potential refcount " "database file", mFilename, CommonException, OSFileError); } mIsModified = false; - mIsTemporaryFile = false; + mIsTemporaryDB = false; + mIsPotentialDB = false; } // -------------------------------------------------------------------------- @@ -127,11 +134,15 @@ void BackupStoreRefCountDatabase::Discard() // -------------------------------------------------------------------------- BackupStoreRefCountDatabase::~BackupStoreRefCountDatabase() { - if (mIsTemporaryFile) + if (mIsTemporaryDB || mIsPotentialDB) { // Don't throw exceptions in a destructor. - BOX_ERROR("BackupStoreRefCountDatabase destroyed without " - "explicit commit or discard"); + if(mIsPotentialDB) + { + BOX_ERROR("Potential new BackupStoreRefCountDatabase destroyed " + "without explicit commit or discard"); + } + try { Discard(); @@ -145,17 +156,13 @@ BackupStoreRefCountDatabase::~BackupStoreRefCountDatabase() } std::string BackupStoreRefCountDatabase::GetFilename(const - BackupStoreAccountDatabase::Entry& rAccount, bool Temporary) + BackupStoreAccountDatabase::Entry& rAccount) { std::string RootDir = BackupStoreAccounts::GetAccountRoot(rAccount); ASSERT(RootDir[RootDir.size() - 1] == '/' || RootDir[RootDir.size() - 1] == DIRECTORY_SEPARATOR_ASCHAR); std::string fn(RootDir + REFCOUNT_FILENAME ".rdb"); - if(Temporary) - { - fn += "X"; - } RaidFileController &rcontroller(RaidFileController::GetController()); RaidFileDiscSet rdiscSet(rcontroller.GetDiscSet(rAccount.GetDiscSet())); return RaidFileUtil::MakeWriteFileName(rdiscSet, fn); @@ -164,48 +171,85 @@ std::string BackupStoreRefCountDatabase::GetFilename(const // -------------------------------------------------------------------------- // // Function -// Name: BackupStoreRefCountDatabase::Create(int32_t, -// const std::string &, int, bool) +// Name: BackupStoreRefCountDatabase::Create( +// const BackupStoreAccountDatabase::Entry& rAccount) // Purpose: Create a blank database, using a temporary file that // you must Discard() or Commit() to make permanent. // Created: 2003/08/28 // // -------------------------------------------------------------------------- std::auto_ptr - BackupStoreRefCountDatabase::Create - (const BackupStoreAccountDatabase::Entry& rAccount) +BackupStoreRefCountDatabase::Create(const BackupStoreAccountDatabase::Entry& rAccount) { - // Initial header - refcount_StreamFormat hdr; - hdr.mMagicValue = htonl(REFCOUNT_MAGIC_VALUE); - hdr.mAccountID = htonl(rAccount.GetID()); - - std::string Filename = GetFilename(rAccount, true); // temporary + std::string Filename = GetFilename(rAccount); + return Create(Filename, rAccount.GetID()); +} +// -------------------------------------------------------------------------- +// +// Function +// Name: BackupStoreRefCountDatabase::Create( +// const std::string& Filename, int64_t AccountID, +// bool reuse_existing_file) +// Purpose: Create a blank database, using a temporary file that +// you must Discard() or Commit() to make permanent. +// Be careful with reuse_existing_file, because it +// makes it easy to bypass the restriction of only one +// (committable) temporary database at a time, and to +// accidentally overwrite the main DB. +// Created: 2003/08/28 +// +// -------------------------------------------------------------------------- +std::auto_ptr +BackupStoreRefCountDatabase::Create(const std::string& Filename, int64_t AccountID, + bool reuse_existing_file) +{ // Open the file for writing - if (FileExists(Filename)) + std::string temp_filename = Filename + (reuse_existing_file ? "" : "X"); + int flags = O_CREAT | O_BINARY | O_RDWR | O_EXCL; + + if(FileExists(temp_filename)) { - BOX_WARNING(BOX_FILE_MESSAGE(Filename, "Overwriting existing " - "temporary reference count database")); - if (EMU_UNLINK(Filename.c_str()) != 0) + if(reuse_existing_file) + { + // Don't warn, and don't fail because the file already exists. This allows + // creating a temporary file securely with mkstemp() and then opening it as + // a refcount database, avoiding a race condition. + flags &= ~O_EXCL; + } + else { - THROW_SYS_FILE_ERROR("Failed to delete old temporary " - "reference count database file", Filename, - CommonException, OSFileError); + BOX_WARNING(BOX_FILE_MESSAGE(temp_filename, "Overwriting existing " + "temporary reference count database")); + if(EMU_UNLINK(temp_filename.c_str()) != 0) + { + THROW_SYS_FILE_ERROR("Failed to delete old temporary " + "reference count database file", temp_filename, + CommonException, OSFileError); + } } } - int flags = O_CREAT | O_BINARY | O_RDWR | O_EXCL; - std::auto_ptr DatabaseFile(new FileStream(Filename, flags)); +#ifdef BOX_OPEN_LOCK + flags |= BOX_OPEN_LOCK; +#endif + std::auto_ptr database_file(new FileStream(temp_filename, flags)); // Write header - DatabaseFile->Write(&hdr, sizeof(hdr)); + refcount_StreamFormat hdr; + hdr.mMagicValue = htonl(REFCOUNT_MAGIC_VALUE); + hdr.mAccountID = htonl(AccountID); + database_file->Write(&hdr, sizeof(hdr)); // Make new object - std::auto_ptr refcount( - new BackupStoreRefCountDatabase(rAccount, false, true, - DatabaseFile)); - + BackupStoreRefCountDatabase* p_impl = new BackupStoreRefCountDatabase( + Filename, AccountID, + false, // ReadOnly + !reuse_existing_file, // PotentialDB + reuse_existing_file, // TemporaryDB + database_file); + std::auto_ptr refcount(p_impl); + // The root directory must always have one reference for a database // to be valid, so set that now on the new database. This will leave // mIsModified set to true. @@ -231,7 +275,7 @@ std::auto_ptr BackupStoreRefCountDatabase::Load( { // Generate the filename. Cannot open a temporary database, so it must // be a permanent one. - std::string Filename = GetFilename(rAccount, false); + std::string Filename = GetFilename(rAccount); int flags = ReadOnly ? O_RDONLY : O_RDWR; // Open the file for read/write @@ -258,9 +302,11 @@ std::auto_ptr BackupStoreRefCountDatabase::Load( // Make new object std::auto_ptr refcount( - new BackupStoreRefCountDatabase(rAccount, ReadOnly, false, + new BackupStoreRefCountDatabase(Filename, rAccount.GetID(), ReadOnly, + false, // PotentialDB + false, // TemporaryDB dbfile)); - + // return it to caller return refcount; } diff --git a/lib/backupstore/BackupStoreRefCountDatabase.h b/lib/backupstore/BackupStoreRefCountDatabase.h index 48aba4d03..5a5edd380 100644 --- a/lib/backupstore/BackupStoreRefCountDatabase.h +++ b/lib/backupstore/BackupStoreRefCountDatabase.h @@ -59,8 +59,8 @@ class BackupStoreRefCountDatabase ~BackupStoreRefCountDatabase(); private: // Creation through static functions only - BackupStoreRefCountDatabase(const BackupStoreAccountDatabase::Entry& - rAccount, bool ReadOnly, bool Temporary, + BackupStoreRefCountDatabase(const std::string& Filename, int64_t AccountID, + bool ReadOnly, bool PotentialDB, bool TemporaryDB, std::auto_ptr apDatabaseFile); // No copying allowed BackupStoreRefCountDatabase(const BackupStoreRefCountDatabase &); @@ -70,6 +70,8 @@ class BackupStoreRefCountDatabase // Discard() or Commit() to make permanent. static std::auto_ptr Create (const BackupStoreAccountDatabase::Entry& rAccount); + static std::auto_ptr Create + (const std::string& Filename, int64_t AccountID, bool reuse_existing_file = false); void Commit(); void Discard(); @@ -90,8 +92,7 @@ class BackupStoreRefCountDatabase int ReportChangesTo(BackupStoreRefCountDatabase& rOldRefs); private: - static std::string GetFilename(const BackupStoreAccountDatabase::Entry& - rAccount, bool Temporary); + static std::string GetFilename(const BackupStoreAccountDatabase::Entry& rAccount); IOStream::pos_type GetSize() const { @@ -110,16 +111,17 @@ class BackupStoreRefCountDatabase void SetRefCount(int64_t ObjectID, refcount_t NewRefCount); // Location information - BackupStoreAccountDatabase::Entry mAccount; - std::string mFilename; - bool mReadOnly; - bool mIsModified; - bool mIsTemporaryFile; - std::auto_ptr mapDatabaseFile; + int64_t mAccountID; + std::string mFilename, mFinalFilename; + bool mReadOnly; + bool mIsModified; + bool mIsPotentialDB; + bool mIsTemporaryDB; + std::auto_ptr mapDatabaseFile; bool NeedsCommitOrDiscard() { - return mapDatabaseFile.get() && mIsModified && mIsTemporaryFile; + return mapDatabaseFile.get() && mIsModified && mIsPotentialDB; } };