tequilaOS/platform_frameworks_native
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Hook IDumpstateDevice 1.1 up to dumpstate binary.

Browse source 
Bug reports will now prefer v1.1 of the HAL since it supports
well-scoped modes that mirror BugreportManager. For new devices
launching with R, v1.1 must be supported if this (optional) HAL is
implemented. It's also fairly trivial to update existing devices to 1.1
if an OEM chooses.

As for what this means, bug reports will:
- Be smaller in many cases, and as a result, faster to collect
- Contain less unnecessary PII (e.g. camera logs should now be excluded
    for MODE_TELEPHONY)

There's still some minor cleanup to be done, but for now leaving TODOs.

Bug: 143183758
Bug: 143184495
Test: atest dumpstate_test
Test: take bug report on Cuttlefish and Blueline, check for v1.1 usage
Change-Id: I1660ff3983931fdeed51c85f2342700c89a012aa
This commit is contained in:
Hunter Knepshield 2020-02-04 19:47:20 -08:00
parent 612231d21e
commit 8540faff16
4 changed files with 85 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
cmds/dumpstate/Android.bp
View file

@ -76,6 +76,7 @@ cc_defaults {
defaults: ["dumpstate_cflag_defaults"],
shared_libs: [
"android.hardware.dumpstate@1.0",
"android.hardware.dumpstate@1.1",
"libziparchive",
"libbase",
"libbinder",

83
cmds/dumpstate/dumpstate.cpp
View file

@ -64,6 +64,8 @@
#include <android-base/unique_fd.h>
#include <android/content/pm/IPackageManagerNative.h>
#include <android/hardware/dumpstate/1.0/IDumpstateDevice.h>
#include <android/hardware/dumpstate/1.1/IDumpstateDevice.h>
#include <android/hardware/dumpstate/1.1/types.h>
#include <android/hidl/manager/1.0/IServiceManager.h>
#include <android/os/IIncidentCompanion.h>
#include <binder/IServiceManager.h>
@ -85,7 +87,11 @@
#include "DumpstateService.h"
#include "dumpstate.h"
using ::android::hardware::dumpstate::V1_0::IDumpstateDevice;
using IDumpstateDevice_1_0 = ::android::hardware::dumpstate::V1_0::IDumpstateDevice;
using IDumpstateDevice_1_1 = ::android::hardware::dumpstate::V1_1::IDumpstateDevice;
using ::android::hardware::dumpstate::V1_1::DumpstateMode;
using ::android::hardware::dumpstate::V1_1::DumpstateStatus;
using ::android::hardware::dumpstate::V1_1::toString;
using ::std::literals::chrono_literals::operator""ms;
using ::std::literals::chrono_literals::operator""s;
@ -1892,8 +1898,8 @@ void Dumpstate::DumpstateBoard() {
std::bind([](std::string path) { android::os::UnlinkAndLogOnError(path); }, paths[i])));
}
sp<IDumpstateDevice> dumpstate_device(IDumpstateDevice::getService());
if (dumpstate_device == nullptr) {
sp<IDumpstateDevice_1_0> dumpstate_device_1_0(IDumpstateDevice_1_0::getService());
if (dumpstate_device_1_0 == nullptr) {
MYLOGE("No IDumpstateDevice implementation\n");
return;
}
@ -1924,29 +1930,54 @@ void Dumpstate::DumpstateBoard() {
handle.get()->data[i] = fd.release();
}
// Given that bugreport is required to diagnose failures, it's better to
// set an arbitrary amount of timeout for IDumpstateDevice than to block the
// rest of bugreport. In the timeout case, we will kill dumpstate board HAL
// and grab whatever dumped
std::packaged_task<bool()>
dumpstate_task([paths, dumpstate_device, &handle]() -> bool {
android::hardware::Return<void> status = dumpstate_device->dumpstateBoard(handle.get());
// Given that bugreport is required to diagnose failures, it's better to set an arbitrary amount
// of timeout for IDumpstateDevice than to block the rest of bugreport. In the timeout case, we
// will kill the HAL and grab whatever it dumped in time.
constexpr size_t timeout_sec = 30;
// Prefer version 1.1 if available. New devices launching with R are no longer allowed to
// implement just 1.0.
const char* descriptor_to_kill;
using DumpstateBoardTask = std::packaged_task<bool()>;
DumpstateBoardTask dumpstate_board_task;
sp<IDumpstateDevice_1_1> dumpstate_device_1_1(
IDumpstateDevice_1_1::castFrom(dumpstate_device_1_0));
if (dumpstate_device_1_1 != nullptr) {
MYLOGI("Using IDumpstateDevice v1.1");
descriptor_to_kill = IDumpstateDevice_1_1::descriptor;
dumpstate_board_task = DumpstateBoardTask([this, dumpstate_device_1_1, &handle]() -> bool {
::android::hardware::Return<DumpstateStatus> status =
dumpstate_device_1_1->dumpstateBoard_1_1(handle.get(), options_->dumpstate_hal_mode,
SEC_TO_MSEC(timeout_sec));
if (!status.isOk()) {
MYLOGE("dumpstateBoard failed: %s\n", status.description().c_str());
return false;
} else if (status != DumpstateStatus::OK) {
MYLOGE("dumpstateBoard failed with DumpstateStatus::%s\n", toString(status).c_str());
return false;
}
return true;
});
} else {
MYLOGI("Using IDumpstateDevice v1.0");
descriptor_to_kill = IDumpstateDevice_1_0::descriptor;
dumpstate_board_task = DumpstateBoardTask([dumpstate_device_1_0, &handle]() -> bool {
::android::hardware::Return<void> status =
dumpstate_device_1_0->dumpstateBoard(handle.get());
if (!status.isOk()) {
MYLOGE("dumpstateBoard failed: %s\n", status.description().c_str());
return false;
}
return true;
});
}
auto result = dumpstate_board_task.get_future();
std::thread(std::move(dumpstate_board_task)).detach();
auto result = dumpstate_task.get_future();
std::thread(std::move(dumpstate_task)).detach();
constexpr size_t timeout_sec = 30;
if (result.wait_for(std::chrono::seconds(timeout_sec)) != std::future_status::ready) {
MYLOGE("dumpstateBoard timed out after %zus, killing dumpstate vendor HAL\n", timeout_sec);
if (!android::base::SetProperty("ctl.interface_restart",
android::base::StringPrintf("%s/default",
IDumpstateDevice::descriptor))) {
if (!android::base::SetProperty(
"ctl.interface_restart",
android::base::StringPrintf("%s/default", descriptor_to_kill))) {
MYLOGE("Couldn't restart dumpstate HAL\n");
}
}
@ -1978,15 +2009,14 @@ void Dumpstate::DumpstateBoard() {
continue;
}
AddZipEntry(kDumpstateBoardFiles[i], paths[i]);
printf("*** See %s entry ***\n", kDumpstateBoardFiles[i].c_str());
}
printf("*** See dumpstate-board.txt entry ***\n");
}
static void ShowUsage() {
fprintf(stderr,
"usage: dumpstate [-h] [-b soundfile] [-e soundfile] [-d] [-p] "
"[-z]] [-s] [-S] [-q] [-P] [-R] [-V version]\n"
"[-z] [-s] [-S] [-q] [-P] [-R] [-V version]\n"
" -h: display this help message\n"
" -b: play sound file instead of vibrate, at beginning of job\n"
" -e: play sound file instead of vibrate, at end of job\n"
@ -2196,33 +2226,40 @@ static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOpt
switch (mode) {
case Dumpstate::BugreportMode::BUGREPORT_FULL:
options->do_fb = true;
options->dumpstate_hal_mode = DumpstateMode::FULL;
break;
case Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE:
// Currently, the dumpstate binder is only used by Shell to update progress.
options->do_start_service = true;
options->do_progress_updates = true;
options->do_fb = false;
options->dumpstate_hal_mode = DumpstateMode::INTERACTIVE;
break;
case Dumpstate::BugreportMode::BUGREPORT_REMOTE:
options->do_vibrate = false;
options->is_remote_mode = true;
options->do_fb = false;
options->dumpstate_hal_mode = DumpstateMode::REMOTE;
break;
case Dumpstate::BugreportMode::BUGREPORT_WEAR:
options->do_start_service = true;
options->do_progress_updates = true;
options->do_zip_file = true;
options->do_fb = true;
options->dumpstate_hal_mode = DumpstateMode::WEAR;
break;
// TODO(b/148168577) rename TELEPHONY everywhere to CONNECTIVITY.
case Dumpstate::BugreportMode::BUGREPORT_TELEPHONY:
options->telephony_only = true;
options->do_progress_updates = true;
options->do_fb = false;
options->dumpstate_hal_mode = DumpstateMode::CONNECTIVITY;
break;
case Dumpstate::BugreportMode::BUGREPORT_WIFI:
options->wifi_only = true;
options->do_zip_file = true;
options->do_fb = false;
options->dumpstate_hal_mode = DumpstateMode::WIFI;
break;
case Dumpstate::BugreportMode::BUGREPORT_DEFAULT:
break;
@ -2233,11 +2270,13 @@ static void LogDumpOptions(const Dumpstate::DumpOptions& options) {
MYLOGI(
"do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_fb: %d "
"is_remote_mode: %d show_header_only: %d do_start_service: %d telephony_only: %d "
"wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s args: %s\n",
"wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s dumpstate_hal_mode: %s "
"args: %s\n",
options.do_zip_file, options.do_vibrate, options.use_socket, options.use_control_socket,
options.do_fb, options.is_remote_mode, options.show_header_only, options.do_start_service,
options.telephony_only, options.wifi_only, options.do_progress_updates,
options.bugreport_fd.get(), options.bugreport_mode.c_str(), options.args.c_str());
options.bugreport_fd.get(), options.bugreport_mode.c_str(),
toString(options.dumpstate_hal_mode).c_str(), options.args.c_str());
}
void Dumpstate::DumpOptions::Initialize(BugreportMode bugreport_mode,

6
cmds/dumpstate/dumpstate.h
View file

@ -27,6 +27,7 @@
#include <android-base/macros.h>
#include <android-base/unique_fd.h>
#include <android/hardware/dumpstate/1.1/types.h>
#include <android/os/BnIncidentAuthListener.h>
#include <android/os/IDumpstate.h>
#include <android/os/IDumpstateListener.h>
@ -366,6 +367,11 @@ class Dumpstate {
bool wifi_only = false;
// Whether progress updates should be published.
bool do_progress_updates = false;
// The mode we'll use when calling IDumpstateDevice::dumpstateBoard.
// TODO(b/148168577) get rid of the AIDL values, replace them with the HAL values instead.
// The HAL is actually an API surface that can be validated, while the AIDL is not (@hide).
::android::hardware::dumpstate::V1_1::DumpstateMode dumpstate_hal_mode =
::android::hardware::dumpstate::V1_1::DumpstateMode::DEFAULT;
// File descriptor to output zip file.
android::base::unique_fd bugreport_fd;
// File descriptor to screenshot file.

20
cmds/dumpstate/tests/dumpstate_test.cpp
View file

@ -36,20 +36,22 @@
#include <android-base/properties.h>
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <cutils/properties.h>
#include <android-base/unique_fd.h>
#include <android/hardware/dumpstate/1.1/types.h>
#include <cutils/properties.h>
namespace android {
namespace os {
namespace dumpstate {
using ::android::hardware::dumpstate::V1_1::DumpstateMode;
using ::testing::EndsWith;
using ::testing::HasSubstr;
using ::testing::IsNull;
using ::testing::IsEmpty;
using ::testing::IsNull;
using ::testing::NotNull;
using ::testing::StrEq;
using ::testing::StartsWith;
using ::testing::StrEq;
using ::testing::Test;
using ::testing::internal::CaptureStderr;
using ::testing::internal::CaptureStdout;
@ -174,6 +176,7 @@ TEST_F(DumpOptionsTest, InitializeNone) {
EXPECT_FALSE(options_.do_fb);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
TEST_F(DumpOptionsTest, InitializeAdbBugreport) {
@ -200,6 +203,7 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) {
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
EXPECT_FALSE(options_.use_socket);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) {
@ -224,6 +228,7 @@ TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) {
EXPECT_FALSE(options_.do_fb);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
TEST_F(DumpOptionsTest, InitializeFullBugReport) {
@ -231,6 +236,7 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) {
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_fb);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::FULL);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@ -249,6 +255,7 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) {
EXPECT_TRUE(options_.do_progress_updates);
EXPECT_TRUE(options_.do_start_service);
EXPECT_FALSE(options_.do_fb);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::INTERACTIVE);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@ -265,6 +272,7 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) {
EXPECT_TRUE(options_.is_remote_mode);
EXPECT_FALSE(options_.do_vibrate);
EXPECT_FALSE(options_.do_fb);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::REMOTE);
// Other options retain default values
EXPECT_FALSE(options_.use_control_socket);
@ -280,6 +288,7 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) {
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.do_progress_updates);
EXPECT_TRUE(options_.do_start_service);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WEAR);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@ -296,6 +305,7 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.telephony_only);
EXPECT_TRUE(options_.do_progress_updates);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::CONNECTIVITY);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@ -311,6 +321,7 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) {
EXPECT_FALSE(options_.do_fb);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.wifi_only);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WIFI);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@ -337,6 +348,7 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) {
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_fb);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@ -375,6 +387,7 @@ TEST_F(DumpOptionsTest, InitializePartial1) {
EXPECT_FALSE(options_.do_fb);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
TEST_F(DumpOptionsTest, InitializePartial2) {
@ -403,6 +416,7 @@ TEST_F(DumpOptionsTest, InitializePartial2) {
EXPECT_FALSE(options_.do_zip_file);
EXPECT_FALSE(options_.use_socket);
EXPECT_FALSE(options_.use_control_socket);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
TEST_F(DumpOptionsTest, InitializeHelp) {