Re: [systemd-devel] [RFC][PATCH] udev: net_id - support multi-function, multi-port enpo* device names

2015-07-15 Thread Michael Marineau
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

2015-06-02 Thread Michael Marineau
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

2015-05-20 Thread Michael Marineau
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

2015-05-15 Thread Michael Marineau
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

2015-05-15 Thread Michael Marineau
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

2015-05-15 Thread Michael Marineau
(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

2015-05-15 Thread Michael Marineau
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

2015-04-02 Thread Michael Marineau
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

2015-03-13 Thread Michael Marineau
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

2015-03-13 Thread Michael Marineau
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

2015-01-28 Thread Michael Marineau
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

2015-01-15 Thread Michael Marineau
---
 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

2015-01-15 Thread Michael Marineau
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

2014-12-08 Thread Michael Marineau
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

2014-12-08 Thread Michael Marineau
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

2014-12-06 Thread Michael Marineau
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

2014-12-04 Thread Michael Marineau
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

2014-12-02 Thread Michael Marineau
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

2014-11-06 Thread Michael Marineau
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

2014-11-06 Thread Michael Marineau
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.

2014-09-15 Thread Michael Marineau
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

2014-08-14 Thread Michael Marineau
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

2014-08-04 Thread Michael Marineau
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

2014-07-29 Thread Michael Marineau
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

2014-07-29 Thread Michael Marineau
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

2014-07-29 Thread Michael Marineau
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

2014-07-22 Thread Michael Marineau
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

2014-07-21 Thread Michael Marineau
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

2014-07-03 Thread Michael Marineau
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

2014-06-20 Thread Michael Marineau
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

2014-06-19 Thread Michael Marineau
---
 .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

2014-06-19 Thread Michael Marineau
---
 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

2014-06-19 Thread Michael Marineau
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

2014-06-19 Thread Michael Marineau
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

2014-06-19 Thread Michael Marineau
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

2014-06-19 Thread Michael Marineau
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

2014-06-15 Thread Michael Marineau
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

2014-06-13 Thread Michael Marineau
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

2014-06-11 Thread Michael Marineau
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

2014-05-29 Thread Michael Marineau
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

2014-05-28 Thread Michael Marineau
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

2014-05-17 Thread Michael Marineau
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-

2014-05-16 Thread Michael Marineau
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.

2014-05-15 Thread Michael Marineau
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.

2014-05-06 Thread Michael Marineau
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.

2014-05-06 Thread Michael Marineau
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

2014-03-13 Thread Michael Marineau
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.

2014-03-13 Thread Michael Marineau
---
 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.

2014-03-13 Thread Michael Marineau
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

2014-01-31 Thread Michael Marineau
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

2014-01-30 Thread Michael Marineau
---
 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.

2013-09-10 Thread Michael Marineau
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.

2013-09-10 Thread Michael Marineau
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.

2013-08-28 Thread Michael Marineau
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.

2013-08-22 Thread Michael Marineau
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.

2013-08-16 Thread Michael Marineau
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