From 6d36464065601f79a352367cf099be8907d8f9aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 10 Jan 2019 16:23:49 +0100 Subject: [PATCH 1/4] udev,networkd: use the interface name as fallback basis for MAC and IPv4LL seed Fixes #3374. The problem is that we set MACPolicy=persistent (i.e. we would like to generate persistent MAC addresses for interfaces which don't have a fixed MAC address), but various virtual interfaces including bridges, tun/tap, bonds, etc., do not not have the necessary ID_NET_NAME_* attributes and udev would not assing the address and warn: Could not generate persistent MAC address for $name: No such file or directory Basic requirements which I think a solution for this needs to satisfy: 1. No changes to MAC address generation for those cases which are currently handled successfully. This means that net_get_unique_predictable_data() must keep returning the same answer, which in turn means net_get_name() must keep returning the same answer. We can only add more things we look at with lower priority so that we start to cover cases which were not covered before. 2. Like 1, but for IPvLL seed and DHCP IAD. This is less important, but "nice to have". 3. Keep MACPolicy=persistent. If people don't want it, they can always apply local configuration, but in general stable MACs are a good thing. I have never seen anyone complain about that. == Various approaches that have been proposed === https://github.com/systemd/systemd/issues/3374#issuecomment-223753264 (tomty89) if !ID_BUS and INTERFACE, use INTERFACE I think this almost does the good thing, but I don't see the reason to reject ID_BUS (i.e. physical hardware). Stable MACs are very useful for physical hardware that has no physical MAC. === https://github.com/systemd/systemd/issues/3374#issuecomment-224733069 (teg) if (should_rename(device, true)) This means looking at name_assign_type. In particular for NET_NAME_USER should_rename(..., true) returns true. It only returns false for NET_NAME_PREDICTABLE. So this would cover stuff like br0, bond0, etc, but would not cover lo and other devices with predictable names. That doesn't make much sense. But did teg mean should_rename() or !should_rename()? === https://github.com/systemd/systemd/issues/3374#issuecomment-234628502 (tomty89): + if (!should_rename(device, true)) + return udev_device_get_sysname(device) This covers only devices with NET_NAME_PREDICTABLE. Since the problem applies as much to bridges and such, this isn't neough. === https://github.com/systemd/systemd/issues/3374#issuecomment-281745967 (grafi-tt) + /* if the machine doesn't provide data about the device, use the ifname specified by userspace + * (this is the case when the device is virtual, e.g., bridge or bond) */ + s = udev_device_get_sysattr_value(device, "name_assign_type"); + if (s && safe_atou(s, &type) >= 0 && type == NET_NAME_USER) + return udev_device_get_sysname(device); This does not cover bond0, vnet0, tun/tap and similar. grafi-tt also proposes patching the kernel, but *not* setting name_assign_type seems intentional in those cases, because the device name is a result of enumeration, not set by the userspace. === https://github.com/systemd/systemd/issues/3374#issuecomment-288882355 (tomty89) (also PR #11372) - MACAddressPolicy=persistent This break requirement 3. above. It would solve the immediate problem, but I think the disruption is too big. === This patch This patch means that we will set a "stable" MAC for pretty much any virtual device by default, where "stable" means keyed off the machine-id and interface name. It seems like a big change, but we already did this for most physical devices. Doing it also for virtual devices doesn't seem like a big issue. It will make the setup and monitoring of virtualized networks slightly nicer. I don't think anyone is depending on having the MAC address changed when those devices are destoryed and recreated. If they do, they'd have to change MACAddressPolicy=. == Implementation net_get_name() is called from dhcp_ident_set_iaid() so I didn't change net_get_name() like in grafi-tt's patch, but net_get_unique_predictable_data(). net_get_unique_predictable_data() is called from get_mac() in link-config.c and sd_ipv4ll_set_address_seed(), so both of those code paths are affected and will now get data in some cases where they errored out previously. The return code is changed to -ENODATA since that gives a nicer error string. --- src/libsystemd-network/network-internal.c | 19 ++++++++++++------- src/udev/net/link-config.c | 7 +++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c index b3b134d650..9e7838527c 100644 --- a/src/libsystemd-network/network-internal.c +++ b/src/libsystemd-network/network-internal.c @@ -10,6 +10,7 @@ #include "alloc-util.h" #include "condition.h" #include "conf-parser.h" +#include "device-util.h" #include "dhcp-lease-internal.h" #include "ether-addr-util.h" #include "hexdecoct.h" @@ -40,31 +41,35 @@ const char *net_get_name(sd_device *device) { int net_get_unique_predictable_data(sd_device *device, uint64_t *result) { size_t l, sz = 0; - const char *name = NULL; + const char *name; int r; uint8_t *v; assert(device); + /* net_get_name() will return one of the device names based on stable information about the + * device. If this is not available, we fall back to using the device name. */ name = net_get_name(device); if (!name) - return -ENOENT; + (void) sd_device_get_sysname(device, &name); + if (!name) + return log_device_debug_errno(device, SYNTHETIC_ERRNO(ENODATA), + "No stable identifying information found"); + log_device_debug(device, "Using \"%s\" as stable identifying information", name); l = strlen(name); sz = sizeof(sd_id128_t) + l; v = alloca(sz); - /* fetch some persistent data unique to this machine */ + /* Fetch some persistent data unique to this machine */ r = sd_id128_get_machine((sd_id128_t*) v); if (r < 0) return r; memcpy(v + sizeof(sd_id128_t), name, l); - /* Let's hash the machine ID plus the device name. We - * use a fixed, but originally randomly created hash - * key here. */ + /* Let's hash the machine ID plus the device name. We use + * a fixed, but originally randomly created hash key here. */ *result = htole64(siphash24(v, sz, HASH_KEY.bytes)); - return 0; } diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index eb2477cea4..0fda4d16b9 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -313,8 +313,7 @@ static bool mac_is_random(sd_device *device) { return type == NET_ADDR_RANDOM; } -static int get_mac(sd_device *device, bool want_random, - struct ether_addr *mac) { +static int get_mac(sd_device *device, bool want_random, struct ether_addr *mac) { int r; if (want_random) @@ -459,7 +458,7 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, case MACPOLICY_PERSISTENT: if (mac_is_random(device)) { r = get_mac(device, false, &generated_mac); - if (r == -ENOENT) { + if (r == -ENODATA) { log_warning_errno(r, "Could not generate persistent MAC address for %s: %m", old_name); break; } else if (r < 0) @@ -470,7 +469,7 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, case MACPOLICY_RANDOM: if (!mac_is_random(device)) { r = get_mac(device, true, &generated_mac); - if (r == -ENOENT) { + if (r == -ENODATA) { log_warning_errno(r, "Could not generate random MAC address for %s: %m", old_name); break; } else if (r < 0) From 25ec18c4d8f71cb3977a5766d1aa247d03197e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jan 2019 16:41:49 +0100 Subject: [PATCH 2/4] basic/missing: add more addr_assign_type values --- src/basic/missing_network.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/basic/missing_network.h b/src/basic/missing_network.h index 59a8cd2c60..80ef13fd3e 100644 --- a/src/basic/missing_network.h +++ b/src/basic/missing_network.h @@ -98,10 +98,22 @@ #endif /* netdevice.h */ +#ifndef NET_ADDR_PERM +#define NET_ADDR_PERM 0 +#endif + #ifndef NET_ADDR_RANDOM #define NET_ADDR_RANDOM 1 #endif +#ifndef NET_ADDR_STOLEN +#define NET_ADDR_STOLEN 2 +#endif + +#ifndef NET_ADDR_SET +#define NET_ADDR_SET 3 +#endif + #ifndef NET_NAME_UNKNOWN #define NET_NAME_UNKNOWN 0 #endif From 015b097cce608df59f862f2202d128704abc1724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 19 Jan 2019 15:19:03 +0100 Subject: [PATCH 3/4] udev: add debug logging about the choice of MAC --- src/udev/net/link-config.c | 93 ++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 55 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 0fda4d16b9..60da08a86f 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -187,19 +187,19 @@ static bool enable_name_policy(void) { return proc_cmdline_get_bool("net.ifnames", &b) <= 0 || b; } -static int link_name_type(sd_device *device, unsigned *type) { +static int link_unsigned_attribute(sd_device *device, const char *attr, unsigned *type) { const char *s; int r; - r = sd_device_get_sysattr_value(device, "name_assign_type", &s); + r = sd_device_get_sysattr_value(device, attr, &s); if (r < 0) - return log_device_debug_errno(device, r, "Failed to query name_assign_type: %m"); + return log_device_debug_errno(device, r, "Failed to query %s: %m", attr); r = safe_atou(s, type); if (r < 0) - return log_device_warning_errno(device, r, "Failed to parse name_assign_type \"%s\": %m", s); + return log_device_warning_errno(device, r, "Failed to parse %s \"%s\": %m", attr, s); - log_device_debug(device, "Device has name_assign_type=%d", *type); + log_device_debug(device, "Device has %s=%u", attr, *type); return 0; } @@ -265,11 +265,9 @@ int link_config_get(link_config_ctx *ctx, sd_device *device, link_config **ret) devtype, sysname)) { if (link->match_name) { - unsigned char name_assign_type = NET_NAME_UNKNOWN; - const char *attr_value; + unsigned name_assign_type = NET_NAME_UNKNOWN; - if (sd_device_get_sysattr_value(device, "name_assign_type", &attr_value) >= 0) - (void) safe_atou8(attr_value, &name_assign_type); + (void) link_unsigned_attribute(device, "name_assign_type", &name_assign_type); if (name_assign_type == NET_NAME_ENUM) { log_warning("Config file %s applies to device based on potentially unpredictable interface name '%s'", @@ -297,33 +295,41 @@ int link_config_get(link_config_ctx *ctx, sd_device *device, link_config **ret) return -ENOENT; } -static bool mac_is_random(sd_device *device) { - const char *s; - unsigned type; +static int get_mac(sd_device *device, MACPolicy policy, struct ether_addr *mac) { + unsigned addr_type; + bool want_random = policy == MACPOLICY_RANDOM; int r; - /* if we can't get the assign type, assume it is not random */ - if (sd_device_get_sysattr_value(device, "addr_assign_type", &s) < 0) - return false; + assert(IN_SET(policy, MACPOLICY_RANDOM, MACPOLICY_PERSISTENT)); - r = safe_atou(s, &type); + r = link_unsigned_attribute(device, "addr_assign_type", &addr_type); if (r < 0) - return false; + return r; + switch (addr_type) { + case NET_ADDR_SET: + return log_device_debug(device, "MAC on the device already set by userspace"); + case NET_ADDR_STOLEN: + return log_device_debug(device, "MAC on the device already set based on another device"); + case NET_ADDR_RANDOM: + case NET_ADDR_PERM: + break; + default: + return log_device_warning(device, "Unknown addr_assign_type %u, ignoring", addr_type); + } - return type == NET_ADDR_RANDOM; -} + if (want_random == (addr_type == NET_ADDR_RANDOM)) + return log_device_debug(device, "MAC on the device already matches policy *%s*", + mac_policy_to_string(policy)); -static int get_mac(sd_device *device, bool want_random, struct ether_addr *mac) { - int r; - - if (want_random) + if (want_random) { + log_device_debug(device, "Using random bytes to generate MAC"); random_bytes(mac->ether_addr_octet, ETH_ALEN); - else { + } else { uint64_t result; r = net_get_unique_predictable_data(device, &result); if (r < 0) - return r; + return log_device_warning_errno(device, r, "Could not generate persistent MAC: %m"); assert_cc(ETH_ALEN <= sizeof(result)); memcpy(mac->ether_addr_octet, &result, ETH_ALEN); @@ -332,8 +338,7 @@ static int get_mac(sd_device *device, bool want_random, struct ether_addr *mac) /* see eth_random_addr in the kernel */ mac->ether_addr_octet[0] &= 0xfe; /* clear multicast bit */ mac->ether_addr_octet[0] |= 0x02; /* set local assignment bit (IEEE802) */ - - return 0; + return 1; } int link_config_apply(link_config_ctx *ctx, link_config *config, @@ -397,7 +402,7 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, return log_device_warning_errno(device, r, "Could not find ifindex: %m"); - (void) link_name_type(device, &name_type); + (void) link_unsigned_attribute(device, "name_assign_type", &name_type); if (IN_SET(name_type, NET_NAME_USER, NET_NAME_RENAMED) && !naming_scheme_has(NAMING_ALLOW_RERENAMES)) { @@ -454,33 +459,11 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, log_device_debug(device, "Policies didn't yield a name and Name= is not given, not renaming."); no_rename: - switch (config->mac_policy) { - case MACPOLICY_PERSISTENT: - if (mac_is_random(device)) { - r = get_mac(device, false, &generated_mac); - if (r == -ENODATA) { - log_warning_errno(r, "Could not generate persistent MAC address for %s: %m", old_name); - break; - } else if (r < 0) - return r; - mac = &generated_mac; - } - break; - case MACPOLICY_RANDOM: - if (!mac_is_random(device)) { - r = get_mac(device, true, &generated_mac); - if (r == -ENODATA) { - log_warning_errno(r, "Could not generate random MAC address for %s: %m", old_name); - break; - } else if (r < 0) - return r; - mac = &generated_mac; - } - break; - case MACPOLICY_NONE: - default: - mac = config->mac; - } + if (IN_SET(config->mac_policy, MACPOLICY_PERSISTENT, MACPOLICY_RANDOM)) { + if (get_mac(device, config->mac_policy, &generated_mac) > 0) + mac = &generated_mac; + } else + mac = config->mac; r = rtnl_set_link_properties(&ctx->rtnl, ifindex, config->alias, mac, config->mtu); if (r < 0) From a2b818edffe464bcdcc02ec6f958cfeb449f1a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jan 2019 09:58:51 +0100 Subject: [PATCH 4/4] test-libudev: modernize and add more debugging info --- src/test/test-libudev.c | 124 ++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 49 deletions(-) diff --git a/src/test/test-libudev.c b/src/test/test-libudev.c index 15c0f8853d..f634ca28db 100644 --- a/src/test/test-libudev.c +++ b/src/test/test-libudev.c @@ -12,8 +12,12 @@ #include "libudev-list-internal.h" #include "libudev-util.h" #include "log.h" +#include "main-func.h" #include "stdio-util.h" #include "string-util.h" +#include "tests.h" + +static bool arg_monitor = false; static void print_device(struct udev_device *device) { const char *str; @@ -23,7 +27,7 @@ static void print_device(struct udev_device *device) { log_info("*** device: %p ***", device); str = udev_device_get_action(device); - if (str != NULL) + if (str) log_info("action: '%s'", str); str = udev_device_get_syspath(device); @@ -33,26 +37,26 @@ static void print_device(struct udev_device *device) { log_info("sysname: '%s'", str); str = udev_device_get_sysnum(device); - if (str != NULL) + if (str) log_info("sysnum: '%s'", str); str = udev_device_get_devpath(device); log_info("devpath: '%s'", str); str = udev_device_get_subsystem(device); - if (str != NULL) + if (str) log_info("subsystem: '%s'", str); str = udev_device_get_devtype(device); - if (str != NULL) + if (str) log_info("devtype: '%s'", str); str = udev_device_get_driver(device); - if (str != NULL) + if (str) log_info("driver: '%s'", str); str = udev_device_get_devnode(device); - if (str != NULL) + if (str) log_info("devname: '%s'", str); devnum = udev_device_get_devnum(device); @@ -78,30 +82,30 @@ static void print_device(struct udev_device *device) { log_info("found %i properties", count); str = udev_device_get_property_value(device, "MAJOR"); - if (str != NULL) + if (str) log_info("MAJOR: '%s'", str); str = udev_device_get_sysattr_value(device, "dev"); - if (str != NULL) + if (str) log_info("attr{dev}: '%s'", str); } static void test_device(struct udev *udev, const char *syspath) { _cleanup_(udev_device_unrefp) struct udev_device *device; - log_info("looking at device: %s", syspath); + log_info("/* %s, device %s */", __func__, syspath); device = udev_device_new_from_syspath(udev, syspath); - if (device == NULL) - log_warning_errno(errno, "udev_device_new_from_syspath: %m"); - else + if (device) print_device(device); + else + log_warning_errno(errno, "udev_device_new_from_syspath: %m"); } static void test_device_parents(struct udev *udev, const char *syspath) { _cleanup_(udev_device_unrefp) struct udev_device *device; struct udev_device *device_parent; - log_info("looking at device: %s", syspath); + log_info("/* %s, device %s */", __func__, syspath); device = udev_device_new_from_syspath(udev, syspath); if (device == NULL) return; @@ -125,12 +129,13 @@ static void test_device_devnum(struct udev *udev) { dev_t devnum = makedev(1, 3); _cleanup_(udev_device_unrefp) struct udev_device *device; - log_info("looking up device: %u:%u", major(devnum), minor(devnum)); + log_info("/* %s, device %d:%d */", __func__, major(devnum), minor(devnum)); + device = udev_device_new_from_devnum(udev, 'c', devnum); - if (device == NULL) - log_warning_errno(errno, "udev_device_new_from_devnum: %m"); - else + if (device) print_device(device); + else + log_warning_errno(errno, "udev_device_new_from_devnum: %m"); } static void test_device_subsys_name(struct udev *udev, const char *subsys, const char *dev) { @@ -144,7 +149,7 @@ static void test_device_subsys_name(struct udev *udev, const char *subsys, const print_device(device); } -static int test_enumerate_print_list(struct udev_enumerate *enumerate) { +static int enumerate_print_list(struct udev_enumerate *enumerate) { struct udev_list_entry *list_entry; int count = 0; @@ -176,6 +181,8 @@ static void test_monitor(struct udev *udev) { .data.fd = STDIN_FILENO, }; + log_info("/* %s */", __func__); + fd_ep = epoll_create1(EPOLL_CLOEXEC); assert_se(fd_ep >= 0); @@ -225,8 +232,9 @@ static void test_queue(struct udev *udev) { struct udev_queue *udev_queue; bool empty; - udev_queue = udev_queue_new(udev); - assert_se(udev_queue); + log_info("/* %s */", __func__); + + assert_se(udev_queue = udev_queue_new(udev)); empty = udev_queue_get_queue_is_empty(udev_queue); log_info("queue is %s", empty ? "empty" : "not empty"); @@ -237,13 +245,15 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { struct udev_enumerate *udev_enumerate; int r; + log_info("/* %s */", __func__); + log_info("enumerate '%s'", subsystem == NULL ? "" : subsystem); udev_enumerate = udev_enumerate_new(udev); if (udev_enumerate == NULL) return -1; udev_enumerate_add_match_subsystem(udev_enumerate, subsystem); udev_enumerate_scan_devices(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); log_info("enumerate 'net' + duplicated scan + null + zero"); @@ -263,7 +273,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { udev_enumerate_add_syspath(udev_enumerate, "/sys/class/mem/zero"); udev_enumerate_add_syspath(udev_enumerate, "/sys/class/mem/zero"); udev_enumerate_scan_devices(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); log_info("enumerate 'block'"); @@ -277,7 +287,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { return r; } udev_enumerate_scan_devices(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); log_info("enumerate 'not block'"); @@ -286,7 +296,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { return -1; udev_enumerate_add_nomatch_subsystem(udev_enumerate, "block"); udev_enumerate_scan_devices(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); log_info("enumerate 'pci, mem, vc'"); @@ -297,7 +307,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { udev_enumerate_add_match_subsystem(udev_enumerate, "mem"); udev_enumerate_add_match_subsystem(udev_enumerate, "vc"); udev_enumerate_scan_devices(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); log_info("enumerate 'subsystem'"); @@ -305,7 +315,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { if (udev_enumerate == NULL) return -1; udev_enumerate_scan_subsystems(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); log_info("enumerate 'property IF_FS_*=filesystem'"); @@ -314,7 +324,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { return -1; udev_enumerate_add_match_property(udev_enumerate, "ID_FS*", "filesystem"); udev_enumerate_scan_devices(udev_enumerate); - test_enumerate_print_list(udev_enumerate); + enumerate_print_list(udev_enumerate); udev_enumerate_unref(udev_enumerate); return 0; } @@ -323,7 +333,11 @@ static void test_hwdb(struct udev *udev, const char *modalias) { struct udev_hwdb *hwdb; struct udev_list_entry *entry; + log_info("/* %s */", __func__); + hwdb = udev_hwdb_new(udev); + if (!hwdb) + log_warning_errno(errno, "Failed to open hwdb: %m"); udev_list_entry_foreach(entry, udev_hwdb_get_properties_list_entry(hwdb, modalias, 0)) log_info("'%s'='%s'", udev_list_entry_get_name(entry), udev_list_entry_get_value(entry)); @@ -348,6 +362,8 @@ static void test_util_replace_whitespace_one(const char *str, const char *expect } static void test_util_replace_whitespace(void) { + log_info("/* %s */", __func__); + test_util_replace_whitespace_one("hogehoge", "hogehoge"); test_util_replace_whitespace_one("hoge hoge", "hoge_hoge"); test_util_replace_whitespace_one(" hoge hoge ", "hoge_hoge"); @@ -385,16 +401,19 @@ static void test_util_replace_whitespace(void) { } static void test_util_resolve_subsys_kernel_one(const char *str, bool read_value, int retval, const char *expected) { - char result[UTIL_PATH_SIZE]; + char result[UTIL_PATH_SIZE] = ""; int r; r = util_resolve_subsys_kernel(str, result, sizeof(result), read_value); + log_info("\"%s\" → expect: \"%s\", %d, actual: \"%s\", %d", str, strnull(expected), retval, result, r); assert_se(r == retval); if (r >= 0) assert_se(streq(result, expected)); } static void test_util_resolve_subsys_kernel(void) { + log_info("/* %s */", __func__); + test_util_resolve_subsys_kernel_one("hoge", false, -EINVAL, NULL); test_util_resolve_subsys_kernel_one("[hoge", false, -EINVAL, NULL); test_util_resolve_subsys_kernel_one("[hoge/foo", false, -EINVAL, NULL); @@ -471,9 +490,7 @@ static void test_list(void) { udev_list_cleanup(&list); } -int main(int argc, char *argv[]) { - _cleanup_(udev_unrefp) struct udev *udev = NULL; - bool arg_monitor = false; +static int parse_args(int argc, char *argv[], const char **syspath, const char **subsystem) { static const struct option options[] = { { "syspath", required_argument, NULL, 'p' }, { "subsystem", required_argument, NULL, 's' }, @@ -483,52 +500,59 @@ int main(int argc, char *argv[]) { { "monitor", no_argument, NULL, 'm' }, {} }; - const char *syspath = "/devices/virtual/mem/null"; - const char *subsystem = NULL; int c; - udev = udev_new(); - log_info("context: %p", udev); - if (udev == NULL) { - log_info("no context"); - return 1; - } - while ((c = getopt_long(argc, argv, "p:s:dhVm", options, NULL)) >= 0) switch (c) { - case 'p': - syspath = optarg; + *syspath = optarg; break; case 's': - subsystem = optarg; + *subsystem = optarg; break; case 'd': - if (log_get_max_level() < LOG_INFO) - log_set_max_level(LOG_INFO); + log_set_max_level(LOG_DEBUG); break; case 'h': printf("--debug --syspath= --subsystem= --help\n"); - return EXIT_SUCCESS; + return 0; case 'V': printf("%s\n", GIT_VERSION); - return EXIT_SUCCESS; + return 0; case 'm': arg_monitor = true; break; case '?': - return EXIT_FAILURE; + return -EINVAL; default: assert_not_reached("Unhandled option code."); } + return 1; +} + +static int run(int argc, char *argv[]) { + _cleanup_(udev_unrefp) struct udev *udev = NULL; + + const char *syspath = "/devices/virtual/mem/null"; + const char *subsystem = NULL; + int r; + + test_setup_logging(LOG_INFO); + + r = parse_args(argc, argv, &syspath, &subsystem); + if (r <= 0) + return r; + + assert_se(udev = udev_new()); + /* add sys path if needed */ if (!startswith(syspath, "/sys")) syspath = strjoina("/sys/", syspath); @@ -555,5 +579,7 @@ int main(int argc, char *argv[]) { test_list(); - return EXIT_SUCCESS; + return 0; } + +DEFINE_MAIN_FUNCTION(run);