Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Check disk space before downloading files. #1520

Merged
merged 1 commit into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions src/libaktualizr/package_manager/fetcher_test.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <gtest/gtest.h>

#include <sys/statvfs.h>
#include <chrono>
#include <future>
#include <iostream>
Expand Down Expand Up @@ -272,6 +273,43 @@ TEST(Fetcher, DownloadLengthZero) {
EXPECT_EQ(http->counter, 1);
}

/* Don't bother downloading a target that is larger than the available disk
* space. */
TEST(Fetcher, NotEnoughDiskSpace) {
TemporaryDirectory temp_dir;
config.storage.path = temp_dir.Path();
config.uptane.repo_server = server;

std::shared_ptr<INvStorage> storage(new SQLStorage(config.storage, false));
auto http = std::make_shared<HttpZeroLength>(temp_dir.Path());
auto pacman = std::make_shared<PackageManagerFake>(config.pacman, config.bootloader, storage, http);
KeyManager keys(storage, config.keymanagerConfig());
Uptane::Fetcher fetcher(config, http);

// Find how much space is available on disk.
struct statvfs stvfsbuf {};
EXPECT_EQ(statvfs(temp_dir.Path().c_str(), &stvfsbuf), 0);
const uint64_t available_bytes = (stvfsbuf.f_bsize * stvfsbuf.f_bavail);

// Try to fetch a target larger than the available disk space: an exception is
// thrown and the http module is never called.
Json::Value empty_target_json;
empty_target_json["hashes"]["sha256"] = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
empty_target_json["length"] = available_bytes * 2;
Uptane::Target empty_target("empty_file", empty_target_json);
EXPECT_THROW(pacman->fetchTarget(empty_target, fetcher, keys, progress_cb, nullptr), std::runtime_error);
EXPECT_EQ(http->counter, 0);

// Try to fetch a 1-byte target: download succeeds, and http module is called.
// This is done purely to make sure the test is designed correctly.
Json::Value nonempty_target_json;
nonempty_target_json["hashes"]["sha256"] = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
nonempty_target_json["length"] = 1;
Uptane::Target nonempty_target("fake_file", nonempty_target_json);
EXPECT_TRUE(pacman->fetchTarget(nonempty_target, fetcher, keys, progress_cb, nullptr));
EXPECT_EQ(http->counter, 1);
}

#ifndef __NO_MAIN__
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
Expand Down
5 changes: 5 additions & 0 deletions src/libaktualizr/package_manager/packagemanagerinterface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ bool PackageManagerInterface::fetchTarget(const Uptane::Target& target, Uptane::
ds->fhandle = storage_->allocateTargetFile(false, target);
}

const uint64_t required_bytes = target.length() - ds->downloaded_length;
if (!storage_->checkAvailableDiskSpace(required_bytes)) {
throw std::runtime_error("Insufficient disk space available to download target");
}

std::string target_url = target.uri();
if (target_url.empty()) {
target_url = fetcher.getRepoServer() + "/targets/" + Utils::urlEncode(target.filename());
Expand Down
1 change: 1 addition & 0 deletions src/libaktualizr/storage/invstorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ class INvStorage {
virtual void saveEcuReportCounter(const Uptane::EcuSerial& ecu_serial, int64_t counter) = 0;
virtual bool loadEcuReportCounter(std::vector<std::pair<Uptane::EcuSerial, int64_t>>* results) = 0;

virtual bool checkAvailableDiskSpace(uint64_t required_bytes) const = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one minor remark, regarding the "checkAvailableDiskSpace" function, is it really necessary to make this function pure virtual in the "invstorage" interface?

I suspect this function will be called quite often, does it make sense to increase virtual table?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I have a choice here. We only have one fully functional storage implementation at present, but I think this depends on the specific implementation. For SQL, we store the files on disk in the same directory as the database, so I know I can check the filesystem that all that lives on. But a different implementation might do something else, like use multiple partitions, and then my current logic would have to be adapted to that.

virtual boost::optional<std::pair<size_t, std::string>> checkTargetFile(const Uptane::Target& target) const = 0;

// Incremental file API
Expand Down
20 changes: 20 additions & 0 deletions src/libaktualizr/storage/sqlstorage.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "sqlstorage.h"

#include <sys/stat.h>
#include <sys/statvfs.h>
#include <iostream>
#include <map>
#include <memory>
Expand Down Expand Up @@ -1396,6 +1397,25 @@ void SQLStorage::clearInstallationResults() {
db.commitTransaction();
}

bool SQLStorage::checkAvailableDiskSpace(const uint64_t required_bytes) const {
struct statvfs stvfsbuf {};
const int stat_res = statvfs(dbPath().c_str(), &stvfsbuf);
if (stat_res < 0) {
LOG_WARNING << "Unable to read filesystem statistics: error code " << stat_res;
return true;
}
const uint64_t available_bytes = (stvfsbuf.f_bsize * stvfsbuf.f_bavail);
const uint64_t reserved_bytes = 1 << 20;

if (required_bytes + reserved_bytes < available_bytes) {
return true;
} else {
LOG_ERROR << "Insufficient disk space available to download target! Required: " << required_bytes
<< ", available: " << available_bytes << ", reserved: " << reserved_bytes;
return false;
}
}

boost::optional<std::pair<size_t, std::string>> SQLStorage::checkTargetFile(const Uptane::Target& target) const {
SQLite3Guard db = dbConnection();

Expand Down
2 changes: 2 additions & 0 deletions src/libaktualizr/storage/sqlstorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class SQLStorage : public SQLStorageBase, public INvStorage {
bool loadEcuReportCounter(std::vector<std::pair<Uptane::EcuSerial, int64_t>>* results) override;
void clearInstallationResults() override;

bool checkAvailableDiskSpace(uint64_t required_bytes) const override;

std::unique_ptr<StorageTargetWHandle> allocateTargetFile(bool from_director, const Uptane::Target& target) override;
std::unique_ptr<StorageTargetRHandle> openTargetFile(const Uptane::Target& target) override;
boost::optional<std::pair<size_t, std::string>> checkTargetFile(const Uptane::Target& target) const override;
Expand Down