Skip to content

Commit fa8666c

Browse files
ComixHeComixHe
committed
refactor(container): inline container_data and use std::move for efficiency
- Move container_data members directly into container class - Remove get_private_data friend function and dynamic allocation - Use std::move to avoid unnecessary copies in options and config parsing - Fix XDG_RUNTIME_DIR default root initialization order Signed-off-by: ComixHe <ComixHe1895@outlook.com>
1 parent 0b64b2e commit fa8666c

4 files changed

Lines changed: 63 additions & 68 deletions

File tree

‎src/linyaps_box/command/options.cpp‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ linyaps_box::command::options linyaps_box::command::parse(int argc, char *argv[]
2626

2727
linyaps_box::command::options options;
2828

29-
auto default_root = std::filesystem::current_path().root_path() / "run" / "user"
30-
/ std::to_string(geteuid()) / "linglong" / "box";
31-
29+
std::filesystem::path default_root;
3230
if (auto *env = getenv("XDG_RUNTIME_DIR"); env != nullptr) {
3331
default_root = std::filesystem::path{ env } / "linglong" / "box";
32+
} else {
33+
default_root = std::filesystem::current_path().root_path() / "run" / "user"
34+
/ std::to_string(geteuid()) / "linglong" / "box";
3435
}
3536

3637
app.add_option("--root", options.global.root, "Root directory for storage of container state")
@@ -150,13 +151,13 @@ linyaps_box::command::options linyaps_box::command::parse(int argc, char *argv[]
150151
}
151152

152153
if (cmd_list->parsed()) {
153-
options.subcommand_opt = list_opt;
154+
options.subcommand_opt = std::move(list_opt);
154155
} else if (cmd_run->parsed()) {
155-
options.subcommand_opt = run_opt;
156+
options.subcommand_opt = std::move(run_opt);
156157
} else if (cmd_exec->parsed()) {
157-
options.subcommand_opt = exec_opt;
158+
options.subcommand_opt = std::move(exec_opt);
158159
} else if (cmd_kill->parsed()) {
159-
options.subcommand_opt = kill_opt;
160+
options.subcommand_opt = std::move(kill_opt);
160161
}
161162

162163
return options;

‎src/linyaps_box/config.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,12 @@ void from_json(const nlohmann::json &j, config::process_t &v)
200200
j.at("cwd").get_to(v.cwd);
201201

202202
if (auto it = j.find("env"); it != j.end()) {
203-
auto env = it->get<std::vector<std::string>>();
204-
for (const auto &e : env) {
203+
it->get_to(v.env);
204+
for (const auto &e : v.env) {
205205
if (e.find('=') == std::string::npos) {
206206
throw std::runtime_error("invalid env entry: " + e);
207207
}
208208
}
209-
v.env = std::move(env);
210209
}
211210

212211
j.at("args").get_to(v.args);
@@ -299,12 +298,14 @@ void from_json(const nlohmann::json &j, config::hooks_t::hook_t &v)
299298

300299
if (auto it = j.find("env"); it != j.end()) {
301300
std::unordered_map<std::string, std::string> env;
302-
for (const auto &e : it->get<std::vector<std::string>>()) {
301+
for (auto &e : it->get<std::vector<std::string>>()) {
303302
auto pos = e.find('=');
304303
if (pos == std::string::npos) {
305304
throw std::runtime_error("invalid env entry: " + e);
306305
}
307-
env[e.substr(0, pos)] = e.substr(pos + 1);
306+
auto key = e.substr(0, pos);
307+
e.erase(0, pos + 1);
308+
env[std::move(key)] = std::move(e);
308309
}
309310
v.env = std::move(env);
310311
}

‎src/linyaps_box/container.cpp‎

Lines changed: 24 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,6 @@
5555
constexpr auto propagations_flag = (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE);
5656
constexpr auto max_symlink_depth{ 32 };
5757

58-
namespace linyaps_box {
59-
60-
struct container_data
61-
{
62-
bool deny_setgroups{ false };
63-
bool mount_dev_from_host{ false };
64-
unsigned int rootfs_propagation{ 0 };
65-
};
66-
67-
container_data &get_private_data(const linyaps_box::container &c) noexcept
68-
{
69-
return *(c.data);
70-
}
71-
72-
} // namespace linyaps_box
73-
7458
namespace {
7559

7660
enum class sync_message : std::uint8_t {
@@ -335,7 +319,7 @@ void execute_hook(const linyaps_box::config::hooks_t::hook_t &hook,
335319
struct clone_fn_args
336320
{
337321
int preserve_fds;
338-
const linyaps_box::container *container;
322+
linyaps_box::container *container{ nullptr };
339323
linyaps_box::unix_socket socket;
340324
std::optional<linyaps_box::unix_socket> console_socket;
341325
};
@@ -643,7 +627,7 @@ void do_propagation_mount(const linyaps_box::utils::file_descriptor &destination
643627
throw std::runtime_error("mount cgroup: Not implemented");
644628
}
645629

646-
[[nodiscard]] std::optional<remount_t> do_mount(const linyaps_box::container &container,
630+
[[nodiscard]] std::optional<remount_t> do_mount(linyaps_box::container &container,
647631
const linyaps_box::utils::file_descriptor &root,
648632
const linyaps_box::config::mount_t &mount)
649633
{
@@ -693,7 +677,7 @@ void do_propagation_mount(const linyaps_box::utils::file_descriptor &destination
693677
}
694678

695679
if (mount.destination == "/dev") {
696-
linyaps_box::get_private_data(container).mount_dev_from_host = true;
680+
container.set_mount_dev_from_host();
697681
}
698682
} else {
699683
// mount other types
@@ -771,16 +755,15 @@ class mounter
771755
}
772756

773757
public:
774-
explicit mounter(linyaps_box::utils::file_descriptor rootfd,
775-
const linyaps_box::container &container)
758+
explicit mounter(linyaps_box::utils::file_descriptor rootfd, linyaps_box::container &container)
776759
: container(container)
777760
, root(std::move(rootfd))
778761
{
779762
}
780763

781764
void configure_rootfs()
782765
{
783-
const auto &config = container.get_config();
766+
const auto &config = container.get().get_config();
784767

785768
if (!config.linux || !config.linux->namespaces) {
786769
return;
@@ -816,7 +799,7 @@ class mounter
816799
// pivot root will reset the propagation type of rootfs mountpoint
817800
// we need to save the propagation type to make sure the parent mountpoint of new root is
818801
// what we want
819-
get_private_data(container).rootfs_propagation = flags;
802+
container.get().set_rootfs_propagation(flags);
820803

821804
LINYAPS_BOX_DEBUG() << "rebind container rootfs";
822805

@@ -841,7 +824,7 @@ class mounter
841824

842825
void do_mounts()
843826
{
844-
for (const auto &mount : container.get_config().mounts) {
827+
for (const auto &mount : container.get().get_config().mounts) {
845828
this->mount(mount);
846829
}
847830
}
@@ -866,7 +849,7 @@ class mounter
866849

867850
void make_path_readonly()
868851
{
869-
const auto &linux = container.get_config().linux;
852+
const auto &linux = container.get().get_config().linux;
870853
if (!linux || !linux->readonly_paths) {
871854
LINYAPS_BOX_DEBUG() << "no readonly paths";
872855
return;
@@ -914,7 +897,7 @@ class mounter
914897

915898
void make_path_masked()
916899
{
917-
const auto &linux = container.get_config().linux;
900+
const auto &linux = container.get().get_config().linux;
918901
if (!linux || !linux->masked_paths) {
919902
LINYAPS_BOX_DEBUG() << "no masked paths";
920903
return;
@@ -969,7 +952,7 @@ class mounter
969952
this->configure_default_filesystems();
970953

971954
// maybe user will bind mount the sub directory of / from host
972-
if (!linyaps_box::get_private_data(container).mount_dev_from_host) {
955+
if (!container.get().mount_dev_from_host()) {
973956
this->configure_default_devices();
974957
this->configure_dev_symlinks();
975958
}
@@ -981,7 +964,7 @@ class mounter
981964
}
982965

983966
private:
984-
const linyaps_box::container &container;
967+
std::reference_wrapper<linyaps_box::container> container;
985968
linyaps_box::utils::file_descriptor root;
986969
std::vector<remount_t> remounts;
987970

@@ -1210,8 +1193,8 @@ class mounter
12101193

12111194
constexpr auto default_mode = 0666;
12121195
constexpr auto default_type = std::filesystem::file_type::character;
1213-
auto uid = container.get_config().process.user.uid;
1214-
auto gid = container.get_config().process.user.gid;
1196+
auto uid = container.get().get_config().process.user.uid;
1197+
auto gid = container.get().get_config().process.user.gid;
12151198

12161199
this->configure_device("/dev/null", default_mode, default_type, makedev(1, 3), uid, gid);
12171200
this->configure_device("/dev/zero", default_mode, default_type, makedev(1, 5), uid, gid);
@@ -1244,7 +1227,7 @@ class mounter
12441227
}
12451228
};
12461229

1247-
void configure_mounts(const linyaps_box::container &container, const std::filesystem::path &rootfs)
1230+
void configure_mounts(linyaps_box::container &container, const std::filesystem::path &rootfs)
12481231
{
12491232
LINYAPS_BOX_DEBUG() << "Configure mounts";
12501233

@@ -1460,7 +1443,7 @@ void do_pivot_root(const linyaps_box::container &container, const std::filesyste
14601443

14611444
// restore the propagation type of rootfs mountpoint
14621445
do_propagation_mount(linyaps_box::utils::open("/", O_PATH | O_CLOEXEC | O_DIRECTORY),
1463-
get_private_data(container).rootfs_propagation);
1446+
container.rootfs_propagation());
14641447
}
14651448

14661449
void set_umask(const std::optional<mode_t> &mask)
@@ -1790,7 +1773,7 @@ try {
17901773
except_fds.insert(static_cast<unsigned int>(args.socket.fd().get()));
17911774
close_other_fds(std::move(except_fds));
17921775

1793-
const auto &container = *args.container;
1776+
auto &container = *args.container;
17941777
const auto &config = container.get_config();
17951778
auto &socket = args.socket;
17961779

@@ -1934,7 +1917,7 @@ void set_rlimits(const linyaps_box::config::process_t::rlimits_t &rlimits)
19341917
}
19351918

19361919
std::tuple<int, linyaps_box::unix_socket> start_container_process(
1937-
const linyaps_box::container &container, linyaps_box::run_container_options_t &options)
1920+
linyaps_box::container &container, linyaps_box::run_container_options_t &options)
19381921
{
19391922
const auto &config = container.get_config();
19401923
LINYAPS_BOX_DEBUG() << "All opened file describers before open sockets:\n"
@@ -2047,10 +2030,9 @@ std::tuple<int, linyaps_box::unix_socket> start_container_process(
20472030
throw std::runtime_error("user_namespace helper exited abnormally");
20482031
}
20492032

2050-
void set_deny_groups(const linyaps_box::container &container, const std::filesystem::path &filepath)
2033+
void set_deny_groups(linyaps_box::container &container, const std::filesystem::path &filepath)
20512034
{
2052-
auto data = linyaps_box::get_private_data(container);
2053-
if (data.deny_setgroups) {
2035+
if (container.deny_setgroups()) {
20542036
throw std::runtime_error("denying setgroups");
20552037
}
20562038

@@ -2060,10 +2042,10 @@ void set_deny_groups(const linyaps_box::container &container, const std::filesys
20602042
throw std::system_error{ errno, std::system_category(), "write setgroups" };
20612043
}
20622044

2063-
data.deny_setgroups = true;
2045+
container.set_deny_setgroups();
20642046
}
20652047

2066-
void configure_gid_mapping(pid_t pid, const linyaps_box::container &container)
2048+
void configure_gid_mapping(pid_t pid, linyaps_box::container &container)
20672049
{
20682050
LINYAPS_BOX_DEBUG() << "Configure GID mappings";
20692051

@@ -2081,8 +2063,7 @@ void configure_gid_mapping(pid_t pid, const linyaps_box::container &container)
20812063
const auto is_single_mapping = (gid_mappings_v.size() == 1 && gid_mappings_v[0].size == 1
20822064
&& gid_mappings_v[0].host_id == gid_mappings_v[0].container_id);
20832065
if (is_single_mapping) {
2084-
const auto &data = linyaps_box::get_private_data(container);
2085-
if (!data.deny_setgroups) {
2066+
if (!container.deny_setgroups()) {
20862067
set_deny_groups(container,
20872068
std::filesystem::path{ "/proc" } / std::to_string(pid) / "setgroups");
20882069
}
@@ -2236,7 +2217,7 @@ void configure_container_cgroup([[maybe_unused]] const linyaps_box::container &c
22362217
// do some other settings -> configuration done
22372218
}
22382219

2239-
void configure_container_namespaces(const linyaps_box::container &container,
2220+
void configure_container_namespaces(linyaps_box::container &container,
22402221
linyaps_box::unix_socket &socket)
22412222
{
22422223
LINYAPS_BOX_DEBUG()
@@ -2420,7 +2401,6 @@ void poststop_hooks(const linyaps_box::container &container) noexcept
24202401
linyaps_box::container::container(const status_directory &status_dir,
24212402
const create_container_options_t &options)
24222403
: container_ref(status_dir, options.ID)
2423-
, data(new linyaps_box::container_data)
24242404
, bundle(options.bundle)
24252405
{
24262406
auto config = options.config;
@@ -2469,11 +2449,6 @@ linyaps_box::container::container(const status_directory &status_dir,
24692449
}
24702450
}
24712451

2472-
linyaps_box::container::~container() noexcept
2473-
{
2474-
delete data;
2475-
}
2476-
24772452
const linyaps_box::config &linyaps_box::container::get_config() const
24782453
{
24792454
return this->config;
@@ -2485,7 +2460,7 @@ const std::filesystem::path &linyaps_box::container::get_bundle() const
24852460
}
24862461

24872462
// maybe we need a internal run function?
2488-
int linyaps_box::container::run(run_container_options_t options) const
2463+
int linyaps_box::container::run(run_container_options_t options)
24892464
{
24902465
int container_process_exit_code{ -1 };
24912466

‎src/linyaps_box/container.h‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,23 +40,41 @@ class container final : public container_ref
4040

4141
[[nodiscard]] auto get_config() const -> const linyaps_box::config &;
4242
[[nodiscard]] auto get_bundle() const -> const std::filesystem::path &;
43-
[[nodiscard]] auto run(run_container_options_t options) const -> int;
43+
[[nodiscard]] auto run(run_container_options_t options) -> int;
44+
4445
// TODO:: support fully container capabilities, e.g. create, start, stop, delete...
45-
friend auto get_private_data(const container &c) noexcept -> container_data &;
46-
~container() noexcept override;
46+
47+
~container() noexcept override = default;
4748

4849
[[nodiscard]] auto host_gid() const noexcept { return host_gid_; }
4950

5051
[[nodiscard]] auto host_uid() const noexcept { return host_uid_; }
5152

53+
[[nodiscard]] auto deny_setgroups() const noexcept { return deny_setgroups_; }
54+
55+
[[nodiscard]] auto mount_dev_from_host() const noexcept { return mount_dev_from_host_; }
56+
57+
[[nodiscard]] auto rootfs_propagation() const noexcept { return rootfs_propagation_; }
58+
59+
auto set_rootfs_propagation(unsigned int propagation) noexcept
60+
{
61+
rootfs_propagation_ = propagation;
62+
}
63+
64+
auto set_deny_setgroups() noexcept { deny_setgroups_ = true; }
65+
66+
auto set_mount_dev_from_host() noexcept { mount_dev_from_host_ = true; }
67+
5268
private:
5369
void cgroup_preenter(const cgroup_options &options, utils::file_descriptor &dirfd);
54-
gid_t host_gid_;
55-
uid_t host_uid_;
56-
container_data *data{ nullptr };
70+
linyaps_box::config config;
5771
std::filesystem::path bundle;
5872
std::unique_ptr<cgroup_manager> manager;
59-
linyaps_box::config config;
73+
unsigned int rootfs_propagation_{ 0 };
74+
gid_t host_gid_;
75+
uid_t host_uid_;
76+
bool deny_setgroups_{ false };
77+
bool mount_dev_from_host_{ false };
6078
};
6179

6280
} // namespace linyaps_box

0 commit comments

Comments
 (0)