From f283816acb6e7fd444df06b103dd179ce5249126 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 24 Oct 2024 19:29:57 +0900 Subject: [PATCH 01/12] machine: use sd_json_variant_append_arraybo() and JSON_BUILD_PAIR_VARIANT_NON_NULL() Follow-up for 45755275e5ae747bf79e6c7bdd9a055711ebc71f. --- src/machine/machined-varlink.c | 41 ++++++++++++++-------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c index 2c2eb2d918..723c755f70 100644 --- a/src/machine/machined-varlink.c +++ b/src/machine/machined-varlink.c @@ -392,56 +392,49 @@ static int vl_method_get_memberships(sd_varlink *link, sd_json_variant *paramete } static int json_build_local_addresses(const struct local_address *addresses, size_t n_addresses, sd_json_variant **ret) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *array = NULL; int r; - if (n_addresses == 0) - return 0; - - assert(addresses); + assert(addresses || n_addresses == 0); assert(ret); FOREACH_ARRAY(a, addresses, n_addresses) { - _cleanup_(sd_json_variant_unrefp) sd_json_variant *entry = NULL; - r = sd_json_buildo( - &entry, + r = sd_json_variant_append_arraybo( + &array, JSON_BUILD_PAIR_UNSIGNED_NON_ZERO("ifindex", a->ifindex), SD_JSON_BUILD_PAIR_INTEGER("family", a->family), SD_JSON_BUILD_PAIR_BYTE_ARRAY("address", &a->address.bytes, FAMILY_ADDRESS_SIZE(a->family))); if (r < 0) return r; - - r = sd_json_variant_append_array(ret, entry); - if (r < 0) - return r; } + *ret = TAKE_PTR(array); return 0; } static int list_machine_one_and_maybe_read_metadata(sd_varlink *link, Machine *m, bool more, AcquireMetadata am) { - _cleanup_(sd_json_variant_unrefp) sd_json_variant *addr_array = NULL; + _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL, *addr_array = NULL; _cleanup_strv_free_ char **os_release = NULL; uid_t shift = UID_INVALID; - int r, n = 0; + int r; assert(link); assert(m); - _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; - if (should_acquire_metadata(am)) { _cleanup_free_ struct local_address *addresses = NULL; - n = machine_get_addresses(m, &addresses); - if (n < 0 && am == ACQUIRE_METADATA_GRACEFUL) - log_debug_errno(n, "Failed to get address (graceful mode), ignoring: %m"); - else if (n == -ENONET) + + r = machine_get_addresses(m, &addresses); + if (r < 0 && am == ACQUIRE_METADATA_GRACEFUL) + log_debug_errno(r, "Failed to get address (graceful mode), ignoring: %m"); + else if (r == -ENONET) return sd_varlink_error(link, "io.systemd.Machine.NoPrivateNetworking", NULL); - else if (ERRNO_IS_NEG_NOT_SUPPORTED(n)) + else if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) return sd_varlink_error(link, "io.systemd.Machine.NotAvailable", NULL); - else if (n < 0) - return log_debug_errno(n, "Failed to get addresses: %m"); + else if (r < 0) + return log_debug_errno(r, "Failed to get addresses: %m"); else { - r = json_build_local_addresses(addresses, n, &addr_array); + r = json_build_local_addresses(addresses, r, &addr_array); if (r < 0) return r; } @@ -480,7 +473,7 @@ static int list_machine_one_and_maybe_read_metadata(sd_varlink *link, Machine *m SD_JSON_BUILD_PAIR_CONDITION(m->vsock_cid != VMADDR_CID_ANY, "vSockCid", SD_JSON_BUILD_UNSIGNED(m->vsock_cid)), JSON_BUILD_PAIR_STRING_NON_EMPTY("sshAddress", m->ssh_address), JSON_BUILD_PAIR_STRING_NON_EMPTY("sshPrivateKeyPath", m->ssh_private_key_path), - SD_JSON_BUILD_PAIR_CONDITION(n > 0, "addresses", SD_JSON_BUILD_VARIANT(addr_array)), + JSON_BUILD_PAIR_VARIANT_NON_NULL("addresses", addr_array), SD_JSON_BUILD_PAIR_CONDITION(!strv_isempty(os_release), "OSRelease", JSON_BUILD_STRV_ENV_PAIR(os_release)), JSON_BUILD_PAIR_UNSIGNED_NOT_EQUAL("UIDShift", shift, UID_INVALID)); if (r < 0) From 20e1237ce1bb6f7c2f8df587c6cd032702ef5fc9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 24 Oct 2024 19:32:36 +0900 Subject: [PATCH 02/12] machined: ACQUIRE_METADATA_NO is zero Follow-ups for a94fbcaa35dc63f32fbf86d25f63f6ac40a0d8b0 and 9de215219c8783e3239af27baf62275730ab51a8. --- src/machine/machined-varlink.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c index 723c755f70..f7cfaba78a 100644 --- a/src/machine/machined-varlink.c +++ b/src/machine/machined-varlink.c @@ -510,7 +510,6 @@ static int vl_method_list(sd_varlink *link, sd_json_variant *parameters, sd_varl Manager *m = ASSERT_PTR(userdata); _cleanup_(machine_lookup_parameters_done) MachineLookupParameters p = { .pidref = PIDREF_NULL, - .acquire_metadata = ACQUIRE_METADATA_NO, }; Machine *machine; @@ -644,7 +643,7 @@ static int vl_method_list_images(sd_varlink *link, sd_json_variant *parameters, struct params { const char *image_name; AcquireMetadata acquire_metadata; - } p = { .acquire_metadata = ACQUIRE_METADATA_NO }; + } p = {}; int r; static const sd_json_dispatch_field dispatch_table[] = { From e79b58c825d7c04cdd2c48c8cffbd5fed192cde3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 24 Oct 2024 06:00:53 +0900 Subject: [PATCH 03/12] TEST-13-NSPAWN: fix race between container exit and varlink call Follow-up for 3cb72c7862d8950bc2a963aaa3ca3255eea374b6. The test container exits shortly, hence when varlinkctl is called, the container may be already terminated. Let's make the container live infinitely. Also, this makes the os-release files removed after the container is started. --- test/units/TEST-13-NSPAWN.machined.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/units/TEST-13-NSPAWN.machined.sh b/test/units/TEST-13-NSPAWN.machined.sh index 244582677d..24eb673af3 100755 --- a/test/units/TEST-13-NSPAWN.machined.sh +++ b/test/units/TEST-13-NSPAWN.machined.sh @@ -304,7 +304,26 @@ varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Unreg # test io.systemd.Machine.List with addresses, OSRelease, and UIDShift fields create_dummy_container "/var/lib/machines/container-without-os-release" +cat >>/var/lib/machines/container-without-os-release/sbin/init <<\EOF +PID=0 + +trap 'kill 0' RTMIN+3 +trap 'kill $PID' EXIT + +# We need to wait for the sleep process asynchronously in order to allow +# bash to process signals +sleep infinity & + +# notify that the process is ready +touch /ready + +PID=$! +while :; do + wait || : +done +EOF machinectl start "container-without-os-release" +timeout 30 bash -c "until test -e /var/lib/machines/container-without-os-release/ready; do sleep .5; done" rm -f /var/lib/machines/container-without-os-release/etc/os-release /var/lib/machines/container-without-os-release/usr/lib/os-release (! varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name": "container-without-os-release", "acquireMetadata": "yes"}') varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name": "container-without-os-release", "acquireMetadata": "graceful"}' From 1856ab4503a1c06994e82ff6c0dfc3a6dc41c69a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 25 Oct 2024 00:24:04 +0900 Subject: [PATCH 04/12] TEST-13-NSPAWN: check returned machine list --- test/units/TEST-13-NSPAWN.machined.sh | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/units/TEST-13-NSPAWN.machined.sh b/test/units/TEST-13-NSPAWN.machined.sh index 24eb673af3..d014dfe1ba 100755 --- a/test/units/TEST-13-NSPAWN.machined.sh +++ b/test/units/TEST-13-NSPAWN.machined.sh @@ -305,6 +305,10 @@ varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Unreg # test io.systemd.Machine.List with addresses, OSRelease, and UIDShift fields create_dummy_container "/var/lib/machines/container-without-os-release" cat >>/var/lib/machines/container-without-os-release/sbin/init <<\EOF +ip link add hoge type dummy +ip link set hoge up +ip address add 192.0.2.1/24 dev hoge + PID=0 trap 'kill 0' RTMIN+3 @@ -326,7 +330,17 @@ machinectl start "container-without-os-release" timeout 30 bash -c "until test -e /var/lib/machines/container-without-os-release/ready; do sleep .5; done" rm -f /var/lib/machines/container-without-os-release/etc/os-release /var/lib/machines/container-without-os-release/usr/lib/os-release (! varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name": "container-without-os-release", "acquireMetadata": "yes"}') -varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name": "container-without-os-release", "acquireMetadata": "graceful"}' +output=$(varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name": "container-without-os-release", "acquireMetadata": "graceful"}') +assert_eq "$(echo "$output" | jq --seq .name | tr -d \\036)" '"container-without-os-release"' +assert_eq "$(echo "$output" | jq --seq .class | tr -d \\036)" '"container"' +assert_eq "$(echo "$output" | jq --seq .service | tr -d \\036)" '"systemd-nspawn"' +assert_eq "$(echo "$output" | jq --seq .rootDirectory | tr -d \\036)" '"/var/lib/machines/container-without-os-release"' +assert_eq "$(echo "$output" | jq --seq .unit | tr -d \\036)" '"systemd-nspawn@container-without-os-release.service"' +assert_eq "$(echo "$output" | jq --seq .addresses[0].family | tr -d \\036)" '2' +assert_eq "$(echo "$output" | jq --seq .addresses[0].address[0] | tr -d \\036)" '192' +assert_eq "$(echo "$output" | jq --seq .addresses[0].address[1] | tr -d \\036)" '0' +assert_eq "$(echo "$output" | jq --seq .addresses[0].address[2] | tr -d \\036)" '2' +assert_eq "$(echo "$output" | jq --seq .addresses[0].address[3] | tr -d \\036)" '1' machinectl terminate "container-without-os-release" (ip addr show lo | grep -q 192.168.1.100) || ip address add 192.168.1.100/24 dev lo From f0fdefc045b4c639e45c6731d3fa6d44e7468f06 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 25 Oct 2024 00:42:28 +0900 Subject: [PATCH 05/12] TEST-13-NSPAWN: trivially kill all processes in the container on termination Follow-up for 841988f80d2440501a3baddba712cdb955225ab7. No functional change, as $PID is 0 when the trap is inserted. --- test/units/TEST-13-NSPAWN.machined.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/units/TEST-13-NSPAWN.machined.sh b/test/units/TEST-13-NSPAWN.machined.sh index d014dfe1ba..b259a5bc16 100755 --- a/test/units/TEST-13-NSPAWN.machined.sh +++ b/test/units/TEST-13-NSPAWN.machined.sh @@ -39,10 +39,10 @@ cat >/var/lib/machines/long-running/sbin/init <<\EOF PID=0 -trap "touch /terminate; kill $PID" RTMIN+3 -trap "touch /poweroff" RTMIN+4 -trap "touch /reboot" INT -trap "touch /trap" TRAP +trap 'touch /terminate; kill 0' RTMIN+3 +trap 'touch /poweroff' RTMIN+4 +trap 'touch /reboot' INT +trap 'touch /trap' TRAP trap 'kill $PID' EXIT # We need to wait for the sleep process asynchronously in order to allow From b1e11a252ddd6b826e17e630e4c8f2f42ef7773d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 31 Oct 2024 11:02:23 +0900 Subject: [PATCH 06/12] TEST-13-NSPAWN: add test cases for listing multiple machines --- test/units/TEST-13-NSPAWN.machined.sh | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/units/TEST-13-NSPAWN.machined.sh b/test/units/TEST-13-NSPAWN.machined.sh index b259a5bc16..9a6e86cefb 100755 --- a/test/units/TEST-13-NSPAWN.machined.sh +++ b/test/units/TEST-13-NSPAWN.machined.sh @@ -341,7 +341,23 @@ assert_eq "$(echo "$output" | jq --seq .addresses[0].address[0] | tr -d \\036)" assert_eq "$(echo "$output" | jq --seq .addresses[0].address[1] | tr -d \\036)" '0' assert_eq "$(echo "$output" | jq --seq .addresses[0].address[2] | tr -d \\036)" '2' assert_eq "$(echo "$output" | jq --seq .addresses[0].address[3] | tr -d \\036)" '1' -machinectl terminate "container-without-os-release" +# test for listing multiple machines. +long_running_machine_start +varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{}' +varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"acquireMetadata": "no"}' +varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"acquireMetadata": "graceful"}' +# check if machined does not try to send anything after error message +journalctl --sync +TS="$(date '+%H:%M:%S')" +(! varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"acquireMetadata": "yes"}') +journalctl --sync +(! journalctl -u systemd-machined.service --since="$TS" --grep 'Connection busy') +# terminate machines +machinectl terminate container-without-os-release +machinectl terminate long-running +# wait for the container being stopped, otherwise acquiring image metadata by io.systemd.MachineImage.List may fail in the below. +timeout 10 bash -c "while machinectl status long-running &>/dev/null; do sleep .5; done" +systemctl kill --signal=KILL systemd-nspawn@long-running.service || : (ip addr show lo | grep -q 192.168.1.100) || ip address add 192.168.1.100/24 dev lo (! varlinkctl --more call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.List '{"name": ".host"}' | grep 'addresses') From e4d477efc6e8500d3e360466ed498aaf1d9a04fa Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 26 Oct 2024 01:46:39 +0900 Subject: [PATCH 07/12] env-util: replace 'char **' with 'char**' --- src/basic/env-util.c | 4 ++-- src/basic/env-util.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 4c06d1a165..3b4b570b9c 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -266,7 +266,7 @@ static bool env_entry_has_name(const char *entry, const char *name) { return *t == '='; } -char **strv_env_delete(char **x, size_t n_lists, ...) { +char** strv_env_delete(char **x, size_t n_lists, ...) { size_t n, i = 0; _cleanup_strv_free_ char **t = NULL; va_list ap; @@ -564,7 +564,7 @@ char* strv_env_pairs_get(char **l, const char *name) { return result; } -char **strv_env_clean_with_callback(char **e, void (*invalid_callback)(const char *p, void *userdata), void *userdata) { +char** strv_env_clean_with_callback(char **e, void (*invalid_callback)(const char *p, void *userdata), void *userdata) { int k = 0; STRV_FOREACH(p, e) { diff --git a/src/basic/env-util.h b/src/basic/env-util.h index 5f20f1d69c..f5ceba453a 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -34,14 +34,14 @@ int replace_env_argv(char **argv, char **env, char ***ret, char ***ret_unset_var bool strv_env_is_valid(char **e); #define strv_env_clean(l) strv_env_clean_with_callback(l, NULL, NULL) -char **strv_env_clean_with_callback(char **l, void (*invalid_callback)(const char *p, void *userdata), void *userdata); +char** strv_env_clean_with_callback(char **l, void (*invalid_callback)(const char *p, void *userdata), void *userdata); bool strv_env_name_is_valid(char **l); bool strv_env_name_or_assignment_is_valid(char **l); char** _strv_env_merge(char **first, ...); #define strv_env_merge(first, ...) _strv_env_merge(first, __VA_ARGS__, POINTER_MAX) -char **strv_env_delete(char **x, size_t n_lists, ...); /* New copy */ +char** strv_env_delete(char **x, size_t n_lists, ...); /* New copy */ char** strv_env_unset(char **l, const char *p); /* In place ... */ char** strv_env_unset_many_internal(char **l, ...) _sentinel_; From 7633001cddc2ddd906f39826be35cff20a327543 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 26 Oct 2024 02:01:19 +0900 Subject: [PATCH 08/12] env-util: introduce strv_env_get_merged() --- src/basic/env-util.c | 28 ++++++++++++++++++++++++++++ src/basic/env-util.h | 1 + src/test/test-env-util.c | 11 +++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 3b4b570b9c..99ce1a1842 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -564,6 +564,34 @@ char* strv_env_pairs_get(char **l, const char *name) { return result; } +int strv_env_get_merged(char **l, char ***ret) { + _cleanup_strv_free_ char **v = NULL; + size_t n = 0; + int r; + + assert(ret); + + /* This converts a strv with pairs of environment variable name + value into a strv of name and + * value concatenated with a "=" separator. E.g. + * input : { "NAME", "value", "FOO", "var" } + * output : { "NAME=value", "FOO=var" } */ + + STRV_FOREACH_PAIR(key, value, l) { + char *s; + + s = strjoin(*key, "=", *value); + if (!s) + return -ENOMEM; + + r = strv_consume_with_size(&v, &n, s); + if (r < 0) + return r; + } + + *ret = TAKE_PTR(v); + return 0; +} + char** strv_env_clean_with_callback(char **e, void (*invalid_callback)(const char *p, void *userdata), void *userdata) { int k = 0; diff --git a/src/basic/env-util.h b/src/basic/env-util.h index f5ceba453a..203ed65bd1 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -60,6 +60,7 @@ static inline char* strv_env_get(char * const *x, const char *n) { } char* strv_env_pairs_get(char **l, const char *name) _pure_; +int strv_env_get_merged(char **l, char ***ret); int getenv_bool(const char *p); int secure_getenv_bool(const char *p); diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index e2c009dc9c..7eda66bd3e 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -581,4 +581,15 @@ TEST(getenv_path_list) { assert_se(unsetenv("TEST_GETENV_PATH_LIST") >= 0); } +TEST(strv_env_get_merged) { + char **l = STRV_MAKE("ONE", "1", "TWO", "2", "THREE", "3", "FOUR", "4", "FIVE", "5"), + **expected = STRV_MAKE("ONE=1", "TWO=2", "THREE=3", "FOUR=4", "FIVE=5"); + _cleanup_strv_free_ char **m = NULL; + + ASSERT_OK(strv_env_get_merged(NULL, &m)); + ASSERT_NULL(m); + ASSERT_OK(strv_env_get_merged(l, &m)); + ASSERT_TRUE(strv_equal(m, expected)); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 32d77f5df8b942805d6084b695be0f72bf1e487d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 26 Oct 2024 02:07:02 +0900 Subject: [PATCH 09/12] sd-json: use strv_env_get_merged() --- src/libsystemd/sd-json/sd-json.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c index 5e9c1ba15b..293df301a2 100644 --- a/src/libsystemd/sd-json/sd-json.c +++ b/src/libsystemd/sd-json/sd-json.c @@ -11,6 +11,7 @@ #include "alloc-util.h" #include "ansi-color.h" +#include "env-util.h" #include "errno-util.h" #include "escape.h" #include "ether-addr-util.h" @@ -3867,22 +3868,13 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) { l = va_arg(ap, char **); - _cleanup_strv_free_ char **el = NULL; - STRV_FOREACH_PAIR(x, y, l) { - char *n = NULL; + if (current->n_suppress == 0) { + _cleanup_strv_free_ char **el = NULL; - n = strjoin(*x, "=", *y); - if (!n) { - r = -ENOMEM; - goto finish; - } - - r = strv_consume(&el, n); + r = strv_env_get_merged(l, &el); if (r < 0) goto finish; - } - if (current->n_suppress == 0) { r = sd_json_variant_new_array_strv(&add, el); if (r < 0) goto finish; From dbceb0507ffa4745d9d73db2316473055fc98834 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 26 Oct 2024 02:09:01 +0900 Subject: [PATCH 10/12] sd-json: introduce JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY() macro It is similar to JSON_BUILD_PAIR_STRV_NON_EMPTY, but takes the list of environment variables. --- src/libsystemd/sd-json/json-util.h | 2 ++ src/libsystemd/sd-json/sd-json.c | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-json/json-util.h b/src/libsystemd/sd-json/json-util.h index 2aeb076823..81c0fd02a6 100644 --- a/src/libsystemd/sd-json/json-util.h +++ b/src/libsystemd/sd-json/json-util.h @@ -157,6 +157,7 @@ enum { _JSON_BUILD_PAIR_FINITE_USEC, _JSON_BUILD_PAIR_STRING_NON_EMPTY, _JSON_BUILD_PAIR_STRV_NON_EMPTY, + _JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY, _JSON_BUILD_PAIR_VARIANT_NON_NULL, /* _SD_JSON_BUILD_PAIR_VARIANT_ARRAY_NON_EMPTY, */ _JSON_BUILD_PAIR_BYTE_ARRAY_NON_EMPTY, @@ -204,6 +205,7 @@ enum { #define JSON_BUILD_PAIR_FINITE_USEC(name, u) _JSON_BUILD_PAIR_FINITE_USEC, (const char*) { name }, (usec_t) { u } #define JSON_BUILD_PAIR_STRING_NON_EMPTY(name, s) _JSON_BUILD_PAIR_STRING_NON_EMPTY, (const char*) { name }, (const char*) { s } #define JSON_BUILD_PAIR_STRV_NON_EMPTY(name, l) _JSON_BUILD_PAIR_STRV_NON_EMPTY, (const char*) { name }, (char**) { l } +#define JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY(name, l) _JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY, (const char*) { name }, (char**) { l } #define JSON_BUILD_PAIR_VARIANT_NON_NULL(name, v) _JSON_BUILD_PAIR_VARIANT_NON_NULL, (const char*) { name }, (sd_json_variant*) { v } #define JSON_BUILD_PAIR_BYTE_ARRAY_NON_EMPTY(name, v, n) _JSON_BUILD_PAIR_BYTE_ARRAY_NON_EMPTY, (const char*) { name }, (const void*) { v }, (size_t) { n } #define JSON_BUILD_PAIR_IN4_ADDR_NON_NULL(name, v) _JSON_BUILD_PAIR_IN4_ADDR_NON_NULL, (const char*) { name }, (const struct in_addr*) { v } diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c index 293df301a2..f0a9470a17 100644 --- a/src/libsystemd/sd-json/sd-json.c +++ b/src/libsystemd/sd-json/sd-json.c @@ -4533,7 +4533,8 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) { break; } - case _JSON_BUILD_PAIR_STRV_NON_EMPTY: { + case _JSON_BUILD_PAIR_STRV_NON_EMPTY: + case _JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY: { const char *n; char **l; @@ -4546,11 +4547,19 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) { l = va_arg(ap, char **); if (!strv_isempty(l) && current->n_suppress == 0) { + _cleanup_strv_free_ char **el = NULL; + + if (command == _JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY) { + r = strv_env_get_merged(l, &el); + if (r < 0) + goto finish; + } + r = sd_json_variant_new_string(&add, n); if (r < 0) goto finish; - r = sd_json_variant_new_array_strv(&add_more, l); + r = sd_json_variant_new_array_strv(&add_more, el ?: l); if (r < 0) goto finish; } From 0ef4a21b09a4c5a492728c4d0de14d72d26b35d8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 26 Oct 2024 02:17:29 +0900 Subject: [PATCH 11/12] machine: use JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY() Also use JSON_BUILD_PAIR_UNSIGNED_NOT_EQUAL(). --- src/machine/machined-varlink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c index f7cfaba78a..104958d4b8 100644 --- a/src/machine/machined-varlink.c +++ b/src/machine/machined-varlink.c @@ -470,11 +470,11 @@ static int list_machine_one_and_maybe_read_metadata(sd_varlink *link, Machine *m JSON_BUILD_PAIR_STRING_NON_EMPTY("unit", m->unit), SD_JSON_BUILD_PAIR_CONDITION(pidref_is_set(&m->leader), "leader", JSON_BUILD_PIDREF(&m->leader)), SD_JSON_BUILD_PAIR_CONDITION(dual_timestamp_is_set(&m->timestamp), "timestamp", JSON_BUILD_DUAL_TIMESTAMP(&m->timestamp)), - SD_JSON_BUILD_PAIR_CONDITION(m->vsock_cid != VMADDR_CID_ANY, "vSockCid", SD_JSON_BUILD_UNSIGNED(m->vsock_cid)), + JSON_BUILD_PAIR_UNSIGNED_NOT_EQUAL("vSockCid", m->vsock_cid, VMADDR_CID_ANY), JSON_BUILD_PAIR_STRING_NON_EMPTY("sshAddress", m->ssh_address), JSON_BUILD_PAIR_STRING_NON_EMPTY("sshPrivateKeyPath", m->ssh_private_key_path), JSON_BUILD_PAIR_VARIANT_NON_NULL("addresses", addr_array), - SD_JSON_BUILD_PAIR_CONDITION(!strv_isempty(os_release), "OSRelease", JSON_BUILD_STRV_ENV_PAIR(os_release)), + JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY("OSRelease", os_release), JSON_BUILD_PAIR_UNSIGNED_NOT_EQUAL("UIDShift", shift, UID_INVALID)); if (r < 0) return r; @@ -627,8 +627,8 @@ static int list_image_one_and_maybe_read_metadata(sd_varlink *link, Image *image &v, JSON_BUILD_PAIR_STRING_NON_EMPTY("hostname", image->hostname), SD_JSON_BUILD_PAIR_CONDITION(!sd_id128_is_null(image->machine_id), "machineId", SD_JSON_BUILD_ID128(image->machine_id)), - SD_JSON_BUILD_PAIR_CONDITION(!strv_isempty(image->machine_info), "machineInfo", JSON_BUILD_STRV_ENV_PAIR(image->machine_info)), - SD_JSON_BUILD_PAIR_CONDITION(!strv_isempty(image->os_release), "OSRelease", JSON_BUILD_STRV_ENV_PAIR(image->os_release))); + JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY("machineInfo", image->machine_info), + JSON_BUILD_PAIR_STRV_ENV_PAIR_NON_EMPTY("OSRelease", image->os_release)); if (r < 0) return r; } From 76423f301e671780c86e440161c4c7e3b1656f7e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 31 Oct 2024 10:24:06 +0900 Subject: [PATCH 12/12] machine: lookup_machine_by_name_or_pidref() returns negative errno on failure This effectively reverts d2c1451b7398f13439b694450a23def3cf31c8db. After the commit d2ebf5cc1d59e29139f06efaa3a9b2c184cdaa25, sd_varlink_error() returns negative errno, hence the function always return negative errno on failure. --- src/machine/machine-varlink.c | 4 +--- src/machine/machined-varlink.c | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/machine/machine-varlink.c b/src/machine/machine-varlink.c index c9f44f20ab..bfa4095a3b 100644 --- a/src/machine/machine-varlink.c +++ b/src/machine/machine-varlink.c @@ -236,8 +236,6 @@ int lookup_machine_by_name_or_pidref(sd_varlink *link, Manager *manager, const c assert(manager); assert(ret_machine); - /* This returns 0 on success, 1 on error and it is replied, and a negative errno otherwise. */ - if (machine_name) { r = lookup_machine_by_name(link, manager, machine_name, &machine); if (r == -EINVAL) @@ -350,7 +348,7 @@ int vl_method_kill(sd_varlink *link, sd_json_variant *parameters, sd_varlink_met r = lookup_machine_by_name_or_pidref(link, manager, p.name, &p.pidref, &machine); if (r == -ESRCH) return sd_varlink_error(link, "io.systemd.Machine.NoSuchMachine", NULL); - if (r != 0) + if (r < 0) return r; if (isempty(p.swhom)) diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c index 104958d4b8..151d06e5f4 100644 --- a/src/machine/machined-varlink.c +++ b/src/machine/machined-varlink.c @@ -526,7 +526,7 @@ static int vl_method_list(sd_varlink *link, sd_json_variant *parameters, sd_varl r = lookup_machine_by_name_or_pidref(link, m, p.name, &p.pidref, &machine); if (r == -ESRCH) return sd_varlink_error(link, "io.systemd.Machine.NoSuchMachine", NULL); - if (r != 0) + if (r < 0) return r; return list_machine_one_and_maybe_read_metadata(link, machine, /* more = */ false, p.acquire_metadata); @@ -576,7 +576,7 @@ static int lookup_machine_and_call_method(sd_varlink *link, sd_json_variant *par r = lookup_machine_by_name_or_pidref(link, manager, p.name, &p.pidref, &machine); if (r == -ESRCH) return sd_varlink_error(link, "io.systemd.Machine.NoSuchMachine", NULL); - if (r != 0) + if (r < 0) return r; return method(link, parameters, flags, machine);