roots: Fix an issue with volume_for_path().

The earlier commit in 2dfc1a3898
unintentionally changed the behavior. It gives a different result when
looking up non-existent mount points (e.g. /cache on marlin).

The logic behind volume_for_path("/xyz") is unclear:
- It's fine to return non-null value if it's called by
  ensure_path_mounted() before accessing that file "/xyz". (Just based
  on the function name, we're not actually having this case.)
- It should return nullptr if the caller is interested in the existence
  of that particular mount point "/xyz".

This CL renames the function to volume_for_mount_point(), which does an
exact match by querying the given mount point from libfs_mgr. The former
volume_for_path() has been moved down to function scope for serving
ensure_path_mounted() only.

Test: Build and boot into recovery on bullhead and marlin respectively.
      'View recovery logs'.
Test: 'Mount /system'
Test: 'Apply update from ADB'
Change-Id: I1a16390f57540cae08a2b8f3d439d17886975217
This commit is contained in:
Tao Bao
2017-09-29 17:11:13 -07:00
parent e8ee697364
commit 3e18f2bf40
4 changed files with 11 additions and 5 deletions

View File

@@ -653,7 +653,7 @@ int install_package(const std::string& path, bool* wipe_cache, const std::string
std::chrono::duration<double> duration = std::chrono::system_clock::now() - start;
int time_total = static_cast<int>(duration.count());
bool has_cache = volume_for_path("/cache") != nullptr;
bool has_cache = volume_for_mount_point("/cache") != nullptr;
// Skip logging the uncrypt_status on devices without /cache.
if (has_cache) {
static constexpr const char* UNCRYPT_STATUS = "/cache/recovery/uncrypt_status";

View File

@@ -1396,7 +1396,7 @@ int main(int argc, char **argv) {
printf("Starting recovery (pid %d) on %s", getpid(), ctime(&start));
load_volume_table();
has_cache = volume_for_path(CACHE_ROOT) != nullptr;
has_cache = volume_for_mount_point(CACHE_ROOT) != nullptr;
std::vector<std::string> args = get_args(argc, argv);
std::vector<char*> args_to_parse(args.size());

View File

@@ -69,11 +69,15 @@ void load_volume_table() {
printf("\n");
}
Volume* volume_for_mount_point(const std::string& mount_point) {
return fs_mgr_get_entry_for_mount_point(fstab, mount_point);
}
// Finds the volume specified by the given path. fs_mgr_get_entry_for_mount_point() does exact match
// only, so it attempts the prefixes recursively (e.g. "/cache/recovery/last_log",
// "/cache/recovery", "/cache", "/" for a given path of "/cache/recovery/last_log") and returns the
// first match or nullptr.
Volume* volume_for_path(const char* path) {
static Volume* volume_for_path(const char* path) {
if (path == nullptr || path[0] == '\0') return nullptr;
std::string str(path);
while (true) {

View File

@@ -17,13 +17,15 @@
#ifndef RECOVERY_ROOTS_H_
#define RECOVERY_ROOTS_H_
#include <string>
typedef struct fstab_rec Volume;
// Load and parse volume data from /etc/recovery.fstab.
void load_volume_table();
// Return the Volume* record for this path (or NULL).
Volume* volume_for_path(const char* path);
// Return the Volume* record for this mount point (or nullptr).
Volume* volume_for_mount_point(const std::string& mount_point);
// Make sure that the volume 'path' is on is mounted. Returns 0 on
// success (volume is mounted).