Re: [systemd-devel] [RFC][PATCH] udev: net_id - support multi-function, multi-port enpo* device names
Found one of these fabaled multi-function network devices you dropped from the patch, an Intel I350 Gigabit device on a Supermicro X9DRI-LN4F+ motherboard. The 4 different network interfaces are all are fighting over the 'eno1' name and are functions 06:00.0, 06:00.1, 06:00.2, and 06:00.3. On Wed, Apr 1, 2015 at 1:58 PM, Tom Gundersen t...@jklm.no wrote: I pushed a version of this only handling the multi-port devices. We can deal with multi-function if and when they appear in the wild. -t On Wed, Apr 1, 2015 at 4:52 PM, Tom Gundersen t...@jklm.no wrote: I'd argue that having firmware labels for such devices makes no sense, but they exist, so make sure we handle them as best as we can. --- src/udev/udev-builtin-net_id.c | 64 -- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 71f3a59..1a72190 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -35,7 +35,7 @@ * Type of names: * bnumber -- BCMA bus core number * ccwname -- CCW bus group name - * oindex -- on-board device index number + * oindex[ffunction][ddev_port]-- on-board device index number * sslot[ffunction][ddev_port] -- hotplug slot index number * xMAC-- MAC address * [Pdomain]pbussslot[ffunction][ddev_port] @@ -126,11 +126,38 @@ struct netnames { char ccw_group[IFNAMSIZ]; }; +/* read the 256 bytes PCI configuration space to check the multi-function bit */ +static bool is_pci_multifunction(struct udev_device *dev) { +_cleanup_fclose_ FILE *f = NULL; +const char *filename; +uint8_t config[64]; + +filename = strjoina(udev_device_get_syspath(dev), /config); +f = fopen(filename, re); +if (!f) +return false; +if (fread(config, sizeof(config), 1, f) != 1) +return false; + +/* bit 0-6 header type, bit 7 multi/single function device */ +if ((config[PCI_HEADER_TYPE] 0x80) != 0) +return true; + +return false; +} + /* retrieve on-board index number and label from firmware */ static int dev_pci_onboard(struct udev_device *dev, struct netnames *names) { +unsigned func, dev_port = 0; +size_t l; +char *s; +const char *attr; const char *index; int idx; +if (sscanf(udev_device_get_sysname(names-pcidev), %*x:%*x:%*x.%u, func) != 1) +return -ENOENT; + /* ACPI _DSM -- device specific method for naming a PCI or PCI Express device */ index = udev_device_get_sysattr_value(names-pcidev, acpi_index); /* SMBIOS type 41 -- Onboard Devices Extended Information */ @@ -141,30 +168,25 @@ static int dev_pci_onboard(struct udev_device *dev, struct netnames *names) { idx = strtoul(index, NULL, 0); if (idx = 0) return -EINVAL; -snprintf(names-pci_onboard, sizeof(names-pci_onboard), o%d, idx); -names-pci_onboard_label = udev_device_get_sysattr_value(names-pcidev, label); -return 0; -} - -/* read the 256 bytes PCI configuration space to check the multi-function bit */ -static bool is_pci_multifunction(struct udev_device *dev) { -_cleanup_fclose_ FILE *f = NULL; -const char *filename; -uint8_t config[64]; +/* kernel provided multi-device index */ +attr = udev_device_get_sysattr_value(dev, dev_port); +if (attr) +dev_port = strtol(attr, NULL, 10); -filename = strjoina(udev_device_get_syspath(dev), /config); -f = fopen(filename, re); -if (!f) -return false; -if (fread(config, sizeof(config), 1, f) != 1) -return false; +s = names-pci_onboard; +l = sizeof(names-pci_onboard); +l = strpcpyf(s, l, o%d, idx); +if (func 0 || is_pci_multifunction(names-pcidev)) +l = strpcpyf(s, l, f%d, func); +if (dev_port 0) +l = strpcpyf(s, l, d%d, dev_port); +if (l == 0) +names-pci_onboard[0] = '\0'; -/* bit 0-6 header type, bit 7 multi/single function device */ -if ((config[PCI_HEADER_TYPE] 0x80) != 0) -return true; +names-pci_onboard_label = udev_device_get_sysattr_value(names-pcidev, label); -return false; +return 0; } static int dev_pci_slot(struct udev_device *dev, struct netnames *names) { -- 2.3.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [ANNOUNCE] Git development moved to github
On Tue, Jun 2, 2015 at 6:31 AM, Daniel Mack dan...@zonque.org wrote: On 06/02/2015 02:19 PM, Jason A. Donenfeld wrote: On Tue, Jun 2, 2015 at 1:06 PM, David Herrmann dh.herrm...@gmail.com wrote: Regarding the final github address: David Strauss kindly offered the 'systemd' user to us. Hence, we hope to move the repository to github.com/systemd/systemd this week. Sorry for the confusion, I hope we can settle all this this week. I recommend you get this sorted out as soon as possible and not wait another moment. People have already submitted pull requests to both repos, and things are going to get quite confusing if you don't move fast on this. It's sorted out now. https://github.com/systemd/systemd is now the official upstream. The old repo from systemd-devs was transferred withing GitHub, which means that the old web and ssh URLs are currently redirected automatically. However, we will remove the systemd-devs organization any time soon to avoid further confusion. As an FYI for everyone who previously forked the mirror repo that previously lived at https://github.com/systemd/systemd, since that repo was deleted and replaced the github fork network of repos that descended from it is now on its own and random other repos are now listed as the forked from repo, in my case my parent repo is listed as https://github.com/terencehonles/systemd now. Since it is not possible to make pull requests between fork networks in order to submit PRs to the new systemd repo old repos have to be deleted and re-forked. As far as I know there isn't a nicer way to fix it. :( ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] networkd: fix systemd-networkd-wait-online with multiple NICs
On May 20, 2015 9:48 AM, Tom Gundersen t...@jklm.no wrote: On Tue, Apr 21, 2015 at 11:59 PM, Nick Owens misch...@offblast.org wrote: hello tom, On Mon, Apr 20, 2015 at 2:32 PM, Tom Gundersen t...@jklm.no wrote: On Fri, Apr 3, 2015 at 12:48 AM, Michael Marineau michael.marin...@coreos.com wrote: On Thu, Apr 2, 2015 at 3:08 PM, Nick Owens misch...@offblast.org wrote: hi, sorry for the delay. from http://www.freedesktop.org/software/systemd/man/systemd-networkd-wait-online.service.html : By default, it will wait for all links it is aware of and which are managed by systemd-networkd.service(8) to be fully configured or failed, *and for at least one link to gain a carrier.*. the import part here is the end of the sentence. without this patch, systemd-networkd-wait-online will block until all configured interfaces have carrier.. you can reproduce this by running systemd-networkd in qemu with two ethernet interfaces, and issue 'info network' and then 'set_link if down' to simulate no carrier. then you can run systemd-networkd-wait-online, and observe that it will block until both interfaces are up, not just one. nick On Wed, Mar 25, 2015 at 10:53 PM, Andrei Borzenkov arvidj...@gmail.com wrote: On Wed, Mar 25, 2015 at 11:49 PM, misch...@offblast.org wrote: From: mischief misch...@offblast.org when checking interface status, systemd-networkd-wait-online will continue to wait if any interface is still configuring or being processed by udev. this patch allows it to return if any one interface is degraded/routable, as per the manual. But current behavior is exactly what manual says: By default, it will wait for all links it is aware of and which are managed by systemd-networkd.service(8) to be fully configured or failed. Or do I miss something? It is worth noting that there may be some issues with tracking interface states in networkd, there appear to be ways to get an interface stuck in a 'configuring' state despite the fact that the interface has no network config and/or has no carrier. Do you have any more info on this? Can you reproduce with current git? There was a fix after the last release which should fix a problem with enumerating devices. the original issue was discussed at https://github.com/coreos/bugs/issues/279. i just tested commit cffacc741cb79f63999720525ceaa65aae01a542. https://github.com/coreos/init/blob/master/systemd/network/zz-default.network is our default for networkd. it seems logical that given this config, systemd-networkd-wait-online would wait for all of the dhcp interfaces, no matter how many. however, i'm not sure what use case there is for this. it seems like there's no way to wait for any one nic to be routeable/configured without knowing its name ahead of time. Correct. I mean, waiting for the system coming online like this is mostly a legacy thing, so we support this in a relatively limited way. Anything modern better explicitly listen for rtnl events and act accordingly. another instance of this problem is having a bridge like [NetDev] Name=br0 Kind=bridge and run 'systemctl restart systemd-networkd; /usr/bin/systemd-networkd-wait-online'. systemd-networkd-wait-online will not return. is this intended behavior? Hm, I'm not able to reproduce this. Can you still reproduce with git HEAD? If so what are the other config files that are applied, and what is the output of networkctl whilst wait-online is hanging? I haven't retested HEAD yet but up through 219 it would report 'no-carrier configuring' which seems bogus since it shouldn't be configuring an interface in such a state and there is no .network config to apply to the interface either. We have seen similar looking networkctl output for physical interfaces too but since several different states get squashed into 'configuring' I'm not sure if it is the same situation, it was just easier to demo with a .netdev and bridge. Interestingly no other mechanism for creating a bridge (ip or brctl) got it into the same state but I'm not sure why. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] networkd: do not change kernel forwarding parameters when IPForwarding is unset
On Fri, May 15, 2015 at 12:18 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 15.05.15 12:08, Nick Owens (nick.ow...@coreos.com) wrote: In 5a8bcb674f71a20e95df55319b34c556638378ce, IPForwarding was introduced to set forwarding flags on interfaces in .network files. networkd sets forwarding options regardless of the previous setting, even if it was set by e.g. sysctl. This commit makes IPForwarding not change forwarding settings, so that systems using sysctl continue to work even if IPForwarding is unset in their .network files. See https://bugs.freedesktop.org/show_bug.cgi?id=89509 for the initial bug report. I think there should be an explicit way to enable the kernel default mode, i.e. the parser for this one option should consider a special value kernel or so to explicitly ask for the kernel default. I'd still prefer if we'd default to ip forwarding off, rather than ip forwarding as kernel default, for security reasons. Well, in CoreOS we *have* to use the kernel default if the value is unset, there simply is no way to safely upgrade existing systems to the new configuration scheme from the old sysctl one. The semantics of the two are too different. Even if there was a reasonable translation we are not in the business of modifying user configs. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] networkd: do not change kernel forwarding parameters when IPForwarding is unset
On Fri, May 15, 2015 at 12:52 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 15.05.15 12:42, Michael Marineau (michael.marin...@coreos.com) wrote: On Fri, May 15, 2015 at 12:18 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 15.05.15 12:08, Nick Owens (nick.ow...@coreos.com) wrote: In 5a8bcb674f71a20e95df55319b34c556638378ce, IPForwarding was introduced to set forwarding flags on interfaces in .network files. networkd sets forwarding options regardless of the previous setting, even if it was set by e.g. sysctl. This commit makes IPForwarding not change forwarding settings, so that systems using sysctl continue to work even if IPForwarding is unset in their .network files. See https://bugs.freedesktop.org/show_bug.cgi?id=89509 for the initial bug report. I think there should be an explicit way to enable the kernel default mode, i.e. the parser for this one option should consider a special value kernel or so to explicitly ask for the kernel default. I'd still prefer if we'd default to ip forwarding off, rather than ip forwarding as kernel default, for security reasons. Well, in CoreOS we *have* to use the kernel default if the value is unset, there simply is no way to safely upgrade existing systems to the new configuration scheme from the old sysctl one. The semantics of the two are too different. Even if there was a reasonable translation we are not in the business of modifying user configs. Well, but I think I would prefer if upstream would default to off, even if coreos then deviates from that and defaults to kernel... Fair enough, should it be a option to configure then? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] networkd: do not change kernel forwarding parameters when IPForwarding is unset
(build time option to ./configure that is) On Fri, May 15, 2015 at 12:55 PM, Michael Marineau michael.marin...@coreos.com wrote: On Fri, May 15, 2015 at 12:52 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 15.05.15 12:42, Michael Marineau (michael.marin...@coreos.com) wrote: On Fri, May 15, 2015 at 12:18 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 15.05.15 12:08, Nick Owens (nick.ow...@coreos.com) wrote: In 5a8bcb674f71a20e95df55319b34c556638378ce, IPForwarding was introduced to set forwarding flags on interfaces in .network files. networkd sets forwarding options regardless of the previous setting, even if it was set by e.g. sysctl. This commit makes IPForwarding not change forwarding settings, so that systems using sysctl continue to work even if IPForwarding is unset in their .network files. See https://bugs.freedesktop.org/show_bug.cgi?id=89509 for the initial bug report. I think there should be an explicit way to enable the kernel default mode, i.e. the parser for this one option should consider a special value kernel or so to explicitly ask for the kernel default. I'd still prefer if we'd default to ip forwarding off, rather than ip forwarding as kernel default, for security reasons. Well, in CoreOS we *have* to use the kernel default if the value is unset, there simply is no way to safely upgrade existing systems to the new configuration scheme from the old sysctl one. The semantics of the two are too different. Even if there was a reasonable translation we are not in the business of modifying user configs. Well, but I think I would prefer if upstream would default to off, even if coreos then deviates from that and defaults to kernel... Fair enough, should it be a option to configure then? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] networkd: do not change kernel forwarding parameters when IPForwarding is unset
On Fri, May 15, 2015 at 1:49 PM, Tom Gundersen t...@jklm.no wrote: On Fri, May 15, 2015 at 10:02 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 15.05.15 12:56, Michael Marineau (michael.marin...@coreos.com) wrote: (build time option to ./configure that is) I guess I'd be OK with that... It would be a shame if we started diverging on the defaults I think. Would be nice if we could come up with some scheme that would work for everyone. Would an option be to use a script to append IPForward='kernel' to your network files on upgrades? Pretty dirty, but I don't know how you usually deal with config changes... So far we don't do anything to modify user configs and try to ensure we maintain compatibility. Since there are a limited number of things in the CoreOS base image this usually isn't a problem. In the past we have made incompatible changes, the biggest in order to follow upstream docker[1] but it was well advertised in the docker community and impacted users would have a pretty easily googlable situation. In this case if we fail to migrate a user properly networking in a container is going to silently stop and it won't be immediately obvious why. [1]: https://coreos.com/blog/docker-1-3-2-stable-channel/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] networkd: fix systemd-networkd-wait-online with multiple NICs
On Thu, Apr 2, 2015 at 3:08 PM, Nick Owens misch...@offblast.org wrote: hi, sorry for the delay. from http://www.freedesktop.org/software/systemd/man/systemd-networkd-wait-online.service.html: By default, it will wait for all links it is aware of and which are managed by systemd-networkd.service(8) to be fully configured or failed, *and for at least one link to gain a carrier.*. the import part here is the end of the sentence. without this patch, systemd-networkd-wait-online will block until all configured interfaces have carrier.. you can reproduce this by running systemd-networkd in qemu with two ethernet interfaces, and issue 'info network' and then 'set_link if down' to simulate no carrier. then you can run systemd-networkd-wait-online, and observe that it will block until both interfaces are up, not just one. nick On Wed, Mar 25, 2015 at 10:53 PM, Andrei Borzenkov arvidj...@gmail.com wrote: On Wed, Mar 25, 2015 at 11:49 PM, misch...@offblast.org wrote: From: mischief misch...@offblast.org when checking interface status, systemd-networkd-wait-online will continue to wait if any interface is still configuring or being processed by udev. this patch allows it to return if any one interface is degraded/routable, as per the manual. But current behavior is exactly what manual says: By default, it will wait for all links it is aware of and which are managed by systemd-networkd.service(8) to be fully configured or failed. Or do I miss something? It is worth noting that there may be some issues with tracking interface states in networkd, there appear to be ways to get an interface stuck in a 'configuring' state despite the fact that the interface has no network config and/or has no carrier. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] network: add UseNTP DHCP option
Despite having the internal logic in place to enable/disable using NTP servers provided by DHCP the network config didn't expose the option. --- man/systemd.network.xml | 8 src/network/networkd-network-gperf.gperf | 1 + 2 files changed, 9 insertions(+) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 5d7518b..95be132 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -515,6 +515,14 @@ /listitem /varlistentry varlistentry + termvarnameUseNTP=/varname/term + listitem +paraWhen true (the default), the NTP servers received +from the DHCP server will be used by systemd-timesyncd +and take precedence over any statically configured ones./para + /listitem +/varlistentry +varlistentry termvarnameUseMTU=/varname/term listitem paraWhen true, the interface maximum transmission unit diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 93df83a..8abf5bc 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -60,6 +60,7 @@ Route.Metric,config_parse_route_priority, 0, Route.Scope, config_parse_route_scope, 0, 0 DHCP.ClientIdentifier, config_parse_dhcp_client_identifier,0, offsetof(Network, dhcp_client_identifier) DHCP.UseDNS, config_parse_bool, 0, offsetof(Network, dhcp_dns) +DHCP.UseNTP, config_parse_bool, 0, offsetof(Network, dhcp_ntp) DHCP.UseMTU, config_parse_bool, 0, offsetof(Network, dhcp_mtu) DHCP.UseHostname,config_parse_bool, 0, offsetof(Network, dhcp_hostname) DHCP.UseDomains, config_parse_bool, 0, offsetof(Network, dhcp_domains) -- 2.0.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Enabling timesyncd in virtual machines
Greetings, Currently systemd-timesyncd.service includes ConditionVirtualization=no, disabling it in both containers and virtual machines. Each VM platform tends to deal with or ignore the time problem in their own special ways, KVM/QEMU has the kernel time source kvm-clock, Xen has had different schemes over the years, VMware expects a userspace daemon sync the clock, and other platforms are content to drift with the wind as far as I can tell. I don't know of a robust way to know if a platform needs a little extra help from userspace to keep the clock sane or not but it seems generally safer to try than to risk drifting. Does anyone know of a reason to leave timesyncd off by default? Otherwise switching to ConditionVirtualization=!container should be reasonable. Thanks, Mike ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] networkd: accept a trailing '.' on the end of domains
On Wed, Jan 28, 2015 at 8:49 AM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 15.01.15 13:24, Michael Marineau (michael.marin...@coreos.com) wrote: While not common outside of BIND configs the implied top level '.' in domains is commonly accepted and crops up in random places. Starting with commit 784d9b9c networkd began validating domains as hostnames which rejects trailing dots, breaking short name resolution in some environments such as Google Compute Engine. This change splits the validation code into two functions to be more tolerant for domains. Did I get this right? the Google Compute Engine returns a domain name with trailing dot in the DHCP domain option? Our DHCP client should certainly accept that, if that's what people send, but I am not sure we should be equally liberal for locally configured bits... I now made this change: http://cgit.freedesktop.org/systemd/systemd/commit/?id=f50f01f4b738f2f00b30d0e02e8cf54ab99a9f27 Does that make things work on the google thing? That should be sufficient, and certainly fair to be stricter about local config files. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] missing: add macros for OFD locks
--- src/shared/missing.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/shared/missing.h b/src/shared/missing.h index cdc38b2..d074405 100644 --- a/src/shared/missing.h +++ b/src/shared/missing.h @@ -79,6 +79,12 @@ #define F_SEAL_WRITE0x0008 /* prevent writes */ #endif +#ifndef F_OFD_GETLK +#define F_OFD_GETLK 36 +#define F_OFD_SETLK 37 +#define F_OFD_SETLKW38 +#endif + #ifndef MFD_ALLOW_SEALING #define MFD_ALLOW_SEALING 0x0002U #endif -- 2.0.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] networkd: accept a trailing '.' on the end of domains
While not common outside of BIND configs the implied top level '.' in domains is commonly accepted and crops up in random places. Starting with commit 784d9b9c networkd began validating domains as hostnames which rejects trailing dots, breaking short name resolution in some environments such as Google Compute Engine. This change splits the validation code into two functions to be more tolerant for domains. --- src/libsystemd-network/sd-dhcp-lease.c | 2 +- src/network/networkd-network.c | 2 +- src/shared/util.c | 13 ++--- src/shared/util.h | 1 + src/test/test-util.c | 14 ++ 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 00fef16..53a329e 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -502,7 +502,7 @@ int dhcp_lease_parse_options(uint8_t code, uint8_t len, const uint8_t *option, if (r 0) return r; -if (!hostname_is_valid(domainname) || is_localhost(domainname)) +if (!domainname_is_valid(domainname) || is_localhost(domainname)) break; free(lease-domainname); diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 34a06d3..1c4c1ff 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -407,7 +407,7 @@ int config_parse_domains(const char *unit, STRV_FOREACH(domain, *domains) { if (is_localhost(*domain)) log_syntax(unit, LOG_ERR, filename, line, EINVAL, 'localhost' domain names may not be configured, ignoring assignment: %s, *domain); -else if (!hostname_is_valid(*domain)) { +else if (!domainname_is_valid(*domain)) { if (!streq(*domain, *)) log_syntax(unit, LOG_ERR, filename, line, EINVAL, domain name is not valid, ignoring assignment: %s, *domain); } else diff --git a/src/shared/util.c b/src/shared/util.c index 884e782..9e03da4 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -4231,7 +4231,7 @@ static bool hostname_valid_char(char c) { c == '.'; } -bool hostname_is_valid(const char *s) { +bool domainname_is_valid(const char *s) { const char *p; bool dot; @@ -4252,10 +4252,17 @@ bool hostname_is_valid(const char *s) { } } -if (dot) +if (p-s HOST_NAME_MAX) return false; -if (p-s HOST_NAME_MAX) +return true; +} + +bool hostname_is_valid(const char *s) { +if (!domainname_is_valid(s)) +return false; + +if (s[strlen(s)-1] == '.') return false; return true; diff --git a/src/shared/util.h b/src/shared/util.h index fdb9fb6..e332239 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -547,6 +547,7 @@ bool nulstr_contains(const char*nulstr, const char *needle); bool plymouth_running(void); bool hostname_is_valid(const char *s) _pure_; +bool domainname_is_valid(const char *s) _pure_; char* hostname_cleanup(char *s, bool lowercase); bool machine_name_is_valid(const char *s) _pure_; diff --git a/src/test/test-util.c b/src/test/test-util.c index 4bb5154..010d93b 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -539,6 +539,20 @@ static void test_hostname_is_valid(void) { assert_se(!hostname_is_valid()); } +static void test_domainname_is_valid(void) { +assert_se(domainname_is_valid(foobar)); +assert_se(domainname_is_valid(foobar.)); +assert_se(domainname_is_valid(foobar.com)); +assert_se(domainname_is_valid(foobar.com.)); +assert_se(!domainname_is_valid(fööbar)); +assert_se(!domainname_is_valid()); +assert_se(!domainname_is_valid(.)); +assert_se(!domainname_is_valid(..)); +assert_se(!domainname_is_valid(.foobar)); +assert_se(!domainname_is_valid(foo..bar)); + assert_se(!domainname_is_valid()); +} + static void test_u64log2(void) { assert_se(u64log2(0) == 0); assert_se(u64log2(8) == 3); -- 2.0.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fstab-generator: Allow mount.usr without mount.usrflags, honor rw/ro
On Sun, Dec 7, 2014 at 4:51 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sat, Dec 06, 2014 at 02:47:51PM -0800, Michael Marineau wrote: There is no need to require mount.usrflags. The original implementation assumed that a btrfs subvolume would always be needed but that is not applicable to systems that do not use btrfs for /usr. Similar to using rootflags= for the default of mount.usrflags=, append the classic 'ro' and 'rw' flags to the mount options. --- src/fstab-generator/fstab-generator.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index e8a21f7..236fb37 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -476,7 +476,7 @@ static int add_usr_mount(void) { return log_oom(); } -if (!arg_usr_what || !arg_usr_options) +if (!arg_usr_what) return 0; what = fstab_node_to_udev_node(arg_usr_what); @@ -485,7 +485,14 @@ static int add_usr_mount(void) { return -1; } -opts = arg_usr_options; +if (!arg_usr_options) +opts = arg_root_rw 0 ? rw : ro; +else if (arg_root_rw = 0 || + (!mount_test_option(arg_usr_options, ro) + !mount_test_option(arg_usr_options, rw))) This condition looks wrong. rw or ro will be always appended when arg_root_rw is set. Is the intent to override arg_usr_options? Zbyszek Hm, yes. I read this wrong when coping it from add_root_mount, assuming that 'rootflags=' had precedence over the bare 'ro' and 'rw' options but it apparently is the other way around. For /usr we can just drop the 'arg_root_rw = 0 ||' bit so 'mount.usrflags=' has precedence. Will resend the patch. +opts = strappenda(arg_usr_options, ,, arg_root_rw 0 ? rw : ro); +else +opts = arg_usr_options; log_debug(Found entry what=%s where=/sysroot/usr type=%s, what, strna(arg_usr_fstype)); return add_mount(what, -- 2.0.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2] fstab-generator: Allow mount.usr without mount.usrflags, honor rw/ro
There is no need to require mount.usrflags. The original implementation assumed that a btrfs subvolume would always be needed but that is not applicable to systems that do not use btrfs for /usr. Similar to using rootflags= for the default of mount.usrflags=, append the classic 'ro' and 'rw' flags to the mount options. --- src/fstab-generator/fstab-generator.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index e8a21f7..c8d3ac0 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -476,7 +476,7 @@ static int add_usr_mount(void) { return log_oom(); } -if (!arg_usr_what || !arg_usr_options) +if (!arg_usr_what) return 0; what = fstab_node_to_udev_node(arg_usr_what); @@ -485,7 +485,13 @@ static int add_usr_mount(void) { return -1; } -opts = arg_usr_options; +if (!arg_usr_options) +opts = arg_root_rw 0 ? rw : ro; +else if (!mount_test_option(arg_usr_options, ro) + !mount_test_option(arg_usr_options, rw)) +opts = strappenda(arg_usr_options, ,, arg_root_rw 0 ? rw : ro); +else +opts = arg_usr_options; log_debug(Found entry what=%s where=/sysroot/usr type=%s, what, strna(arg_usr_fstype)); return add_mount(what, -- 2.0.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fstab-generator: Allow mount.usr without mount.usrflags, honor rw/ro
There is no need to require mount.usrflags. The original implementation assumed that a btrfs subvolume would always be needed but that is not applicable to systems that do not use btrfs for /usr. Similar to using rootflags= for the default of mount.usrflags=, append the classic 'ro' and 'rw' flags to the mount options. --- src/fstab-generator/fstab-generator.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index e8a21f7..236fb37 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -476,7 +476,7 @@ static int add_usr_mount(void) { return log_oom(); } -if (!arg_usr_what || !arg_usr_options) +if (!arg_usr_what) return 0; what = fstab_node_to_udev_node(arg_usr_what); @@ -485,7 +485,14 @@ static int add_usr_mount(void) { return -1; } -opts = arg_usr_options; +if (!arg_usr_options) +opts = arg_root_rw 0 ? rw : ro; +else if (arg_root_rw = 0 || + (!mount_test_option(arg_usr_options, ro) + !mount_test_option(arg_usr_options, rw))) +opts = strappenda(arg_usr_options, ,, arg_root_rw 0 ? rw : ro); +else +opts = arg_usr_options; log_debug(Found entry what=%s where=/sysroot/usr type=%s, what, strna(arg_usr_fstype)); return add_mount(what, -- 2.0.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] preset enables everything by default
On Thu, Dec 4, 2014 at 5:50 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 02.12.14 09:40, Michael Marineau (michael.marin...@coreos.com) wrote: I didn't catch this behavior when it was first introduced since originally it was much harder to trigger systemd's empty /etc logic but now that it only requires /etc/machine-id to be missing it is quite easy, booting a new instance from an image for example. By default applying presets enables everything unless there is a preset config that defines otherwise. I found this to be rather surprising, booting a fresh machine reported all sorts of failures by trying to start oodles of unconfigured services. Those services should not be listed as enable in the preset file if they fail to start unless explicitly configured. They aren't listed, that's why I'm asking about the default. :) Also the options are only enable and disable so the existing pattern of pre-preconfiguring a reference host and then creating an image (EC2 AMIs for example) won't work very well since the preset defaults will clobber what the user enabled/disabled. (assuming the user properly clears machine-id before creating the image which may be rare, in all likelihood many people just get away with having non-unique machine ids) We use the machine-id file as check whether /etc is populated or not. If people pre-populate /etc, and don't wan't the full first-boot logic of systemd to take action, then they should also add machine-id file there (they can even just touch it if they want, which will disable the first-boot logic, but still initialize the file either directly if writable or overmounted if not). So when assembling a machine image that will be used to create a bunch of pre-configured hosts the correct thing to do is 'echo machine-id', not 'rm machine-id'? This behavior is probably ok in the case of interactively using systemctl preset and preset-all when it is known that the user explicitly asked the system to do something and can see what it did. In the case of the system booting I would expect do nothing to be the default when no preset file explicitly sates otherwise. Then ship a disable * preset file in /sr. In this case, nothing will be enabled by default. THis is what we do on Fedora. And I've added this to CoreOS too. The gist of my rambling email was why is this not the default? Is there a particular reason for the existing behavior? Would switching the default to disable be reasonable or should the automatic at boot mode gain a third do nothing option? Not sure where the safest and least-surprising behavior lies while continuing to provide this preset functionality. Personally I've always found the enable/disable terminology to be incredibly misleading to begin with since it only refers to configuration in /etc and units can be equally activated in /usr. If disable and mask were equivalent then the distro's presets would just be whatever is in /usr and there won't be a need for this extra preset mechanism to initialize /etc. We have the static state for units that are statically on via /usr, and hence aren't subject to systemctl enable and systemctl disable. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] preset enables everything by default
I didn't catch this behavior when it was first introduced since originally it was much harder to trigger systemd's empty /etc logic but now that it only requires /etc/machine-id to be missing it is quite easy, booting a new instance from an image for example. By default applying presets enables everything unless there is a preset config that defines otherwise. I found this to be rather surprising, booting a fresh machine reported all sorts of failures by trying to start oodles of unconfigured services. Also the options are only enable and disable so the existing pattern of pre-preconfiguring a reference host and then creating an image (EC2 AMIs for example) won't work very well since the preset defaults will clobber what the user enabled/disabled. (assuming the user properly clears machine-id before creating the image which may be rare, in all likelihood many people just get away with having non-unique machine ids) This behavior is probably ok in the case of interactively using systemctl preset and preset-all when it is known that the user explicitly asked the system to do something and can see what it did. In the case of the system booting I would expect do nothing to be the default when no preset file explicitly sates otherwise. Is there a particular reason for the existing behavior? Would switching the default to disable be reasonable or should the automatic at boot mode gain a third do nothing option? Not sure where the safest and least-surprising behavior lies while continuing to provide this preset functionality. Personally I've always found the enable/disable terminology to be incredibly misleading to begin with since it only refers to configuration in /etc and units can be equally activated in /usr. If disable and mask were equivalent then the distro's presets would just be whatever is in /usr and there won't be a need for this extra preset mechanism to initialize /etc. Cheers, Mike ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Allow PID 1 systemd --user instances to exit
On Nov 6, 2014 5:17 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 06.11.14 16:59, Vito Caputo (vito.cap...@coreos.com) wrote: Because for all intents and purposes it's effectively still a user instance, just having its own PID namespace isn't cause --system behaviors like disabling systemctl exit for example. I am pretty sure doing something like this will break at a ton of other places. I really wonder if it's worth supporting this, after all a lot of our code checks getpid() == 1 to see if we are run in system mode. I mean, once upon a time we had a mode in systemd, where we supported running --system system as PID != 1. We removed that because it only ever half-worked, because it confused things, because the usecase was weak, because nobody really cared and because it bitrotted. Now, supporting running systemd --user in a PID namespace kinda feels like the same story, just inverted. Which makes me immediately wonder why this should be different for this case. So, what's the real usecase for all of this? Can you elaborate on that? The basic idea is to create a container that has the ability to return a normal exit code when it exits. System instances don't allow this. User instances do and also avoid other undesired things like reading extra args from /proc/cmdline which isn't applicable to a container. There seems to be a odd fit here between expecting a system instance to behave nicely like yet another service on a host or a user instance to be pid 1 in a container. Preventing exit from PID 1 makes sense when you're going to panic the kernel, but doesn't --user imply otherwise? Well, the --user switch as PID 1 is probably something we should refuse early on... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Allow PID 1 systemd --user instances to exit
On Thu, Nov 6, 2014 at 6:02 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 06.11.14 17:48, Michael Marineau (michael.marin...@coreos.com) wrote: So, what's the real usecase for all of this? Can you elaborate on that? The basic idea is to create a container that has the ability to return a normal exit code when it exits. System instances don't allow this. Well, but this is something we could allow. In fact our shutdown code invokes exit(0) if reboot() returns EPERM already (in case of CAP_SYS_BOOT is missing). That said, note that in a PID namespace reboot() nowadays results in the equivalent of raise(SIGINT) anyway, which isn't too different from a simple exit(). The trick then is just reworking that to support plumbing through an exit code to exit() instead. User instances do and also avoid other undesired things like reading extra args from /proc/cmdline which isn't applicable to a container. There's already an explicit check in place that makes sure read /proc/1/cmdline in place of of /proc/cmdline if we detect we are run in a container: http://cgit.freedesktop.org/systemd/systemd/tree/src/shared/util.c#n6174 Missed that one, some other difference in behaviour mislead us I suppose. There seems to be a odd fit here between expecting a system instance to behave nicely like yet another service on a host or a user instance to be pid 1 in a container. Hmm, so what you are trying to do running one systemd instance as a service on another one, in a way that they see the same file hiearchy but operate based on unit files in a different directory? Is that correct? Wouldn't a container (maybe nspawn) work for this with some clevery set up --bind= mounts? I nspawn (or similar? I'm not actually part of this particular project) is being used here, hence running as PID 1. Running the instance as --user seemed like the more fruitful way to plumb through an exit code but fixing --system probably would fit our needs too. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] man: use the escape for - in example instead of space.
This sentence can be misread to mean that \x20 is the escape code for - which is the only character explicitly mentioned. This lead to at least one user loosing hair over why a mount unit for /foo/bar-baz didn't work. The example escape is arbitrary so lets prevent hair loss. --- man/systemd.unit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 6ea552e..67d46ed 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -226,7 +226,7 @@ result is usable as part of a filename. Basically, given a path, / is replaced by -, and all unprintable characters and the - are replaced by -C-style \x20 escapes. The root directory / is +C-style \x2d escapes. The root directory / is encoded as single dash, while otherwise the initial and ending / is removed from all paths during transformation. This escaping is reversible./para -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
On Aug 14, 2014 1:21 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 04.08.14 10:05, Michael Marineau (michael.marin...@coreos.com) wrote: Patch looks pretty good, though I'd really prefer if we could do the UseDomain= thing as discussed in the other mail, and not propagate DHCP-supplied domain names unless explicitly requested. This would means we probably mean we'd need two new sd-network.h calls: int sd_network_get_link_route_domains(int ifindex, char **domains); int sd_network_get_link_search_domains(int ifindex, char **domains); The former would return the list of domains whose requests shall be routed to the specified interface, and the latter would be the list of domains we actively use for searching single-label domains in. Any domains configured statically for a link in the .network files would be listed in both lists. And depending on the UseDomains= settings the dhcp provides domains might be listed on none, both or only one of them. or something like that... src/network/networkd-link.c| 9 + src/network/sd-network.c | 24 src/resolve/resolved-link.c| 20 src/resolve/resolved-link.h| 2 ++ src/resolve/resolved-manager.c | 10 +- src/systemd/sd-network.h | 3 +++ 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 172be64..42d528f 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2385,6 +2385,15 @@ int link_save(Link *link) { (address + 1 ? : )); fputs(\n, f); + +if (link-network-dhcp_domainname +link-dhcp_lease) { +const char *domainname; + +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, domainname); +if (r = 0) +fprintf(f, DOMAINNAME=%s\n, domainname); THis should be plural really, from the beginning. After all the newer DHCP specs allow a full list... and we want to allow a full list to be provided in the .network files too... Lennart -- Lennart Poettering, Red Hat Right now the search domains DHCP option is unsupported so it is indeed singular. Also I had assumed since search domains is both a different DHCP option and a different resolve.conf option that they would be recorded separately but I suppose the two options is more of a legacy artifact than meaningful distinction so it is equally as valid to squash them together into the search domain list. I am happy to write a follow up patch to implement the search domains option and support providing additional domains in the .network file but I think this patch can stand alone since it fixes a regression. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
When the code for generating resolv.conf was moved from networkd to resolved the DHCP domain name code was dropped. --- This is a refresh of the patch on recent master with a little bit of cleanup from the last. Regarding the robustness/correctness/etc of setting the domain resolv.conf attribute from DNS, I don't think that is practical to address in this patch. The implementation is already clearly incomplete because networkd doesn't handle search domains which is actually an entirely different option. My goal here is to just fix the regression from when resolved was first introduced. The most common setup is for domain to correspond to the domain suffix for the local host name. If you are concerned about which interface's domain attribute wins and lands in resolv.conf, there is a related issue of which interface's host name winds up being applied as the host's transient host name. What ever interface wins the two should probably match but this is complicated significantly by the two being handled by different daemons, resolved vs hostnamed, with two different integration points with networkd, reading state files in /run vs dbus method calls. I don't have a good recommendation to make sense of any of this right now. src/network/networkd-link.c| 9 + src/network/sd-network.c | 24 src/resolve/resolved-link.c| 20 src/resolve/resolved-link.h| 2 ++ src/resolve/resolved-manager.c | 10 +- src/systemd/sd-network.h | 3 +++ 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 172be64..42d528f 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2385,6 +2385,15 @@ int link_save(Link *link) { (address + 1 ? : )); fputs(\n, f); + +if (link-network-dhcp_domainname +link-dhcp_lease) { +const char *domainname; + +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, domainname); +if (r = 0) +fprintf(f, DOMAINNAME=%s\n, domainname); +} } if (link-dhcp_lease) { diff --git a/src/network/sd-network.c b/src/network/sd-network.c index bfb8321..a427a27 100644 --- a/src/network/sd-network.c +++ b/src/network/sd-network.c @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char ***ret) { return network_get_strv(NTP, ifindex, ret); } +_public_ int sd_network_get_domainname(int ifindex, char **domainname) { +_cleanup_free_ char *s = NULL, *p = NULL; +int r; + +assert_return(ifindex 0, -EINVAL); +assert_return(domainname, -EINVAL); + +if (asprintf(p, /run/systemd/netif/links/%d, ifindex) 0) +return -ENOMEM; + +r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL); +if (r == -ENOENT) +return -ENODATA; +else if (r 0) +return r; +else if (!s) +return -EIO; + +*domainname = s; +s = NULL; + +return 0; +} + static inline int MONITOR_TO_FD(sd_network_monitor *m) { return (int) (unsigned long) m - 1; } diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 2c02f09..9d582e4 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -74,6 +74,7 @@ Link *link_free(Link *l) { while (l-dns_servers) dns_server_free(l-dns_servers); +free(l-domainname); free(l); return NULL; } @@ -188,10 +189,29 @@ clear: return r; } +static int link_update_domainname(Link *l) { +char *domainname = NULL; +int r; + +assert(l); + +free(l-domainname); +l-domainname = NULL; + +r = sd_network_get_domainname(l-ifindex, domainname); +if (r 0) +return r; + +l-domainname = domainname; + +return 0; +} + int link_update_monitor(Link *l) { assert(l); link_update_dns_servers(l); +link_update_domainname(l); link_allocate_scopes(l); link_add_rrs(l, false); diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h index 38bb392..eed9f42 100644 --- a/src/resolve/resolved-link.h +++ b/src/resolve/resolved-link.h @@ -68,6 +68,8 @@ struct Link { RateLimit mdns_ratelimit; RateLimit llmnr_ratelimit; + +char *domainname; }; int link_new(Manager *m, Link **ret, int ifindex); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 1b6dc8a..8f28eaf 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -648,6 +648,7 @@ int manager_write_resolv_conf(Manager *m) { static const char path[] = /run/systemd/resolve/resolv.conf;
[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
When the code for generating resolv.conf was moved from networkd to resolved the DHCP domain name code was dropped. --- This is a resend, rebased since some recent changes changed how this patch needed to be implemented. src/network/networkd-link.c| 13 + src/network/sd-network.c | 24 src/resolve/resolved-link.c| 20 src/resolve/resolved-link.h| 2 ++ src/resolve/resolved-manager.c | 10 +- src/systemd/sd-network.h | 3 +++ 6 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 3b8b7ed..827c428 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2451,6 +2451,19 @@ int link_save(Link *link) { (address + 1 ? : )); fputs(\n, f); + +fprintf(f, DOMAINNAME=); + +if (link-network-dhcp_domainname +link-dhcp_lease) { +const char *domainname; + +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, domainname); +if (r = 0) +fputs(domainname, f); +} + +fputs(\n, f); } if (link-dhcp_lease) { diff --git a/src/network/sd-network.c b/src/network/sd-network.c index bfb8321..a427a27 100644 --- a/src/network/sd-network.c +++ b/src/network/sd-network.c @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char ***ret) { return network_get_strv(NTP, ifindex, ret); } +_public_ int sd_network_get_domainname(int ifindex, char **domainname) { +_cleanup_free_ char *s = NULL, *p = NULL; +int r; + +assert_return(ifindex 0, -EINVAL); +assert_return(domainname, -EINVAL); + +if (asprintf(p, /run/systemd/netif/links/%d, ifindex) 0) +return -ENOMEM; + +r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL); +if (r == -ENOENT) +return -ENODATA; +else if (r 0) +return r; +else if (!s) +return -EIO; + +*domainname = s; +s = NULL; + +return 0; +} + static inline int MONITOR_TO_FD(sd_network_monitor *m) { return (int) (unsigned long) m - 1; } diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 6ac7c5b..f6b7f6a 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -77,6 +77,7 @@ Link *link_free(Link *l) { while (l-dns_servers) dns_server_free(l-dns_servers); +free(l-domainname); free(l); return NULL; } @@ -191,10 +192,29 @@ clear: return r; } +static int link_update_domainname(Link *l) { +char *domainname = NULL; +int r; + +assert(l); + +free(l-domainname); +l-domainname = NULL; + +r = sd_network_get_domainname(l-ifindex, domainname); +if (r 0) +return r; + +l-domainname = domainname; + +return 0; +} + int link_update_monitor(Link *l) { assert(l); link_update_dns_servers(l); +link_update_domainname(l); link_allocate_scopes(l); link_add_rrs(l); diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h index f58bd54..9730aec 100644 --- a/src/resolve/resolved-link.h +++ b/src/resolve/resolved-link.h @@ -68,6 +68,8 @@ struct Link { RateLimit mdns_ratelimit; RateLimit llmnr_ratelimit; + +char *domainname; }; int link_new(Manager *m, Link **ret, int ifindex); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index a8715bd..253a97e 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -522,6 +522,7 @@ int manager_write_resolv_conf(Manager *m) { const char *path = /run/systemd/resolve/resolv.conf; _cleanup_free_ char *temp_path = NULL; _cleanup_fclose_ FILE *f = NULL; +const char *domainname = NULL; unsigned count = 0; DnsServer *s; Iterator i; @@ -542,13 +543,20 @@ int manager_write_resolv_conf(Manager *m) { # resolv.conf(5) in a different way, replace the symlink by a\n # static file or a different symlink.\n\n, f); -HASHMAP_FOREACH(l, m-links, i) +HASHMAP_FOREACH(l, m-links, i) { LIST_FOREACH(servers, s, l-dns_servers) write_resolve_conf_server(s, f, count); +if (!domainname l-domainname) +domainname = l-domainname; +} + LIST_FOREACH(servers, s, m-dns_servers) write_resolve_conf_server(s, f, count); +if (domainname) +fprintf(f, domain %s\n, domainname); + r = fflush_and_check(f); if (r 0)
[systemd-devel] [PATCH] nspawn: fix truncation of machine names in interface names
When deriving the network interface name from machine name strncpy was not properly null terminating the string and the maximum string size as returned by strlen() is actually IFNAMSIZ-1, not IFNAMSIZ. --- src/nspawn/nspawn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 7c47f6e..73eeed6 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -69,6 +69,7 @@ #include missing.h #include cgroup-util.h #include strv.h +#include strxcpyx.h #include path-util.h #include loopback-setup.h #include dev-setup.h @@ -1663,7 +1664,7 @@ static int setup_veth(pid_t pid, char iface_name[IFNAMSIZ], int *ifi) { memcpy(iface_name, vb-, 3); else memcpy(iface_name, ve-, 3); -strncpy(iface_name+3, arg_machine, IFNAMSIZ - 3); +strscpy(iface_name+3, IFNAMSIZ - 4, arg_machine); r = get_mac(mac); if (r 0) { -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
On Tue, Jul 29, 2014 at 3:37 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Tue, Jul 29, 2014 at 02:48:18PM -0700, Michael Marineau wrote: When the code for generating resolv.conf was moved from networkd to resolved the DHCP domain name code was dropped. --- This is a resend, rebased since some recent changes changed how this patch needed to be implemented. src/network/networkd-link.c| 13 + src/network/sd-network.c | 24 src/resolve/resolved-link.c| 20 src/resolve/resolved-link.h| 2 ++ src/resolve/resolved-manager.c | 10 +- src/systemd/sd-network.h | 3 +++ 6 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 3b8b7ed..827c428 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2451,6 +2451,19 @@ int link_save(Link *link) { (address + 1 ? : )); fputs(\n, f); + +fprintf(f, DOMAINNAME=); + +if (link-network-dhcp_domainname +link-dhcp_lease) { +const char *domainname; + +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, domainname); +if (r = 0) +fputs(domainname, f); +} + +fputs(\n, f); Is it really necessary to write anything if the name is not available? Other parts of this function don't write anyting in similar cases. I was just matching the above lines which may write DNS= or NTP= with blank values. I don't think it matters either way. Omitting DOMAINNAME= if it is blank certainly looks a little cleaner since the writes get squashed into a single fprintf. Will update. if (link-dhcp_lease) { diff --git a/src/network/sd-network.c b/src/network/sd-network.c index bfb8321..a427a27 100644 --- a/src/network/sd-network.c +++ b/src/network/sd-network.c @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char ***ret) { return network_get_strv(NTP, ifindex, ret); } +_public_ int sd_network_get_domainname(int ifindex, char **domainname) { +_cleanup_free_ char *s = NULL, *p = NULL; +int r; + +assert_return(ifindex 0, -EINVAL); +assert_return(domainname, -EINVAL); + +if (asprintf(p, /run/systemd/netif/links/%d, ifindex) 0) +return -ENOMEM; Not terribly important, but please spell that as: char p[sizeof(/run/systemd/netif/links/) + DECIMAL_STRING_MAX(int)]; snprintf(p, sizeof(p), /run/systemd/netif/links/%d, ifindex); This was copied verbatim from similar functions in this file, should I update the style of the others to match your suggestion? Why the preference of manually calculating a buffer length than using asprintf? +r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL); +if (r == -ENOENT) +return -ENODATA; +else if (r 0) +return r; +else if (!s) +return -EIO; + +*domainname = s; +s = NULL; + +return 0; +} + static inline int MONITOR_TO_FD(sd_network_monitor *m) { return (int) (unsigned long) m - 1; } diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 6ac7c5b..f6b7f6a 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -77,6 +77,7 @@ Link *link_free(Link *l) { while (l-dns_servers) dns_server_free(l-dns_servers); +free(l-domainname); free(l); return NULL; } @@ -191,10 +192,29 @@ clear: return r; } +static int link_update_domainname(Link *l) { +char *domainname = NULL; +int r; + +assert(l); + +free(l-domainname); +l-domainname = NULL; + +r = sd_network_get_domainname(l-ifindex, domainname); +if (r 0) +return r; + +l-domainname = domainname; + +return 0; +} + int link_update_monitor(Link *l) { assert(l); link_update_dns_servers(l); +link_update_domainname(l); link_allocate_scopes(l); link_add_rrs(l); diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h index f58bd54..9730aec 100644 --- a/src/resolve/resolved-link.h +++ b/src/resolve/resolved-link.h @@ -68,6 +68,8 @@ struct Link { RateLimit mdns_ratelimit; RateLimit llmnr_ratelimit; + +char *domainname; }; int link_new(Manager *m, Link **ret, int ifindex); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index a8715bd..253a97e 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -522,6 +522,7 @@ int
[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
When the code for generating resolv.conf was moved from networkd to resolved the DHCP domain name code was dropped. --- src/network/networkd-link.c| 2 ++ src/network/sd-network.c | 4 src/resolve/resolved-link.c| 31 +++ src/resolve/resolved-link.h| 2 ++ src/resolve/resolved-manager.c | 7 +++ src/systemd/sd-network.h | 3 +++ 6 files changed, 49 insertions(+) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 0a6f524..afca172 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2524,9 +2524,11 @@ int link_save(Link *link) { fprintf(f, DHCP_LEASE=%s\n DHCP_USE_DNS=%s\n +DHCP_USE_DOMAINNAME=%s\n DHCP_USE_NTP=%s\n, link-lease_file, yes_no(link-network-dhcp_dns), +yes_no(link-network-dhcp_domainname), yes_no(link-network-dhcp_ntp)); } else unlink(link-lease_file); diff --git a/src/network/sd-network.c b/src/network/sd-network.c index 91d6275..5dfdd59 100644 --- a/src/network/sd-network.c +++ b/src/network/sd-network.c @@ -202,6 +202,10 @@ _public_ int sd_network_dhcp_use_dns(int ifindex) { return network_get_boolean(DHCP_USE_DNS, ifindex); } +_public_ int sd_network_dhcp_use_domainname(int ifindex) { +return network_get_boolean(DHCP_USE_DOMAINNAME, ifindex); +} + _public_ int sd_network_dhcp_use_ntp(int ifindex) { return network_get_boolean(DHCP_USE_NTP, ifindex); } diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 078301a..c0b19a6 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -77,6 +77,7 @@ Link *link_free(Link *l) { while (l-link_dns_servers) dns_server_free(l-link_dns_servers); +free(l-dhcp_domainname); free(l); return NULL; } @@ -249,11 +250,41 @@ clear: return r; } +static int link_update_dhcp_domainname(Link *l) { +_cleanup_dhcp_lease_unref_ sd_dhcp_lease *lease = NULL; +const char *domainname = NULL; +int r; + +assert(l); + +free(l-dhcp_domainname); +l-dhcp_domainname = NULL; + +r = sd_network_dhcp_use_dns(l-ifindex); +if (r = 0) +return r; + +r = sd_network_get_dhcp_lease(l-ifindex, lease); +if (r 0) +return r; + +r = sd_dhcp_lease_get_domainname(lease, domainname); +if (r 0) +return r; + +l-dhcp_domainname = strdup(domainname); +if (!l-dhcp_domainname) +return -ENOMEM; + +return 0; +} + int link_update_monitor(Link *l) { assert(l); link_update_dhcp_dns_servers(l); link_update_link_dns_servers(l); +link_update_dhcp_domainname(l); link_allocate_scopes(l); return 0; diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h index bd32a70..8ea3acd 100644 --- a/src/resolve/resolved-link.h +++ b/src/resolve/resolved-link.h @@ -65,6 +65,8 @@ struct Link { RateLimit mdns_ratelimit; RateLimit llmnr_ratelimit; + +char *dhcp_domainname; }; int link_new(Manager *m, Link **ret, int ifindex); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 9672843..09f7695 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -497,6 +497,7 @@ int manager_write_resolv_conf(Manager *m) { const char *path = /run/systemd/resolve/resolv.conf; _cleanup_free_ char *temp_path = NULL; _cleanup_fclose_ FILE *f = NULL; +const char *domainname = NULL; unsigned count = 0; DnsServer *s; Iterator i; @@ -523,11 +524,17 @@ int manager_write_resolv_conf(Manager *m) { LIST_FOREACH(servers, s, l-dhcp_dns_servers) write_resolve_conf_server(s, f, count); + +if (!domainname l-dhcp_domainname) +domainname = l-dhcp_domainname; } LIST_FOREACH(servers, s, m-dns_servers) write_resolve_conf_server(s, f, count); +if (domainname) +fprintf(f, domain %s\n, domainname); + r = fflush_and_check(f); if (r 0) goto fail; diff --git a/src/systemd/sd-network.h b/src/systemd/sd-network.h index e454705..826cec7 100644 --- a/src/systemd/sd-network.h +++ b/src/systemd/sd-network.h @@ -79,6 +79,9 @@ int sd_network_get_dhcp_lease(int ifindex, sd_dhcp_lease **ret); /* Returns true if link is configured to respect DNS entries received by DHCP */ int sd_network_dhcp_use_dns(int ifindex); +/* Returns true if link is configured to use the domain name received by DHCP */ +int
[systemd-devel] [PATCH] networkd: fix reporting errors from hostnamed
The return value may be -EINVAL or a positive errno from the dbus message. Check both ranges, otherwise most errors are silently ignored. --- src/network/networkd-link.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 7a0f30b..be879fd 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -845,7 +845,9 @@ static int set_hostname_handler(sd_bus *bus, sd_bus_message *m, void *userdata, r = sd_bus_message_get_errno(m); if (r 0) -log_warning_link(link, Could not set hostname: %s, strerror(-r)); +r = -r; +if (r 0) +log_warning_link(link, Could not set hostname: %s, strerror(r)); return 1; } -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Persistent virtio device name removal
Working on bumping to 215 over here in CoreOS land, but I've got a question regarding the removal of persistent device names for virtio devices since changing the network device names creates a difficult upgrade path from 212. The commit was: http://cgit.freedesktop.org/systemd/systemd/commit/?id=bf81e792f3c0aed54edf004c1c95cc6f6d81d0ee udev: persistent naming - we cannot use virtio numbers as they are not stable The commit doesn't say what the issue was, in what situations are the virtio numbers not stable? Thanks, Mike ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] build-sys: require elfutils = 158
The recently added stacktrace support in 8d4e028f uses functions added in elfutils 158. Check for one of the new functions to avoid attempting to build against older versions. --- configure.ac | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 1391d03..c1f83ee 100644 --- a/configure.ac +++ b/configure.ac @@ -639,7 +639,7 @@ AC_ARG_ENABLE([elfutils], if test x${have_elfutils} != xno ; then AC_CHECK_HEADERS( [elfutils/libdwfl.h], -[have_elfutils=yes], +[], [if test x$have_elfutils = xyes ; then AC_MSG_ERROR([*** ELFUTILS headers not found.]) fi]) @@ -647,11 +647,19 @@ if test x${have_elfutils} != xno ; then AC_CHECK_LIB( [dw], [dwfl_begin], -[have_elfutils=yes], +[], [if test x$have_elfutils = xyes ; then AC_MSG_ERROR([*** ELFUTILS libs not found.]) fi]) +AC_CHECK_LIB( +[dw], +[dwfl_core_file_attach], +[have_elfutils=yes], +[if test x$have_elfutils = xyes ; then +AC_MSG_ERROR([*** ELFUTILS = 158 is required.]) +fi]) + if test x$have_elfutils = xyes ; then ELFUTILS_LIBS=-lelf -ldw AC_DEFINE(HAVE_ELFUTILS, 1, [ELFUTILS available]) -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/5] test: ensure conf_files_list returns absolute paths
--- .gitignore | 1 + Makefile.am| 9 - src/test/test-conf-files.c | 84 ++ 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 src/test/test-conf-files.c diff --git a/.gitignore b/.gitignore index c7c0793..979ab45 100644 --- a/.gitignore +++ b/.gitignore @@ -138,6 +138,7 @@ /test-cgroup /test-cgroup-mask /test-cgroup-util +/test-conf-files /test-daemon /test-date /test-device-nodes diff --git a/Makefile.am b/Makefile.am index fd3205d..bd26780 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1243,7 +1243,8 @@ tests += \ test-xml \ test-architecture \ test-socket-util \ - test-fdset + test-fdset \ + test-conf-files EXTRA_DIST += \ test/sched_idle_bad.service \ @@ -1600,6 +1601,12 @@ test_sched_prio_LDADD = \ libsystemd-core.la \ $(RT_LIBS) +test_conf_files_SOURCES = \ + src/test/test-conf-files.c + +test_conf_files_LDADD = \ + libsystemd-shared.la + # -- ## .PHONY so it always rebuilds it .PHONY: coverage lcov-run lcov-report coverage-sync diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c new file mode 100644 index 000..e801c59 --- /dev/null +++ b/src/test/test-conf-files.c @@ -0,0 +1,84 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2014 Michael Marineau + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see http://www.gnu.org/licenses/. +***/ + +#include stdio.h +#include stdarg.h + +#include conf-files.h +#include macro.h +#include strv.h +#include util.h + + +static void setup_test_dir(char *tmp_dir, const char *files, ...) { +va_list ap; + +assert_se(mkdtemp(tmp_dir) != NULL); + +va_start(ap, files); +while (files != NULL) { +_cleanup_free_ char *path = strappend(tmp_dir, files); +assert_se(touch_file(path, true, (usec_t) -1, (uid_t) -1, (gid_t) -1, 0) == 0); +files = va_arg(ap, const char *); +} +va_end(ap); +} + +static void test_conf_files_list(bool use_root) { +char tmp_dir[] = /tmp/test-conf-files-XX; +_cleanup_strv_free_ char **found_files = NULL; +const char *root_dir, *search_1, *search_2, *expect_a, *expect_b; + +setup_test_dir(tmp_dir, + /dir1/a.conf, + /dir2/a.conf, + /dir2/b.conf, + NULL); + +if (use_root) { +root_dir = tmp_dir; +search_1 = /dir1; +search_2 = /dir2; +} else { +root_dir = NULL; +search_1 = strappenda(tmp_dir, /dir1); +search_2 = strappenda(tmp_dir, /dir2); +} + +expect_a = strappenda(tmp_dir, /dir1/a.conf); +expect_b = strappenda(tmp_dir, /dir2/b.conf); + +assert_se(conf_files_list(found_files, .conf, root_dir, search_1, search_2, NULL) == 0); +strv_print(found_files); + +assert_se(found_files); +assert_se(streq_ptr(found_files[0], expect_a)); +assert_se(streq_ptr(found_files[1], expect_b)); +assert_se(found_files[2] == NULL); + +assert_se(rm_rf_dangerous(tmp_dir, false, true, false) == 0); +} + +int main(int argc, char **argv) { +test_conf_files_list(false); +test_conf_files_list(true); +return 0; +} -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/5] test: unit test for using alternate roots with path_strv_resolve
--- src/test/test-path-util.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 9f8ae4d..4ee33a9 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -20,10 +20,12 @@ ***/ #include stdio.h +#include unistd.h #include path-util.h #include util.h #include macro.h +#include strv.h static void test_path(void) { @@ -191,11 +193,40 @@ static void test_make_relative(void) { test(//extra/slashes///won'tfool///anybody//, extra///slashesare/just///fine///, ../../../are/just/fine); } +static void test_strv_resolve(void) { +char tmp_dir[] = /tmp/test-path-util-XX; +_cleanup_strv_free_ char **search_dirs = NULL; +_cleanup_strv_free_ char **absolute_dirs = NULL; +char **d; + +assert_se(mkdtemp(tmp_dir) != NULL); + +search_dirs = strv_new(/dir1, /dir2, /dir3, NULL); +assert_se(search_dirs); +STRV_FOREACH(d, search_dirs) { +char *p = strappend(tmp_dir, *d); +assert_se(p); +assert_se(strv_push(absolute_dirs, p) == 0); +} + +assert_se(mkdir(absolute_dirs[0], 0700) == 0); +assert_se(mkdir(absolute_dirs[1], 0700) == 0); +assert_se(symlink(dir2, absolute_dirs[2]) == 0); + +path_strv_resolve(search_dirs, tmp_dir); +assert_se(streq(search_dirs[0], /dir1)); +assert_se(streq(search_dirs[1], /dir2)); +assert_se(streq(search_dirs[2], /dir2)); + +assert_se(rm_rf_dangerous(tmp_dir, false, true, false) == 0); +} + int main(int argc, char **argv) { test_path(); test_find_binary(argv[0]); test_prefixes(); test_fsck_exists(); test_make_relative(); +test_strv_resolve(); return 0; } -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/5] conf-files: include root in returned file paths
This restores the original root handling logic that was present prior to 112cfb18 when path expansion moved to path_strv_canonicalize_absolute. That behavior partially went away in 12ed81d9. Alternatively all users of conf_files_list* could be updated to concatenate the paths themselves as unit_file_query_preset did but since no user needs the un-concatenated form that is pointless duplication. --- src/shared/conf-files.c | 16 ++-- src/shared/install.c| 13 +++-- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c index 44e137e..64ce8a0 100644 --- a/src/shared/conf-files.c +++ b/src/shared/conf-files.c @@ -37,20 +37,16 @@ #include hashmap.h #include conf-files.h -static int files_add(Hashmap *h, const char *dirpath, const char *suffix, const char *root) { +static int files_add(Hashmap *h, const char *root, const char *path, const char *suffix) { _cleanup_closedir_ DIR *dir = NULL; +char *dirpath; -assert(dirpath); +assert(path); assert(suffix); -if (isempty(root)) -dir = opendir(dirpath); -else { -const char *p; +dirpath = strappenda(root ? root : , path); -p = strappenda3(root, /, dirpath); -dir = opendir(p); -} +dir = opendir(dirpath); if (!dir) { if (errno == ENOENT) return 0; @@ -118,7 +114,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const return -ENOMEM; STRV_FOREACH(p, dirs) { -r = files_add(fh, *p, suffix, root); +r = files_add(fh, root, *p, suffix); if (r == -ENOMEM) { hashmap_free_free(fh); return r; diff --git a/src/shared/install.c b/src/shared/install.c index 4f71793..190c554 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1776,7 +1776,7 @@ UnitFileState unit_file_get_state( int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name) { _cleanup_strv_free_ char **files = NULL; -char **i; +char **p; int r; assert(scope = 0); @@ -1804,17 +1804,10 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char if (r 0) return r; -STRV_FOREACH(i, files) { -_cleanup_free_ char *buf = NULL; +STRV_FOREACH(p, files) { _cleanup_fclose_ FILE *f; -const char *p; - -if (root_dir) -p = buf = strjoin(root_dir, /, *i, NULL); -else -p = *i; -f = fopen(p, re); +f = fopen(*p, re); if (!f) { if (errno == ENOENT) continue; -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/5] shared: fix search_and_fopen with alternate roots
Update for the current behavior of path_strv_resolve which now returns paths relative to the given root, not the full absolute paths. --- src/shared/util.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shared/util.c b/src/shared/util.c index c1e1f9f..aaf109e 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -5686,7 +5686,10 @@ static int search_and_fopen_internal(const char *path, const char *mode, const c _cleanup_free_ char *p = NULL; FILE *f; -p = strjoin(*i, /, path, NULL); +if (root) +p = strjoin(root, *i, /, path, NULL); +else +p = strjoin(*i, /, path, NULL); if (!p) return -ENOMEM; -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/5] shared: rename path_strv_canonicalize_absolute functions
Since 12ed81d9 path_strv_canonicalize_absolute leaves the search list relative to the given root directory instead of resolving paths to their true location as the name implies. To better reflect this behavior rename to the less strongly worded path_strv_resolve. --- src/shared/conf-files.c | 2 +- src/shared/path-lookup.c | 6 +++--- src/shared/path-util.c | 6 +++--- src/shared/path-util.h | 4 ++-- src/shared/util.c| 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c index 59bc8ce..44e137e 100644 --- a/src/shared/conf-files.c +++ b/src/shared/conf-files.c @@ -110,7 +110,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const assert(suffix); /* This alters the dirs string array */ -if (!path_strv_canonicalize_absolute_uniq(dirs, root)) +if (!path_strv_resolve_uniq(dirs, root)) return -ENOMEM; fh = hashmap_new(string_hash_func, string_compare_func); diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index e072fd6..e0aaf44 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -284,7 +284,7 @@ int lookup_paths_init( } } -if (!path_strv_canonicalize_absolute_uniq(p-unit_path, root_dir)) +if (!path_strv_resolve_uniq(p-unit_path, root_dir)) return -ENOMEM; if (!strv_isempty(p-unit_path)) { @@ -338,10 +338,10 @@ int lookup_paths_init( return -ENOMEM; } -if (!path_strv_canonicalize_absolute_uniq(p-sysvinit_path, root_dir)) +if (!path_strv_resolve_uniq(p-sysvinit_path, root_dir)) return -ENOMEM; -if (!path_strv_canonicalize_absolute_uniq(p-sysvrcnd_path, root_dir)) +if (!path_strv_resolve_uniq(p-sysvrcnd_path, root_dir)) return -ENOMEM; if (!strv_isempty(p-sysvinit_path)) { diff --git a/src/shared/path-util.c b/src/shared/path-util.c index efe464d..d193494 100644 --- a/src/shared/path-util.c +++ b/src/shared/path-util.c @@ -238,7 +238,7 @@ char **path_strv_make_absolute_cwd(char **l) { return l; } -char **path_strv_canonicalize_absolute(char **l, const char *prefix) { +char **path_strv_resolve(char **l, const char *prefix) { char **s; unsigned k = 0; bool enomem = false; @@ -323,12 +323,12 @@ char **path_strv_canonicalize_absolute(char **l, const char *prefix) { return l; } -char **path_strv_canonicalize_absolute_uniq(char **l, const char *prefix) { +char **path_strv_resolve_uniq(char **l, const char *prefix) { if (strv_isempty(l)) return l; -if (!path_strv_canonicalize_absolute(l, prefix)) +if (!path_strv_resolve(l, prefix)) return NULL; return strv_uniq(l); diff --git a/src/shared/path-util.h b/src/shared/path-util.h index 6882d78..976d2b2 100644 --- a/src/shared/path-util.h +++ b/src/shared/path-util.h @@ -47,8 +47,8 @@ char* path_startswith(const char *path, const char *prefix) _pure_; bool path_equal(const char *a, const char *b) _pure_; char** path_strv_make_absolute_cwd(char **l); -char** path_strv_canonicalize_absolute(char **l, const char *prefix); -char** path_strv_canonicalize_absolute_uniq(char **l, const char *prefix); +char** path_strv_resolve(char **l, const char *prefix); +char** path_strv_resolve_uniq(char **l, const char *prefix); int path_is_mount_point(const char *path, bool allow_symlink); int path_is_read_only_fs(const char *path); diff --git a/src/shared/util.c b/src/shared/util.c index 34e9176..c1e1f9f 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -5679,7 +5679,7 @@ static int search_and_fopen_internal(const char *path, const char *mode, const c assert(mode); assert(_f); -if (!path_strv_canonicalize_absolute_uniq(search, root)) +if (!path_strv_resolve_uniq(search, root)) return -ENOMEM; STRV_FOREACH(i, search) { -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/5] shared: rename path_strv_canonicalize_absolute functions
On Thu, Jun 19, 2014 at 9:14 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Thu, Jun 19, 2014 at 07:07:02PM -0700, Michael Marineau wrote: Since 12ed81d9 path_strv_canonicalize_absolute leaves the search list relative to the given root directory instead of resolving paths to their true location as the name implies. To better reflect this behavior rename to the less strongly worded path_strv_resolve. All five applied! Thanks! I lost track a bit: are all --root support issues fixed, or are you aware of additional things which need fixing? This patch series covered all the issues I know of. :) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] new user/group population on bootup
On Sun, Jun 15, 2014 at 2:56 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 13.06.14 12:35, Michael Marineau (michael.marin...@coreos.com) wrote: As a side note, regardless of whether an empty /etc is actually viable or not the more packages that support gracefully dealing with configuration in both /etc and /usr the fewer files there will be in /etc that are in the awkward position of being managed by both users and packages. rpm and dpkg's behavior of just picking one and backing up the other or gentoo's scheme of forcing users to wade through 3-way diffs are all pretty terrible. If packages did their best to avoid installing files to /etc, giving users exclusive control over that space, everyone wins. :) BTW: given that there's now at least Colin, Kay, me, and CoreOS working on getting empty /etc working, can we at least try to agree where the vendor versions of the files should be? I am kinda voting for /usr/share/etc, and this is prime bike shedding material, but we should try to get some consensus there what we are pushing for, especially regarding prospects to maybe get this into RPM, to always implicitly place a copy of the config files there... For CoreOS I've been using /usr/share/pkg for most things with the exception of some stuff in /usr/share/baselayout simply because that's the name of the basic filesystem layout package we inherited from Gentoo. Using /usr/share/etc sounds good to me and we can easily switch to that for common shared data/conf files. /usr/share/pkg should probably be preferred where possible. So the default sh-compatible copy of /etc/profile may come from /usr/share/etc but the default global bashrc should probably be in /usr/share/bash. That follows the existing pattern of /usr/share so it doesn't become a random choice where default configuration files land. That said I don't really care that much so if it is easier to make /usr/share/etc a strait-up mirror of /etc I'm not going to fuss about it. :) As a side note there is already that /usr/share/misc but perhaps best to leave that alone. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] new user/group population on bootup
On Fri, Jun 13, 2014 at 6:37 AM, Colin Walters walt...@verbum.org wrote: On Fri, Jun 13, 2014, at 05:36 AM, Colin Walters wrote: My high level takeaway right now is that this looks OK for nspawn containers, but it's not clear to me it's viable or right for the host OS, at least for general purpose systems. That was wrongly stated - basically I'm just skeptical of seeing an empty /etc on general purpose systems in the next few years, at least without lots of non-upstream patches and hacks. A good example is this: https://github.com/coreos/baselayout/commit/98c6211c45c8c53d84df1657776314cc5f5f9ec1 For what its worth, in my efforts to make CoreOS boot with a completely empty root filesystem I found that the changes required were usually not too dramatic. Fixing many packages, like sudo, just amounted to shipping different config files and adding --sysconfdir=/usr/share to ./configure at build time. For dbus it only takes that configure option plus a update to session.conf and system.conf to support both /usr/share/dbus-1 and /etc/dbus-1 for configs: https://github.com/coreos/coreos-overlay/blob/master/sys-apps/dbus/files/dbus-1.6.x-add-explicit-etc-path.patch Where new code is required it is similarly not very complicated, to support reading account information, services, hosts, and other nss data files we have a small project that is just a strait-up copy of the files nss module from glibc with some hard coded paths changed to configurable macros. https://github.com/coreos/nss-altfiles So working through all these details is a damn tedious task and I don't doubt it could be years before we and others get around to getting good support for reading configuration from both /usr and /etc into upstream packages. The good news is that while we bang out the best way to implement this the changes aren't any weirder than the patches and hacks that distros typically have to do when integrating packages. As a side note, regardless of whether an empty /etc is actually viable or not the more packages that support gracefully dealing with configuration in both /etc and /usr the fewer files there will be in /etc that are in the awkward position of being managed by both users and packages. rpm and dpkg's behavior of just picking one and backing up the other or gentoo's scheme of forcing users to wade through 3-way diffs are all pretty terrible. If packages did their best to avoid installing files to /etc, giving users exclusive control over that space, everyone wins. :) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shared: fix searching for configs in alternate roots
Commit 12ed81d9 changed path_strv_canonicalize_absolute's behavior to return a directory list without the root prefix if one was given but did not update other users of the function to the new behavior. This broke the --root option in systemd-tmpfiles, a regression in v213. To better reflect that path_strv_canonicalize_absolute does not return fully resolved paths any more as canonicalize may imply it is now simply called path_strv_cleanup. --- Resending this patch since it didn't make it in for v314. :( src/shared/conf-files.c | 18 +- src/shared/path-lookup.c | 6 +++--- src/shared/path-util.c | 6 +++--- src/shared/path-util.h | 4 ++-- src/shared/util.c| 7 +-- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c index 5201782..6f1dc7f 100644 --- a/src/shared/conf-files.c +++ b/src/shared/conf-files.c @@ -37,10 +37,18 @@ #include hashmap.h #include conf-files.h -static int files_add(Hashmap *h, const char *dirpath, const char *suffix) { +static int files_add(Hashmap *h, const char *dirpath, const char *suffix, const char *root) { _cleanup_closedir_ DIR *dir = NULL; +_cleanup_free_ char *fullpath = NULL; -dir = opendir(dirpath); +if (root) +fullpath = strappend(root, dirpath); +else +fullpath = strdup(dirpath); +if (!fullpath) +return -ENOMEM; + +dir = opendir(fullpath); if (!dir) { if (errno == ENOENT) return 0; @@ -63,7 +71,7 @@ static int files_add(Hashmap *h, const char *dirpath, const char *suffix) { if (!dirent_is_file_with_suffix(de, suffix)) continue; -p = strjoin(dirpath, /, de-d_name, NULL); +p = strjoin(fullpath, /, de-d_name, NULL); if (!p) return -ENOMEM; @@ -100,7 +108,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const assert(suffix); /* This alters the dirs string array */ -if (!path_strv_canonicalize_absolute_uniq(dirs, root)) +if (!path_strv_cleanup_uniq(dirs, root)) return -ENOMEM; fh = hashmap_new(string_hash_func, string_compare_func); @@ -108,7 +116,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const return -ENOMEM; STRV_FOREACH(p, dirs) { -r = files_add(fh, *p, suffix); +r = files_add(fh, *p, suffix, root); if (r == -ENOMEM) { hashmap_free_free(fh); return r; diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index e072fd6..1a497f9 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -284,7 +284,7 @@ int lookup_paths_init( } } -if (!path_strv_canonicalize_absolute_uniq(p-unit_path, root_dir)) +if (!path_strv_cleanup_uniq(p-unit_path, root_dir)) return -ENOMEM; if (!strv_isempty(p-unit_path)) { @@ -338,10 +338,10 @@ int lookup_paths_init( return -ENOMEM; } -if (!path_strv_canonicalize_absolute_uniq(p-sysvinit_path, root_dir)) +if (!path_strv_cleanup_uniq(p-sysvinit_path, root_dir)) return -ENOMEM; -if (!path_strv_canonicalize_absolute_uniq(p-sysvrcnd_path, root_dir)) +if (!path_strv_cleanup_uniq(p-sysvrcnd_path, root_dir)) return -ENOMEM; if (!strv_isempty(p-sysvinit_path)) { diff --git a/src/shared/path-util.c b/src/shared/path-util.c index 5863429..37490be 100644 --- a/src/shared/path-util.c +++ b/src/shared/path-util.c @@ -238,7 +238,7 @@ char **path_strv_make_absolute_cwd(char **l) { return l; } -char **path_strv_canonicalize_absolute(char **l, const char *prefix) { +char **path_strv_cleanup(char **l, const char *prefix) { char **s; unsigned k = 0; bool enomem = false; @@ -323,12 +323,12 @@ char **path_strv_canonicalize_absolute(char **l, const char *prefix) { return l; } -char **path_strv_canonicalize_absolute_uniq(char **l, const char *prefix) { +char **path_strv_cleanup_uniq(char **l, const char *prefix) { if (strv_isempty(l)) return l; -if (!path_strv_canonicalize_absolute(l, prefix)) +if (!path_strv_cleanup(l, prefix)) return NULL; return strv_uniq(l); diff --git a/src/shared/path-util.h b/src/shared/path-util.h index 6882d78..b523bcc 100644 --- a/src/shared/path-util.h +++ b/src/shared/path-util.h @@ -47,8 +47,8 @@ char* path_startswith(const char *path, const char *prefix) _pure_; bool path_equal(const char *a, const
[systemd-devel] [PATCH] shared: fix searching for configs in alternate roots
Commit 12ed81d9 changed path_strv_canonicalize_absolute's behavior to return a directory list without the root prefix if one was given but did not update other users of the function to the new behavior. This broke the --root option in systemd-tmpfiles, a regression in v213. To better reflect that path_strv_canonicalize_absolute does not return fully resolved paths any more as canonicalize may imply it is now simply called path_strv_cleanup. --- src/shared/conf-files.c | 18 +- src/shared/path-lookup.c | 6 +++--- src/shared/path-util.c | 6 +++--- src/shared/path-util.h | 4 ++-- src/shared/util.c| 7 +-- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c index 5201782..6f1dc7f 100644 --- a/src/shared/conf-files.c +++ b/src/shared/conf-files.c @@ -37,10 +37,18 @@ #include hashmap.h #include conf-files.h -static int files_add(Hashmap *h, const char *dirpath, const char *suffix) { +static int files_add(Hashmap *h, const char *dirpath, const char *suffix, const char *root) { _cleanup_closedir_ DIR *dir = NULL; +_cleanup_free_ char *fullpath = NULL; -dir = opendir(dirpath); +if (root) +fullpath = strappend(root, dirpath); +else +fullpath = strdup(dirpath); +if (!fullpath) +return -ENOMEM; + +dir = opendir(fullpath); if (!dir) { if (errno == ENOENT) return 0; @@ -63,7 +71,7 @@ static int files_add(Hashmap *h, const char *dirpath, const char *suffix) { if (!dirent_is_file_with_suffix(de, suffix)) continue; -p = strjoin(dirpath, /, de-d_name, NULL); +p = strjoin(fullpath, /, de-d_name, NULL); if (!p) return -ENOMEM; @@ -100,7 +108,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const assert(suffix); /* This alters the dirs string array */ -if (!path_strv_canonicalize_absolute_uniq(dirs, root)) +if (!path_strv_cleanup_uniq(dirs, root)) return -ENOMEM; fh = hashmap_new(string_hash_func, string_compare_func); @@ -108,7 +116,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const return -ENOMEM; STRV_FOREACH(p, dirs) { -r = files_add(fh, *p, suffix); +r = files_add(fh, *p, suffix, root); if (r == -ENOMEM) { hashmap_free_free(fh); return r; diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index e072fd6..1a497f9 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -284,7 +284,7 @@ int lookup_paths_init( } } -if (!path_strv_canonicalize_absolute_uniq(p-unit_path, root_dir)) +if (!path_strv_cleanup_uniq(p-unit_path, root_dir)) return -ENOMEM; if (!strv_isempty(p-unit_path)) { @@ -338,10 +338,10 @@ int lookup_paths_init( return -ENOMEM; } -if (!path_strv_canonicalize_absolute_uniq(p-sysvinit_path, root_dir)) +if (!path_strv_cleanup_uniq(p-sysvinit_path, root_dir)) return -ENOMEM; -if (!path_strv_canonicalize_absolute_uniq(p-sysvrcnd_path, root_dir)) +if (!path_strv_cleanup_uniq(p-sysvrcnd_path, root_dir)) return -ENOMEM; if (!strv_isempty(p-sysvinit_path)) { diff --git a/src/shared/path-util.c b/src/shared/path-util.c index 5863429..37490be 100644 --- a/src/shared/path-util.c +++ b/src/shared/path-util.c @@ -238,7 +238,7 @@ char **path_strv_make_absolute_cwd(char **l) { return l; } -char **path_strv_canonicalize_absolute(char **l, const char *prefix) { +char **path_strv_cleanup(char **l, const char *prefix) { char **s; unsigned k = 0; bool enomem = false; @@ -323,12 +323,12 @@ char **path_strv_canonicalize_absolute(char **l, const char *prefix) { return l; } -char **path_strv_canonicalize_absolute_uniq(char **l, const char *prefix) { +char **path_strv_cleanup_uniq(char **l, const char *prefix) { if (strv_isempty(l)) return l; -if (!path_strv_canonicalize_absolute(l, prefix)) +if (!path_strv_cleanup(l, prefix)) return NULL; return strv_uniq(l); diff --git a/src/shared/path-util.h b/src/shared/path-util.h index 6882d78..b523bcc 100644 --- a/src/shared/path-util.h +++ b/src/shared/path-util.h @@ -47,8 +47,8 @@ char* path_startswith(const char *path, const char *prefix) _pure_; bool path_equal(const char *a, const char *b) _pure_; char** path_strv_make_absolute_cwd(char **l);
Re: [systemd-devel] [PATCH] sd-dhcp-client: Sets broadcast flag to 1
On Wed, May 28, 2014 at 4:13 PM, Camilo Aguilar camilo.agui...@gmail.com wrote: Oh, never mind, there is no rush since we are already custom patching in CoreOS for now. Hey, don't get ahead of yourself. I haven't merged your patch into CoreOS just yet ;-) I'm fine with the patch being a temporary fix as long as we can sort out the final version soon. On Wed, May 28, 2014 at 7:10 PM, Camilo Aguilar camilo.agui...@gmail.com wrote: It makes total sense. I can provide that patch in addition if you like, But, can we merge this change for now so we can fix https://github.com/coreos/bugs/issues/12 and create a new issue to make it more robust? On Wed, May 28, 2014 at 7:03 PM, Tom Gundersen t...@jklm.no wrote: On Wed, May 28, 2014 at 7:43 PM, Camilo Aguilar camilo.agui...@gmail.com wrote: In systems running on hypervisors this flag needs to be set ON, so offers can reach the virtual machines. For more information please refer to this thread in CoreOS: https://github.com/coreos/bugs/issues/12 Signed-off-by: Camilo Aguilar camilo.agui...@gmail.com --- src/libsystemd-network/sd-dhcp-client.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 0300a6b..8f54906 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -286,6 +286,15 @@ static int client_message_init(sd_dhcp_client *client, DHCPPacket **ret, refuse to issue an DHCP lease if 'secs' is set to zero */ packet-dhcp.secs = htobe16(client-secs); +/* RFC2132 section 4.1 + A client that cannot receive unicast IP datagrams until its protocol + software has been configured with an IP address SHOULD set the + BROADCAST bit in the 'flags' field to 1 in any DHCPDISCOVER or + DHCPREQUEST messages that client sends. The BROADCAST bit will + provide a hint to the DHCP server and BOOTP relay agent to broadcast + any messages to the client on the client's subnet. */ +packet-dhcp.flags = htobe16(0x8000); Hm, most clients can and should use unicast, so we should definitely not request broadcast unconditionally. If there really is a situation where we need broadcast, we should detect that and only request broadcast in this case. Do you have any ideas on how to detect the issue documented here with VMware virtual machines? I haven't dug into this bug yet myself so I'm not sure what other DHCP implementations are doing off the top of my head. https://github.com/coreos/bugs/issues/12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] man: note that entire sections can now be ignored
Prefixing a section name with X- will cause it and all of its contents to be silently ignored as of commit 342aea19. --- man/systemd.unit.xml | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 157530b..e903156 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -139,10 +139,12 @@ paraUnit files may contain additional options on top of those listed here. If systemd encounters an unknown option, it will write a warning log message but -continue loading the unit. If an option is prefixed -with optionX-/option, it is ignored completely by -systemd. Applications may use this to include -additional information in the unit files./para +continue loading the unit. If an option or section name +is prefixed with optionX-/option, it is ignored +completely by systemd. Options within an ignored +section do not need the prefix. Applications may use +this to include additional information in the unit +files./para paraBoolean arguments used in unit files can be written in various formats. For positive settings the -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] conf-parser: silently ignore sections starting with X-
This allows external tools to keep additional unit information in a separate section without scaring users with a big warning. --- src/shared/conf-parser.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index d27b1b7..062b15b 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -204,6 +204,7 @@ static int parse_line(const char* unit, bool allow_include, char **section, unsigned *section_line, + bool *section_ignored, char *l, void *userdata) { @@ -266,7 +267,7 @@ static int parse_line(const char* unit, if (sections !nulstr_contains(sections, n)) { -if (!relaxed) +if (!relaxed !startswith(n, X-)) log_syntax(unit, LOG_WARNING, filename, line, EINVAL, Unknown section '%s'. Ignoring., n); @@ -274,10 +275,12 @@ static int parse_line(const char* unit, free(*section); *section = NULL; *section_line = 0; +*section_ignored = true; } else { free(*section); *section = n; *section_line = line; +*section_ignored = false; } return 0; @@ -285,7 +288,7 @@ static int parse_line(const char* unit, if (sections !*section) { -if (!relaxed) +if (!relaxed !*section_ignored) log_syntax(unit, LOG_WARNING, filename, line, EINVAL, Assignment outside of section. Ignoring.); @@ -328,6 +331,7 @@ int config_parse(const char *unit, _cleanup_free_ char *section = NULL, *continuation = NULL; _cleanup_fclose_ FILE *ours = NULL; unsigned line = 0, section_line = 0; +bool section_ignored = false; int r; assert(filename); @@ -399,6 +403,7 @@ int config_parse(const char *unit, allow_include, section, section_line, + section_ignored, p, userdata); free(c); -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] device: Add stub serialization methods to enable job serialization.
On Thu, May 15, 2014 at 4:24 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 06.05.14 19:08, Michael Marineau (michael.marin...@coreos.com) wrote: If a unit type doesn't provide its own serialization methods then none of the generic serialization will happen either. For devices this means jobs used for waiting on device dependencies are dropped during reloads, breaking dependency state that was relying on those jobs. Oh yuck! This is quite some bug, which I figure is the source of quite a few bug reports we had where people were reloading the daemon during the boot process... I commited a different fix now which avoids installing stub callbacks, and simply fixes to the core always serialize the job regardless if the unit has anything else to serialize... Please test! Looks good to me, thanks! --- src/core/device.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/src/core/device.c b/src/core/device.c index 444286e..07c0860 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -130,6 +130,25 @@ static void device_dump(Unit *u, FILE *f, const char *prefix) { prefix, strna(d-sysfs)); } +static int device_serialize(Unit *u, FILE *f, FDSet *fds) { +assert(u); +assert(f); +assert(fds); + +return 0; +} + +static int device_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) { +assert(u); +assert(key); +assert(value); +assert(fds); + +log_debug(Unknown serialization key '%s', key); + +return 0; +} + _pure_ static UnitActiveState device_active_state(Unit *u) { assert(u); @@ -693,6 +712,9 @@ const UnitVTable device_vtable = { .dump = device_dump, +.serialize = device_serialize, +.deserialize_item = device_deserialize_item, + .active_state = device_active_state, .sub_state_to_string = device_sub_state_to_string, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] job: always add waiting jobs to run queue during coldplug.
commit 20a83d7bf was not equivalent to the original bug fix proposed by Michal Sekletar msekl...@redhat.com. The committed version only added the job to the run queue if the job had a timeout, which most jobs do not have. Just re-ordering the code gets us the intended functionality. --- src/core/job.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/job.c b/src/core/job.c index 835cfe1..dc4f441 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1060,15 +1060,15 @@ int job_coldplug(Job *j) { if (r 0) return r; +if (j-state == JOB_WAITING) +job_add_to_run_queue(j); + if (j-begin_usec == 0 || j-unit-job_timeout == 0) return 0; if (j-timer_event_source) j-timer_event_source = sd_event_source_unref(j-timer_event_source); -if (j-state == JOB_WAITING) -job_add_to_run_queue(j); - r = sd_event_add_time( j-manager-event, j-timer_event_source, -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] device: Add stub serialization methods to enable job serialization.
If a unit type doesn't provide its own serialization methods then none of the generic serialization will happen either. For devices this means jobs used for waiting on device dependencies are dropped during reloads, breaking dependency state that was relying on those jobs. --- src/core/device.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/src/core/device.c b/src/core/device.c index 444286e..07c0860 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -130,6 +130,25 @@ static void device_dump(Unit *u, FILE *f, const char *prefix) { prefix, strna(d-sysfs)); } +static int device_serialize(Unit *u, FILE *f, FDSet *fds) { +assert(u); +assert(f); +assert(fds); + +return 0; +} + +static int device_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) { +assert(u); +assert(key); +assert(value); +assert(fds); + +log_debug(Unknown serialization key '%s', key); + +return 0; +} + _pure_ static UnitActiveState device_active_state(Unit *u) { assert(u); @@ -693,6 +712,9 @@ const UnitVTable device_vtable = { .dump = device_dump, +.serialize = device_serialize, +.deserialize_item = device_deserialize_item, + .active_state = device_active_state, .sub_state_to_string = device_sub_state_to_string, -- 1.8.5.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] shared: add root argument to search_and_fopen
This adds the same root argument to search_and_fopen that conf_files_list already has. Tools that use those two functions as a pair can now be easily modified to load configuration files from an alternate root filesystem tree. --- src/binfmt/binfmt.c | 2 +- src/modules-load/modules-load.c | 2 +- src/shared/util.c | 12 ++-- src/shared/util.h | 4 ++-- src/sysctl/sysctl.c | 2 +- src/tmpfiles/tmpfiles.c | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c index a1877c4..9fc5d4e 100644 --- a/src/binfmt/binfmt.c +++ b/src/binfmt/binfmt.c @@ -86,7 +86,7 @@ static int apply_file(const char *path, bool ignore_enoent) { assert(path); -r = search_and_fopen_nulstr(path, re, conf_file_dirs, f); +r = search_and_fopen_nulstr(path, re, NULL, conf_file_dirs, f); if (r 0) { if (ignore_enoent r == -ENOENT) return 0; diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index 49b153d..ecb84da 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -145,7 +145,7 @@ static int apply_file(struct kmod_ctx *ctx, const char *path, bool ignore_enoent assert(ctx); assert(path); -r = search_and_fopen_nulstr(path, re, conf_file_dirs, f); +r = search_and_fopen_nulstr(path, re, NULL, conf_file_dirs, f); if (r 0) { if (ignore_enoent r == -ENOENT) return 0; diff --git a/src/shared/util.c b/src/shared/util.c index 9e8cd54..8b8d2fb 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -5668,14 +5668,14 @@ int on_ac_power(void) { return found_online || !found_offline; } -static int search_and_fopen_internal(const char *path, const char *mode, char **search, FILE **_f) { +static int search_and_fopen_internal(const char *path, const char *mode, const char *root, char **search, FILE **_f) { char **i; assert(path); assert(mode); assert(_f); -if (!path_strv_canonicalize_absolute_uniq(search, NULL)) +if (!path_strv_canonicalize_absolute_uniq(search, root)) return -ENOMEM; STRV_FOREACH(i, search) { @@ -5699,7 +5699,7 @@ static int search_and_fopen_internal(const char *path, const char *mode, char ** return -ENOENT; } -int search_and_fopen(const char *path, const char *mode, const char **search, FILE **_f) { +int search_and_fopen(const char *path, const char *mode, const char *root, const char **search, FILE **_f) { _cleanup_strv_free_ char **copy = NULL; assert(path); @@ -5722,10 +5722,10 @@ int search_and_fopen(const char *path, const char *mode, const char **search, FI if (!copy) return -ENOMEM; -return search_and_fopen_internal(path, mode, copy, _f); +return search_and_fopen_internal(path, mode, root, copy, _f); } -int search_and_fopen_nulstr(const char *path, const char *mode, const char *search, FILE **_f) { +int search_and_fopen_nulstr(const char *path, const char *mode, const char *root, const char *search, FILE **_f) { _cleanup_strv_free_ char **s = NULL; if (path_is_absolute(path)) { @@ -5744,7 +5744,7 @@ int search_and_fopen_nulstr(const char *path, const char *mode, const char *sear if (!s) return -ENOMEM; -return search_and_fopen_internal(path, mode, s, _f); +return search_and_fopen_internal(path, mode, root, s, _f); } char *strextend(char **x, ...) { diff --git a/src/shared/util.h b/src/shared/util.h index 81831e2..e99f8d1 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -696,8 +696,8 @@ char *strip_tab_ansi(char **p, size_t *l); int on_ac_power(void); -int search_and_fopen(const char *path, const char *mode, const char **search, FILE **_f); -int search_and_fopen_nulstr(const char *path, const char *mode, const char *search, FILE **_f); +int search_and_fopen(const char *path, const char *mode, const char *root, const char **search, FILE **_f); +int search_and_fopen_nulstr(const char *path, const char *mode, const char *root, const char *search, FILE **_f); #define FOREACH_LINE(line, f, on_error) \ for (;;)\ diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index 76efacb..8868732 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -123,7 +123,7 @@ static int parse_file(Hashmap *sysctl_options, const char *path, bool ignore_eno assert(path); -r = search_and_fopen_nulstr(path, re, conf_file_dirs, f); +r = search_and_fopen_nulstr(path, re, NULL, conf_file_dirs, f); if (r 0) { if (ignore_enoent r == -ENOENT) return 0; diff --git
[systemd-devel] [PATCH 3/3] tmpfiles: Add --root to the man page.
--- man/systemd-tmpfiles.xml | 8 1 file changed, 8 insertions(+) diff --git a/man/systemd-tmpfiles.xml b/man/systemd-tmpfiles.xml index 0b62640..193acb7 100644 --- a/man/systemd-tmpfiles.xml +++ b/man/systemd-tmpfiles.xml @@ -152,6 +152,14 @@ prefix. This option can be specified multiple times./para/listitem /varlistentry +varlistentry +termoption--root=ROOT/option/term +listitemparaTakes a directory path +as an argument. All paths will be +prefixed with the given alternate ROOT +path, including config search paths. +/para/listitem +/varlistentry xi:include href=standard-options.xml xpointer=help / xi:include href=standard-options.xml xpointer=version / -- 1.8.3.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/3] tmpfiles: Add --root option to operate on an alternate fs tree.
This makes it possible to initialize or cleanup an arbitrary filesystem hierarchy in the same way that it would be during system boot. --- src/tmpfiles/tmpfiles.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 3684289..4ce35b5 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -111,6 +111,7 @@ static bool arg_boot = false; static char **include_prefixes = NULL; static char **exclude_prefixes = NULL; +static char *arg_root = NULL; static const char conf_file_dirs[] = /etc/tmpfiles.d\0 @@ -1188,6 +1189,15 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (!should_include_path(i-path)) return 0; +if (arg_root) { +char *p = strjoin(arg_root, i-path, NULL); +if (!p) +return log_oom(); + +free(i-path); +i-path = p; +} + if (user !streq(user, -)) { const char *u = user; @@ -1277,7 +1287,8 @@ static int help(void) { --remove Remove marked files/directories\n --boot Execute actions only safe at boot\n --prefix=PATH Only apply rules that apply to paths with the specified prefix\n ---exclude-prefix=PATH Ignore rules that apply to paths with the specified prefix\n, +--exclude-prefix=PATH Ignore rules that apply to paths with the specified prefix\n +--root=PATHOperate on an alternate filesystem root\n, program_invocation_short_name); return 0; @@ -1293,6 +1304,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_BOOT, ARG_PREFIX, ARG_EXCLUDE_PREFIX, +ARG_ROOT, }; static const struct option options[] = { @@ -1304,6 +1316,7 @@ static int parse_argv(int argc, char *argv[]) { { boot, no_argument, NULL, ARG_BOOT }, { prefix, required_argument, NULL, ARG_PREFIX }, { exclude-prefix, required_argument, NULL, ARG_EXCLUDE_PREFIX }, +{ root, required_argument, NULL, ARG_ROOT }, {} }; @@ -1350,6 +1363,13 @@ static int parse_argv(int argc, char *argv[]) { return log_oom(); break; +case ARG_ROOT: +arg_root = path_make_absolute_cwd(optarg); +if (!arg_root) +return log_oom(); +path_kill_slashes(arg_root); +break; + case '?': return -EINVAL; @@ -1376,7 +1396,7 @@ static int read_config_file(const char *fn, bool ignore_enoent) { assert(fn); -r = search_and_fopen_nulstr(fn, re, NULL, conf_file_dirs, f); +r = search_and_fopen_nulstr(fn, re, arg_root, conf_file_dirs, f); if (r 0) { if (ignore_enoent r == -ENOENT) return 0; @@ -1477,7 +1497,7 @@ int main(int argc, char *argv[]) { _cleanup_strv_free_ char **files = NULL; char **f; -r = conf_files_list_nulstr(files, .conf, NULL, conf_file_dirs); +r = conf_files_list_nulstr(files, .conf, arg_root, conf_file_dirs); if (r 0) { log_error(Failed to enumerate tmpfiles.d files: %s, strerror(-r)); goto finish; @@ -1508,6 +1528,7 @@ finish: free(include_prefixes); free(exclude_prefixes); +free(arg_root); set_free_free(unix_sockets); -- 1.8.3.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shared: include root when canonicalizing conf paths
The conf_files_list family accepts an alternate root path to prefix all directories in the list but path_strv_canonicalize_uniq doesn't use it. This results in the suspicious behavior of resolving directory symlinks based on the contents of / instead of the alternate root. This adds a prefix argument to path_strv_canonicalize which will now prepend the prefix, if given, to every path in the list. To avoid answering what a relative path means when called with a root prefix path_strv_canonicalize is now path_strv_canonicalize_absolute and only considers absolute paths. Fortunately all users of already call path_strv_canonicalize with a list of absolute paths. --- Note: I'm posting this mostly just to point out this odd behavior since I couldn't decide if this patch really is the best way to fix it or not. So feedback welcome :) src/shared/conf-files.c | 10 +++--- src/shared/path-lookup.c | 6 +++--- src/shared/path-util.c | 11 +++ src/shared/path-util.h | 4 ++-- src/shared/util.c| 2 +- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c index c86bb03..5201782 100644 --- a/src/shared/conf-files.c +++ b/src/shared/conf-files.c @@ -37,12 +37,8 @@ #include hashmap.h #include conf-files.h -static int files_add(Hashmap *h, const char *root, const char *path, const char *suffix) { +static int files_add(Hashmap *h, const char *dirpath, const char *suffix) { _cleanup_closedir_ DIR *dir = NULL; -_cleanup_free_ char *dirpath = NULL; - -if (asprintf(dirpath, %s%s, root ? root : , path) 0) -return -ENOMEM; dir = opendir(dirpath); if (!dir) { @@ -104,7 +100,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const assert(suffix); /* This alters the dirs string array */ -if (!path_strv_canonicalize_uniq(dirs)) +if (!path_strv_canonicalize_absolute_uniq(dirs, root)) return -ENOMEM; fh = hashmap_new(string_hash_func, string_compare_func); @@ -112,7 +108,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const return -ENOMEM; STRV_FOREACH(p, dirs) { -r = files_add(fh, root, *p, suffix); +r = files_add(fh, *p, suffix); if (r == -ENOMEM) { hashmap_free_free(fh); return r; diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index e2ca942..63af43c 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -275,7 +275,7 @@ int lookup_paths_init( } } -if (!path_strv_canonicalize(p-unit_path)) +if (!path_strv_canonicalize_absolute(p-unit_path, NULL)) return -ENOMEM; strv_uniq(p-unit_path); @@ -331,10 +331,10 @@ int lookup_paths_init( return -ENOMEM; } -if (!path_strv_canonicalize(p-sysvinit_path)) +if (!path_strv_canonicalize_absolute(p-sysvinit_path, NULL)) return -ENOMEM; -if (!path_strv_canonicalize(p-sysvrcnd_path)) +if (!path_strv_canonicalize_absolute(p-sysvrcnd_path, NULL)) return -ENOMEM; strv_uniq(p-sysvinit_path); diff --git a/src/shared/path-util.c b/src/shared/path-util.c index fc42a70..a0925e4 100644 --- a/src/shared/path-util.c +++ b/src/shared/path-util.c @@ -153,7 +153,7 @@ char **path_strv_make_absolute_cwd(char **l) { return l; } -char **path_strv_canonicalize(char **l) { +char **path_strv_canonicalize_absolute(char **l, const char *prefix) { char **s; unsigned k = 0; bool enomem = false; @@ -168,7 +168,10 @@ char **path_strv_canonicalize(char **l) { STRV_FOREACH(s, l) { char *t, *u; -t = path_make_absolute_cwd(*s); +if (!path_is_absolute(*s)) +continue; + +t = strjoin(prefix ? prefix : , *s, NULL); free(*s); *s = NULL; @@ -203,11 +206,11 @@ char **path_strv_canonicalize(char **l) { return l; } -char **path_strv_canonicalize_uniq(char **l) { +char **path_strv_canonicalize_absolute_uniq(char **l, const char *prefix) { if (strv_isempty(l)) return l; -if (!path_strv_canonicalize(l)) +if (!path_strv_canonicalize_absolute(l, prefix)) return NULL; return strv_uniq(l); diff --git a/src/shared/path-util.h b/src/shared/path-util.h index 178bed5..2b8ea02 100644 --- a/src/shared/path-util.h +++ b/src/shared/path-util.h @@ -46,8 +46,8 @@ char* path_startswith(const char *path, const char *prefix) _pure_; bool path_equal(const char *a, const char *b) _pure_; char**
[systemd-devel] [PATCH] tmpfiles: Add free that was missed in 5c795114
--- src/tmpfiles/tmpfiles.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index bff9527..ae74af9 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1509,6 +1509,7 @@ finish: hashmap_free(globs); strv_free(include_prefixes); +strv_free(exclude_prefixes); set_free_free(unix_sockets); -- 1.8.5.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH resend] getty-generator: Enable getty on all active serial consoles.
On Tue, Sep 10, 2013 at 8:41 AM, Lennart Poettering lenn...@poettering.netwrote: On Wed, 28.08.13 13:12, Michael Marineau (michael.marin...@coreos.com) wrote: This enables a getty on active kernel consoles even when they are not the last one specified on the kernel command line and mapped to /dev/console. Now the order console=ttyS0 console=tty0 works in addition to console=tty0 console=ttyS0. Hmm, so when we added the generator initially we though about this and came to the conclusion that it might be risky to spawn gettys on all configured console outputs and we should limit the magic logic to the primary console only. The kernel already makes a distinction between the primary console, and all others (the latter only receive logs, but you cannot read from them via /dev/console iirc), and so we propagated that distinction into systemd, too. I can totally see that this is confusing though, as nobody remembers the right ordering... And in my case there isn't a truly correct ordering either because I'm creating filesystem images that need to boot on a wide variety of platforms including QEMU which gives the user a VGA console most of the time but a serial console when using the -nographic option. The bootloader doesn't know the difference. Kay, do you have an opinion about this? --- src/getty-generator/getty-generator.c | 37 ++- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index 4b7a60a..6c93806 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -122,33 +122,42 @@ int main(int argc, char *argv[]) { } if (read_one_line_file(/sys/class/tty/console/active, active) = 0) { -const char *tty; - -tty = strrchr(active, ' '); -if (tty) -tty ++; -else -tty = active; - -/* Automatically add in a serial getty on the kernel - * console */ -if (isempty(tty) || tty_is_vc(tty)) -free(active); -else { +char *w, *state; +size_t l; + +/* Automatically add in a serial getty on all active + * kernel consoles */ +FOREACH_WORD(w, l, active, state) { +char *tty; int k; +tty = strndup(w, l); +if (!tty) { +log_oom(); +free(active); +r = EXIT_FAILURE; +goto finish; +} + +if (isempty(tty) || tty_is_vc(tty)) { +free(tty); +continue; +} + /* We assume that gettys on virtual terminals are * started via manual configuration and do this magic * only for non-VC terminals. */ k = add_serial_getty(tty); -free(active); if (k 0) { +free(tty); +free(active); r = EXIT_FAILURE; goto finish; } } +free(active); } /* Automatically add in a serial getty on the first Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH resend] getty-generator: Enable getty on all active serial consoles.
On Sep 10, 2013 6:41 PM, Jan Engelhardt jeng...@inai.de wrote: On Tuesday 2013-09-10 17:41, Lennart Poettering wrote: On Wed, 28.08.13 13:12, Michael Marineau (michael.marin...@coreos.com) wrote: This enables a getty on active kernel consoles even when they are not the last one specified on the kernel command line and mapped to /dev/console. Now the order console=ttyS0 console=tty0 works in addition to console=tty0 console=ttyS0. The kernel already makes a distinction between the primary console, and all others (the latter only receive logs, but you cannot read from them via /dev/console iirc), and so we propagated that distinction into systemd, too. I can totally see that this is confusing though, as nobody remembers the right ordering... Though console= just follows the standard principle of later values override earlier ones, if nobody can remember the ordering, perhaps the kernel should be taught a suboption to explicitly specify the primary console. Think console.primary=ttyS0 console=tty0 console=tty0 console.primary=ttyS0 That doesn't answer the question whether a getty should be started on secondary consoles though. My surprise was that the generator doesn't already do that, not how the primary console that gets mapped to /dev/console is chosen. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH resend] getty-generator: Enable getty on all active serial consoles.
This enables a getty on active kernel consoles even when they are not the last one specified on the kernel command line and mapped to /dev/console. Now the order console=ttyS0 console=tty0 works in addition to console=tty0 console=ttyS0. --- src/getty-generator/getty-generator.c | 37 ++- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index 4b7a60a..6c93806 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -122,33 +122,42 @@ int main(int argc, char *argv[]) { } if (read_one_line_file(/sys/class/tty/console/active, active) = 0) { -const char *tty; - -tty = strrchr(active, ' '); -if (tty) -tty ++; -else -tty = active; - -/* Automatically add in a serial getty on the kernel - * console */ -if (isempty(tty) || tty_is_vc(tty)) -free(active); -else { +char *w, *state; +size_t l; + +/* Automatically add in a serial getty on all active + * kernel consoles */ +FOREACH_WORD(w, l, active, state) { +char *tty; int k; +tty = strndup(w, l); +if (!tty) { +log_oom(); +free(active); +r = EXIT_FAILURE; +goto finish; +} + +if (isempty(tty) || tty_is_vc(tty)) { +free(tty); +continue; +} + /* We assume that gettys on virtual terminals are * started via manual configuration and do this magic * only for non-VC terminals. */ k = add_serial_getty(tty); -free(active); if (k 0) { +free(tty); +free(active); r = EXIT_FAILURE; goto finish; } } +free(active); } /* Automatically add in a serial getty on the first -- 1.8.1.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] getty-generator: Enable getty on all active serial consoles.
Greetings, sent this last week but didn't get any feedback. Is this change acceptable for systemd or is there a particular reason for not starting gettys on all active consoles? On Fri, Aug 16, 2013 at 8:28 PM, Michael Marineau michael.marin...@coreos.com wrote: This enables a getty on active kernel consoles even when they are not the last one specified on the kernel command line and mapped to /dev/console. Now the order console=ttyS0 console=tty0 works in addition to console=tty0 console=ttyS0. --- I'm not sure if there is a particular reason for the more limited existing behavior but it took me by surprise. src/getty-generator/getty-generator.c | 37 ++- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index 4b7a60a..6c93806 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -122,33 +122,42 @@ int main(int argc, char *argv[]) { } if (read_one_line_file(/sys/class/tty/console/active, active) = 0) { -const char *tty; - -tty = strrchr(active, ' '); -if (tty) -tty ++; -else -tty = active; - -/* Automatically add in a serial getty on the kernel - * console */ -if (isempty(tty) || tty_is_vc(tty)) -free(active); -else { +char *w, *state; +size_t l; + +/* Automatically add in a serial getty on all active + * kernel consoles */ +FOREACH_WORD(w, l, active, state) { +char *tty; int k; +tty = strndup(w, l); +if (!tty) { +log_oom(); +free(active); +r = EXIT_FAILURE; +goto finish; +} + +if (isempty(tty) || tty_is_vc(tty)) { +free(tty); +continue; +} + /* We assume that gettys on virtual terminals are * started via manual configuration and do this magic * only for non-VC terminals. */ k = add_serial_getty(tty); -free(active); if (k 0) { +free(tty); +free(active); r = EXIT_FAILURE; goto finish; } } +free(active); } /* Automatically add in a serial getty on the first -- 1.8.1.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] getty-generator: Enable getty on all active serial consoles.
This enables a getty on active kernel consoles even when they are not the last one specified on the kernel command line and mapped to /dev/console. Now the order console=ttyS0 console=tty0 works in addition to console=tty0 console=ttyS0. --- I'm not sure if there is a particular reason for the more limited existing behavior but it took me by surprise. src/getty-generator/getty-generator.c | 37 ++- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index 4b7a60a..6c93806 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -122,33 +122,42 @@ int main(int argc, char *argv[]) { } if (read_one_line_file(/sys/class/tty/console/active, active) = 0) { -const char *tty; - -tty = strrchr(active, ' '); -if (tty) -tty ++; -else -tty = active; - -/* Automatically add in a serial getty on the kernel - * console */ -if (isempty(tty) || tty_is_vc(tty)) -free(active); -else { +char *w, *state; +size_t l; + +/* Automatically add in a serial getty on all active + * kernel consoles */ +FOREACH_WORD(w, l, active, state) { +char *tty; int k; +tty = strndup(w, l); +if (!tty) { +log_oom(); +free(active); +r = EXIT_FAILURE; +goto finish; +} + +if (isempty(tty) || tty_is_vc(tty)) { +free(tty); +continue; +} + /* We assume that gettys on virtual terminals are * started via manual configuration and do this magic * only for non-VC terminals. */ k = add_serial_getty(tty); -free(active); if (k 0) { +free(tty); +free(active); r = EXIT_FAILURE; goto finish; } } +free(active); } /* Automatically add in a serial getty on the first -- 1.8.1.5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel