From 2285b528de460e2100f6852522963e6ac8ca18b4 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 9 Nov 2021 18:26:39 -0800 Subject: [PATCH] init: Add a way to class_restart only enabled services. class_restart accidentally restarts disabled services. Changing this behavior is risky as it could break compatibility. Instead, add an "--only-enabled" argument to class_restart to opt-in to the new functionality. This syntax is backward compatible, as previously only a 1-argument form was accepted. Bug: 190065372 Bug: 198105685 Test: add a class_restart action and a disabled service, make sure service is not restarted. Change-Id: Idb08779de7ac7a21e23f8b8a3276bd5a66a43299 --- init/README.md | 5 +++-- init/builtins.cpp | 28 ++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/init/README.md b/init/README.md index b10ca9e2b..ebab07359 100644 --- a/init/README.md +++ b/init/README.md @@ -496,8 +496,9 @@ Commands currently running, without disabling them. They can be restarted later using `class_start`. -`class_restart ` -> Restarts all services of the specified class. +`class_restart [--only-enabled] ` +> Restarts all services of the specified class. If `--only-enabled` is + specified, then disabled services are skipped. `copy ` > Copies a file. Similar to write, but useful for binary/large diff --git a/init/builtins.cpp b/init/builtins.cpp index 50a0cb2ca..98831e10d 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -190,7 +190,31 @@ static Result do_class_restart(const BuiltinArguments& args) { // Do not restart a class if it has a property persist.dont_start_class.CLASS set to 1. if (android::base::GetBoolProperty("persist.init.dont_start_class." + args[1], false)) return {}; - ForEachServiceInClass(args[1], &Service::Restart); + + std::string classname; + + CHECK(args.size() == 2 || args.size() == 3); + + bool only_enabled = false; + if (args.size() == 3) { + if (args[1] != "--only-enabled") { + return Error() << "Unexpected argument: " << args[1]; + } + only_enabled = true; + classname = args[2]; + } else if (args.size() == 2) { + classname = args[1]; + } + + for (const auto& service : ServiceList::GetInstance()) { + if (!service->classnames().count(classname)) { + continue; + } + if (only_enabled && !service->IsEnabled()) { + continue; + } + service->Restart(); + } return {}; } @@ -1392,7 +1416,7 @@ const BuiltinFunctionMap& GetBuiltinFunctionMap() { {"chmod", {2, 2, {true, do_chmod}}}, {"chown", {2, 3, {true, do_chown}}}, {"class_reset", {1, 1, {false, do_class_reset}}}, - {"class_restart", {1, 1, {false, do_class_restart}}}, + {"class_restart", {1, 2, {false, do_class_restart}}}, {"class_start", {1, 1, {false, do_class_start}}}, {"class_stop", {1, 1, {false, do_class_stop}}}, {"copy", {2, 2, {true, do_copy}}},