Add a check for free disk space at first startup. #15848

pull darosior wants to merge 1 commits into bitcoin:master from darosior:check_free_disk_size changing 2 files +19 −8

darosior commented at 8:32 PM on April 18, 2019: member

As mentioned in #15813, there is a check for available disk space at startup in bitcoin-qt. This PR adds it to bitcoind too, at least for the first startup.

in src/init.cpp:1680 in 138bf9a1d2 outdated

1676 | @@ -1677,12 +1677,19 @@ bool AppInitMain(InitInterfaces& interfaces)
1677 |      }
1678 |  
1679 |      // ********************************************************* Step 11: import blocks
1680 | -
1681 | +    uint64_t bytes_needed = 0;

promag commented at 11:01 AM on April 20, 2019:

nit, rename to additional_bytes_needed.

in src/init.cpp:1684 in 138bf9a1d2 outdated
```
1676 | @@ -1677,12 +1677,19 @@ bool AppInitMain(InitInterfaces& interfaces)
1677 |      }
1678 |  
1679 |      // ********************************************************* Step 11: import blocks
1680 | -
1681 | +    uint64_t bytes_needed = 0;
1682 | +    if (!chainActive.Height()) {
1683 | +        // If first startup
1684 | +        if (fPruneMode)
1685 | +            bytes_needed = nPruneTarget;
```
promag commented at 11:02 AM on April 20, 2019:

Doesn't look like nPruneTarget is in bytes.

darosior commented at 1:57 PM on April 20, 2019:

nPruneTarget is in bytes : https://github.com/darosior/bitcoin/blob/138bf9a1d2d198410b1b15d3c285c2b12a200c3f/src/init.cpp#L1066-L1071

promag commented at 8:08 AM on April 21, 2019:

So this comment should be fixed:

https://github.com/bitcoin/bitcoin/blob/56376f336548b53cf31e98a58dfb4db22cede6e5/src/validation.h#L186-L187

darosior commented at 4:23 PM on April 21, 2019:

Indeed.
in src/init.cpp:1681 in 138bf9a1d2 outdated
```
1676 | @@ -1677,12 +1677,19 @@ bool AppInitMain(InitInterfaces& interfaces)
1677 |      }
1678 |  
1679 |      // ********************************************************* Step 11: import blocks
1680 | -
1681 | +    uint64_t bytes_needed = 0;
1682 | +    if (!chainActive.Height()) {
```
promag commented at 11:05 AM on April 20, 2019:

I don't think this is the right check for first run. I think the best one is to check if data dir existed.

darosior commented at 2:02 PM on April 20, 2019:

Why ? data dir could exist without any block downloaded and it would be necessary to check for available disk space too.

promag commented at 3:05 PM on April 20, 2019:

Ok. But note that genesis is being loaded asynchronously.
darosior commented at 5:58 PM on April 23, 2019:

Maybe

if (chainActive.Height() <= 1)

would be better then ?
promag commented at 11:06 AM on April 20, 2019: member

Concept ACK.
jamesob commented at 6:09 PM on April 23, 2019: member

If this is actually something we want to do, we should also include a flag that disables this check since there are situations where users may want to run bitcoind without expecting to complete a full sync, e.g. when benchmarking.
darosior force-pushed on Apr 23, 2019
darosior commented at 6:13 PM on April 23, 2019: member

@promag I addressed the changes (renamed bytes_needed and check if height is <= 1 instead of 0), thank you for your review. Edit: changed also the misleading comment in validation.h
darosior commented at 6:18 PM on April 23, 2019: member

@jamesob could we make it an argument, like -dontcheckdiskpace ?
MarcoFalke commented at 6:23 PM on April 23, 2019: member

Does this work with reindex?
darosior force-pushed on Apr 23, 2019
DrahtBot added the label Validation on Apr 23, 2019
Add a check for free disk space at first startup + fix a wrong comment in validation.h 776f66b33c
darosior force-pushed on Apr 23, 2019
darosior commented at 7:00 PM on April 23, 2019: member

@MarcoFalke this caused a segfault with -reindex, I added a check for fReindex.

in src/init.cpp:1694 in 776f66b33c

1696 | +                additional_bytes_needed = chainparams.AssumedBlockchainSize() * 1024 * 1024 * 1024;
1697 | +        }
1698 | +        if (!CheckDiskSpace(GetDataDir())) {
1699 | +            InitError(strprintf(_("Error: Disk space is low for %s"), GetDataDir()));
1700 | +            return false;
1701 | +        }

Empact commented at 9:38 AM on April 27, 2019:

May as well leave this check out of the locked section, as it has no need for the related info.

darosior commented at 12:43 PM on May 2, 2019:

Thank you for the review. I tested outside of the lock and it makes bitcoind hanging forever on error.

luke-jr commented at 5:40 AM on May 1, 2019: member

IMO this is too much hand-holding for a server.
darosior renamed this:
~~Add a check for free disk space at first startup. ref #15813~~
Add a check for free disk space at first startup.
on May 2, 2019
DrahtBot commented at 1:49 AM on June 17, 2019: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #15340 (gui: Introduce bilingual GUI error messages by hebasto)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
darosior closed this on Jun 17, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#15813 Should we check free disk size before syncing blocks #25299 doc: Correct comments re. units of constants #25315 Add warning on first startup if free disk space is less than necessary

Add a check for free disk space at first startup. #15848

Conflicts