Remove legacy persistent BufferHub use case
This CL cleans up unused BufferHub code path. The motivation of doing
this is multifold:
1. Reduces future maintainace cost (e.g. when moving to libgui, there
will be less code to rename and re-style).
2. Removes unnecessary code complexity so that the code base is easier
and cleaner for future new reader to understand.
3. Prepares future work for supporting standalone GraphicBuffer in
BufferHub by avoid future logical and naming conflicts between
legacy PersistentBuffer/Detach/Attach and newly proposed detached
GraphicBuffer in BufferHub.
Bug: 70046255
Bug: 70912269
Test: buffer_hub-test, buffer_hub_queue-test,
buffer_hub_queue_producer-test, dvr_api-test, libgui_test
Change-Id: Ie9a0f55e3f620769bac58c81439d840402451b82
diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp
index cdb1f91..1eb4ef9 100644
--- a/services/vr/bufferhubd/buffer_hub.cpp
+++ b/services/vr/bufferhubd/buffer_hub.cpp
@@ -62,8 +62,6 @@
stream << std::setw(18) << "Signaled";
stream << " ";
stream << std::setw(10) << "Index";
- stream << " ";
- stream << "Name";
stream << std::endl;
for (const auto& channel : channels) {
@@ -100,8 +98,6 @@
stream << std::dec << std::setfill(' ');
stream << " ";
stream << std::setw(8) << info.index;
- stream << " ";
- stream << info.name;
stream << std::endl;
}
}
@@ -166,9 +162,7 @@
stream << std::right;
stream << std::setw(6) << "Id";
stream << " ";
- stream << std::setw(14) << "Geometry";
- stream << " ";
- stream << "Name";
+ stream << std::setw(14) << "Info";
stream << std::endl;
for (const auto& channel : channels) {
@@ -216,16 +210,6 @@
*this, &BufferHubService::OnCreateBuffer, message);
return {};
- case BufferHubRPC::CreatePersistentBuffer::Opcode:
- DispatchRemoteMethod<BufferHubRPC::CreatePersistentBuffer>(
- *this, &BufferHubService::OnCreatePersistentBuffer, message);
- return {};
-
- case BufferHubRPC::GetPersistentBuffer::Opcode:
- DispatchRemoteMethod<BufferHubRPC::GetPersistentBuffer>(
- *this, &BufferHubService::OnGetPersistentBuffer, message);
- return {};
-
case BufferHubRPC::CreateProducerQueue::Opcode:
DispatchRemoteMethod<BufferHubRPC::CreateProducerQueue>(
*this, &BufferHubService::OnCreateProducerQueue, message);
@@ -236,12 +220,6 @@
}
}
-void BufferHubService::OnChannelClose(Message&,
- const std::shared_ptr<Channel>& channel) {
- if (auto buffer = std::static_pointer_cast<BufferHubChannel>(channel))
- buffer->Detach();
-}
-
Status<void> BufferHubService::OnCreateBuffer(Message& message, uint32_t width,
uint32_t height, uint32_t format,
uint64_t usage,
@@ -273,117 +251,6 @@
}
}
-Status<void> BufferHubService::OnCreatePersistentBuffer(
- Message& message, const std::string& name, int user_id, int group_id,
- uint32_t width, uint32_t height, uint32_t format, uint64_t usage,
- size_t meta_size_bytes) {
- const uint32_t kDefaultLayerCount = 1;
- const int channel_id = message.GetChannelId();
- ALOGD_IF(TRACE,
- "BufferHubService::OnCreatePersistentBuffer: channel_id=%d name=%s "
- "user_id=%d group_id=%d width=%u height=%u format=%u "
- "usage=%" PRIx64 " meta_size_bytes=%zu",
- channel_id, name.c_str(), user_id, group_id, width, height, format,
- usage, meta_size_bytes);
-
- // See if this channel is already attached to a buffer.
- if (const auto channel = message.GetChannel<BufferHubChannel>()) {
- ALOGE(
- "BufferHubService::OnCreatePersistentBuffer: Channel already attached "
- "to buffer: channel_id=%d buffer_id=%d",
- channel_id, channel->buffer_id());
- return ErrorStatus(EALREADY);
- }
-
- const int euid = message.GetEffectiveUserId();
- const int egid = message.GetEffectiveGroupId();
-
- if (auto buffer = GetNamedBuffer(name)) {
- if (!buffer->CheckAccess(euid, egid)) {
- ALOGE(
- "BufferHubService::OnCreatePersistentBuffer: Requesting process does "
- "not have permission to access named buffer: name=%s euid=%d egid=%d",
- name.c_str(), euid, euid);
- return ErrorStatus(EPERM);
- } else if (!buffer->CheckParameters(width, height, kDefaultLayerCount,
- format, usage, meta_size_bytes)) {
- ALOGE(
- "BufferHubService::OnCreatePersistentBuffer: Requested an existing "
- "buffer with different parameters: name=%s",
- name.c_str());
- return ErrorStatus(EINVAL);
- } else if (!buffer->IsDetached()) {
- ALOGE(
- "BufferHubService::OnCreatePersistentBuffer: Requesting a persistent "
- "buffer that is already attached to a channel: name=%s",
- name.c_str());
- return ErrorStatus(EINVAL);
- } else {
- buffer->Attach(channel_id);
- message.SetChannel(buffer);
- return {};
- }
- } else {
- auto status = ProducerChannel::Create(this, channel_id, width, height,
- kDefaultLayerCount, format, usage,
- meta_size_bytes);
- if (!status) {
- ALOGE("BufferHubService::OnCreateBuffer: Failed to create producer!!");
- return status.error_status();
- }
- auto persistent_buffer = status.take();
- auto make_persistent_status = persistent_buffer->OnProducerMakePersistent(
- message, name, user_id, group_id);
- if (make_persistent_status)
- message.SetChannel(persistent_buffer);
- return make_persistent_status;
- }
-}
-
-Status<void> BufferHubService::OnGetPersistentBuffer(Message& message,
- const std::string& name) {
- const int channel_id = message.GetChannelId();
- ALOGD_IF(TRACE,
- "BufferHubService::OnGetPersistentBuffer: channel_id=%d name=%s",
- channel_id, name.c_str());
-
- // See if this channel is already attached to a buffer.
- if (const auto channel = message.GetChannel<BufferHubChannel>()) {
- ALOGE(
- "BufferHubService::OnGetPersistentBuffer: Channel already attached to "
- "buffer: channel_id=%d buffer_id=%d",
- channel_id, channel->buffer_id());
- return ErrorStatus(EALREADY);
- }
-
- const int euid = message.GetEffectiveUserId();
- const int egid = message.GetEffectiveGroupId();
-
- if (auto buffer = GetNamedBuffer(name)) {
- if (!buffer->CheckAccess(euid, egid)) {
- ALOGE(
- "BufferHubService::OnGetPersistentBuffer: Requesting process does "
- "not have permission to access named buffer: name=%s euid=%d egid=%d",
- name.c_str(), euid, egid);
- return ErrorStatus(EPERM);
- } else if (!buffer->IsDetached()) {
- ALOGE(
- "BufferHubService::OnGetPersistentBuffer: Requesting a persistent "
- "buffer that is already attached to a channel: name=%s",
- name.c_str());
- return ErrorStatus(EINVAL);
- } else {
- buffer->Attach(channel_id);
- message.SetChannel(buffer);
- return {};
- }
- } else {
- ALOGE("BufferHubService::OnGetPersistentBuffer: Buffer \"%s\" not found!",
- name.c_str());
- return ErrorStatus(ENOENT);
- }
-}
-
Status<QueueInfo> BufferHubService::OnCreateProducerQueue(
pdx::Message& message, const ProducerQueueConfig& producer_config,
const UsagePolicy& usage_policy) {
@@ -410,52 +277,17 @@
}
}
-bool BufferHubService::AddNamedBuffer(
- const std::string& name, const std::shared_ptr<ProducerChannel>& buffer) {
- auto search = named_buffers_.find(name);
- if (search == named_buffers_.end()) {
- named_buffers_.emplace(name, buffer);
- return true;
- } else {
- return false;
- }
-}
-
-std::shared_ptr<ProducerChannel> BufferHubService::GetNamedBuffer(
- const std::string& name) {
- auto search = named_buffers_.find(name);
- if (search != named_buffers_.end())
- return search->second;
- else
- return nullptr;
-}
-
-bool BufferHubService::RemoveNamedBuffer(const ProducerChannel& buffer) {
- for (auto it = named_buffers_.begin(); it != named_buffers_.end();) {
- if (it->second.get() == &buffer) {
- named_buffers_.erase(it);
- return true;
- }
- ++it;
- }
- return false;
-}
-
void BufferHubChannel::SignalAvailable() {
ATRACE_NAME("BufferHubChannel::SignalAvailable");
ALOGD_IF(TRACE,
"BufferHubChannel::SignalAvailable: channel_id=%d buffer_id=%d",
channel_id(), buffer_id());
- if (!IsDetached()) {
- signaled_ = true;
- const auto status = service_->ModifyChannelEvents(channel_id_, 0, POLLIN);
- ALOGE_IF(!status,
- "BufferHubChannel::SignalAvailable: failed to signal availability "
- "channel_id=%d: %s",
- channel_id_, status.GetErrorMessage().c_str());
- } else {
- ALOGD_IF(TRACE, "BufferHubChannel::SignalAvailable: detached buffer.");
- }
+ signaled_ = true;
+ const auto status = service_->ModifyChannelEvents(channel_id_, 0, POLLIN);
+ ALOGE_IF(!status,
+ "BufferHubChannel::SignalAvailable: failed to signal availability "
+ "channel_id=%d: %s",
+ channel_id_, status.GetErrorMessage().c_str());
}
void BufferHubChannel::ClearAvailable() {
@@ -463,31 +295,23 @@
ALOGD_IF(TRACE,
"BufferHubChannel::ClearAvailable: channel_id=%d buffer_id=%d",
channel_id(), buffer_id());
- if (!IsDetached()) {
- signaled_ = false;
- const auto status = service_->ModifyChannelEvents(channel_id_, POLLIN, 0);
- ALOGE_IF(!status,
- "BufferHubChannel::ClearAvailable: failed to clear availability "
- "channel_id=%d: %s",
- channel_id_, status.GetErrorMessage().c_str());
- } else {
- ALOGD_IF(TRACE, "BufferHubChannel::ClearAvailable: detached buffer.");
- }
+ signaled_ = false;
+ const auto status = service_->ModifyChannelEvents(channel_id_, POLLIN, 0);
+ ALOGE_IF(!status,
+ "BufferHubChannel::ClearAvailable: failed to clear availability "
+ "channel_id=%d: %s",
+ channel_id_, status.GetErrorMessage().c_str());
}
void BufferHubChannel::Hangup() {
ATRACE_NAME("BufferHubChannel::Hangup");
ALOGD_IF(TRACE, "BufferHubChannel::Hangup: channel_id=%d buffer_id=%d",
channel_id(), buffer_id());
- if (!IsDetached()) {
- const auto status = service_->ModifyChannelEvents(channel_id_, 0, POLLHUP);
- ALOGE_IF(
- !status,
- "BufferHubChannel::Hangup: failed to signal hangup channel_id=%d: %s",
- channel_id_, status.GetErrorMessage().c_str());
- } else {
- ALOGD_IF(TRACE, "BufferHubChannel::Hangup: detached buffer.");
- }
+ const auto status = service_->ModifyChannelEvents(channel_id_, 0, POLLHUP);
+ ALOGE_IF(
+ !status,
+ "BufferHubChannel::Hangup: failed to signal hangup channel_id=%d: %s",
+ channel_id_, status.GetErrorMessage().c_str());
}
} // namespace dvr
diff --git a/services/vr/bufferhubd/buffer_hub.h b/services/vr/bufferhubd/buffer_hub.h
index 270ac95..b487b0b 100644
--- a/services/vr/bufferhubd/buffer_hub.h
+++ b/services/vr/bufferhubd/buffer_hub.h
@@ -27,8 +27,6 @@
kConsumerQueueType,
};
- enum : int { kDetachedId = -1 };
-
BufferHubChannel(BufferHubService* service, int buffer_id, int channel_id,
ChannelType channel_type)
: service_(service),
@@ -57,7 +55,6 @@
uint64_t state = 0;
uint64_t signaled_mask = 0;
uint64_t index = 0;
- std::string name;
// Data filed for producer queue.
size_t capacity = 0;
@@ -66,7 +63,7 @@
BufferInfo(int id, size_t consumer_count, uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format, uint64_t usage,
size_t pending_count, uint64_t state, uint64_t signaled_mask,
- uint64_t index, const std::string& name)
+ uint64_t index)
: id(id),
type(kProducerType),
consumer_count(consumer_count),
@@ -78,8 +75,7 @@
pending_count(pending_count),
state(state),
signaled_mask(signaled_mask),
- index(index),
- name(name) {}
+ index(index) {}
BufferInfo(int id, size_t consumer_count, size_t capacity,
const UsagePolicy& usage_policy)
@@ -109,19 +105,9 @@
int buffer_id() const { return buffer_id_; }
int channel_id() const { return channel_id_; }
- bool IsDetached() const { return channel_id_ == kDetachedId; }
bool signaled() const { return signaled_; }
- void Detach() {
- if (channel_type_ == kProducerType)
- channel_id_ = kDetachedId;
- }
- void Attach(int channel_id) {
- if (channel_type_ == kProducerType && channel_id_ == kDetachedId)
- channel_id_ = channel_id;
- }
-
private:
BufferHubService* service_;
@@ -132,8 +118,7 @@
// general because channel ids are not used for any lookup in this service.
int buffer_id_;
- // The channel id of the buffer. This may change for a persistent producer
- // buffer if it is detached and re-attached to another channel.
+ // The channel id of the buffer.
int channel_id_;
bool signaled_;
@@ -152,34 +137,15 @@
pdx::Status<void> HandleMessage(pdx::Message& message) override;
void HandleImpulse(pdx::Message& message) override;
- void OnChannelClose(pdx::Message& message,
- const std::shared_ptr<pdx::Channel>& channel) override;
-
bool IsInitialized() const override;
std::string DumpState(size_t max_length) override;
- bool AddNamedBuffer(const std::string& name,
- const std::shared_ptr<ProducerChannel>& buffer);
- std::shared_ptr<ProducerChannel> GetNamedBuffer(const std::string& name);
- bool RemoveNamedBuffer(const ProducerChannel& buffer);
-
private:
friend BASE;
- std::unordered_map<std::string, std::shared_ptr<ProducerChannel>>
- named_buffers_;
-
pdx::Status<void> OnCreateBuffer(pdx::Message& message, uint32_t width,
uint32_t height, uint32_t format,
uint64_t usage, size_t meta_size_bytes);
- pdx::Status<void> OnCreatePersistentBuffer(pdx::Message& message,
- const std::string& name,
- int user_id, int group_id,
- uint32_t width, uint32_t height,
- uint32_t format, uint64_t usage,
- size_t meta_size_bytes);
- pdx::Status<void> OnGetPersistentBuffer(pdx::Message& message,
- const std::string& name);
pdx::Status<QueueInfo> OnCreateProducerQueue(
pdx::Message& message, const ProducerQueueConfig& producer_config,
const UsagePolicy& usage_policy);
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index 716db5e..8160193 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -145,7 +145,7 @@
return BufferInfo(buffer_id(), consumer_channels_.size(), buffer_.width(),
buffer_.height(), buffer_.layer_count(), buffer_.format(),
buffer_.usage(), pending_consumers_, buffer_state_->load(),
- signaled_mask, metadata_header_->queue_index, name_);
+ signaled_mask, metadata_header_->queue_index);
}
void ProducerChannel::HandleImpulse(Message& message) {
@@ -183,16 +183,6 @@
*this, &ProducerChannel::OnProducerGain, message);
return true;
- case BufferHubRPC::ProducerMakePersistent::Opcode:
- DispatchRemoteMethod<BufferHubRPC::ProducerMakePersistent>(
- *this, &ProducerChannel::OnProducerMakePersistent, message);
- return true;
-
- case BufferHubRPC::ProducerRemovePersistence::Opcode:
- DispatchRemoteMethod<BufferHubRPC::ProducerRemovePersistence>(
- *this, &ProducerChannel::OnRemovePersistence, message);
- return true;
-
default:
return false;
}
@@ -459,50 +449,6 @@
buffer_state_->load(), fence_state_->load());
}
-Status<void> ProducerChannel::OnProducerMakePersistent(Message& message,
- const std::string& name,
- int user_id,
- int group_id) {
- ATRACE_NAME("ProducerChannel::OnProducerMakePersistent");
- ALOGD_IF(TRACE,
- "ProducerChannel::OnProducerMakePersistent: buffer_id=%d name=%s "
- "user_id=%d group_id=%d",
- buffer_id(), name.c_str(), user_id, group_id);
-
- if (name.empty() || (user_id < 0 && user_id != kNoCheckId) ||
- (group_id < 0 && group_id != kNoCheckId)) {
- return ErrorStatus(EINVAL);
- }
-
- // Try to add this buffer with the requested name.
- if (service()->AddNamedBuffer(name, std::static_pointer_cast<ProducerChannel>(
- shared_from_this()))) {
- // If successful, set the requested permissions.
-
- // A value of zero indicates that the ids from the sending process should be
- // used.
- if (user_id == kUseCallerId)
- user_id = message.GetEffectiveUserId();
- if (group_id == kUseCallerId)
- group_id = message.GetEffectiveGroupId();
-
- owner_user_id_ = user_id;
- owner_group_id_ = group_id;
- name_ = name;
- return {};
- } else {
- // Otherwise a buffer with that name already exists.
- return ErrorStatus(EALREADY);
- }
-}
-
-Status<void> ProducerChannel::OnRemovePersistence(Message&) {
- if (service()->RemoveNamedBuffer(*this))
- return {};
- else
- return ErrorStatus(ENOENT);
-}
-
void ProducerChannel::AddConsumer(ConsumerChannel* channel) {
consumer_channels_.push_back(channel);
}
@@ -546,16 +492,6 @@
}
}
-// Returns true if either the user or group ids match the owning ids or both
-// owning ids are not set, in which case access control does not apply.
-bool ProducerChannel::CheckAccess(int euid, int egid) {
- const bool no_check =
- owner_user_id_ == kNoCheckId && owner_group_id_ == kNoCheckId;
- const bool euid_check = euid == owner_user_id_ || euid == kRootId;
- const bool egid_check = egid == owner_group_id_ || egid == kRootId;
- return no_check || euid_check || egid_check;
-}
-
// Returns true if the given parameters match the underlying buffer parameters.
bool ProducerChannel::CheckParameters(uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format,
diff --git a/services/vr/bufferhubd/producer_channel.h b/services/vr/bufferhubd/producer_channel.h
index e280f4d..c5dbf0e 100644
--- a/services/vr/bufferhubd/producer_channel.h
+++ b/services/vr/bufferhubd/producer_channel.h
@@ -57,16 +57,10 @@
void AddConsumer(ConsumerChannel* channel);
void RemoveConsumer(ConsumerChannel* channel);
- bool CheckAccess(int euid, int egid);
bool CheckParameters(uint32_t width, uint32_t height, uint32_t layer_count,
uint32_t format, uint64_t usage,
size_t user_metadata_size);
- pdx::Status<void> OnProducerMakePersistent(Message& message,
- const std::string& name,
- int user_id, int group_id);
- pdx::Status<void> OnRemovePersistence(Message& message);
-
private:
std::vector<ConsumerChannel*> consumer_channels_;
// This counts the number of consumers left to process this buffer. If this is
@@ -98,16 +92,6 @@
pdx::LocalHandle release_fence_fd_;
pdx::LocalHandle dummy_fence_fd_;
- static constexpr int kNoCheckId = -1;
- static constexpr int kUseCallerId = 0;
- static constexpr int kRootId = 0;
-
- // User and group id to check when obtaining a persistent buffer.
- int owner_user_id_ = kNoCheckId;
- int owner_group_id_ = kNoCheckId;
-
- std::string name_;
-
ProducerChannel(BufferHubService* service, int channel, uint32_t width,
uint32_t height, uint32_t layer_count, uint32_t format,
uint64_t usage, size_t user_metadata_size, int* error);