* changes:
adb-remount-test: Refactor test cleanup
adb-remount-test: Replace libc.so test with build.prop test
adb-remount-test: Check override_creds only if overlayfs is used
adb-remount-test: Print log timestamp & auto-detect color
* Call adb_wait in adb_reboot, as virtually all adb_reboot callsites are
immediately followed by adb_wait.
* Remove |data| option from skip_administrative_mounts. The |data|
option doesn't really work anyway, because vold & init creates
bewildering heirarchy of /data bind-mounts, so it's not feasible to
filter /data by mountpoints. It's more sensible to filter by the /data
device node name, which should be done by the caller.
* Untangle skip_administrative_mounts and skip_unrelated_mounts.
I don't know why we need two separate functions that do similar
things. Just merge them together.
Bug: 243116800
Test: adb-remount-test
Change-Id: I847f0b8cc2a952bb4c8656a43da783f312670061
* If adb remount calls for a reboot during cleanup, this means it's
trying to recreate vendor overlay. Don't reboot in this case because
it's pointless. Total test runtime reduced by one reboot.
* Since this entire script assumes /system & /vendor must exist and
remountable, add them to the MOUNTS list unconditionally.
* Remove /system/hello & /vendor/hello test, as we can just loop over
MOUNTS to check those.
Bug: 243116800
Test: adb-remount-test
Change-Id: I2360314c404ee247356146760314c91ba2795ff5
Since /bionic mountpoint is deprecated, we don't _have_ to explicitly
check consistency of /system/lib/bootstrap/libc.so anymore.
Remove the test which adds junk to the end of libc.so. Editing libc.so
looks dangerous (albeit not!) and has unclear expectations.
Add test which edits /system/build.prop. Editing build.prop file is more
"safe" as it's just text edit, and the expectations are clear (edited
system properties should be loaded after reboot.)
Bug: 243116800
Test: adb-remount-test
Change-Id: I51bd32c6ffcc57eb646eeec0537e996847e6c2a5
Instead of probing the kernel to see if overlayfs is supported, just
check `df` after disable-verity.
If after disable-verity and overlays were mounted, then check that
override_creds patches are applied.
Bug: 243116800
Test: adb-remount-test
Change-Id: Icb1363278536a8177836263882b1a8a0d9f246c9
* Change --print-time to default true, and print timestamp of each log
message.
* Auto-detect color support. If stdout if terminal, then color default
to true, else default to false.
Bug: 243116800
Test: adb-remount-test
Change-Id: Id8425488c4b18fe0bc4dd7e50c3e2ae2e8c74cfe
Move "raw remount test" right after "disable-verity -R test".
Device is expected to be in a clean state right after disable-verity, so
we can perform "raw remount test" immediately after. This saves us one
reboot.
Move "remount from scratch test" right before "remount -R test".
Since they both require overlay teardown state, group them together so
we only need to teardown (and reboot) once. This saves us one reboot.
Total test runtime reduced by two reboots.
Bug: 243116800
Test: adb-remount-test
Change-Id: Ifd95ba713f1819a7d31e88cd70077dc306c64c58
Instead of relying on a local dev tree (which CI machines never have),
just pull the vendor partition image from device.
This way we can have CI coverage on fastbootd as well.
Stop redefining cleanup() hook, just toss all temporary files to $TMPDIR
and always clean up $TMPDIR on exit.
Clarify logs and error messages.
Bug: 243116800
Test: adb-remount-test
Change-Id: I08fb8df58a61c03db3274b22b51e40a1a8f41095
* Check mount flag changes (ro/rw) before and after "adb remount".
* Add comment explaining what's going on with the |uses_dynamic_scratch|
and |scratch_partition| variables.
* Add rich logs reporting infomation about the scratch partition.
* Add rich error messages.
* Filter out /data devices and external volumes (vold managed device)
when checking RW partitions. We are only interested in system
partitions.
* Remove redundant "remount from setup" test from end-of-file, as they
are testing the same thing as the refactored remount test. Total test
runtime reduced by one reboot.
Bug: 243116800
Test: adb-remount-test
Change-Id: Icda5bff78372bebfe2e166d8537a06be66fff886
Pick exactly one fstab file whose pathname suffix matches one of the
fstab suffix properties.
This helps on CF who ships redundant copies of fstab.
Bug: 243116800
Test: adb-remount-test
Change-Id: I4d38859014161e14dba1f7e19dbce44a2621d0f1
There's quite a lot of noise from running "Checking current overlayfs
status". Improve the test output by filtering uninteresting df lines.
* "/apex/..." mounts not interesting.
* "rw" mounts not interesting.
* "fuse" devices not interesting.
Bug: 243116800
Test: adb-remount-test
Change-Id: Id15844d853aaf3f7ed86f1a83544494b697b5b39
* When guessing the ANDROID_SERIAL, use output of `adb devices` instead
of ro.serialno, because ro.serialno won't work for network devices.
* Ensure ANDROID_SERIAL is exported so the test don't fail if a new
device is plugged into the host machine mid test.
* Change --wait-screen warning to info. The "warning" isn't helpful as
it's not showing any potential problems.
* Register cleanup hooks to EXIT trap. This ensures cleanup code are
always executed, and failure to clean up counts as test failure.
* Rewrite some unnecessarily complex command chaining to plain exit
status check.
* Use `test` command to test file existence. Don't use `ls` or `cat` to
test file as this isn't their intended usage, and parsing their error
output can be finicky.
Bug: 243116800
Fixes: 178256393
Test: adb-remount-test
Change-Id: Iec4224d8a236a9852ce417b1129c27205d435d5b
Since remount -R and disable-verity -R have similar expectations, group
them together and reuse each other's test code.
Remove the redundant "remount -R" test at end of file.
Total test runtime reduced by one reboot.
Bug: 243116800
Test: adb-remount-test
Change-Id: I510a9de39f94b73450df9abf82a55496df96bea1
While doing precondition check, verify that device is debuggable and
unlocked.
The /sys/module/overlay mining code had some remarkably written chained
... && ... || ... expressions. This is also remarkably unreadable for
those untrained of bash command chaining pitfalls.
Just rewrite these with plain old if-then-else expressions.
Bug: 243116800
Test: adb-remount-test
Change-Id: I56b1dea5b9147755a43462682a51bc5802ee64c1
The redefining of cleanup hooks are making the script rather difficult
to read. Instead of redefining restore() just to skip some parts of it,
let restore() check flags and conditionally execute cleanup code.
Bug: 243116800
Test: adb-remount-test
Change-Id: If9d627618b54e215200455e8133492670737571d
Just use bash [[ for regex compare, which result in shorter code and
more robust.
Simplify the messaging pipeline:
If success, don't print anything and return 0. Let caller decide what
to log.
If failure and --warning, log error message and return 1.
If failure and ! --warning, die with error message.
Bug: 243116800
Test: adb-remount-test
Change-Id: Ie5426ff3fa57395aa6b4fe71c9bf96bd8e9afc35
Right now there are a lot of log commands in the form of
"echo <color code><log type><color code> [msg]... >&2"
which is painful to read, and test writers often accidentally omit the
trailing ">&2".
Add a LOG() function which takes care of the log formatting and stderr
redirecting once and for all.
Also bulk edit existing log commands to use LOG() everywhere.
Bug: 243116800
Test: adb-remount-test --color
Change-Id: I04beb9e09b28c08a3a6f4309bf2d4b6de906df90
The use of errno in this function is very difficult to reason about, and
leads to a lot of complexity (eg saving and restoring errno on a case by
case basis).
This CL adds explicit logging in error paths and simplifies the return
state to "succeeded" or "failed".
In addition, the "change" outparam has been simplified as well.
Previously it indicated that *anything* in the filesystem changed. This
is not super useful since the only thing callers care about is whether
or not overlayfs went from "disabled" to "enabled". The outparam now
reflects that.
Bug: 241179247
Test: remount
Change-Id: I5a2b4dcc942e6807c9965cd484de152b47022c4e
Right now some test output are print to stdout and some to stderr.
Stdout mostly contain output of test commands.
Stderr mostly contain test result and device diagnostic status.
The logs in both streams also don't have timestamps, so separating the
two streams would be incredibly unuseful, because it would be very
difficult to deduce the causuality between the log lines.
In practice only the concatenated log stream is useful, so let's just
redirect all meaningful logs to stderr for good measure.
Why not stdout? Because stdout is often captured by command
substitution as command output.
foo() {
echo "Log nessage..." >&2
echo "function output..."
}
A=$(foo)
"Log message..." would go to stderr, and "function output..." would be
captured into ${A}.
Bug: 243116800
Test: adb-remount-test
Change-Id: I692a1b6cf352681cca65354688908e4becf9d31a
Remove the ERR trap handler as it doesn't work as intended and is rather
finicky. (Read more: mywiki.wooledge.org/BashFAQ/105)
Trap ERR (and set -e) could be an useful tool if applied sparingly, like
in a subshell, but they seem almost useless, even harmful, if applied
globally due to the following reasons.
The problems it brings includes but not limited to:
* ERR trap handler doesn't propagate inside subshells and functions.
This makes it rather useless for reporting unchecked errors.
* Set '-o errtrace' kind of fixes previous issue, but it would report
superfluous errors, because as the non-zero error code propagates up
the call stack, each subshell expression would evaluate to non-zero
status, repeatedly triggering the trap handler.
* Breaks the rather common "execute comand and check ${?}" pattern in
this script, for example:
H=$(adb remount)
[ "${?}" != 0 ] && echo warning....
script would prematurely exit if $(adb remount) fails, not having a
chance to recover from error.
--
`expr ...` is problematic because it exits with non-zero status if the
calculated result is zero. This makes ordinary harmless looking
expressions, which evaluates perfectly fine, to exit with error status
A=$(expr 1 + 1) # $? = 0
A=$(expr 1 - 1) # $? = 1
Just replace all `expr` with the more robust `$(( ... ))` construct.
--
Also fix typo scratch_paritition -> scratch_partition.
Bug: 243116800
Test: adb-remount-test.sh
Change-Id: I2a8941341a7a12359896f0e08ecd21766396eb45
Don't try to parse stdout of disable-verity, just pass -R to ask for
auto reboot.
This eliminates the complex logic of "disable-verity && check &&
reboot && disable-verity again ...", and increase robustness.
`${overlayfs_supported} && ${overlayfs_needed}` can be simplified to
just `${overlayfs_needed}` because `${overlayfs_needed}` implies
`${overlayfs_supported}`.
Move the curious recurring "overlay takeover unexpected" check to the
"Checking current overlayfs status" section so we don't need to repeat
it so many times.
Bug: 243116800
Bug: 241688845
Test: adb-remount-test
Change-Id: I96ec44e2b9d172c06c3b4850e061e7b6bb46833c
The option was added for debugging purposes in case the "test argv[0] ==
clean_scratch_files" method breaks.
Now it has no user at all and no reason to maintain it, so just remove it.
Also don't use MyLogger() when running as clean_scratch_files, as
clean_scratch_files don't have stdout/stderr.
Bug: 241179247
Test: Presubmit
Change-Id: I2d8069f59fe6b85fc84ab07bb2df6efb39d6ecaa
Reorder COW ops vector based on merge sequence. We don't
need additional vector to be stored in memory.
Memory usage for a full OTA on Pixel:
Without Patch:
RssAnon: 61020 kB
With Patch:
RssAnon: 51112 kB
Bug: 237490659
Test: OTA on Pixel
Signed-off-by: Akilesh Kailash <akailash@google.com>
Change-Id: I543dd73acfa7cf4e57379e82bc184e943072e7c8
This test has always been flaky, and is not testing something super
valuable: we know that image creation succeeds throughout the rest of
the suite, so it's not very interesting to know that it can succeed in a
low-space scenario.
The inverse test is much more valuable, since we want the correct status
code when creation fails due to low space.
Bug: 240391002
Test: vts_libsnapshot_test
Change-Id: I6235d11033d2f30efe530077b877863ba2574810
Diagnosing DM_DEV_REMOVE failures in the test harness is quite
difficult, and it's not clear if failures are spurious or not. Instead
use SnapshotManager's helper function, which can retry on failure, and
will self-diagnose issues on legitimate failures.
Bug: N/A
Test: vts_libsnapshot_test
Change-Id: Ibcaa8406e8b1e8758b99a8e9b58c58d68ed57685
These checks have historically been unreliable, and we make no
guarantees around dev_t with overlayfs.
Bug: 242240650
Test: adb-remount-test.sh
Change-Id: I19e7aabec424a22beb0b56d35b198906841178b0
aosp/I212bdb0e97016dec50618962d7c24f46d35764c7 changes
`remount` to return MUST_REBOOT after initial overlay setup.
This causes DsuGsiIntegrationTest to fail because the non-zero exit code
cause the test script to think the `remount` command failed.
Change it so that we return 0 instead, to indicate that we
"successfully" setup a new overlay.
We should only return non-zero on unrecoverable error, like when we
failed to disable verity, failed to setup overlay, failed to perform
MS_REMOUNT.
Bug: 241179247
Test: DsuGsiIntegrationTest
Change-Id: I280ffa988118c59e366cdd5bd1479bb43896c278
Cuttlefish's combined fstab has two entries for every partition, which
causes a lot of error spam when remount fails. Fix this by only
remounting entries that match an actual mount point (if such a mount
point exists).
Bug: 241179247
Test: remount on broken kernel
Change-Id: I3ddab553706f98b45f83221fd195f481dfdcc5c0
This moves more logic out of do_remount and into main(). This eliminates
some redundant code.
Bug: 241179247
Test: remount
Change-Id: I212bdb0e97016dec50618962d7c24f46d35764c7
If next same mount point also is logical partition, but its blk_device
is not updated yet and still is logical partition name not a mapper
device (/dev/block/dm-X) to cause mount failed.
To support the below fstab configs in fstab.postinstall.
system /postinstall ext4 ro,nosuid,nodev,noexec slotselect_other,logical
system /postinstall erofs ro,nosuid,nodev,noexec slotselect_other,logical
Error logs:
system_b: Can't open blockdev
init: [libfs_mgr]__mount(source=system_b(missing),target=/postinstall,type=erofs)=-1: No such file or directory
Bug: b/241716684
Test: After factory device reset, ensure system_b is mounted as EROFS
Change-Id: I02f20f3dfd8c42be9981915eaff88a5948482724
AutoDevice has an explicitly deleted move constructor, so its
derived types cannot be default-movable.
Bug: 241680050
Test: presubmit
Change-Id: I65de669a55fd9923c9a1f25a7fdbd16770e2f101
* changes:
remount: Refactor remounting into a helper function.
remount: Refactor dsu enabling into a helper function.
remount: Refactor verity disabling into a helper function.
There is a small change of behavior in this patch. If "remount"
specifies a list of arguments, and one of them is invalid, it will now
error rather than try to remount the remaining partitions.
Bug: 241179247
Test: remount
Change-Id: I7a9ef41886f32ae97173796358b41844a1e42ea8
Using vector + unordered_map to retrieve the index in
a COW op vector consumes significant memory; this
is a problem especially when there are hundreds
of thousands of operations.
Instead, just store the index of the COW op vector
during pre-processing.
On Pixel, peak memory usage when all the partitions
are mapped:
Without patch:
RssAnon: 118804 kB
With path:
RssAnon: 55772 kB
Additionally, post OTA reboot, memory usage further goes
down as the partition merge completes.
Bug: 238052240
Test: OTA on Pixel
Signed-off-by: Akilesh Kailash <akailash@google.com>
Change-Id: Icc68a9688ceb89572821cee2dac689779f5e7c11
