[minfs] cleanup
- Addresses a todo to convert protected members to private where
applicable.
- Removes arguments where they are not used.
- clang tidy.
TEST: No behavior change.
Change-Id: I85fc67cb1970c0544bd28d1b74800b1ccd920960
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/442582
Commit-Queue: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Brett Wilson <brettw@google.com>
Testability-Review: Brett Wilson <brettw@google.com>
diff --git a/src/storage/minfs/directory.cc b/src/storage/minfs/directory.cc
index 85347f6..87804ef 100644
--- a/src/storage/minfs/directory.cc
+++ b/src/storage/minfs/directory.cc
@@ -80,18 +80,18 @@
Directory::~Directory() = default;
-blk_t Directory::GetBlockCount() const { return inode_.block_count; }
+blk_t Directory::GetBlockCount() const { return GetInode()->block_count; }
-uint64_t Directory::GetSize() const { return inode_.size; }
+uint64_t Directory::GetSize() const { return GetInode()->size; }
-void Directory::SetSize(uint32_t new_size) { inode_.size = new_size; }
+void Directory::SetSize(uint32_t new_size) { GetMutableInode()->size = new_size; }
void Directory::AcquireWritableBlock(Transaction* transaction, blk_t local_bno, blk_t old_bno,
blk_t* out_bno) {
bool using_new_block = (old_bno == 0);
if (using_new_block) {
- fs_->BlockNew(transaction, out_bno);
- inode_.block_count++;
+ Vfs()->BlockNew(transaction, out_bno);
+ GetMutableInode()->block_count++;
}
}
@@ -100,7 +100,7 @@
// If we found a block that was previously allocated, delete it.
if (old_bno != 0) {
transaction->DeallocateBlock(old_bno);
- inode_.block_count--;
+ GetMutableInode()->block_count--;
}
}
@@ -113,7 +113,7 @@
.dev_offset = dev_offset,
.length = count,
};
- UnownedVmoBuffer buffer(zx::unowned_vmo(vmo_.get()));
+ UnownedVmoBuffer buffer(vmo());
transaction->EnqueueMetadata(operation, &buffer);
}
@@ -134,7 +134,7 @@
zx_status_t Directory::CanUnlink() const {
// directories must be empty (dirent_count == 2)
- if (inode_.dirent_count != 2) {
+ if (GetInode()->dirent_count != 2) {
// if we have more than "." and "..", not empty, cannot unlink
return ZX_ERR_NOT_EMPTY;
#ifdef __Fuchsia__
@@ -213,11 +213,11 @@
TruncateInternal(transaction, off + kMinfsDirentSize);
}
- inode_.dirent_count--;
+ GetMutableInode()->dirent_count--;
if (MinfsMagicType(childvn->GetInode()->magic) == kMinfsTypeDir) {
// Child directory had '..' which pointed to parent directory
- inode_.link_count--;
+ GetMutableInode()->link_count--;
}
status = childvn->RemoveInodeLink(transaction);
@@ -238,7 +238,7 @@
fbl::RefPtr<VnodeMinfs> vn;
zx_status_t status;
- if ((status = vndir->fs_->VnodeGet(&vn, de->ino)) < 0) {
+ if ((status = vndir->Vfs()->VnodeGet(&vn, de->ino)) < 0) {
return status;
}
@@ -261,7 +261,7 @@
fbl::RefPtr<VnodeMinfs> vn;
zx_status_t status;
- if ((status = vndir->fs_->VnodeGet(&vn, de->ino)) < 0) {
+ if ((status = vndir->Vfs()->VnodeGet(&vn, de->ino)) < 0) {
return status;
}
return vndir->UnlinkChild(args->transaction, std::move(vn), de, &args->offs);
@@ -284,7 +284,7 @@
fbl::RefPtr<VnodeMinfs> vn;
zx_status_t status;
- if ((status = vndir->fs_->VnodeGet(&vn, de->ino)) < 0) {
+ if ((status = vndir->Vfs()->VnodeGet(&vn, de->ino)) < 0) {
return status;
}
if (args->ino == vn->GetIno()) {
@@ -419,11 +419,11 @@
if (args->type == kMinfsTypeDir) {
// Child directory has '..' which will point to parent directory
- inode_.link_count++;
+ GetMutableInode()->link_count++;
}
- inode_.dirent_count++;
- inode_.seq_num++;
+ GetMutableInode()->dirent_count++;
+ GetMutableInode()->seq_num++;
InodeSync(args->transaction, kMxFsSyncMtime);
args->transaction->PinVnode(fbl::RefPtr(this));
return ZX_OK;
@@ -464,7 +464,7 @@
case kDirIteratorNext:
break;
case kDirIteratorSaveSync:
- inode_.seq_num++;
+ GetMutableInode()->seq_num++;
InodeSync(args->transaction, kMxFsSyncMtime);
args->transaction->PinVnode(fbl::RefPtr(this));
return ZX_OK;
@@ -506,15 +506,15 @@
args.name = name;
bool success = false;
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics = fbl::MakeAutoCall(
- [&ticker, &success, this]() { fs_->UpdateLookupMetrics(success, ticker.End()); });
+ [&ticker, &success, this]() { Vfs()->UpdateLookupMetrics(success, ticker.End()); });
if (zx_status_t status = ForEachDirent(&args, DirentCallbackFind); status != ZX_OK) {
return status;
}
fbl::RefPtr<VnodeMinfs> vn;
- if (zx_status_t status = fs_->VnodeGet(&vn, args.ino); status != ZX_OK) {
+ if (zx_status_t status = Vfs()->VnodeGet(&vn, args.ino); status != ZX_OK) {
return status;
}
*out = std::move(vn);
@@ -549,7 +549,7 @@
DirentBuffer dirent_buffer;
Dirent* de = &dirent_buffer.dirent;
- if (off != 0 && dc->seqno != inode_.seq_num) {
+ if (off != 0 && dc->seqno != GetInode()->seq_num) {
// The offset *might* be invalid, if we called Readdir after a directory
// has been modified. In this case, we need to re-read the directory
// until we get to the direntry at or after the previously identified offset.
@@ -596,7 +596,7 @@
done:
// save our place in the DirCookie
dc->off = off;
- dc->seqno = inode_.seq_num;
+ dc->seqno = GetInode()->seq_num;
*out_actual = df.BytesFilled();
ZX_DEBUG_ASSERT(*out_actual <= len); // Otherwise, we're overflowing the input buffer.
return ZX_OK;
@@ -614,9 +614,9 @@
}
bool success = false;
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics = fbl::MakeAutoCall(
- [&ticker, &success, this]() { fs_->UpdateCreateMetrics(success, ticker.End()); });
+ [&ticker, &success, this]() { Vfs()->UpdateCreateMetrics(success, ticker.End()); });
if (IsUnlinked()) {
return ZX_ERR_BAD_STATE;
@@ -648,7 +648,7 @@
// Calculate maximum blocks to reserve for the current directory, based on the size and offset
// of the new direntry (Assuming that the offset is the current size of the directory).
- auto reserve_blocks_or = GetRequiredBlockCount(GetSize(), args.reclen, fs_->BlockSize());
+ auto reserve_blocks_or = GetRequiredBlockCount(GetSize(), args.reclen, Vfs()->BlockSize());
if (reserve_blocks_or.is_error()) {
return reserve_blocks_or.error_value();
}
@@ -656,17 +656,17 @@
// Reserve 1 additional block for the new directory's initial . and .. entries.
blk_t reserve_blocks = reserve_blocks_or.value() + 1;
- ZX_DEBUG_ASSERT(reserve_blocks <= fs_->Limits().GetMaximumMetaDataBlocks());
+ ZX_DEBUG_ASSERT(reserve_blocks <= Vfs()->Limits().GetMaximumMetaDataBlocks());
// In addition to reserve_blocks, reserve 1 inode for the vnode to be created.
std::unique_ptr<Transaction> transaction;
- if ((status = fs_->BeginTransaction(1, reserve_blocks, &transaction)) != ZX_OK) {
+ if ((status = Vfs()->BeginTransaction(1, reserve_blocks, &transaction)) != ZX_OK) {
return status;
}
// mint a new inode and vnode for it
fbl::RefPtr<VnodeMinfs> vn;
- if ((status = fs_->VnodeNew(transaction.get(), &vn, type)) < 0) {
+ if ((status = Vfs()->VnodeNew(transaction.get(), &vn, type)) < 0) {
return status;
}
@@ -691,7 +691,7 @@
transaction->PinVnode(fbl::RefPtr(this));
transaction->PinVnode(vn);
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
if ((status = vn->OpenValidating(fs::VnodeConnectionOptions(), nullptr)) != ZX_OK) {
return status;
@@ -705,13 +705,13 @@
TRACE_DURATION("minfs", "Directory::Unlink", "name", name);
ZX_DEBUG_ASSERT(fs::vfs_valid_name(name));
bool success = false;
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics = fbl::MakeAutoCall(
- [&ticker, &success, this]() { fs_->UpdateUnlinkMetrics(success, ticker.End()); });
+ [&ticker, &success, this]() { Vfs()->UpdateUnlinkMetrics(success, ticker.End()); });
zx_status_t status;
std::unique_ptr<Transaction> transaction;
- if ((status = fs_->BeginTransaction(0, 0, &transaction)) != ZX_OK) {
+ if ((status = Vfs()->BeginTransaction(0, 0, &transaction)) != ZX_OK) {
return status;
}
@@ -725,7 +725,7 @@
return status;
}
transaction->PinVnode(fbl::RefPtr(this));
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
success = true;
return ZX_OK;
}
@@ -756,9 +756,9 @@
bool dst_must_be_dir) {
TRACE_DURATION("minfs", "Directory::Rename", "src", oldname, "dst", newname);
bool success = false;
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics = fbl::MakeAutoCall(
- [&ticker, &success, this]() { fs_->UpdateRenameMetrics(success, ticker.End()); });
+ [&ticker, &success, this]() { Vfs()->UpdateRenameMetrics(success, ticker.End()); });
ZX_DEBUG_ASSERT(fs::vfs_valid_name(oldname));
ZX_DEBUG_ASSERT(fs::vfs_valid_name(newname));
@@ -783,7 +783,7 @@
if ((status = ForEachDirent(&args, DirentCallbackFind)) < 0) {
return status;
}
- if ((status = fs_->VnodeGet(&oldvn, args.ino)) < 0) {
+ if ((status = Vfs()->VnodeGet(&oldvn, args.ino)) < 0) {
return status;
}
if (oldvn->IsDirectory()) {
@@ -821,13 +821,13 @@
// Reserve potential blocks to add a new direntry to newdir.
auto reserved_blocks_or =
- GetRequiredBlockCount(newdir->GetInode()->size, args.reclen, fs_->BlockSize());
+ GetRequiredBlockCount(newdir->GetInode()->size, args.reclen, Vfs()->BlockSize());
if (reserved_blocks_or.is_error()) {
return reserved_blocks_or.error_value();
}
std::unique_ptr<Transaction> transaction;
- if ((status = fs_->BeginTransaction(0, reserved_blocks_or.value(), &transaction)) != ZX_OK) {
+ if ((status = Vfs()->BeginTransaction(0, reserved_blocks_or.value(), &transaction)) != ZX_OK) {
return status;
}
@@ -873,7 +873,7 @@
}
transaction->PinVnode(oldvn);
transaction->PinVnode(newdir);
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
success = true;
return ZX_OK;
}
@@ -913,13 +913,14 @@
}
// Reserve potential blocks to write a new direntry.
- auto reserved_blocks_or = GetRequiredBlockCount(GetInode()->size, args.reclen, fs_->BlockSize());
+ auto reserved_blocks_or =
+ GetRequiredBlockCount(GetInode()->size, args.reclen, Vfs()->BlockSize());
if (reserved_blocks_or.is_error()) {
return reserved_blocks_or.error_value();
}
std::unique_ptr<Transaction> transaction;
- if ((status = fs_->BeginTransaction(0, reserved_blocks_or.value(), &transaction)) != ZX_OK) {
+ if ((status = Vfs()->BeginTransaction(0, reserved_blocks_or.value(), &transaction)) != ZX_OK) {
return status;
}
@@ -934,7 +935,7 @@
target->InodeSync(transaction.get(), kMxFsSyncDefault);
transaction->PinVnode(fbl::RefPtr(this));
transaction->PinVnode(target);
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
return ZX_OK;
}
diff --git a/src/storage/minfs/file.cc b/src/storage/minfs/file.cc
index a330b1b..4934099 100644
--- a/src/storage/minfs/file.cc
+++ b/src/storage/minfs/file.cc
@@ -43,7 +43,7 @@
File::~File() {
#ifdef __Fuchsia__
- ZX_DEBUG_ASSERT_MSG(allocation_state_.GetNodeSize() == inode_.size,
+ ZX_DEBUG_ASSERT_MSG(allocation_state_.GetNodeSize() == GetInode()->size,
"File being destroyed with pending updates to the inode size");
#endif
}
@@ -62,8 +62,8 @@
// the smallest between half the capacity of the writeback buffer, and the number of direct
// blocks needed to touch the maximum allowed number of indirect blocks.
const uint32_t max_direct_blocks =
- kMinfsDirect + (kMinfsDirectPerIndirect * fs_->Limits().GetMaximumMetaDataBlocks());
- const uint32_t max_writeback_blocks = static_cast<blk_t>(fs_->WritebackCapacity() / 2);
+ kMinfsDirect + (kMinfsDirectPerIndirect * Vfs()->Limits().GetMaximumMetaDataBlocks());
+ const uint32_t max_writeback_blocks = static_cast<blk_t>(Vfs()->WritebackCapacity() / 2);
const uint32_t max_blocks = std::min(max_direct_blocks, max_writeback_blocks);
fbl::Array<blk_t> allocated_blocks(new blk_t[max_blocks], max_blocks);
@@ -74,15 +74,15 @@
ZX_ASSERT(expected_blocks <= max_blocks);
if (expected_blocks == 0) {
- if (inode_.size != allocation_state_.GetNodeSize()) {
- inode_.size = allocation_state_.GetNodeSize();
- ValidateVmoTail(inode_.size);
+ if (GetInode()->size != allocation_state_.GetNodeSize()) {
+ GetMutableInode()->size = allocation_state_.GetNodeSize();
+ ValidateVmoTail(GetInode()->size);
}
// Since we may have pending reservations from an expected update, reset the allocation
// state. This may happen if the same block range is allocated and de-allocated (e.g.
// written and truncated) before the state is resolved.
- ZX_ASSERT(allocation_state_.GetNodeSize() == inode_.size);
+ ZX_ASSERT(allocation_state_.GetNodeSize() == GetInode()->size);
allocation_state_.Reset(allocation_state_.GetNodeSize());
ZX_DEBUG_ASSERT(allocation_state_.IsEmpty());
break;
@@ -96,12 +96,12 @@
ZX_ASSERT(BlocksSwap(transaction.get(), bno_start, bno_count, &allocated_blocks[0]) == ZX_OK);
// Enqueue each data block one at a time, as they may not be contiguous on disk.
- UnownedVmoBuffer buffer(zx::unowned_vmo(vmo_.get()));
+ UnownedVmoBuffer buffer(vmo());
for (blk_t i = 0; i < bno_count; i++) {
storage::Operation operation = {
.type = storage::OperationType::kWrite,
.vmo_offset = bno_start + i,
- .dev_offset = allocated_blocks[i] + fs_->Info().dat_block,
+ .dev_offset = allocated_blocks[i] + Vfs()->Info().dat_block,
.length = 1,
};
transaction->EnqueueData(operation, &buffer);
@@ -109,20 +109,20 @@
// Since we are updating the file in "chunks", only update the on-disk inode size
// with the portion we've written so far.
- blk_t last_byte = (bno_start + bno_count) * fs_->BlockSize();
- ZX_ASSERT(last_byte <= fbl::round_up(allocation_state_.GetNodeSize(), fs_->BlockSize()));
+ blk_t last_byte = (bno_start + bno_count) * Vfs()->BlockSize();
+ ZX_ASSERT(last_byte <= fbl::round_up(allocation_state_.GetNodeSize(), Vfs()->BlockSize()));
- if (last_byte > inode_.size && last_byte < allocation_state_.GetNodeSize()) {
+ if (last_byte > GetInode()->size && last_byte < allocation_state_.GetNodeSize()) {
// If we have written past the end of the recorded size but have not yet reached the
// allocated size, update the recorded size to the last byte written.
- inode_.size = last_byte;
+ GetMutableInode()->size = last_byte;
} else if (allocation_state_.GetNodeSize() <= last_byte) {
// If we have just written to the allocated inode size, update the recorded size
// accordingly.
- inode_.size = allocation_state_.GetNodeSize();
+ GetMutableInode()->size = allocation_state_.GetNodeSize();
}
- ValidateVmoTail(inode_.size);
+ ValidateVmoTail(GetInode()->size);
// In the future we could resolve on a per state (i.e. reservation) basis, but since swaps are
// currently only made within a single thread, for now it is okay to resolve everything.
@@ -130,7 +130,7 @@
}
InodeSync(transaction.get(), kMxFsSyncMtime);
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
}
zx_status_t File::BlocksSwap(Transaction* transaction, blk_t start, blk_t count, blk_t* bnos) {
@@ -151,11 +151,11 @@
// is sparse or unmapped. We should add something for this magic constant and fix all places
// that currently hard code zero.
if (old_block == 0) {
- inode_.block_count++;
+ GetMutableInode()->block_count++;
}
// For copy-on-write, swap the block out if it's a data block.
blk_t new_block = old_block;
- fs_->BlockSwap(transaction, old_block, &new_block);
+ Vfs()->BlockSwap(transaction, old_block, &new_block);
zx_status_t status = iterator.SetBlk(new_block);
if (status != ZX_OK)
return status;
@@ -174,9 +174,9 @@
blk_t File::GetBlockCount() const {
#ifdef __Fuchsia__
- return inode_.block_count + allocation_state_.GetNewPending();
+ return GetInode()->block_count + allocation_state_.GetNewPending();
#else
- return inode_.block_count;
+ return GetInode()->block_count;
#endif
}
@@ -184,14 +184,14 @@
#ifdef __Fuchsia__
return allocation_state_.GetNodeSize();
#endif
- return inode_.size;
+ return GetInode()->size;
}
void File::SetSize(uint32_t new_size) {
#ifdef __Fuchsia__
allocation_state_.SetNodeSize(new_size);
#else
- inode_.size = new_size;
+ GetMutableInode()->size = new_size;
#endif
}
@@ -202,8 +202,8 @@
allocation_state_.SetPending(local_bno, !using_new_block);
#else
if (using_new_block) {
- fs_->BlockNew(transaction, out_bno);
- inode_.block_count++;
+ Vfs()->BlockNew(transaction, out_bno);
+ GetMutableInode()->block_count++;
} else {
*out_bno = old_bno;
}
@@ -214,7 +214,7 @@
// If we found a block that was previously allocated, delete it.
if (old_bno != 0) {
transaction->DeallocateBlock(old_bno);
- inode_.block_count--;
+ GetMutableInode()->block_count--;
}
#ifdef __Fuchsia__
if (!indirect) {
@@ -237,7 +237,7 @@
void File::CancelPendingWriteback() {
// Drop all pending writes, revert the size of the inode to the "pre-pending-write" size.
- allocation_state_.Reset(inode_.size);
+ allocation_state_.Reset(GetInode()->size);
}
#endif
@@ -248,35 +248,33 @@
zx_status_t File::Read(void* data, size_t len, size_t off, size_t* out_actual) {
TRACE_DURATION("minfs", "File::Read", "ino", GetIno(), "len", len, "off", off);
- ZX_DEBUG_ASSERT_MSG(FdCount() > 0, "Reading from ino with no fds open");
FS_TRACE_DEBUG("minfs_read() vn=%p(#%u) len=%zd off=%zd\n", this, GetIno(), len, off);
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics = fbl::MakeAutoCall(
- [&ticker, &out_actual, this]() { fs_->UpdateReadMetrics(*out_actual, ticker.End()); });
+ [&ticker, &out_actual, this]() { Vfs()->UpdateReadMetrics(*out_actual, ticker.End()); });
- Transaction transaction(fs_);
+ Transaction transaction(Vfs());
return ReadInternal(&transaction, data, len, off, out_actual);
}
zx_status_t File::Write(const void* data, size_t len, size_t offset, size_t* out_actual) {
TRACE_DURATION("minfs", "File::Write", "ino", GetIno(), "len", len, "off", offset);
- ZX_DEBUG_ASSERT_MSG(FdCount() > 0, "Writing to ino with no fds open");
FS_TRACE_DEBUG("minfs_write() vn=%p(#%u) len=%zd off=%zd\n", this, GetIno(), len, offset);
*out_actual = 0;
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics = fbl::MakeAutoCall(
- [&ticker, &out_actual, this]() { fs_->UpdateWriteMetrics(*out_actual, ticker.End()); });
+ [&ticker, &out_actual, this]() { Vfs()->UpdateWriteMetrics(*out_actual, ticker.End()); });
// Calculate maximum number of blocks to reserve for this write operation.
- auto reserve_blocks_or = GetRequiredBlockCount(offset, len, fs_->BlockSize());
+ auto reserve_blocks_or = GetRequiredBlockCount(offset, len, Vfs()->BlockSize());
if (reserve_blocks_or.is_error()) {
return reserve_blocks_or.error_value();
}
std::unique_ptr<Transaction> transaction;
zx_status_t status;
- if ((status = fs_->BeginTransaction(0, reserve_blocks_or.value(), &transaction)) != ZX_OK) {
+ if ((status = Vfs()->BeginTransaction(0, reserve_blocks_or.value(), &transaction)) != ZX_OK) {
return status;
}
@@ -295,7 +293,7 @@
AllocateAndCommitData(std::move(transaction));
#else
InodeSync(transaction.get(), kMxFsSyncMtime); // Successful writes updates mtime
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
#endif
}
@@ -311,16 +309,16 @@
zx_status_t File::Truncate(size_t len) {
TRACE_DURATION("minfs", "File::Truncate");
- fs::Ticker ticker(fs_->StartTicker());
+ fs::Ticker ticker(Vfs()->StartTicker());
auto get_metrics =
- fbl::MakeAutoCall([&ticker, this] { fs_->UpdateTruncateMetrics(ticker.End()); });
+ fbl::MakeAutoCall([&ticker, this] { Vfs()->UpdateTruncateMetrics(ticker.End()); });
std::unique_ptr<Transaction> transaction;
// Due to file copy-on-write, up to 1 new (data) block may be required.
size_t reserve_blocks = 1;
zx_status_t status;
- if ((status = fs_->BeginTransaction(0, reserve_blocks, &transaction)) != ZX_OK) {
+ if ((status = Vfs()->BeginTransaction(0, reserve_blocks, &transaction)) != ZX_OK) {
return status;
}
@@ -332,10 +330,10 @@
// Shortcut case: If we don't have any data blocks to update, we may as well just update
// the inode by itself.
//
- // This allows us to avoid "only setting inode_.size" in the data task responsible for
+ // This allows us to avoid "only setting GetInode()->size" in the data task responsible for
// calling "AllocateAndCommitData()".
if (allocation_state_.IsEmpty()) {
- inode_.size = allocation_state_.GetNodeSize();
+ GetMutableInode()->size = allocation_state_.GetNodeSize();
}
#endif
@@ -348,7 +346,7 @@
AllocateAndCommitData(std::move(transaction));
#else
InodeSync(transaction.get(), kMxFsSyncMtime);
- fs_->CommitTransaction(std::move(transaction));
+ Vfs()->CommitTransaction(std::move(transaction));
#endif
return ZX_OK;
}
diff --git a/src/storage/minfs/vnode.cc b/src/storage/minfs/vnode.cc
index a66e16f..57cf5405 100644
--- a/src/storage/minfs/vnode.cc
+++ b/src/storage/minfs/vnode.cc
@@ -129,16 +129,12 @@
#ifdef __Fuchsia__
-// Since we cannot yet register the filesystem as a paging service (and cleanly
-// fault on pages when they are actually needed), we currently read an entire
-// file to a VMO when a file's data block are accessed.
-//
// TODO(smklein): Even this hack can be optimized; a bitmap could be used to
// track all 'empty/read/dirty' blocks for each vnode, rather than reading
// the entire file.
//
// TODO(fxbug.dev/51589): Add init metrics.
-zx_status_t VnodeMinfs::InitVmo(PendingWork* transaction) {
+zx_status_t VnodeMinfs::InitVmo() {
if (vmo_.is_valid()) {
return ZX_OK;
}
@@ -387,7 +383,7 @@
zx_status_t status;
#ifdef __Fuchsia__
- if ((status = InitVmo(transaction)) != ZX_OK) {
+ if ((status = InitVmo()) != ZX_OK) {
return status;
} else if ((status = vmo_.read(vdata, off, len)) != ZX_OK) {
return status;
@@ -445,7 +441,7 @@
#ifdef __Fuchsia__
// TODO(planders): Once we are splitting up write transactions, assert this on host as well.
ZX_DEBUG_ASSERT(len <= TransactionLimits::kMaxWriteBytes);
- if ((status = InitVmo(transaction)) != ZX_OK) {
+ if ((status = InitVmo()) != ZX_OK) {
return status;
}
@@ -488,7 +484,7 @@
}
IssueWriteback(transaction, n, bno + fs_->Info().dat_block, 1);
-#else // __Fuchsia__
+#else // __Fuchsia__
blk_t bno;
if ((status = BlockGetWritable(transaction, n, &bno))) {
break;
@@ -539,7 +535,8 @@
// This transaction exists because acquiring the block size and block
// count may be unsafe without locking.
//
- // TODO: Improve locking semantics of pending data allocation to make this less confusing.
+ // TODO(unknown): Improve locking semantics of pending data allocation to make this less
+ // confusing.
Transaction transaction(fs_);
*a = fs::VnodeAttributes();
a->mode = DTYPE_TO_VTYPE(MinfsMagicType(inode_.magic)) | V_IRUSR | V_IWUSR | V_IRGRP | V_IROTH;
@@ -703,7 +700,7 @@
#ifdef __Fuchsia__
// TODO(smklein): We should only init up to 'len'; no need
// to read in the portion of a large file we plan on deleting.
- if ((status = InitVmo(transaction)) != ZX_OK) {
+ if ((status = InitVmo()) != ZX_OK) {
FS_TRACE_ERROR("minfs: Truncate failed to initialize VMO: %d\n", status);
return ZX_ERR_IO;
}
@@ -776,7 +773,7 @@
}
IssueWriteback(transaction, rel_bno, bno + fs_->Info().dat_block, 1);
}
-#else // __Fuchsia__
+#else // __Fuchsia__
if (bno != 0) {
if (fs_->bc_->Readblk(bno + fs_->Info().dat_block, bdata)) {
return ZX_ERR_IO;
diff --git a/src/storage/minfs/vnode.h b/src/storage/minfs/vnode.h
index 5c2be35..1e694ca 100644
--- a/src/storage/minfs/vnode.h
+++ b/src/storage/minfs/vnode.h
@@ -66,6 +66,7 @@
public fbl::Recyclable<VnodeMinfs> {
#endif
public:
+ explicit VnodeMinfs(Minfs* fs);
~VnodeMinfs() override;
// Allocates a new Vnode and initializes the in-memory inode structure given the type, where
@@ -133,7 +134,7 @@
// Should update the in-memory representation of the Vnode, but not necessarily
// write it out to persistent storage.
//
- // TODO: Upgrade internal size to 64-bit integer.
+ // TODO(unknown): Upgrade internal size to 64-bit integer.
virtual void SetSize(uint32_t new_size) = 0;
// Accesses a block in the vnode at |vmo_offset| relative to the start of the file,
@@ -174,6 +175,9 @@
void GetAllocatedRegions(GetAllocatedRegionsCompleter::Sync& completer) final;
void GetMountState(GetMountStateCompleter::Sync& completer) final;
+ // Returns a copy of unowned vmo.
+ zx::unowned_vmo vmo() const { return zx::unowned_vmo(vmo_.get()); }
+
#endif
Minfs* Vfs() { return fs_; }
@@ -211,20 +215,6 @@
// bnos
zx_status_t BlocksShrink(PendingWork* transaction, blk_t start);
- // TODO(smklein): These operations and members are protected as a historical artifact
- // of "File + Directory + Vnode" being a single class. They should be transitioned to
- // private.
- protected:
- explicit VnodeMinfs(Minfs* fs);
-
- // fs::Vnode interface.
- zx_status_t GetAttributes(fs::VnodeAttributes* a) final;
- zx_status_t SetAttributes(fs::VnodeAttributesUpdate a) final;
-#ifdef __Fuchsia__
- zx_status_t QueryFilesystem(llcpp::fuchsia::io::FilesystemInfo* out) final;
- zx_status_t GetDevicePath(size_t buffer_len, char* out_name, size_t* out_len) final;
-#endif
-
// Although file sizes don't need to be block-aligned, the underlying VMO is
// always kept at a size which is a multiple of |kMinfsBlockSize|.
//
@@ -234,12 +224,21 @@
// assumption.
void ValidateVmoTail(uint64_t inode_size) const;
+ private:
+ // fs::Vnode interface.
+ zx_status_t GetAttributes(fs::VnodeAttributes* a) final;
+ zx_status_t SetAttributes(fs::VnodeAttributesUpdate a) final;
+#ifdef __Fuchsia__
+ zx_status_t QueryFilesystem(llcpp::fuchsia::io::FilesystemInfo* out) final;
+ zx_status_t GetDevicePath(size_t buffer_len, char* out_name, size_t* out_len) final;
+#endif
+
// Get the disk block 'bno' corresponding to the 'n' block
//
// May or may not allocate |bno|; certain Vnodes (like File) delay allocation
// until writeback, and will return a sentinel value of zero.
//
- // TODO: Use types to represent that |bno|, as an output, is optional.
+ // TODO(unknown): Use types to represent that |bno|, as an output, is optional.
zx_status_t BlockGetWritable(Transaction* transaction, blk_t n, blk_t* bno);
// Get the disk block 'bno' corresponding to relative block address |n| within the file.
@@ -259,15 +258,22 @@
void Sync(SyncCallback closure) final;
zx_status_t AttachRemote(fs::MountChannel h) final;
- zx_status_t InitVmo(PendingWork* transaction);
+
+ // Initializes vmo that contains file's data by reading data from the disk.
+ // Since we cannot yet register the filesystem as a paging service (and
+ // cleanly fault on pages when they are actually needed), we currently read an
+ // entire file to a VMO when a file's data block are accessed.
+ zx_status_t InitVmo();
// Use the watcher container to implement a directory watcher
void Notify(fbl::StringPiece name, unsigned event) final;
zx_status_t WatchDir(fs::Vfs* vfs, uint32_t mask, uint32_t options, zx::channel watcher) final;
#endif
+
uint32_t FdCount() const { return fd_count_; }
Minfs* const fs_;
+
#ifdef __Fuchsia__
// TODO(smklein): When we have can register MinFS as a pager service, and
// it can properly handle pages faults on a vnode's contents, then we can
