pnor_partition: Refactor to enforce stronger boundaries for abstractions
The RORequest and RWRequest classes did not provide a clear abstraction
over the operation of populating a window or partition associated with a
CREATE_{READ,WRITE}_WINDOW request. The role of the classes was to find
the partition for the provided offset, locate and then open its backing
file.
However, the file-descriptor for the backing file was exposed outside of
the class, as was the FFS partition struct, both of which were managed
_internal_ to the class. Thus the classes provided no encapsulation of
state and awkwardly split the tasks of managing and utilising the
resources between the callee and caller.
This commit inverts the behaviour in a fulfil() method handles the
mechanics of locating, opening, manipulating and closing the backing
file, requiring nothing of the caller. The pnor_partition reference is
managed entirely inside the Request class, derived from the offset
passed to the constructor.
Unifying the mechanics into fulfil() results in a decent reduction in
lines of code at the expense of some cyclomatic complexity. fulfil() is
publicly exposed via read() and write() wrappers on the class, and the
RORequest and RWRequest classes are removed as a result.
Change-Id: Ie587ed31f1a6db97bcb490dbcc2e27ece0b1f82c
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/pnor_partition.cpp b/pnor_partition.cpp
index cdb9633..390c4d5 100644
--- a/pnor_partition.cpp
+++ b/pnor_partition.cpp
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright (C) 2018 IBM Corp.
#include "pnor_partition.hpp"
+#include "pnor_partition_table.hpp"
#include "config.h"
#include "mboxd_flash.h"
#include "mboxd_pnor_partition_table.h"
@@ -8,6 +9,7 @@
#include <phosphor-logging/log.hpp>
#include <phosphor-logging/elog-errors.hpp>
+#include <assert.h>
#include <stdint.h>
#include <stdlib.h>
#include <syslog.h>
@@ -26,185 +28,126 @@
namespace virtual_pnor
{
-using namespace phosphor::logging;
-using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-using namespace std::string_literals;
+namespace fs = std::experimental::filesystem;
-ReturnCode Request::open(const std::string& path, int mode)
+fs::path Request::getPartitionFilePath(int flags)
{
- if (mode == O_RDWR && partition->data.user.data[1] & PARTITION_READONLY)
- {
- MSG_ERR("Can't open RO partition '%s' for write\n", path.c_str());
- return ReturnCode::PARTITION_READ_ONLY;
- }
-
- fs::path partitionFilePath = path;
-
- if (!fs::exists(partitionFilePath))
- {
- return ReturnCode::FILE_NOT_FOUND;
- }
-
- auto descriptor = ::open(partitionFilePath.c_str(), mode);
- if (descriptor < 0)
- {
- return ReturnCode::FILE_OPEN_FAILURE;
- }
-
- fd.set(descriptor);
- descriptor = -1;
-
- return ReturnCode::SUCCESS;
-}
-
-std::string Request::getPartitionFilePath(struct mbox_context* context,
- uint32_t offset)
-{
- partition = vpnor_get_partition(context, offset);
- if (!partition)
- {
- MSG_ERR("Couldn't get the partition info for offset 0x%" PRIx32 "\n",
- offset);
- elog<InternalFailure>();
- }
-
- fs::path partitionFilePath;
-
// Check if partition exists in patch location
- partitionFilePath = context->paths.patch_loc;
- partitionFilePath /= partition->data.name;
- if (fs::is_regular_file(partitionFilePath))
+ auto dst = fs::path(ctx->paths.patch_loc) / partition.data.name;
+ if (fs::is_regular_file(dst))
{
- return partitionFilePath.string();
+ return dst;
}
- switch (partition->data.user.data[1] &
+ switch (partition.data.user.data[1] &
(PARTITION_PRESERVED | PARTITION_READONLY))
{
case PARTITION_PRESERVED:
- partitionFilePath = context->paths.prsv_loc;
+ dst = ctx->paths.prsv_loc;
break;
case PARTITION_READONLY:
- partitionFilePath = context->paths.ro_loc;
+ dst = ctx->paths.ro_loc;
break;
default:
- partitionFilePath = context->paths.rw_loc;
+ dst = ctx->paths.rw_loc;
}
- partitionFilePath /= partition->data.name;
- return partitionFilePath.string();
-}
+ dst /= partition.data.name;
-const pnor_partition* RORequest::getPartitionInfo(struct mbox_context* context,
- uint32_t offset)
-{
- std::string path = getPartitionFilePath(context, offset);
- ReturnCode rc = Request::open(path, O_RDONLY);
- if (rc == ReturnCode::SUCCESS)
+ if (fs::exists(dst))
{
- return partition;
+ return dst;
}
- // not interested in any other error except FILE_NOT_FOUND
- if (rc != ReturnCode::FILE_NOT_FOUND)
+
+ if (flags == O_RDONLY)
{
- MSG_ERR(
- "RORequest: Error opening partition file '%s' (offset 0x%" PRIx32
- "): %u\n",
- path.c_str(), offset, static_cast<uint8_t>(rc));
- elog<InternalFailure>();
+ dst = fs::path(ctx->paths.ro_loc) / partition.data.name;
+ assert(fs::exists(dst));
+ return dst;
}
- // if the offset lies in read only partition then throw error.
- if (partition->data.user.data[1] & PARTITION_READONLY)
- {
- MSG_ERR("RORequest: Requested offset 0x%" PRIx32
- " is in a read-only partition (%s)\n",
- offset, path.c_str());
- elog<InternalFailure>();
- }
-
- // we don't get the file in the respective partition(RW/PSRV)
- // try to open it from RO location.
-
- fs::path partitionFilePath = context->paths.ro_loc;
- partitionFilePath /= partition->data.name;
-
- rc = Request::open(partitionFilePath, O_RDONLY);
- if (rc != ReturnCode::SUCCESS)
- {
- MSG_ERR("RORequest: Failed to open partition file '%s' at RO fallback "
- "(offset 0x%" PRIx32 "): %u\n",
- partitionFilePath.c_str(), offset, static_cast<uint8_t>(rc));
- elog<InternalFailure>();
- }
-
- return partition;
-}
-
-const pnor_partition* RWRequest::getPartitionInfo(struct mbox_context* context,
- uint32_t offset)
-{
- std::string path = getPartitionFilePath(context, offset);
- std::error_code ec;
-
- ReturnCode rc = Request::open(path, O_RDWR);
- if (rc == ReturnCode::SUCCESS)
- {
- return partition;
- }
- // not interested in any other error except FILE_NOT_FOUND
- if (rc != ReturnCode::FILE_NOT_FOUND)
- {
- MSG_ERR(
- "RWRequest: Error opening partition file '%s' (offset 0x%" PRIx32
- "): %d\n",
- path.c_str(), offset, static_cast<uint8_t>(rc));
- elog<InternalFailure>();
- }
-
- // if the file is not available in the respective(RW/PSRV) partition
- // then copy the file from RO to the respective(RW/PSRV) partition
- // and open it for writing.
-
- fs::path fromPath = context->paths.ro_loc;
- fromPath /= partition->data.name;
- if (!fs::exists(fromPath))
- {
- MSG_ERR("RWRequest: Partition '%s' for offset 0x%" PRIx32
- " not found in RO directory '%s'\n",
- partition->data.name, offset, context->paths.ro_loc);
- elog<InternalFailure>();
- }
- // copy the file from ro to respective partition
- fs::path toPath = context->paths.rw_loc;
-
- if (partition->data.user.data[1] & PARTITION_PRESERVED)
- {
- toPath = context->paths.prsv_loc;
- }
+ assert(flags == O_RDWR);
+ auto src = fs::path(ctx->paths.ro_loc) / partition.data.name;
+ assert(fs::exists(src));
MSG_DBG("RWRequest: Didn't find '%s' under '%s', copying from '%s'\n",
- partition->data.name, toPath.c_str(), fromPath.c_str());
+ partition.data.name, dst.c_str(), src.c_str());
- toPath /= partition->data.name;
- if (!fs::copy_file(fromPath, toPath, ec))
+ dst = ctx->paths.rw_loc;
+ if (partition.data.user.data[1] & PARTITION_PRESERVED)
{
- MSG_ERR("RWRequest: Failed to copy file from '%s' to '%s': %d\n",
- fromPath.c_str(), toPath.c_str(), ec.value());
- elog<InternalFailure>();
+ dst = ctx->paths.prsv_loc;
}
- rc = Request::open(toPath.c_str(), O_RDWR);
+ dst /= partition.data.name;
+ fs::copy_file(src, dst);
- if (rc != ReturnCode::SUCCESS)
+ return dst;
+}
+
+ssize_t Request::fulfil(void *buf, size_t len, int flags)
+{
+ if (!(flags == O_RDONLY || flags == O_RDWR))
{
- MSG_ERR("RWRequest: Failed to open file at '%s': %d\n", toPath.c_str(),
- static_cast<uint8_t>(rc));
- elog<InternalFailure>();
+ std::stringstream err;
+ err << "Require O_RDONLY (0x" << std::hex << O_RDONLY << " or O_RDWR "
+ << std::hex << O_RDWR << " for flags, got: 0x" << std::hex << flags;
+ throw std::invalid_argument(err.str());
}
- return partition;
+ fs::path path = getPartitionFilePath(flags);
+
+ int fd = ::open(path.c_str(), flags);
+ if (fd == -1)
+ {
+ MSG_ERR("Failed to open backing file at '%s': %d\n",
+ path.c_str(), errno);
+ throw std::system_error(errno, std::system_category());
+ }
+
+ int mprot = PROT_READ | ((flags == O_RDWR) ? PROT_WRITE : 0);
+ auto map = mmap(NULL, partition.data.actual, mprot, MAP_SHARED, fd, 0);
+ if (map == MAP_FAILED)
+ {
+ close(fd);
+ MSG_ERR("Failed to map backing file '%s' for %d bytes: %d\n",
+ path.c_str(), partition.data.actual, errno);
+ throw std::system_error(errno, std::system_category());
+ }
+
+ // copy to the reserved memory area
+ if (flags == O_RDONLY)
+ {
+ len = std::min(partition.data.actual - offset, len);
+ memcpy(buf, (char *)map + offset, len);
+ }
+ else
+ {
+ // if the asked offset + no of bytes to read is greater
+ // then size of the partition file then throw error.
+ //
+ // FIXME: Don't use .actual, use (.size << ctx->block_size_shift),
+ // otherwise we can't grow the size of the data to fill the partition
+ if ((base + offset + len) > (base + partition.data.actual))
+ {
+ munmap(map, partition.data.actual);
+ close(fd);
+
+ /* FIXME: offset calculation */
+ std::stringstream err;
+ err << "Request size 0x" << std::hex << len << " from offset 0x"
+ << std::hex << offset << " exceeds the partition size 0x"
+ << std::hex << partition.data.actual;
+ throw OutOfBoundsOffset(err.str());
+ }
+ memcpy((char *)map + offset, buf, len);
+ set_flash_bytemap(ctx, base + offset, len, FLASH_DIRTY);
+ }
+ munmap(map, partition.data.actual);
+ close(fd);
+
+ return len;
}
} // namespace virtual_pnor