This removes all remaining save/restores of errno in
fs_mgr_overlayfs.cpp.
Bug: 241179247
Test: adb-remount-test.sh
Change-Id: I8bae6eb2752fe9460763455e88f9b82ad57c10e4
This patch eliminates errno as part of the return contract for
fs_mgr_overlayfs_teardown().
The non-standard use of implicit errno makes it extremely difficult to
reason about how these functions can fail. As it turns out,
fs_mgr_overlayfs_teardown has been consistently failing for a long time,
but in a place where errno isn't set, which meant "enable-verity" never
saw the failure.
The failure was originating from umount2(MNT_DETACH) which guaranteed
that DeleteBackingImage would fail with EBUSY, and DeleteBackingImage is
a binder call that doesn't set errno.
This patch switches to umount() and returns a "busy" status if the
unmount fails with EBUSY. In this case it will also disable the scratch
partition. There is a long-standing existing bug where, for non-VAB
devices, it will delete the underlying scratch partition off super. This
is pretty risky with MNT_DETACH, but that path is left unchanged here.
Some duplicated code in set-verity-state was refactored as well, since
the return value of fs_mgr_overlayfs_teardown is now more complex.
Bug: 241179247
Test: adb-remount-test.sh
Change-Id: I2ca75332b75a302622ba9b86d122a6f2accdda3e
* 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
It's useful to have a group that shares a numeric id with Linux and
macOS. "root" doesn't count because group 0 is actually called "wheel"
on the BSDs, and macOS has "daemon" and "kmem" as its groups 1 and 2.
This lets us have toybox tar tests that have the same results on all
supported OSes without special handling.
Test: treehugger
Change-Id: I4704d6f9ada84f3065831a099b34d1c20c4c6b03
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
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
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
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
* 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
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
The function objects work equally well without them, and the base
classes were wrong for both types:
* HashForEntry: returns size_t but declared to return hash_t
(uint32_t)
* EqualityForHashedEntries: returns bool and takes two parameters but
declared to return hash_t and take one parameter
std::unary_function was deprecated in C++11 and removed in C++17.
Upstream libc++ now removes the type for new-enough C++ dialects.
Bug: http://b/175635923
Test: treehugger
Change-Id: I2ff15c5da6a4e4f71df08c243f8af2f11d8d2b0d
* According to healthd_mode_charger.cpp#L106 this should be
res/images/default/charger and not res/images/charger/default
Change-Id: Iabb5f5ecdd3b94c90a4b46849dd0a048da51364a
