blockstorage: do not flush block to disk if it is already there

test: ensure we can reindex from read-only block files now

src/node/blockstorage.cpp

@@ -906,19 +906,19 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne
         if (!fKnown) {
             LogPrint(BCLog::BLOCKSTORAGE, "Leaving block file %i: %s (onto %i) (height %i)\n",
                 last_blockfile, m_blockfile_info[last_blockfile].ToString(), nFile, nHeight);
-        }
 
-        // Do not propagate the return code. The flush concerns a previous block
-        // and undo file that has already been written to. If a flush fails
-        // here, and we crash, there is no expected additional block data
-        // inconsistency arising from the flush failure here. However, the undo
-        // data may be inconsistent after a crash if the flush is called during
-        // a reindex. A flush error might also leave some of the data files
-        // untrimmed.
-        if (!FlushBlockFile(last_blockfile, !fKnown, finalize_undo)) {
-            LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning,
-                          "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n",
-                          last_blockfile, !fKnown, finalize_undo, nFile);
+            // Do not propagate the return code. The flush concerns a previous block
+            // and undo file that has already been written to. If a flush fails
+            // here, and we crash, there is no expected additional block data
+            // inconsistency arising from the flush failure here. However, the undo
+            // data may be inconsistent after a crash if the flush is called during
+            // a reindex. A flush error might also leave some of the data files
+            // untrimmed.
+            if (!FlushBlockFile(last_blockfile, !fKnown, finalize_undo)) {
+                LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning,
+                              "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n",
+                              last_blockfile, !fKnown, finalize_undo, nFile);
+            }
         }
         // No undo data yet in the new file, so reset our undo-height tracking.
         m_blockfile_cursors[chain_type] = BlockfileCursor{nFile};

test/functional/feature_reindex_readonly.py

@@ -24,6 +24,7 @@ class BlockstoreReindexTest(BitcoinTestFramework):
         opreturn = "6a"
         nulldata = fastprune_blockfile_size * "ff"
         self.generateblock(self.nodes[0], output=f"raw({opreturn}{nulldata})", transactions=[])
+        block_count = self.nodes[0].getblockcount()
         self.stop_node(0)
 
         assert (self.nodes[0].chain_path / "blocks" / "blk00000.dat").exists()
@@ -73,10 +74,10 @@ class BlockstoreReindexTest(BitcoinTestFramework):
                 pass
 
         if undo_immutable:
-            self.log.info("Attempt to restart and reindex the node with the unwritable block file")
-            with self.nodes[0].assert_debug_log(expected_msgs=['FlushStateToDisk', 'failed to open file'], unexpected_msgs=[]):
-                self.nodes[0].assert_start_raises_init_error(extra_args=['-reindex', '-fastprune'],
-                    expected_msg="Error: A fatal internal error occurred, see debug.log for details")
+            self.log.debug("Attempt to restart and reindex the node with the unwritable block file")
+            with self.nodes[0].wait_for_debug_log([b"Reindexing finished"]):
+                self.start_node(0, extra_args=['-reindex', '-fastprune'])
+            assert block_count == self.nodes[0].getblockcount()
             undo_immutable()
 
         filename.chmod(0o777)