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

View File

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

View File

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

View File

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

View File

@@ -39,4 +39,10 @@ cc_test {
"general-tests", "general-tests",
], ],
ldflags: ["-Wl,--allow-multiple-definition"], 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/file.h>
#include <android-base/result.h> #include <android-base/result.h>
#include "rust/cxx.h"
#include "aconfig_storage/lib.rs.h"
using namespace android::base; using namespace android::base;
namespace api = aconfig_storage; namespace api = aconfig_storage;
@@ -85,6 +88,23 @@ TEST_F(AconfigStorageTest, test_boolean_flag_value_update) {
ASSERT_TRUE(value.ok()); ASSERT_TRUE(value.ok());
ASSERT_TRUE(*value); 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 /// 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); *mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok()); ASSERT_TRUE(attribute.ok());
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasServerOverride); 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); *mapped_file, api::FlagValueType::Boolean, offset, false);
ASSERT_TRUE(update_result.ok()); ASSERT_TRUE(update_result.ok());
attribute = api::get_flag_attribute( auto attribute = api::get_flag_attribute(
*mapped_file, api::FlagValueType::Boolean, offset); *mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok()); ASSERT_TRUE(attribute.ok());
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasServerOverride); 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 /// 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); *mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok()); ASSERT_TRUE(attribute.ok());
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasLocalOverride); 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); *mapped_file, api::FlagValueType::Boolean, offset, false);
ASSERT_TRUE(update_result.ok()); ASSERT_TRUE(update_result.ok());
attribute = api::get_flag_attribute( auto attribute = api::get_flag_attribute(
*mapped_file, api::FlagValueType::Boolean, offset); *mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok()); ASSERT_TRUE(attribute.ok());
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasLocalOverride); 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);
}
} }