StrixOS/build
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

flush mmap file to disk after updating

Browse Source 
The change was applied in the file in memory. Without explicitly calling
msync, there is no guarantee that the changes are written back to disk.
Thus the flag value may not be changed.

Bug: b/348693143
Test: atest aconfig_storage_write_api.test.cpp
aconfig_storage_write_api.test.rust

Change-Id: I8b1e0cbbb90bcb3c2e5bbf418a6a1a9cd047c81b
This commit is contained in:
Zhi Dou
2024-06-24 21:14:15 +00:00
parent 4c47a8925e
commit 436f1368cd
6 changed files with 131 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
tools/aconfig/aconfig_storage_write_api/aconfig_storage_write_api.cpp
View File

@@ -1,6 +1,7 @@
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/unique_fd.h>
#include <sys/mman.h>
#include <sys/stat.h>
@@ -13,8 +14,8 @@
namespace aconfig_storage {
/// Map a storage file
android::base::Result<MutableMappedStorageFile*> map_mutable_storage_file(
std::string const& file) {
android::base::Result<MutableMappedStorageFile *> map_mutable_storage_file(
std::string const &file) {
struct stat file_stat;
if (stat(file.c_str(), &file_stat) < 0) {
return android::base::ErrnoError() << "stat failed";
@@ -26,13 +27,13 @@ android::base::Result<MutableMappedStorageFile*> map_mutable_storage_file(
size_t file_size = file_stat.st_size;
const int fd = open(file.c_str(), O_RDWR | O_NOFOLLOW | O_CLOEXEC);
if (fd == -1) {
android::base::unique_fd ufd(open(file.c_str(), O_RDWR | O_NOFOLLOW | O_CLOEXEC));
if (ufd.get() == -1) {
return android::base::ErrnoError() << "failed to open " << file;
};
void* const map_result =
mmap(nullptr, file_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
void *const map_result =
mmap(nullptr, file_size, PROT_READ | PROT_WRITE, MAP_SHARED, ufd.get(), 0);
if (map_result == MAP_FAILED) {
return android::base::ErrnoError() << "mmap failed";
}
@@ -46,47 +47,56 @@ android::base::Result<MutableMappedStorageFile*> map_mutable_storage_file(
/// Set boolean flag value
android::base::Result<void> set_boolean_flag_value(
const MutableMappedStorageFile& file,
const MutableMappedStorageFile &file,
uint32_t offset,
bool value) {
auto content = rust::Slice<uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
static_cast<uint8_t *>(file.file_ptr), file.file_size);
auto update_cxx = update_boolean_flag_value_cxx(content, offset, value);
if (!update_cxx.update_success) {
return android::base::Error() << update_cxx.error_message.c_str();
}
if (!msync(static_cast<uint8_t *>(file.file_ptr) + update_cxx.offset, 1, MS_SYNC)) {
return android::base::ErrnoError() << "msync failed";
}
return {};
}
/// Set if flag has server override
android::base::Result<void> set_flag_has_server_override(
const MutableMappedStorageFile& file,
const MutableMappedStorageFile &file,
FlagValueType value_type,
uint32_t offset,
bool value) {
auto content = rust::Slice<uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
static_cast<uint8_t *>(file.file_ptr), file.file_size);
auto update_cxx = update_flag_has_server_override_cxx(
content, static_cast<uint16_t>(value_type), offset, value);
if (!update_cxx.update_success) {
return android::base::Error() << update_cxx.error_message.c_str();
}
if (!msync(static_cast<uint8_t *>(file.file_ptr) + update_cxx.offset, 1, MS_SYNC)) {
return android::base::ErrnoError() << "msync failed";
}
return {};
}
/// Set if flag has local override
android::base::Result<void> set_flag_has_local_override(
const MutableMappedStorageFile& file,
const MutableMappedStorageFile &file,
FlagValueType value_type,
uint32_t offset,
bool value) {
auto content = rust::Slice<uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
static_cast<uint8_t *>(file.file_ptr), file.file_size);
auto update_cxx = update_flag_has_local_override_cxx(
content, static_cast<uint16_t>(value_type), offset, value);
if (!update_cxx.update_success) {
return android::base::Error() << update_cxx.error_message.c_str();
}
if (!msync(static_cast<uint8_t *>(file.file_ptr) + update_cxx.offset, 1, MS_SYNC)) {
return android::base::ErrnoError() << "msync failed";
}
return {};
}

8
tools/aconfig/aconfig_storage_write_api/src/flag_info_update.rs
View File

@@ -67,13 +67,13 @@ pub fn update_flag_has_server_override(
flag_type: FlagValueType,
flag_index: u32,
value: bool,
) -> Result<(), AconfigStorageError> {
) -> Result<usize, AconfigStorageError> {
let (attribute, head) = get_flag_attribute_and_offset(buf, flag_type, flag_index)?;
let has_override = (attribute & (FlagInfoBit::HasServerOverride as u8)) != 0;
if has_override != value {
buf[head] = (attribute ^ FlagInfoBit::HasServerOverride as u8).to_le_bytes()[0];
}
Ok(())
Ok(head)
}
/// Set if flag has local override
@@ -82,13 +82,13 @@ pub fn update_flag_has_local_override(
flag_type: FlagValueType,
flag_index: u32,
value: bool,
) -> Result<(), AconfigStorageError> {
) -> Result<usize, AconfigStorageError> {
let (attribute, head) = get_flag_attribute_and_offset(buf, flag_type, flag_index)?;
let has_override = (attribute & (FlagInfoBit::HasLocalOverride as u8)) != 0;
if has_override != value {
buf[head] = (attribute ^ FlagInfoBit::HasLocalOverride as u8).to_le_bytes()[0];
}
Ok(())
Ok(head)
}
#[cfg(test)]

4
tools/aconfig/aconfig_storage_write_api/src/flag_value_update.rs
View File

@@ -24,7 +24,7 @@ pub fn update_boolean_flag_value(
buf: &mut [u8],
flag_index: u32,
flag_value: bool,
) -> Result<(), AconfigStorageError> {
) -> Result<usize, AconfigStorageError> {
let interpreted_header = FlagValueHeader::from_bytes(buf)?;
if interpreted_header.version > FILE_VERSION {
return Err(AconfigStorageError::HigherStorageFileVersion(anyhow!(
@@ -43,7 +43,7 @@ pub fn update_boolean_flag_value(
}
buf[head] = u8::from(flag_value).to_le_bytes()[0];
Ok(())
Ok(head)
}
#[cfg(test)]

22
tools/aconfig/aconfig_storage_write_api/src/lib.rs
View File

@@ -194,18 +194,21 @@ mod ffi {
// Flag value update return for cc interlop
pub struct BooleanFlagValueUpdateCXX {
pub update_success: bool,
pub offset: usize,
pub error_message: String,
}
// Flag has server override update return for cc interlop
pub struct FlagHasServerOverrideUpdateCXX {
pub update_success: bool,
pub offset: usize,
pub error_message: String,
}
// Flag has local override update return for cc interlop
pub struct FlagHasLocalOverrideUpdateCXX {
pub update_success: bool,
pub offset: usize,
pub error_message: String,
}
@@ -251,11 +254,14 @@ pub(crate) fn update_boolean_flag_value_cxx(
value: bool,
) -> ffi::BooleanFlagValueUpdateCXX {
match crate::flag_value_update::update_boolean_flag_value(file, offset, value) {
Ok(()) => {
ffi::BooleanFlagValueUpdateCXX { update_success: true, error_message: String::from("") }
}
Ok(head) => ffi::BooleanFlagValueUpdateCXX {
update_success: true,
offset: head,
error_message: String::from(""),
},
Err(errmsg) => ffi::BooleanFlagValueUpdateCXX {
update_success: false,
offset: usize::MAX,
error_message: format!("{:?}", errmsg),
},
}
@@ -272,18 +278,21 @@ pub(crate) fn update_flag_has_server_override_cxx(
match crate::flag_info_update::update_flag_has_server_override(
file, value_type, offset, value,
) {
Ok(()) => ffi::FlagHasServerOverrideUpdateCXX {
Ok(head) => ffi::FlagHasServerOverrideUpdateCXX {
update_success: true,
offset: head,
error_message: String::from(""),
},
Err(errmsg) => ffi::FlagHasServerOverrideUpdateCXX {
update_success: false,
offset: usize::MAX,
error_message: format!("{:?}", errmsg),
},
}
}
Err(errmsg) => ffi::FlagHasServerOverrideUpdateCXX {
update_success: false,
offset: usize::MAX,
error_message: format!("{:?}", errmsg),
},
}
@@ -300,18 +309,21 @@ pub(crate) fn update_flag_has_local_override_cxx(
match crate::flag_info_update::update_flag_has_local_override(
file, value_type, offset, value,
) {
Ok(()) => ffi::FlagHasLocalOverrideUpdateCXX {
Ok(head) => ffi::FlagHasLocalOverrideUpdateCXX {
update_success: true,
offset: head,
error_message: String::from(""),
},
Err(errmsg) => ffi::FlagHasLocalOverrideUpdateCXX {
update_success: false,
offset: usize::MAX,
error_message: format!("{:?}", errmsg),
},
}
}
Err(errmsg) => ffi::FlagHasLocalOverrideUpdateCXX {
update_success: false,
offset: usize::MAX,
error_message: format!("{:?}", errmsg),
},
}

6
tools/aconfig/aconfig_storage_write_api/tests/Android.bp
View File

@@ -39,4 +39,10 @@ cc_test {
"general-tests",
],
ldflags: ["-Wl,--allow-multiple-definition"],
generated_headers: [
"cxx-bridge-header",
"libcxx_aconfig_storage_read_api_bridge_header",
],
generated_sources: ["libcxx_aconfig_storage_read_api_bridge_code"],
whole_static_libs: ["libaconfig_storage_read_api_cxx_bridge"],
}

84
tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.cpp
View File

@@ -25,6 +25,9 @@
#include <android-base/file.h>
#include <android-base/result.h>
#include "rust/cxx.h"
#include "aconfig_storage/lib.rs.h"
using namespace android::base;
namespace api = aconfig_storage;
@@ -85,6 +88,23 @@ TEST_F(AconfigStorageTest, test_boolean_flag_value_update) {
ASSERT_TRUE(value.ok());
ASSERT_TRUE(*value);
}
// load the file on disk and check has been updated
std::ifstream file(flag_val, std::ios::binary | std::ios::ate);
std::streamsize size = file.tellg();
file.seekg(0, std::ios::beg);
std::vector<uint8_t> buffer(size);
file.read(reinterpret_cast<char *>(buffer.data()), size);
auto content = rust::Slice<const uint8_t>(
buffer.data(), mapped_file->file_size);
for (int offset = 0; offset < 8; ++offset) {
auto value_cxx = get_boolean_flag_value_cxx(content, offset);
ASSERT_TRUE(value_cxx.query_success);
ASSERT_TRUE(value_cxx.flag_value);
}
}
/// Negative test to lock down the error when querying flag value out of range
@@ -112,15 +132,43 @@ TEST_F(AconfigStorageTest, test_flag_has_server_override_update) {
*mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasServerOverride);
}
update_result = api::set_flag_has_server_override(
// load the file on disk and check has been updated
std::ifstream file(flag_info, std::ios::binary | std::ios::ate);
std::streamsize size = file.tellg();
file.seekg(0, std::ios::beg);
std::vector<uint8_t> buffer(size);
file.read(reinterpret_cast<char *>(buffer.data()), size);
auto content = rust::Slice<const uint8_t>(
buffer.data(), mapped_file->file_size);
for (int offset = 0; offset < 8; ++offset) {
auto attribute = get_flag_attribute_cxx(content, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.query_success);
ASSERT_TRUE(attribute.flag_attribute & api::FlagInfoBit::HasServerOverride);
}
for (int offset = 0; offset < 8; ++offset) {
auto update_result = api::set_flag_has_server_override(
*mapped_file, api::FlagValueType::Boolean, offset, false);
ASSERT_TRUE(update_result.ok());
attribute = api::get_flag_attribute(
auto attribute = api::get_flag_attribute(
*mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasServerOverride);
}
std::ifstream file2(flag_info, std::ios::binary);
buffer.clear();
file2.read(reinterpret_cast<char *>(buffer.data()), size);
for (int offset = 0; offset < 8; ++offset) {
auto attribute = get_flag_attribute_cxx(content, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.query_success);
ASSERT_FALSE(attribute.flag_attribute & api::FlagInfoBit::HasServerOverride);
}
}
/// Test to lock down storage flag has local override update api
@@ -137,13 +185,41 @@ TEST_F(AconfigStorageTest, test_flag_has_local_override_update) {
*mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasLocalOverride);
}
update_result = api::set_flag_has_local_override(
// load the file on disk and check has been updated
std::ifstream file(flag_info, std::ios::binary | std::ios::ate);
std::streamsize size = file.tellg();
file.seekg(0, std::ios::beg);
std::vector<uint8_t> buffer(size);
file.read(reinterpret_cast<char *>(buffer.data()), size);
auto content = rust::Slice<const uint8_t>(
buffer.data(), mapped_file->file_size);
for (int offset = 0; offset < 8; ++offset) {
auto attribute = get_flag_attribute_cxx(content, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.query_success);
ASSERT_TRUE(attribute.flag_attribute & api::FlagInfoBit::HasLocalOverride);
}
for (int offset = 0; offset < 8; ++offset) {
auto update_result = api::set_flag_has_local_override(
*mapped_file, api::FlagValueType::Boolean, offset, false);
ASSERT_TRUE(update_result.ok());
attribute = api::get_flag_attribute(
auto attribute = api::get_flag_attribute(
*mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasLocalOverride);
}
std::ifstream file2(flag_info, std::ios::binary);
buffer.clear();
file2.read(reinterpret_cast<char *>(buffer.data()), size);
for (int offset = 0; offset < 8; ++offset) {
auto attribute = get_flag_attribute_cxx(content, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.query_success);
ASSERT_FALSE(attribute.flag_attribute & api::FlagInfoBit::HasLocalOverride);
}
}