OMS: extract verifyIdmap from createIdmap
For clarity, split IIdmap2::createIdmap into two separate functions:
- IIdmap2::verifyIdmap [check if an existing idmap file is OK to use]
- IIdmap2::createIdmap [unconditionally (re)create an idmap file]
Teach the IdmapManager to call verifyIdmap and to proceed with
createIdmap only if actually needed.
Test: atest OverlayDeviceTests OverlayHostTests
Change-Id: I9f6f1192011fcb094adffeca1eb3f709520bbd24
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index cf72cb9..86b00f1 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -78,6 +78,18 @@
}
}
+Status Idmap2Service::verifyIdmap(const std::string& overlay_apk_path,
+ int32_t user_id ATTRIBUTE_UNUSED, bool* _aidl_return) {
+ assert(_aidl_return);
+ const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
+ std::ifstream fin(idmap_path);
+ const std::unique_ptr<const IdmapHeader> header = IdmapHeader::FromBinaryStream(fin);
+ fin.close();
+ std::stringstream dev_null;
+ *_aidl_return = header && header->IsUpToDate(dev_null);
+ return ok();
+}
+
Status Idmap2Service::createIdmap(const std::string& target_apk_path,
const std::string& overlay_apk_path, int32_t user_id,
std::unique_ptr<std::string>* _aidl_return) {
@@ -90,17 +102,6 @@
_aidl_return->reset(nullptr);
- const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
- std::ifstream fin(idmap_path);
- const std::unique_ptr<const IdmapHeader> header = IdmapHeader::FromBinaryStream(fin);
- fin.close();
- // do not reuse error stream from IsUpToDate below, or error messages will be
- // polluted with irrelevant data
- std::stringstream dev_null;
- if (header && header->IsUpToDate(dev_null)) {
- return ok();
- }
-
const std::unique_ptr<const ApkAssets> target_apk = ApkAssets::Load(target_apk_path);
if (!target_apk) {
return error("failed to load apk " + target_apk_path);
@@ -119,6 +120,7 @@
}
umask(0133); // u=rw,g=r,o=r
+ const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
std::ofstream fout(idmap_path);
if (fout.fail()) {
return error("failed to open idmap path " + idmap_path);
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.h b/cmds/idmap2/idmap2d/Idmap2Service.h
index 2b32042..4e5abc9 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.h
+++ b/cmds/idmap2/idmap2d/Idmap2Service.h
@@ -39,6 +39,9 @@
binder::Status removeIdmap(const std::string& overlay_apk_path, int32_t user_id,
bool* _aidl_return);
+ binder::Status verifyIdmap(const std::string& overlay_apk_path, int32_t user_id,
+ bool* _aidl_return);
+
binder::Status createIdmap(const std::string& target_apk_path,
const std::string& overlay_apk_path, int32_t user_id,
std::unique_ptr<std::string>* _aidl_return);
diff --git a/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl b/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl
index 5d19610..d475417 100644
--- a/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl
+++ b/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl
@@ -22,6 +22,7 @@
interface IIdmap2 {
@utf8InCpp String getIdmapPath(@utf8InCpp String overlayApkPath, int userId);
boolean removeIdmap(@utf8InCpp String overlayApkPath, int userId);
+ boolean verifyIdmap(@utf8InCpp String overlayApkPath, int userId);
@nullable @utf8InCpp String createIdmap(@utf8InCpp String targetApkPath,
@utf8InCpp String overlayApkPath, int userId);
}
diff --git a/services/core/java/com/android/server/om/IdmapManager.java b/services/core/java/com/android/server/om/IdmapManager.java
index 6d59827..16143d3 100644
--- a/services/core/java/com/android/server/om/IdmapManager.java
+++ b/services/core/java/com/android/server/om/IdmapManager.java
@@ -60,7 +60,6 @@
boolean createIdmap(@NonNull final PackageInfo targetPackage,
@NonNull final PackageInfo overlayPackage, int userId) {
- // unused userId: see comment in OverlayManagerServiceImpl.removeIdmapIfPossible
if (DEBUG) {
Slog.d(TAG, "create idmap for " + targetPackage.packageName + " and "
+ overlayPackage.packageName);
@@ -70,16 +69,19 @@
final String overlayPath = overlayPackage.applicationInfo.getBaseCodePath();
try {
if (FEATURE_FLAG_IDMAP2) {
- mIdmap2Service.createIdmap(targetPath, overlayPath, userId);
+ if (mIdmap2Service.verifyIdmap(overlayPath, userId)) {
+ return true;
+ }
+ return mIdmap2Service.createIdmap(targetPath, overlayPath, userId) != null;
} else {
mInstaller.idmap(targetPath, overlayPath, sharedGid);
+ return true;
}
} catch (Exception e) {
Slog.w(TAG, "failed to generate idmap for " + targetPath + " and "
+ overlayPath + ": " + e.getMessage());
return false;
}
- return true;
}
boolean removeIdmap(@NonNull final OverlayInfo oi, final int userId) {
@@ -88,15 +90,15 @@
}
try {
if (FEATURE_FLAG_IDMAP2) {
- mIdmap2Service.removeIdmap(oi.baseCodePath, userId);
+ return mIdmap2Service.removeIdmap(oi.baseCodePath, userId);
} else {
mInstaller.removeIdmap(oi.baseCodePath);
+ return true;
}
} catch (Exception e) {
Slog.w(TAG, "failed to remove idmap for " + oi.baseCodePath + ": " + e.getMessage());
return false;
}
- return true;
}
boolean idmapExists(@NonNull final OverlayInfo oi) {