From: Mickey Keeley Date: Wed, 30 May 2018 00:43:22 +0000 (-0700) Subject: BootParameters: Use proto for disk io. X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=81121bd0acc6e6a43213a25fa90d89f392a6b37a;p=android-x86%2Fframeworks-base.git BootParameters: Use proto for disk io. Use proto for disk usage. Bug: 79932027 Test: Ran through the following scenarios- - RebootActivity parses previous boot parameters; sets next as expected. - Push old parameters to device and ensured bootanimations reads it and saves new format (serialized proto) that can be read in userspace (RebootActivity). - Unit tests pass. Change-Id: I98aefb7a832c985d5061ee80ab85a4523f067641 --- diff --git a/cmds/bootanimation/Android.mk b/cmds/bootanimation/Android.mk index 453fb727bab8..6943dab0acbe 100644 --- a/cmds/bootanimation/Android.mk +++ b/cmds/bootanimation/Android.mk @@ -27,7 +27,9 @@ ifeq ($(PRODUCT_IOT),true) LOCAL_SHARED_LIBRARIES += \ libandroidthings \ + libandroidthings_protos \ libchrome \ + libprotobuf-cpp-lite \ LOCAL_STATIC_LIBRARIES += \ libjsoncpp diff --git a/cmds/bootanimation/iot/Android.mk b/cmds/bootanimation/iot/Android.mk index 8b475d3f3c95..3d288e4e111b 100644 --- a/cmds/bootanimation/iot/Android.mk +++ b/cmds/bootanimation/iot/Android.mk @@ -25,9 +25,11 @@ LOCAL_CFLAGS := -Wall -Werror -Wunused -Wunreachable-code LOCAL_SHARED_LIBRARIES := \ libandroidthings \ + libandroidthings_protos \ libbase \ libchrome \ liblog \ + libprotobuf-cpp-lite \ LOCAL_STATIC_LIBRARIES += \ libjsoncpp diff --git a/cmds/bootanimation/iot/BootParameters.cpp b/cmds/bootanimation/iot/BootParameters.cpp index 2cf1c19e9301..30a9b2895c44 100644 --- a/cmds/bootanimation/iot/BootParameters.cpp +++ b/cmds/bootanimation/iot/BootParameters.cpp @@ -22,10 +22,13 @@ #include #include +#include #include using android::base::ReadFileToString; using android::base::RemoveFileIfExists; +using android::base::WriteStringToFile; +using Json::ArrayIndex; using Json::Reader; using Json::Value; @@ -33,23 +36,34 @@ namespace android { namespace { -// Keys for supporting a silent boot and user-defined BootAction parameters. -constexpr const char *kKeySilentBoot = "silent_boot"; -constexpr const char* kKeyParams = "params"; +// Keys for deprecated parameters. Devices that OTA from N to O and that used +// the hidden BootParameters API will store these in the JSON blob. To support +// the transition from N to O, these keys are mapped to the new parameters. +constexpr const char *kKeyLegacyVolume = "volume"; +constexpr const char *kKeyLegacyAnimationsDisabled = "boot_animation_disabled"; +constexpr const char *kKeyLegacyParamNames = "param_names"; +constexpr const char *kKeyLegacyParamValues = "param_values"; -constexpr const char* kNextBootFile = "/data/misc/bootanimation/next_boot.json"; -constexpr const char* kLastBootFile = "/data/misc/bootanimation/last_boot.json"; +constexpr const char *kNextBootFile = "/data/misc/bootanimation/next_boot.proto"; +constexpr const char *kLastBootFile = "/data/misc/bootanimation/last_boot.proto"; -void swapBootConfigs() { - // rename() will fail if next_boot.json doesn't exist, so delete - // last_boot.json manually first. +constexpr const char *kLegacyNextBootFile = "/data/misc/bootanimation/next_boot.json"; +constexpr const char *kLegacyLastBootFile = "/data/misc/bootanimation/last_boot.json"; + +void removeLegacyFiles() { std::string err; - if (!RemoveFileIfExists(kLastBootFile, &err)) - ALOGE("Unable to delete last boot file: %s", err.c_str()); + if (!RemoveFileIfExists(kLegacyLastBootFile, &err)) { + ALOGW("Unable to delete %s: %s", kLegacyLastBootFile, err.c_str()); + } - if (rename(kNextBootFile, kLastBootFile) && errno != ENOENT) - ALOGE("Unable to swap boot files: %s", strerror(errno)); + err.clear(); + if (!RemoveFileIfExists(kLegacyNextBootFile, &err)) { + ALOGW("Unable to delete %s: %s", kLegacyNextBootFile, err.c_str()); + } +} +void createNextBootFile() { + errno = 0; int fd = open(kNextBootFile, O_CREAT, DEFFILEMODE); if (fd == -1) { ALOGE("Unable to create next boot file: %s", strerror(errno)); @@ -64,54 +78,120 @@ void swapBootConfigs() { } // namespace +// Renames the 'next' boot file to the 'last' file and reads its contents. +bool BootParameters::swapAndLoadBootConfigContents(const char *lastBootFile, + const char *nextBootFile, + std::string *contents) { + if (!ReadFileToString(nextBootFile, contents)) { + RemoveFileIfExists(lastBootFile); + return false; + } + + errno = 0; + if (rename(nextBootFile, lastBootFile) && errno != ENOENT) + ALOGE("Unable to swap boot files: %s", strerror(errno)); + + return true; +} + BootParameters::BootParameters() { - swapBootConfigs(); loadParameters(); } +// Saves the boot parameters state to disk so the framework can read it. +void BootParameters::storeParameters() { + errno = 0; + if (!WriteStringToFile(mProto.SerializeAsString(), kLastBootFile)) { + ALOGE("Failed to write boot parameters to %s: %s", kLastBootFile, strerror(errno)); + } + + // WriteStringToFile sets the file permissions to 0666, but these are not + // honored by the system. + errno = 0; + if (chmod(kLastBootFile, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) { + ALOGE("Failed to set permissions for %s: %s", kLastBootFile, strerror(errno)); + } +} + +// Load the boot parameters from disk, try the old location and format if the +// file does not exist. Note: +// - Parse errors result in defaults being used (a normal boot). +// - Legacy boot parameters default to a silent boot. void BootParameters::loadParameters() { + // Precedence is given to the new file format (.proto). std::string contents; - if (!ReadFileToString(kLastBootFile, &contents)) { - if (errno != ENOENT) - ALOGE("Unable to read from %s: %s", kLastBootFile, strerror(errno)); + if (swapAndLoadBootConfigContents(kLastBootFile, kNextBootFile, &contents)) { + parseBootParameters(contents); + } else if (swapAndLoadBootConfigContents(kLegacyLastBootFile, kLegacyNextBootFile, &contents)) { + parseLegacyBootParameters(contents); + storeParameters(); + removeLegacyFiles(); + } + + createNextBootFile(); +} +void BootParameters::parseBootParameters(const std::string &contents) { + if (!mProto.ParseFromString(contents)) { + ALOGW("Failed to parse parameters from %s", kLastBootFile); return; } - loadParameters(contents); + loadStateFromProto(); } -// If the boot parameters - -// - File is missing, we assume a normal, non-silent boot. -// - Are well-formed, initially assume a normal, non-silent boot and parse. -void BootParameters::loadParameters(const std::string& raw_json) { - if (!Reader().parse(raw_json, mJson)) { - return; - } +// Parses the JSON in the proto. +void BootParameters::parseLegacyBootParameters(const std::string &contents) { + Value json; + if (!Reader().parse(contents, json)) { + ALOGW("Failed to parse parameters from %s", kLegacyLastBootFile); + return; + } - parseBootParameters(); -} + int volume = 0; + bool bootAnimationDisabled = true; -void BootParameters::parseBootParameters() { - // A missing key returns a safe, missing value. - // Ignore invalid or missing JSON parameters. - Value &jsonValue = mJson[kKeySilentBoot]; - if (jsonValue.isBool()) { - mIsSilentBoot = jsonValue.asBool(); + Value &jsonValue = json[kKeyLegacyVolume]; + if (jsonValue.isIntegral()) { + volume = jsonValue.asInt(); } - jsonValue = mJson[kKeyParams]; - if (jsonValue.isObject()) { - // getMemberNames returns a copy of the keys which must be stored. - mKeys = jsonValue.getMemberNames(); - for (auto &key : mKeys) { - Value &value = jsonValue[key]; - if (value.isString()) { - mParameters.push_back( - {.key = key.c_str(), .value = value.asCString()}); + jsonValue = json[kKeyLegacyAnimationsDisabled]; + if (jsonValue.isIntegral()) { + bootAnimationDisabled = jsonValue.asInt() == 1; + } + + // Assume a silent boot unless all of the following are true - + // 1. The volume is neither 0 nor -1000 (the legacy default value). + // 2. The boot animations are explicitly enabled. + // Note: brightness was never used. + mProto.set_silent_boot((volume == 0) || (volume == -1000) || bootAnimationDisabled); + + Value &keys = json[kKeyLegacyParamNames]; + Value &values = json[kKeyLegacyParamValues]; + if (keys.isArray() && values.isArray() && (keys.size() == values.size())) { + for (ArrayIndex i = 0; i < keys.size(); ++i) { + auto &key = keys[i]; + auto &value = values[i]; + if (key.isString() && value.isString()) { + auto userParameter = mProto.add_user_parameter(); + userParameter->set_key(key.asString()); + userParameter->set_value(value.asString()); } } } + + loadStateFromProto(); +} + +void BootParameters::loadStateFromProto() { + // A missing key returns a safe, default value. + // Ignore invalid or missing parameters. + mIsSilentBoot = mProto.silent_boot(); + + for (const auto ¶m : mProto.user_parameter()) { + mParameters.push_back({.key = param.key().c_str(), .value = param.value().c_str()}); + } } } // namespace android diff --git a/cmds/bootanimation/iot/BootParameters.h b/cmds/bootanimation/iot/BootParameters.h index 49c34dda4c74..cbd1ca61cfc3 100644 --- a/cmds/bootanimation/iot/BootParameters.h +++ b/cmds/bootanimation/iot/BootParameters.h @@ -22,7 +22,7 @@ #include #include // libandroidthings native API. -#include +#include namespace android { @@ -39,22 +39,33 @@ public: // Returns the additional boot parameters that were set on reboot. const std::vector& getParameters() const { return mParameters; } - // Exposed for testing. Updates the parameters with new JSON values. - void loadParameters(const std::string& raw_json); -private: + // Exposed for testing. Sets the parameters to the serialized proto. + void parseBootParameters(const std::string &contents); + + // For devices that OTA from N to O. + // Exposed for testing. Sets the parameters to the raw JSON. + void parseLegacyBootParameters(const std::string &contents); + + // Exposed for testing. Loads the contents from |nextBootFile| and replaces + // |lastBootFile| with |nextBootFile|. + static bool swapAndLoadBootConfigContents(const char *lastBootFile, const char *nextBootFile, + std::string *contents); + + private: void loadParameters(); - void parseBootParameters(); + // Replaces the legacy JSON blob with the updated version, allowing the + // framework to read it. + void storeParameters(); + + void loadStateFromProto(); bool mIsSilentBoot = false; std::vector mParameters; - // Store parsed JSON because mParameters makes a shallow copy. - Json::Value mJson; - - // Store parameter keys because mParameters makes a shallow copy. - Json::Value::Members mKeys; + // Store the proto because mParameters makes a shallow copy. + android::things::proto::BootParameters mProto; }; } // namespace android diff --git a/cmds/bootanimation/iot/BootParameters_test.cpp b/cmds/bootanimation/iot/BootParameters_test.cpp index 27d7d6fdf8be..d55bce6eecc3 100644 --- a/cmds/bootanimation/iot/BootParameters_test.cpp +++ b/cmds/bootanimation/iot/BootParameters_test.cpp @@ -16,6 +16,13 @@ #include "BootParameters.h" +#include +#include +#include + +#include +#include +#include #include namespace android { @@ -23,50 +30,131 @@ namespace android { namespace { TEST(BootParametersTest, TestNoBootParametersIsNotSilent) { - BootParameters boot_parameters = BootParameters(); - boot_parameters.loadParameters(""); + android::things::proto::BootParameters proto; + + BootParameters bootParameters = BootParameters(); + bootParameters.parseBootParameters(proto.SerializeAsString()); - ASSERT_FALSE(boot_parameters.isSilentBoot()); - ASSERT_EQ(0u, boot_parameters.getParameters().size()); + ASSERT_FALSE(bootParameters.isSilentBoot()); + ASSERT_EQ(0u, bootParameters.getParameters().size()); } TEST(BootParametersTest, TestParseIsSilent) { - BootParameters boot_parameters = BootParameters(); - boot_parameters.loadParameters(R"( + android::things::proto::BootParameters proto; + proto.set_silent_boot(true); + + BootParameters bootParameters = BootParameters(); + bootParameters.parseBootParameters(proto.SerializeAsString()); + + ASSERT_TRUE(bootParameters.isSilentBoot()); +} + +TEST(BootParametersTest, TestParseIsNotSilent) { + android::things::proto::BootParameters proto; + proto.set_silent_boot(false); + + BootParameters bootParameters = BootParameters(); + bootParameters.parseBootParameters(proto.SerializeAsString()); + + ASSERT_FALSE(bootParameters.isSilentBoot()); +} + +TEST(BootParametersTest, TestParseBootParameters) { + android::things::proto::BootParameters proto; + proto.set_silent_boot(false); + + auto userParameter = proto.add_user_parameter(); + userParameter->set_key("key1"); + userParameter->set_value("value1"); + + userParameter = proto.add_user_parameter(); + userParameter->set_key("key2"); + userParameter->set_value("value2"); + + BootParameters bootParameters = BootParameters(); + bootParameters.parseBootParameters(proto.SerializeAsString()); + + auto ¶meters = bootParameters.getParameters(); + ASSERT_EQ(2u, parameters.size()); + ASSERT_STREQ(parameters[0].key, "key1"); + ASSERT_STREQ(parameters[0].value, "value1"); + ASSERT_STREQ(parameters[1].key, "key2"); + ASSERT_STREQ(parameters[1].value, "value2"); +} + +TEST(BootParametersTest, TestParseLegacyDisableBootAnimationIsSilent) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( { - "silent_boot":true, - "params":{} + "brightness":200, + "volume":100, + "boot_animation_disabled":1, + "param_names":[], + "param_values":[] } )"); - ASSERT_TRUE(boot_parameters.isSilentBoot()); + ASSERT_TRUE(bootParameters.isSilentBoot()); } -TEST(BootParametersTest, TestParseIsNotSilent) { - BootParameters boot_parameters = BootParameters(); - boot_parameters.loadParameters(R"( +TEST(BootParametersTest, TestParseLegacyZeroVolumeIsSilent) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( { - "silent_boot":false, - "params":{} + "brightness":200, + "volume":0, + "boot_animation_disabled":0, + "param_names":[], + "param_values":[] } )"); - ASSERT_FALSE(boot_parameters.isSilentBoot()); + ASSERT_TRUE(bootParameters.isSilentBoot()); } -TEST(BootParametersTest, TestParseBootParameters) { - BootParameters boot_parameters = BootParameters(); - boot_parameters.loadParameters(R"( +TEST(BootParametersTest, TestParseLegacyDefaultVolumeIsSilent) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( { - "silent_boot":false, - "params":{ - "key1":"value1", - "key2":"value2" - } + "brightness":200, + "volume":-1000, + "boot_animation_disabled":0, + "param_names":[], + "param_values":[] } )"); - auto ¶meters = boot_parameters.getParameters(); + ASSERT_TRUE(bootParameters.isSilentBoot()); +} + +TEST(BootParametersTest, TestParseLegacyNotSilent) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( + { + "brightness":200, + "volume":500, + "boot_animation_disabled":0, + "param_names":[], + "param_values":[] + } + )"); + + ASSERT_FALSE(bootParameters.isSilentBoot()); +} + +TEST(BootParametersTest, TestParseLegacyParameters) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( + { + "brightness":200, + "volume":100, + "boot_animation_disabled":1, + "param_names":["key1", "key2"], + "param_values":["value1", "value2"] + } + )"); + + auto parameters = bootParameters.getParameters(); ASSERT_EQ(2u, parameters.size()); ASSERT_STREQ(parameters[0].key, "key1"); ASSERT_STREQ(parameters[0].value, "value1"); @@ -74,35 +162,102 @@ TEST(BootParametersTest, TestParseBootParameters) { ASSERT_STREQ(parameters[1].value, "value2"); } -TEST(BootParametersTest, TestParseMissingParametersIsNotSilent) { - BootParameters boot_parameters = BootParameters(); - boot_parameters.loadParameters(R"( +TEST(BootParametersTest, TestParseLegacyZeroParameters) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( { - "params":{} + "brightness":200, + "volume":100, + "boot_animation_disabled":1, + "param_names":[], + "param_values":[] } )"); - ASSERT_FALSE(boot_parameters.isSilentBoot()); + ASSERT_EQ(0u, bootParameters.getParameters().size()); } -TEST(BootParametersTest, TestParseMalformedParametersAreSkipped) { - BootParameters boot_parameters = BootParameters(); - boot_parameters.loadParameters(R"( +TEST(BootParametersTest, TestMalformedLegacyParametersAreSkipped) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( { - "silent_boot":false, - "params":{ - "key1":123, - "key2":"value2" - } + "brightness":500, + "volume":500, + "boot_animation_disabled":0, + "param_names":["key1", "key2"], + "param_values":[1, "value2"] } )"); - auto ¶meters = boot_parameters.getParameters(); + auto parameters = bootParameters.getParameters(); ASSERT_EQ(1u, parameters.size()); ASSERT_STREQ(parameters[0].key, "key2"); ASSERT_STREQ(parameters[0].value, "value2"); } +TEST(BootParametersTest, TestLegacyUnequalParameterSizesAreSkipped) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( + { + "brightness":500, + "volume":500, + "boot_animation_disabled":0, + "param_names":["key1", "key2"], + "param_values":["value1"] + } + )"); + + ASSERT_EQ(0u, bootParameters.getParameters().size()); +} + +TEST(BootParametersTest, TestMissingLegacyBootParametersIsSilent) { + BootParameters bootParameters = BootParameters(); + bootParameters.parseLegacyBootParameters(R"( + { + "brightness":500 + } + )"); + + EXPECT_TRUE(bootParameters.isSilentBoot()); + ASSERT_EQ(0u, bootParameters.getParameters().size()); +} + +TEST(BootParametersTest, TestLastFileIsRemovedOnError) { + TemporaryFile lastFile; + TemporaryDir tempDir; + std::string nonExistentFilePath(std::string(tempDir.path) + "/nonexistent"); + std::string contents; + + BootParameters::swapAndLoadBootConfigContents(lastFile.path, nonExistentFilePath.c_str(), + &contents); + + struct stat buf; + ASSERT_EQ(-1, lstat(lastFile.path, &buf)); + ASSERT_TRUE(contents.empty()); +} + +TEST(BootParametersTest, TestNextFileIsRemovedLastFileExistsOnSuccess) { + TemporaryFile lastFile; + TemporaryFile nextFile; + + base::WriteStringToFile("foo", nextFile.path); + + std::string contents; + // Expected side effects: + // - |next_file| is moved to |last_file| + // - |contents| is the contents of |next_file| before being moved. + BootParameters::swapAndLoadBootConfigContents(lastFile.path, nextFile.path, &contents); + + struct stat buf; + ASSERT_EQ(0, lstat(lastFile.path, &buf)); + ASSERT_EQ(-1, lstat(nextFile.path, &buf)); + ASSERT_EQ(contents, "foo"); + + contents.clear(); + ASSERT_TRUE(base::ReadFileToString(lastFile.path, &contents)); + ASSERT_EQ(contents, "foo"); +} + } // namespace } // namespace android