From 533c20ca5bd7002d8983b7d48f47ff78c25347c3 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 20 Sep 2024 23:58:14 +0200 Subject: [PATCH 1/3] machined: fix bogus error check for machine_link() --- src/machine/machine.c | 3 +-- src/machine/machined-dbus.c | 12 +++++++----- src/machine/machined.h | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 518eabed10..91e0e6be3d 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -53,6 +53,7 @@ int machine_new(MachineClass class, const char *name, Machine **ret) { return -ENOMEM; *m = (Machine) { + .class = class, .leader = PIDREF_NULL, .vsock_cid = VMADDR_CID_ANY, }; @@ -63,8 +64,6 @@ int machine_new(MachineClass class, const char *name, Machine **ret) { return -ENOMEM; } - m->class = class; - *ret = TAKE_PTR(m); return 0; } diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 58997e6ec1..97a8a67236 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1487,7 +1487,7 @@ int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **machine) { return 1; } -int manager_add_machine(Manager *m, const char *name, Machine **_machine) { +int manager_add_machine(Manager *m, const char *name, Machine **ret) { Machine *machine; int r; @@ -1501,12 +1501,14 @@ int manager_add_machine(Manager *m, const char *name, Machine **_machine) { return r; r = machine_link(m, machine); - if (r < 0) - return 0; + if (r < 0) { + machine_free(machine); + return r; + } } - if (_machine) - *_machine = machine; + if (ret) + *ret = machine; return 0; } diff --git a/src/machine/machined.h b/src/machine/machined.h index 4dbb083e8a..3dffcb9fe0 100644 --- a/src/machine/machined.h +++ b/src/machine/machined.h @@ -42,7 +42,7 @@ struct Manager { sd_varlink_server *varlink_machine_server; }; -int manager_add_machine(Manager *m, const char *name, Machine **_machine); +int manager_add_machine(Manager *m, const char *name, Machine **ret); int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **machine); extern const BusObjectImplementation manager_object; From 85ab917bc8a6ec74badeede30a9c18bc95ec2639 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 21 Sep 2024 00:05:36 +0200 Subject: [PATCH 2/3] machined: rename machine_{units,leaders} to machines_by_* Also port machines_by_leader to store PidRef-s. --- src/machine/machine.c | 13 +++++++++---- src/machine/machined-dbus.c | 24 +++++++++++++----------- src/machine/machined.c | 15 ++++++++------- src/machine/machined.h | 6 +++--- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 91e0e6be3d..036a5b4edb 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -120,7 +120,7 @@ Machine* machine_free(Machine *m) { m->leader_pidfd_event_source = sd_event_source_disable_unref(m->leader_pidfd_event_source); if (pidref_is_set(&m->leader)) { if (m->manager) - (void) hashmap_remove_value(m->manager->machine_leaders, PID_TO_PTR(m->leader.pid), m); + (void) hashmap_remove_value(m->manager->machines_by_leader, &m->leader, m); pidref_done(&m->leader); } @@ -470,7 +470,10 @@ static int machine_ensure_scope(Machine *m, sd_bus_message *properties, sd_bus_e } assert(m->unit); - hashmap_put(m->manager->machine_units, m->unit, m); + + r = hashmap_ensure_put(&m->manager->machines_by_unit, &string_hash_ops, m->unit, m); + if (r < 0) + return r; return 0; } @@ -518,7 +521,7 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { if (m->started) return 0; - r = hashmap_put(m->manager->machine_leaders, PID_TO_PTR(m->leader.pid), m); + r = hashmap_ensure_put(&m->manager->machines_by_leader, &pidref_hash_ops, &m->leader, m); if (r < 0) return r; @@ -705,6 +708,8 @@ void machine_release_unit(Machine *m) { if (!m->unit) return; + assert(m->manager); + if (m->referenced) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r; @@ -718,7 +723,7 @@ void machine_release_unit(Machine *m) { m->referenced = false; } - (void) hashmap_remove(m->manager->machine_units, m->unit); + (void) hashmap_remove_value(m->manager->machines_by_unit, m->unit, m); m->unit = mfree(m->unit); } diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 97a8a67236..a44d8e7598 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -169,7 +169,7 @@ static int method_get_machine_by_pid(sd_bus_message *message, void *userdata, sd r = manager_get_machine_by_pid(m, pid, &machine); if (r < 0) return r; - if (!machine) + if (r == 0) return sd_bus_error_setf(error, BUS_ERROR_NO_MACHINE_FOR_PID, "PID "PID_FMT" does not belong to any known machine", pid); p = machine_bus_path(machine); @@ -1229,7 +1229,7 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err return 0; } - machine = hashmap_get(m->machine_units, unit); + machine = hashmap_get(m->machines_by_unit, unit); if (!machine) return 0; @@ -1276,7 +1276,7 @@ int match_properties_changed(sd_bus_message *message, void *userdata, sd_bus_err return 0; } - machine = hashmap_get(m->machine_units, unit); + machine = hashmap_get(m->machines_by_unit, unit); if (!machine) return 0; @@ -1298,7 +1298,7 @@ int match_unit_removed(sd_bus_message *message, void *userdata, sd_bus_error *er return 0; } - machine = hashmap_get(m->machine_units, unit); + machine = hashmap_get(m->machines_by_unit, unit); if (!machine) return 0; @@ -1464,26 +1464,28 @@ int manager_job_is_active(Manager *manager, const char *path) { return true; } -int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **machine) { +int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **ret) { Machine *mm; int r; assert(m); - assert(pid >= 1); - assert(machine); + assert(pid_is_valid(pid)); + assert(ret); - mm = hashmap_get(m->machine_leaders, PID_TO_PTR(pid)); + mm = hashmap_get(m->machines_by_leader, &PIDREF_MAKE_FROM_PID(pid)); if (!mm) { _cleanup_free_ char *unit = NULL; r = cg_pid_get_unit(pid, &unit); if (r >= 0) - mm = hashmap_get(m->machine_units, unit); + mm = hashmap_get(m->machines_by_unit, unit); } - if (!mm) + if (!mm) { + *ret = NULL; return 0; + } - *machine = mm; + *ret = mm; return 1; } diff --git a/src/machine/machined.c b/src/machine/machined.c index 491f7c735e..a0c4ef751a 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -45,10 +45,7 @@ static int manager_new(Manager **ret) { return -ENOMEM; m->machines = hashmap_new(&machine_hash_ops); - m->machine_units = hashmap_new(&string_hash_ops); - m->machine_leaders = hashmap_new(NULL); - - if (!m->machines || !m->machine_units || !m->machine_leaders) + if (!m->machines) return -ENOMEM; r = sd_event_default(&m->event); @@ -83,9 +80,13 @@ static Manager* manager_unref(Manager *m) { assert(m->n_operations == 0); - hashmap_free(m->machines); /* This will free all machines, so that the machine_units/machine_leaders is empty */ - hashmap_free(m->machine_units); - hashmap_free(m->machine_leaders); + hashmap_free(m->machines); /* This will free all machines, thus the by_unit/by_leader hashmaps shall be empty */ + + assert(hashmap_isempty(m->machines_by_unit)); + hashmap_free(m->machines_by_unit); + assert(hashmap_isempty(m->machines_by_leader)); + hashmap_free(m->machines_by_leader); + hashmap_free(m->image_cache); sd_event_source_unref(m->image_cache_defer_event); diff --git a/src/machine/machined.h b/src/machine/machined.h index 3dffcb9fe0..4ba54ebe20 100644 --- a/src/machine/machined.h +++ b/src/machine/machined.h @@ -21,8 +21,8 @@ struct Manager { sd_bus *bus; Hashmap *machines; - Hashmap *machine_units; - Hashmap *machine_leaders; + Hashmap *machines_by_unit; + Hashmap *machines_by_leader; sd_event_source *deferred_gc_event_source; @@ -43,7 +43,7 @@ struct Manager { }; int manager_add_machine(Manager *m, const char *name, Machine **ret); -int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **machine); +int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **ret); extern const BusObjectImplementation manager_object; From f3270bed489367de2a1bbf84b4686362eb1471d5 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 21 Sep 2024 00:15:15 +0200 Subject: [PATCH 3/3] machined-dbus: move manager_add_machine() and _get_machine_by_pid() to -core --- src/machine/machined-core.c | 53 +++++++++++++++++++++++++++++++++++++ src/machine/machined-dbus.c | 51 ----------------------------------- 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/machine/machined-core.c b/src/machine/machined-core.c index e3072a5503..1a840a6a37 100644 --- a/src/machine/machined-core.c +++ b/src/machine/machined-core.c @@ -1,9 +1,62 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "cgroup-util.h" #include "machined.h" +#include "process-util.h" #include "strv.h" #include "user-util.h" +int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **ret) { + Machine *mm; + int r; + + assert(m); + assert(pid_is_valid(pid)); + assert(ret); + + mm = hashmap_get(m->machines_by_leader, &PIDREF_MAKE_FROM_PID(pid)); + if (!mm) { + _cleanup_free_ char *unit = NULL; + + r = cg_pid_get_unit(pid, &unit); + if (r >= 0) + mm = hashmap_get(m->machines_by_unit, unit); + } + if (!mm) { + *ret = NULL; + return 0; + } + + *ret = mm; + return 1; +} + +int manager_add_machine(Manager *m, const char *name, Machine **ret) { + Machine *machine; + int r; + + assert(m); + assert(name); + + machine = hashmap_get(m->machines, name); + if (!machine) { + r = machine_new(_MACHINE_CLASS_INVALID, name, &machine); + if (r < 0) + return r; + + r = machine_link(m, machine); + if (r < 0) { + machine_free(machine); + return r; + } + } + + if (ret) + *ret = machine; + + return 0; +} + int manager_find_machine_for_uid(Manager *m, uid_t uid, Machine **ret_machine, uid_t *ret_internal_uid) { Machine *machine; int r; diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index a44d8e7598..e86be6d4d5 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1463,54 +1463,3 @@ int manager_job_is_active(Manager *manager, const char *path) { return true; } - -int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **ret) { - Machine *mm; - int r; - - assert(m); - assert(pid_is_valid(pid)); - assert(ret); - - mm = hashmap_get(m->machines_by_leader, &PIDREF_MAKE_FROM_PID(pid)); - if (!mm) { - _cleanup_free_ char *unit = NULL; - - r = cg_pid_get_unit(pid, &unit); - if (r >= 0) - mm = hashmap_get(m->machines_by_unit, unit); - } - if (!mm) { - *ret = NULL; - return 0; - } - - *ret = mm; - return 1; -} - -int manager_add_machine(Manager *m, const char *name, Machine **ret) { - Machine *machine; - int r; - - assert(m); - assert(name); - - machine = hashmap_get(m->machines, name); - if (!machine) { - r = machine_new(_MACHINE_CLASS_INVALID, name, &machine); - if (r < 0) - return r; - - r = machine_link(m, machine); - if (r < 0) { - machine_free(machine); - return r; - } - } - - if (ret) - *ret = machine; - - return 0; -}