[devmgr] Reduce visibility of Coordinator members
Reduce the visibility of some of the Coordinator member functions so
that we can more aggressively reorganize and refactor it.
Also rename some of the member functions to match the FIDL interface, so
it's clearer that they are used to implement the interface.
ZX-3286
Test: Build Zircon.
Change-Id: I8d61aecf1794162caa920e19f10e3fee4c8c8096
diff --git a/system/core/devmgr/devmgr/coordinator-test.cpp b/system/core/devmgr/devmgr/coordinator-test.cpp
index cc94c00..ab61bad 100644
--- a/system/core/devmgr/devmgr/coordinator-test.cpp
+++ b/system/core/devmgr/devmgr/coordinator-test.cpp
@@ -52,7 +52,7 @@
zx::channel sender, receiver;
status = zx::channel::create(0, &sender, &receiver);
ASSERT_EQ(ZX_OK, status);
- status = coordinator.OpenVirtcon(std::move(sender));
+ status = coordinator.DmOpenVirtcon(std::move(sender));
ASSERT_EQ(ZX_OK, status);
zx_signals_t signals;
diff --git a/system/core/devmgr/devmgr/coordinator.cpp b/system/core/devmgr/devmgr/coordinator.cpp
index 50d69ff..9bfe00f 100644
--- a/system/core/devmgr/devmgr/coordinator.cpp
+++ b/system/core/devmgr/devmgr/coordinator.cpp
@@ -177,7 +177,7 @@
}
bool Coordinator::InSuspend() const {
- return suspend_context_.flags() == SuspendContext::Flags::kSuspend;
+ return suspend_context().flags() == SuspendContext::Flags::kSuspend;
}
zx_status_t Coordinator::InitializeCoreDevices() {
@@ -242,7 +242,7 @@
return ZX_OK;
}
-zx_status_t Coordinator::OpenVirtcon(zx::channel virtcon_receiver) const {
+zx_status_t Coordinator::DmOpenVirtcon(zx::channel virtcon_receiver) const {
zx_handle_t raw_virtcon_receiver = virtcon_receiver.release();
return virtcon_channel_.write(0, nullptr, 0, &raw_virtcon_receiver, 1);
}
@@ -263,7 +263,7 @@
}
}
-zx_status_t Coordinator::HandleDmctlWrite(size_t len, const char* cmd) {
+zx_status_t Coordinator::DmCommand(size_t len, const char* cmd) {
if (len == 4) {
if (!memcmp(cmd, "dump", 4)) {
DumpState();
@@ -527,7 +527,7 @@
zx_handle_t get_service_root();
-zx_status_t Coordinator::GetTopoPath(const Device* dev, char* out, size_t max) const {
+zx_status_t Coordinator::GetTopologicalPath(const Device* dev, char* out, size_t max) const {
// TODO: Remove VLA.
char tmp[max];
char* path = tmp + max - 1;
@@ -1087,7 +1087,7 @@
// if no metadata is found, check list of metadata added via device_publish_metadata()
char path[fuchsia_device_manager_PATH_MAX];
- zx_status_t status = GetTopoPath(dev, path, sizeof(path));
+ zx_status_t status = GetTopologicalPath(dev, path, sizeof(path));
if (status != ZX_OK) {
return status;
}
@@ -1122,7 +1122,7 @@
// if no metadata is found, check list of metadata added via device_publish_metadata()
char path[fuchsia_device_manager_PATH_MAX];
- zx_status_t status = GetTopoPath(dev, path, sizeof(path));
+ zx_status_t status = GetTopologicalPath(dev, path, sizeof(path));
if (status != ZX_OK) {
return status;
}
@@ -1156,7 +1156,7 @@
zx_status_t Coordinator::PublishMetadata(Device* dev, const char* path, uint32_t type,
const void* data, uint32_t length) {
char caller_path[fuchsia_device_manager_PATH_MAX];
- zx_status_t status = GetTopoPath(dev, caller_path, sizeof(caller_path));
+ zx_status_t status = GetTopologicalPath(dev, caller_path, sizeof(caller_path));
if (status != ZX_OK) {
return status;
}
@@ -1279,7 +1279,7 @@
auto dev = static_cast<Device*>(ctx);
zx_status_t status;
- if ((status = dev->coordinator->GetTopoPath(dev, path, sizeof(path))) != ZX_OK) {
+ if ((status = dev->coordinator->GetTopologicalPath(dev, path, sizeof(path))) != ZX_OK) {
return fuchsia_device_manager_CoordinatorGetTopologicalPath_reply(txn, status, nullptr, 0);
}
return fuchsia_device_manager_CoordinatorGetTopologicalPath_reply(txn, ZX_OK, path,
@@ -1361,14 +1361,14 @@
dev->coordinator->set_dmctl_socket(std::move(log_socket));
}
- zx_status_t status = dev->coordinator->HandleDmctlWrite(command_size, command_data);
+ zx_status_t status = dev->coordinator->DmCommand(command_size, command_data);
dev->coordinator->set_dmctl_socket(zx::socket());
return fuchsia_device_manager_CoordinatorDmCommand_reply(txn, status);
}
static zx_status_t fidl_DmOpenVirtcon(void* ctx, zx_handle_t raw_vc_receiver) {
auto dev = static_cast<Device*>(ctx);
- return dev->coordinator->OpenVirtcon(zx::channel(raw_vc_receiver));
+ return dev->coordinator->DmOpenVirtcon(zx::channel(raw_vc_receiver));
}
static zx_status_t fidl_DmMexec(void* ctx, zx_handle_t raw_kernel, zx_handle_t raw_bootdata) {
@@ -1439,7 +1439,7 @@
return ZX_ERR_INTERNAL;
}
- dev->coordinator->Mexec(std::move(kernel), std::move(bootdata));
+ dev->coordinator->DmMexec(std::move(kernel), std::move(bootdata));
return ZX_OK;
}
@@ -1549,8 +1549,8 @@
if (resp->status != ZX_OK) {
log(ERROR, "devcoord: rpc: suspend '%s' status %d\n", dev->name.data(), resp->status);
}
- suspend_context_.set_status(resp->status);
- ContinueSuspend(&suspend_context_, root_resource());
+ suspend_context().set_status(resp->status);
+ ContinueSuspend(&suspend_context(), root_resource());
break;
}
default:
@@ -1787,22 +1787,22 @@
// Returns the devhost at the front of the queue.
void Coordinator::BuildSuspendList() {
- auto& devhosts = suspend_context_.devhosts();
+ auto& devhosts = suspend_context().devhosts();
// sys_device must suspend last as on x86 it invokes
// ACPI S-state transition
devhosts.push_front(sys_device_.proxy->host);
- append_suspend_list(&suspend_context_, sys_device_.proxy->host);
+ append_suspend_list(&suspend_context(), sys_device_.proxy->host);
devhosts.push_front(root_device_.proxy->host);
- append_suspend_list(&suspend_context_, root_device_.proxy->host);
+ append_suspend_list(&suspend_context(), root_device_.proxy->host);
devhosts.push_front(misc_device_.proxy->host);
- append_suspend_list(&suspend_context_, misc_device_.proxy->host);
+ append_suspend_list(&suspend_context(), misc_device_.proxy->host);
// test devices do not (yet) participate in suspend
- suspend_context_.set_dh(&devhosts.front());
+ suspend_context().set_dh(&devhosts.front());
}
static int suspend_timeout_thread(void* arg) {
@@ -1831,30 +1831,30 @@
if (!sys_device_.proxy || !root_device_.proxy || !misc_device_.proxy) {
return;
}
- if (suspend_context_.flags() == SuspendContext::Flags::kSuspend) {
+ if (suspend_context().flags() == SuspendContext::Flags::kSuspend) {
return;
}
// Move the socket in to prevent the rpc handler from closing the handle.
- suspend_context_ = std::move(ctx);
+ suspend_context() = std::move(ctx);
BuildSuspendList();
- if (suspend_fallback_ || suspend_debug_) {
+ if (suspend_fallback() || suspend_debug()) {
thrd_t t;
- int ret = thrd_create_with_name(&t, suspend_timeout_thread, &suspend_context_,
+ int ret = thrd_create_with_name(&t, suspend_timeout_thread, &suspend_context(),
"devcoord-suspend-timeout");
if (ret != thrd_success) {
log(ERROR, "devcoord: can't create suspend timeout thread\n");
}
}
- process_suspend_list(&suspend_context_);
+ process_suspend_list(&suspend_context());
}
void Coordinator::Suspend(uint32_t flags) {
Suspend(SuspendContext(this, SuspendContext::Flags::kSuspend, flags, std::move(dmctl_socket_)));
}
-void Coordinator::Mexec(zx::vmo kernel, zx::vmo bootdata) {
+void Coordinator::DmMexec(zx::vmo kernel, zx::vmo bootdata) {
Suspend(SuspendContext(this, SuspendContext::Flags::kSuspend, DEVICE_SUSPEND_FLAG_MEXEC,
zx::socket(), std::move(kernel), std::move(bootdata)));
}
@@ -1993,14 +1993,6 @@
return ZX_OK;
}
-static int system_driver_loader(void* arg) {
- auto coordinator = static_cast<Coordinator*>(arg);
- find_loadable_drivers("/system/driver",
- fit::bind_member(coordinator, &Coordinator::DriverAddedSys));
- async::PostTask(coordinator->dispatcher(), [coordinator] { coordinator->BindSystemDrivers(); });
- return 0;
-}
-
void Coordinator::ScanSystemDrivers() {
if (system_loaded_) {
return;
@@ -2010,7 +2002,15 @@
// This avoids deadlocks between the devhosts hosting the block devices that
// these drivers may be served from and the devcoordinator loading them.
thrd_t t;
- int ret = thrd_create_with_name(&t, system_driver_loader, this, "system-driver-loader");
+ auto callback = [](void* arg) {
+ auto coordinator = static_cast<Coordinator*>(arg);
+ find_loadable_drivers("/system/driver",
+ fit::bind_member(coordinator, &Coordinator::DriverAddedSys));
+ async::PostTask(coordinator->dispatcher(),
+ [coordinator] { coordinator->BindSystemDrivers(); });
+ return 0;
+ };
+ int ret = thrd_create_with_name(&t, callback, this, "system-driver-loader");
if (ret == thrd_success) {
thrd_detach(t);
}
diff --git a/system/core/devmgr/devmgr/coordinator.h b/system/core/devmgr/devmgr/coordinator.h
index 4063887..2b51ddb 100644
--- a/system/core/devmgr/devmgr/coordinator.h
+++ b/system/core/devmgr/devmgr/coordinator.h
@@ -270,7 +270,6 @@
public:
Coordinator(const Coordinator&) = delete;
Coordinator& operator=(const Coordinator&) = delete;
-
Coordinator(Coordinator&&) = delete;
Coordinator& operator=(Coordinator&&) = delete;
@@ -278,38 +277,25 @@
~Coordinator();
zx_status_t InitializeCoreDevices();
-
- zx_status_t OpenVirtcon(zx::channel virtcon_receiver) const;
-
- void DmPrintf(const char* fmt, ...) const;
- zx_status_t HandleDmctlWrite(size_t len, const char* cmd);
-
- const Driver* LibnameToDriver(const fbl::String& libname) const;
- zx_status_t LibnameToVmo(const fbl::String& libname, zx::vmo* out_vmo) const;
-
zx_status_t SetBootdata(const zx::unowned_vmo& vmo);
-
bool InSuspend() const;
- void DumpDevice(const Device* dev, size_t indent) const;
- void DumpState() const;
- void DumpDeviceProps(const Device* dev) const;
- void DumpGlobalDeviceProps() const;
- void DumpDrivers() const;
+ void ScanSystemDrivers();
+ void BindDrivers();
+ void UseFallbackDrivers();
+ void DriverAdded(Driver* drv, const char* version);
+ void DriverAddedInit(Driver* drv, const char* version);
+ zx_status_t LibnameToVmo(const fbl::String& libname, zx::vmo* out_vmo) const;
- zx_status_t GetTopoPath(const Device* dev, char* out, size_t max) const;
-
- zx_status_t NewDevhost(const char* name, Devhost* parent, Devhost** out);
- void ReleaseDevhost(Devhost* dh);
- void ReleaseDevice(Device* dev);
-
+ // Used to implement fuchsia::device::manager::Coordinator.
zx_status_t AddDevice(Device* parent, zx::channel rpc, const uint64_t* props_data,
size_t props_count, fbl::StringPiece name, uint32_t protocol_id,
fbl::StringPiece driver_path, fbl::StringPiece args, bool invisible,
zx::channel client_remote);
- zx_status_t MakeVisible(Device* dev);
zx_status_t RemoveDevice(Device* dev, bool forced);
-
+ zx_status_t MakeVisible(Device* dev);
+ zx_status_t BindDevice(Device* dev, fbl::StringPiece drvlibname);
+ zx_status_t GetTopologicalPath(const Device* dev, char* out, size_t max) const;
zx_status_t LoadFirmware(Device* dev, const char* path, zx::vmo* vmo, size_t* size);
zx_status_t GetMetadata(Device* dev, uint32_t type, void* buffer, size_t buflen,
@@ -318,30 +304,14 @@
zx_status_t AddMetadata(Device* dev, uint32_t type, const void* data, uint32_t length);
zx_status_t PublishMetadata(Device* dev, const char* path, uint32_t type, const void* data,
uint32_t length);
-
- zx_status_t BindDevice(Device* dev, fbl::StringPiece drvlibname);
- zx_status_t BindDriver(Driver* drv);
+ zx_status_t DmCommand(size_t len, const char* cmd);
+ zx_status_t DmOpenVirtcon(zx::channel virtcon_receiver) const;
+ void DmMexec(zx::vmo kernel, zx::vmo bootdata);
zx_status_t HandleDeviceRead(Device* dev);
-
- zx_status_t PrepareProxy(Device* dev);
- zx_status_t AttemptBind(const Driver* drv, Device* dev);
-
void HandleNewDevice(Device* dev);
-
- void ScanSystemDrivers();
- void BindSystemDrivers();
- void BindDrivers();
- void UseFallbackDrivers();
-
- void Mexec(zx::vmo kernel, zx::vmo bootdata);
-
- void Suspend(uint32_t flags);
- void BuildSuspendList();
-
- void DriverAdded(Driver* drv, const char* version);
- void DriverAddedInit(Driver* drv, const char* version);
- void DriverAddedSys(Driver* drv, const char* version);
+ zx_status_t PrepareProxy(Device* dev);
+ void DumpState() const;
const zx::resource& root_resource() const { return config_.root_resource; }
const zx::event& fshost_event() const { return config_.fshost_event; }
@@ -370,11 +340,11 @@
Device& test_device() { return test_device_; }
SuspendContext& suspend_context() { return suspend_context_; }
+ const SuspendContext& suspend_context() const { return suspend_context_; }
bool suspend_fallback() const { return suspend_fallback_; }
void set_suspend_fallback(bool suspend_fallback) { suspend_fallback_ = suspend_fallback; }
bool suspend_debug() const { return suspend_debug_; }
void set_suspend_debug(bool suspend_debug) { suspend_debug_ = suspend_debug; }
-
bool system_available() const { return system_available_; }
void set_system_available(bool system_available) { system_available_ = system_available; }
bool system_loaded() const { return system_loaded_; }
@@ -422,8 +392,27 @@
bool system_available_ = false;
bool system_loaded_ = false;
+ void DmPrintf(const char* fmt, ...) const;
+ void DumpDevice(const Device* dev, size_t indent) const;
+ void DumpDeviceProps(const Device* dev) const;
+ void DumpGlobalDeviceProps() const;
+ void DumpDrivers() const;
+
+ void BuildSuspendList();
void Suspend(SuspendContext ctx);
+ void Suspend(uint32_t flags);
+
fbl::unique_ptr<Driver> ValidateDriver(fbl::unique_ptr<Driver> drv);
+ const Driver* LibnameToDriver(const fbl::String& libname) const;
+
+ zx_status_t NewDevhost(const char* name, Devhost* parent, Devhost** out);
+ void ReleaseDevhost(Devhost* dh);
+ void ReleaseDevice(Device* dev);
+
+ zx_status_t BindDriver(Driver* drv);
+ zx_status_t AttemptBind(const Driver* drv, Device* dev);
+ void BindSystemDrivers();
+ void DriverAddedSys(Driver* drv, const char* version);
};
void coordinator_setup(Coordinator* coordinator, DevmgrArgs args);
