diff options
author | Nikita Ioffe <ioffe@google.com> | 2020-05-02 01:28:30 +0100 |
---|---|---|
committer | Nikita Ioffe <ioffe@google.com> | 2020-05-05 22:31:04 +0100 |
commit | 78d2bce6ff69eff15268bdd284d3f4ec76fcae7b (patch) | |
tree | 40e81405b95d7bddf4b93b28e289f5e0067de314 | |
parent | ef2d94a7f2ae11700dbb7f1ce06455d4a1364a61 (diff) | |
download | platform_system_apex-78d2bce6ff69eff15268bdd284d3f4ec76fcae7b.tar.gz platform_system_apex-78d2bce6ff69eff15268bdd284d3f4ec76fcae7b.tar.bz2 platform_system_apex-78d2bce6ff69eff15268bdd284d3f4ec76fcae7b.zip |
apexd: Add remountPackages
This can be used to speed up development cycle for module authors. Here
is an example of how this can be used:
1. adb shell stop
2. adb sync
3. adb shell cmd -w apexservice remountPackages
4. adb shell start
This won't help all the modules (e.g. bootstap apexes or adbd apex
unfortunately can't use this command), but should help modules that only
depend on the framework.
Test: atest ApexTestCases
Test: atest apexd_host_tests
Bug: 155275196
Change-Id: I32fe96385ab0494981c3f4c1fdc7d9c0bcd37b55
-rw-r--r-- | apexd/Android.bp | 1 | ||||
-rw-r--r-- | apexd/aidl/android/apex/IApexService.aidl | 20 | ||||
-rw-r--r-- | apexd/apex_database.cpp | 33 | ||||
-rw-r--r-- | apexd/apex_database.h | 2 | ||||
-rw-r--r-- | apexd/apexd.cpp | 43 | ||||
-rw-r--r-- | apexd/apexd.h | 4 | ||||
-rw-r--r-- | apexd/apexservice.cpp | 51 | ||||
-rw-r--r-- | apexd/apexservice_test.cpp | 100 | ||||
-rw-r--r-- | tests/Android.bp | 1 | ||||
-rw-r--r-- | tests/src/com/android/tests/apex/ApexdHostTest.java | 37 |
10 files changed, 280 insertions, 12 deletions
diff --git a/apexd/Android.bp b/apexd/Android.bp index b071da56..e5c7fd95 100644 --- a/apexd/Android.bp +++ b/apexd/Android.bp @@ -306,6 +306,7 @@ cc_test { ], shared_libs: [ "libbinder", + "libfs_mgr", "libutils", ], test_suites: ["device-tests"], diff --git a/apexd/aidl/android/apex/IApexService.aidl b/apexd/aidl/android/apex/IApexService.aidl index 57784a8b..c4405b8b 100644 --- a/apexd/aidl/android/apex/IApexService.aidl +++ b/apexd/aidl/android/apex/IApexService.aidl @@ -95,4 +95,24 @@ interface IApexService { * functional on user builds. */ void resumeRevertIfNeeded(); + /** + * Forces apexd to remount all active packages. + * + * This call is mostly useful for speeding up development of APEXes. + * Instead of going through a full APEX installation that requires a reboot, + * developers can incorporate this method in much faster `adb sync` based + * workflow: + * + * 1. adb shell stop + * 2. adb sync + * 3. adb shell cmd -w apexservice remountPackages + * 4. adb shell start + * + * Note, that for an APEX package will be successfully remounted only if + * there are no alive processes holding a reference to it. + * + * Not meant for use outside of testing. This call will not be functional + * on user builds. Only root is allowed to call this method. + */ + void remountPackages(); } diff --git a/apexd/apex_database.cpp b/apexd/apex_database.cpp index f8549a1d..d027f95c 100644 --- a/apexd/apex_database.cpp +++ b/apexd/apex_database.cpp @@ -34,6 +34,7 @@ #include <unordered_map> #include <utility> +using android::base::ConsumeSuffix; using android::base::ParseInt; using android::base::ReadFileToString; using android::base::Result; @@ -168,6 +169,25 @@ Result<void> PopulateLoopInfo(const BlockDevice& top_device, return {}; } +// This is not the right place to do this normalization, but proper solution +// will require some refactoring first. :( +// TODO(ioffe): introduce MountedApexDataBuilder and delegate all +// building/normalization logic to it. +void NormalizeIfDeleted(MountedApexData* apex_data) { + std::string_view full_path = apex_data->full_path; + if (ConsumeSuffix(&full_path, "(deleted)")) { + apex_data->deleted = true; + auto it = full_path.rbegin(); + while (it != full_path.rend() && isspace(*it)) { + it++; + } + full_path.remove_suffix(it - full_path.rbegin()); + } else { + apex_data->deleted = false; + } + apex_data->full_path = full_path; +} + Result<MountedApexData> resolveMountInfo(const BlockDevice& block, const std::string& mountPoint) { // Now, see if it is dm-verity or loop mounted @@ -177,9 +197,11 @@ Result<MountedApexData> resolveMountInfo(const BlockDevice& block, if (!backingFile.ok()) { return backingFile.error(); } - return MountedApexData(block.DevPath(), *backingFile, mountPoint, - /* device_name= */ "", - /* hashtree_loop_name= */ ""); + auto result = MountedApexData(block.DevPath(), *backingFile, mountPoint, + /* device_name= */ "", + /* hashtree_loop_name= */ ""); + NormalizeIfDeleted(&result); + return result; } case DeviceMapperDevice: { auto name = block.GetProperty("dm/name"); @@ -192,6 +214,7 @@ Result<MountedApexData> resolveMountInfo(const BlockDevice& block, if (auto status = PopulateLoopInfo(block, &result); !status.ok()) { return status.error(); } + NormalizeIfDeleted(&result); return result; } case UnknownDevice: { @@ -254,7 +277,9 @@ void MountedApexDatabase::PopulateFromMounts() { activeVersions[package] = version; SetLatest(package, mountData->full_path); } - LOG(INFO) << "Found " << mountPoint; + LOG(INFO) << "Found " << mountPoint << " backed by" + << (mountData->deleted ? " deleted " : " ") << "file " + << mountData->full_path; } LOG(INFO) << mounted_apexes_.size() << " packages restored."; diff --git a/apexd/apex_database.h b/apexd/apex_database.h index 5b738b70..1fb5bd3a 100644 --- a/apexd/apex_database.h +++ b/apexd/apex_database.h @@ -38,6 +38,8 @@ class MountedApexDatabase { // Name of the loop device backing up hashtree or empty string in case // hashtree is embedded inside an APEX. std::string hashtree_loop_name; + // Whenever apex file specified in full_path was deleted. + bool deleted; MountedApexData() {} MountedApexData(const std::string& loop_name, const std::string& full_path, diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp index aec354ce..f52781af 100644 --- a/apexd/apexd.cpp +++ b/apexd/apexd.cpp @@ -63,6 +63,7 @@ #include <algorithm> #include <array> #include <chrono> +#include <cstdlib> #include <filesystem> #include <fstream> #include <iomanip> @@ -1820,13 +1821,10 @@ int onBootstrap() { } Result<void> remountApexFile(const std::string& path) { - auto ret = deactivatePackage(path); - if (!ret.ok()) return ret.error(); - - ret = activatePackage(path); - if (!ret.ok()) return ret.error(); - - return {}; + if (auto ret = deactivatePackage(path); !ret.ok()) { + return ret; + } + return activatePackage(path); } void initialize(CheckpointInterface* checkpoint_service) { @@ -2168,5 +2166,36 @@ bool isBooting() { return status != kApexStatusReady && status != kApexStatusActivated; } +Result<void> remountPackages() { + std::vector<std::string> apexes; + gMountedApexes.ForallMountedApexes([&apexes](const std::string& /*package*/, + const MountedApexData& data, + bool latest) { + if (latest) { + LOG(DEBUG) << "Found active APEX " << data.full_path; + apexes.push_back(data.full_path); + } + }); + std::vector<std::string> failed; + for (const std::string& apex : apexes) { + // Since this is only used during development workflow, we are trying to + // remount as many apexes as possible instead of failing fast. + if (auto ret = remountApexFile(apex); !ret) { + LOG(WARNING) << "Failed to remount " << apex << " : " << ret.error(); + failed.emplace_back(apex); + } + } + static constexpr const char* kErrorMessage = + "Failed to remount following APEX packages, hence previous versions of " + "them are still active. If APEX you are developing is in this list, it " + "means that there still are alive processes holding a reference to the " + "previous version of your APEX.\n"; + if (!failed.empty()) { + return Error() << kErrorMessage << "Failed (" << failed.size() << ") " + << "APEX packages: [" << Join(failed, ',') << "]"; + } + return {}; +} + } // namespace apex } // namespace android diff --git a/apexd/apexd.h b/apexd/apexd.h index e757ae68..cde69899 100644 --- a/apexd/apexd.h +++ b/apexd/apexd.h @@ -114,6 +114,10 @@ int snapshotOrRestoreDeUserData(); int unmountAll(); +// Optimistically tries to remount as many APEX packages as possible. +// For more documentation see corresponding binder call in IApexService.aidl. +android::base::Result<void> remountPackages(); + } // namespace apex } // namespace android diff --git a/apexd/apexservice.cpp b/apexd/apexservice.cpp index 24013ff7..b8aced60 100644 --- a/apexd/apexservice.cpp +++ b/apexd/apexservice.cpp @@ -32,6 +32,7 @@ #include <binder/LazyServiceRegistrar.h> #include <binder/ProcessState.h> #include <binder/Status.h> +#include <private/android_filesystem_config.h> #include <utils/String16.h> #include "apex_file.h" @@ -51,6 +52,16 @@ namespace { using BinderStatus = ::android::binder::Status; +BinderStatus CheckCallerIsRoot(const std::string& name) { + uid_t uid = IPCThreadState::self()->getCallingUid(); + if (uid != AID_ROOT) { + std::string msg = "Only root is allowed to call " + name; + return BinderStatus::fromExceptionCode(BinderStatus::EX_SECURITY, + String8(name.c_str())); + } + return BinderStatus::ok(); +} + class ApexService : public BnApexService { public: using BinderStatus = ::android::binder::Status; @@ -88,6 +99,7 @@ class ApexService : public BnApexService { BinderStatus destroyDeSnapshots(int rollback_id) override; BinderStatus destroyCeSnapshotsNotSpecified( int user_id, const std::vector<int>& retain_rollback_ids) override; + BinderStatus remountPackages() override; status_t dump(int fd, const Vector<String16>& args) override; @@ -525,6 +537,22 @@ BinderStatus ApexService::destroyCeSnapshotsNotSpecified( return BinderStatus::ok(); } +BinderStatus ApexService::remountPackages() { + LOG(DEBUG) << "remountPackages() received by ApexService"; + if (auto debug = CheckDebuggable("remountPackages"); !debug.isOk()) { + return debug; + } + if (auto root = CheckCallerIsRoot("remountPackages"); !root.isOk()) { + return root; + } + if (auto res = ::android::apex::remountPackages(); !res.ok()) { + return BinderStatus::fromExceptionCode( + BinderStatus::EX_SERVICE_SPECIFIC, + String8(res.error().message().c_str())); + } + return BinderStatus::ok(); +} + status_t ApexService::onTransact(uint32_t _aidl_code, const Parcel& _aidl_data, Parcel* _aidl_reply, uint32_t _aidl_flags) { switch (_aidl_code) { @@ -632,8 +660,20 @@ status_t ApexService::shellCommand(int in, int out, int err, << std::endl << " getStagedSessionInfo [sessionId] - displays information about a " "given session previously submitted" + << std::endl << " submitStagedSession [sessionId] - attempts to submit the " "installer session with given id" + << std::endl + << " remountPackages - Force apexd to remount active packages. This " + "call can be used to speed up development workflow of an APEX " + "package. Example of usage:\n" + " 1. adb shell stop\n" + " 2. adb sync\n" + " 3. adb shell cmd -w apexservice remountPackages\n" + " 4. adb shell start\n" + "\n" + "Note: APEX package will be successfully remounted only if there " + "are no alive processes holding a reference to it" << std::endl; dprintf(fd, "%s", log.operator std::string().c_str()); }; @@ -852,6 +892,17 @@ status_t ApexService::shellCommand(int in, int out, int err, return BAD_VALUE; } + if (cmd == String16("remountPackages")) { + BinderStatus status = remountPackages(); + if (status.isOk()) { + return OK; + } + std::string msg = StringLog() << "remountPackages failed: " + << status.toString8().string() << std::endl; + dprintf(err, "%s", msg.c_str()); + return BAD_VALUE; + } + if (cmd == String16("help")) { if (args.size() != 1) { print_help(err, "Help has no options"); diff --git a/apexd/apexservice_test.cpp b/apexd/apexservice_test.cpp index 6ea85f1a..2fc3e8b1 100644 --- a/apexd/apexservice_test.cpp +++ b/apexd/apexservice_test.cpp @@ -40,6 +40,7 @@ #include <android-base/strings.h> #include <android/os/IVold.h> #include <binder/IServiceManager.h> +#include <fs_mgr_overlayfs.h> #include <fstab/fstab.h> #include <gmock/gmock.h> #include <gtest/gtest.h> @@ -136,6 +137,21 @@ class ApexServiceTest : public ::testing::Test { static bool IsSelinuxEnforced() { return 0 != security_getenforce(); } + Result<bool> IsActive(const std::string& name) { + std::vector<ApexInfo> list; + android::binder::Status status = service_->getActivePackages(&list); + if (!status.isOk()) { + return Error() << "Failed to check if " << name + << " is active : " << status.exceptionMessage().c_str(); + } + for (const ApexInfo& apex : list) { + if (apex.moduleName == name) { + return true; + } + } + return false; + } + Result<bool> IsActive(const std::string& name, int64_t version, const std::string& path) { std::vector<ApexInfo> list; @@ -552,6 +568,16 @@ std::vector<std::string> ListSlavesOfDmDevice(const std::string& name) { return slaves; } +Result<void> CopyFile(const std::string& from, const std::string& to, + const fs::copy_options& options) { + std::error_code ec; + if (!fs::copy_file(from, to, options)) { + return Error() << "Failed to copy file " << from << " to " << to << " : " + << ec.message(); + } + return {}; +} + } // namespace TEST_F(ApexServiceTest, HaveSelinux) { @@ -2679,6 +2705,80 @@ TEST_F(ApexServiceTest, StageCorruptApexFails_b146895998) { ASSERT_FALSE(IsOk(service_->stagePackages({installer.test_file}))); } +TEST_F(ApexServiceTest, RemountPackagesPackageOnSystemChanged) { + static constexpr const char* kSystemPath = + "/system_ext/apex/apex.apexd_test.apex"; + static constexpr const char* kPackageName = "com.android.apex.test_package"; + if (!fs_mgr_overlayfs_is_setup()) { + GTEST_SKIP() << "/system_ext is not overlayed into read-write"; + } + if (auto res = IsActive(kPackageName); !res.ok()) { + FAIL() << res.error(); + } else { + ASSERT_FALSE(*res) << kPackageName << " is active"; + } + ASSERT_EQ(0, access(kSystemPath, F_OK)) + << "Failed to stat " << kSystemPath << " : " << strerror(errno); + ASSERT_TRUE(IsOk(service_->activatePackage(kSystemPath))); + std::string backup_path = GetTestFile("apex.apexd_test.apexd.bak"); + // Copy original /system_ext apex file. We will need to restore it after test + // runs. + ASSERT_RESULT_OK(CopyFile(kSystemPath, backup_path, fs::copy_options::none)); + + // Make sure we cleanup after ourselves. + auto deleter = android::base::make_scope_guard([&]() { + if (auto ret = service_->deactivatePackage(kSystemPath); !ret.isOk()) { + LOG(ERROR) << ret.exceptionMessage(); + } + auto ret = CopyFile(backup_path, kSystemPath, + fs::copy_options::overwrite_existing); + if (!ret.ok()) { + LOG(ERROR) << ret.error(); + } + }); + + // Copy v2 version to /system_ext/apex/ and then call remountPackages. + PrepareTestApexForInstall installer(GetTestFile("apex.apexd_test_v2.apex")); + if (!installer.Prepare()) { + FAIL() << GetDebugStr(&installer); + } + ASSERT_RESULT_OK(CopyFile(installer.test_file, kSystemPath, + fs::copy_options::overwrite_existing)); + // Don't check that remountPackages succeeded. Most likely it will fail, but + // it should still remount our test apex. + service_->remountPackages(); + + // Check that v2 is now active. + auto active_apex = GetActivePackage("com.android.apex.test_package"); + ASSERT_RESULT_OK(active_apex); + ASSERT_EQ(2u, active_apex->versionCode); + // Sanity check that module path didn't change. + ASSERT_EQ(kSystemPath, active_apex->modulePath); +} + +TEST_F(ApexServiceActivationSuccessTest, RemountPackagesPackageOnDataChanged) { + ASSERT_TRUE(IsOk(service_->activatePackage(installer_->test_installed_file))) + << GetDebugStr(installer_.get()); + // Copy v2 version to /data/apex/active and then call remountPackages. + PrepareTestApexForInstall installer2(GetTestFile("apex.apexd_test_v2.apex")); + if (!installer2.Prepare()) { + FAIL() << GetDebugStr(&installer2); + } + ASSERT_RESULT_OK(CopyFile(installer2.test_file, + installer_->test_installed_file, + fs::copy_options::overwrite_existing)); + // Don't check that remountPackages succeeded. Most likely it will fail, but + // it should still remount our test apex. + service_->remountPackages(); + + // Check that v2 is now active. + auto active_apex = GetActivePackage("com.android.apex.test_package"); + ASSERT_RESULT_OK(active_apex); + ASSERT_EQ(2u, active_apex->versionCode); + // Sanity check that module path didn't change. + ASSERT_EQ(installer_->test_installed_file, active_apex->modulePath); +} + class LogTestToLogcat : public ::testing::EmptyTestEventListener { void OnTestStart(const ::testing::TestInfo& test_info) override { #ifdef __ANDROID__ diff --git a/tests/Android.bp b/tests/Android.bp index 320b666e..ed8ae9c8 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -299,6 +299,7 @@ java_test_host { test_suites: ["general-tests"], data: [ ":apex.apexd_test_v2", + ":com.android.apex.cts.shim.v2_prebuilt", ":com.android.apex.cts.shim.v2_no_pb", ], } diff --git a/tests/src/com/android/tests/apex/ApexdHostTest.java b/tests/src/com/android/tests/apex/ApexdHostTest.java index b7b6a951..2e3a8328 100644 --- a/tests/src/com/android/tests/apex/ApexdHostTest.java +++ b/tests/src/com/android/tests/apex/ApexdHostTest.java @@ -29,6 +29,7 @@ import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test; import org.junit.Test; import org.junit.runner.RunWith; +import java.io.File; import java.time.Duration; import java.util.Set; @@ -38,10 +39,13 @@ import java.util.Set; @RunWith(DeviceJUnit4ClassRunner.class) public class ApexdHostTest extends BaseHostJUnit4Test { + private static final String SHIM_APEX_PATH = "/system/apex/com.android.apex.cts.shim.apex"; + private final ModuleTestUtils mTestUtils = new ModuleTestUtils(this); @Test public void testOrphanedApexIsNotActivated() throws Exception { + assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported()); assumeTrue("Device requires root", getDevice().isAdbRoot()); try { assertThat(getDevice().pushFile(mTestUtils.getTestFile("apex.apexd_test_v2.apex"), @@ -61,8 +65,9 @@ public class ApexdHostTest extends BaseHostJUnit4Test { } @Test public void testApexWithoutPbIsNotActivated() throws Exception { - final String testApexFile = "com.android.apex.cts.shim.v2_no_pb.apex"; + assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported()); assumeTrue("Device requires root", getDevice().isAdbRoot()); + final String testApexFile = "com.android.apex.cts.shim.v2_no_pb.apex"; try { assertThat(getDevice().pushFile(mTestUtils.getTestFile(testApexFile), "/data/apex/active/" + testApexFile)).isTrue(); @@ -79,4 +84,34 @@ public class ApexdHostTest extends BaseHostJUnit4Test { getDevice().executeShellV2Command("rm /data/apex/active/" + testApexFile); } } + + @Test + public void testRemountApex() throws Exception { + assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported()); + assumeTrue("Device requires root", getDevice().isAdbRoot()); + final File oldFile = getDevice().pullFile(SHIM_APEX_PATH); + try { + getDevice().remountSystemWritable(); + // In case remount requires a reboot, wait for boot to complete. + getDevice().waitForBootComplete(Duration.ofMinutes(3).toMillis()); + final File newFile = mTestUtils.getTestFile("com.android.apex.cts.shim.v2.apex"); + // Stop framework + getDevice().executeShellV2Command("stop"); + // Push new shim APEX. This simulates adb sync. + getDevice().pushFile(newFile, SHIM_APEX_PATH); + // Ask apexd to remount packages + getDevice().executeShellV2Command("cmd -w apexservice remountPackages"); + // Start framework + getDevice().executeShellV2Command("start"); + // Give enough time for system_server to boot. + Thread.sleep(Duration.ofSeconds(15).toMillis()); + final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes(); + ITestDevice.ApexInfo testApex = new ITestDevice.ApexInfo( + "com.android.apex.cts.shim", 2L); + assertThat(activeApexes).contains(testApex); + } finally { + getDevice().pushFile(oldFile, SHIM_APEX_PATH); + getDevice().reboot(); + } + } } |