Merge "storaged: Fix bug in empty check"
diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp
index 4c7c3f1..84bcb94 100644
--- a/fs_mgr/libsnapshot/snapuserd/Android.bp
+++ b/fs_mgr/libsnapshot/snapuserd/Android.bp
@@ -187,7 +187,9 @@
"libstorage_literals_headers",
"libfiemap_headers",
],
- test_min_api_level: 30,
+ test_options: {
+ min_shipping_api_level: 30,
+ },
auto_gen_config: true,
require_root: false,
}
diff --git a/init/README.md b/init/README.md
index ebab073..c102b1f 100644
--- a/init/README.md
+++ b/init/README.md
@@ -641,9 +641,10 @@
configurations. Intended to be used only once when apexd notifies the mount
event by setting `apexd.status` to ready.
-`restart <service>`
+`restart [--only-if-running] <service>`
> Stops and restarts a running service, does nothing if the service is currently
- restarting, otherwise, it just starts the service.
+ restarting, otherwise, it just starts the service. If "--only-if-running" is
+ specified, the service is only restarted if it is already running.
`restorecon <path> [ <path>\* ]`
> Restore the file named by _path_ to the security context specified
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 98831e1..8045c71 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -774,8 +774,21 @@
}
static Result<void> do_restart(const BuiltinArguments& args) {
- Service* svc = ServiceList::GetInstance().FindService(args[1]);
- if (!svc) return Error() << "service " << args[1] << " not found";
+ bool only_if_running = false;
+ if (args.size() == 3) {
+ if (args[1] == "--only-if-running") {
+ only_if_running = true;
+ } else {
+ return Error() << "Unknown argument to restart: " << args[1];
+ }
+ }
+
+ const auto& classname = args[args.size() - 1];
+ Service* svc = ServiceList::GetInstance().FindService(classname);
+ if (!svc) return Error() << "service " << classname << " not found";
+ if (only_if_running && !svc->IsRunning()) {
+ return {};
+ }
svc->Restart();
return {};
}
@@ -1453,7 +1466,7 @@
{"update_linker_config", {0, 0, {false, do_update_linker_config}}},
{"readahead", {1, 2, {true, do_readahead}}},
{"remount_userdata", {0, 0, {false, do_remount_userdata}}},
- {"restart", {1, 1, {false, do_restart}}},
+ {"restart", {1, 2, {false, do_restart}}},
{"restorecon", {1, kMax, {true, do_restorecon}}},
{"restorecon_recursive", {1, kMax, {true, do_restorecon_recursive}}},
{"rm", {1, 1, {true, do_rm}}},
diff --git a/libprocessgroup/profiles/task_profiles.json b/libprocessgroup/profiles/task_profiles.json
index 45d3c7c..b668dcb 100644
--- a/libprocessgroup/profiles/task_profiles.json
+++ b/libprocessgroup/profiles/task_profiles.json
@@ -651,6 +651,10 @@
{
"Name": "Dex2OatBootComplete",
"Profiles": [ "Dex2oatPerformance", "LowIoPriority", "TimerSlackHigh" ]
+ },
+ {
+ "Name": "OtaProfiles",
+ "Profiles": [ "ServiceCapacityLow", "LowIoPriority", "HighEnergySaving" ]
}
]
}
diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index e935f99..3834f91 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -144,30 +144,13 @@
return true;
}
-bool SetCgroupAction::IsAppDependentPath(const std::string& path) {
- return path.find("<uid>", 0) != std::string::npos || path.find("<pid>", 0) != std::string::npos;
-}
-
-SetCgroupAction::SetCgroupAction(const CgroupController& c, const std::string& p)
- : controller_(c), path_(p) {
- // file descriptors for app-dependent paths can't be cached
- if (IsAppDependentPath(path_)) {
- // file descriptor is not cached
- fd_.reset(FDS_APP_DEPENDENT);
- return;
- }
-
- // file descriptor can be cached later on request
- fd_.reset(FDS_NOT_CACHED);
-}
-
-void SetCgroupAction::EnableResourceCaching() {
+void CachedFdProfileAction::EnableResourceCaching() {
std::lock_guard<std::mutex> lock(fd_mutex_);
if (fd_ != FDS_NOT_CACHED) {
return;
}
- std::string tasks_path = controller_.GetTasksFilePath(path_);
+ std::string tasks_path = GetPath();
if (access(tasks_path.c_str(), W_OK) != 0) {
// file is not accessible
@@ -185,7 +168,7 @@
fd_ = std::move(fd);
}
-void SetCgroupAction::DropResourceCaching() {
+void CachedFdProfileAction::DropResourceCaching() {
std::lock_guard<std::mutex> lock(fd_mutex_);
if (fd_ == FDS_NOT_CACHED) {
return;
@@ -194,6 +177,26 @@
fd_.reset(FDS_NOT_CACHED);
}
+bool CachedFdProfileAction::IsAppDependentPath(const std::string& path) {
+ return path.find("<uid>", 0) != std::string::npos || path.find("<pid>", 0) != std::string::npos;
+}
+
+void CachedFdProfileAction::InitFd(const std::string& path) {
+ // file descriptors for app-dependent paths can't be cached
+ if (IsAppDependentPath(path)) {
+ // file descriptor is not cached
+ fd_.reset(FDS_APP_DEPENDENT);
+ return;
+ }
+ // file descriptor can be cached later on request
+ fd_.reset(FDS_NOT_CACHED);
+}
+
+SetCgroupAction::SetCgroupAction(const CgroupController& c, const std::string& p)
+ : controller_(c), path_(p) {
+ InitFd(controller_.GetTasksFilePath(path_));
+}
+
bool SetCgroupAction::AddTidToCgroup(int tid, int fd, const char* controller_name) {
if (tid <= 0) {
return true;
@@ -270,7 +273,7 @@
std::string tasks_path = controller()->GetTasksFilePath(path_);
unique_fd tmp_fd(TEMP_FAILURE_RETRY(open(tasks_path.c_str(), O_WRONLY | O_CLOEXEC)));
if (tmp_fd < 0) {
- PLOG(WARNING) << "Failed to open " << tasks_path << ": " << strerror(errno);
+ PLOG(WARNING) << "Failed to open " << tasks_path;
return false;
}
if (!AddTidToCgroup(tid, tmp_fd, controller()->name())) {
@@ -281,37 +284,73 @@
return true;
}
-bool WriteFileAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
- std::string filepath(filepath_), value(value_);
+WriteFileAction::WriteFileAction(const std::string& path, const std::string& value,
+ bool logfailures)
+ : path_(path), value_(value), logfailures_(logfailures) {
+ InitFd(path_);
+}
- filepath = StringReplace(filepath, "<uid>", std::to_string(uid), true);
- filepath = StringReplace(filepath, "<pid>", std::to_string(pid), true);
- value = StringReplace(value, "<uid>", std::to_string(uid), true);
- value = StringReplace(value, "<pid>", std::to_string(pid), true);
+bool WriteFileAction::WriteValueToFile(const std::string& value, const std::string& path,
+ bool logfailures) {
+ // Use WriteStringToFd instead of WriteStringToFile because the latter will open file with
+ // O_TRUNC which causes kernfs_mutex contention
+ unique_fd tmp_fd(TEMP_FAILURE_RETRY(open(path.c_str(), O_WRONLY | O_CLOEXEC)));
- if (!WriteStringToFile(value, filepath)) {
- if (logfailures_) PLOG(ERROR) << "Failed to write '" << value << "' to " << filepath;
+ if (tmp_fd < 0) {
+ if (logfailures) PLOG(WARNING) << "Failed to open " << path;
+ return false;
+ }
+
+ if (!WriteStringToFd(value, tmp_fd)) {
+ if (logfailures) PLOG(ERROR) << "Failed to write '" << value << "' to " << path;
return false;
}
return true;
}
+bool WriteFileAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
+ std::lock_guard<std::mutex> lock(fd_mutex_);
+ std::string value(value_);
+ std::string path(path_);
+
+ value = StringReplace(value, "<uid>", std::to_string(uid), true);
+ value = StringReplace(value, "<pid>", std::to_string(pid), true);
+ path = StringReplace(path, "<uid>", std::to_string(uid), true);
+ path = StringReplace(path, "<pid>", std::to_string(pid), true);
+
+ return WriteValueToFile(value, path, logfailures_);
+}
+
bool WriteFileAction::ExecuteForTask(int tid) const {
- std::string filepath(filepath_), value(value_);
+ std::lock_guard<std::mutex> lock(fd_mutex_);
+ std::string value(value_);
int uid = getuid();
- filepath = StringReplace(filepath, "<uid>", std::to_string(uid), true);
- filepath = StringReplace(filepath, "<pid>", std::to_string(tid), true);
value = StringReplace(value, "<uid>", std::to_string(uid), true);
value = StringReplace(value, "<pid>", std::to_string(tid), true);
- if (!WriteStringToFile(value, filepath)) {
- if (logfailures_) PLOG(ERROR) << "Failed to write '" << value << "' to " << filepath;
+ if (IsFdValid()) {
+ // fd is cached, reuse it
+ if (!WriteStringToFd(value, fd_)) {
+ if (logfailures_) PLOG(ERROR) << "Failed to write '" << value << "' to " << path_;
+ return false;
+ }
+ return true;
+ }
+
+ if (fd_ == FDS_INACCESSIBLE) {
+ // no permissions to access the file, ignore
+ return true;
+ }
+
+ if (fd_ == FDS_APP_DEPENDENT) {
+ // application-dependent path can't be used with tid
+ PLOG(ERROR) << "Application profile can't be applied to a thread";
return false;
}
- return true;
+ return WriteValueToFile(value, path_, logfailures_);
}
bool ApplyProfileAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h
index 97c38f4..278892d 100644
--- a/libprocessgroup/task_profiles.h
+++ b/libprocessgroup/task_profiles.h
@@ -108,50 +108,67 @@
std::string value_;
};
-// Set cgroup profile element
-class SetCgroupAction : public ProfileAction {
+// Abstract profile element for cached fd
+class CachedFdProfileAction : public ProfileAction {
public:
- SetCgroupAction(const CgroupController& c, const std::string& p);
-
- virtual bool ExecuteForProcess(uid_t uid, pid_t pid) const;
- virtual bool ExecuteForTask(int tid) const;
virtual void EnableResourceCaching();
virtual void DropResourceCaching();
- const CgroupController* controller() const { return &controller_; }
- std::string path() const { return path_; }
-
- private:
+ protected:
enum FdState {
FDS_INACCESSIBLE = -1,
FDS_APP_DEPENDENT = -2,
FDS_NOT_CACHED = -3,
};
- CgroupController controller_;
- std::string path_;
android::base::unique_fd fd_;
mutable std::mutex fd_mutex_;
static bool IsAppDependentPath(const std::string& path);
- static bool AddTidToCgroup(int tid, int fd, const char* controller_name);
+ void InitFd(const std::string& path);
bool IsFdValid() const { return fd_ > FDS_INACCESSIBLE; }
+
+ virtual const std::string GetPath() const = 0;
};
-// Write to file action
-class WriteFileAction : public ProfileAction {
+// Set cgroup profile element
+class SetCgroupAction : public CachedFdProfileAction {
public:
- WriteFileAction(const std::string& filepath, const std::string& value,
- bool logfailures) noexcept
- : filepath_(filepath), value_(value), logfailures_(logfailures) {}
+ SetCgroupAction(const CgroupController& c, const std::string& p);
virtual bool ExecuteForProcess(uid_t uid, pid_t pid) const;
virtual bool ExecuteForTask(int tid) const;
+ const CgroupController* controller() const { return &controller_; }
+
+ protected:
+ const std::string GetPath() const override { return controller_.GetTasksFilePath(path_); }
+
private:
- std::string filepath_, value_;
+ CgroupController controller_;
+ std::string path_;
+
+ static bool AddTidToCgroup(int tid, int fd, const char* controller_name);
+};
+
+// Write to file action
+class WriteFileAction : public CachedFdProfileAction {
+ public:
+ WriteFileAction(const std::string& path, const std::string& value, bool logfailures);
+
+ virtual bool ExecuteForProcess(uid_t uid, pid_t pid) const;
+ virtual bool ExecuteForTask(int tid) const;
+
+ protected:
+ const std::string GetPath() const override { return path_; }
+
+ private:
+ std::string path_, value_;
bool logfailures_;
+
+ static bool WriteValueToFile(const std::string& value, const std::string& path,
+ bool logfailures);
};
class TaskProfile {
diff --git a/storaged/Android.bp b/storaged/Android.bp
index f2366cf..7960af3 100644
--- a/storaged/Android.bp
+++ b/storaged/Android.bp
@@ -115,6 +115,9 @@
static_libs: [
"libstoraged",
],
+ test_suites: [
+ "general-tests",
+ ],
}
// AIDL interface between storaged and framework.jar
diff --git a/storaged/tests/storaged_test.cpp b/storaged/tests/storaged_test.cpp
index e987f76..bb71bf3 100644
--- a/storaged/tests/storaged_test.cpp
+++ b/storaged/tests/storaged_test.cpp
@@ -25,6 +25,7 @@
#include <gtest/gtest.h>
+#include <aidl/android/hardware/health/IHealth.h>
#include <healthhalutils/HealthHalUtils.h>
#include <storaged.h> // data structures
#include <storaged_utils.h> // functions to test
@@ -249,6 +250,13 @@
// testing if detect() will return the right value
disk_stats_monitor dsm_detect{healthService};
ASSERT_TRUE(dsm_detect.enabled());
+
+ // Even if enabled(), healthService may not support disk stats. Check if it is supported.
+ std::vector<aidl::android::hardware::health::DiskStats> halStats;
+ if (healthService->getDiskStats(&halStats).getExceptionCode() == EX_UNSUPPORTED_OPERATION) {
+ GTEST_SKIP();
+ }
+
// feed monitor with constant perf data for io perf baseline
// using constant perf is reasonable since the functionality of stream_stats
// has already been tested