Closed t-w closed 3 months ago
ad. 2. In fact, the current code seems to be fine. In adfSetBlockUsed/Free()
, it marks the status of each allocated/deallocated block to "changed". Then, adfUpdateBitmap()
, before updating the bitmap blocks, sets BM_INVALID
flag in root block, updates the bitmap writing bitmap blocks that changed, and then, if all is successful, sets BM_VALID
in root block.
BUT! If there was some change written to the disk, which changed the block allocation, and root block for some reason is not read (there is an error) in adfUpdateBitmap()
, after which the function aborts(!), there is a situation where there are changes in the blocks allocation done, but neither block allocation bitmap is updated nor the rootblock's bitmap validity flag set to invalid. What, further, can lead to data corruption due to use of not up-to-date bitmap.
To minimize this possibility, the invalid flag should be set (and written to the disk!) immediately after any change is done in regards of block allocation.
However, the current code does not keep the rootblock contests in memory, it would be necessary to read it (and, if necessary, write) each time block allocation/deallocation occurs.
So either additional flag has to be added to the internal bitmap structure (to keep this information), or the rootblock should be kept in the memory all the time (and written when necessary).
ad. 2 (cont.) In general, there seem to be 3 ways of minimizing the possibility of occurring the case when the bitmap on the disk is invalid and the bitmap invalidity flag in root block is not set as invalid:
Not sure which way is better... so comments welcomed.
ad. 2 (cont.) Some pros and cons:
At the moment, I am leaning towards 3. - so just ignore bitmap blocks and its data completely (do not read or write it) and just make sure that bitmap allocation data is set as invalid in root block and let (eventually) a real Amiga OS rebuild it (or, eventually, update the bitmap only when the volume is being unmounted, and then mark it as "valid").
Kind of a related matter - I haven't found any way that an OFS or FFS volume could have bad blocks marked (except maybe a special file named eg. BADBLOCKS, which would have all the bad blocks assigned... some utilities seem to do that: https://forum.amiga.org/index.php?topic=1068.0 https://comp.sys.amiga.hardware.narkive.com/S0P0ALdP/mark-bad-block-in-hdd ) But in case something like this existed - to consider along with the things discussed here...
For the moment, the code works as follows (after the merge above):
adfReconstructBitmap()
and is done in-memory, it is not written until other write operations (so as before) or an explicit call to adfUpdateBitmap()
Eventual further improvement would be to somehow split volume mount with or without filesystem. In the first case, the bitmap (as any other volume metadata) would not be read or used (to support a custom data layout, just on the volume blocks level).
Reviewing some pieces of bitmap allocation code (for #63) revealed a couple of potential issues:
(in)validity flag (of the bitmap) is not checked while mounting a volume
the use of the (in)validity flag is limited to only
adfUpdateBitmap
(one function), while it seems is should be used to mark "dirty" bitmap while writing any block (data, directory, link) until the bitmap is updated accordingly, as the block can be (de)allocated by those operations (meaning the bitmap will be invalid if the operation gets interrupted for some reason)(All this have to be checked further, this is just a brief note for now).