Merge changes from topic "no_snapshot_callstack"

* changes:
  Revert "libsnapshot::RemoveAllUpdateState log stack"
  Revert "libsnapshot callstack files readable by dumpstate"
  libsnapshot: delete InitiateMergeAndWait.
This commit is contained in:
Yifan Hong 2020-03-10 17:08:54 +00:00 committed by Gerrit Code Review
commit 86008cccb3
5 changed files with 15 additions and 251 deletions

View file

@ -96,16 +96,6 @@ cc_library_static {
static_libs: [
"libfs_mgr_binder"
],
shared_libs: [
// TODO(b/148818798): remove when parent bug is fixed
"libutilscallstack",
],
cflags: [
"-g",
"-O0",
"-DLIBSNAPSHOT_USE_CALLSTACK",
],
}
cc_library_static {
@ -179,9 +169,6 @@ cc_defaults {
"libsparse",
"libutils",
"libz",
// TODO(b/148818798): remove when parent bug is fixed
"libutilscallstack",
],
static_libs: [
"libfs_mgr",
@ -231,8 +218,5 @@ cc_binary {
"libprotobuf-cpp-lite",
"libstatslog",
"libutils",
// TODO(b/148818798): remove when parent bug is fixed.
"libutilscallstack",
],
}

View file

@ -178,16 +178,6 @@ class SnapshotManager final {
UpdateState ProcessUpdateState(const std::function<bool()>& callback = {},
const std::function<bool()>& before_cancel = {});
// Initiate the merge if necessary, then wait for the merge to finish.
// See InitiateMerge() and ProcessUpdateState() for details.
// Returns:
// - None if no merge to initiate
// - Unverified if called on the source slot
// - MergeCompleted if merge is completed
// - other states indicating an error has occurred
UpdateState InitiateMergeAndWait(SnapshotMergeReport* report = nullptr,
const std::function<bool()>& before_cancel = {});
// Find the status of the current update, if any.
//
// |progress| depends on the returned status:

View file

@ -21,7 +21,6 @@
#include <sys/unistd.h>
#include <optional>
#include <sstream>
#include <thread>
#include <unordered_set>
@ -38,10 +37,6 @@
#include <libfiemap/image_manager.h>
#include <liblp/liblp.h>
#ifdef LIBSNAPSHOT_USE_CALLSTACK
#include <utils/CallStack.h>
#endif
#include <android/snapshot/snapshot.pb.h>
#include <libsnapshot/snapshot_stats.h>
#include "device_info.h"
@ -228,25 +223,6 @@ bool SnapshotManager::RemoveAllUpdateState(LockedFile* lock, const std::function
LOG(INFO) << "Removing all update state.";
#ifdef LIBSNAPSHOT_USE_CALLSTACK
LOG(WARNING) << "Logging stack; see b/148818798.";
// Do not use CallStack's log functions because snapshotctl relies on
// android-base/logging to save log to files.
// TODO(b/148818798): remove this before we ship.
CallStack callstack;
callstack.update();
auto callstack_str = callstack.toString();
LOG(WARNING) << callstack_str.c_str();
std::stringstream path;
path << "/data/misc/snapshotctl_log/libsnapshot." << Now() << ".log";
std::string path_str = path.str();
android::base::WriteStringToFile(callstack_str.c_str(), path_str);
if (chmod(path_str.c_str(), 0644) == -1) {
PLOG(WARNING) << "Unable to chmod 0644 "
<< ", file maybe dropped from bugreport:" << path_str;
}
#endif
if (!RemoveAllSnapshots(lock)) {
LOG(ERROR) << "Could not remove all snapshots";
return false;
@ -2498,68 +2474,6 @@ std::unique_ptr<AutoDevice> SnapshotManager::EnsureMetadataMounted() {
return AutoUnmountDevice::New(device_->GetMetadataDir());
}
UpdateState SnapshotManager::InitiateMergeAndWait(SnapshotMergeReport* stats_report,
const std::function<bool()>& before_cancel) {
{
auto lock = LockExclusive();
// Sync update state from file with bootloader.
if (!WriteUpdateState(lock.get(), ReadUpdateState(lock.get()))) {
LOG(WARNING) << "Unable to sync write update state, fastboot may "
<< "reject / accept wipes incorrectly!";
}
}
auto merge_stats = SnapshotMergeStats::GetInstance(*this);
unsigned int last_progress = 0;
auto callback = [&]() -> bool {
double progress;
GetUpdateState(&progress);
if (last_progress < static_cast<unsigned int>(progress)) {
last_progress = progress;
LOG(INFO) << "Waiting for merge to complete: " << last_progress << "%.";
}
return true; // continue
};
LOG(INFO) << "Waiting for any previous merge request to complete. "
<< "This can take up to several minutes.";
merge_stats->Start();
auto state = ProcessUpdateState(callback, before_cancel);
merge_stats->set_state(state);
if (state == UpdateState::None) {
LOG(INFO) << "Can't find any snapshot to merge.";
return state;
}
if (state == UpdateState::Unverified) {
if (GetCurrentSlot() != Slot::Target) {
LOG(INFO) << "Cannot merge until device reboots.";
return state;
}
if (!InitiateMerge()) {
LOG(ERROR) << "Failed to initiate merge.";
return state;
}
// All other states can be handled by ProcessUpdateState.
LOG(INFO) << "Waiting for merge to complete. This can take up to several minutes.";
last_progress = 0;
state = ProcessUpdateState(callback, before_cancel);
merge_stats->set_state(state);
}
LOG(INFO) << "Merge finished with state \"" << state << "\".";
if (stats_report) {
auto result = merge_stats->Finish();
if (result) {
*stats_report = result->report();
} else {
LOG(WARNING) << "SnapshotMergeStatus::Finish failed.";
}
}
return state;
}
bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callback) {
if (!device_->IsRecovery()) {
LOG(ERROR) << "Data wipes are only allowed in recovery.";

View file

@ -1027,7 +1027,8 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) {
}
// Initiate the merge and wait for it to be completed.
ASSERT_EQ(UpdateState::MergeCompleted, init->InitiateMergeAndWait());
ASSERT_TRUE(init->InitiateMerge());
ASSERT_EQ(UpdateState::MergeCompleted, init->ProcessUpdateState());
// Check that the target partitions have the same content after the merge.
for (const auto& name : {"sys_b", "vnd_b", "prd_b"}) {
@ -1201,7 +1202,8 @@ TEST_F(SnapshotUpdateTest, ReclaimCow) {
// Initiate the merge and wait for it to be completed.
auto new_sm = SnapshotManager::New(new TestDeviceInfo(fake_super, "_b"));
ASSERT_EQ(UpdateState::MergeCompleted, new_sm->InitiateMergeAndWait());
ASSERT_TRUE(new_sm->InitiateMerge());
ASSERT_EQ(UpdateState::MergeCompleted, new_sm->ProcessUpdateState());
// Execute the second update.
ASSERT_TRUE(new_sm->BeginUpdate());
@ -1341,7 +1343,8 @@ TEST_F(SnapshotUpdateTest, MergeCannotRemoveCow) {
ASSERT_GE(fd, 0);
// COW cannot be removed due to open fd, so expect a soft failure.
ASSERT_EQ(UpdateState::MergeNeedsReboot, init->InitiateMergeAndWait());
ASSERT_TRUE(init->InitiateMerge());
ASSERT_EQ(UpdateState::MergeNeedsReboot, init->ProcessUpdateState());
// Simulate shutting down the device.
fd.reset();
@ -1354,7 +1357,7 @@ TEST_F(SnapshotUpdateTest, MergeCannotRemoveCow) {
ASSERT_FALSE(sm->IsSnapshotDevice("sys_b", nullptr));
// Merge should be able to complete now.
ASSERT_EQ(UpdateState::MergeCompleted, init->InitiateMergeAndWait());
ASSERT_EQ(UpdateState::MergeCompleted, init->ProcessUpdateState());
}
class MetadataMountedTest : public SnapshotUpdateTest {
@ -1691,7 +1694,8 @@ TEST_P(FlashAfterUpdateTest, FlashSlotAfterUpdate) {
// There should be no snapshot to merge.
auto new_sm = SnapshotManager::New(new TestDeviceInfo(fake_super, flashed_slot_suffix));
ASSERT_EQ(UpdateState::Cancelled, new_sm->InitiateMergeAndWait());
// update_enigne calls ProcessUpdateState first -- should see Cancelled.
ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
// Next OTA calls CancelUpdate no matter what.
ASSERT_TRUE(new_sm->CancelUpdate());

View file

@ -24,12 +24,8 @@
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/unique_fd.h>
#include <android/snapshot/snapshot.pb.h>
#include <libsnapshot/snapshot.h>
#include <libsnapshot/snapshot_stats.h>
#include <statslog.h>
#include "utility.h"
#include <libsnapshot/snapshot.h>
using namespace std::string_literals;
@ -39,146 +35,22 @@ int Usage() {
"Actions:\n"
" dump\n"
" Print snapshot states.\n"
" merge [--logcat] [--log-to-file] [--report] [--dry-run]\n"
" Initialize merge and wait for it to be completed.\n"
" If --logcat is specified, log to logcat.\n"
" If --log-to-file is specified, log to /data/misc/snapshotctl_log/.\n"
" If both specified, log to both. If none specified, log to stdout.\n"
" If --report is specified, send merge statistics to statsd.\n"
" If --dry-run flag, no real merge operation is is triggered, and\n"
" sample statistics are sent to statsd for testing purpose.\n";
" merge\n"
" Deprecated.\n";
return EX_USAGE;
}
namespace android {
namespace snapshot {
static SnapshotMergeReport GetDummySnapshotMergeReport() {
SnapshotMergeReport fake_report;
fake_report.set_state(UpdateState::MergeCompleted);
fake_report.set_resume_count(56);
return fake_report;
}
bool DumpCmdHandler(int /*argc*/, char** argv) {
android::base::InitLogging(argv, &android::base::StderrLogger);
return SnapshotManager::New()->Dump(std::cout);
}
class FileLogger {
public:
FileLogger() {
static constexpr const char* kLogFilePath = "/data/misc/snapshotctl_log/";
std::stringstream ss;
ss << kLogFilePath << "snapshotctl." << Now() << ".log";
fd_.reset(TEMP_FAILURE_RETRY(
open(ss.str().c_str(),
O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW | O_SYNC, 0644)));
if (fd_ == -1) {
PLOG(ERROR) << "Cannot open persistent log " << ss.str();
return;
}
// Explicitly chmod again because mode in open() may be masked by umask.
if (fchmod(fd_.get(), 0644) == -1) {
PLOG(ERROR) << "Cannot chmod 0644 persistent log " << ss.str();
return;
}
}
// Copy-contuctor needed to be converted to std::function.
FileLogger(const FileLogger& other) { fd_.reset(dup(other.fd_)); }
void operator()(android::base::LogId, android::base::LogSeverity, const char* /*tag*/,
const char* /*file*/, unsigned int /*line*/, const char* message) {
if (fd_ == -1) return;
std::stringstream ss;
ss << Now() << ":" << message << "\n";
(void)android::base::WriteStringToFd(ss.str(), fd_);
}
private:
android::base::unique_fd fd_;
};
class MergeCmdLogger {
public:
MergeCmdLogger(int argc, char** argv) {
for (int i = 0; i < argc; ++i) {
if (argv[i] == "--logcat"s) {
loggers_.push_back(android::base::LogdLogger());
}
if (argv[i] == "--log-to-file"s) {
loggers_.push_back(std::move(FileLogger()));
}
}
if (loggers_.empty()) {
loggers_.push_back(&android::base::StdioLogger);
}
}
void operator()(android::base::LogId id, android::base::LogSeverity severity, const char* tag,
const char* file, unsigned int line, const char* message) {
for (auto&& logger : loggers_) {
logger(id, severity, tag, file, line, message);
}
}
private:
std::vector<android::base::LogFunction> loggers_;
};
bool MergeCmdHandler(int argc, char** argv) {
std::chrono::milliseconds passed_ms;
bool report_to_statsd = false;
bool dry_run = false;
for (int i = 2; i < argc; ++i) {
if (argv[i] == "--report"s) {
report_to_statsd = true;
} else if (argv[i] == "--dry-run"s) {
dry_run = true;
}
}
// 'snapshotctl merge' is stripped away from arguments to
// Logger.
android::base::InitLogging(argv);
android::base::SetLogger(MergeCmdLogger(argc - 2, argv + 2));
UpdateState state;
SnapshotMergeReport merge_report;
if (dry_run) {
merge_report = GetDummySnapshotMergeReport();
state = merge_report.state();
passed_ms = std::chrono::milliseconds(1234);
} else {
auto begin = std::chrono::steady_clock::now();
state = SnapshotManager::New()->InitiateMergeAndWait(&merge_report);
// We could wind up in the Unverified state if the device rolled back or
// hasn't fully rebooted. Ignore this.
if (state == UpdateState::None || state == UpdateState::Unverified) {
return true;
}
auto end = std::chrono::steady_clock::now();
passed_ms = std::chrono::duration_cast<std::chrono::milliseconds>(end - begin);
}
if (report_to_statsd) {
android::util::stats_write(android::util::SNAPSHOT_MERGE_REPORTED,
static_cast<int32_t>(merge_report.state()),
static_cast<int64_t>(passed_ms.count()),
static_cast<int32_t>(merge_report.resume_count()));
}
if (state == UpdateState::MergeCompleted) {
LOG(INFO) << "Snapshot merged in " << passed_ms.count() << " ms.";
return true;
}
LOG(ERROR) << "Snapshot failed to merge with state \"" << state << "\".";
bool MergeCmdHandler(int /*argc*/, char** argv) {
android::base::InitLogging(argv, &android::base::StderrLogger);
LOG(WARNING) << "Deprecated. Call update_engine_client --merge instead.";
return false;
}