Add a chunk reading API.

[sosthene-nitrokey]

An upstream PR will be prepared once trussed-dev#96 and trussed-dev/littlefs2#34 are merged.

[robin-nitrokey]

Looks good to me, just two notes:

• The commit contains some changes that look unrelated (WrapKeyData). Did you include them intentionally?
• I think the request and reply types should be called ReadFileChunk and WriteFileChunk for consistency.

[sosthene-nitrokey]

• The commit contains some changes that look unrelated (WrapKeyData). Did you include them intentionally?

Yes. Initially I used the SeekFrom type from littlefs2, which does not implement the serde traits. So I removed the serde traits from the macros that generate the API types, but that was used in WrapKey, so I introduced the new WrapKeyData type to get around the removal of the serde traits.

But since I ended up adding an OpenSeekFrom type in trussed, we can actually implement the serde traits for it.

Edit: done it.

[sosthene-nitrokey]

There's one remaining issue with this API:

If a client starts writing data to a file, it is immediately committed to the filesystem, even before all chunks have been written. This could lead to truncated data being stored.

[daringer]

So should we maybe add some safe-guard mechanism ?
Something like an empty file with the same filename + .write_lock, this would only be removed if the write was finished ?
On the other side, I do not see how this would improve things, the outcome will always be: you need to write the file again....

[sosthene-nitrokey]

So should we maybe add some safe-guard mechanism ? Something like an empty file with the same filename + .write_lock, this would only be removed if the write was finished ? On the other side, I do not see how this would improve things, the outcome will always be: you need to write the file again....

The issue is that its existence would need to be checked every time we read a file. So the solution would probably be to write the file to another directory /client_id/parts/... instead of /client_id/dat/..., and only once it has been flushed, move it to its "real" place?

[daringer]

This sounds better. Do we have a proper move available for littlefs2 ? (otherwise this would just move the problem to a point later in time, right?) overall this feels a little like preliminary optimization.

Looking on it from another side: We should anyways be more defensive about contents which are read from the filesystem. Essentially plausibility checks or similar mechanisms after reading data might be something the application should be responsible for and this would also catch such issues right ?

[sosthene-nitrokey]

There's a rename that works as a move.

We should anyways be more defensive about contents which are read from the filesystem

I don't understand what you mean? Checksums?

[daringer]

There's a rename that works as a move.

That's great, yeah then let's go for it, but I would suggest to not "waste" another directory for it but moreover just have something like <filename>.incomplete which will then be renamed ?

I don't understand what you mean? Checksums?

goood question :D checksums might be too heavy, maybe keeping filesizes ... haven't thought this through, but generally I would see the application being responsible to check if data read from the filesystem is not corrupt.

[sosthene-nitrokey]

The issue I have with filename.incomplete is that is would be readable from the standard read calls, and could also conflict with some clients in the future that probably wouldn't expect this to happen.

[robin-nitrokey]

What do you think about first writing to volatile and then copying chunk by chunk to the target storage? This would at least mean that syscalls are atomic. We could still use some renaming mechanism (leading dot?) to avoid file corruption if the final syscall is aborted e. g. due to power loss.

[sosthene-nitrokey]

I don't see what it would bring to do so. We could easily have some garbage collection on startup if you're afraid of a missed call to abort.

[robin-nitrokey]

It would slightly remove the required writes to the real FS and avoid the need for another directory.

[sosthene-nitrokey]

You would still need a directory in the volatile filesystem (one for each target filesystem). But indeed it would save the physical FS. I'll do that.