Merge "flush mmap file to disk after updating" into main

This commit is contained in:
Treehugger Robot
2024-06-24 22:31:49 +00:00
committed by Gerrit Code Review
6 changed files with 131 additions and 27 deletions

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 {};
}

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)]

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)]

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),
},
}

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"],
}

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);
}
}