pw_kvs: Add free space accounting for write errors
Add accounting of byte written to flash when write operation has an error.
Change-Id: I4268cf36de16648fa10117fb6a828327685d4417
diff --git a/pw_kvs/alignment.cc b/pw_kvs/alignment.cc
index 90418e6..cba0e40 100644
--- a/pw_kvs/alignment.cc
+++ b/pw_kvs/alignment.cc
@@ -16,7 +16,7 @@
namespace pw {
-Status AlignedWriter::Write(span<const std::byte> data) {
+StatusWithSize AlignedWriter::Write(span<const std::byte> data) {
while (!data.empty()) {
size_t to_copy = std::min(write_size_ - bytes_in_buffer_, data.size());
@@ -26,16 +26,20 @@
// If the buffer is full, write it out.
if (bytes_in_buffer_ == write_size_) {
- if (auto result = output_.Write(buffer_, write_size_); !result.ok()) {
- return result.status();
+ StatusWithSize result = output_.Write(buffer_, write_size_);
+
+ // Always use write_size_ for the bytes written. If there was an error
+ // assume the space was written or at least disturbed.
+ bytes_written_ += write_size_;
+ if (!result.ok()) {
+ return StatusWithSize(result.status(), bytes_written_);
}
- bytes_written_ += write_size_;
bytes_in_buffer_ = 0;
}
}
- return Status::OK;
+ return StatusWithSize(bytes_written_);
}
StatusWithSize AlignedWriter::Flush() {
diff --git a/pw_kvs/alignment_test.cc b/pw_kvs/alignment_test.cc
index d5bf620..542428d 100644
--- a/pw_kvs/alignment_test.cc
+++ b/pw_kvs/alignment_test.cc
@@ -138,23 +138,23 @@
AlignedWriterBuffer<32> writer(kAlignment, output);
// Write values smaller than the alignment.
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(0, 1)));
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(1, 9)));
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(0, 1)).status());
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(1, 9)).status());
// Write values larger than the alignment but smaller than the buffer.
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(10, 11)));
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(10, 11)).status());
// Exactly fill the remainder of the buffer.
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(21, 11)));
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(21, 11)).status());
// Fill the buffer more than once.
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(32, 66)));
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(32, 66)).status());
// Write nothing.
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(98, 0)));
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(98, 0)).status());
// Write the remaining data.
- EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(98, 2)));
+ EXPECT_EQ(Status::OK, writer.Write(kBytes.subspan(98, 2)).status());
auto result = writer.Flush();
EXPECT_EQ(Status::OK, result.status());
diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 934b296..4268ede 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -708,14 +708,27 @@
entry.transaction_id(),
size_t(address));
- TRY_ASSIGN(const size_t written, entry.Write(key, value));
+ StatusWithSize result = entry.Write(key, value);
+ // Remove any bytes that were written, even if the write was not successful.
+ sector->RemoveWritableBytes(result.size());
+
+ if (!result.ok()) {
+ // TODO: add testing coverage for this once flash errors are supported in
+ // tests.
+ ERR("Failed to write %zu bytes. %zu actually written",
+ entry.size(),
+ result.size());
+ return result.status();
+ }
if (options_.verify_on_write) {
TRY(entry.VerifyChecksumInFlash(entry_header_format_.checksum));
}
+ // Entry was written successfully, update the key descriptor and the sector
+ // descriptor to reflect the new entry.
entry.UpdateDescriptor(key_descriptor);
- sector->MarkValidBytesWritten(written);
+ sector->AddValidBytes(result.size());
return Status::OK;
}
diff --git a/pw_kvs/public/pw_kvs/alignment.h b/pw_kvs/public/pw_kvs/alignment.h
index 2663846..08965ad 100644
--- a/pw_kvs/public/pw_kvs/alignment.h
+++ b/pw_kvs/public/pw_kvs/alignment.h
@@ -58,10 +58,11 @@
~AlignedWriter() { Flush(); }
// Writes bytes to the AlignedWriter. The output may be called if the internal
- // buffer is filled.
- Status Write(span<const std::byte> data);
+ // buffer is filled. The size in the return value represents the total number
+ // of bytes written since flush/reset.
+ StatusWithSize Write(span<const std::byte> data);
- Status Write(const void* data, size_t size) {
+ StatusWithSize Write(const void* data, size_t size) {
return Write(span(static_cast<const std::byte*>(data), size));
}
@@ -106,8 +107,8 @@
AlignedWriterBuffer<kBufferSize> buffer(alignment_bytes, output);
for (const span<const std::byte>& chunk : data) {
- if (Status status = buffer.Write(chunk); !status.ok()) {
- return StatusWithSize(status);
+ if (StatusWithSize status = buffer.Write(chunk); !status.ok()) {
+ return status;
}
}
diff --git a/pw_kvs/public/pw_kvs/internal/sector_descriptor.h b/pw_kvs/public/pw_kvs/internal/sector_descriptor.h
index 96f4be3..79b79b5 100644
--- a/pw_kvs/public/pw_kvs/internal/sector_descriptor.h
+++ b/pw_kvs/public/pw_kvs/internal/sector_descriptor.h
@@ -50,15 +50,13 @@
}
}
- // Adds valid bytes and removes writable bytes.
- void MarkValidBytesWritten(size_t written) {
- valid_bytes_ += written;
-
- if (written > tail_free_bytes_) {
+ // Removes writable bytes without updating the valid bytes.
+ void RemoveWritableBytes(uint16_t bytes) {
+ if (bytes > writable_bytes()) {
// TODO: use a DCHECK instead -- this is a programming error
tail_free_bytes_ = 0;
} else {
- tail_free_bytes_ -= written;
+ tail_free_bytes_ -= bytes;
}
}
