From 1318983d20262576b9333087487773804fb7a88a Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 17 Nov 2020 11:14:18 +0800 Subject: [PATCH] Record reason why the rollback is deleted (5/n) It would be useful to know the reason why a rollback is deleted, e.g explicit expiration or session error. Bug: 172644981 Test: atest RollbackStoreTest Change-Id: I0ca6d0831af2071ee9ce86453ccb9e6882fd2bf3 --- .../com/android/server/rollback/Rollback.java | 37 +++++++++++++++---- .../rollback/RollbackManagerServiceImpl.java | 27 +++++++------- .../server/rollback/RollbackStore.java | 4 +- .../server/rollback/RollbackStoreTest.java | 3 ++ .../server/rollback/RollbackUnitTest.java | 8 ++-- 5 files changed, 54 insertions(+), 25 deletions(-) diff --git a/services/core/java/com/android/server/rollback/Rollback.java b/services/core/java/com/android/server/rollback/Rollback.java index 2d51cf9a5083d..63ed416f2859f 100644 --- a/services/core/java/com/android/server/rollback/Rollback.java +++ b/services/core/java/com/android/server/rollback/Rollback.java @@ -141,10 +141,16 @@ class Rollback { /** * The current state of the rollback. - * ENABLING, AVAILABLE, or COMMITTED. + * ENABLING, AVAILABLE, DELETED, or COMMITTED. */ private @RollbackState int mState; + /** + * The detailed description of the current state. For a DELETED state, it describes + * the reason why the rollback is deleted. + */ + private @NonNull String mStateDescription = ""; + /** * True if we are expecting the package manager to call restoreUserData * for this rollback because it has just been committed but the rollback @@ -231,7 +237,7 @@ class Rollback { * Constructs a pre-populated Rollback instance. */ Rollback(RollbackInfo info, File backupDir, Instant timestamp, int stagedSessionId, - @RollbackState int state, boolean restoreUserDataInProgress, + @RollbackState int state, String stateDescription, boolean restoreUserDataInProgress, int userId, String installerPackageName, SparseIntArray extensionVersions) { this.info = info; mUserId = userId; @@ -240,6 +246,7 @@ class Rollback { mTimestamp = timestamp; mStagedSessionId = stagedSessionId; mState = state; + mStateDescription = stateDescription; mRestoreUserDataInProgress = restoreUserDataInProgress; mExtensionVersions = Objects.requireNonNull(extensionVersions); // TODO(b/120200473): Include this field during persistence. This field will be used to @@ -478,7 +485,7 @@ class Rollback { Slog.w(TAG, "Cannot make deleted rollback available."); return; } - mState = ROLLBACK_STATE_AVAILABLE; + setState(ROLLBACK_STATE_AVAILABLE, ""); mTimestamp = Instant.now(); RollbackStore.saveRollback(this); } @@ -598,7 +605,7 @@ class Rollback { // Why would we expect commit not to fail again? // TODO: Could this cause a rollback to be resurrected // if it should otherwise have expired by now? - mState = ROLLBACK_STATE_AVAILABLE; + setState(ROLLBACK_STATE_AVAILABLE, "Commit failed"); mRestoreUserDataInProgress = false; info.setCommittedSessionId(-1); sendFailure(context, statusReceiver, @@ -642,7 +649,7 @@ class Rollback { }; final LocalIntentReceiver receiver = new LocalIntentReceiver(onResult); - mState = ROLLBACK_STATE_COMMITTED; + setState(ROLLBACK_STATE_COMMITTED, ""); info.setCommittedSessionId(parentSessionId); mRestoreUserDataInProgress = true; parentSession.commit(receiver.getIntentSender()); @@ -691,7 +698,7 @@ class Rollback { * Deletes app data snapshots associated with this rollback, and moves to the DELETED state. */ @WorkerThread - void delete(AppDataRollbackHelper dataHelper) { + void delete(AppDataRollbackHelper dataHelper, @NonNull String reason) { assertInWorkerThread(); boolean containsApex = false; Set apexUsers = new ArraySet<>(); @@ -717,7 +724,7 @@ class Rollback { } RollbackStore.deleteRollback(this); - mState = ROLLBACK_STATE_DELETED; + setState(ROLLBACK_STATE_DELETED, reason); } /** @@ -847,6 +854,7 @@ class Rollback { case Rollback.ROLLBACK_STATE_ENABLING: return "enabling"; case Rollback.ROLLBACK_STATE_AVAILABLE: return "available"; case Rollback.ROLLBACK_STATE_COMMITTED: return "committed"; + case Rollback.ROLLBACK_STATE_DELETED: return "deleted"; } throw new AssertionError("Invalid rollback state: " + state); } @@ -858,6 +866,7 @@ class Rollback { case "enabling": return Rollback.ROLLBACK_STATE_ENABLING; case "available": return Rollback.ROLLBACK_STATE_AVAILABLE; case "committed": return Rollback.ROLLBACK_STATE_COMMITTED; + case "deleted": return Rollback.ROLLBACK_STATE_DELETED; } throw new ParseException("Invalid rollback state: " + state, 0); } @@ -926,6 +935,7 @@ class Rollback { ipw.println(info.getRollbackId() + ":"); ipw.increaseIndent(); ipw.println("-state: " + getStateAsString()); + ipw.println("-stateDescription: " + mStateDescription); ipw.println("-timestamp: " + getTimestamp()); if (getStagedSessionId() != -1) { ipw.println("-stagedSessionId: " + getStagedSessionId()); @@ -955,4 +965,17 @@ class Rollback { } ipw.decreaseIndent(); } + + @WorkerThread + String getStateDescription() { + assertInWorkerThread(); + return mStateDescription; + } + + @VisibleForTesting + void setState(@RollbackState int state, String description) { + assertInWorkerThread(); + mState = state; + mStateDescription = description; + } } diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 6602bba84872b..192a00303a305 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -203,7 +203,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba } else { // Delete rollbacks when build fingerprint has changed. for (Rollback rollback : mRollbacks) { - deleteRollback(rollback); + deleteRollback(rollback, "Fingerprint changed"); } mRollbacks.clear(); } @@ -273,7 +273,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba Rollback rollback = getRollbackForSession(sessionId); if (rollback != null && rollback.isEnabling()) { mRollbacks.remove(rollback); - deleteRollback(rollback); + deleteRollback(rollback, "Rollback canceled"); } } } @@ -479,14 +479,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba } @WorkerThread - private void expireRollbackForPackageInternal(String packageName) { + private void expireRollbackForPackageInternal(String packageName, String reason) { assertInWorkerThread(); Iterator iter = mRollbacks.iterator(); while (iter.hasNext()) { Rollback rollback = iter.next(); if (rollback.includesPackage(packageName)) { iter.remove(); - deleteRollback(rollback); + deleteRollback(rollback, reason); } } } @@ -498,7 +498,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba mContext.enforceCallingOrSelfPermission( Manifest.permission.TEST_MANAGE_ROLLBACKS, "expireRollbackForPackage"); - awaitResult(() -> expireRollbackForPackageInternal(packageName)); + awaitResult(() -> expireRollbackForPackageInternal(packageName, "Expired by API")); } @ExtThread @@ -614,7 +614,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba .getPackageInstaller().getSessionInfo(rollback.getStagedSessionId()); if (session == null || session.isStagedSessionFailed()) { iter.remove(); - deleteRollback(rollback); + deleteRollback(rollback, + "Session " + session.getSessionId() + " not existed or failed"); continue; } @@ -668,7 +669,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba && rollback.includesPackageWithDifferentVersion(packageName, installedVersion)) { iter.remove(); - deleteRollback(rollback); + deleteRollback(rollback, "Package " + packageName + " replaced"); } } } @@ -680,7 +681,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba @WorkerThread private void onPackageFullyRemoved(String packageName) { assertInWorkerThread(); - expireRollbackForPackageInternal(packageName); + expireRollbackForPackageInternal(packageName, "Package " + packageName + " removed"); } /** @@ -722,7 +723,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba if (!now.isBefore(rollbackTimestamp.plusMillis(mRollbackLifetimeDurationInMillis))) { Slog.i(TAG, "runExpiration id=" + rollback.info.getRollbackId()); iter.remove(); - deleteRollback(rollback); + deleteRollback(rollback, "Expired by timeout"); } else if (oldest == null || oldest.isAfter(rollbackTimestamp)) { oldest = rollbackTimestamp; } @@ -1134,7 +1135,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba Slog.w(TAG, "Delete rollback id=" + rollback.info.getRollbackId() + " for failed session id=" + sessionId); mRollbacks.remove(rollback); - deleteRollback(rollback); + deleteRollback(rollback, "Session " + sessionId + " failed"); } } } @@ -1161,7 +1162,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba if (!rollback.allPackagesEnabled()) { Slog.e(TAG, "Failed to enable rollback for all packages in session."); mRollbacks.remove(rollback); - deleteRollback(rollback); + deleteRollback(rollback, "Failed to enable rollback for all packages in session"); return false; } @@ -1345,9 +1346,9 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub implements Rollba } @WorkerThread - private void deleteRollback(Rollback rollback) { + private void deleteRollback(Rollback rollback, String reason) { assertInWorkerThread(); + rollback.delete(mAppDataRollbackHelper, reason); mRollbackStore.saveRollbackToHistory(rollback); - rollback.delete(mAppDataRollbackHelper); } } diff --git a/services/core/java/com/android/server/rollback/RollbackStore.java b/services/core/java/com/android/server/rollback/RollbackStore.java index 83976cc4076d6..35c9f9ae66834 100644 --- a/services/core/java/com/android/server/rollback/RollbackStore.java +++ b/services/core/java/com/android/server/rollback/RollbackStore.java @@ -278,6 +278,7 @@ class RollbackStore { dataJson.put("timestamp", rollback.getTimestamp().toString()); dataJson.put("stagedSessionId", rollback.getStagedSessionId()); dataJson.put("state", rollback.getStateAsString()); + dataJson.put("stateDescription", rollback.getStateDescription()); dataJson.put("restoreUserDataInProgress", rollback.isRestoreUserDataInProgress()); dataJson.put("userId", rollback.getUserId()); dataJson.putOpt("installerPackageName", rollback.getInstallerPackageName()); @@ -302,7 +303,7 @@ class RollbackStore { } /** - * Saves the rollback to mRollbackHistoryDir/ROLLBACKID-HEX for debugging purpose. + * Saves the rollback to $mRollbackHistoryDir/ROLLBACKID-HEX for debugging purpose. */ void saveRollbackToHistory(Rollback rollback) { // The same id might be allocated to different historical rollbacks. @@ -345,6 +346,7 @@ class RollbackStore { Instant.parse(dataJson.getString("timestamp")), dataJson.getInt("stagedSessionId"), rollbackStateFromString(dataJson.getString("state")), + dataJson.optString("stateDescription"), dataJson.getBoolean("restoreUserDataInProgress"), dataJson.optInt("userId", UserHandle.SYSTEM.getIdentifier()), dataJson.optString("installerPackageName", ""), diff --git a/services/tests/servicestests/src/com/android/server/rollback/RollbackStoreTest.java b/services/tests/servicestests/src/com/android/server/rollback/RollbackStoreTest.java index 96bfc98aaefc6..c42f936d3ab4d 100644 --- a/services/tests/servicestests/src/com/android/server/rollback/RollbackStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/rollback/RollbackStoreTest.java @@ -204,6 +204,8 @@ public class RollbackStoreTest { origRb.info.getPackages().add(pkgInfo1); origRb.info.getPackages().add(pkgInfo2); + origRb.setState(Rollback.ROLLBACK_STATE_AVAILABLE, "hello world"); + RollbackStore.saveRollback(origRb); List loadedRollbacks = mRollbackStore.loadRollbacks(); @@ -355,6 +357,7 @@ public class RollbackStoreTest { assertThat(b.isEnabling()).isEqualTo(a.isEnabling()); assertThat(b.isAvailable()).isEqualTo(a.isAvailable()); assertThat(b.isCommitted()).isEqualTo(a.isCommitted()); + assertThat(b.getStateDescription()).isEqualTo(a.getStateDescription()); assertThat(b.isStaged()).isEqualTo(a.isStaged()); diff --git a/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java b/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java index cd2c9230221c4..cf1ed4815a748 100644 --- a/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java +++ b/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java @@ -123,7 +123,7 @@ public class RollbackUnitTest { public void deletedRollbackCannotBeMadeAvailable() { Rollback rollback = new Rollback(123, new File("/test/testing"), -1, USER, INSTALLER); - rollback.delete(mMockDataHelper); + rollback.delete(mMockDataHelper, "test"); assertThat(rollback.isDeleted()).isTrue(); @@ -221,7 +221,7 @@ public class RollbackUnitTest { PackageRollbackInfo pkgInfo2 = newPkgInfoFor(PKG_2, 18, 12, true); rollback.info.getPackages().addAll(Arrays.asList(pkgInfo1, pkgInfo2)); - rollback.delete(mMockDataHelper); + rollback.delete(mMockDataHelper, "test"); assertThat(rollback.isDeleted()).isTrue(); @@ -247,7 +247,7 @@ public class RollbackUnitTest { verify(mMockDataHelper).snapshotAppData(eq(123), pkgRollbackInfoFor(PKG_2), eq(userIds)); - rollback.delete(mMockDataHelper); + rollback.delete(mMockDataHelper, "test"); verify(mMockDataHelper).destroyAppDataSnapshot(eq(123), pkgRollbackInfoFor(PKG_2), eq(111)); verify(mMockDataHelper).destroyAppDataSnapshot(eq(123), pkgRollbackInfoFor(PKG_2), eq(222)); @@ -269,7 +269,7 @@ public class RollbackUnitTest { verify(mMockDataHelper).snapshotAppData(eq(123), pkgRollbackInfoFor(PKG_2), eq(userIds)); - rollback.delete(mMockDataHelper); + rollback.delete(mMockDataHelper, "test"); verify(mMockDataHelper, never()) .destroyAppDataSnapshot(anyInt(), pkgRollbackInfoFor(PKG_2), anyInt());