gsid: Make enableGsi() idempotent
Right now doing this leads to an error:
$ gsi_tool disable # success
$ gsi_tool enable # success
$ gsi_tool enable # failure!
because enableGsi() fails if the DSU/GSI installation is already
enabled. This makes switching boot modes rather inconvenient:
$ gsi_tool enable
$ gsi_tool disable # else "enable -s" would fail
$ gsi_tool enable -s
And cause the following workflow a catastrophic failure:
$ Install DSU/GSI with DynamicSystemInstallationService
$ gsi_tool enable
$ Click DynamicSystemInstallationService "Restart"
$ => "Restart" would call gsid::enableGsi() which would fail,
causing the entire DSU installation to be discarded.
This CL makes the above two cases work without error, by making
enableGsi() idempotent, so that successive calls of enableGsi() wouldn't
fail.
Bug: 220978519
Bug: 225310919
Test: Install DSU with DynamicSystemInstallationService
gsi_tool enable
Click DynamicSystemInstallationService "Restart"
=> no failure and boots into GSI with one-shot mode
Change-Id: I8c37d9577dc0da7c13c1a260fa6ace0990c071b1
diff --git a/aidl/android/gsi/IGsiService.aidl b/aidl/android/gsi/IGsiService.aidl
index c889987..3b5d6c0 100644
--- a/aidl/android/gsi/IGsiService.aidl
+++ b/aidl/android/gsi/IGsiService.aidl
@@ -74,8 +74,10 @@
boolean commitGsiChunkFromAshmem(long bytes);
/**
- * Complete a GSI installation and mark it as bootable. The caller is
- * responsible for rebooting the device as soon as possible.
+ * Mark a completed DSU installation as bootable. The caller is responsible
+ * for rebooting the device as soon as possible.
+ *
+ * Could leave the installation in "disabled" state if failure.
*
* @param oneShot If true, the GSI will boot once and then disable itself.
* It can still be re-enabled again later with setGsiBootable.
diff --git a/gsi_service.cpp b/gsi_service.cpp
index 122bbf6..3392a1d 100644
--- a/gsi_service.cpp
+++ b/gsi_service.cpp
@@ -314,12 +314,10 @@
binder::Status GsiService::enableGsiAsync(bool one_shot, const std::string& dsuSlot,
const sp<IGsiServiceCallback>& resultCallback) {
- int result;
- auto status = enableGsi(one_shot, dsuSlot, &result);
- if (!status.isOk()) {
- LOG(ERROR) << "Could not enableGsi: " << status.exceptionMessage().string();
- result = IGsiService::INSTALL_ERROR_GENERIC;
- }
+ ENFORCE_SYSTEM_OR_SHELL;
+ std::lock_guard<std::mutex> guard(lock_);
+
+ const auto result = EnableGsi(one_shot, dsuSlot);
resultCallback->onResult(result);
return binder::Status::ok();
}
@@ -328,20 +326,7 @@
ENFORCE_SYSTEM_OR_SHELL;
std::lock_guard<std::mutex> guard(lock_);
- if (!WriteStringToFile(dsuSlot, kDsuActiveFile)) {
- PLOG(ERROR) << "write failed: " << GetDsuSlot(install_dir_);
- *_aidl_return = INSTALL_ERROR_GENERIC;
- return binder::Status::ok();
- }
- RestoreconMetadataFiles();
-
- if (installer_) {
- LOG(ERROR) << "cannot enable an ongoing installation, was closeInstall() called?";
- *_aidl_return = IGsiService::INSTALL_ERROR_GENERIC;
- return binder::Status::ok();
- }
-
- *_aidl_return = ReenableGsi(one_shot);
+ *_aidl_return = EnableGsi(one_shot, dsuSlot);
return binder::Status::ok();
}
@@ -946,26 +931,6 @@
return kDefaultDsuImageFolder;
}
-int GsiService::ReenableGsi(bool one_shot) {
- if (!android::gsi::IsGsiInstalled()) {
- LOG(ERROR) << "no gsi installed - cannot re-enable";
- return INSTALL_ERROR_GENERIC;
- }
- std::string boot_key;
- if (!GetInstallStatus(&boot_key)) {
- PLOG(ERROR) << "read " << kDsuInstallStatusFile;
- return INSTALL_ERROR_GENERIC;
- }
- if (boot_key != kInstallStatusDisabled) {
- LOG(ERROR) << "GSI is not currently disabled";
- return INSTALL_ERROR_GENERIC;
- }
- if (!SetBootMode(one_shot) || !ResetBootAttemptCounter()) {
- return IGsiService::INSTALL_ERROR_GENERIC;
- }
- return IGsiService::INSTALL_OK;
-}
-
static android::sp<android::os::IVold> GetVoldService() {
return android::waitForService<android::os::IVold>(android::String16("vold"));
}
@@ -1021,6 +986,35 @@
return ok;
}
+int GsiService::EnableGsi(bool one_shot, const std::string& dsu_slot) {
+ if (!android::gsi::IsGsiInstalled()) {
+ LOG(ERROR) << "no gsi installed - cannot enable";
+ return IGsiService::INSTALL_ERROR_GENERIC;
+ }
+ if (installer_) {
+ LOG(ERROR) << "cannot enable an ongoing installation, was closeInstall() called?";
+ return IGsiService::INSTALL_ERROR_GENERIC;
+ }
+
+ if (!DisableGsi()) {
+ PLOG(ERROR) << "cannot write DSU status file";
+ return IGsiService::INSTALL_ERROR_GENERIC;
+ }
+ if (!SetBootMode(one_shot)) {
+ return IGsiService::INSTALL_ERROR_GENERIC;
+ }
+ if (!ResetBootAttemptCounter()) {
+ return IGsiService::INSTALL_ERROR_GENERIC;
+ }
+
+ if (!WriteStringToFile(dsu_slot, kDsuActiveFile)) {
+ PLOG(ERROR) << "cannot write active DSU slot (" << dsu_slot << "): " << kDsuActiveFile;
+ return IGsiService::INSTALL_ERROR_GENERIC;
+ }
+ RestoreconMetadataFiles();
+ return IGsiService::INSTALL_OK;
+}
+
bool GsiService::DisableGsiInstall() {
if (!android::gsi::IsGsiInstalled()) {
LOG(ERROR) << "cannot disable gsi install - no install detected";
diff --git a/gsi_service.h b/gsi_service.h
index 0ec7620..c50c101 100644
--- a/gsi_service.h
+++ b/gsi_service.h
@@ -92,8 +92,8 @@
GsiService();
static int ValidateInstallParams(std::string& install_dir);
+ int EnableGsi(bool one_shot, const std::string& dsu_slot);
bool DisableGsiInstall();
- int ReenableGsi(bool one_shot);
static void CleanCorruptedInstallation();
static int SaveInstallation(const std::string&);
static bool IsInstallationComplete(const std::string&);
