Fail AppInitMain if either disk space check fails #15124

pull Empact wants to merge 1 commits into bitcoin:master from Empact:disk-space-check changing 1 files +7 −1

Empact commented at 6:58 PM on January 7, 2019: member

Rather than both.

Introduced in 386a6b62a8a1db9dd0f354cb95b7585f555c7e5d, #12653
Empact renamed this:
~~Fail if either disk space check fails~~
Fail AppInitMain if either disk space check fails
on Jan 7, 2019
promag commented at 11:23 PM on January 7, 2019: member

I think it's reasonable to split the expression to log which directory needs more space.
jimpo commented at 2:18 AM on January 8, 2019: contributor

utACK 11e85ca

in src/init.cpp:1634 in 11e85cab3c outdated

1630 | @@ -1631,7 +1631,7 @@ bool AppInitMain(InitInterfaces& interfaces)
1631 |  
1632 |      // ********************************************************* Step 11: import blocks
1633 |  
1634 | -    if (!CheckDiskSpace() && !CheckDiskSpace(0, true))
1635 | +    if (!CheckDiskSpace() || !CheckDiskSpace(0, true))

MarcoFalke commented at 8:29 AM on January 8, 2019:

Could use named arguments for literals?

Empact commented at 9:52 AM on January 8, 2019:

Done. #15118 clarifies these more so I'm partly deferring to that.

MarcoFalke commented at 8:29 AM on January 8, 2019: member

utACK 11e85ca
Empact force-pushed on Jan 8, 2019
Empact force-pushed on Jan 8, 2019

in src/init.cpp:1634 in 3a514fed17 outdated

1630 | @@ -1631,7 +1631,7 @@ bool AppInitMain(InitInterfaces& interfaces)
1631 |  
1632 |      // ********************************************************* Step 11: import blocks
1633 |  
1634 | -    if (!CheckDiskSpace() && !CheckDiskSpace(0, true))
1635 | +    if (!CheckDiskSpace() || !CheckDiskSpace(/* additional_bytes */ 0, /* blocks_dir */ true))

laanwj commented at 1:48 PM on January 8, 2019:

When you are changing this anyway: please provide explicit arguments to the first call too, instead of relying on default arguments

jamesob commented at 7:30 PM on January 8, 2019: member

Agree with @promag: while we're in the neighborhood, we should specify which disk is full.
Empact force-pushed on Jan 9, 2019

Fail if either disk space check fails

Rather than both.

Introduced in 386a6b62a8a1db9dd0f354cb95b7585f555c7e5d

ba8c8b2227

Empact force-pushed on Jan 9, 2019
Empact commented at 10:43 AM on January 9, 2019: member

Updated to address feedback.
promag commented at 11:54 AM on January 9, 2019: member

utACK ba8c8b22, thanks!
laanwj commented at 2:31 PM on January 9, 2019: member

utACK ba8c8b22272ad40fe2de465d7e745532bab48d3b
laanwj merged this on Jan 9, 2019
laanwj closed this on Jan 9, 2019
laanwj referenced this in commit 195d28fecb on Jan 9, 2019
Empact deleted the branch on Jan 9, 2019
MarcoFalke commented at 11:01 PM on January 9, 2019: member

Could add a test, if possible somehow?
Empact commented at 10:07 PM on January 10, 2019: member

I don't know of any way to set up the scenario as-is - could refactor for more testability with stubbing/mocking.
random-zebra referenced this in commit edfaec63c2 on May 1, 2021
PastaPastaPasta referenced this in commit c0fa59f214 on Jun 26, 2021
PastaPastaPasta referenced this in commit 69dd85bd3d on Jun 28, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

Linked (view graph)

#12653 Allow to optional specify the directory for the blocks storage