| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Currently it's not possible to know from these logs if the backup was
cancelled due to timeout or the app crashing or due to backups being
explicitly cancelled by an external caller. This CL fixes that issue.
Flag: EXEMPT log only update
Test: manual // PFTBT is notoriously impossible to write unit tests for.
Bug: 399645990
Change-Id: Ia18fdec1d9547130926bf62666ac055e320fb0b6
|
|
agent pipe is broken" into main
|
|
|
|
pipe is broken
Normally the bug fix and the extra logging should be in separate CLs.
However as explained in the linked bug, there is a race condition
between the method throwing the IOException returning and us getting a
cancellation callback from ActivityManager. If we get the callback
first, the session fails. The extra IPC from the logging in handling the
exception means the callback gets a few additional milliseconds to win
the race. So with the logging, the race condition is much more likely to
happen and the logging should not be checked in without the bug fix. And
if one gets rolled back, the other should as well. This is why I made
them a single CL.
Flag: EXEMPT bugfix
Bug: 400669414
Test: manual // PFTBT is notoriously impossible to write unit tests for.
Change-Id: Icbcb72a876f927a94675c9837b0206f1a631dff5
|
|
In `dispatchNextRestore` we ask the transport for the next package to
restore. If this package is null, we immediately abort the restore. This
can happen when the transport encounters some permanent failure and
returns null, or when the transport disappears and the
`BackupTransportClient` returns null.
The problem with the current implementation is that it doesn't set
`mStatus` to `TRANSPORT_ERROR`, which means the restore session is
reported to have ended successfully. This means we won't attempt to
retry the restore.
See also ag/26064663.
Bug: 397944266
Bug: 381408714
Flag: EXEMPT bugfix
Test: manually tested
Test: presubmits
Change-Id: I40f4406e4180f3224ab3a5d7c32e4c7028d6484e
|
|
This is to give more granular information to tasks about why they got
cancelled. This will help them report why they failed.
The cancelAll parameter was only set to false for cancellations due to
timeouts, so I've removed it.
This is a no-op refactor.
Flag: EXEMPT refactor
Bug: 399645990
Test: atest CtsBackupTestCases
Change-Id: I1bd6d9eff42c5edeb0f4df453bae792cf994bb6a
|
|
The currentPackage member variable in PFTBT is not necessary and is set
to the last package that was passed in to the constructor. Then all
events are sent with this package name even if they are for different
packages.
Bug: 396664485
Flag: EXEMPT Bugfix
Test: presubmit
Change-Id: Icd6c04399d6e5345a94dfdf4bb46e15928dcca7a
|
|
This is to avoid concurrent modification errors.
Fixes: 390483439
Test: atest CtsBackupTestCases
Flag: EXEMPT bug fix
Change-Id: I8d75ea1fc15705ee847e911452b6ba0029d7a000
|
|
And format it properly. The variable formats better compared to a method
call.
Test: n/a
Flag: EXEMPT no-op refactor
Change-Id: I617ee8d1b05a65e21b408186cf2c25cae76ed7bb
|
|
There's no reason for it to be under UserBackupManagerService and bloat
it up.
Test: n/a
Flag: EXEMPT no-op refactor
Change-Id: I595fe26e97e804e59033a5dc95e8fa8e16de895f
|
|
The existing DEBUG and DEBUG_SCHEDULING had been set to true (maybe
accidentally?) for 8+ years. Whereas MORE_DEBUG is false as expected. In
this CL we:
1) Remove existing DEBUG and DEBUG_SCHEDULING checks. This is a no-op.
2) Rename MORE_DEBUG to DEBUG. Also no-op.
3) Make any Log.v log that isn't guarded by if(DEBUG) a Log.d instead.
Log.v logs should always be guarded by a default false constant so
the compiler strips out the Strings from the binary.
Test: n/a
Flag: EXEMPT no-op refactor
Change-Id: I117f7fe04da1a62c3ad42d71b080fed6454f73af
|
|
Currently these go through the IBackupManager.aidl interface, which is
unnecessary since they are local calls within the system server.
Test: atest CtsBackupTestCases
Flag: EXEMPT no-op refactor
Change-Id: I260197d30184d10a92f356f729d28fd0ab22aec1
|
|
There have been a few reports of the backup service failing to call
release() on the wakelock and draining battery for hours. These are rare
failures and hard to debug individually but with the timeout we will
limit the worst case significantly.
I am setting the default timeout to 30 minutes. This should be more than
enough time for a backup or restore operation. If in the future these
operation get longer, the timeout is configurable by a setting.
Test: atest CtsBackupTestCases
Fixes: 364931501
Fixes: 357769443
Fixes: 289789401
Flag: EXEMPT bugfix
Change-Id: Iaac0218d5258031b6f67c3c56c776ce098e8a8c4
|
|
This is to prevent a rare crash that can happen if something releases
the wakelock beforehand. I'm not sure exactly what sequence of events
leads to the crash scenario but this seems like a sensible check
to have anyway since we have multiple operations that can
acquire/release the wakelock.
Flag: EXEMPT bugfix
Fixes: 349280741
Test: CtsBackupTestCases, CtsBackupHostTestCases
Change-Id: I4a5c7ea0cf381de051a4a4afdf2426f9715515c8
|
|
It seems this was intended to be removed for S. I've also double checked
this setting isn't used anywhere that I can find.
Fixes: 154822946
Test: presubmit
Change-Id: I974b30a54883280d16dacf9a2de876d87f1e2c13
|
|
Also respect the killAfterRestore flag for them.
Before this change, the android:killAfterRestore attribute was only
respected for apps that do Key/Value backups. This was because Full
backup apps used to always be started in restricted mode for B&R
operations and the framework needed to kill the app to reset the
restricted mode state.
Now, full backup apps may not be started in restricted mode
(after ag/30155472) so there's no need to kill them unless if they set
killAfterRestore.
For this to work, we need to keep track of whether an app is in
restricted mode. I've moved all connection/disconnection logic to the
new connection manager class so this is now fairly easy.
Note that apps will only not be in restricted mode if they are K/V or if
enable_restricted_mode_changes is enabled and they opted out. So I think
all the new behavior is guarded by the same flag.
Flag: com.android.server.backup.enable_restricted_mode_changes
Bug: 376661510
Test: atest BackupAgentConnectionManagerTest & atest CtsBackupHostTestCases
Change-Id: I1e94fb3b794961904a6c603ac34b00f3878f4e3e
|
|
We have a 10s timeout in bindToAgentSynchronous and if we don't get an
agentConnected callback by then the method returns null. There was a
potential bug here because agentConnected doesn't check if the agent is
the one we are currently expecting. It could happen with the following
sequence of events:
1. bindToAgentSynchronous(A)
2. app A times out
3. bindToAgentSynchronous(B)
4. agentConnected(A)
5. the call from 3 returns the agent for app A instead of B.
With this change we start tracking the state of the current connection
better.
Test: atest BackupAgentConnectionManagerTest & atest
CtsBackupHostTestCases
Bug: 328019933
Change-Id: I0556aff00fe8c2bbfe56fe10f15b7b15ff260590
Change-Id: I4c04e9279d9863716fc080c6ab218665e1591d51
|
|
This is to isolate some scope of UserBackupManagerService. It's too big
and unreadable.
I've also added some tests. But since these are blocking and async
operations, I couldn't see any way other than adding some thread sleeps
to avoid flakiness.
These tests could still end up being too flaky. If that's the case we
might have to leave this code untested :(
This is a no-op refactor.
Bug: 376661510
Flag: EXEMPT refactor
Test: atest BackupAgentConnectionManagerTest & atest CtsBackupHostTestCases
Change-Id: I9893336fb224148ce5b2f3c6fa2fc26828d2a1e9
|
|
In this change we introduce a new ApplicationManifest property which
allows apps to specify whether they want to be put in restricted mode
for B&R operations. If the app explicitly set the property, it's always
respected.
If the app has not set the property then we use targetSdk gating:
* For targetSdk < 36, we keep the status quo and put the app into
restricted mode.
* For targetSdk >= 36, we call a new API to the BackupTransport for it
to make a decision on a per-package basis.
Some implementation details explained:
* In order to not block process creation in ActivityManager on an IPC to
the BackupTransport, we call the transport earlier in
PerformFullTransportBackupTask (for backup) and
PerforUnifiedRestoreTask (for restore). We cache the list in memory in
BackupManagerService.
* When AMS#bindBackupAgent is called, the BackupManagerService tells it
whether to use restricted mode. AMS stores this in the existing
BackupRecord data class and uses it in attachApplication().
* PerformUnifiedRestoreTask is an extremely untestable state machine and
testing this properly is difficult without a significant rewrite so
I'm using @VisibleForTesting.
* Seems like AMS#attachApplication also requires a lot of mocking so
couldn't add tests there.
Flag: com.android.server.backup.enable_restricted_mode_changes
Bug: 376661510
Test: atest (see tests changed) and manually with a test app that
defined its own Application subclass.
API-Coverage-Bug: 379086316
Change-Id: Ie060890131ba526d58ec9134e52fd80acc23ef63
|
|
Allow TelephonyProvider to be eligible for backup&restore on non-user-0
for HSUM devices, since on HSUM the provider really belongs to the
MainUser.
Test: manual backup and restore of messages on HSUM device
Bug: 369560193
Flag: EXEMPT bugfix
Change-Id: I4dfd4287a1f291b8c7ec3c2075a109696ffb8baa
|
|
Bug: 358316882
Test: not needed, just adding a javadoc comment
Flag: DOCS_ONLY
Change-Id: Icf5fe938dd9cd3efdf03a781e019d602f9e3919b
|
|
Bug: 351750747
Test: atest -v TarBackupReaderTest, manual test
Flag: EXEMPT bugfix
Change-Id: Icf4fe058e7d261c197e04ccad940595413bd4a99
|
|
|
|
A previous change (ag/26748034) added this call for BMM logging. getPackageInfo is a relatively heavy binder call, which triggered performance regression alerts (b/342082681 and b/342273383). The additional getPackageInfo call resulted in 5-10% increase in latency for package commit operation.
For the purposes of BMM logging in restoreAtInstall flow, we just need the name of the package. Fields like version code are already logged in subsequent BMM logging during restore.
Bug: 342082681
Test: Verified that logging is correct
Change-Id: I8025789b2c696eac1b7738a1ae332c67aea8ce5f
|
|
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/27486895
Change-Id: I552392cf12b59ba0c931cbc246bd3791f799162b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
|
|
Add a check in TarBackupReader to enable the restore of packages in the V_To_U_Allowlist in the V to U downgrade scenario
Bug: 341732443
Test: atest -v TarBackupReaderTest, manual test (details: https://b.corp.google.com/issues/341732443#comment18)
Change-Id: I12a1f272b17a12abb532a98c0d93f0a9854fcb51
|
|
When `TransportConnection` handles `onServiceConnected` or
`onBindingDied` it calls `Context.unbindService`. In a very small number
of cases this fails because the connection is not (or no longer?)
registered. When this happens, the system server crashes causing the
device to reboot. We don't really care if the service is not registered
when we try to unbind it, so we just catch and ignore the exception to
avoid crashing the system server.
Bug: 335547110
Test: atest TransportConnectionTest.java
Change-Id: Ic7f34eacd1f621c42a68128ef992043dba6808f1
|
|
introduced after ag/26833810
Change-Id: I501151d4d73490b3f6f3c0326e741ec046cad36b
|
|
This is a re-submit of the previously reverted ag/22327091 by
piee@google.com.
Original description:
This change refactors `dumpsys backup` code and adds some improvements -
- Removes `isUserReadyForBackup()` check on calling user. This check is not needed, as it prevents a User with INTERACT_ACROSS_USERS_FULL permission, but not Backup activated, to get info about Backup service on other users. This is particularly relevant in case of headless mode, as Backup service isn't running for SYSTEM_USER.
- Adds `--user <userId>` optional parameter for `dumpsys backup`, which will return info about Backup service for the user passed in argument. This is also helpul in case of multiple users on the device.
Changes by sarpm@google.com over the original:
- Some test improvements: remove @VisibleForTesting, increase
coverage slightly.
- Remove redundant robo tests that are now covered by the unit
tests.
Bug: 267755698
Test: atest BackupManagerServiceTest
Manual test by running dumpsys backup with optional arguments.
Change-Id: Id7790a501a8befb12a212f66adfc0cd8cc9f9e8b
|
|
24D1-dev am: 6afb3e754e
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/26748034
Change-Id: I5216ce32ba2c6b7db3e8cbe1dd8af816f958455d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
|
|
This change adds BMM logging in restore at install flow right from the point when PackageManager triggers restore.
The change also adds more detailed logging in Full Restore flow, around various failure points. This'll help with quantify different failures that happen in Full Restore flow, some of which are masked by the complexity in Framework code.
Bug: 331749778
Test: Ensure that the change builds (m -j frameworks/base)
Ensure CTS and GTS B&R tests pass
Change-Id: Ieff6df433398ffff71c7eeda73174f9f1d509b58
|
|
When the PackageManagerBackupAgent restores an old backup that does not
have an ancestral record version and the backup contains a package that
does not have any signatures it will enter an infinite loop. This
happens because it calls `continue` without reading the next key/value
pair in the backup.
Test: atest PackageManagerBackupAgentTest
Bug: 330708733
Change-Id: Ifb71293b75ccb68dc7e721593f04128417360491
|
|
The ancestral record version is a number that's contained in the backup
created by the PackageManagerBackupAgent. It's used to indicate how to
restore the backed up data. This allows the PackageManagerBackupAgent to
change how it stores its data without worrying that it may break other
versions of the backup agent during restore.
Backup agents should only back up key/value pairs that haven't changed
since the last time a backup was done. It should do this by maintaining
a local state. The agent is given the state file from the previous
backup run (if any) and is given a file to write the new state to.
The ancestral record version that the PackageManagerBackupAgent writes
to the backup almost never changes. However, this key/value pair was not
stored in the local state. As a result, the backup agent backs up this
key/value pair on every backup causing unnecessary backup traffic.
This change fixes that by storing the ancestral record version in the
local state. It then avoids backing it up again when it's present in the
local state.
It's possible for the ancestral record version in the state file to be
different from the current value in the backup agent, for example after
a system update. If this happens that means a change was made to the
format of the data we back up, so we intentionally back up all key/value
pairs to ensure they're all written in the new format. In addition,
there may be key/value pairs that were backed up in the past but are no
longer backed up (e.g. an app was uninstalled). We remove these
key/value pairs since they were created with the older ancestral record
version but we can't update them.
Bug: 268341874
Test: atest PackageManagerBackupAgentTest.java
Change-Id: Iaa805198801b38b0c4867536749095fff243e18e
|
|
Change-Id: I28490272c9e739802288f40078432750fc985fc6
Bug: 277594991
Test: atest PackageManagerBackupAgentTest.java
Change-Id: I13744aaf54e546ab696f98cd775a4b93da6bc4d5
|
|
This is a pure refactor. The following changes were made:
- run `google-java-format --aosp`
- move some non-javadoc method comments inside that method
- add curly braces where required
- add @Override annotations for `onBackup` and `onRestore`.
Bug: 277594991
Test: m services.backup
Change-Id: I091586d1d1b8f008e6d5a6262b5916eb8998b7dc
|
|
Add logs so that we can record the V to U scenario in the backup
dumpsys and in clearcut
We only print the allowlist/denylist once in the backup dumpsys (at the beginning of system restore)
Test: manual (check that the VtoU events appear in the dumpsys - See for an example: https://paste.googleplex.com/4697312460275712#), atest PerformUnifiedRestoreTaskTest, atest BackupManagerMonitorUtilsTest
Bug: 324233962
Change-Id: I07c483b80093037e36076bc174ef6d9484f01d65
|
|
We read the value of list from secure settings. So we move the lists after transport.startRestore, given that the corresponding secure settings are set during transport.startRestore
Test: manual (check that the VtoU secure settings are set by the time the lists are created), atest PerformUnifiedRestoreTaskTest
Bug: 324233962
Change-Id: Icc442f344106c1cecd764e2004523e82649bcf59
|
|
A package is eligible for V to U downgrade restore if either:
- The package has restoreAnyVersion set to false and is part of the V to U allowst
- The package has restoreAnyVersion set to true and is not part of the denylist
Bug : 324233962
Test: atest PerformUnifiedRestoreTaskTest
Manual:check that a package with restoreanyversion=true
is not restored if in the denylist,
check that a package with restoreanyversion=false
is restored if in the allowlist
Change-Id: Id1cf031c593730132ebe24ecab0ffc4a3f87920e
|
|
`PerformUnifiedRestoreTask` is a state-machine that is responsible for
restoring the backups a given set of packages. At every point in this
process it's possible for a transport error to occur. This is treated as
a fatal error, immediately moving the state machine into the `FINAL`
state.
The `mStatus` variable is used to indicate the result in the
`RestoreObserver.restoreFinished` callback. A non-zero value for
`mStatus` indicates something failed during restore.
`dispatchNextRestore` is the only place in this state machine where a
transport error does not set `mStatus` to `TRANSPORT_ERROR`, even though
the state is set to `FINAL`. As a result, the `RestoreObserver` listener
is likely to wrongly assume that the restore succeeded in this
particular case.
Bug: 283932000
Test: atest services/tests/mockingservicestests/src/com/android/server/backup
Test: Manual testing with custom build that returns an error in `dispatchNextRestore`.
Change-Id: I769e8200ad4ac12b58dbb8c8181b21ff9933453a
|
|
PerformUnifiedRestoreTask uses the `RestoreObserver` to report its
progress as it restores packages. The `onUpdate` method takes two
parameters: the package index and the package name.
PerformUnifiedRestoreTask keeps track of the package index by counting
the number of packages being restored. The count was only incremented
after successfully restoring a key-value app. This meant that the wrong
package index was reported when a key-value app was not restored (e.g.
when it was skipped, or the restore failed), or when a non-key-value app
was restored.
This change fixes that by incrementing the index for every package and
before the restore starts.
Bug: 291249666
Test: Manually performed a system restore (see linked bug for details)
Change-Id: I3bdb8efb3c7deab47bf801e10c42ef2e8fd29182
|
|
There are helper methods for calling the restore observer, but there was
still one place that didn't use it.
Bug: 291249666
Test: atest services/tests/mockingservicestests/src/com/android/server/backup/restore/
Test: manually checked logs when doing a restore
Change-Id: I1f8628c339830a4e9b55243a3fefe4f04ce7bd3e
|
|
from/to pipes in Backup/Restore flow.
ag/21072118 used of feature flags to control buffer sizes, which are now deprecated. Those flags were also never used.
This change makes use of trunk-stable frozen flags to set the buffer size to match Linux pipe internal buffer when reading/writing from/to pipes in Backup/Restore flow.
See 'Pipe capacity' section in https://man7.org/linux/man-pages/man7/pipe.7.html
Bug: 265976737
Test: Ensure that the change builds (m -j frameworks/base)
Ensure CTS and GTS B&R tests pass
Change-Id: I8bb96e2e273b0c7eb4809af985fe322e49bb0268
|
|
I've formatted these files with `google-java-format --aosp` before
making further changes. All checkstyle errors and parameter comment
style have also been fixed.
Bug: 291249666
Test: atest PerformUnifiedRestoreTaskTest
Change-Id: Ia3e7751d7477e067aa6956dffc9819690adba1e2
|
|
This is to ensure we don't overwrite existing state. The behavior will
be controlled by the BackupTransport implementation that provides the
restore data.
Bug: 308401499
Test: atest BackupEligibilityRulesTest
atest ActiveRestoreSessionTest
Manual: perform initial restore, verify all apps restore, rerun
restore for already restored / launched apps, verify no restore
Change-Id: I6ee1a2aca49224f0228f7ad340d3424fbe50e7bb
|
|
933caefe55
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/24770152
Change-Id: I27c8b8db185a3650cbfc40d15d044560b16ea6b4
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
|
|
* changes:
Implement the ability to log extras relating to version mismatches between packages on the source and target. Imporve readability of events in dumpsys.
Add a 2.5 MB size limit to the text file storing BMM Events.
Add more BackupManagerMonitor events to PerformUnifiedRestoreTask.
Add a 60 days retention period to BMM Events
Overload putMonitoringExtra so that it can take an int as a parameter. This will allow to correcly store EXTRA_LOG_OPERATION_TYPE
|
|
packages on the source and target.
Imporve readability of events in dumpsys.
Test: manual, do restore on test device, verify that the correct events
are printed in the dumpsys
atest CtsBackupHostTestCases, GtsBackupHostTestCases
atest BackupManagerMonitorDumpsysUtilsTest
Bug: 296818666
Change-Id: Ifd2d68861fcd33637c49f0e7802c456e43ffd9a2
|
|
Test: atest CtsBackupHostTestCases, GtsBackupHostTestCases
atest BackupManagerMonitorDumpsysUtilsTest, UserBackupManagerServiceTest
Bug: 297163567
Change-Id: Ia1fdc0b465b960337f5e5c626460e305d96f4eb2
|
|
In particular
add events to cover
-If this is KV or Full restore
-For each package, when the restore started and when it ended
-When a restore operation starts, if it is system restore or restore at install
-Any errors
Test: manual testing. Run `adb shell bmgr restore 1` and verify that the
new restore events are added to the dumpsys
atest CtsBackupHostTestCases, GtsBackupHostTestCases
atest BackupManagerMonitorDumpsysUtilsTest,
BackupManagerMonitorEventSenderTest, UserBackupManagerServiceTest,
PerformUnifiedRestoreTaskTest, BmgrTest
Bug: 290747920
Change-Id: I0d221f10932fea3e8fb90a1827c7f1b5bf21d25d
|