From a799a588423df683493ddd6a73ee6585205be741 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Mon, 31 Oct 2022 19:19:52 +0800 Subject: [PATCH] remount: Use MyLogger class and sundry improvements * Use the MyLogger class from set-verity-state.cpp and eliminate the |verbose| global variable. * Support specifying reboot() reason. Log error if reboot() failed to reboot. * Move precondition checks after all |argv| are processed. This way (-h) help message can be shown w/o root user. * Log "remount [succeeded|failed]" message before auto-reboot. Give users a clear succeeded/failed message. Bug: 241688845 Test: adb-remount-test Change-Id: If45de2eba0d532632de43b19c797ffdeea90cd6d --- fs_mgr/fs_mgr_remount.cpp | 95 ++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp index 2edaaad24..dfc89836c 100644 --- a/fs_mgr/fs_mgr_remount.cpp +++ b/fs_mgr/fs_mgr_remount.cpp @@ -79,23 +79,32 @@ const FstabEntry* GetWrappedEntry(const Fstab& overlayfs_candidates, const Fstab return &(*it); } -auto verbose = false; +class MyLogger { + public: + explicit MyLogger(bool verbose) : verbose_(verbose) {} -void MyLogger(android::base::LogId id, android::base::LogSeverity severity, const char* tag, - const char* file, unsigned int line, const char* message) { - if (verbose || severity == android::base::ERROR || message[0] != '[') { - fprintf(stderr, "%s\n", message); + void operator()(android::base::LogId id, android::base::LogSeverity severity, const char* tag, + const char* file, unsigned int line, const char* message) { + // By default, print ERROR logs and logs of this program (does not start with '[') + // Print [libfs_mgr] INFO logs only if -v is given. + if (verbose_ || severity >= android::base::ERROR || message[0] != '[') { + fprintf(stderr, "%s\n", message); + } + logd_(id, severity, tag, file, line, message); } - static auto logd = android::base::LogdLogger(); - logd(id, severity, tag, file, line, message); -} -[[noreturn]] void reboot() { + private: + android::base::LogdLogger logd_; + bool verbose_; +}; + +[[noreturn]] void reboot(const std::string& name) { LOG(INFO) << "Rebooting device for new settings to take effect"; ::sync(); - android::base::SetProperty(ANDROID_RB_PROPERTY, "reboot,remount"); + android::base::SetProperty(ANDROID_RB_PROPERTY, "reboot," + name); ::sleep(60); - ::exit(0); // SUCCESS + LOG(ERROR) << "Failed to reboot"; + ::exit(1); } static android::sp GetVold() { @@ -465,29 +474,11 @@ int main(int argc, char* argv[]) { return 0; } - android::base::InitLogging(argv, MyLogger); - - // Make sure we are root. - if (::getuid() != 0) { - LOG(ERROR) << "Not running as root. Try \"adb root\" first."; - return 1; - } - - // If somehow this executable is delivered on a "user" build, it can - // not function, so providing a clear message to the caller rather than - // letting if fall through and provide a lot of confusing failure messages. - if (!ALLOW_ADBD_DISABLE_VERITY || !android::base::GetBoolProperty("ro.debuggable", false)) { - LOG(ERROR) << "Device must be userdebug build"; - return 1; - } - - if (android::base::GetProperty("ro.boot.vbmeta.device_state", "") == "locked") { - LOG(ERROR) << "Device must be bootloader unlocked"; - return 1; - } + android::base::InitLogging(argv, MyLogger(false /* verbose */)); const char* fstab_file = nullptr; - auto auto_reboot = false; + bool auto_reboot = false; + bool verbose = false; std::vector partition_args; struct option longopts[] = { @@ -507,7 +498,7 @@ int main(int argc, char* argv[]) { break; case 'T': if (fstab_file) { - LOG(ERROR) << "Cannot supply two fstabs: -T " << fstab_file << " -T" << optarg; + LOG(ERROR) << "Cannot supply two fstabs: -T " << fstab_file << " -T " << optarg; usage(); return 1; } @@ -517,16 +508,39 @@ int main(int argc, char* argv[]) { verbose = true; break; default: - LOG(ERROR) << "Bad Argument -" << char(opt); + LOG(ERROR) << "Bad argument -" << char(opt); usage(); return 1; } } + if (verbose) { + android::base::SetLogger(MyLogger(verbose)); + } + for (; argc > optind; ++optind) { partition_args.emplace_back(argv[optind]); } + // Make sure we are root. + if (::getuid() != 0) { + LOG(ERROR) << "Not running as root. Try \"adb root\" first."; + return 1; + } + + // If somehow this executable is delivered on a "user" build, it can + // not function, so providing a clear message to the caller rather than + // letting if fall through and provide a lot of confusing failure messages. + if (!ALLOW_ADBD_DISABLE_VERITY || !android::base::GetBoolProperty("ro.debuggable", false)) { + LOG(ERROR) << "Device must be userdebug build"; + return 1; + } + + if (android::base::GetProperty("ro.boot.vbmeta.device_state", "") == "locked") { + LOG(ERROR) << "Device must be bootloader unlocked"; + return 1; + } + // Make sure checkpointing is disabled if necessary. if (auto rv = VerifyCheckpointing(); rv != REMOUNT_SUCCESS) { return rv; @@ -549,7 +563,11 @@ int main(int argc, char* argv[]) { } else if (check_result.setup_overlayfs) { LOG(INFO) << "Overlayfs enabled."; } - + if (result == REMOUNT_SUCCESS) { + LOG(INFO) << "remount succeeded"; + } else { + LOG(ERROR) << "remount failed"; + } if (check_result.reboot_later) { if (auto_reboot) { // If (1) remount requires a reboot to take effect, (2) system is currently @@ -560,16 +578,11 @@ int main(int argc, char* argv[]) { LOG(ERROR) << "Unable to automatically enable DSU"; return rv; } - reboot(); + reboot("remount"); } else { LOG(INFO) << "Now reboot your device for settings to take effect"; } return REMOUNT_SUCCESS; } - if (result == REMOUNT_SUCCESS) { - printf("remount succeeded\n"); - } else { - printf("remount failed\n"); - } return result; }