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

Conversation

pattivacek
Copy link
Collaborator

There is no reason to download a file that we don't have space for on disk. I've set 1MB as reserved so that we can still (hopefully) do the database operations to let libaktualizr send a manifest to the server.

This should help with the problems observed with non-OSTree images (see advancedtelematic/meta-updater#637) when additional overhead isn't provided to the qemu image.

@codecov-io
Copy link

Codecov Report

Merging #1520 into master will increase coverage by 0.11%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1520      +/-   ##
==========================================
+ Coverage   80.74%   80.85%   +0.11%     
==========================================
  Files         184      184              
  Lines       11181    11195      +14     
==========================================
+ Hits         9028     9052      +24     
+ Misses       2153     2143      -10
Impacted Files Coverage Δ
src/libaktualizr/storage/sqlstorage.h 50% <ø> (ø) ⬆️
src/libaktualizr/storage/invstorage.h 97.56% <ø> (ø) ⬆️
...tualizr/package_manager/packagemanagerinterface.cc 94.28% <100%> (+0.16%) ⬆️
src/libaktualizr/storage/sqlstorage.cc 76.62% <81.81%> (+0.28%) ⬆️
src/libaktualizr/storage/sqlstorage_base.cc 79.72% <0%> (+5.4%) ⬆️
src/libaktualizr/storage/sqlstorage_base.h 100% <0%> (+40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 635962f...4bcf603. Read the comment docs.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #1520 into master will increase coverage by 0.21%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1520      +/-   ##
==========================================
+ Coverage   80.74%   80.95%   +0.21%     
==========================================
  Files         184      184              
  Lines       11181    11196      +15     
==========================================
+ Hits         9028     9064      +36     
+ Misses       2153     2132      -21
Impacted Files Coverage Δ
src/libaktualizr/storage/sqlstorage.h 50% <ø> (ø) ⬆️
src/libaktualizr/storage/invstorage.h 97.56% <ø> (ø) ⬆️
...tualizr/package_manager/packagemanagerinterface.cc 94.28% <100%> (+0.16%) ⬆️
src/libaktualizr/storage/sqlstorage.cc 77.42% <83.33%> (+1.08%) ⬆️
src/libaktualizr/primary/sotauptaneclient.cc 89.09% <0%> (+0.27%) ⬆️
src/libaktualizr/package_manager/ostreemanager.cc 80.14% <0%> (+1.12%) ⬆️
src/libaktualizr/storage/sql_utils.h 86.11% <0%> (+1.38%) ⬆️
src/libaktualizr/storage/sqlstorage_base.cc 78.37% <0%> (+4.05%) ⬆️
src/libaktualizr/storage/sqlstorage_base.h 100% <0%> (+40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 635962f...d56dd03. Read the comment docs.

@@ -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.

@pattivacek pattivacek merged commit 6112216 into master Jan 17, 2020
@pattivacek pattivacek deleted the fix/OTA-4308/check-disk-space branch January 17, 2020 07:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants