From 7f95bb22d38b29fabb5f9127ac6602b4871317ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Nov 2019 10:15:57 +0100 Subject: [PATCH 1/8] resolve: rename define fixing a typo --- src/resolve/resolved-dnstls-gnutls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dnstls-gnutls.c b/src/resolve/resolved-dnstls-gnutls.c index 9e5e60fcce..ed0a31e8bf 100644 --- a/src/resolve/resolved-dnstls-gnutls.c +++ b/src/resolve/resolved-dnstls-gnutls.c @@ -9,7 +9,7 @@ #include "resolved-dns-stream.h" #include "resolved-dnstls.h" -#define PRIORTY_STRING "NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2" +#define TLS_PROTOCOL_PRIORITY "NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2" DEFINE_TRIVIAL_CLEANUP_FUNC(gnutls_session_t, gnutls_deinit); static ssize_t dnstls_stream_writev(gnutls_transport_ptr_t p, const giovec_t *iov, int iovcnt) { @@ -38,7 +38,7 @@ int dnstls_stream_connect_tls(DnsStream *stream, DnsServer *server) { return r; /* As DNS-over-TLS is a recent protocol, older TLS versions can be disabled */ - r = gnutls_priority_set_direct(gs, PRIORTY_STRING, NULL); + r = gnutls_priority_set_direct(gs, TLS_PROTOCOL_PRIORITY, NULL); if (r < 0) return r; From 8a99bd0c4601f639678b80db90db45609c90c5b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 20 Nov 2019 18:33:32 +0100 Subject: [PATCH 2/8] nspawn: dump capability list with --capabilities=help --- man/systemd-nspawn.xml | 10 ++++-- src/nspawn/nspawn.c | 76 ++++++++++++++++++++++++++---------------- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml index 92987ff32f..55809934f1 100644 --- a/man/systemd-nspawn.xml +++ b/man/systemd-nspawn.xml @@ -953,7 +953,10 @@ CAP_SETGID, CAP_SETPCAP, CAP_SETUID, CAP_SYS_ADMIN, CAP_SYS_BOOT, CAP_SYS_CHROOT, CAP_SYS_NICE, CAP_SYS_PTRACE, CAP_SYS_RESOURCE, CAP_SYS_TTY_CONFIG. Also CAP_NET_ADMIN is retained if is specified. If the special value - all is passed, all capabilities are retained. + all is passed, all capabilities are retained. + + If the special value of help is passed, the program will print known + capability names and exit. @@ -962,7 +965,10 @@ Specify one or more additional capabilities to drop for the container. This allows running the container with fewer capabilities than the default (see - above). + above). + + If the special value of help is passed, the program will print known + capability names and exit. diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 6286a28f1d..7f44272a88 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -492,6 +492,46 @@ static int detect_unified_cgroup_hierarchy_from_image(const char *directory) { return 0; } +static int parse_capability_spec(const char *spec, uint64_t *ret_mask) { + uint64_t mask = 0; + int r; + + for (;;) { + _cleanup_free_ char *t = NULL; + + r = extract_first_word(&spec, &t, ",", 0); + if (r < 0) + return log_error_errno(r, "Failed to parse capability %s.", t); + if (r == 0) + break; + + if (streq(t, "help")) { + for (int i = 0; i < capability_list_length(); i++) { + const char *name; + + name = capability_to_name(i); + if (name) + puts(name); + } + + return 0; /* quit */ + } + + if (streq(t, "all")) + mask = (uint64_t) -1; + else { + r = capability_from_name(t); + if (r < 0) + return log_error_errno(r, "Failed to parse capability %s.", t); + + mask |= 1ULL << r; + } + } + + *ret_mask = mask; + return 1; /* continue */ +} + static int parse_share_ns_env(const char *name, unsigned long ns_flag) { int r; @@ -695,7 +735,6 @@ static int parse_argv(int argc, char *argv[]) { }; int c, r; - const char *p; uint64_t plus = 0, minus = 0; bool mask_all_settings = false, mask_no_settings = false; @@ -937,37 +976,18 @@ static int parse_argv(int argc, char *argv[]) { case ARG_CAPABILITY: case ARG_DROP_CAPABILITY: { - p = optarg; - for (;;) { - _cleanup_free_ char *t = NULL; - - r = extract_first_word(&p, &t, ",", 0); - if (r < 0) - return log_error_errno(r, "Failed to parse capability %s.", t); - if (r == 0) - break; - - if (streq(t, "all")) { - if (c == ARG_CAPABILITY) - plus = (uint64_t) -1; - else - minus = (uint64_t) -1; - } else { - r = capability_from_name(t); - if (r < 0) - return log_error_errno(r, "Failed to parse capability %s.", t); - - if (c == ARG_CAPABILITY) - plus |= 1ULL << r; - else - minus |= 1ULL << r; - } - } + uint64_t m; + r = parse_capability_spec(optarg, &m); + if (r <= 0) + return r; + if (c == ARG_CAPABILITY) + plus |= m; + else + minus |= m; arg_settings_mask |= SETTING_CAPABILITY; break; } - case ARG_NO_NEW_PRIVILEGES: r = parse_boolean(optarg); if (r < 0) From ec562515331ee0d1b8de0e1a3364a35762206fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 20 Nov 2019 18:35:26 +0100 Subject: [PATCH 3/8] man: use for capability names in nspawn page --- man/systemd-nspawn.xml | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml index 55809934f1..8a2f792c5e 100644 --- a/man/systemd-nspawn.xml +++ b/man/systemd-nspawn.xml @@ -754,7 +754,7 @@ container, with the exception of the loopback device and those specified with and configured with . If this - option is specified, the CAP_NET_ADMIN capability will be + option is specified, the CAP_NET_ADMIN capability will be added to the set of capabilities the container retains. The latter may be disabled by using . If this option is not specified (or implied by one of the options @@ -943,17 +943,24 @@ - List one or more additional capabilities to grant the container. - Takes a comma-separated list of capability names, see - capabilities7 + List one or more additional capabilities to grant the container. Takes a + comma-separated list of capability names, see capabilities7 for more information. Note that the following capabilities will be granted in any way: - CAP_AUDIT_CONTROL, CAP_AUDIT_WRITE, CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH, - CAP_FOWNER, CAP_FSETID, CAP_IPC_OWNER, CAP_KILL, CAP_LEASE, CAP_LINUX_IMMUTABLE, - CAP_MKNOD, CAP_NET_BIND_SERVICE, CAP_NET_BROADCAST, CAP_NET_RAW, CAP_SETFCAP, - CAP_SETGID, CAP_SETPCAP, CAP_SETUID, CAP_SYS_ADMIN, CAP_SYS_BOOT, CAP_SYS_CHROOT, - CAP_SYS_NICE, CAP_SYS_PTRACE, CAP_SYS_RESOURCE, CAP_SYS_TTY_CONFIG. Also CAP_NET_ADMIN - is retained if is specified. If the special value - all is passed, all capabilities are retained. + CAP_AUDIT_CONTROL, CAP_AUDIT_WRITE, + CAP_CHOWN, CAP_DAC_OVERRIDE, + CAP_DAC_READ_SEARCH, CAP_FOWNER, + CAP_FSETID, CAP_IPC_OWNER, CAP_KILL, + CAP_LEASE, CAP_LINUX_IMMUTABLE, + CAP_MKNOD, CAP_NET_BIND_SERVICE, + CAP_NET_BROADCAST, CAP_NET_RAW, + CAP_SETFCAP, CAP_SETGID, CAP_SETPCAP, + CAP_SETUID, CAP_SYS_ADMIN, + CAP_SYS_BOOT, CAP_SYS_CHROOT, + CAP_SYS_NICE, CAP_SYS_PTRACE, + CAP_SYS_RESOURCE, CAP_SYS_TTY_CONFIG. Also + CAP_NET_ADMIN is retained if is specified. + If the special value all is passed, all capabilities are retained. If the special value of help is passed, the program will print known capability names and exit. From f47bd0974918abdb2f2453e8efec9be7409d9add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 20 Nov 2019 19:02:36 +0100 Subject: [PATCH 4/8] nspawn: log syscalls we cannot add at debug level Without out at least a debug log line it is hard to figure out when something goes wrong. Reduce scope of a variable while at it. --- src/nspawn/nspawn-seccomp.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c index 0b39cda9ba..f94f131f22 100644 --- a/src/nspawn/nspawn-seccomp.c +++ b/src/nspawn/nspawn-seccomp.c @@ -139,11 +139,10 @@ static int seccomp_add_default_syscall_filter( */ }; - int r; - size_t i; char **p; + int r; - for (i = 0; i < ELEMENTSOF(whitelist); i++) { + for (size_t i = 0; i < ELEMENTSOF(whitelist); i++) { if (whitelist[i].capability != 0 && (cap_list_retain & (1ULL << whitelist[i].capability)) == 0) continue; @@ -153,7 +152,7 @@ static int seccomp_add_default_syscall_filter( } STRV_FOREACH(p, syscall_whitelist) { - r = seccomp_add_syscall_filter_item(ctx, *p, SCMP_ACT_ALLOW, syscall_blacklist, false); + r = seccomp_add_syscall_filter_item(ctx, *p, SCMP_ACT_ALLOW, syscall_blacklist, true); if (r < 0) log_warning_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", *p, seccomp_arch_to_string(arch)); From af22794712102f362db1429d3c77c2f8aca14f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Nov 2019 13:44:33 +0100 Subject: [PATCH 5/8] machine: make machine_start_scope() static Having this function which is called only from one place in a separate file makes the code harder to follow. In preparation for subsequent changes, let's make it static. --- src/machine/machine.c | 98 +++++++++++++++++++++++++++++++++++-- src/machine/machined-dbus.c | 92 ---------------------------------- src/machine/machined.h | 1 - 3 files changed, 95 insertions(+), 96 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index acd57aedc4..f5892df8f6 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -330,7 +330,99 @@ int machine_load(Machine *m) { return r; } -static int machine_start_scope(Machine *m, sd_bus_message *properties, sd_bus_error *error) { +static int machine_start_scope( + Manager *manager, + const char *scope, + pid_t pid, + const char *slice, + const char *description, + sd_bus_message *more_properties, + sd_bus_error *error, + char **job) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; + int r; + + assert(manager); + assert(scope); + assert(pid > 1); + + r = sd_bus_message_new_method_call( + manager->bus, + &m, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "StartTransientUnit"); + if (r < 0) + return r; + + r = sd_bus_message_append(m, "ss", strempty(scope), "fail"); + if (r < 0) + return r; + + r = sd_bus_message_open_container(m, 'a', "(sv)"); + if (r < 0) + return r; + + if (!isempty(slice)) { + r = sd_bus_message_append(m, "(sv)", "Slice", "s", slice); + if (r < 0) + return r; + } + + if (!isempty(description)) { + r = sd_bus_message_append(m, "(sv)", "Description", "s", description); + if (r < 0) + return r; + } + + r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", + "PIDs", "au", 1, pid, + "Delegate", "b", 1, + "CollectMode", "s", "inactive-or-failed", + "AddRef", "b", 1, + "TasksMax", "t", UINT64_C(16384)); + if (r < 0) + return r; + + if (more_properties) { + r = sd_bus_message_copy(m, more_properties, true); + if (r < 0) + return r; + } + + r = sd_bus_message_close_container(m); + if (r < 0) + return r; + + r = sd_bus_message_append(m, "a(sa(sv))", 0); + if (r < 0) + return r; + + r = sd_bus_call(manager->bus, m, 0, error, &reply); + if (r < 0) + return r; + + if (job) { + const char *j; + char *copy; + + r = sd_bus_message_read(reply, "o", &j); + if (r < 0) + return r; + + copy = strdup(j); + if (!copy) + return -ENOMEM; + + *job = copy; + } + + return 1; +} + +static int machine_ensure_scope(Machine *m, sd_bus_message *properties, sd_bus_error *error) { assert(m); assert(m->class != MACHINE_HOST); @@ -349,7 +441,7 @@ static int machine_start_scope(Machine *m, sd_bus_message *properties, sd_bus_er description = strjoina(m->class == MACHINE_VM ? "Virtual Machine " : "Container ", m->name); - r = manager_start_scope(m->manager, scope, m->leader, SPECIAL_MACHINE_SLICE, description, properties, error, &job); + r = machine_start_scope(m->manager, scope, m->leader, SPECIAL_MACHINE_SLICE, description, properties, error, &job); if (r < 0) return log_error_errno(r, "Failed to start machine scope: %s", bus_error_message(error, r)); @@ -380,7 +472,7 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { return r; /* Create cgroup */ - r = machine_start_scope(m, properties, error); + r = machine_ensure_scope(m, properties, error); if (r < 0) return r; diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 35e06f5d6a..6fc3b93057 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1294,98 +1294,6 @@ int match_reloading(sd_bus_message *message, void *userdata, sd_bus_error *error return 0; } -int manager_start_scope( - Manager *manager, - const char *scope, - pid_t pid, - const char *slice, - const char *description, - sd_bus_message *more_properties, - sd_bus_error *error, - char **job) { - - _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; - int r; - - assert(manager); - assert(scope); - assert(pid > 1); - - r = sd_bus_message_new_method_call( - manager->bus, - &m, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "StartTransientUnit"); - if (r < 0) - return r; - - r = sd_bus_message_append(m, "ss", strempty(scope), "fail"); - if (r < 0) - return r; - - r = sd_bus_message_open_container(m, 'a', "(sv)"); - if (r < 0) - return r; - - if (!isempty(slice)) { - r = sd_bus_message_append(m, "(sv)", "Slice", "s", slice); - if (r < 0) - return r; - } - - if (!isempty(description)) { - r = sd_bus_message_append(m, "(sv)", "Description", "s", description); - if (r < 0) - return r; - } - - r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", - "PIDs", "au", 1, pid, - "Delegate", "b", 1, - "CollectMode", "s", "inactive-or-failed", - "AddRef", "b", 1, - "TasksMax", "t", UINT64_C(16384)); - if (r < 0) - return r; - - if (more_properties) { - r = sd_bus_message_copy(m, more_properties, true); - if (r < 0) - return r; - } - - r = sd_bus_message_close_container(m); - if (r < 0) - return r; - - r = sd_bus_message_append(m, "a(sa(sv))", 0); - if (r < 0) - return r; - - r = sd_bus_call(manager->bus, m, 0, error, &reply); - if (r < 0) - return r; - - if (job) { - const char *j; - char *copy; - - r = sd_bus_message_read(reply, "o", &j); - if (r < 0) - return r; - - copy = strdup(j); - if (!copy) - return -ENOMEM; - - *job = copy; - } - - return 1; -} - int manager_unref_unit( Manager *m, const char *unit, diff --git a/src/machine/machined.h b/src/machine/machined.h index 2298a652c4..205d90f83d 100644 --- a/src/machine/machined.h +++ b/src/machine/machined.h @@ -49,7 +49,6 @@ int match_unit_removed(sd_bus_message *message, void *userdata, sd_bus_error *er int match_properties_changed(sd_bus_message *message, void *userdata, sd_bus_error *error); int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *error); -int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, sd_bus_message *more_properties, sd_bus_error *error, char **job); int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_kill_unit(Manager *manager, const char *unit, int signo, sd_bus_error *error); int manager_unref_unit(Manager *m, const char *unit, sd_bus_error *error); From a01ecfa982e0e72f1ab52dade7bafe644bda893e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Nov 2019 14:32:51 +0100 Subject: [PATCH 6/8] machine: simplify machine_start_scope() It is called from only one place, and we can make things simpler by calculating the necessary stuff directly in the function. No functional change. --- src/machine/machine.c | 97 +++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 59 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index f5892df8f6..38c09d7117 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -331,24 +331,29 @@ int machine_load(Machine *m) { } static int machine_start_scope( - Manager *manager, - const char *scope, - pid_t pid, - const char *slice, - const char *description, + Machine *machine, sd_bus_message *more_properties, - sd_bus_error *error, - char **job) { + sd_bus_error *error) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; + _cleanup_free_ char *escaped = NULL, *unit = NULL; + const char *description; int r; - assert(manager); - assert(scope); - assert(pid > 1); + assert(machine); + assert(machine->leader > 0); + assert(!machine->unit); + + escaped = unit_name_escape(machine->name); + if (!escaped) + return log_oom(); + + unit = strjoin("machine-", escaped, ".scope"); + if (!unit) + return log_oom(); r = sd_bus_message_new_method_call( - manager->bus, + machine->manager->bus, &m, "org.freedesktop.systemd1", "/org/freedesktop/systemd1", @@ -357,7 +362,7 @@ static int machine_start_scope( if (r < 0) return r; - r = sd_bus_message_append(m, "ss", strempty(scope), "fail"); + r = sd_bus_message_append(m, "ss", unit, "fail"); if (r < 0) return r; @@ -365,20 +370,17 @@ static int machine_start_scope( if (r < 0) return r; - if (!isempty(slice)) { - r = sd_bus_message_append(m, "(sv)", "Slice", "s", slice); - if (r < 0) - return r; - } + r = sd_bus_message_append(m, "(sv)", "Slice", "s", SPECIAL_MACHINE_SLICE); + if (r < 0) + return r; - if (!isempty(description)) { - r = sd_bus_message_append(m, "(sv)", "Description", "s", description); - if (r < 0) - return r; - } + description = strjoina(machine->class == MACHINE_VM ? "Virtual Machine " : "Container ", machine->name); + r = sd_bus_message_append(m, "(sv)", "Description", "s", description); + if (r < 0) + return r; r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", - "PIDs", "au", 1, pid, + "PIDs", "au", 1, machine->leader, "Delegate", "b", 1, "CollectMode", "s", "inactive-or-failed", "AddRef", "b", 1, @@ -400,58 +402,35 @@ static int machine_start_scope( if (r < 0) return r; - r = sd_bus_call(manager->bus, m, 0, error, &reply); + r = sd_bus_call(NULL, m, 0, error, &reply); if (r < 0) return r; - if (job) { - const char *j; - char *copy; + machine->unit = TAKE_PTR(unit); + machine->referenced = true; - r = sd_bus_message_read(reply, "o", &j); - if (r < 0) - return r; + const char *job; + r = sd_bus_message_read(reply, "o", &job); + if (r < 0) + return r; - copy = strdup(j); - if (!copy) - return -ENOMEM; - - *job = copy; - } - - return 1; + return free_and_strdup(&machine->scope_job, job); } static int machine_ensure_scope(Machine *m, sd_bus_message *properties, sd_bus_error *error) { + int r; + assert(m); assert(m->class != MACHINE_HOST); if (!m->unit) { - _cleanup_free_ char *escaped = NULL, *scope = NULL; - char *description, *job = NULL; - int r; - - escaped = unit_name_escape(m->name); - if (!escaped) - return log_oom(); - - scope = strjoin("machine-", escaped, ".scope"); - if (!scope) - return log_oom(); - - description = strjoina(m->class == MACHINE_VM ? "Virtual Machine " : "Container ", m->name); - - r = machine_start_scope(m->manager, scope, m->leader, SPECIAL_MACHINE_SLICE, description, properties, error, &job); + r = machine_start_scope(m, properties, error); if (r < 0) return log_error_errno(r, "Failed to start machine scope: %s", bus_error_message(error, r)); - - m->unit = TAKE_PTR(scope); - m->referenced = true; - free_and_replace(m->scope_job, job); } - if (m->unit) - hashmap_put(m->manager->machine_units, m->unit, m); + assert(m->unit); + hashmap_put(m->manager->machine_units, m->unit, m); return 0; } From eec12b7756e2cada16e3b8a9c02f9eaf62df5409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Nov 2019 14:41:32 +0100 Subject: [PATCH 7/8] machined: simplify reference handling for units Before, we'd unref from machine_stop_unit, still keeping the unit name around, and only forget the name later, when garbage collecting. If we didn't call manager_stop_unit(), then we wouldn't do the unref. Let's unref at the same point where we do garbage collection, so that it is always true that iff we have the name generated with AddRef=1, then have a reference to the unit, and as soon as we forget the name, we drop the reference. This should fix the issue when repeated systemd-nspawn --register=yes fails with "scope already exists" error. Incidentally, this fixes an error in the code path where r was used instead of q. --- src/machine/machine.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 38c09d7117..d35eb1c14d 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -478,7 +478,7 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { static int machine_stop_scope(Machine *m) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *job = NULL; - int r, q; + int r; assert(m); assert(m->class != MACHINE_HOST); @@ -487,20 +487,11 @@ static int machine_stop_scope(Machine *m) { return 0; r = manager_stop_unit(m->manager, m->unit, &error, &job); - if (r < 0) { - log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); - sd_bus_error_free(&error); - } else - free_and_replace(m->scope_job, job); + if (r < 0) + return log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); - if (m->referenced) { - q = manager_unref_unit(m->manager, m->unit, &error); - if (q < 0) - log_warning_errno(q, "Failed to drop reference to machine scope, ignoring: %s", bus_error_message(&error, r)); - m->referenced = false; - } - - return r; + free_and_replace(m->scope_job, job); + return 0; } int machine_stop(Machine *m) { @@ -654,6 +645,18 @@ void machine_release_unit(Machine *m) { if (!m->unit) return; + if (m->referenced) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + int r; + + r = manager_unref_unit(m->manager, m->unit, &error); + if (r < 0) + log_warning_errno(r, "Failed to drop reference to machine scope, ignoring: %s", + bus_error_message(&error, r)); + + m->referenced = false; + } + (void) hashmap_remove(m->manager->machine_units, m->unit); m->unit = mfree(m->unit); } From 698876640d6e8ecbcfb99acc32ad3005877842f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Nov 2019 14:54:11 +0100 Subject: [PATCH 8/8] machine: fold machine_stop_scope() into machine_stop() No functional change. --- src/machine/machine.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index d35eb1c14d..efe327f381 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -475,40 +475,31 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { return 0; } -static int machine_stop_scope(Machine *m) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - char *job = NULL; - int r; - - assert(m); - assert(m->class != MACHINE_HOST); - - if (!m->unit) - return 0; - - r = manager_stop_unit(m->manager, m->unit, &error, &job); - if (r < 0) - return log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); - - free_and_replace(m->scope_job, job); - return 0; -} - int machine_stop(Machine *m) { int r; + assert(m); if (!IN_SET(m->class, MACHINE_CONTAINER, MACHINE_VM)) return -EOPNOTSUPP; - r = machine_stop_scope(m); + if (m->unit) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + char *job = NULL; + + r = manager_stop_unit(m->manager, m->unit, &error, &job); + if (r < 0) + return log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); + + free_and_replace(m->scope_job, job); + } m->stopping = true; machine_save(m); (void) manager_enqueue_nscd_cache_flush(m->manager); - return r; + return 0; } int machine_finalize(Machine *m) {