diff --git a/Programs/CosmOS/README.md b/Programs/CosmOS/README.md index 5477018..d76a186 100644 --- a/Programs/CosmOS/README.md +++ b/Programs/CosmOS/README.md @@ -761,9 +761,26 @@ A create can be refused for want of a run long enough even on a disk with plenty The ordering protects the original against **every way a save can fail while it is running**, and it is worth naming those, because they are the ones that actually happen: there is no run of free blocks long enough, or none at all; the disk refuses a block write; the name turns out to belong to a directory; the writer gives up part way through. In all of them the file that was already there is untouched, and what is lost is the temporary, which nothing had come to depend on yet. -It is **not** power-loss atomic, and nothing about SBFS claims it is. The commit is two block writes - delete the old entry, then give the temporary its name - and a machine that stops between them leaves the old file gone and the new one under the temporary's name. Both writes are to the directory, so `sbfs.part` or `sbfs.out` is sitting there holding every byte of the work; the data survives and the name does not, and putting it right is one `rename` typed by hand. +It is **not** power-loss atomic, and nothing about SBFS claims it is. The commit is two block writes - delete the old entry, then give the temporary its name - and a machine that stops between them leaves the old file gone and the new one under the temporary's name. Both writes are to the directory, so `sbfs.part` or `sbfs.out` is sitting there holding every byte of the work; the data survives and the name does not, and putting it right is one `rename` typed by hand. `dir` marks it `` so that it can be found, which is the whole of the recovery this format offers. -Closing that window means a journal or a second copy of the directory, and both are a great deal of machinery to buy back a two-write gap on a machine with no power failures to speak of. The honest description is the one to write down: **safe against the failures of ordinary operation, not against the machine stopping.** A future consistency check at boot could reclaim an abandoned temporary, and would want the entry to say it is one rather than to be recognised by its name. +Closing that window means a journal or a second copy of the directory, and both are a great deal of machinery to buy back a two-write gap on a machine with no power failures to speak of. The honest description is the one to write down: **safe against the failures of ordinary operation, not against the machine stopping.** + +#### What tells a temporary from a file: + +While the save runs the temporary is an ordinary entry in every way that matters - it holds real blocks and answers to a name - and the only thing that makes it different is that nobody has committed it yet. That is **not a property of its contents.** The same bytes become the finished file the instant the rename lands, so there is nothing to put inside it that would be true. It belongs in the entry, which is the thing the commit changes, and it is **flag bit `0x04`**. + +It used to be told apart by being *called* `sbfs.part` or `sbfs.out`, and those are names anybody is entitled to give a file of their own. Starting a save deleted whatever answered to one, as stale scratch - so saving anything at all in a directory destroyed your file of that name there, silently, and the first you would know of it is going to look for it. A file that does not carry the flag now belongs to somebody, and the save is **refused** rather than helping itself to the name. + +The same bit is what makes an interrupted save recoverable. Both listings show an unfinished write rather than sizing it, because the size in the entry is the room it asked for and not what was written into it: + +``` +> dir +stranded.txt +``` + +Rename it to keep the data, delete it to give the blocks back. Nothing reclaims it on its own; a boot-time consistency check could, and this is the field it would read. + +**A committed file never carries the bit**, so a disk this writes is byte for byte the disk the older code wrote - the agreement tests compare whole images and say so. Only the wreckage of a save that stopped looks different, and code that has never heard of the flag reads that as an ordinary file, which is exactly what it did before. Finding room is a walk through the directory rather than a lookup, because there is no allocation table. With files laid down contiguously the directory already says which blocks are spoken for, and a second copy of that would be a second thing to keep right. The free count in the superblock is kept up to date but it is a note rather than the truth: it can be worked out again from the directory, and the directory is the one to believe. diff --git a/Programs/CosmOS/Source/cosmos.asm b/Programs/CosmOS/Source/cosmos.asm index 252e063..2267633 100644 --- a/Programs/CosmOS/Source/cosmos.asm +++ b/Programs/CosmOS/Source/cosmos.asm @@ -356,6 +356,19 @@ dirCheck: MVQA CALL printSpaces + ; A save that stopped before it committed says so instead of saying a size, because the + ; size it holds is the room it asked for rather than what was written into it. Asked + ; first, since it is the one thing here that is not really a file yet. + ; + ; IT IS SHOWN RATHER THAN HIDDEN, and that is the whole of the recovery this format + ; offers: the bytes are all there under that name, so seeing it is what lets somebody + ; rename it back. Left off the listing it would be blocks nobody could account for. + SETD.0 SbfsFoundFlags + LDA.0 + INIB 0x04 + AND + BNQ dirUnfinished + ; A directory says so instead of saying a size. It has no blocks, so the arithmetic ; below would call it a file of no bytes - which is a different thing that happens to ; look the same from here. @@ -406,6 +419,12 @@ dirInBlocks: CALL newLine BRI dirStep +dirUnfinished: + SETD.0 UnfinishedText + CALL printString + CALL newLine + BRI dirStep + dirIsDirectory: SETD.0 DirFolders LDA.0 @@ -3312,6 +3331,8 @@ Farewell: "halted" DirectoryText: "" +UnfinishedText: +"" BlocksText: " blocks" NotDirectory: diff --git a/Programs/CosmOS/Source/sbfs.asm b/Programs/CosmOS/Source/sbfs.asm index 372349c..8240e6f 100644 --- a/Programs/CosmOS/Source/sbfs.asm +++ b/Programs/CosmOS/Source/sbfs.asm @@ -9,6 +9,12 @@ ; between them but the written specification, so anything that changes here has to change ; there in the same breath. ; +; ---- What an entry's flags mean ---- +; +; 0x01 In use. A zero here is a free slot, whatever else the bytes hold. +; 0x02 A directory, which has no blocks at all. +; 0x04 A file being written and not yet committed. See sbfsSaveFile. +; ; The disk's buffer is registered as bank 3, and every block read lands there and is then ; blitted where it is wanted. The CPU never touches the buffer directly, because nothing ; can: memory a device brings is reachable only through the controller. @@ -1559,6 +1565,22 @@ sbfsCreate: BNQ sbfsCreateFailed sbfsCreateAt: + ; A plain file, unless somebody came in at the other door. Set here rather than left + ; over from last time, so that one temporary does not make the next ordinary file one. + INIA 0x01 + SETD.0 SbfsMakeFlags + STA.0 + BRI sbfsCreateGo + +; The same, for the half written file a save puts down before it dares touch the original. +; It is an ordinary entry in every other way - it holds blocks and answers to a name - and +; the flag is the whole of what says it is not finished. See sbfsSaveFile. +sbfsCreateTempAt: + INIA 0x05 + SETD.0 SbfsMakeFlags + STA.0 + +sbfsCreateGo: CALL sbfsFileExtent CALL sbfsAllocate BNQ sbfsCreateFailed @@ -1614,8 +1636,9 @@ sbfsCreateFill: ; DP2 is on the entry. Fill it in, then put the whole block back on the disk. PSHD.2 POPD.3 - INIA 0x01 - STA.3 ; In use. + SETD.0 SbfsMakeFlags + LDA.0 + STA.3 ; In use, and possibly not finished being written. PSHD.3 POPD.1 @@ -2450,11 +2473,17 @@ sbfsStreamStart: BNQ sbfsStreamNo sbfsStreamFresh: - ; A temporary left by a stream that did not finish would be in the way. Whether there was - ; one is not worth asking about, since either answer leads here. + ; A temporary left by a stream that did not finish would be in the way, so it goes - but + ; only if it really is one. See sbfsSaveFile: the flag says so and the name does not, + ; because "sbfs.out" is a name somebody may have chosen for themselves. CALL sbfsStreamTemp CALL sbfsScanFor BNQ sbfsStreamNoTemp + SETD.0 SbfsFoundFlags + LDA.0 + INIB 0x04 + AND + BRQ sbfsStreamNo ; Somebody's own file, under a name we wanted. Left alone. CALL sbfsWipeFound sbfsStreamNoTemp: @@ -2468,7 +2497,7 @@ sbfsStreamNoTemp: STA.1 CALL sbfsStreamTemp - CALL sbfsCreateAt + CALL sbfsCreateTempAt BNQ sbfsStreamNo ; WHERE THE TEMPORARY BEGINS, KEPT NOW. Nothing moves a file once it is made, so every @@ -2650,6 +2679,13 @@ sbfsStreamNoOld: CALL sbfsScanFor BNQ sbfsStreamNo + ; Finished: the flag goes down in the same block write that gives it its name and its + ; size. Flat rather than cleared, for the reason sbfsSaveFile gives. + PSHD.3 + POPD.1 + INIA 0x01 + STA.1 + PSHD.3 POPD.1 DPUP.1 0d06 @@ -2805,11 +2841,20 @@ sbfsSaveFile: BNQ sbfsSaveFailed sbfsSaveNotThere: - ; A temporary left behind by a save that did not finish would be in the way. Whether - ; there was one is not worth asking about, since either answer leads here. + ; A temporary left behind by a save that did not finish would be in the way, so it goes. + ; + ; WHAT MAKES IT ONE IS THE FLAG AND NOT THE NAME. "sbfs.part" is a name a person is + ; perfectly entitled to give a file of their own, and this used to delete whatever + ; answered to it - so saving anything at all, once, quietly destroyed that file. The + ; entry now says what it is, and something that is not ours stops the save instead. CALL sbfsSaveTemp CALL sbfsScanFor BNQ sbfsSaveNoTemp + SETD.0 SbfsFoundFlags + LDA.0 + INIB 0x04 + AND + BRQ sbfsSaveFailed ; Somebody's own file, under a name we wanted. Left alone. CALL sbfsWipeFound sbfsSaveNoTemp: @@ -2822,7 +2867,7 @@ sbfsSaveNoTemp: STA.1 CALL sbfsSaveTemp - CALL sbfsCreateAt + CALL sbfsCreateTempAt BNQ sbfsSaveFailed SETD.2 SbfsSaveData @@ -2852,6 +2897,14 @@ sbfsSaveNoOld: CALL sbfsScanFor BNQ sbfsSaveFailed + ; FINISHED, WHICH IS THE FLAG AND THE NAME TOGETHER. Written flat rather than by + ; clearing the one bit: a temporary is a file, so in use is the only other thing it can + ; ever have been, and the whole byte is known. + PSHD.3 + POPD.1 + INIA 0x01 + STA.1 + PSHD.3 POPD.1 DPUP.1 0d06 @@ -2987,6 +3040,11 @@ SbfsUpParent: SbfsFoundFlags: 0x00 +; What the next entry made is to be marked with. sbfsCreateAt sets it to a plain file and +; sbfsCreateTempAt to a temporary, so it is never read without having just been written. +SbfsMakeFlags: + 0x00 + ; Whether what is in SbfsFileStart and the rest really describes where the walk is now. ; Going up leaves them describing the entry it came from, and only the end of a path is ; close enough to care. diff --git a/Source/DiskTool/SplitDisk.c b/Source/DiskTool/SplitDisk.c index a868362..1172a1f 100644 --- a/Source/DiskTool/SplitDisk.c +++ b/Source/DiskTool/SplitDisk.c @@ -149,6 +149,13 @@ static int entryIsDirectory(const uint8_t *entry) { return (entry[SBFS_ENTRY_FLAGS] & SBFS_FLAG_DIRECTORY) != 0; } +// A save that was interrupted between writing its temporary and committing it. The blocks +// are genuinely spoken for - entryInUse says so, and the allocator must keep believing it +// - but nothing has claimed them under a name anybody asked for. +static int entryIsTemporary(const uint8_t *entry) { + return (entry[SBFS_ENTRY_FLAGS] & SBFS_FLAG_TEMPORARY) != 0; +} + // The index of the entry this one lives in, or -1 for the root. Stored as index plus one // so that a zeroed field - which is what every version one entry has - means the root. static int entryParent(const uint8_t *entry) { @@ -470,6 +477,7 @@ static int entryPath(const Directory *directory, int index, char *into, size_t r typedef struct { int files; int directories; + int temporaries; } Tally; // Prints one directory and everything under it, depth first and in entry order, which is @@ -498,9 +506,11 @@ static void listTree(const Directory *directory, int parent, char *prefix, size_ tally->directories++; listTree(directory, i, prefix, at + 1 + n, tally); } else { - printf("%-34s %8u %7u %7u\n", prefix, entrySize(entry), - readWord(entry + SBFS_ENTRY_START), entryBlocksUsed(entry)); + printf("%-34s %8u %7u %7u%s\n", prefix, entrySize(entry), + readWord(entry + SBFS_ENTRY_START), entryBlocksUsed(entry), + entryIsTemporary(entry) ? " unfinished" : ""); tally->files++; + tally->temporaries += entryIsTemporary(entry); } prefix[at] = '\0'; } @@ -522,7 +532,7 @@ static int commandList(const char *path, const char *within) { super.version); printf("%-34s %8s %7s %7s\n", "NAME", "BYTES", "START", "BLOCKS"); char prefix[SBFS_PATH_BYTES]; - Tally tally = { 0, 0 }; + Tally tally = { 0, 0, 0 }; int start = -1; if (within != NULL) { const char *why = NULL; @@ -573,6 +583,16 @@ static int commandList(const char *path, const char *within) { } printf(".\n"); + // A save that stopped between deleting the old entry and naming the new one. The + // bytes are all there under the temporary's name and one rename brings them back, + // which is the whole of the recovery this format offers - so the thing that matters + // is that a listing says so rather than showing a file with an odd name. + if (tally.temporaries > 0) { + printf("%d unfinished write%s: the blocks are held and the data is there, under" + " that name. Rename it to keep it, delete it to let the blocks go.\n", + tally.temporaries, tally.temporaries == 1 ? "" : "s"); + } + // Everything in use should have been reached by walking down from the root. Anything // that was not is pointing at a parent that is not there, or at itself, and that is // worth saying out loud rather than quietly leaving off the listing. diff --git a/Source/DiskTool/sbfs.h b/Source/DiskTool/sbfs.h index 03b7c12..33e03e3 100644 --- a/Source/DiskTool/sbfs.h +++ b/Source/DiskTool/sbfs.h @@ -97,6 +97,22 @@ #define SBFS_FLAG_IN_USE 0x01 #define SBFS_FLAG_DIRECTORY 0x02 +// A FILE BEING WRITTEN, WHICH IS NOT YET A FILE. Saving something that already exists is +// done by writing a temporary, deleting the original and giving the temporary its name, +// so that nothing is lost if the writing fails. The temporary has to be an ordinary entry +// while that happens - it holds real blocks and needs a name - and the only thing that +// distinguishes it from a finished file is that nobody has committed it yet. +// +// That is not a property of its contents. The same bytes become the real file the moment +// the rename lands, so there is nothing to put inside it that would be true; it belongs +// in the entry, which is the thing the commit changes. It was formerly told apart by +// being called "sbfs.part" or "sbfs.out", and those are legal names a user may also +// choose, so cleaning up by name could delete somebody's file. +// +// Cleared as part of committing. An entry still carrying it is the wreckage of a write +// that stopped, and its blocks are spoken for until something clears it up. +#define SBFS_FLAG_TEMPORARY 0x04 + // The root is not an entry. It is the absence of a parent, written as zero, which is why // the field is an index plus one and why a freshly zeroed entry is already in the root. #define SBFS_PARENT_ROOT 0 diff --git a/Tests/agree.sh b/Tests/agree.sh index eab5126..ad7f3c3 100755 --- a/Tests/agree.sh +++ b/Tests/agree.sh @@ -175,6 +175,103 @@ else report FAIL "native compare" "Compare did not call the copied files equal" fi +# ---- A temporary is what the entry says it is, not what it is called ---- +# +# Saving something writes a temporary first, and the temporary was told apart from a real +# file by being called "sbfs.part" or "sbfs.out". Those are legal names. A file of your own +# under either of them was deleted by the next save of anything at all in the same +# directory - so this puts one there with the host, saves with the machine, and takes it +# off again to see whether it survived. +# +# The entry now says outright that it is a temporary, and a file that does not say so +# belongs to somebody, so the save is refused instead of helping itself to the name. +# +# ONE DISK EACH, and that is not tidiness. Both on one disk, the first save ate sbfs.part +# and the streaming test's source file WAS sbfs.part - so with the guard removed the copy +# failed for want of a source, never opened a stream, and the check passed while reporting +# on nothing at all. +"$ASM" -I "$ROOT/Programs/CosmOS/Source" \ + "$ROOT/Programs/CosmOS/Apps/Files.asm" -o Files.sbx >/dev/null 2>&1 +printf 'this file is mine and is not scratch\n' > mine.txt + +"$TOOL" format part.img 512 4 >/dev/null +"$TOOL" mkdir part.img /Apps >/dev/null +"$TOOL" put part.img Files.sbx /Apps/Files.sbx >/dev/null +"$TOOL" put part.img mine.txt /sbfs.part >/dev/null +printf 'Files\nexit\n' | "$EMU" cosmos.bin --fast --disk part.img > part.txt 2>&1 +"$TOOL" get part.img /sbfs.part keptPart.txt >/dev/null 2>&1 +if cmp -s mine.txt keptPart.txt; then + report ok "a file called sbfs.part" "a whole-file save left it alone" +else + report FAIL "a file called sbfs.part" "saving a different file destroyed it" +fi +if grep -q "would not save" part.txt; then + report ok "and the save was refused" "it did not take a name it did not own" +else + report FAIL "and the save was refused" "the save reported success" +fi + +"$TOOL" format out.img 512 4 >/dev/null +"$TOOL" mkdir out.img /Apps >/dev/null +"$TOOL" put out.img Copy.sbx /Apps/Copy.sbx >/dev/null +"$TOOL" put out.img mine.txt /source.txt >/dev/null +"$TOOL" put out.img mine.txt /sbfs.out >/dev/null +printf 'Copy /source.txt /copied.txt\nexit\n' \ + | "$EMU" cosmos.bin --fast --cycles 100000000 --disk out.img > out.txt 2>&1 +"$TOOL" get out.img /sbfs.out keptOut.txt >/dev/null 2>&1 +if cmp -s mine.txt keptOut.txt; then + report ok "a file called sbfs.out" "a streamed write left it alone" +else + report FAIL "a file called sbfs.out" "streaming a different file destroyed it" +fi + +# The source is still there too, which says the refusal happened before anything was +# deleted rather than half way through. +"$TOOL" get out.img /source.txt keptSource.txt >/dev/null 2>&1 +if cmp -s mine.txt keptSource.txt; then + report ok "and it stopped early" "the source was never touched" +else + report FAIL "and it stopped early" "the copy got far enough to disturb the source" +fi + +# ---- And a write that stopped is visible from both sides ---- +# +# Nothing here can crash the machine half way through a commit, so the wreckage is forged: +# the flag is set by hand on a finished file, which is byte for byte what a save that was +# interrupted between writing its temporary and naming it would have left. Both listings +# have to say so, because the bytes are recoverable and only a listing can point at them. +"$TOOL" format wreck.img 512 4 >/dev/null +"$TOOL" put wreck.img mine.txt /stranded.txt >/dev/null +python3 - wreck.img <<'PATCH' +import sys +image = open(sys.argv[1], "r+b") +image.seek(8) +start = int.from_bytes(image.read(2), "big") # First directory block. +image.seek(start * 256) +directory = bytearray(image.read(256)) +for at in range(0, 256, 32): + if directory[at] & 0x01 and directory[at + 6:at + 14] == b"stranded": + directory[at] |= 0x04 # Never committed. + image.seek(start * 256) + image.write(directory) + break +else: + sys.exit("could not find the entry to strand") +image.close() +PATCH + +if "$TOOL" list wreck.img | grep -q "unfinished write"; then + report ok "the host sees the wreckage" "listed and explained" +else + report FAIL "the host sees the wreckage" "SplitDisk listed it as an ordinary file" +fi +if printf 'dir\nexit\n' | "$EMU" cosmos.bin --fast --disk wreck.img 2>&1 \ + | grep -q ""; then + report ok "the machine sees it too" "dir marks it rather than sizing it" +else + report FAIL "the machine sees it too" "dir showed it as an ordinary file" +fi + # ---- And each can read what the other wrote ---- # # Matching bytes and being readable are not the same claim. A field both of them write