some CheckDiskSpace() related changes #2068

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:CheckDiskSpace changing 1 files +19 −15

Diapolo commented at 6:58 AM on December 4, 2012: none
call CheckDiskSpace() before pre-allocating space
- even if we are allowed to fail pre-allocating, it's better to check for sufficient space before calling AllocateFileRange() and if we are out of disk space return with error()
- the above change allows us to remove the CheckDiskSpace() check in CBlock::AcceptBlock()
BitcoinPullTester commented at 7:22 AM on December 4, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/d29a6037cf283503646a3d02a1e46d507882b6e4 for binaries and test log.
ghost commented at 7:54 AM on December 4, 2012: none

This seems like a much better way to do it.

in src/main.cpp:None in d29a6037cf outdated

1913 | @@ -1914,11 +1914,13 @@ bool FindBlockPos(CDiskBlockPos &pos, unsigned int nAddSize, unsigned int nHeigh
1914 |          unsigned int nOldChunks = (pos.nPos + BLOCKFILE_CHUNK_SIZE - 1) / BLOCKFILE_CHUNK_SIZE;
1915 |          unsigned int nNewChunks = (infoLastBlockFile.nSize + BLOCKFILE_CHUNK_SIZE - 1) / BLOCKFILE_CHUNK_SIZE;
1916 |          if (nNewChunks > nOldChunks) {
1917 | -            FILE *file = OpenBlockFile(pos);
1918 | -            if (file) {
1919 | -                printf("Pre-allocating up to position 0x%x in blk%05u.dat\n", nNewChunks * BLOCKFILE_CHUNK_SIZE, pos.nFile);
1920 | -                AllocateFileRange(file, pos.nPos, nNewChunks * BLOCKFILE_CHUNK_SIZE - pos.nPos);
1921 | -                fclose(file);
1922 | +            if (CheckDiskSpace(nNewChunks * BLOCKFILE_CHUNK_SIZE - pos.nPos)) {

sipa commented at 1:00 PM on December 5, 2012:

In case there is not enough disk space to do pre-allocation, we probably want to fail gracefully, instead of not just preallocating.

Diapolo commented at 1:35 PM on December 5, 2012:

@sipa See below, we now return error() on a failed check.

in src/main.cpp:None in d29a6037cf outdated

2090 | @@ -2087,11 +2091,11 @@ bool CBlock::AcceptBlock(CDiskBlockPos *dbp)
2091 |      unsigned int nBlockSize = ::GetSerializeSize(*this, SER_DISK, CLIENT_VERSION);
2092 |      CDiskBlockPos blockPos;
2093 |      if (dbp == NULL) {
2094 | -        if (!CheckDiskSpace(::GetSerializeSize(*this, SER_DISK, CLIENT_VERSION)))

sipa commented at 1:01 PM on December 5, 2012:

If the CheckDiskSpae check is moved to FindBlockPos (ACK on that), I don't think we need it here anymore at all.

Diapolo commented at 1:19 PM on December 5, 2012:

Good catch, I'll update that.

Edit: Removed...

call CheckDiskSpace() before pre-allocating space

- even if we are allowed to fail pre-allocating, it's better to check
  for sufficient space before calling AllocateFileRange() and if we
  are out of disk space return with error()
- the above change allows us to remove the CheckDiskSpace() check
  in CBlock::AcceptBlock()

fa45c26a0e

BitcoinPullTester commented at 1:55 PM on December 5, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/fa45c26a0e5d3597814cef5ebd64e3db1b3b017d for binaries and test log.
sipa commented at 10:12 PM on December 5, 2012: member

ACK
laanwj commented at 7:23 AM on December 6, 2012: member

ACK
sipa referenced this in commit 622da5df6e on Dec 7, 2012
sipa merged this on Dec 7, 2012
sipa closed this on Dec 7, 2012
laudney referenced this in commit 523bfe2f5a on Mar 19, 2014
sidhujag referenced this in commit fbc94de4df on Jul 31, 2018
DrahtBot locked this on Sep 8, 2021

Contributors