Merge "CreateSnapshot: reduce zero-filling for CoW images"
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index e3fc4f6..1f3828e 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -278,6 +278,9 @@
SnapshotState state;
uint64_t device_size;
uint64_t snapshot_size;
+ uint64_t cow_partition_size;
+ uint64_t cow_file_size;
+
// These are non-zero when merging.
uint64_t sectors_allocated = 0;
uint64_t metadata_sectors = 0;
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 75528ab..bea6739 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -152,7 +152,13 @@
auto lock = LockExclusive();
if (!lock) return false;
- if (ReadUpdateState(lock.get()) != UpdateState::Initiated) {
+ auto update_state = ReadUpdateState(lock.get());
+ if (update_state == UpdateState::Unverified) {
+ LOG(INFO) << "FinishedSnapshotWrites already called before. Ignored.";
+ return true;
+ }
+
+ if (update_state != UpdateState::Initiated) {
LOG(ERROR) << "Can only transition to the Unverified state from the Initiated state.";
return false;
}
@@ -203,6 +209,7 @@
.state = SnapshotState::Created,
.device_size = device_size,
.snapshot_size = snapshot_size,
+ .cow_file_size = cow_size,
};
if (!WriteSnapshotStatus(lock, name, status)) {
PLOG(ERROR) << "Could not write snapshot status: " << name;
@@ -965,7 +972,7 @@
// flushed remaining I/O. We could in theory replace with dm-zero (or
// re-use the table above), but for now it's better to know why this
// would fail.
- if (!dm.DeleteDeviceIfExists(dm_name)) {
+ if (dm_name != name && !dm.DeleteDeviceIfExists(dm_name)) {
LOG(ERROR) << "Unable to delete snapshot device " << dm_name << ", COW cannot be "
<< "reclaimed until after reboot.";
return false;
@@ -1333,7 +1340,7 @@
return false;
}
auto pieces = android::base::Split(contents, " ");
- if (pieces.size() != 5) {
+ if (pieces.size() != 7) {
LOG(ERROR) << "Invalid status line for snapshot: " << path;
return false;
}
@@ -1356,11 +1363,19 @@
LOG(ERROR) << "Invalid snapshot size in status line for: " << path;
return false;
}
- if (!android::base::ParseUint(pieces[3], &status->sectors_allocated)) {
+ if (!android::base::ParseUint(pieces[3], &status->cow_partition_size)) {
+ LOG(ERROR) << "Invalid cow linear size in status line for: " << path;
+ return false;
+ }
+ if (!android::base::ParseUint(pieces[4], &status->cow_file_size)) {
+ LOG(ERROR) << "Invalid cow file size in status line for: " << path;
+ return false;
+ }
+ if (!android::base::ParseUint(pieces[5], &status->sectors_allocated)) {
LOG(ERROR) << "Invalid snapshot size in status line for: " << path;
return false;
}
- if (!android::base::ParseUint(pieces[4], &status->metadata_sectors)) {
+ if (!android::base::ParseUint(pieces[6], &status->metadata_sectors)) {
LOG(ERROR) << "Invalid snapshot size in status line for: " << path;
return false;
}
@@ -1398,6 +1413,8 @@
to_string(status.state),
std::to_string(status.device_size),
std::to_string(status.snapshot_size),
+ std::to_string(status.cow_partition_size),
+ std::to_string(status.cow_file_size),
std::to_string(status.sectors_allocated),
std::to_string(status.metadata_sectors),
};
diff --git a/init/epoll.cpp b/init/epoll.cpp
index 01d8867..17d63fa 100644
--- a/init/epoll.cpp
+++ b/init/epoll.cpp
@@ -69,19 +69,24 @@
return {};
}
-Result<void> Epoll::Wait(std::optional<std::chrono::milliseconds> timeout) {
+Result<std::vector<std::function<void()>*>> Epoll::Wait(
+ std::optional<std::chrono::milliseconds> timeout) {
int timeout_ms = -1;
if (timeout && timeout->count() < INT_MAX) {
timeout_ms = timeout->count();
}
- epoll_event ev;
- auto nr = TEMP_FAILURE_RETRY(epoll_wait(epoll_fd_, &ev, 1, timeout_ms));
- if (nr == -1) {
+ const auto max_events = epoll_handlers_.size();
+ epoll_event ev[max_events];
+ auto num_events = TEMP_FAILURE_RETRY(epoll_wait(epoll_fd_, ev, max_events, timeout_ms));
+ if (num_events == -1) {
return ErrnoError() << "epoll_wait failed";
- } else if (nr == 1) {
- std::invoke(*reinterpret_cast<std::function<void()>*>(ev.data.ptr));
}
- return {};
+ std::vector<std::function<void()>*> pending_functions;
+ for (int i = 0; i < num_events; ++i) {
+ pending_functions.emplace_back(reinterpret_cast<std::function<void()>*>(ev[i].data.ptr));
+ }
+
+ return pending_functions;
}
} // namespace init
diff --git a/init/epoll.h b/init/epoll.h
index ca84266..c32a661 100644
--- a/init/epoll.h
+++ b/init/epoll.h
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#ifndef _INIT_EPOLL_H
-#define _INIT_EPOLL_H
+#pragma once
#include <stdint.h>
#include <sys/epoll.h>
@@ -24,6 +23,7 @@
#include <functional>
#include <map>
#include <optional>
+#include <vector>
#include <android-base/unique_fd.h>
@@ -39,7 +39,8 @@
Result<void> Open();
Result<void> RegisterHandler(int fd, std::function<void()> handler, uint32_t events = EPOLLIN);
Result<void> UnregisterHandler(int fd);
- Result<void> Wait(std::optional<std::chrono::milliseconds> timeout);
+ Result<std::vector<std::function<void()>*>> Wait(
+ std::optional<std::chrono::milliseconds> timeout);
private:
android::base::unique_fd epoll_fd_;
@@ -48,5 +49,3 @@
} // namespace init
} // namespace android
-
-#endif
diff --git a/init/init.cpp b/init/init.cpp
index ce898de..d4cbb5f 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -787,8 +787,17 @@
if (am.HasMoreCommands()) epoll_timeout = 0ms;
}
- if (auto result = epoll.Wait(epoll_timeout); !result) {
- LOG(ERROR) << result.error();
+ auto pending_functions = epoll.Wait(epoll_timeout);
+ if (!pending_functions) {
+ LOG(ERROR) << pending_functions.error();
+ } else if (!pending_functions->empty()) {
+ // We always reap children before responding to the other pending functions. This is to
+ // prevent a race where other daemons see that a service has exited and ask init to
+ // start it again via ctl.start before init has reaped it.
+ ReapAnyOutstandingChildren();
+ for (const auto& function : *pending_functions) {
+ (*function)();
+ }
}
}
diff --git a/init/keychords_test.cpp b/init/keychords_test.cpp
index 33373d4..6e9b337 100644
--- a/init/keychords_test.cpp
+++ b/init/keychords_test.cpp
@@ -212,7 +212,11 @@
}
void TestFrame::RelaxForMs(std::chrono::milliseconds wait) {
- epoll_.Wait(wait);
+ auto pending_functions = epoll_.Wait(wait);
+ ASSERT_TRUE(pending_functions) << pending_functions.error();
+ for (const auto& function : *pending_functions) {
+ (*function)();
+ }
}
void TestFrame::SetChord(int key, bool value) {
diff --git a/libunwindstack/Android.bp b/libunwindstack/Android.bp
index 73237e6..da18af6 100644
--- a/libunwindstack/Android.bp
+++ b/libunwindstack/Android.bp
@@ -153,12 +153,12 @@
shared_libs: [
"libunwindstack",
],
+ relative_install_path: "libunwindstack_test",
}
-cc_test {
- name: "libunwindstack_test",
+cc_defaults {
+ name: "libunwindstack_testlib_flags",
defaults: ["libunwindstack_flags"],
- isolated: true,
srcs: [
"tests/ArmExidxDecodeTest.cpp",
@@ -183,7 +183,6 @@
"tests/ElfTestUtils.cpp",
"tests/IsolatedSettings.cpp",
"tests/JitDebugTest.cpp",
- "tests/LocalUnwinderTest.cpp",
"tests/LogFake.cpp",
"tests/MapInfoCreateMemoryTest.cpp",
"tests/MapInfoGetBuildIDTest.cpp",
@@ -211,6 +210,7 @@
"tests/UnwindOfflineTest.cpp",
"tests/UnwindTest.cpp",
"tests/UnwinderTest.cpp",
+ "tests/VerifyBionicTerminationTest.cpp",
],
cflags: [
@@ -252,11 +252,28 @@
"tests/files/offline/straddle_arm/*",
"tests/files/offline/straddle_arm64/*",
],
+}
+
+cc_test {
+ name: "libunwindstack_test",
+ defaults: ["libunwindstack_testlib_flags"],
+ isolated: true,
+
+ srcs: [
+ "tests/LocalUnwinderTest.cpp",
+ ],
required: [
"libunwindstack_local",
],
}
+// Skip LocalUnwinderTest until atest understands required properly.
+cc_test {
+ name: "libunwindstack_unit_test",
+ defaults: ["libunwindstack_testlib_flags"],
+ isolated: true,
+}
+
//-------------------------------------------------------------------------
// Tools
//-------------------------------------------------------------------------
diff --git a/libunwindstack/TEST_MAPPING b/libunwindstack/TEST_MAPPING
new file mode 100644
index 0000000..55771c0
--- /dev/null
+++ b/libunwindstack/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+ "presubmit": [
+ {
+ "name": "libunwindstack_unit_test"
+ }
+ ]
+}
diff --git a/libunwindstack/Unwinder.cpp b/libunwindstack/Unwinder.cpp
index 7556482..0b9b85c 100644
--- a/libunwindstack/Unwinder.cpp
+++ b/libunwindstack/Unwinder.cpp
@@ -309,7 +309,7 @@
}
}
-std::string Unwinder::FormatFrame(const FrameData& frame) {
+std::string Unwinder::FormatFrame(const FrameData& frame) const {
std::string data;
if (regs_->Is32Bit()) {
data += android::base::StringPrintf(" #%02zu pc %08" PRIx64, frame.num, frame.rel_pc);
@@ -355,7 +355,7 @@
return data;
}
-std::string Unwinder::FormatFrame(size_t frame_num) {
+std::string Unwinder::FormatFrame(size_t frame_num) const {
if (frame_num >= frames_.size()) {
return "";
}
diff --git a/libunwindstack/include/unwindstack/Unwinder.h b/libunwindstack/include/unwindstack/Unwinder.h
index 52b3578..11ad9de 100644
--- a/libunwindstack/include/unwindstack/Unwinder.h
+++ b/libunwindstack/include/unwindstack/Unwinder.h
@@ -77,7 +77,7 @@
void Unwind(const std::vector<std::string>* initial_map_names_to_skip = nullptr,
const std::vector<std::string>* map_suffixes_to_ignore = nullptr);
- size_t NumFrames() { return frames_.size(); }
+ size_t NumFrames() const { return frames_.size(); }
const std::vector<FrameData>& frames() { return frames_; }
@@ -87,8 +87,8 @@
return frames;
}
- std::string FormatFrame(size_t frame_num);
- std::string FormatFrame(const FrameData& frame);
+ std::string FormatFrame(size_t frame_num) const;
+ std::string FormatFrame(const FrameData& frame) const;
void SetJitDebug(JitDebug* jit_debug, ArchEnum arch);
diff --git a/libunwindstack/tests/LocalUnwinderTest.cpp b/libunwindstack/tests/LocalUnwinderTest.cpp
index 56a18cd..9936f7a 100644
--- a/libunwindstack/tests/LocalUnwinderTest.cpp
+++ b/libunwindstack/tests/LocalUnwinderTest.cpp
@@ -170,10 +170,10 @@
std::string testlib(testing::internal::GetArgvs()[0]);
auto const value = testlib.find_last_of('/');
- if (value == std::string::npos) {
- testlib = "../";
+ if (value != std::string::npos) {
+ testlib = testlib.substr(0, value + 1);
} else {
- testlib = testlib.substr(0, value + 1) + "../";
+ testlib = "";
}
testlib += "libunwindstack_local.so";
diff --git a/libunwindstack/tests/VerifyBionicTerminationTest.cpp b/libunwindstack/tests/VerifyBionicTerminationTest.cpp
new file mode 100644
index 0000000..6a3e91a
--- /dev/null
+++ b/libunwindstack/tests/VerifyBionicTerminationTest.cpp
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define _GNU_SOURCE 1
+#include <stdint.h>
+#include <string.h>
+
+#include <string>
+
+#if defined(__BIONIC__)
+
+#include <gtest/gtest.h>
+
+#include <unwindstack/DwarfSection.h>
+#include <unwindstack/Elf.h>
+#include <unwindstack/ElfInterface.h>
+#include <unwindstack/Regs.h>
+#include <unwindstack/RegsGetLocal.h>
+#include <unwindstack/Unwinder.h>
+
+// This test is specific to bionic to verify that __libc_init is
+// properly setting the return address to undefined so that the
+// unwind properly terminates.
+
+namespace unwindstack {
+
+static std::string DumpFrames(const UnwinderFromPid& unwinder) {
+ std::string unwind;
+ for (size_t i = 0; i < unwinder.NumFrames(); i++) {
+ unwind += unwinder.FormatFrame(i) + '\n';
+ }
+ return unwind;
+}
+
+static DwarfLocationEnum GetReturnAddressLocation(uint64_t rel_pc, DwarfSection* section) {
+ if (section == nullptr) {
+ return DWARF_LOCATION_INVALID;
+ }
+
+ const DwarfFde* fde = section->GetFdeFromPc(rel_pc);
+ if (fde == nullptr || fde->cie == nullptr) {
+ return DWARF_LOCATION_INVALID;
+ }
+ dwarf_loc_regs_t regs;
+ if (!section->GetCfaLocationInfo(rel_pc, fde, ®s)) {
+ return DWARF_LOCATION_INVALID;
+ }
+
+ auto reg_entry = regs.find(fde->cie->return_address_register);
+ if (reg_entry == regs.end()) {
+ return DWARF_LOCATION_INVALID;
+ }
+ return reg_entry->second.type;
+}
+
+static void VerifyReturnAddress(const FrameData& frame) {
+ // Now go and find information about the register data and verify that the relative pc results in
+ // an undefined register.
+ Elf elf(Memory::CreateFileMemory(frame.map_name, 0).release());
+ ASSERT_TRUE(elf.Init()) << "Failed to init elf object from " << frame.map_name;
+ ASSERT_TRUE(elf.valid()) << "Elf " << frame.map_name << " is not valid.";
+ ElfInterface* interface = elf.interface();
+
+ // Only check the eh_frame and the debug_frame since the undefined register
+ // is set using a cfi directive.
+ // Check debug_frame first, then eh_frame since debug_frame always
+ // contains the most specific data.
+ DwarfLocationEnum location = GetReturnAddressLocation(frame.rel_pc, interface->debug_frame());
+ if (location == DWARF_LOCATION_UNDEFINED) {
+ return;
+ }
+
+ location = GetReturnAddressLocation(frame.rel_pc, interface->eh_frame());
+ ASSERT_EQ(DWARF_LOCATION_UNDEFINED, location);
+}
+
+// This test assumes that it starts from the main thread, and that the
+// libc.so on device will include symbols so that function names can
+// be resolved.
+TEST(VerifyBionicTermination, local_terminate) {
+ std::unique_ptr<Regs> regs(Regs::CreateFromLocal());
+
+ UnwinderFromPid unwinder(512, getpid());
+ ASSERT_TRUE(unwinder.Init(regs->Arch()));
+ unwinder.SetRegs(regs.get());
+
+ RegsGetLocal(regs.get());
+ unwinder.Unwind();
+ ASSERT_LT(0U, unwinder.NumFrames());
+
+ SCOPED_TRACE(DumpFrames(unwinder));
+
+ // Look for the frame that includes __libc_init, there should only
+ // be one and it should be the last.
+ bool found = false;
+ const std::vector<FrameData>& frames = unwinder.frames();
+ for (size_t i = 0; i < unwinder.NumFrames(); i++) {
+ const FrameData& frame = frames[i];
+ if (frame.function_name == "__libc_init" && !frame.map_name.empty() &&
+ std::string("libc.so") == basename(frame.map_name.c_str())) {
+ ASSERT_EQ(unwinder.NumFrames(), i + 1) << "__libc_init is not last frame.";
+ ASSERT_NO_FATAL_FAILURE(VerifyReturnAddress(frame));
+ found = true;
+ }
+ }
+ ASSERT_TRUE(found) << "Unable to find libc.so:__libc_init frame\n";
+}
+
+} // namespace unwindstack
+
+#endif