Re: [systemd-devel] ipv6 configuration error

2016-05-27 Thread Tom Gundersen
On Fri, May 27, 2016 at 5:11 PM, Michał Zegan 
wrote:

> Hello, I have encountered a very interesting problem: I have a wired
> ipv4 network configured via systemd-network, and an ipv6 sit tunnel.
> The problem is it does not start. trying to restart systemd-networkd
> gives the error like both local and remote addresses of the tunnel are
> incompatible. I assume that means there is no interface matching those
> ipv4 addresses. I know that eth0 interface is configured *after* this
> sit one, and it is configured via dhcp, so it is quite possible. What is
> happening?
>

Hi Michał,

Could you configure it manually (using ip(8) or something) and paste the
output of `ip link` and `ip addr`, and also attach the networkd
configuration you tried to use (would be great of course if you could make
it as minimal as possible, but still showing your problem).

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] network unit Match by router advertisement?

2016-05-20 Thread Tom Gundersen
On Wed, May 11, 2016 at 7:31 PM, Lennart Poettering 
wrote:

> On Wed, 11.05.16 11:32, Brian Kroth (bpkr...@gmail.com) wrote:
>
> > Hi again all,
> >
> > TL;DR: would it be possible (or make sense) to have systemd Match rules
> for
> > network units that could match on some artifact of the network the link
> is
> > attached to like vlan tag, router advertisement, wireless access point or
> > gateway mac, etc.?
>
> Well, .network files contain the definition how to set up a network
> interface, i.e. how to place it into UP state so that packets can be
> received and how to configure IP routing so that communication further
> on works. Hence: it uses relatively static properties of the device
> that are already available when the device is offline, to find the
> right .network file to read the dynamic configuration to apply in
> order to put it online. The router advertisment info and things like
> the gateway mac are pieces of information that are only available when
> the network is already up, when the network configuration is already
> applied. Hence using that as match for the configuration can't work:
> at the time we could use that information we already would have had to
> apply it. And if we don't apply it, we would never get the information
> to acquire...
>
> The VLAN tag is a different case though: it's assigned when the
> VLAN networkd device is created, and configured in the .netdev
> configuration file for that. Thus, it's already set the moment the
> network device pops up, and it could be used nicely for the
> matching. So yupp, added a MatchVLANId= or so, might make
> sense. Please file an RFE issue on github about this, if you'd like to
> see this implemented.
>
> Matching by AP could work. Iirc today's WLAN drivers actually will
> create virtual links for the network you connect to, and the ESSID for
> each would be set before networkd would take notice of it, hence this
> is probably something we could do. Note however, that networkd does
> not interface with the WLAN stack at all at this point, a WLAN device
> is treated like any other Ethernet device atm


It was always my intention to extend the Match logic to be able to do this
kind of matching on the environment. But as Lennart says, it would have to
be only based on things we can passively observe. I think it would possibly
be acceptable to eagerly UP an interface if it matches all but the
"environment" matches (and that is essentially how WLAN must work I guess,
though the UP might in that case be done by someone other than us), thought
that stuff definitely requires some thinking. I agree that VLANId is
totally fine to match on, and that ESSID (and probably similar things for
Bluetooth etc) would be fine if they are anyway available. But yeah,
whether to match on things that requires us to UP the interface would need
to be discussed some more.


> > However, the missing bit then would be network address assignment for the
> > various instances to the right interfaces.  Ideally, I'd just stamp out
> > network unit files and have the apache instance units depend upon that,
> but
> > the trouble is that traditionally NIC naming hasn't always been
> consistent
> > in the past.
> >
> > I've read through [1], but it doesn't really provide what I'm looking
> for.
> > Physical layout of the nic-port-types is semi interesting and perhaps
> > consistent, but network operator error may result in a misassigned vlan
> > port, or simply the wrong cable to the wrong port (which can be true for
> > physical or virtual realms unfortunately), etc.
> >
> > What I did in the past to work around that was to use ndisc6 or something
> > similar to verify that the expected interface had the expected network
> > properties - in this case a router advertisement.
>
> Hmm, schemes like this sound a bit dangerous, no? I mean, if you base
> your decision whether to apply the relatively open "internal LAN"
> config to an interface or the restricted "internet" config on the
> traffic you see on the port, then you make yourself vulnerable to
> people sending you rogue IP packets...
>
> I see your usecase though, but I don't really have any good suggestion
> what to do in this case I must say...
>
> Maybe adding something like a RequireDHCPServer= setting or so, that
> allows configuration of a DHCP server address, and when set would
> result in logged warnings if DHCP leases are offered from other
> servers thatn the configured one, might be an option? i.e. so that you
> at least get a loggable event when some .network file is applied to
> the wrong iface?
>
> But dunno, maybe Tom has an idea about this? Tom?
>

I'm very skeptical about these kind of schemes. We can really not promise
anything about where DHCP/NDisc come form. If someone has access to the
local lan, they can spoof absolutely anything, so we better not make the
impression that we can guarantee anything we can't. If you want some sort
of sanity checking, that might make sense, but probably as some

Re: [systemd-devel] [RFC] the chopping block

2016-02-11 Thread Tom Gundersen
On Thu, Feb 11, 2016 at 6:06 PM, Lennart Poettering 
wrote:

> Heya!
>
> So I am thinking about some spring cleaning, and would love to remove
> the following bits from the systemd package:
>

All this looks good to me.


> 1) systemd-initctl (i.e. the /dev/initctl SysV compat support). Last
>time Debian was still using that, maybe this changed now?
>
> 2) compat support for libsystemd-login.so and friends (these were
>merged into a single libsystemd.so a long time ago). We are still
>building compat libraries to ease the transition, but that was a
>long time ago, hence I'd really love to see this go. Any distro
>still using this?
>
> 3) systemd-reply-password – this is really old stuff used by the GNOME
>ask-password stuff which was experimental at best, and never found
>much use. Unless am very wrong pretty much nobody is using this,
>and we can just kill this without replacement. Anybody knows a user
>of this that I am not aware of?
>
> 4) Capabilities= support, i.e. the non-ambient and non-bounding-set kind
>of capabilities. They are pretty useless, as fcaps reduce them to
>nothing in pretty much all cases, which is precisely why the
>ambient caps were created. I am pretty sure nothing uses this, as
>it's not realistic to use this at all.
>
> 5) Here's the controversial one I think: support for booting up
>without /var. We have kludges at quite a few places because we
>cannot access /var early during boot. I am tempted to stop
>supporting this altogether. Of course, this does *not* mean that
>people with split off /var would be left in the cold. It just means
>that they have to mount /var from the initrd, exactly like this is
>already handled from /usr.
>

This would be particularly nice (and controversial for sure). I totally
agree that this should be very doable though, as all the initrd
infrastructure is already there. The way things are now with no /var at
early boot is sort of idiotic I must say. Big +1 from me.


> 6) The .snapshot unit type. These sounded like a smart idea, I am
>pretty sure though nobody is using them properly, and they are
>pretty hard to use. If anything like this should exist at al, then
>probably as a concept of "transient targets", but not as a separate
>unit type. Anyone knows any real users of this stuff?
>
> And that's all for now. Opinions?
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Where is DHCP Address stored?

2015-11-28 Thread Tom Gundersen
On Sat, Nov 28, 2015 at 4:42 PM, J Decker  wrote:
> On Sat, Nov 28, 2015 at 7:36 AM, Tom Gundersen  wrote:
>> On Sat, Nov 28, 2015 at 4:33 PM, J Decker  wrote:
>>> Okay maybe it's not stored anywhere
>>>
>>> I was just having a plethora of network issues after updating.
>>>
>>> After finding others were reporting issues with the linux kernel 4.2.5
>>> I rollback to the prior stable 4.1.6... that resolved some of the
>>> issues; but I was still getting a spam of messages every few seconds.
>>> I reverted to systemd-227 and these stopped.
>>
>> Distro?
>
> Arch; did an update lastnight/this morning when my networking fell apart.

As far as I can tell the fixes for these issues are upstream, but not
backported. You could try with current git, or ask downstream to
backport.

>>> Nov 28 07:18:14 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:14 tower2 systemd-networkd[354]: br0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:19 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:19 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:19 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:19 tower2 systemd-networkd[354]: br0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:25 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:25 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:25 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:25 tower2 systemd-networkd[354]: br0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:30 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:30 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:30 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:30 tower2 systemd-networkd[354]: br0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:35 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:35 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:35 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:35 tower2 systemd-networkd[354]: br0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>> Nov 28 07:18:41 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
>>> client on NDisc request failed: Invalid argument
>>>
>>>
>>>
>>> On Sat, Nov 28, 2015 at 4:18 AM, J Decker  wrote:
>>>> I recently updated my system, and am probably using systemd-228
>>>>
>>>> When the network starts, the device I have configured as DHCP
>>>> [Match]
>>>> Name=eth0
>>>>
>>>> [Network]
>>>> DHCP=ipv4
>>>> Tunnel=he-ipv6
>>>>
>>>> comes up with the old address and no default route set.  If I disable
>>>> that and manually run dhcpcd eth0 I get a valid IP and a default route
>>>> that is an entirely different network than I get from systemd... so it
>>>> must be saving the old address somewhere; but I cannot find any
>>>> reference to that lease/state file.
>>> ___
>>> 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] Where is DHCP Address stored?

2015-11-28 Thread Tom Gundersen
On Sat, Nov 28, 2015 at 4:33 PM, J Decker  wrote:
> Okay maybe it's not stored anywhere
>
> I was just having a plethora of network issues after updating.
>
> After finding others were reporting issues with the linux kernel 4.2.5
> I rollback to the prior stable 4.1.6... that resolved some of the
> issues; but I was still getting a spam of messages every few seconds.
> I reverted to systemd-227 and these stopped.

Distro?

> Nov 28 07:18:14 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:14 tower2 systemd-networkd[354]: br0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:19 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:19 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:19 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:19 tower2 systemd-networkd[354]: br0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:25 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:25 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:25 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:25 tower2 systemd-networkd[354]: br0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:30 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:30 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:30 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:30 tower2 systemd-networkd[354]: br0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:35 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:35 tower2 systemd-networkd[354]: he-ipv6: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:35 tower2 systemd-networkd[354]: eno1: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:35 tower2 systemd-networkd[354]: br0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
> Nov 28 07:18:41 tower2 systemd-networkd[354]: eth0: Starting DHCPv6
> client on NDisc request failed: Invalid argument
>
>
>
> On Sat, Nov 28, 2015 at 4:18 AM, J Decker  wrote:
>> I recently updated my system, and am probably using systemd-228
>>
>> When the network starts, the device I have configured as DHCP
>> [Match]
>> Name=eth0
>>
>> [Network]
>> DHCP=ipv4
>> Tunnel=he-ipv6
>>
>> comes up with the old address and no default route set.  If I disable
>> that and manually run dhcpcd eth0 I get a valid IP and a default route
>> that is an entirely different network than I get from systemd... so it
>> must be saving the old address somewhere; but I cannot find any
>> reference to that lease/state file.
> ___
> 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] DHCPC Events?

2015-11-19 Thread Tom Gundersen
This is not really exposed nicely. There is a C library sd-network, which
will give you the events you want, but it is not yet public (you can still
copy it out of git of course).
On Nov 12, 2015 05:24, "J Decker"  wrote:

> Should I have not said specifically Arch linux?
> Is it something that can't be done?
> It is something that should be so obvious it doesn't merit an answer?
>
>
>
> On Fri, Nov 6, 2015 at 1:56 PM, J Decker  wrote:
> > I have Arch Linux setup as my router.
> > It's on a connection that can change the IP that I'm given, when that
> > happens I need to rerun firewall rules and rebuild my ipv6 tunnel.
> > How do I run some script or something when the address changes (or
> > when it's initially given in the case of boot?)
> >
> > Also there seems to be no way to specify default ipv6 route for next
> > hop... ie 'ip -6 route replace ::/0 dev he-ipv6'
> > It's been a couple months I've been limping along so I forget; I
> > vaguely remember that this should have been setup in the configuration
> > scripts; but it didn't work unless I did it this way.  The iniital
> > method I think was 'add' instead of 'replace' which no longer works (I
> > think something changed in the kernel that affected that; but I don't
> > know).
> ___
> 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] Fwd: [PATCH] Add support for detecting NIC partitions on Dell Servers

2015-11-10 Thread Tom Gundersen
On Tue, Nov 10, 2015 at 2:29 PM, Jordan Hargrave  wrote:
> On Tue, Nov 10, 2015 at 4:53 AM, Kay Sievers  wrote:
>> On Tue, Nov 10, 2015 at 5:49 AM, Jordan Hargrave  wrote:
>>> Cleaned up linux coding style
>>>
>>> This patch will integrate some of the features of biosdevname into systemd.
>>> The code detects the port and index for detecting NIC partitions. This 
>>> creates
>>> a new environment variable, ID_NET_NAME_PARTITION of the format
>>> _
>>>
>>> The patch will also decode SMBIOS slot number for NIC, and store in the 
>>> variable
>>> ID_NET_NAME_SMBIOS_SLOT.  Systemd does not have a method for naming
>>> ports on a multi-port card plugged into a slot.
>>
>> Again, I don't think systemd should carry an SMBIOS parser.
>>
>> Sorry,
>> Kay
>
> From a customer usability standpoint, having the slot numbers as part
> of systemd would be a very useful feature.

Sure, but I think Kay's point was that the needed info should be
exposed from the kernel in a sysattr, not be parsed from udev. Any
reason this cannot be done that way?

>  The current method only
> works for single-port NICs in a slot.  Multi-port NICs, especially
> ones with SR-IOV or multiple partitions get garbled names like

Just to make sure we are on the same page, when you say "garbled" you
mean that the naming scheme is not the one you want, but there are no
bugs here, right?

> enp4s0
> enp4s1
> enp4s0d1
> enp4s0f1
> enp4s0f2
> enp4s0f3
> enp4s0f4
> enp4s0f5
> enp4s0f6
> enp4s0f7
> enp4s0f1d1
> enp4s0f2d1
> enp4s0f3d1
> enp4s0f4d1
> enp4s0f5d1
> enp4s0f6d1
> enp4s0f7d1
> enp4s1d1
> enp68s0f0
> enp68s0f1
> enp69s0f0
> enp69s0f1
>
> That's another annoying thing with systemd names, the bus number is
> *decimal*.  lspci is in hex, so the customer has to do a conversion to
> figure out even what PCI device that is.

I guess too late to change that now.

> All enp4 are a dual-port NIC in Slot 3 with 8 SR-IOV devices.

Hm, there are 17 devices listed, shouldn't there be 16 based on your
description?

> All enp68xx and enp69xxx are a single quad-port NIC in slot 2.
> Systemd breaks here if trying to name using slot numbers with the
> existing method.  As there are 4 devices under the slot with same
> device numbers, systemd would name them
> ens2f0
> ens2f1
> ens2f0
> ens2f1
>
> Which causes name collision.  I was able to verify this as either they
> got named:
> ens2f0
> ens2f1
> enp69s0f0
> enp69s0f1
>
> or
> enp68s0f0
> enp68s0f1
> ens2f0
> ens2f1
>
> at startup.
>
> That's the best feature of biosdevname, being able to tell which slot
> the NIC is located just from the name.  Systemd still has some
> limitations and/or bugs in this regard.

So how would your proposed naming scheme look in the examples you
gave? Is the information needed to generate the name taken from the
device in question or any of its parent devices (but never from its
siblings or other devices) and hence independent of probe-ordering?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Secret machine-id for RFC 7217 stable addresses

2015-10-08 Thread Tom Gundersen
Hi Lubomir,

Sorry not to have responded to this earlier, but as I was just
reminded of this, here are my take:

On Mon, Sep 7, 2015 at 7:49 PM, Lubomir Rintel  wrote:
> the RFC 7217 specifies an algorithm for generating an IPv6 host address
> that stays stable in a particular network but changes when the machine
> enters another network to prevent tracking [1]. It works by hashing a
> tuple of various parameters one of which is "secret_key" -- a secret
> value specific to a particular machine.
>
> [1] https://tools.ietf.org/html/rfc7217#section-5
>
> This sounds a bit like machine-id, unfortunately given it's world
> readable and available via DBus (and possibly on a network?) it doesn'tseem 
> to be secret enough.
>
> I'm wondering if it would make sense to reuse some of the tooling?
> Would it make sense to extend systemd-machine-id-setup(1) to generate
> one more identifier or maybe add another tool to set up the secret id?

A priori, it would perhaps have been nice to consider the real
machine-id on disk to be "secret", and only ever expose a hash of it,
but that ship has sailed I'm afraid. We could of course introduce a
second machine-id as you propose, but before doing that I'd like to
fully understand if that really solves the problem.

If I understand correctly, most of the point of RFC7217 is achieved
even if the secret key is known. The important point is to have a good
hashing function, and in that case knowing the secret key will not let
you discover any of the other parameters (which are the ones you
really want to hide).

Moreover, if the point is privacy, if an attacker has access (in some
way) to the machine-id, there is no point in him going after the
interface identifier as he can already identify the client.

Given those two facts, might it not be sufficient to use the
machine-id as the secret key after all? Or am I missing something?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Persistent virtio device name removal

2015-09-02 Thread Tom Gundersen
Hi Michael,

A follow-up on this, we finally figured out a way to make virtio
netdev naming persistent [0], so this will work again as of the next
release. It's a shame we had to go back and forth on this, but I hope
it does not cause you too much headaches.

Cheers,

Tom

[0]: 
<https://github.com/systemd/systemd/commit/54683f0f9b97a8f88aaf4fbb45b4d729057b101c>

On Fri, Jul 4, 2014 at 1:31 AM, Tom Gundersen  wrote:
> On Fri, Jul 4, 2014 at 12:55 AM, Michael Marineau
>  wrote:
>> 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?
>
> They suffer from the same problem as the ethX enumeration. In many
> cases they appear to be stable (as drivers typically get loaded in the
> same order etc), but there is nothing ensuring it, so we can't rely on
> that.
>
> Cheers,
>
> Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Configurability for systemd logging API

2015-07-30 Thread Tom Gundersen
On Thu, Jul 30, 2015 at 8:16 AM, SF Markus Elfring
 wrote:
>>> Such messages correspond to specific data structures.
>>> * The log origin and log level are repeated there while the recorded
>>>   information might occasionally not be detailed enough.
>>>   I find that such details can be better handled by the software build 
>>> system.
> …
>> I appreciate you wanting to help, but this is not helpful.
>
> Thanks for your feedback.
>
>
>> Please post patches if you have suggestions for improvements.
>
> I find that the change acceptance is unclear for fine-tuning of this software
> also around message log programming interfaces.
> Which design approaches do you find acceptable for further considerations?

Sorry, I really can't follow. If you post a patch we can take it from there.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Configurability for systemd logging API

2015-07-29 Thread Tom Gundersen
Hi Markus,

On Wed, Jul 29, 2015 at 8:54 PM, SF Markus Elfring
 wrote:
> Programs from the systemd software library output also some log messages
> as it is usual for such server applications.
>
> Such messages correspond to specific data structures.
> * The log origin and log level are repeated there while the recorded
>   information might occasionally not be detailed enough.
>   I find that such details can be better handled by the software build system.
>
> * The involved text repetition results in improvement possibilities
>   depending on preferred design directions.
>   Some substrings could be eventually shared between message variants
>   if this aspect can be selected as an optimisation goal
>   for the corresponding generation of executable software.
>   https://en.wikipedia.org/wiki/Longest_common_subsequence_problem

I appreciate you wanting to help, but this is not helpful. Please post
patches if you have suggestions for improvements.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] kdbus, udev and systemd in the initramfs

2015-07-29 Thread Tom Gundersen
On Wed, Jul 29, 2015 at 2:23 PM, Michael Biebl  wrote:
> 2015-07-29 5:40 GMT+02:00 Tom Gundersen :
>> On Wed, Jul 29, 2015 at 1:21 AM, Michael Biebl  wrote:
>>> something I was wondering regarding kdbus and udev.
>>> If udev wants to drop the netlink transport and instead rely on kdbus,
>>> would this mean, systemd becomes mandatory in the initramfs to setup
>>> kdbus before udev is run?
>>
>> _If_ udev drops netlink entirely, then yes someone would need to set
>> up kdbus. How this will all look has not been finally decided yet
>> though.
>
> Is this documented somewhere, what kind of setup needs to be done for
> kdbus?

Well, kdbus itself is extensively documented in the kernel man pages
[0], but I'm not aware of any step-by-step documentation for how to
make an alternative kdbus userspace implementation. Probably your best
bet is to look at the systemd code.

> How complicated is it, to get kdbus up and running with a
> minimal environment like the initramfs?

In principle, not hugely complicated. However, probably not worth the
effort unless you have a good reason to. Wouldn't it be simpler to
just use systemd (even if just as a wrapper around your current
scripts)?

Either way, I probably wouldn't put any serious amount of work into
this just for the sake of udev quite yet. We still don't know
precisely how it will shake out. In general though, I would expect
that one day you'll want kdbus in the initramfs for one reason or
another.

Cheers,

Tom

[0]: <http://www.freedesktop.org/software/systemd/kdbus/kdbus.html>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] kdbus, udev and systemd in the initramfs

2015-07-28 Thread Tom Gundersen
On Wed, Jul 29, 2015 at 5:40 AM, Tom Gundersen  wrote:
> On Wed, Jul 29, 2015 at 1:21 AM, Michael Biebl  wrote:
>> something I was wondering regarding kdbus and udev.
>> If udev wants to drop the netlink transport and instead rely on kdbus,
>> would this mean, systemd becomes mandatory in the initramfs to setup
>> kdbus before udev is run?
>
> _If_ udev drops netlink entirely, then yes someone would need to set
> up kdbus. How this will all look has not been finally decided yet
> though.

Actually, the problem might not even be netlink (as you may not have
anything using libudev in your intrd), but then the problem will be
the custom udev control protocol which would surely also be swapped
out if netlink is.

>> Will it still be possible in the future to run udev  without systemd
>> in the initramfs?
>
> Maybe, but I, for one, don't see this as a long-term goal. Having the
> same software in the initrd as on the real system seems much more
> reasonable to me.
>
> Cheers,
>
> Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] kdbus, udev and systemd in the initramfs

2015-07-28 Thread Tom Gundersen
On Wed, Jul 29, 2015 at 1:21 AM, Michael Biebl  wrote:
> something I was wondering regarding kdbus and udev.
> If udev wants to drop the netlink transport and instead rely on kdbus,
> would this mean, systemd becomes mandatory in the initramfs to setup
> kdbus before udev is run?

_If_ udev drops netlink entirely, then yes someone would need to set
up kdbus. How this will all look has not been finally decided yet
though.

> Will it still be possible in the future to run udev  without systemd
> in the initramfs?

Maybe, but I, for one, don't see this as a long-term goal. Having the
same software in the initrd as on the real system seems much more
reasonable to me.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Remove accelerometer helper

2015-06-29 Thread Tom Gundersen
On Sat, Jun 27, 2015 at 10:02 PM, Kay Sievers  wrote:
> On Fri, May 22, 2015 at 3:42 PM, Bastien Nocera  wrote:
>> It's moved to the iio-sensor-proxy D-Bus service.
>> ---
>>  .gitignore |   1 -
>>  Makefile.am|  15 --
>>  rules/61-accelerometer.rules   |   3 -
>>  src/udev/accelerometer/Makefile|   1 -
>>  src/udev/accelerometer/accelerometer.c | 303 
>
> https://github.com/systemd/systemd/pull/387

This was now merged, so distros should be aware that they need to pick
up iio-sensor-proxy with the next systemd release.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd-nspawn: cannot join existing macvlan

2015-06-18 Thread Tom Gundersen
On Thu, Jun 18, 2015 at 9:10 PM, Lennart Poettering
 wrote:
> On Sat, 30.05.15 19:55, Kai Krakow (hurikha...@gmail.com) wrote:
>
>> The next issue with your argument is: AFAIR nspawn doesn't create a macvlan
>> interface based on the machine name. You have to pass the name of a physical
>> interface which transports this macvlan. The man page at least states that
>> you use an existing physical interface:
>
> True, I was a bit confused there...
>
>>
>> --network-macvlan=
>> Create a "macvlan" interface of the specified Ethernet network
>> interface and add it to the container. A "macvlan" interface is a
>> virtual interface that adds a second MAC address to an existing
>> physical Ethernet link. The interface in the container will be named
>> after the interface on the host, prefixed with "mv-". Note that
>> --network-macvlan= implies --private-network. This option may be
>> used more than once to add multiple network interfaces to the
>> container.
>>
>> Trying to nspawn a macvlan without giving a physical device results in "no
>> such device":
>>
>> # systemd-nspawn --boot --link-journal=try-guest --machine=gentoo-mysql-base
>> --network-macvlan=
>> systemd-nspawn: option '--network-macvlan' requires an argument
>>
>> # systemd-nspawn --boot --link-journal=try-guest --machine=test --network-
>> macvlan=test
>> Spawning container test on /var/lib/machines/test.
>> Press ^] three times within 1s to kill container.
>> Failed to resolve interface test: No such device
>>
>> So your assumption about macvlan seems to be incorrect. The other network
>> types may be based off the machine name but it doesn't work this way with
>> macvlan.
>
> Yeah, nspawn creates a n interface "mv-foo" from a network interface
> "foo" on the host.
>
>> So I wonder what the purpose of macvlan is if you can only create one
>> machine using macvlan on my single physical link - and that's it?
>
> I am not sure I follow. The mv-foo interface will be part of the
> container, not the host. That means if you run 10 containers, all with
> --network-macvlan=foo, then they each will have an interface called
> "mv-foo", but with a different ifindex each. Or are you suggesting
> that that's currently broken?
>
>> I think the logic is wrong here in systemd-nspawn. Instead of trying to
>> create the host-side macvlan itself it should insist of it being there
>> already (to have one well-defined state to start with, and only optionally
>> create it by itself). Then, it can join multiple machines to the same
>> macvlan.
>
> I don't grok this?
>
> "the same macvlan"?
>
>> I'm using the following networkd config on the host to define this "well-
>> defined state", enabling the host to also communicate through macvlan:
>>
>> # cat /etc/systemd/network/10-macvlan.{netdev,network}
>> [NetDev]
>> Name=mv-enp5s0
>> Kind=macvlan
>>
>> [MACVLAN]
>> Mode=bridge
>> #
>> [Match]
>> Name=en*
>>
>> [Network]
>> MACVLAN=mv-enp5s0
>> LinkLocalAddressing=no
>> DHCP=no
>>
>> If I remove this I am able to start one of the containers, but not two or
>> more. If I use this I am not able to start any macvlan container.
>
> I have the suspicion that the confusion here stems from the fact that
> nspawn creates the macvlan iface on the host first, then moves it into
> the container.

Hm, you sure about this? I thought we create the macvlan device
directly in the network namespace. There should be no possible
name-conflict with devices on the host...

> but if you already have an iface by that name on the
> host, then it cannot create the macvlan under that name.
>
> I figure we can fix that by creating the iface under a random name
> first on the host, then move it into the container, and then rename it
> to the right name afterwarads.
>
> A work-around would be to name the .netdev iface of yours something
> else than "mv-enp5s0", call it "waldi" or so, so that it doesn't
> conflict with the name for the contaainer in the short time-frame that
> the iface nspawn creates exists on the host...
>
> Can you verify if such a change fixes your issue? If so, we can
> randomoize the name initially, as sugegsted above.
>
> 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] Regression: loop device detach errors in 220

2015-06-03 Thread Tom Gundersen
Thanks for the report.

This should hopefully be fixed by https://github.com/systemd/systemd/pull/65

Cheers,

Tom

On Wed, Jun 3, 2015 at 5:34 PM, Jan Janssen  wrote:
> Hi,
>
> systemd-shutdown in 220 has errors when detaching loop devices:
>
> systemd-shutdown[1]: Failed to detach loop devices: Invalid argument
> cgroup: option or name mismatch, new: 0x0 "", old: 0x4 "systemd"
> systemd-shutdown[1]: Failed to detach loop devices: Invalid argument
> systemd-shutdown[1]: Failed to detach loop devices: Invalid argument
> systemd-shutdown[1]: Failed to finalize _ loop devices, ignoring
>
> https://bugs.archlinux.org/task/45111
>
> c32eb440bab953a0169cd207dfef5cad16dfb340 is the first bad commit
> Author: Tom Gundersen 
> Date:   Tue Apr 14 16:25:06 2015 +0200
>
> libudev: make libudev-enumerate a thin wrapper around sd-device
>
> :100644 100644 837fd36381315029171562b344dca8620528d327
> 68d8252b84c13591cf8e0b0e15a99780f5dd0309 M  Makefile.am
> :04 04 c54e32bc21e34cc28693fbf653c4128a0383d3d7
> 11e1eeec94338e9294e25e720007c35f229d24cf M  src
>
> Jan
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] mount: use libmount to monitor mountinfo & utab

2015-06-02 Thread Tom Gundersen
On Tue, Jun 2, 2015 at 1:53 PM, Karel Zak  wrote:
> On Mon, Jun 01, 2015 at 05:06:56PM +0200, Tom Gundersen wrote:
>> > -(void) 
>> > sd_event_source_set_description(m->mount_utab_event_source, 
>> > "mount-utab-dispatch");
>> > +sd_event_source_set_description(m->mount_event_source, 
>> > "mount-monitor-dispatch");
>>
>> This should be cast to (void) unless you check the return.
>
> Frankly, I don't like it. It's old-style programming garbage. For
> compiler it's probably irrelevant construction and for developers
> (code readers) we have better things like warn_unused_result.

The policy in systemd is that as a general rule we check the return
value of functions, only if it truly does not matter do we explicitly
indicate this by casting to void. This is as much for readers of the
code (who can then distinguish between accidentally forgetting to
check the return and intentionally ignoring it) as for static checkers
(Coverity in particular will complain).

> I have removed many of these (void) from util-linux and nobody
> complains.
>
> If you really want to force people to check return code than mark
> function by warn_unused_result and if you still want to ignore the
> result for these functions in some situations then you can use
> something like:

I don't think warn_unused_result is the right thing to do btw, that is
about the provider enforcing checking of result whereas our policy is
as a consumer (both of our internal functions and external libraries).

> # define ignore_result(x) __extension__ ({ \
> __typeof__(x) __dummy __attribute__((__unused__)) = (x);
> (void) __dummy; \
> })
>
>
> the result is more readable and obvious:
>
>ignore_result( sd_event_source_set_description(foo, bar ) );
>
>
> Sometimes we use this macro to keep silent some crazy glibc functions.
>
>
> Anyway, if (void) is really systemd coding style then I'm going to
> update the patch. No problem ;-)

Thanks!

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] mount-setup: create /run/systemd/netif/links/ before accessing

2015-06-02 Thread Tom Gundersen
On Mon, Jun 1, 2015 at 9:16 PM, Robert Schwebel
 wrote:
> systemd-timesyncd breaks with
>
>   Starting Network Time Synchronization...
>   [FAILED] Failed to start Network Time Synchronization.
>
> when we have timesyncd activated and systemd-networkd not. Create
> directory before using it.

Hm, this is surely the wrong fix. PID1 should not know about
networkd/timesyncd. Either this should be created by a tmpfiles
fragment, or timesyncd should learn not to care about missing
networkd.

-t

> ---
>  src/core/mount-setup.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
> index ba96741e9549..25412a1c42d3 100644
> --- a/src/core/mount-setup.c
> +++ b/src/core/mount-setup.c
> @@ -393,6 +393,8 @@ int mount_setup(bool loaded_policy) {
>  mkdir_label("/run/systemd", 0755);
>  mkdir_label("/run/systemd/system", 0755);
>  mkdir_label("/run/systemd/inaccessible", );
> +   mkdir_label("/run/systemd/netif", 0755);
> +   mkdir_label("/run/systemd/netif/links", 0755);
>
>  return 0;
>  }
> ___
> 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] mount: use libmount to monitor mountinfo & utab

2015-06-01 Thread Tom Gundersen
Hi Karel,

On Mon, Jun 1, 2015 at 2:07 PM, Karel Zak  wrote:
> The current implementation directly monitor /proc/self/mountinfo and
> /run/mount/utab files. It's really not optimal because utab file is
> private libmount stuff without any official guaranteed semantic.
>
> The libmount since v2.26 provides API to monitor mount kernel &
> userspace changes. This patch replaces the current implementation with
> libmount based solution.
>
> Now the manager.h includes , so $MOUNT_CFLAGS has been
> necessary to add to many tests CFLAGS.
>
> Note that mnt_monitor_event_cleanup() in v2.26 is broken, so the patch
> uses mnt_monitor_next_change(). It's exactly the same solution which
> uses the current libmount HEAD (mnt_monitor_event_cleanup() is API
> shorcut only).


Tiny nitpick below, otherwise look good to me.

Cheers,

Tom

> ---
>  Makefile.am|  33 --
>  configure.ac   |   2 +-
>  src/core/manager.c |   2 +-
>  src/core/manager.h |   5 ++-
>  src/core/mount.c   | 100 
> -
>  5 files changed, 49 insertions(+), 93 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index ed5135d..3815e72 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1352,7 +1352,8 @@ systemd_SOURCES = \
>
>  systemd_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  systemd_LDADD = \
> libsystemd-core.la \
> @@ -1554,7 +1555,8 @@ test_engine_SOURCES = \
>
>  test_engine_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_engine_LDADD = \
> libsystemd-core.la \
> @@ -1565,7 +1567,8 @@ test_job_type_SOURCES = \
>
>  test_job_type_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_job_type_LDADD = \
> libsystemd-core.la \
> @@ -1609,7 +1612,8 @@ test_unit_name_SOURCES = \
>
>  test_unit_name_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_unit_name_LDADD = \
> libsystemd-core.la \
> @@ -1620,7 +1624,8 @@ test_unit_file_SOURCES = \
>
>  test_unit_file_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_unit_file_LDADD = \
> libsystemd-core.la \
> @@ -1838,7 +1843,8 @@ test_tables_CPPFLAGS = \
>
>  test_tables_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_tables_LDADD = \
> libsystemd-logs.la \
> @@ -1973,7 +1979,8 @@ test_cgroup_mask_SOURCES = \
> src/test/test-cgroup-mask.c
>
>  test_cgroup_mask_CPPFLAGS = \
> -   $(AM_CPPFLAGS)
> +   $(AM_CPPFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_cgroup_mask_CFLAGS = \
> $(AM_CFLAGS) \
> @@ -2022,7 +2029,8 @@ test_path_SOURCES = \
> src/test/test-path.c
>
>  test_path_CFLAGS = \
> -   $(AM_CFLAGS)
> +   $(AM_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_path_LDADD = \
> libsystemd-core.la
> @@ -2031,7 +2039,8 @@ test_execute_SOURCES = \
> src/test/test-execute.c
>
>  test_execute_CFLAGS = \
> -   $(AM_CFLAGS)
> +   $(AM_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_execute_LDADD = \
> libsystemd-core.la
> @@ -2061,7 +2070,8 @@ test_sched_prio_SOURCES = \
> src/test/test-sched-prio.c
>
>  test_sched_prio_CPPFLAGS = \
> -   $(AM_CPPFLAGS)
> +   $(AM_CPPFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  test_sched_prio_CFLAGS = \
> $(AM_CFLAGS) \
> @@ -2133,7 +2143,8 @@ systemd_analyze_SOURCES = \
>
>  systemd_analyze_CFLAGS = \
> $(AM_CFLAGS) \
> -   $(SECCOMP_CFLAGS)
> +   $(SECCOMP_CFLAGS) \
> +   $(MOUNT_CFLAGS)
>
>  systemd_analyze_LDADD = \
> libsystemd-core.la \
> diff --git a/configure.ac b/configure.ac
> index 48cedb5..74ec386 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -454,7 +454,7 @@ AM_CONDITIONAL(HAVE_BLKID, [test "$have_blkid" = "yes"])
>
>  # 
> --
>  have_libmount=no
> -PKG_CHECK_MODULES(MOUNT, [ mount >= 2.20 ],
> +PKG_CHECK_MODULES(MOUNT, [ mount >= 2.26 ],
>  [AC_DEFINE(HAVE_LIBMOUNT, 1, [Define if libmount is available]) 
> have_libmount=yes], have_libmount=no)
>  if test "x$have_libmount" = xno; then
>  AC_MSG_ERROR([*** libmount support required but libraries not found])
> diff --git a/src/core/manager.c b/src/core/manager.c
> index b931b0d..6881bb2 100644
> --- a/src/core/manager.c
> +++ b/src/core/manager.c
> @@ -567,7 +567,7 @@ int manager_new(ManagerRunningAs running_as, bool 
> test_run, Manager **_m) {
>
>  m->idle_pipe[0] = m->idle_pipe[1] = m->idle_pipe[2] = 
> m->idle_pipe[3] = -1;
>
> -m->pin_cgroupfs_fd = m->notify_fd = m->signal_fd = m->time_ch

Re: [systemd-devel] sd-device: fix invalid property strv pointers

2015-06-01 Thread Tom Gundersen
On Mon, Jun 1, 2015 at 11:39 AM, Martin Pitt  wrote:
> With our 220 package I still get a broken environment in udev
> callouts, even with Tom's recent fix 0e3e605 applied.
>
> Curiously it works for devices like "lo" which don't have a lot of
> properties, but for "real" wlan devices I get invalid environment
> variables. With some debugging applied (http://paste.ubuntu.com/11492452/)
> this is visible in the bogus strings that udev_device_get_properties_envp()
> returns: http://paste.ubuntu.com/11492458/
>
> I tracked that down to invalid memory handling in
> device_update_properties_bufs(). Patch attached with detailled
> explanation.

Thanks for figuring this out Martin. The patch looks good to me,
though maybe we should use NULSTR_FOREACH for the second loop?

Go ahead and push.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] udev --daemon is broken again

2015-05-31 Thread Tom Gundersen
On Sun, May 31, 2015 at 9:45 PM, Tom Gundersen  wrote:
> On Sun, May 31, 2015 at 8:45 PM, Abdó Roig-Maranges  
> wrote:
>>
>> Andrei Borzenkov writes:
>>
>>> Was not it fixed by
>>> 693d371d30fee1da58365121801445b404416ada?
>>
>> No.
>>
>> The first time it broke was due to the udev manager wanting to be used by a
>> single process, and was fixed in 040e689654ef08c63.
>>
>> Then it broke again by the commit you point 693d371d30fee1da58, due to the 
>> fact
>> that the sd-event event loop does not like to be passed across forks. This
>> breakage still remains in git.

This should now be fixed, please let me know if that is not the case.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] udev --daemon is broken again

2015-05-31 Thread Tom Gundersen
On Sun, May 31, 2015 at 8:45 PM, Abdó Roig-Maranges  wrote:
>
> Andrei Borzenkov writes:
>
>> Was not it fixed by
>> 693d371d30fee1da58365121801445b404416ada?
>
> No.
>
> The first time it broke was due to the udev manager wanting to be used by a
> single process, and was fixed in 040e689654ef08c63.
>
> Then it broke again by the commit you point 693d371d30fee1da58, due to the 
> fact
> that the sd-event event loop does not like to be passed across forks. This
> breakage still remains in git.

Indeed, I failed at rebasing. Thanks for the report. Will fix!

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Questions about socket activated services

2015-05-30 Thread Tom Gundersen
On Sat, May 30, 2015 at 3:36 PM, cee1  wrote:
> Which service type should a socket activated service be?
> 1. For systemd-udevd.service and systemd-journald.service, they are notify 
> type
> 2. For dbus.service, it is simple type

A service can be of type 'simple' if all that other services care
about that order themselves after it is that the sockets are available
(which they will always be as they are started by PID1 early on).
However, if the service in question does more work at startup that
other services want to wait for, then it needs to be of type 'notify',
and notify PID1 once it is ready. In the case of udev the work it
needs to do at startup is to apply permission to static device nodes
(you want to guarantee that this has finished before starting
follow-up services), if it had not been for that udev too could have
been of type 'simple'.

HTH,

Tom

> Does socket activation handles the timeout case?
> E.g. A.service connects to B.socket, but B.service spends a long time
> to be ready, may cause A.service receives ETIMEDOUT?
>
> When the service is activated, systemd will still listen to the socket
> but do nothing for incoming data, right?
>
> BTW, netstat -lp shows only systemd is listening to a socket, but not
> show the one who also is listening to the socket, e.g.
> """
> unix  2  [ ACC ] SEQPACKET  LISTENING 326  1/systemd
>  /run/udev/control
> """
> Curious why?
>
>
>
> --
> Regards,
>
> - cee1
> ___
> 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] 'udevadm settle' brakes lvm on top of imsm raid

2015-05-28 Thread Tom Gundersen
On Thu, May 28, 2015 at 10:10 AM, Oleg Samarin  wrote:
> Hi!
>
> I have an imsm raid-1 device /dev/md126 assembled of /dev/sda and /dev/sdb.
> I have a lvm group on top of /dev/md126p2 with some logical volumes. All
> this work fine with Fedora 21.
>
> I'm trying to fresh install Fedora 22 in some of lvm logical volume. I boot
> with Fedora USB live media and run "Install to hard disk". But anaconda does
> not see any existing lvm volumes so I can not choose them as a destination.
>
> I've created a bug https://bugzilla.redhat.com/show_bug.cgi?id=1178181 on
> this issue.
>
> After some discovering I found that the reason of this issue is that
> anaconda brakes lvm:
>
> before launching anaconda pvdisplay reports there are two physical volumes
> /dev/md126p2 and /dev/sdc2 (/dev/sdc is an SSD that does not belong any raid
> and contains a separate lvm volume group)
>
> after launching anaconda pvdisplay reports another two physical volumes
> /dev/sdb2 and /dev/sdc2, that is wrong, because /dev/sdb is a part of
> /dev/md126 raid.
>
> journalctl shows:
>
> May 28 02:44:08 localhost systemd[1]: Stopping LVM2 PV scan on device
> 259:1...
> May 28 02:44:08 localhost systemd[1]: Stopped LVM2 PV scan on device 259:1.
> May 28 02:44:08 localhost audit[1]:  pid=1 uid=0 auid=4294967295
> ses=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=lvm2-pvscan@259:1
> comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
> res=success'
> May 28 02:44:09 localhost kernel:  sde:
> May 28 02:44:09 localhost kernel:  sda: sda1 sda2
> May 28 02:44:09 localhost systemd[1]: Starting LVM2 PV scan on device 8:2...
> May 28 02:44:09 localhost kernel:  sdb: sdb1 sdb2
>
> Seems lvm stops using /dev/md126p2 and starts using /dev/sdb2 at this moment
>
> I can see in anaconda program.log that it ran 'udevadm settle' at this
> moment:
>
> 02:44:07,141 INFO program: ...done [9] (exit code: 0)
> 02:44:09,290 INFO program: Running... udevadm settle --timeout=300
> 02:44:09,305 DEBUG program: Return code: 0
> 02:44:09,306 INFO program: Running... udevadm settle --timeout=300
> 02:44:09,315 DEBUG program: Return code: 0
>
> So 'udevadm settle' breaks lvm from using '/dev/md126p2'. Futher lvm rescans
> force it to use /dev/sdb2 instead.
>
> The attached storage.log contains a trace of udev events of this. All other
> logs are available
> in https://bugzilla.redhat.com/show_bug.cgi?id=1178181
>
> What is wrong? How to force lvm to use /dev/md126p2 instead of /dev/sdb2
> after starting anaconda?

This seems unlikely to be udevadm settle's fault. All it does is wait
for udev to process events, you may think of it as "sleep 5" or
something like that. It can obviously affect the timing of things, but
as Lennart said the underlying problem is surely elsewhere...

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Fix systemd.resource-control(5) volume number.

2015-05-27 Thread Tom Gundersen
On Wed, May 27, 2015 at 9:47 PM, Patrick Donnelly  wrote:
> Signed-off-by: Patrick Donnelly 

We don't use s-o-b in systemd, so dropped this when applying. Also
adjusted the subject line a bit (for future reference).

Thanks for the patch! Pushed.

Cheers,

Tom
> ---
>  man/systemd.slice.xml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man/systemd.slice.xml b/man/systemd.slice.xml
> index f0bac41..a501327 100644
> --- a/man/systemd.slice.xml
> +++ b/man/systemd.slice.xml
> @@ -90,7 +90,7 @@
>  slice specific configuration options are configured in
>  the [Slice] section. Currently, only generic resource control settings
>  as described in
> -
> systemd.resource-control7
>  are allowed.
> +
> systemd.resource-control5
>  are allowed.
>  
>
>  Unless DefaultDependencies=false
> --
> Patrick Donnelly
>
> ___
> 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] fix extraneous space in equality check

2015-05-27 Thread Tom Gundersen
Applied. Thanks!

Tom

On Wed, May 27, 2015 at 9:02 PM, Jonathan Boulle
 wrote:
> ---
>  src/core/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/core/main.c b/src/core/main.c
> index c39815b10675..212ab901b18f 100644
> --- a/src/core/main.c
> +++ b/src/core/main.c
> @@ -1496,7 +1496,7 @@ int main(int argc, char *argv[]) {
>  setsid();
>
>  /* Move out of the way, so that we won't block unmounts */
> -assert_se(chdir("/")  == 0);
> +assert_se(chdir("/") == 0);
>
>  /* Reset the console, but only if this is really init and we
>   * are freshly booted */
> --
> 1.9.3
>
> ___
> 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] crda broken with systemd 220

2015-05-27 Thread Tom Gundersen
On Tue, May 26, 2015 at 11:53 AM, Michał Bartoszkiewicz
 wrote:
> Hello,
>
> it seems that udev from systemd 220 does passes empty environment to a
> process spawned by a RUN rule.
> execve("/usr/sbin/crda", ["/usr/sbin/crda"], [/* 0 vars */]) = 0
> This breaks crda, as it expects to see COUNTRY in its environment.

Thanks for the report! This should now be fixed in git. Let me know if
that is not the case.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

2015-05-27 Thread Tom Gundersen
On Wed, May 27, 2015 at 6:23 PM, Tom Gundersen  wrote:
> On Wed, May 27, 2015 at 6:16 PM, Filipe Brandenburger
>  wrote:
>> Hi Tom,
>>
>> On Wed, May 27, 2015 at 8:45 AM, Tom Gundersen  wrote:
>>> It appears a few people see this, but I was not able to reproduce. If
>>> anyone could reproduce with this patch applied [0], it would be most
>>> helpful (and post the output of "journalctl -b -u systemd-udevd").
>>
>> Done.
>>
>> Console output from udev in initramfs:
>> http://paste.fedoraproject.org/226145/43216143/
>>
>> And the output of the journalctl command you asked:
>> http://paste.fedoraproject.org/226146/43274324/
>>
>> I have this on Arch Linux with mkinitcpio. Using latest systemd from
>> git plus your patch.
>
> Thanks, that explains it (and teaches me how to reproduce)!

This should be fixed by 86c3bece38bcf55da6387d20c6f01da9ad0284dc.
Thanks for the help in debugging this, and sorry for the
inconvenience.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

2015-05-27 Thread Tom Gundersen
On Wed, May 27, 2015 at 6:16 PM, Filipe Brandenburger
 wrote:
> Hi Tom,
>
> On Wed, May 27, 2015 at 8:45 AM, Tom Gundersen  wrote:
>> It appears a few people see this, but I was not able to reproduce. If
>> anyone could reproduce with this patch applied [0], it would be most
>> helpful (and post the output of "journalctl -b -u systemd-udevd").
>
> Done.
>
> Console output from udev in initramfs:
> http://paste.fedoraproject.org/226145/43216143/
>
> And the output of the journalctl command you asked:
> http://paste.fedoraproject.org/226146/43274324/
>
> I have this on Arch Linux with mkinitcpio. Using latest systemd from
> git plus your patch.

Thanks, that explains it (and teaches me how to reproduce)!

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

2015-05-27 Thread Tom Gundersen
On Tue, May 26, 2015 at 5:11 PM, Martin Pitt  wrote:
> Hello Tom, all,
>
> with 220 I get a severe boot time regression:
>
>   $ systemd-analyze
>   Startup finished in 30.751s (kernel) + 11.706s (userspace) = 42.458s
>
> which used to be
>
>   $ systemd-analyze
>   Startup finished in 703ms (kernel) + 890ms (userspace) = 1.593s
>
> (this is a VM)
>
> It seems udevd --daemon spends 30 seconds timing out in the initramfs:
>
>   [0.384519] systemd-udevd[55]: starting version 220
>   [   30.736381] systemd-udevd[56]: timeout, giving up waiting for workers to 
> finish
>
> and then some more in the real root:
>
>$ systemd-analyze blame
>  10.826s dev-vda1.device
>  10.067s systemd-tmpfiles-setup-dev.service
>  10.031s systemd-sysctl.service
>  10.019s systemd-journald.service
>  10.005s sys-fs-fuse-connections.mount
>  10.001s tmp.mount
>
> (full journal at http://paste.ubuntu.com/11372265/, but it's not very
> useful)
>
> I bisected this to
>
>   http://cgit.freedesktop.org/systemd/systemd/commit/?id=e237d8c
>   udevd: move file descriptors to Manager
>
> this is hard to revert individually as there are lots of other recent changes
> in udev around this commit, but any version before that commit is fast
> and doesn't give that timeout error.
>
> Current trunk as of commit 185abfc3 still has that problem, so it
> wasn't fixed by one of the recent udev commits.
>
> Does anyone else see this too? Any idea what causes this?

It appears a few people see this, but I was not able to reproduce. If
anyone could reproduce with this patch applied [0], it would be most
helpful (and post the output of "journalctl -b -u systemd-udevd").

Cheers,

Tom

[0]: 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] missing: add more IFLA_VXLAN_* defines

2015-05-27 Thread Tom Gundersen
Applied. Thanks!

Tom

On Tue, May 26, 2015 at 7:48 AM, Michael Olbrich
 wrote:
> Otherwise building faild with kernel headers < v3.16
> ---
>  configure.ac |  2 +-
>  src/shared/missing.h | 11 +--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 48cedb5ab61a..0818dd80cf0c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -334,7 +334,7 @@ AC_CHECK_DECLS([IFLA_INET6_ADDR_GEN_MODE,
>  IFLA_PHYS_PORT_ID,
>  IFLA_BOND_AD_INFO,
>  IFLA_VLAN_PROTOCOL,
> -IFLA_VXLAN_LOCAL6,
> +IFLA_VXLAN_REMCSUM_NOPARTIAL,
>  IFLA_IPTUN_6RD_RELAY_PREFIXLEN,
>  IFLA_BRIDGE_VLAN_INFO,
>  IFLA_BRPORT_UNICAST_FLOOD,
> diff --git a/src/shared/missing.h b/src/shared/missing.h
> index 8ca6f8edb62c..919400949138 100644
> --- a/src/shared/missing.h
> +++ b/src/shared/missing.h
> @@ -713,7 +713,7 @@ static inline int setns(int fd, int nstype) {
>  #define IFLA_VLAN_MAX   (__IFLA_VLAN_MAX - 1)
>  #endif
>
> -#if !HAVE_DECL_IFLA_VXLAN_LOCAL6
> +#if !HAVE_DECL_IFLA_VXLAN_REMCSUM_NOPARTIAL
>  #define IFLA_VXLAN_UNSPEC 0
>  #define IFLA_VXLAN_ID 1
>  #define IFLA_VXLAN_GROUP 2
> @@ -732,7 +732,14 @@ static inline int setns(int fd, int nstype) {
>  #define IFLA_VXLAN_PORT 15
>  #define IFLA_VXLAN_GROUP6 16
>  #define IFLA_VXLAN_LOCAL6 17
> -#define __IFLA_VXLAN_MAX 18
> +#define IFLA_VXLAN_UDP_CSUM 18
> +#define IFLA_VXLAN_UDP_ZERO_CSUM6_TX 19
> +#define IFLA_VXLAN_UDP_ZERO_CSUM6_RX 20
> +#define IFLA_VXLAN_REMCSUM_TX 21
> +#define IFLA_VXLAN_REMCSUM_RX 22
> +#define IFLA_VXLAN_GBP 23
> +#define IFLA_VXLAN_REMCSUM_NOPARTIAL 24
> +#define __IFLA_VXLAN_MAX 25
>
>  #define IFLA_VXLAN_MAX  (__IFLA_VXLAN_MAX - 1)
>  #endif
> --
> 2.1.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


Re: [systemd-devel] Adding additional network interfaces "dynamically", after networkd startup finishes

2015-05-27 Thread Tom Gundersen
On Wed, May 27, 2015 at 2:03 PM, Peter Lemenkov  wrote:
> Hello All!
> My network is managed via systemd-networkd. I'm trying to create
> additional network bridge after another one service (VPN) is started.
>
> Right now I'm having a ExecStartPost directive, which creates
> /run/systemd/network, creates a necessary netdev/link/network files
> here, and restarts networkd (/bin/systemctl restart
> systemd-networkd.service).
>
> I wonder if it's a correct way to "dynamically" create network
> interfaces? Is it possible to ask networkd to re-read its
> configuration w/o restarting? Maybe D-Bus commands or something?

There is no correct way of doing this at the moment. It is next on my
list of features to work on.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

2015-05-26 Thread Tom Gundersen
On Tue, May 26, 2015 at 9:04 PM, Christian Hesse  wrote:
> Martin Pitt  on Tue, 2015/05/26 17:11:
>> Hello Tom, all,
>>
>> with 220 I get a severe boot time regression:
>>
>>   $ systemd-analyze
>>   Startup finished in 30.751s (kernel) + 11.706s (userspace) = 42.458s
>>
>> which used to be
>>
>>   $ systemd-analyze
>>   Startup finished in 703ms (kernel) + 890ms (userspace) = 1.593s
>>
>> (this is a VM)
>>
>> It seems udevd --daemon spends 30 seconds timing out in the initramfs:
>>
>>   [0.384519] systemd-udevd[55]: starting version 220
>>   [   30.736381] systemd-udevd[56]: timeout, giving up waiting for workers
>> to finish
>>
>> and then some more in the real root:
>>
>>$ systemd-analyze blame
>>  10.826s dev-vda1.device
>>  10.067s systemd-tmpfiles-setup-dev.service
>>  10.031s systemd-sysctl.service
>>  10.019s systemd-journald.service
>>  10.005s sys-fs-fuse-connections.mount
>>  10.001s tmp.mount
>>
>> (full journal at http://paste.ubuntu.com/11372265/, but it's not very
>> useful)
>>
>> I bisected this to
>>
>>   http://cgit.freedesktop.org/systemd/systemd/commit/?id=e237d8c
>>   udevd: move file descriptors to Manager
>>
>> this is hard to revert individually as there are lots of other recent
>> changes in udev around this commit, but any version before that commit is
>> fast and doesn't give that timeout error.
>>
>> Current trunk as of commit 185abfc3 still has that problem, so it
>> wasn't fixed by one of the recent udev commits.
>>
>> Does anyone else see this too? Any idea what causes this?
>
> I do see this as well. And probably we have an upstream bug [0] already.

Oh, and this bug report is probably unrelated. It is an old one (that
I could never reproduce), and the relevant parts were reworked
(probably causing the current issues).

-t

> Wondering whether or not my report about "inotify_add_watch() failed: Bad
> file descriptor" [1] is related. Do you see that as well?
> BTW, is it expected to have fd_inotify in udevd.c and inotify_fd in
> udev_watch.c?
>
> [0] https://bugs.freedesktop.org/show_bug.cgi?id=90051
> [1] http://lists.freedesktop.org/archives/systemd-devel/2015-May/032213.html
> --
> main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
> "CX:;",b;for(a/*Chris   get my mail address:*/=0;b=c[a++];)
> putchar(b-1/(/*   gcc -o sig sig.c && ./sig*/b/42*2-3)*42);}
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

2015-05-26 Thread Tom Gundersen
On Tue, May 26, 2015 at 9:04 PM, Christian Hesse  wrote:
> Martin Pitt  on Tue, 2015/05/26 17:11:
>> Hello Tom, all,
>>
>> with 220 I get a severe boot time regression:
>>
>>   $ systemd-analyze
>>   Startup finished in 30.751s (kernel) + 11.706s (userspace) = 42.458s
>>
>> which used to be
>>
>>   $ systemd-analyze
>>   Startup finished in 703ms (kernel) + 890ms (userspace) = 1.593s
>>
>> (this is a VM)
>>
>> It seems udevd --daemon spends 30 seconds timing out in the initramfs:
>>
>>   [0.384519] systemd-udevd[55]: starting version 220
>>   [   30.736381] systemd-udevd[56]: timeout, giving up waiting for workers
>> to finish
>>
>> and then some more in the real root:
>>
>>$ systemd-analyze blame
>>  10.826s dev-vda1.device
>>  10.067s systemd-tmpfiles-setup-dev.service
>>  10.031s systemd-sysctl.service
>>  10.019s systemd-journald.service
>>  10.005s sys-fs-fuse-connections.mount
>>  10.001s tmp.mount
>>
>> (full journal at http://paste.ubuntu.com/11372265/, but it's not very
>> useful)
>>
>> I bisected this to
>>
>>   http://cgit.freedesktop.org/systemd/systemd/commit/?id=e237d8c
>>   udevd: move file descriptors to Manager
>>
>> this is hard to revert individually as there are lots of other recent
>> changes in udev around this commit, but any version before that commit is
>> fast and doesn't give that timeout error.
>>
>> Current trunk as of commit 185abfc3 still has that problem, so it
>> wasn't fixed by one of the recent udev commits.
>>
>> Does anyone else see this too? Any idea what causes this?
>
> I do see this as well. And probably we have an upstream bug [0] already.
>
> Wondering whether or not my report about "inotify_add_watch() failed: Bad
> file descriptor" [1] is related. Do you see that as well?
> BTW, is it expected to have fd_inotify in udevd.c and inotify_fd in
> udev_watch.c?

This is unrelated.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] inotify_add_watch() failed: Bad file descriptor

2015-05-26 Thread Tom Gundersen
On Tue, May 26, 2015 at 10:14 AM, Christian Hesse  wrote:
> Hello everybody,
>
> with systemd v220 I see inotify errors from udevd. I get this once:
>
> systemd-udevd: inotify_add_watch(9, /dev/sr0, 10) failed: Bad file descriptor
>
> And a lot of these:
>
> systemd-udevd: inotify_add_watch(9, /dev/dm-[0-9]+, 10) failed: Bad file
> descriptor

This was fixed by David in 185abfc3d6b4e8f804a3f7216cd8b0459593af87.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] treewide: fix typos

2015-05-26 Thread Tom Gundersen
Applied. Thanks!

Tom

On Tue, May 26, 2015 at 7:17 PM, Torstein Husebø  wrote:
> ---
>  NEWS| 4 ++--
>  man/journal-remote.conf.xml | 2 +-
>  src/libsystemd/sd-bus/bus-control.c | 2 +-
>  src/libsystemd/sd-bus/bus-creds.c   | 6 +++---
>  src/shared/architecture.c   | 2 +-
>  src/shared/architecture.h   | 2 +-
>  src/shared/capability.h | 2 +-
>  src/shared/fdset.c  | 2 +-
>  src/shared/util.c   | 2 +-
>  9 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f72f502129..ee533b4363 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -3997,7 +3997,7 @@ CHANGES WITH 191:
>  * HandleSleepKey= in logind.conf has been split up into
>HandleSuspendKey= and HandleHibernateKey=. The old setting
>is not available anymore. X11 and the kernel are
> -  distuingishing between these keys and we should too. This
> +  distinguishing between these keys and we should too. This
>also means the inhibition lock for these keys has been split
>into two.
>
> @@ -4743,7 +4743,7 @@ CHANGES WITH 43:
>
>  * Various functionality updates to libsystemd-login.so
>
> -* Track class of PAM logins to distuingish greeters from
> +* Track class of PAM logins to distinguish greeters from
>normal user logins.
>
>  Contributions from: Kay Sievers, Lennart Poettering, Michael
> diff --git a/man/journal-remote.conf.xml b/man/journal-remote.conf.xml
> index a7b2227182..fc60258d0b 100644
> --- a/man/journal-remote.conf.xml
> +++ b/man/journal-remote.conf.xml
> @@ -83,7 +83,7 @@
>
>  ServerKeyFile=
>
> -SSL key in PEM format
> +SSL key in PEM format.
>
>
>
> diff --git a/src/libsystemd/sd-bus/bus-control.c 
> b/src/libsystemd/sd-bus/bus-control.c
> index fa4c28174d..43ddfc651d 100644
> --- a/src/libsystemd/sd-bus/bus-control.c
> +++ b/src/libsystemd/sd-bus/bus-control.c
> @@ -429,7 +429,7 @@ static int bus_populate_creds_from_items(
>  c->mask |= SD_BUS_CREDS_PPID;
>  } else if (item->pids.pid == 1) {
>  /* The structure doesn't
> - * really distuingish the case
> + * really distinguish the case
>   * where a process has no
>   * parent and where we don't
>   * know it because it could
> diff --git a/src/libsystemd/sd-bus/bus-creds.c 
> b/src/libsystemd/sd-bus/bus-creds.c
> index fed66823c7..4d67619cf8 100644
> --- a/src/libsystemd/sd-bus/bus-creds.c
> +++ b/src/libsystemd/sd-bus/bus-creds.c
> @@ -303,7 +303,7 @@ _public_ int sd_bus_creds_get_ppid(sd_bus_creds *c, pid_t 
> *ppid) {
>  if (!(c->mask & SD_BUS_CREDS_PPID))
>  return -ENODATA;
>
> -/* PID 1 has no parent process. Let's distuingish the case of
> +/* PID 1 has no parent process. Let's distinguish the case of
>   * not knowing and not having a parent process by the returned
>   * error code. */
>  if (c->ppid == 0)
> @@ -989,7 +989,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, 
> pid_t pid, pid_t tid) {
>  if (missing & SD_BUS_CREDS_EXE) {
>  r = get_process_exe(pid, &c->exe);
>  if (r == -ESRCH) {
> -/* Unfortunately we cannot really distuingish
> +/* Unfortunately we cannot really distinguish
>   * the case here where the process does not
>   * exist, and /proc/$PID/exe being unreadable
>   * because $PID is a kernel thread. Hence,
> @@ -1101,7 +1101,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, 
> pid_t pid, pid_t tid) {
>  }
>
>  /* In case only the exe path was to be read we cannot
> - * distuingish the case where the exe path was unreadable
> + * distinguish the case where the exe path was unreadable
>   * because the process was a kernel thread, or when the
>   * process didn't exist at all. Hence, let's do a final check,
>   * to be sure. */
> diff --git a/src/shared/architecture.c b/src/shared/architecture.c
> index 884abdd3ea..8e72e7a36a 100644
> --- a/src/shared/architecture.c
> +++ b/src/shared/architecture.c
> @@ -35,7 +35,7 @@ int uname_architecture(void) {
>   * 1:1. Instead we try to clean it up and break down the
>   * confusion on x86 and arm in particular.
>   *
> - * We do not try to distuingish CPUs not CPU features, but
> + * We do not try to distinguish CPUs not CPU features, but
>   * actual architectures, i.e. that have genuinely dif

Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

2015-05-26 Thread Tom Gundersen
On Tue, May 26, 2015 at 5:11 PM, Martin Pitt  wrote:
> Hello Tom, all,
>
> with 220 I get a severe boot time regression:
>
>   $ systemd-analyze
>   Startup finished in 30.751s (kernel) + 11.706s (userspace) = 42.458s
>
> which used to be
>
>   $ systemd-analyze
>   Startup finished in 703ms (kernel) + 890ms (userspace) = 1.593s
>
> (this is a VM)
>
> It seems udevd --daemon spends 30 seconds timing out in the initramfs:
>
>   [0.384519] systemd-udevd[55]: starting version 220
>   [   30.736381] systemd-udevd[56]: timeout, giving up waiting for workers to 
> finish
>
> and then some more in the real root:
>
>$ systemd-analyze blame
>  10.826s dev-vda1.device
>  10.067s systemd-tmpfiles-setup-dev.service
>  10.031s systemd-sysctl.service
>  10.019s systemd-journald.service
>  10.005s sys-fs-fuse-connections.mount
>  10.001s tmp.mount
>
> (full journal at http://paste.ubuntu.com/11372265/, but it's not very
> useful)
>
> I bisected this to
>
>   http://cgit.freedesktop.org/systemd/systemd/commit/?id=e237d8c
>   udevd: move file descriptors to Manager
>
> this is hard to revert individually as there are lots of other recent changes
> in udev around this commit, but any version before that commit is fast
> and doesn't give that timeout error.
>
> Current trunk as of commit 185abfc3 still has that problem, so it
> wasn't fixed by one of the recent udev commits.
>
> Does anyone else see this too? Any idea what causes this?

FWIW, I'm looking into this.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 4/6] udevd: move main-loop to sd-event

2015-05-25 Thread Tom Gundersen
---
 src/udev/udevd.c | 378 ++-
 1 file changed, 177 insertions(+), 201 deletions(-)

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index c9b0ed5..8cffd81 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -41,6 +41,8 @@
 #include 
 
 #include "sd-daemon.h"
+#include "sd-event.h"
+#include "event-util.h"
 #include "rtnl-util.h"
 #include "cgroup-util.h"
 #include "process-util.h"
@@ -62,6 +64,7 @@ static usec_t arg_event_timeout_warn_usec = 180 * 
USEC_PER_SEC / 3;
 
 typedef struct Manager {
 struct udev *udev;
+sd_event *event;
 Hashmap *workers;
 struct udev_list_node events;
 char *cgroup;
@@ -74,15 +77,13 @@ typedef struct Manager {
 struct udev_monitor *monitor;
 struct udev_ctrl *ctrl;
 struct udev_ctrl_connection *ctrl_conn_blocking;
-
-int fd_ep;
-int fd_ctrl;
-int fd_uevent;
-int fd_signal;
 int fd_inotify;
-int fd_worker;
 int worker_watch[2];
 
+sd_event_source *ctrl_event;
+sd_event_source *uevent_event;
+sd_event_source *inotify_event;
+
 usec_t last_usec;
 
 bool stop_exec_queue:1;
@@ -112,8 +113,8 @@ struct event {
 dev_t devnum;
 int ifindex;
 bool is_block;
-usec_t start_usec;
-bool warned;
+sd_event_source *timeout_warning;
+sd_event_source *timeout;
 };
 
 static inline struct event *node_to_event(struct udev_list_node *node) {
@@ -153,6 +154,9 @@ static void event_free(struct event *event) {
 udev_device_unref(event->dev);
 udev_device_unref(event->dev_kernel);
 
+sd_event_source_unref(event->timeout_warning);
+sd_event_source_unref(event->timeout);
+
 if (event->worker)
 event->worker->event = NULL;
 
@@ -254,7 +258,12 @@ static int on_event_timeout_warning(sd_event_source *s, 
uint64_t usec, void *use
 }
 
 static void worker_attach_event(struct worker *worker, struct event *event) {
+sd_event *e;
+uint64_t usec;
+int r;
+
 assert(worker);
+assert(worker->manager);
 assert(event);
 assert(!event->worker);
 assert(!worker->event);
@@ -262,9 +271,19 @@ static void worker_attach_event(struct worker *worker, 
struct event *event) {
 worker->state = WORKER_RUNNING;
 worker->event = event;
 event->state = EVENT_RUNNING;
-event->start_usec = now(CLOCK_MONOTONIC);
-event->warned = false;
 event->worker = worker;
+
+e = worker->manager->event;
+
+r = sd_event_now(e, clock_boottime_or_monotonic(), &usec);
+if (r < 0)
+return;
+
+(void) sd_event_add_time(e, &event->timeout_warning, 
clock_boottime_or_monotonic(),
+ usec + arg_event_timeout_warn_usec, 
USEC_PER_SEC, on_event_timeout_warning, event);
+
+(void) sd_event_add_time(e, &event->timeout, 
clock_boottime_or_monotonic(),
+ usec + arg_event_timeout_usec, USEC_PER_SEC, 
on_event_timeout, event);
 }
 
 static void manager_free(Manager *manager) {
@@ -273,7 +292,12 @@ static void manager_free(Manager *manager) {
 
 udev_builtin_exit(manager->udev);
 
+sd_event_source_unref(manager->ctrl_event);
+sd_event_source_unref(manager->uevent_event);
+sd_event_source_unref(manager->inotify_event);
+
 udev_unref(manager->udev);
+sd_event_unref(manager->event);
 manager_workers_free(manager);
 event_queue_cleanup(manager, EVENT_UNDEF);
 
@@ -285,8 +309,6 @@ static void manager_free(Manager *manager) {
 udev_rules_unref(manager->rules);
 free(manager->cgroup);
 
-safe_close(manager->fd_ep);
-safe_close(manager->fd_signal);
 safe_close(manager->fd_inotify);
 safe_close_pair(manager->worker_watch);
 
@@ -336,12 +358,16 @@ static void worker_spawn(Manager *manager, struct event 
*event) {
 manager->monitor = udev_monitor_unref(manager->monitor);
 manager->ctrl_conn_blocking = 
udev_ctrl_connection_unref(manager->ctrl_conn_blocking);
 manager->ctrl = udev_ctrl_unref(manager->ctrl);
-
-manager->fd_ep = safe_close(manager->fd_ep);
-manager->fd_signal = safe_close(manager->fd_signal);
+manager->ctrl_conn_blocking = 
udev_ctrl_connection_unref(manager->ctrl_conn_blocking);
 manager->fd_inotify = safe_close(manager->fd_inotify);
 manager->worker_watch[READ_END] = 
safe_close(manager->worker_watch[READ_END]);
 
+manager->ctrl_event = 
sd_event_source_unref(manager->ctrl_event);
+manager->uevent_event = 
sd_event_source_unref(manager->uevent_event);
+manager->inotify_event = 
sd_event_source_unref(manager->inotify_event);
+
+ 

[systemd-devel] [PATCH 6/6] udevd: event - port spawn_wait() to sd-event

2015-05-25 Thread Tom Gundersen
This allows us to drop the special sigterm handling in spawn_wait()
as this will now be passed directly to the worker event loop.

We now log failing processe at 'warning' leve, otherwise the
behavior is unchanged.
---
 src/test/test-udev.c|   7 --
 src/udev/udev-event.c   | 177 +---
 src/udev/udev.h |   2 -
 src/udev/udevadm-test.c |   8 ---
 src/udev/udevd.c|   8 ---
 5 files changed, 94 insertions(+), 108 deletions(-)

diff --git a/src/test/test-udev.c b/src/test/test-udev.c
index 23b7faa..f3953fe 100644
--- a/src/test/test-udev.c
+++ b/src/test/test-udev.c
@@ -120,11 +120,6 @@ int main(int argc, char *argv[]) {
 
 sigfillset(&mask);
 sigprocmask(SIG_SETMASK, &mask, &sigmask_orig);
-event->fd_signal = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
-if (event->fd_signal < 0) {
-fprintf(stderr, "error creating signalfd\n");
-goto out;
-}
 
 /* do what devtmpfs usually provides us */
 if (udev_device_get_devnode(dev) != NULL) {
@@ -153,8 +148,6 @@ int main(int argc, char *argv[]) {
3 * USEC_PER_SEC, USEC_PER_SEC,
NULL);
 out:
-if (event != NULL && event->fd_signal >= 0)
-close(event->fd_signal);
 mac_selinux_finish();
 
 return err ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 2fa26a4..b8c79b1 100644
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -32,7 +32,14 @@
 
 #include "udev.h"
 #include "rtnl-util.h"
+#include "event-util.h"
 #include "formats-util.h"
+#include "process-util.h"
+
+typedef struct Spawn {
+const char *cmd;
+pid_t pid;
+} Spawn;
 
 struct udev_event *udev_event_new(struct udev_device *dev) {
 struct udev *udev = udev_device_get_udev(dev);
@@ -45,7 +52,6 @@ struct udev_event *udev_event_new(struct udev_device *dev) {
 event->udev = udev;
 udev_list_init(udev, &event->run_list, false);
 udev_list_init(udev, &event->seclabel_list, false);
-event->fd_signal = -1;
 event->birth_usec = now(CLOCK_MONOTONIC);
 return event;
 }
@@ -540,102 +546,107 @@ static void spawn_read(struct udev_event *event,
 result[respos] = '\0';
 }
 
-static int spawn_wait(struct udev_event *event,
-  usec_t timeout_usec,
-  usec_t timeout_warn_usec,
-  const char *cmd, pid_t pid) {
-struct pollfd pfd[1];
-int err = 0;
+static int on_spawn_timeout(sd_event_source *s, uint64_t usec, void *userdata) 
{
+Spawn *spawn = userdata;
 
-pfd[0].events = POLLIN;
-pfd[0].fd = event->fd_signal;
+assert(spawn);
 
-while (pid > 0) {
-int timeout;
-int timeout_warn = 0;
-int fdcount;
+kill_and_sigcont(spawn->pid, SIGKILL);
 
-if (timeout_usec > 0) {
-usec_t age_usec;
+log_error("timeout: '%s' ["PID_FMT"], killing", spawn->cmd, 
spawn->pid);
 
-age_usec = now(CLOCK_MONOTONIC) - event->birth_usec;
-if (age_usec >= timeout_usec)
-timeout = 1000;
-else {
-if (timeout_warn_usec > 0)
-timeout_warn = ((timeout_warn_usec - 
age_usec) / USEC_PER_MSEC) + MSEC_PER_SEC;
+return 1;
+}
 
-timeout = ((timeout_usec - timeout_warn_usec - 
age_usec) / USEC_PER_MSEC) + MSEC_PER_SEC;
-}
-} else {
-timeout = -1;
-}
+static int on_spawn_timeout_warning(sd_event_source *s, uint64_t usec, void 
*userdata) {
+Spawn *spawn = userdata;
 
-fdcount = poll(pfd, 1, timeout_warn);
-if (fdcount < 0) {
-if (errno == EINTR)
-continue;
-err = -errno;
-log_error_errno(errno, "failed to poll: %m");
-goto out;
-}
-if (fdcount == 0) {
-log_warning("slow: '%s' ["PID_FMT"]", cmd, pid);
+assert(spawn);
 
-fdcount = poll(pfd, 1, timeout);
-if (fdcount < 0) {
-if (errno == EINTR)
-continue;
-err = -errno;
-log_error_errno(errno, "failed to poll: %m");
-goto out;
-}
-if (fdcount == 0) {
-log_error("timeout: killing '%s' ["PID_FMT"]", 
cmd, pid);
- 

[systemd-devel] [PATCH 3/6] udevd: explicitly try to start event queue when it may be possible

2015-05-25 Thread Tom Gundersen
Rather than trying to schedule new events on every main-loop iteration, do it 
explicitly when
processing an event finishes, a worker is killed, a new uevent is received, or 
the event queue
is explicitly restarted.
---
 src/udev/udevd.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index e309def..c9b0ed5 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -849,6 +849,9 @@ static int on_worker(sd_event_source *s, int fd, uint32_t 
revents, void *userdat
 event_free(worker->event);
 }
 
+/* we have free workers, try to schedule events */
+event_queue_start(manager);
+
 return 1;
 }
 
@@ -865,6 +868,9 @@ static int on_uevent(sd_event_source *s, int fd, uint32_t 
revents, void *userdat
 r = event_queue_insert(manager, dev);
 if (r < 0)
 udev_device_unref(dev);
+else
+/* we have fresh events, try to schedule them */
+event_queue_start(manager);
 }
 
 return 1;
@@ -903,6 +909,7 @@ static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t 
revents, void *userd
 if (udev_ctrl_get_start_exec_queue(ctrl_msg) > 0) {
 log_debug("udevd message (START_EXEC_QUEUE) received");
 manager->stop_exec_queue = false;
+event_queue_start(manager);
 }
 
 if (udev_ctrl_get_reload(ctrl_msg) > 0) {
@@ -1169,6 +1176,9 @@ static int on_sigchld(sd_event_source *s, const struct 
signalfd_siginfo *si, voi
 worker_free(worker);
 }
 
+/* we can start new workers, try to schedule events */
+event_queue_start(manager);
+
 return 1;
 }
 
@@ -1672,9 +1682,6 @@ int main(int argc, char *argv[]) {
 if (is_uevent)
 on_uevent(NULL, manager->fd_uevent, 0, manager);
 
-/* start new events */
-event_queue_start(manager);
-
 if (is_signal) {
 struct signalfd_siginfo fdsi;
 ssize_t size;
-- 
2.3.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 5/6] units: udevd - hook up watchdog support

2015-05-25 Thread Tom Gundersen
---
 units/systemd-udevd.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/units/systemd-udevd.service.in b/units/systemd-udevd.service.in
index 32f04d9..e7216d6 100644
--- a/units/systemd-udevd.service.in
+++ b/units/systemd-udevd.service.in
@@ -23,3 +23,4 @@ RestartSec=0
 ExecStart=@rootlibexecdir@/systemd-udevd
 MountFlags=slave
 KillMode=mixed
+WatchdogSec=1min
-- 
2.3.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 2/6] udevd: only check for changed config before scheduling new events

2015-05-25 Thread Tom Gundersen
Also move builtin and rules initialization from main loop to
event_queue_start().

No functional change.
---
 src/udev/udevd.c | 42 +-
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 91fe3d9..e309def 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -83,6 +83,8 @@ typedef struct Manager {
 int fd_worker;
 int worker_watch[2];
 
+usec_t last_usec;
+
 bool stop_exec_queue:1;
 bool reload:1;
 bool exit:1;
@@ -732,6 +734,28 @@ static void event_queue_start(Manager *manager) {
 
 assert(manager);
 
+if (udev_list_node_is_empty(&manager->events) ||
+manager->exit || manager->stop_exec_queue)
+return;
+
+/* check for changed config, every 3 seconds at most */
+if (manager->last_usec == 0 ||
+(now(CLOCK_MONOTONIC) - manager->last_usec) > 3 * USEC_PER_SEC) {
+if (udev_rules_check_timestamp(manager->rules) ||
+udev_builtin_validate(manager->udev))
+manager_reload(manager);
+
+manager->last_usec = now(CLOCK_MONOTONIC);
+}
+
+udev_builtin_init(manager->udev);
+
+if (!manager->rules) {
+manager->rules = udev_rules_new(manager->udev, 
arg_resolve_names);
+if (!manager->rules)
+return;
+}
+
 udev_list_node_foreach(loop, &manager->events) {
 struct event *event = node_to_event(loop);
 
@@ -1557,7 +1581,6 @@ int main(int argc, char *argv[]) {
 sd_notify(1, "READY=1");
 
 for (;;) {
-static usec_t last_usec;
 struct epoll_event ev[8];
 int fdcount;
 int timeout;
@@ -1641,15 +1664,6 @@ int main(int argc, char *argv[]) {
 is_ctrl = true;
 }
 
-/* check for changed config, every 3 seconds at most */
-if ((now(CLOCK_MONOTONIC) - last_usec) > 3 * USEC_PER_SEC) {
-if (udev_rules_check_timestamp(manager->rules) ||
-udev_builtin_validate(manager->udev))
-manager_reload(manager);
-
-last_usec = now(CLOCK_MONOTONIC);
-}
-
 /* event has finished */
 if (is_worker)
 on_worker(NULL, manager->fd_worker, 0, manager);
@@ -1659,13 +1673,7 @@ int main(int argc, char *argv[]) {
 on_uevent(NULL, manager->fd_uevent, 0, manager);
 
 /* start new events */
-if (!udev_list_node_is_empty(&manager->events) && 
!manager->exit && !manager->stop_exec_queue) {
-udev_builtin_init(manager->udev);
-if (!manager->rules)
-manager->rules = udev_rules_new(manager->udev, 
arg_resolve_names);
-if (manager->rules)
-event_queue_start(manager);
-}
+event_queue_start(manager);
 
 if (is_signal) {
 struct signalfd_siginfo fdsi;
-- 
2.3.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 1/6] udevd: introduce manager_exit() and manager_reload()

2015-05-25 Thread Tom Gundersen
The behavior is mostly unchanged, but rather than only ever calling these 
functions at
fixed points in the event loop, they are called directly whenever they are 
invoked.
---
 src/udev/udevd.c | 80 +++-
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index b33a262..91fe3d9 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -690,6 +690,43 @@ static bool is_devpath_busy(Manager *manager, struct event 
*event) {
 return false;
 }
 
+static void manager_exit(Manager *manager) {
+
+assert(manager);
+
+manager->exit = true;
+
+/* close sources of new events and discard buffered events */
+if (manager->fd_ctrl >= 0) {
+epoll_ctl(manager->fd_ep, EPOLL_CTL_DEL, manager->fd_ctrl, 
NULL);
+manager->fd_ctrl = safe_close(manager->fd_ctrl);
+}
+
+if (manager->monitor) {
+epoll_ctl(manager->fd_ep, EPOLL_CTL_DEL, manager->fd_uevent, 
NULL);
+manager->monitor = udev_monitor_unref(manager->monitor);
+}
+
+if (manager->fd_inotify >= 0) {
+epoll_ctl(manager->fd_ep, EPOLL_CTL_DEL, manager->fd_inotify, 
NULL);
+manager->fd_inotify = safe_close(manager->fd_inotify);
+}
+
+/* discard queued events and kill workers */
+event_queue_cleanup(manager, EVENT_QUEUED);
+manager_kill_workers(manager);
+}
+
+/* reload requested, HUP signal received, rules changed, builtin changed */
+static void manager_reload(Manager *manager) {
+
+assert(manager);
+
+manager_kill_workers(manager);
+manager->rules = udev_rules_unref(manager->rules);
+udev_builtin_exit(manager->udev);
+}
+
 static void event_queue_start(Manager *manager) {
 struct udev_list_node *loop;
 
@@ -846,7 +883,7 @@ static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t 
revents, void *userd
 
 if (udev_ctrl_get_reload(ctrl_msg) > 0) {
 log_debug("udevd message (RELOAD) received");
-manager->reload = true;
+manager_reload(manager);
 }
 
 str = udev_ctrl_get_set_env(ctrl_msg);
@@ -885,7 +922,7 @@ static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t 
revents, void *userd
 
 if (udev_ctrl_get_exit(ctrl_msg) > 0) {
 log_debug("udevd message (EXIT) received");
-manager->exit = true;
+manager_exit(manager);
 /* keep reference to block the client until we exit
TODO: deal with several blocking exit requests */
 manager->ctrl_conn_blocking = 
udev_ctrl_connection_ref(ctrl_conn);
@@ -1043,7 +1080,7 @@ static int on_sigterm(sd_event_source *s, const struct 
signalfd_siginfo *si, voi
 
 assert(manager);
 
-manager->exit = true;
+manager_exit(manager);
 
 return 1;
 }
@@ -1053,7 +1090,7 @@ static int on_sighup(sd_event_source *s, const struct 
signalfd_siginfo *si, void
 
 assert(manager);
 
-manager->reload = true;
+manager_reload(manager);
 
 return 1;
 }
@@ -1528,26 +1565,6 @@ int main(int argc, char *argv[]) {
 int i;
 
 if (manager->exit) {
-/* close sources of new events and discard buffered 
events */
-if (manager->fd_ctrl >= 0) {
-epoll_ctl(manager->fd_ep, EPOLL_CTL_DEL, 
manager->fd_ctrl, NULL);
-manager->fd_ctrl = 
safe_close(manager->fd_ctrl);
-}
-
-if (manager->monitor) {
-epoll_ctl(manager->fd_ep, EPOLL_CTL_DEL, 
manager->fd_uevent, NULL);
-manager->monitor = 
udev_monitor_unref(manager->monitor);
-}
-
-if (manager->fd_inotify >= 0) {
-epoll_ctl(manager->fd_ep, EPOLL_CTL_DEL, 
manager->fd_inotify, NULL);
-manager->fd_inotify = 
safe_close(manager->fd_inotify);
-}
-
-/* discard queued events and kill workers */
-event_queue_cleanup(manager, EVENT_QUEUED);
-manager_kill_workers(manager);
-
 /* exit after all has cleaned up */
 if (udev_list_node_is_empty(&manager->events) && 
hashmap_isempty(manager->workers))
 break;
@@ -1626,22 +1643,13 @@ int main(int argc, char *argv[]) {
 
 /* check for changed config, every 3 seconds at most */
 if ((now(CLOCK_MONOTONIC) - last_usec) > 3 * USEC_PER_SEC) {
-if (udev_rules_check_timestamp(manager->rules))
-manager->reload = true;

Re: [systemd-devel] [PATCH] swap: use swapon -o

2015-05-25 Thread Tom Gundersen
Applied. Thanks!

Tom

On Mon, May 25, 2015 at 12:11 PM, Karel Zak  wrote:
> This patch simplify swapon usage in systemd. The command swapon(8)
> since util-linux v2.26 supports "-o ". The idea is exactly the
> same like for mount(8). The -o specifies options in fstab-compatible
> way. For systemd it means that it does not have to care about things
> like "discard" or another swapon specific options.
>
> swapon -o 
>
> For backward compatibility the code cares about "Priority:" swap unit
> field (for a case when Priority: is set, but pri= in the Options: is
> missing).
>
> References: 
> http://lists.freedesktop.org/archives/systemd-devel/2014-October/023576.html
> ---
> V2:
>  - update README
>  - add hint to systemd.swap man page
>  - don't care about pri= in systed-fstab-generator at all
>  - add warning about duplicate priority configuration
>  - use warning rather than notice for non-parsable pri=
>
>  README|  2 +-
>  man/systemd.swap.xml  |  3 ++-
>  src/core/swap.c   | 43 
> +++
>  src/fstab-generator/fstab-generator.c | 28 ---
>  4 files changed, 26 insertions(+), 50 deletions(-)
>
> diff --git a/README b/README
> index 039110e..2b8c68e 100644
> --- a/README
> +++ b/README
> @@ -136,7 +136,7 @@ REQUIREMENTS:
>  During runtime, you need the following additional
>  dependencies:
>
> -util-linux >= v2.25 required
> +util-linux >= v2.26 required
>  dbus >= 1.4.0 (strictly speaking optional, but recommended)
>  dracut (optional)
>  PolicyKit (optional)
> diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
> index 5016f45..c398677 100644
> --- a/man/systemd.swap.xml
> +++ b/man/systemd.swap.xml
> @@ -177,7 +177,8 @@
>
>  Swap priority to use when activating the swap
>  device or file. This takes an integer. This setting is
> -optional.
> +optional and ignored when priotiry is set by pri= 
> in the
> +Options= option.
>
>
>
> diff --git a/src/core/swap.c b/src/core/swap.c
> index 12ebf84..193c8c3 100644
> --- a/src/core/swap.c
> +++ b/src/core/swap.c
> @@ -717,8 +717,8 @@ fail:
>  }
>
>  static void swap_enter_activating(Swap *s) {
> -_cleanup_free_ char *discard = NULL;
> -int r, priority = -1;
> +_cleanup_free_ char *opts = NULL;
> +int r;
>
>  assert(s);
>
> @@ -726,13 +726,21 @@ static void swap_enter_activating(Swap *s) {
>  s->control_command = s->exec_command + SWAP_EXEC_ACTIVATE;
>
>  if (s->from_fragment) {
> -fstab_filter_options(s->parameters_fragment.options, 
> "discard\0", NULL, &discard, NULL);
> +int priority = -1;
>
> -priority = s->parameters_fragment.priority;
> -if (priority < 0) {
> -r = fstab_find_pri(s->parameters_fragment.options, 
> &priority);
> +r = fstab_find_pri(s->parameters_fragment.options, 
> &priority);
> +if (r < 0)
> +log_warning_errno(r, "Failed to parse swap priority 
> \"%s\", ignoring: %m", s->parameters_fragment.options);
> +else if (r == 1 && s->parameters_fragment.priority >= 0)
> +log_warning("Duplicate swap priority configuration 
> by Priority and Options fields.");
> +
> +if (r <= 0 && s->parameters_fragment.priority >= 0) {
> +if (s->parameters_fragment.options)
> +r = asprintf(&opts, "%s,pri=%i", 
> s->parameters_fragment.options, s->parameters_fragment.priority);
> +else
> +r = asprintf(&opts, "pri=%i", 
> s->parameters_fragment.priority);
>  if (r < 0)
> -log_notice_errno(r, "Failed to parse swap 
> priority \"%s\", ignoring: %m", s->parameters_fragment.options);
> +goto fail;
>  }
>  }
>
> @@ -740,24 +748,9 @@ static void swap_enter_activating(Swap *s) {
>  if (r < 0)
>  goto fail;
>
> -if (priority >= 0) {
> -char p[DECIMAL_STR_MAX(int)];
> -
> -sprintf(p, "%i", priority);
> -r = exec_command_append(s->control_command, "-p", p, NULL);
> -if (r < 0)
> -goto fail;
> -}
> -
> -if (discard && !streq(discard, "none")) {
> -const char *discard_arg;
> -
> -if (streq(discard, "all"))
> -discard_arg = "--discard";
> -else
> -discard_arg = strjoina("--discard=", discard);
> -
> -r = exec_command_append(s->control_command, discard_arg, 
> NULL);
> +if (s->parameters_fragment.options || opts) {
> +

Re: [systemd-devel] [PATCH] build-sys: fix headers installation

2015-05-25 Thread Tom Gundersen
Applied. Thanks!

Tom

On Mon, May 25, 2015 at 1:35 PM, Marc-Antoine Perennou
 wrote:
> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 4933e6f..8e38010 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3128,7 +3128,7 @@ pkginclude_HEADERS += \
> src/systemd/sd-bus.h \
> src/systemd/sd-bus-protocol.h \
> src/systemd/sd-bus-vtable.h \
> -   src/systemd/sd-event.h
> +   src/systemd/sd-event.h \
> src/systemd/sd-login.h \
> src/systemd/sd-id128.h \
> src/systemd/sd-daemon.h
> --
> 2.4.1
>
> ___
> 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 2/2] build-sys: don't dist generated files

2015-05-25 Thread Tom Gundersen
Applied, with minor fix. Please verify that it still works for you!

Tom

On Mon, May 25, 2015 at 11:18 AM, Marc-Antoine Perennou
 wrote:
> ---
>  Makefile.am | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 9420879..4933e6f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1259,8 +1259,7 @@ DISTCLEANFILES = \
>
>  EXTRA_DIST += \
> $(gperf_gperf_m4_sources) \
> -   $(gperf_gperf_sources) \
> -   $(gperf_txt_sources:-list.txt=-from-name.gperf)
> +   $(gperf_gperf_sources)
>
>  CLEANFILES += \
> $(gperf_txt_sources)
> @@ -4652,7 +4651,9 @@ libsystemd_journal_internal_la_SOURCES = \
> src/journal/mmap-cache.h \
> src/journal/compress.c \
> src/journal/audit-type.h \
> -   src/journal/audit-type.c \
> +   src/journal/audit-type.c
> +
> +nodist_libsystemd_journal_internal_la_SOURCES = \
> src/journal/audit_type-to-name.h
>
>  gperf_txt_sources += \
> @@ -5665,7 +5666,9 @@ systemd_resolved_SOURCES = \
> src/resolve/resolved-dns-stream.h \
> src/resolve/resolved-dns-stream.c \
> src/resolve/dns-type.c \
> -   src/resolve/dns-type.h \
> +   src/resolve/dns-type.h
> +
> +nodist_systemd_resolved_SOURCES = \
> src/resolve/dns_type-from-name.h \
> src/resolve/dns_type-to-name.h
>
> @@ -5766,7 +5769,9 @@ systemd_resolve_host_SOURCES = \
> src/resolve/resolved-dns-domain.c \
> src/resolve/resolved-dns-domain.h \
> src/resolve/dns-type.c \
> -   src/resolve/dns-type.h \
> +   src/resolve/dns-type.h
> +
> +nodist_systemd_resolve_host_SOURCES = \
> src/resolve/dns_type-from-name.h \
> src/resolve/dns_type-to-name.h
>
> --
> 2.4.1
>
> ___
> 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 1/2] build-sys: always include src/boot/efi in tarballs

2015-05-25 Thread Tom Gundersen
Applied. Thanks!

Tom

On Mon, May 25, 2015 at 11:18 AM, Marc-Antoine Perennou
 wrote:
> currently it would only be included if configure was ran with --enable-gnuefi
> ---
>  Makefile.am | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 70d4dc0..9420879 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2601,6 +2601,8 @@ EFI_FORMAT = -O binary
>  else
>  EFI_FORMAT = --target=efi-app-$(EFI_ARCH)
>  endif
> +endif
> +endif
>
>  # 
> --
>  systemd_boot_headers = \
> @@ -2616,13 +2618,16 @@ systemd_boot_sources = \
> src/boot/efi/pefile.c \
> src/boot/efi/boot.c
>
> +EXTRA_DIST += $(systemd_boot_sources) $(systemd_boot_headers)
> +
> +if ENABLE_EFI
> +if HAVE_GNUEFI
>  systemd_boot_objects = $(addprefix 
> $(top_builddir)/,$(systemd_boot_sources:.c=.o))
>  systemd_boot_solib = $(top_builddir)/src/boot/efi/systemd_boot.so
>  systemd_boot = systemd-boot$(EFI_MACHINE_TYPE_NAME).efi
>
>  bootlib_DATA = $(systemd_boot)
>  CLEANFILES += $(systemd_boot_objects) $(systemd_boot_solib) $(systemd_boot)
> -EXTRA_DIST += $(systemd_boot_sources) $(systemd_boot_headers)
>
>  $(top_builddir)/src/boot/efi/%.o: $(top_srcdir)/src/boot/efi/%.c $(addprefix 
> $(top_srcdir)/,$(systemd_boot_headers))
> @$(MKDIR_P) $(top_builddir)/src/boot/efi/
> @@ -2636,6 +2641,8 @@ $(systemd_boot_solib): $(systemd_boot_objects)
>  $(systemd_boot): $(systemd_boot_solib)
> $(AM_V_GEN)$(OBJCOPY) -j .text -j .sdata -j .data -j .dynamic \
>   -j .dynsym -j .rel -j .rela -j .reloc $(EFI_FORMAT) $< $@
> +endif
> +endif
>
>  # 
> --
>  stub_headers = \
> @@ -2653,13 +2660,16 @@ stub_sources = \
> src/boot/efi/linux.c \
> src/boot/efi/stub.c
>
> +EXTRA_DIST += $(stub_sources) $(stub_headers)
> +
> +if ENABLE_EFI
> +if HAVE_GNUEFI
>  stub_objects = $(addprefix $(top_builddir)/,$(stub_sources:.c=.o))
>  stub_solib = $(top_builddir)/src/boot/efi/stub.so
>  stub = linux$(EFI_MACHINE_TYPE_NAME).efi.stub
>
>  bootlib_DATA += $(stub)
>  CLEANFILES += $(stub_objects) $(stub_solib) $(stub)
> -EXTRA_DIST += $(stub_sources) $(stub_headers)
>
>  $(top_builddir)/src/boot/efi/%.o: $(top_srcdir)/src/boot/efi/%.c $(addprefix 
> $(top_srcdir)/,$(stub_headers))
> @$(MKDIR_P) $(top_builddir)/src/boot/efi/
> --
> 2.4.1
>
> ___
> 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] Correct path to systemd-fsck in generated systemd-fsck-root.service

2015-05-24 Thread Tom Gundersen
On Sun, May 24, 2015 at 10:33 PM, Mike Gilbert  wrote:
> ---
>  Makefile.am| 1 +
>  src/shared/generator.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index f84a28d..70d4dc0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -188,6 +188,7 @@ AM_CPPFLAGS = \
> -DCATALOG_DATABASE=\"$(catalogstatedir)/database\" \
> 
> -DSYSTEMD_CGROUP_AGENT_PATH=\"$(rootlibexecdir)/systemd-cgroups-agent\" \
> -DSYSTEMD_BINARY_PATH=\"$(rootlibexecdir)/systemd\" \
> +   -DSYSTEMD_FSCK_PATH=\"$(rootlibexecdir)/systemd-fsck\" \
> -DSYSTEMD_SHUTDOWN_BINARY_PATH=\"$(rootlibexecdir)/systemd-shutdown\" 
> \
> -DSYSTEMD_SLEEP_BINARY_PATH=\"$(rootlibexecdir)/systemd-sleep\" \
> -DSYSTEMCTL_BINARY_PATH=\"$(rootbindir)/systemctl\" \
> diff --git a/src/shared/generator.c b/src/shared/generator.c
> index 8128499..807569a 100644
> --- a/src/shared/generator.c
> +++ b/src/shared/generator.c
> @@ -61,7 +61,7 @@ static int write_fsck_sysroot_service(const char *dir, 
> const char *what) {
>  "[Service]\n"
>  "Type=oneshot\n"
>  "RemainAfterExit=yes\n"
> -"ExecStart=/usr/lib/systemd/systemd-fsck %2$s\n"
> +"ExecStart=" SYSTEMD_FSCK_PATH " %2$s\n"
>  "TimeoutSec=0\n",
>  program_invocation_short_name,
>  what,
> --
> 2.4.1
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Applied. Thanks!

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] core: if PR_SET_CHILD_SUBREAPER fails, log_error instead of warning

2015-05-24 Thread Tom Gundersen
On Sat, May 23, 2015 at 6:04 PM, Cristian Rodríguez
 wrote:
> It was a warning when we still supported kernel < 3.4. current
> minimum version is 3.7.

Hm, we don't actually fail out here, but we still try to continue.
Isn't 'warning' more appropriate in that case?

Cheers,
Tom

>  src/core/main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/src/core/main.c b/src/core/main.c
> index c39815b..3bebc98 100644
> --- a/src/core/main.c
> +++ b/src/core/main.c
> @@ -1608,9 +1608,7 @@ int main(int argc, char *argv[]) {
>  if (arg_running_as == MANAGER_USER) {
>  /* Become reaper of our children */
>  if (prctl(PR_SET_CHILD_SUBREAPER, 1) < 0) {
> -log_warning_errno(errno, "Failed to make us a 
> subreaper: %m");
> -if (errno == EINVAL)
> -log_info("Perhaps the kernel version is too 
> old (< 3.4?)");
> +log_error_errno(errno, "Failed to make us a 
> subreaper: %m");
>  }
>  }
>
> --
> 2.4.1
>
> ___
> 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] nspawn: be verbose about interface names

2015-05-24 Thread Tom Gundersen
On Fri, May 22, 2015 at 4:02 PM, Umut Tezduyar Lindskog
 wrote:
> Allowed interface name is relatively small. Lets not make
> users go in to the source code to figure out what happened.
>
> --machine=debian-tree conflicts with
> --machine=debian-tree2
>
> ex: Failed to add new veth \
>  interfaces (host0, vb-debian-tree): File exists
> ---
>  src/nspawn/nspawn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index 5009363..646edea 100644
> --- a/src/nspawn/nspawn.c
> +++ b/src/nspawn/nspawn.c
> @@ -2627,7 +2627,7 @@ static int setup_veth(pid_t pid, char 
> iface_name[IFNAMSIZ], int *ifi) {
>
>  r = sd_rtnl_call(rtnl, m, 0, NULL);
>  if (r < 0)
> -return log_error_errno(r, "Failed to add new veth 
> interfaces: %m");
> +return log_error_errno(r, "Failed to add new veth interfaces 
> (host0, %s): %m", iface_name);
>
>  i = (int) if_nametoindex(iface_name);
>  if (i <= 0)
> --
> 2.1.4
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Applied. Thansk!

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] udev now crashes in daemon mode

2015-05-24 Thread Tom Gundersen
Hi Mantas,



On Sun, May 24, 2015 at 11:40 AM, Mantas Mikulėnas  wrote:
> So, udev v220 crashes in my initramfs with the following message:
>
>> starting version v220
>> Assertion 'manager->pid == getpid()' failed at src/udev/udevd.c:568,
>> function ev
>> Aborting.
>
> It seems main calls manager_new() before forking, so the parent PID is
> stored instead of child PID.
>
> (I'm using Arch Linux with the traditional mkinitcpio-based initramfs, which
> starts udev using "systemd-udevd --daemon --resolve-names=never".)

Thanks for the report. This should be fixed now in git, please let me
know if that is not the case.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Remove accelerometer helper

2015-05-22 Thread Tom Gundersen
On Fri, May 22, 2015 at 3:42 PM, Bastien Nocera  wrote:
> It's moved to the iio-sensor-proxy D-Bus service.

Nice! When was this released? Should we expect all distros to have
picked up this yet?

> ---
>  .gitignore |   1 -
>  Makefile.am|  15 --
>  rules/61-accelerometer.rules   |   3 -
>  src/udev/accelerometer/Makefile|   1 -
>  src/udev/accelerometer/accelerometer.c | 303 
> -
>  5 files changed, 323 deletions(-)
>  delete mode 100644 rules/61-accelerometer.rules
>  delete mode 12 src/udev/accelerometer/Makefile
>  delete mode 100644 src/udev/accelerometer/accelerometer.c
>
> diff --git a/.gitignore b/.gitignore
> index d2f1a1f..0eaa65f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -26,7 +26,6 @@
>  /GRTAGS
>  /GSYMS
>  /GTAGS
> -/accelerometer
>  /ata_id
>  /bootctl
>  /build-aux
> diff --git a/Makefile.am b/Makefile.am
> index f84a28d..90a78a9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -4087,21 +4087,6 @@ dist_udevrules_DATA += \
> rules/60-persistent-v4l.rules
>
>  # ---
> ---
> -accelerometer_SOURCES = \
> -   src/udev/accelerometer/accelerometer.c
> -
> -accelerometer_LDADD = \
> -   libudev-internal.la \
> -   libsystemd-internal.la \
> -   libsystemd-shared.la
> -
> -udevlibexec_PROGRAMS += \
> -   accelerometer
> -
> -dist_udevrules_DATA += \
> -   rules/61-accelerometer.rules
> -
> -# ---
> ---
>  if ENABLE_GUDEV
>  if ENABLE_GTK_DOC
>  SUBDIRS += \
> diff --git a/rules/61-accelerometer.rules b/rules/61
> -accelerometer.rules
> deleted file mode 100644
> index a6a2bfd..000
> --- a/rules/61-accelerometer.rules
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -# do not edit this file, it will be overwritten on update
> -
> -SUBSYSTEM=="input", ACTION!="remove",
> ENV{ID_INPUT_ACCELEROMETER}=="1", IMPORT{program}="accelerometer %p"
> diff --git a/src/udev/accelerometer/Makefile
> b/src/udev/accelerometer/Makefile
> deleted file mode 12
> index d0b0e8e..000
> --- a/src/udev/accelerometer/Makefile
> +++ /dev/null
> @@ -1 +0,0 @@
> -../Makefile
> \ No newline at end of file
> diff --git a/src/udev/accelerometer/accelerometer.c
> b/src/udev/accelerometer/accelerometer.c
> deleted file mode 100644
> index 9e2c590..000
> --- a/src/udev/accelerometer/accelerometer.c
> +++ /dev/null
> @@ -1,303 +0,0 @@
> -/*
> - * accelerometer - exports device orientation through property
> - *
> - * When an "change" event is received on an accelerometer,
> - * open its device node, and from the value, as well as the previous
> - * value of the property, calculate the device's new orientation,
> - * and export it as ID_INPUT_ACCELEROMETER_ORIENTATION.
> - *
> - * Possible values are:
> - * undefined
> - * * normal
> - * * bottom-up
> - * * left-up
> - * * right-up
> - *
> - * The property will be persistent across sessions, and the new
> - * orientations can be deducted from the previous one (it allows
> - * for a threshold for switching between opposite ends of the
> - * orientation).
> - *
> - * Copyright (C) 2011 Red Hat, Inc.
> - * Author:
> - *   Bastien Nocera 
> - *
> - * orientation_calc() from the sensorfw package
> - * Copyright (C) 2009-2010 Nokia Corporation
> - * Authors:
> - *   Üstün Ergenoglu 
> - *   Timo Rongas 
> - *   Lihan Guo 
> - *
> - * This program is free software; you can redistribute it and/or
> modify
> - * it under the terms of the GNU General Public License as published
> by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program 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 General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> along
> - * with this program; if not, write to the Free Software Foundation,
> Inc.,
> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> - */
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include "libudev.h"
> -#include "libudev-private.h"
> -
> -/* we must use this kernel-compatible implementation */
> -#define BITS_PER_LONG (sizeof(unsigned long) * 8)
> -#define NBITS(x) x)-1)/BITS_PER_LONG)+1)
> -#define OFF(x)  ((x)%BITS_PER_LONG)
> -#define BIT(x)  (1UL< -#define LONG(x) ((x)/BITS_PER_LONG)
> -#define test_bit(bit, array)((array[LONG(bit)] >> OFF(bit)) & 1)
> -
> -typedef enum {
> -ORIENTATION_UNDEFINED,
> -ORIENTATION_NORMAL,
> -ORIENTATION_BOTTOM_UP,
> -ORIENTATION_LEFT_UP,
> -ORIENTATION_RIGHT_UP
> -} OrientationUp;
> -
> -static const char *orien

Re: [systemd-devel] [PATCH v2] networkd: do not change kernel forwarding parameters when IPForwarding is unset

2015-05-21 Thread Tom Gundersen
On Mon, May 18, 2015 at 6:55 PM, Lennart Poettering
 wrote:
> On Fri, 15.05.15 22:49, Tom Gundersen (t...@jklm.no) wrote:
>
>> On Fri, May 15, 2015 at 10:02 PM, Lennart Poettering
>>  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...
>
> Well, I think it is fine if downstream departs from our defaults...

Ok, I guess. As there appears to be no better way.

> Another idea would be to add DefaultIPForward= to
> /etc/systemd/networkd.conf that alters defaults for networks, the same
> way as we have DefaultXYZ= in /etc/systemd/system.conf that affects
> unit defaults?

In principle it sounds like a feature we could add, but I think we
should have a better usecase than just backwards compatibility, then
I'd rather go with the compile-time switch I guess.

-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] networkd: dbus API for networkd reconfiguration at run-time

2015-05-21 Thread Tom Gundersen
On Tue, May 19, 2015 at 10:40 AM, Rauta, Alin  wrote:
> Hi Lennart,
>
> Thanks for the answers.
>
> One more questions. Just a curiosity of mine.
>
> Currently, a user has to write scripts if he wants to save the run-time 
> configuration in networkd format or to use a configuration management tool 
> like chef for example.
> Even with the latter, the user still needs to write scripts.
>
> What about saving the run-time configuration in networkd format with 
> "networkctl" maybe ?
> Something like "networkctl save" or "networkctl save config" with extensions 
> to provide per port configuration saving, output directory for saved 
> configuration and so on ... ?

Not entirely sure I understand the question, but this is what I have
in mind: we support three config sources at the moment: stuff in /lib
which is shipped by packages, stuff in /etc that is provided by the
admin and is persistent between reboots and stuff in /run which is
lost on reboot. Our future API should have a switch allowing the
config we apply to either be persistent (written out to /etc) or
ephemeral (written out to /run), so that any config in networkd is
backed by a config file somewhere. Does that make sense? Does it
answer your question, or is there some other type of config you would
like to write out?

Cheers,

Tom
___
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-21 Thread Tom Gundersen
On Wed, May 20, 2015 at 8:05 PM, Michael Marineau
 wrote:
> 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

"no-carrier configuring" could easily happen as we would enslave a
device and add routes/addresses even before the device gains a
carrier.

> and there is no .network config to apply to the
> interface either.

Now this is strange. A link should never be in "configuring" state
unless we actually have a .network file to apply to it.

> 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.

Not sure to be honest, I'll try to keep an eye on this, but as I said
I have not been able to reproduce.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Fixed incorrect type for MTU

2015-05-21 Thread Tom Gundersen
On Thu, May 21, 2015 at 12:54 PM, dmoneil2  wrote:
> The parser uses size_t for MTU however the structure is defined with unsigned 
> int
> as the target type resulting in a value corruption for the next field of the 
> structure.
>
> dmoneil2 (1):
>   Fixed issue with corruption of speed value when setting on x64
> systems.
>
>  src/udev/net/link-config.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks for the patch!  I now pushed a similar fix, please verify that
it works for you.

Cheers,

Tom
___
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 Tom Gundersen
On Tue, Apr 21, 2015 at 11:59 PM, Nick Owens  wrote:
> hello tom,
>
> On Mon, Apr 20, 2015 at 2:32 PM, Tom Gundersen  wrote:
>> On Fri, Apr 3, 2015 at 12:48 AM, Michael Marineau
>>  wrote:
>>> On Thu, Apr 2, 2015 at 3:08 PM, Nick Owens  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  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  
>>>> wrote:
>>>>> On Wed, Mar 25, 2015 at 11:49 PM,   wrote:
>>>>>> From: mischief 
>>>>>>
>>>>>> 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?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [ANNOUNCE] Separating gudev from systemd

2015-05-19 Thread Tom Gundersen
On Wed, May 20, 2015 at 8:24 AM, Martin Pitt  wrote:
> Hey David,
>
> David Herrmann [2015-05-19 17:06 +0200]:
>> We're about to remove gudev from the systemd repository, as it is in
>> no way related to the systemd code-base, nor used by the systemd
>> project.
>
> This makes sense indeed. gudev used to be a standalone project before
> it was merged into udev, so the circle is complete now :-)
>
> For those of us who already packaged gudev from systemd 219, would it
> be possible to bump the current release to 220, so that gudev can be
> packaged without renaming the tarball and doing ugly version numbers?
> Monotonously increasing version numbers and all.. (Yes, there are
> "epochs" in Debian, and I'm sure RPM has these too, but they might not
> be available everywhere and are generally frowned upon)

While you are at it, why not bump it to 225 or something (just to
guarantee that the last systemd release with gudev has a lower version
number than gudev at that time, so people can switch over whenever
they want without having to worry about going backwards).

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd-networkd and systemd-nspawn: missing host-side network

2015-05-19 Thread Tom Gundersen
On Mon, Apr 27, 2015 at 6:50 AM, Kai Krakow  wrote:
> I've created a container with systemd-nspawn, "machinectl enable"d it, then
> added machines.target to my default target (systemctl enable
> machines.target) so that containers will be autostarted on boot. That works
> so far.
>
> But I discovered that systemd-networkd no longer configures my normal
> ethernet device during boot (it's configured as dhcp client). It just
> configures the ve-* device and that's it. After I manually restart networkd,
> all links are configured.
>
> Steps to reproduce:
>
> $ cat /etc/systemd/network/80-dhcp.network
> [Match]
> Name=en*
> [Network]
> DHCP=yes
> [DHCP]
> UseDomains=true
>
> $ cat /etc/systemd/network/90-veth.network
> # This was added because otherwise after reboot, ve- is stuck in
> # mode "configuring" when looking at networkctl, it changes nothing
> # for the following behaviour, tho...
> [Match]
> Name=ve-*
> [Network]
> DHCP=no
>
> $ machinectl enable test-machine
> $ systemctl enable machines.target
> $ systemctl reboot
> ...[rebooting]...
>
> $ networkctl
> IDX LINK TYPE   OPERATIONAL SETUP
>   1 lo   loopback   n/a n/a
>   2 enp4s0   ether  n/a n/a
>   3 sit0 sitn/a n/a
>   4 ve-  ether  routableconfigured
>
> $ ifconfig
> # shows only lo and ve-

Hm? ifconfig does not show enp4s0? How about "ip link"?

> $ systemctl restart systemd-networkd
> $ networkctl
> IDX LINK TYPE   OPERATIONAL SETUP
>   1 lo   loopback   carrier unmanaged
>   2 enp4s0   ether  routableconfigured
>   3 sit0 sitoff unmanaged
>   4 ve-  ether  routableconfigured

Which version did you observe this in? Is this reproducible with
current git HEAD? If so, could you attach "$ networkctl status enp4s0"
and the output of journalctl -b -u systemd-networkd (preferably after
enabling debug logging in networkd by setting
Environment=SYSTEMD_LOG_LEVEL=debug in the networkd service file).

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] networkd failing sporadically failing to bring up network device

2015-05-19 Thread Tom Gundersen
On Tue, Apr 7, 2015 at 5:46 PM, Richard Maw  wrote:
> On Sat, Apr 04, 2015 at 12:08:04PM +0100, Sitsofe Wheeler wrote:
>> Hi,
>>
>> I've noticed that networkd occasionally fails to bring up one of two
>> network interfaces on boot (this happens about once every 70 or so
>> boots). The machine in question is a VMware ESXi 5.5 guest with two
>> VMXNET3 network adaptors. When this issue occurs the message
>>
>> rtnl: received address for a nonexistent link (1), ignoring
>>
>> is present in the journal.
>>
>> Does anyone know if this is a known issue in systemd 216 (I'm using
>> Fedora 21)? I've attached a journal log below:
>
> There's been a few post-217 changes related to parsing messages in rtnl,
> one of which was related to accidentally interpreting change notifications
> as responses under rare circumstances, so there are some known bugs in
> 216, but I haven't encountered yours before.
>
> Re-testing with the latest systemd snapshot would let us know if it's
> one of the bugs fixed post 217.
>
> If it comes to debugging what netlink is doing, one option is to
> use the nlmon interface, capture it with tcpdump as described in
> http://seclists.org/tcpdump/2014/q4/32 and then use wireshark to parse
> the dump.

Seems I missed parts of this thread (I cannot find the original email
in my archive), so in case I missed something important: could this be
reproduced with current git, or is this seemingly resolved now?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout

2015-05-19 Thread Tom Gundersen
On Sat, May 2, 2015 at 12:21 PM, Nir Soffer  wrote:
> On Tue, Apr 21, 2015 at 12:41 AM, Tom Gundersen  wrote:
>> On Mon, Apr 13, 2015 at 3:04 PM, Nir Soffer  wrote:
>>> On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann  
>>> wrote:
>>>>> A program running this tool can detect a timeout (expected) or an error
>>>>> (unexpected), and can change the program flow based on this result.
>>>>>
>>>>> Without this, the only way to detect a timeout is to implement the timeout
>>>>> in the program calling udevadm.
>>>>
>>>> I cannot really see a use-case here. I mean, yeah, the commit-message
>>>> says it warns about timeouts but fails loudly on real errors. But
>>>> again, what's the use-case? Why is a timeout not a real error? Why do
>>>> you need to handle it differently?
>>>
>>> Timeout means that the value I chose may be too small, or the machine
>>> is overloaded. The administrator may need to configure the system
>>> differently.
>>>
>>> Other errors are not expected, and typically unexpected errors in an
>>> underlying tool means getting the developer of the underlying tool
>>> involved.
>>>
>>>> Anyway, if it's only about diagnostics this patch seems fine to me.
>>>
>>> Yes, it is mainly about diagnostics, and making it easier to debug and 
>>> support.
>>
>> Wouldn't a better solution be to improve the udevadm logging? If we
>> change the exit codes this is basically ABI. Do we really want to make
>> such promises only for diagnostics?
>
> Improving logging is orthogonal, as it does allow the program to change
> the flow based the exit code.
>
> Adding a timeout exit code may break code using undocumented behavior,
> since the return code is not documented.

I don't really have a strong argument against this, except that we
should not add "ABI" without a strong usecase. So you mentioned that
you want this for diagnostics, to which my suggestion was to improve
the logging in udevadm itself. But are there other usecases? What is
the case where the external tool actually needs to change its
behavior?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Setting UseDomains=yes by default DHCP

2015-05-19 Thread Tom Gundersen
It seems this thread was already resolved, just wanted to add one comment:

On Mon, May 18, 2015 at 2:26 PM, Zbigniew Jędrzejewski-Szmek
 wrote:
> So to mount a successful spoof, the attacker needs to
> a) control the dhcp server domain option to return a domain under attacker 
> control

Notice that this is entirely trivial, as the attacker does not need to
control _the_ DHCP server, all they have to do is to provide a
competing DHCP server on the local link and hope they get picked over
the real one.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] networkd-218 seems to ignore .link files

2015-05-19 Thread Tom Gundersen
On Tue, May 19, 2015 at 6:57 PM, Jan Engelhardt  wrote:
>
> On Tuesday 2015-05-19 18:38, Tom Gundersen wrote:
>>>> > # networkctl status --no-pager eth0
>>>> > ??● 3: eth0
>>>> >Link File: n/a
>>>> > Network File: n/a
>>>> > Type: ether
>>>> >State: off (unmanaged)
>>>> > Path: pci-:01:00.0
>>>> >   Driver: r8169 [...]
>>>> > # cat /etc/systemd/network/01-mgmt.link
>>>> > [Match]
>>>> > Path=pci-:01:00.0
>>>> > Type=ether
>>>>
>>>> Type must be the same as what is returned in DEVTYPE=, which I guess
>>>> is unset for this device. So drop this and it should work.
>>>>
>>>> I see where the confusion stems from though, as we try to be helpful
>>>> and use a heuristic to print a Type in networkctl even when the kernel
>>>> does not expose a type. [...]
>>>
>>> What precisely is the heuristic?[...]
>>
>>There are two 'types', the ARP hardware identifier (link type as
>>exposed over rtnl) and the DEVTYPE (as exposed by libudev). The
>>matching logic only uses DEVTYPE.
>
> Unlike hwdevtype, arphrd is at least set _all the time_.

True, but not always to something useful (which is why we special case
ARPHRD_ETHER and DEVTYPE==wlan|wwan).

>>The output of networkctl (and the udev naming to a more limited degree) uses
>>the ARP hardware identifier and then falls back to DEVTYPE in the case of
>>ethernet devices to distinguish wlan and wwan interfaces from other kinds of
>>ethernet interfaces.
>>
>>In an ideal world, we would only use DEVTYPE and the kernel would
>>assign this reasonably to all devices. In the meantime we could do
>>some combination of DEVTYPE and ARPHRD, but the danger here is that we
>>will lock ourselves into some heuristic and make it impossible to
>>improve the kernel DEVTYPE's in the future as it would change the
>>behavior of current setups, which is why I have preferred to not
>>support any matching when DEVTYPE is not set. I'm very open to change
>>this if anyone has a convincing scheme in mind though.
>
> So, perhaps just making networkd support both DevType={match on DEVTYPE 
> string}
> and LinkType={ether,loopback,other ARPHRD_* constants} be the interim solution
> for that.

I'd rather we just decide on some scheme once and for all. Interim
solutions have a way of becoming permanent...

-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] networkd-218 seems to ignore .link files

2015-05-19 Thread Tom Gundersen
On Tue, Apr 21, 2015 at 6:26 PM, Lennart Poettering
 wrote:
> On Tue, 21.04.15 14:47, Tom Gundersen (t...@jklm.no) wrote:
>
>> > I'm having a similar problem while running systemd version-219. Did you 
>> > work
>> > out what was wrong?
>> >
>> > My link file is ignored even when I symlink
>> > /etc/systemd/network/99-default.link to /dev/null. I don't see anything
>> > interesting in 'journalctl'.
>> >
>> > # udevadm info /sys/class/net/eth0
>> > P: /devices/pci:00/:00:04.0/:01:00.0/net/eth0
>> > E: DEVPATH=/devices/pci:00/:00:04.0/:01:00.0/net/eth0
>> > E: ID_BUS=pci
>> > E: ID_MODEL_FROM_DATABASE=RTL8111/8168/8411 PCI Express Gigabit Ethernet
>> > Contror
>> > E: ID_MODEL_ID=0x8168
>> > E: ID_NET_DRIVER=r8169
>> > E: ID_NET_NAME_MAC=enx000db936008c
>> > E: ID_NET_NAME_PATH=enp1s0
>> > E: ID_OUI_FROM_DATABASE=PC Engines GmbH
>> > E: ID_PATH=pci-:01:00.0
>> > E: ID_PATH_TAG=pci-_01_00_0
>> > E: ID_PCI_CLASS_FROM_DATABASE=Network controller
>> > E: ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
>> > E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Co., Ltd.
>> > E: ID_VENDOR_ID=0x10ec
>> > E: IFINDEX=3
>> > E: INTERFACE=eth0
>> > E: SUBSYSTEM=net
>> > E: SYSTEMD_ALIAS=/sys/subsystem/net/devices/eth0
>> > E: TAGS=:systemd:
>> > E: USEC_INITIALIZED=53326
>> >
>> >
>> > # networkctl status --no-pager eth0
>> > ��● 3: eth0
>> >Link File: n/a
>> > Network File: n/a
>> > Type: ether
>> >State: off (unmanaged)
>> > Path: pci-:01:00.0
>> >   Driver: r8169
>> >   Vendor: Realtek Semiconductor Co., Ltd.
>> >Model: RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
>> >   HW Address: 00:0d:b9:36:00:8c (PC Engines GmbH)
>> >  MTU: 1500
>> >
>> >
>> > # cat /etc/systemd/network/01-mgmt.link
>> > [Match]
>> > Path=pci-:01:00.0
>> > Type=ether
>>
>> Type must be the same as what is returned in DEVTYPE=, which I guess
>> is unset for this device. So drop this and it should work.
>>
>> I see where the confusion stems from though, as we try to be helpful
>> and use a heuristic to print a Type in networkctl even when the kernel
>> does not expose a type. We probably should not do that, or allow the
>> same to be used in the .link matching logic (the heuristic is unlikely
>> to be perfect, so I hesitate a bit with the latter).
>
> What precisely is the heuristic? To assume that things are ethernet if
> not otherwise specified? Honestly, I think that's good enough, and
> probably widely accepted. If this goes wrong I am pretty sure that's
> something to fix in the driver, by simply exposing the type then...

There are two 'types', the ARP hardware identifier (link type as
exposed over rtnl) and the DEVTYPE (as exposed by libudev). The
matching logic only uses DEVTYPE. The output of networkctl (and the
udev naming to a more limited degree) uses the ARP hardware identifier
and then falls back to DEVTYPE in the case of ethernet devices to
distinguish wlan and wwan interfaces from other kinds of ethernet
interfaces.

In an ideal world, we would only use DEVTYPE and the kernel would
assign this reasonably to all devices. In the meantime we could do
some combination of DEVTYPE and ARPHRD, but the danger here is that we
will lock ourselves into some heuristic and make it impossible to
improve the kernel DEVTYPE's in the future as it would change the
behavior of current setups, which is why I have preferred to not
support any matching when DEVTYPE is not set. I'm very open to change
this if anyone has a convincing scheme in mind though.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] How to set primary slave in active-backup mode (bonding)

2015-05-19 Thread Tom Gundersen
On Thu, Apr 9, 2015 at 4:29 PM, Mikhail Morfikov  wrote:
> I usually have two network interfaces on my laptops (one eth and one
> wlan), and when I was using sysvinit I also was configuring the bond
> interface via the /etc/network/interfaces file so the two interfaces
> could work in the active-backup mode. But now, they work in balance-rr
> mode which is set via the .netdev file. The problem with this mode is
> that when you have, let's say wifi 30mbit/s and wired 100mbit/s, you
> can get 60mbit/s max, and that's why I wanted to use the active-backup
> mode which switches from wire to wifi and vice versa depending on
> whether the ethernet cable is plugged in. Generally speaking, I have to
> set some additional parameters so this could work well, and that would
> be:

We don't yet fully support all the bonding options.

> bond-primary eth1

This is not currently supported, I suppose we should add the
possibility of marking a slave as 'primary' to the .network file
(rather than listing the slave in the .netdev file).

> bond-primary-reselect always

This is PrimaryReselectPolicy=always in the .netdev file.

> bond-slaves eth1 wlan0

This is achieved by setting Bond= in the .network files applied to
eth1 and wlan0.

> bond-fail-over-mac none

This is FailOverMACPolicy=none in the .netdev file, which is also the
default, so is redundant.

> I'm not sure if all of them are necessary, and the question is how to
> pass these parameters in systemd? I'm asking because in the
> systemd.netdev manual, in the bond section, these options weren't
> specified.

I hope the above helps, but I suspect you really need the feature to
specify the primary slave for this to work as you intended. Happy to
take a patch!

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] unaligned write in dhcp_identifier_set_iaid

2015-05-19 Thread Tom Gundersen
On Tue, Feb 24, 2015 at 7:22 PM, Michael Olbrich
 wrote:
> there is an unaligned write in dhcp_identifier_set_iaid() and I'm not quite
> sure what the correct fix is:
>
> int dhcp_identifier_set_iaid(int ifindex, uint8_t *mac, size_t mac_len, 
> uint32_t *_id) {
> [...]
> *_id = (id & 0x) ^ (id >> 32);
> [...]
> }
>
> And this is called with:
> r = dhcp_identifier_set_iaid(client->index, client->mac_addr, 
> client->mac_addr_len, &client->client_id.ns.iaid);
>
> And iaid is not unaligned because of the packing in struct sd_dhcp_client.
> I'm not sure why '_packed_' is used there. It this just to save some space,
> or is there a reason for this?

Thanks for the report. This seems to have fallen through the cracks.
Should be fixed in git, please verify.

Sorry for the delay.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd-nspawn trouble

2015-05-18 Thread Tom Gundersen
On Sun, May 17, 2015 at 5:30 PM, Michael Biebl  wrote:
> 2015-05-15 22:16 GMT+02:00 Tom Gundersen :
>> on-demand I agree with Lennart that it makes the most sense to simply
>> unconditionally load the modules. If this is undesirable the solution
>> should be to teach the kernel to auto-load the modules, not to expect
>> the admin to figure out that explicit loading is required, IMHO.
>
> And now we expect that the admin figures out how to disable loading of
> the iptables module, which isn't anymore obvious.

Out of interest, what is the 'regression' users would experience by
having the iptables module loaded? Or is it just about the principle
of not wanting to load a module unless it is actually used?

> What I was suggesting was, that the iptables modules should only be
> loaded on demand, i.e. when the firewalling functionality is actually
> used.

If so, this should be done by the kernel.

> Lennart did argue, that he didn't want to do that within
> networkd, since he didn't want to grant networkd that capability to
> load modules and therefor to load the module unconditionally in PID 1.
> But moving the modules loading out of networkd doesn't mean, it has to
> be done unconditonally, see how we did it for
> udev/kmod-static-nodes.service

Hm, this is all about letting the kernel do the module loading lazily
on-demand, so I'd be all for that, but then the kernel would need to
learn how to do that for iptables first...

Cheers,

Tom
___
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 Tom Gundersen
On Fri, May 15, 2015 at 10:02 PM, Lennart Poettering
 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...

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd-nspawn trouble

2015-05-15 Thread Tom Gundersen
On Wed, Apr 22, 2015 at 2:36 PM, Michael Biebl  wrote:
> 2015-04-22 14:26 GMT+02:00 Lennart Poettering :
>> On Wed, 22.04.15 14:22, Michael Biebl (mbi...@gmail.com) wrote:
>>> 2015-04-22 14:14 GMT+02:00 Lennart Poettering :
>>> > On Wed, 22.04.15 14:09, Michael Biebl (mbi...@gmail.com) wrote:
>>> >> 2015-04-22 13:57 GMT+02:00 Lennart Poettering :
>>> >> > http://cgit.freedesktop.org/systemd/systemd/commit/?id=1d3087978a8ee23107cb64aa55ca97aefe9531e2
>>> >>
>>> >> Not everyone is using networkd or nspawn though, so loading this
>>> >> module for everyone is a bit excessive.
>>> >
>>> > Well, then blacklist the module or don't build it at all.
>>>
>>> That's the wrong way around.
>>
>> Nah, I disagree. We do this for a number of modules now. I mean, we
>
> We currently do this static loading for unix, ipv6 and autofs4.
>
>> load tons of modules automatically, even if you don't use them. For
>> example, my laptop always loads the bluetooth modules, even though I
>> never used bluetooth.
>
> Those are all loaded on demand, not statically. I.e. we don't load the
> bluetooth module for each and every user.

We never require the admin to explicitly opt-in to loading any modules
for the functionality we use. For hardware drivers, modules are
unconditionally loaded when the hardware exists, for other modules the
kernel is usually able to load them on demand (filesystems, netdevs,
...). In the cases where the kernel is not able to load modules
on-demand I agree with Lennart that it makes the most sense to simply
unconditionally load the modules. If this is undesirable the solution
should be to teach the kernel to auto-load the modules, not to expect
the admin to figure out that explicit loading is required, IMHO.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Will *.network replace resolv.conf? What about "Options single-request"?

2015-05-15 Thread Tom Gundersen
On Mon, May 4, 2015 at 2:57 PM, Christian Brunotte  wrote:
> systemd.network(5) with Options like "DNS=" and "Domains=" looks like
> /etc/resolv.conf will soon be superfluous.

In many setups, yes, but we will not aim at bug-for-bug compatibility
or anything like that. We are open to adding features that make sense
though.

> If that's the plan, I wonder what happens to "options single-request"
> which I had to use on all IPv6 enabled devices. Is "ResolveOptions" just
> missing in Systemd or considered so "special" that will stay in libc's
> resolv.conf?
>
> (quoting from the man page: "By default, glibc performs IPv4 and IPv6
> lookups in parallel since version 2.9. Some appliance DNS servers cannot
> handle queries properly and make the requests time out.")

Hm, this really does not sound like something that should be
configurable. Are you really seeing these bugs in the wild? And if so,
how do other OSs deal with this? I mean, having to add quirks to every
client does not sound very viable... Surely DNS is something that
should 'just work'. I feel like I'm missing some part of the picture
here.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 3/3] networkd: add man for vti6 tunnel

2015-04-29 Thread Tom Gundersen
Applied. Thanks!

Tom

On Wed, Apr 22, 2015 at 10:44 AM, Susant Sahani  wrote:
> ---
>  man/systemd.netdev.xml | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml
> index f413739..3bfd01b 100644
> --- a/man/systemd.netdev.xml
> +++ b/man/systemd.netdev.xml
> @@ -155,6 +155,9 @@
>vti
>An IPv4 over IPSec tunnel.
>
> +  vti6
> +  An IPv6 over IPSec tunnel.
> +
>vxlan
>A virtual extensible LAN (vxlan), for connecting Cloud 
> computing deployments.
>  
> @@ -441,7 +444,8 @@
>  gretap,
>  ip6gre,
>  ip6gretap,
> -vti, and
> +vti,
> +vti6, and
>  ip6tnl and accepts
>  the following keys:
>
> --
> 2.3.5
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/3] networkd: introduce vti6 tunnel

2015-04-29 Thread Tom Gundersen
Applied. Thanks!

Tom

On Wed, Apr 22, 2015 at 10:44 AM, Susant Sahani  wrote:
> This patch add support to create vti6 tunnel
>
> test:
>
> vt6.network
> [Match]
> Name=wlan0
>
> [Network]
> Tunnel=ip6vti
>
> vti6.netdev
> [NetDev]
> Name=ip6vti
> Kind=vti6
>
> [Tunnel]
> Local=2a00:ffde:4567:edde::4987
> Remote=2001:473:fece:cafe::5179
>
> ip link
>
> 11: ip6_vti0@NONE:  mtu 1500 qdisc noop state DOWN mode DEFAULT
> group default
> link/tunnel6 :: brd ::
> 12: ip6vti@wlan0:  mtu 1500 qdisc noop state DOWN
> mode DEFAULT group default
> link/tunnel6 2a00:ffde:4567:edde::4987 peer 2001:473:fece:cafe::5179
> ---
>  src/libsystemd/sd-rtnl/rtnl-types.c  |  3 +++
>  src/libsystemd/sd-rtnl/rtnl-types.h  |  1 +
>  src/network/networkd-netdev-tunnel.c | 43 
> 
>  src/network/networkd-netdev-tunnel.h |  1 +
>  src/network/networkd-netdev.c|  2 ++
>  src/network/networkd-netdev.h|  2 ++
>  src/network/networkd-network.c   |  1 +
>  7 files changed, 53 insertions(+)
>
> diff --git a/src/libsystemd/sd-rtnl/rtnl-types.c 
> b/src/libsystemd/sd-rtnl/rtnl-types.c
> index a5c9fdf..d211684 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-types.c
> +++ b/src/libsystemd/sd-rtnl/rtnl-types.c
> @@ -204,6 +204,7 @@ static const char* const 
> nl_union_link_info_data_table[_NL_UNION_LINK_INFO_DATA_
>  [NL_UNION_LINK_INFO_DATA_IP6GRETAP_TUNNEL] = "ip6gretap",
>  [NL_UNION_LINK_INFO_DATA_SIT_TUNNEL] = "sit",
>  [NL_UNION_LINK_INFO_DATA_VTI_TUNNEL] = "vti",
> +[NL_UNION_LINK_INFO_DATA_VTI6_TUNNEL] = "vti6",
>  [NL_UNION_LINK_INFO_DATA_IP6TNL_TUNNEL] = "ip6tnl",
>  };
>
> @@ -238,6 +239,8 @@ static const NLTypeSystem 
> rtnl_link_info_data_type_systems[_NL_UNION_LINK_INFO_D
>.types = 
> rtnl_link_info_data_iptun_types },
>  [NL_UNION_LINK_INFO_DATA_VTI_TUNNEL] =  { .max = 
> ELEMENTSOF(rtnl_link_info_data_ipvti_types) - 1,
>.types = 
> rtnl_link_info_data_ipvti_types },
> +[NL_UNION_LINK_INFO_DATA_VTI6_TUNNEL] =  { .max = 
> ELEMENTSOF(rtnl_link_info_data_ipvti_types) - 1,
> +  .types = 
> rtnl_link_info_data_ipvti_types },
>  [NL_UNION_LINK_INFO_DATA_IP6TNL_TUNNEL] =  { .max = 
> ELEMENTSOF(rtnl_link_info_data_ip6tnl_types) - 1,
>   .types = 
> rtnl_link_info_data_ip6tnl_types },
>
> diff --git a/src/libsystemd/sd-rtnl/rtnl-types.h 
> b/src/libsystemd/sd-rtnl/rtnl-types.h
> index 72773ea..de1544b 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-types.h
> +++ b/src/libsystemd/sd-rtnl/rtnl-types.h
> @@ -87,6 +87,7 @@ typedef enum NLUnionLinkInfoData {
>  NL_UNION_LINK_INFO_DATA_IP6GRETAP_TUNNEL,
>  NL_UNION_LINK_INFO_DATA_SIT_TUNNEL,
>  NL_UNION_LINK_INFO_DATA_VTI_TUNNEL,
> +NL_UNION_LINK_INFO_DATA_VTI6_TUNNEL,
>  NL_UNION_LINK_INFO_DATA_IP6TNL_TUNNEL,
>  _NL_UNION_LINK_INFO_DATA_MAX,
>  _NL_UNION_LINK_INFO_DATA_INVALID = -1
> diff --git a/src/network/networkd-netdev-tunnel.c 
> b/src/network/networkd-netdev-tunnel.c
> index 7aa9317..aef2ef4 100644
> --- a/src/network/networkd-netdev-tunnel.c
> +++ b/src/network/networkd-netdev-tunnel.c
> @@ -212,6 +212,31 @@ static int netdev_vti_fill_message_create(NetDev 
> *netdev, Link *link, sd_rtnl_me
>  return r;
>  }
>
> +static int netdev_vti6_fill_message_create(NetDev *netdev, Link *link, 
> sd_rtnl_message *m) {
> +Tunnel *t = VTI6(netdev);
> +int r;
> +
> +assert(netdev);
> +assert(link);
> +assert(m);
> +assert(t);
> +assert(t->family == AF_INET6);
> +
> +r = sd_rtnl_message_append_u32(m, IFLA_VTI_LINK, link->ifindex);
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_LINK attribute: %m");
> +
> +r = sd_rtnl_message_append_in6_addr(m, IFLA_VTI_LOCAL, 
> &t->local.in6);
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_LOCAL attribute: %m");
> +
> +r = sd_rtnl_message_append_in6_addr(m, IFLA_VTI_REMOTE, 
> &t->remote.in6);
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_REMOTE attribute: %m");
> +
> +return r;
> +}
> +
>  static int netdev_ip6tnl_fill_message_create(NetDev *netdev, Link *link, 
> sd_rtnl_message *m) {
>  Tunnel *t = IP6TNL(netdev);
>  uint8_t proto;
> @@ -287,6 +312,9 @@ static int netdev_tunnel_verify(NetDev *netdev, const 
> char *filename) {
>  case NETDEV_KIND_VTI:
>  t = VTI(netdev);
>  break;
> +case NETDEV_KIND_VTI6:
> +t = VTI6(netdev);
> +break;
>  case NETDEV_KIND_IP6TNL:
>  t = IP6TNL(netd

Re: [systemd-devel] [PATCH 1/3] networkd: tunnel improve logging

2015-04-29 Thread Tom Gundersen
Applied. Thanks!

Tom

On Wed, Apr 22, 2015 at 10:44 AM, Susant Sahani  wrote:
> Replaces a lof ot strerror() usage with log_netdev_error_errno()
> ---
>  src/network/networkd-netdev-tunnel.c | 240 
> ++-
>  1 file changed, 64 insertions(+), 176 deletions(-)
>
> diff --git a/src/network/networkd-netdev-tunnel.c 
> b/src/network/networkd-netdev-tunnel.c
> index 89ad3ee..7aa9317 100644
> --- a/src/network/networkd-netdev-tunnel.c
> +++ b/src/network/networkd-netdev-tunnel.c
> @@ -54,44 +54,24 @@ static int netdev_ipip_fill_message_create(NetDev 
> *netdev, Link *link, sd_rtnl_m
>  assert(t->family == AF_INET);
>
>  r = sd_rtnl_message_append_u32(m, IFLA_IPTUN_LINK, link->ifindex);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_LINK 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_LINK attribute: %m");
>
>  r = sd_rtnl_message_append_in_addr(m, IFLA_IPTUN_LOCAL, 
> &t->local.in);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_LOCAL 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_LOCAL attribute: %m");
>
>  r = sd_rtnl_message_append_in_addr(m, IFLA_IPTUN_REMOTE, 
> &t->remote.in);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_REMOTE 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_REMOTE attribute: %m");
>
>  r = sd_rtnl_message_append_u8(m, IFLA_IPTUN_TTL, t->ttl);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_TTL  
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_TTL  attribute: %m");
>
>  r = sd_rtnl_message_append_u8(m, IFLA_IPTUN_PMTUDISC, t->pmtudisc);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_PMTUDISC 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_PMTUDISC attribute: %m");
>
>  return r;
>  }
> @@ -107,44 +87,24 @@ static int netdev_sit_fill_message_create(NetDev 
> *netdev, Link *link, sd_rtnl_me
>  assert(t->family == AF_INET);
>
>  r = sd_rtnl_message_append_u32(m, IFLA_IPTUN_LINK, link->ifindex);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_LINK 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_LINK attribute: %m");
>
>  r = sd_rtnl_message_append_in_addr(m, IFLA_IPTUN_LOCAL, 
> &t->local.in);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_LOCAL 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_LOCAL attribute: %m");
>
>  r = sd_rtnl_message_append_in_addr(m, IFLA_IPTUN_REMOTE, 
> &t->remote.in);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_REMOTE 
> attribute: %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netdev, r, "Could not append 
> IFLA_IPTUN_REMOTE attribute: %m");
>
>  r = sd_rtnl_message_append_u8(m, IFLA_IPTUN_TTL, t->ttl);
> -if (r < 0) {
> -log_netdev_error(netdev,
> - "Could not append IFLA_IPTUN_TTL attribute: 
> %s",
> - strerror(-r));
> -return r;
> -}
> +if (r < 0)
> +return log_netdev_error_errno(netd

Re: [systemd-devel] [PATCH] core: fix event source annotations

2015-04-29 Thread Tom Gundersen
Applied. Thanks!

Tom

On Wed, Apr 29, 2015 at 8:29 PM, Mantas Mikulėnas  wrote:
> These looked like a mass-replace gone slightly wrong – two statements
> with no { }'s, and no error checking.
> ---
>  src/core/busname.c | 4 +++-
>  src/core/manager.c | 5 -
>  src/core/socket.c  | 3 ++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/core/busname.c b/src/core/busname.c
> index 48cc045..94db122 100644
> --- a/src/core/busname.c
> +++ b/src/core/busname.c
> @@ -291,13 +291,15 @@ static int busname_watch_fd(BusName *n) {
>  r = sd_event_source_set_enabled(n->starter_event_source, 
> SD_EVENT_ON);
>  else
>  r = sd_event_add_io(UNIT(n)->manager->event, 
> &n->starter_event_source, n->starter_fd, EPOLLIN, busname_dispatch_io, n);
> -(void) 
> sd_event_source_set_description(n->starter_event_source, "busname-starter");
> +
>  if (r < 0) {
>  log_unit_warning_errno(UNIT(n)->id, r, "Failed to watch 
> starter fd: %m");
>  busname_unwatch_fd(n);
>  return r;
>  }
>
> +(void) sd_event_source_set_description(n->starter_event_source, 
> "busname-starter");
> +
>  return 0;
>  }
>
> diff --git a/src/core/manager.c b/src/core/manager.c
> index 0c94e9e..cf7337e 100644
> --- a/src/core/manager.c
> +++ b/src/core/manager.c
> @@ -90,6 +90,7 @@ static void manager_undo_generators(Manager *m);
>
>  static void manager_watch_jobs_in_progress(Manager *m) {
>  usec_t next;
> +int r;
>
>  assert(m);
>
> @@ -97,12 +98,14 @@ static void manager_watch_jobs_in_progress(Manager *m) {
>  return;
>
>  next = now(CLOCK_MONOTONIC) + JOBS_IN_PROGRESS_WAIT_USEC;
> -(void) sd_event_add_time(
> +r = sd_event_add_time(
>  m->event,
>  &m->jobs_in_progress_event_source,
>  CLOCK_MONOTONIC,
>  next, 0,
>  manager_dispatch_jobs_in_progress, m);
> +if (r < 0)
> +return;
>
>  (void) 
> sd_event_source_set_description(m->jobs_in_progress_event_source, 
> "manager-jobs-in-progress");
>  }
> diff --git a/src/core/socket.c b/src/core/socket.c
> index 702742f..67beda4 100644
> --- a/src/core/socket.c
> +++ b/src/core/socket.c
> @@ -1272,11 +1272,12 @@ static int socket_watch_fds(Socket *s) {
>  else
>  r = sd_event_add_io(UNIT(s)->manager->event, 
> &p->event_source, p->fd, EPOLLIN, socket_dispatch_io, p);
>
> -(void) 
> sd_event_source_set_description(p->event_source, "socket-port-io");
>  if (r < 0) {
>  log_unit_warning_errno(UNIT(s)->id, r, "Failed to 
> watch listening fds: %m");
>  goto fail;
>  }
> +
> +(void) sd_event_source_set_description(p->event_source, 
> "socket-port-io");
>  }
>
>  return 0;
> --
> 2.3.7
>
> ___
> 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] udev leaves dangling symlinks

2015-04-23 Thread Tom Gundersen
Thanks for this, and sorry for the delay! Fix pushed.

Tom

On Thu, Apr 16, 2015 at 9:51 PM, Mantas Mikulėnas  wrote:
> So recently I noticed that udev no longer deletes /dev symlinks for removed
> devices, leaving quite a few dangling links over time (and greatly confusing
> udisks as a side effect). This was broken by:
>
> commit 3c0bab4aaf70b2383aa4cbabf6059c48744e8960
> Author: Tom Gundersen 
> Date:   Fri Mar 6 18:22:35 2015 +0100
>
> udevd: event - make db loading lazy in REMOVE event handling
>
> But udev_node_remove() needs the DB to know about symlinks, so…
>
> --
> Mantas Mikulėnas 
>
> ___
> 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] networkd-218 seems to ignore .link files

2015-04-21 Thread Tom Gundersen
On Tue, Apr 14, 2015 at 7:08 PM, Andrew Cooks  wrote:
> On Tue, Jan 13, 2015 at 6:46 AM, Jan Engelhardt  wrote:
>>
>>
>> On Monday 2015-01-12 18:29, Tom Gundersen wrote:
>> >> In systemd-218, I have configured the following testcase:
>> >>
>> >> /etc/systemd/network# ls -al
>> >> total 20
>> >> drwxr-xr-x 2 root root 4096 Jan 11 18:14 .
>> >> drwxr-xr-x 5 root root 4096 Jan 11 16:23 ..
>> >> -rw-r--r-- 1 root root   96 Jan 11 18:14 99a-ether.link
>> >
>> >Hm, isn't this just a problem of 99a-ether.link being ordered after
>> >99-default.link?
>>
>> Well, the manpage states: "All link files are collectively
>> sorted and processed in lexical order", with that, I would assume
>> that 99a, being processed after 99, would override 99.
>>
>> >It works for me when naming it 98-ether.link instead.
>>
>> Not in my case. I have a feeling networkd won't touch
>> [08:00:27:0a:c5:b2]'s interface name because it has already
>> been named by udev to enp0s3 before networkd got a chance to run.
>> Could that be it?
>
>
> I'm having a similar problem while running systemd version-219. Did you work
> out what was wrong?
>
> My link file is ignored even when I symlink
> /etc/systemd/network/99-default.link to /dev/null. I don't see anything
> interesting in 'journalctl'.
>
> # udevadm info /sys/class/net/eth0
> P: /devices/pci:00/:00:04.0/:01:00.0/net/eth0
> E: DEVPATH=/devices/pci:00/:00:04.0/:01:00.0/net/eth0
> E: ID_BUS=pci
> E: ID_MODEL_FROM_DATABASE=RTL8111/8168/8411 PCI Express Gigabit Ethernet
> Contror
> E: ID_MODEL_ID=0x8168
> E: ID_NET_DRIVER=r8169
> E: ID_NET_NAME_MAC=enx000db936008c
> E: ID_NET_NAME_PATH=enp1s0
> E: ID_OUI_FROM_DATABASE=PC Engines GmbH
> E: ID_PATH=pci-:01:00.0
> E: ID_PATH_TAG=pci-_01_00_0
> E: ID_PCI_CLASS_FROM_DATABASE=Network controller
> E: ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
> E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Co., Ltd.
> E: ID_VENDOR_ID=0x10ec
> E: IFINDEX=3
> E: INTERFACE=eth0
> E: SUBSYSTEM=net
> E: SYSTEMD_ALIAS=/sys/subsystem/net/devices/eth0
> E: TAGS=:systemd:
> E: USEC_INITIALIZED=53326
>
>
> # networkctl status --no-pager eth0
> ��● 3: eth0
>Link File: n/a
> Network File: n/a
> Type: ether
>State: off (unmanaged)
> Path: pci-:01:00.0
>   Driver: r8169
>   Vendor: Realtek Semiconductor Co., Ltd.
>Model: RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
>   HW Address: 00:0d:b9:36:00:8c (PC Engines GmbH)
>  MTU: 1500
>
>
> # cat /etc/systemd/network/01-mgmt.link
> [Match]
> Path=pci-:01:00.0
> Type=ether

Type must be the same as what is returned in DEVTYPE=, which I guess
is unset for this device. So drop this and it should work.

I see where the confusion stems from though, as we try to be helpful
and use a heuristic to print a Type in networkctl even when the kernel
does not expose a type. We probably should not do that, or allow the
same to be used in the .link matching logic (the heuristic is unlikely
to be perfect, so I hesitate a bit with the latter).

Cheers,

Tom

> [Link]
> Name=mgmt
> MACAddressPolicy=persistent
>
> ___
> 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] networkd man: fix man and config name.

2015-04-21 Thread Tom Gundersen
On Tue, Apr 21, 2015 at 1:16 PM, Susant Sahani  wrote:
>
>
> On 04/21/2015 04:33 PM, Lennart Poettering wrote:
>>
>> On Tue, 21.04.15 13:34, Susant Sahani (sus...@redhat.com) wrote:
>>
>>> Rename bond confs and man as well.
>>> ---
>>>   man/systemd.netdev.xml  |  28 
>>>   src/network/networkd-netdev-gperf.gperf | 124
>>> 
>>>   2 files changed, 76 insertions(+), 76 deletions(-)
>>>
>>> diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml
>>> index 3e65f2e..24e2d26 100644
>>> --- a/man/systemd.netdev.xml
>>> +++ b/man/systemd.netdev.xml
>>> @@ -666,7 +666,7 @@
>>> 
>>>
>>> 
>>> -LearnPacketIntvSec,=
>>> +LearnPacketIntervalSec,=
>>
>>
>> Hmm, why is there a trailing comma here?
>
>
> oops ... I guess Tom fixed it while committing . Thanks !

Yup, I noticed. So fixed and pushed.

-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout

2015-04-20 Thread Tom Gundersen
On Mon, Apr 13, 2015 at 3:04 PM, Nir Soffer  wrote:
> On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann  wrote:
>>> A program running this tool can detect a timeout (expected) or an error
>>> (unexpected), and can change the program flow based on this result.
>>>
>>> Without this, the only way to detect a timeout is to implement the timeout
>>> in the program calling udevadm.
>>
>> I cannot really see a use-case here. I mean, yeah, the commit-message
>> says it warns about timeouts but fails loudly on real errors. But
>> again, what's the use-case? Why is a timeout not a real error? Why do
>> you need to handle it differently?
>
> Timeout means that the value I chose may be too small, or the machine
> is overloaded. The administrator may need to configure the system
> differently.
>
> Other errors are not expected, and typically unexpected errors in an
> underlying tool means getting the developer of the underlying tool
> involved.
>
>> Anyway, if it's only about diagnostics this patch seems fine to me.
>
> Yes, it is mainly about diagnostics, and making it easier to debug and 
> support.

Wouldn't a better solution be to improve the udevadm logging? If we
change the exit codes this is basically ABI. Do we really want to make
such promises only for diagnostics?

Cheers,

Tom
___
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-20 Thread Tom Gundersen
On Fri, Apr 3, 2015 at 12:48 AM, Michael Marineau
 wrote:
> On Thu, Apr 2, 2015 at 3:08 PM, Nick Owens  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  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  
>> wrote:
>>> On Wed, Mar 25, 2015 at 11:49 PM,   wrote:
 From: mischief 

 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.

Cheers,

Tom
___
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-20 Thread Tom Gundersen
On Wed, Mar 25, 2015 at 9:49 PM,   wrote:
> From: mischief 
>
> 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.

Sorry for the delay in getting back to you.

I believe this is currently behaving as intended. The intention is
that all devices managed by networkd must be fully configured (not
simply get a carrier), but if there are no devices managed by networkd
then we wait for at least one device to gain carrier (in this case
something else must be bringing the interface up as networkd is up, so
it makes sure that networkd-wait-online does not get in the way of say
NetworkManager).

Does that make sense?

Cheers,

Tom

> ---
>  src/network/networkd-wait-online-manager.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/network/networkd-wait-online-manager.c 
> b/src/network/networkd-wait-online-manager.c
> index 1c997a5..1ac162a 100644
> --- a/src/network/networkd-wait-online-manager.c
> +++ b/src/network/networkd-wait-online-manager.c
> @@ -74,13 +74,13 @@ bool manager_all_configured(Manager *m) {
>  if (!l->state) {
>  log_debug("link %s has not yet been processed by 
> udev",
>l->ifname);
> -return false;
> +continue;
>  }
>
>  if (streq(l->state, "configuring")) {
>  log_debug("link %s is being processed by networkd",
>l->ifname);
> -return false;
> +continue;
>  }
>
>  if (l->operational_state &&
> --
> 2.0.5
>
> ___
> 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] networkd: Add support for bond option.

2015-04-20 Thread Tom Gundersen
Sorry for the delay. Applied. Thanks!

Tom

On Mon, Mar 9, 2015 at 10:58 AM, Susant Sahani  wrote:
> This patch adds configurational support for bond option.
>
> Test conf:
>
> bond.netdev
>
> ---
> [NetDev]
> Name=bond1
> Kind=bond
>
> [Bond]
> ArpAllTargets=all
> PrimaryReselect=better
> ArpIntervalSec=10s
> ArpIpTargets= 192.168.8.102 192.168.8.101 192.168.8.102
> ---
>
> $cat /proc/net/bonding/bond1
> Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
>
> Bonding Mode: load balancing (round-robin)
> MII Status: up
> MII Polling Interval (ms): 0
> Up Delay (ms): 0
> Down Delay (ms): 0
> ARP Polling Interval (ms): 1
> ARP IP target/s (n.n.n.n form): 192.168.8.100, 192.168.8.101, 192.168.8.102
> ---
>  man/systemd.netdev.xml  | 167 +
>  src/libsystemd/sd-rtnl/rtnl-types.c |  26 ++-
>  src/libsystemd/sd-rtnl/rtnl-types.h |  22 +++
>  src/network/networkd-netdev-bond.c  | 318 
> +++-
>  src/network/networkd-netdev-bond.h  |  85 -
>  src/network/networkd-netdev-gperf.gperf |  13 ++
>  6 files changed, 627 insertions(+), 4 deletions(-)
>
> diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml
> index ef58887..4230d19 100644
> --- a/man/systemd.netdev.xml
> +++ b/man/systemd.netdev.xml
> @@ -647,7 +647,174 @@
>  
>
>
> +  
> +LearnPacketIntvSec,=
> +
> +  Specifies the number of seconds between instances where the 
> bonding
> +  driver sends learning packets to each slaves peer switch.
> +  The valid range is 1 - 0x7fff; the default value is 1. This 
> Option
> +  has effect only in balance-tlb and balance-alb modes.
> +
> +  
> +
> +  
> +AdSelect=
> +
> +  Specifies the 802.3ad aggregation selection logic to use. 
> Possible values are
> +  stable,
> +  bandwidth,
> +  count
> +  
> +
> +  
> +
> +  
> +FailOverMac=
> +
> +  Specifies whether active-backup mode should set all slaves to
> +  the same MAC address at enslavement or, when enabled, perform 
> special handling of the
> +  bond's MAC address in accordance with the selected policy. The 
> default policy is none.
> +  Possible values are
> +  none,
> +  active,
> +  follow
> +  
> +
> +  
> +
> +  
> +ArpValidate=
> +
> +  Specifies whether or not ARP probes and replies should be
> +  validated in any mode that supports arp monitoring, or whether
> +  non-ARP traffic should be filtered (disregarded) for link
> +  monitoring purposes. Possible values are
> +  none,
> +  active,
> +  backup,
> +  all
> +  
> +
> +  
> +
> +  
> +ArpIntervalSec=
> +
> +  Specifies the ARP link monitoring frequency in milliseconds.
> +  A value of 0 disables ARP monitoring. The default value is 0.
> +  
> +
> +  
> +
> +  
> +ArpIpTargets=
> +
> +  Specifies the IP addresses to use as ARP monitoring peers 
> when
> +  ArpIntervalSec is greater than 0. These are the targets of the ARP 
> request
> +  sent to determine the health of the link to the targets.
> +  Specify these values in ipv4 dotted decimal format. At least one IP
> +  address must be given for ARP monitoring to function. The
> +  maximum number of targets that can be specified is 16. The
> +  default value is no IP addresses.
> +  
> +
> +  
> +
> +  
> +ArpAllTargets=
> +
> +  Specifies the quantity of ArpIpTargets that must be reachable
> +  in order for the ARP monitor to consider a slave as being up.
> +  This option affects only active-backup mode for slaves with
> +  ArpValidate enabled. Possible values are
> +  any,
> +  all
> +  
> +
> +  
> +
> +  
> +PrimaryReselect=
> +
> +  Specifies the reselection policy for the primary slave.  This
> +  affects how the primary slave is chosen to become the active slave
> +  when failure of the active slave or recovery of the primary slave
> +  occurs. This option is designed to prevent flip-flopping between
> +  the primary slave and other slaves.  Possible values are
> +  always,
> +  better,
> +  failure
> +  
> +
> +  
> +
> +  
> +ResendIGMP=
> +
> +  Specifies the number of IGMP membership reports to be issued 
> after
> +  a failover event. One membership report is issued immediately after
> +  the failover, subsequent packets are sent in each 200ms interval.
> +  The valid range is (0 - 255). Defaults to 1. A value

Re: [systemd-devel] [PATCH] networkd vxlan: Add support for enabling UDP checksums

2015-04-20 Thread Tom Gundersen
Sorry for the delay. Applied. Thanks!

Tom

On Thu, Mar 5, 2015 at 5:32 PM, Susant Sahani  wrote:
> Add UDPCheckSum option to enable transmitting UDP checksums when doing
> VXLAN/IPv4. Add UDP6ZeroChecksumRx, and UDP6ZeroChecksumTx
> options to enable sending zero checksums and receiving zero
> checksums in VXLAN/IPv6
>
> V2: rename Udp to UDP
> ---
>  man/systemd.netdev.xml  | 20 +++-
>  src/network/networkd-netdev-gperf.gperf |  3 +++
>  src/network/networkd-netdev-vxlan.c | 27 +++
>  src/network/networkd-netdev-vxlan.h |  3 +++
>  4 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml
> index e278aa1..7800dc4 100644
> --- a/man/systemd.netdev.xml
> +++ b/man/systemd.netdev.xml
> @@ -391,7 +391,25 @@
>  A boolean. When true 
> route short circuit is turned on.
>  
>  
> -
> +
> +
> UDPCheckSum=
> +
> +A boolean. When true 
> transmitting UDP checksums when doing VXLAN/IPv4 is turned on.
> +
> +
> +
> +
> UDP6ZeroChecksumTx=
> +
> + A boolean. When true 
> sending zero checksums in VXLAN/IPv6 is turned on.
> +
> +
> +
> +
> UDP6ZeroCheckSumRx=
> +
> + A boolean. When true 
> receiving zero checksums in VXLAN/IPv6 is turned on.
> +
> +
> +  
>  
>  
>  [Tunnel] Section Options
> diff --git a/src/network/networkd-netdev-gperf.gperf 
> b/src/network/networkd-netdev-gperf.gperf
> index 963c47c..c06344c 100644
> --- a/src/network/networkd-netdev-gperf.gperf
> +++ b/src/network/networkd-netdev-gperf.gperf
> @@ -47,6 +47,9 @@ VXLAN.ARPProxy,   config_parse_bool,
>   0,
>  VXLAN.L2MissNotification, config_parse_bool,  0, 
> offsetof(VxLan, l2miss)
>  VXLAN.L3MissNotification, config_parse_bool,  0, 
> offsetof(VxLan, l3miss)
>  VXLAN.RouteShortCircuit,  config_parse_bool,  0, 
> offsetof(VxLan, route_short_circuit)
> +VXLAN.UDPCheckSum,config_parse_bool,  0, 
> offsetof(VxLan, udpcsum)
> +VXLAN.UDP6ZeroCheckSumRx, config_parse_bool,  0, 
> offsetof(VxLan, udp6zerocsumrx)
> +VXLAN.UDP6ZeroCheckSumTx, config_parse_bool,  0, 
> offsetof(VxLan, udp6zerocsumtx)
>  VXLAN.FDBAgeingSec,   config_parse_sec,   0, 
> offsetof(VxLan, fdb_ageing)
>  Tun.OneQueue, config_parse_bool,  0, 
> offsetof(TunTap, one_queue)
>  Tun.MultiQueue,   config_parse_bool,  0, 
> offsetof(TunTap, multi_queue)
> diff --git a/src/network/networkd-netdev-vxlan.c 
> b/src/network/networkd-netdev-vxlan.c
> index d5128cb..d9b13e3 100644
> --- a/src/network/networkd-netdev-vxlan.c
> +++ b/src/network/networkd-netdev-vxlan.c
> @@ -135,6 +135,30 @@ static int netdev_vxlan_fill_message_create(NetDev 
> *netdev, Link *link, sd_rtnl_
>  }
>  }
>
> +r = sd_rtnl_message_append_u8(m, IFLA_VXLAN_UDP_CSUM, v->udpcsum);
> +if (r < 0) {
> +log_netdev_error(netdev,
> + "Could not append IFLA_VXLAN_UDP_CSUM 
> attribute: %s",
> + strerror(-r));
> +return r;
> +}
> +
> +r = sd_rtnl_message_append_u8(m, IFLA_VXLAN_UDP_ZERO_CSUM6_TX, 
> v->udp6zerocsumtx);
> +if (r < 0) {
> +log_netdev_error(netdev,
> + "Could not append 
> IFLA_VXLAN_UDP_ZERO_CSUM6_TX attribute: %s",
> + strerror(-r));
> +return r;
> +}
> +
> +r = sd_rtnl_message_append_u8(m, IFLA_VXLAN_UDP_ZERO_CSUM6_RX, 
> v->udp6zerocsumrx);
> +if (r < 0) {
> +log_netdev_error(netdev,
> + "Could not append 
> IFLA_VXLAN_UDP_ZERO_CSUM6_RX attribu

Re: [systemd-devel] udev interface naming for SR-IOV VFs

2015-04-20 Thread Tom Gundersen
On Tue, Apr 14, 2015 at 12:11 PM, Ido Barkan  wrote:
> We are implementing support for SR-IOV network cards. Afer the changing of
> the number of VFs on the card and programmatically querying for all links
> (we use libnl for this) we observe that *during the iteration* over the links
> some of them were renamed by udev and still were not. Meanning, some of them
> are called 'ethN' and some are called 'enp2sNf2' (where N is an arbitrary
> number). Also, there are times that not all of the devices are returned from
> libnl.
> After a _while_ everything stabilizes (# of interfaces and names).
>
> My questions:
> 1. Is this what you would expect from udev? Meaning, this is just async 
> behavior?
> 2. Is there a way to _know_ programmticaly that everything was probed and 
> renamed?
>Not a heuristic?

In short: no. We don't know when the kernel will finish enumerating
all the devices.

What you can know though, is whether a device you receive over netlink
has been fully processed by udev. This is probably what you need,
assuming you always keep listening for new devices in your
daemon/tool.

You can see how we do this in e.g., systemd-networkd where we have
exactly the same situation: our main source of information is rtnl,
but we need to only start using the devices once udev has renamed them
(among other things) so we listen to libudev as well and only start
using a device once both rtnl and libudev has announced it is ready.

HTH,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] SD_BUS_VTABLE_CAPABILITY

2015-04-16 Thread Tom Gundersen
On Thu, Apr 16, 2015 at 5:57 PM, Andy Lutomirski  wrote:
>> We have several uses of this, see my mail to Jiri regarding
>> CAP_SYS_BOOT for instance:
>>   https://lkml.org/lkml/2015/4/16/219
>
> I read that, but I disagree with you.
>
> CAP_SYS_BOOT is the privilege to directly hard-reboot the system, not
> the privilege to initiate a clean reboot.

My main point that being allowed to do a hard-reboot, but not to do a
clean reboot, makes no sense.

>> However, what we are trying to get to the bottom of is if you see any
>> technical problems with the current kdbus capability handling code. My
>> understanding is that you don't.
>>
>
> I have a technical problem with it: it's a design that has
> insufficient justification.  It also seems like it will be quite
> limiting in the future, *especially* wrt user namespaces.

What I was really asking was if you see any actual vulnerabilities.
That we have a different technical opinion on the design of this is a
different matter.

> I agree that it's probably not exploitable *if used carefully* in the
> latest kdbus code.

It would be very helpful if you could go into details on why you think
more care is needed here than for other things. Is there anything
non-trivial going on here that I'm missing? The way capabilites are
exposed through kdbus seems perfectly straight-forward to me, so I'm
really trying to get my head around your concerns here.

So, let me ask explicitly again:

* Are there any actual exploitable vulnerabilities you are aware
of in the kdbus kernel code?

* Are there any actual exploitable vulnerabilities on the sd-bus
userspace code you are aware of, regardless if in the kdbus or dbus1
support in it?

If you are, please provide us with good explanations, so that we can
do something about them. We'd love to fix this!

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] SD_BUS_VTABLE_CAPABILITY

2015-04-16 Thread Tom Gundersen
On Thu, Apr 16, 2015 at 4:52 PM, Andy Lutomirski  wrote:
> Unshare your user namespace, set things up right, and systemd
> or any other server will see you as having all capabilities.  You've
> fixed that in kdbus, but you haven't (and probably can't!) fix it in
> the legacy code, and that legacy code is still there (!).

The dbus1 code (which I assume you mean when you say "legacy code")
does not make use of capabilities, and it should not (see Lennart's
answer for all the details). If anything, this should be an argument
to move to kdbus with native, race-free capability-passing support.

Do I understand correctly, that any concerns you had are about systemd
and its dbus1 compat code (which of course we should take seriously
too), and that you no longer see any security vulnerabilities in the
capability related code of kdbus?

> The ratio of complexity of capability code the kdbus folks have
> already written (hundreds of lines across multiple files) to its
> utility (very near zero AFAICT) is, in my book, not a good sign at
> all.

We have several uses of this, see my mail to Jiri regarding
CAP_SYS_BOOT for instance:
  https://lkml.org/lkml/2015/4/16/219

However, what we are trying to get to the bottom of is if you see any
technical problems with the current kdbus capability handling code. My
understanding is that you don't.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] SD_BUS_VTABLE_CAPABILITY

2015-04-16 Thread Tom Gundersen
Hi Andy,

On Thu, Apr 16, 2015 at 2:55 AM, Andy Lutomirski  wrote:
> Yesterday, I discovered SD_BUS_VTABLE_CAPABILITY.  Are there any
> examples in which it does anything?

Please note that you need to be using kdbus to get any capabilities
transported, so in dbus1 this does nothing (as on dbus1 using
capabilities would be racy). Are you testing this on kdbus?

> If so, I don't suppose any of you
> could give me an example of:
>
> $ cp `which dbus-send` .
> $ sudo setcap all=eip dbus-send
> $ dbus-send [not sure what goes here]
>
> that passes an authentication test that would have failed without the setcap?

Note that dbus-send is the old dbus1 command, and will always go via
the bus proxy that connects dbus1 clients to kdbus. As such, it will
only carry the credentials that are available safely on dbus1, and
hence not capabilities. So even if you do use kdbus on your machine,
this example would not work.

To be clear, the reason we do not use the capability logic on dbus1,
but only on kdbus is as follows: On kdbus, the capabilities are
attached to the method call itself, hence we safely know that at the
time the method call was issued by the client side, that client
actually really had those capabilities. On dbus1 however, which uses
AF_UNIX/SOCK_STREAM, this is not possible: we only get the
UID/GID/PID, hence we could only derive the caps from
/proc/$PID/status. But this opens this up for a vulnerability: if a
client quickly issues a method call, and then exec()s a suid binary,
it could happen that the service side would read the capabilities from
that SUID process, and not the original process, and the exploit was
successful. We cannot allow that, hence no capabilities on dbus1,
instead we open up things to a much, much broader uid == 0. kdbus
allows us to be much stricter there, by allowing us to check only one
specific capability.

An equivalent example to the one you gave using native kdbus would be:

$ cp `which busctl` .
$ sudo setcap all=eip busctl
$ ./busctl call org.freedesktop.timedate1 /org/freedesktop/timedate1
org.freedesktop.timedate1 SetTimezone "sb" Europe/London 0

> In the interest of full disclosure, I'm asking because I think that
> one of two things is true:
>
> 1. The SD_BUS_VTABLE_CAPABILITY code is useless and should therefore be 
> deleted.

This is not the case.

> 2. The SD_BUS_VTABLE_CAPABILITY code is exploitably buggy and should
> therefore be deleted.

I have heard this claim from you many times, but so far I haven't been
able to figure out what you have in mind. Could you either give an
example of an exploit or at least the general scheme you have in mind?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd

2015-04-14 Thread Tom Gundersen
On Tue, Apr 14, 2015 at 4:21 PM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Tue, Apr 14, 2015 at 07:19:42AM -0700, Tom Gundersen wrote:
>>  src/libsystemd/sd-device/sd-device.c |9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> New commits:
>> commit 85091685af65831f379580c75b40776c20e245ee
>> Author: Tom Gundersen 
>> Date:   Tue Apr 14 16:05:53 2015 +0200
>>
>> sd-device: fix reading of subsystem
>>
>> diff --git a/src/libsystemd/sd-device/sd-device.c 
>> b/src/libsystemd/sd-device/sd-device.c
>> index 7d52e3c..d420bd0 100644
>> --- a/src/libsystemd/sd-device/sd-device.c
>> +++ b/src/libsystemd/sd-device/sd-device.c
>> @@ -772,10 +772,10 @@ _public_ int sd_device_get_subsystem(sd_device 
>> *device, const char **ret) {
>>  r = device_set_subsystem(device, "drivers");
>>  else if (path_startswith(device->devpath, "/subsystem/") ||
>>   path_startswith(device->devpath, "/class/") ||
>> - path_startswith(device->devpath, "/buss/"))
>> + path_startswith(device->devpath, "/bus/"))
>>  r = device_set_subsystem(device, "subsystem");
>>  if (r < 0)
>> -return r;
>> +return log_debug_errno(r, "sd-devcie: could not set 
>> subsystem for %s: %m", device->devpath);
>  ^^
>
> Zbyszek

Thanks! Fixed.

-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC 1/6] proxy-discoveryd: Basic core added

2015-04-13 Thread Tom Gundersen
On Mon, Apr 13, 2015 at 12:05 PM, Tomasz Bursztyka
 wrote:
>>> +int manager_new(Manager **ret);
>>> +Manager *manager_free(Manager *m);
>>> +
>>> +DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
>>> +#define _cleanup_manager_free_ _cleanup_(manager_freep)
>>
>> We generally try to avoid this define in internal code, and just use
>> _cleanup_(manager_freep) inline.
>
>
> Ok, I followed networkd but indeed other parts don't do that (machined for
> instance).

Do as I say, not as I do :P I didn't get the memo yet when I wrote the
initial networkd parts, I should fix this up to avoid confusion in the
future.

>>> diff --git a/units/.gitignore b/units/.gitignore
>>> index ad469c1..63ae1af 100644
>>> --- a/units/.gitignore
>>> +++ b/units/.gitignore
>>> @@ -51,6 +51,7 @@
>>>   /systemd-networkd.service
>>>   /systemd-nspawn@.service
>>>   /systemd-poweroff.service
>>> +/systemd-proxy-discoveryd.service
>>>   /systemd-quotacheck.service
>>>   /systemd-random-seed.service
>>>   /systemd-reboot.service
>>> diff --git a/units/systemd-proxy-discoveryd.service.in
>>> b/units/systemd-proxy-discoveryd.service.in
>>> new file mode 100644
>>> index 000..7ac02e0
>>> --- /dev/null
>>> +++ b/units/systemd-proxy-discoveryd.service.in
>>> @@ -0,0 +1,18 @@
>>> +# This file is part of systemd.
>>> +#
>>> +# 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.
>>> +
>>> +[Unit]
>>> +Description=Proxy service
>>> +DefaultDependencies=no
>>> +Requires=dbus.socket
>>> +After=dbus.socket
>>> +Before=remote-fs.target
>>> +
>>> +[Service]
>>> +Restart=on-failure
>>> +ExecStart=@rootlibexecdir@/systemd-proxy-discoveryd
>>> +StandardOutput=null
>>
>> Why this special handling of stdout? Is it the JS engine? Probably
>> should have a comment as it is a bit odd.
>
>
> I copy-pasted the .service file we had for pacrunner, this needs to be
> tailored for proxy-discoveryd.
>
> The stdout directing to /dev/null is to avoid the JS engine to print
> anything.
> Though for duktape specifically, we can hook the debug/printing/warning/...
> functions, so we could
> redirect messages to the logs.

That's what I thought. Would be nice to hook this up properly
internally I think. Either to just redirect to /dev/null there, or to
log correctly.

-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd

2015-04-05 Thread Tom Gundersen
On Sun, Apr 5, 2015 at 11:50 AM, Lennart Poettering
 wrote:
> On Fri, 03.04.15 13:23, Tom Gundersen (tome...@kemper.freedesktop.org) wrote:
>
>>
>> -tags = strdupa(value);
>> +FOREACH_WORD_SEPARATOR(word, l, value, ":", state) {
>> +char *tag;
>>
>> -while ((next = strchr(tags, ':'))) {
>> -next[0] = '\0';
>> +tag = strndupa(word, l);
>>
>
> Repeat after me: I shall not use alloca() within loops. I shall not
> use alloca() within loops. I shall not use alloca() within loops.
>
> This cannot work.

Fixed. Thanks!

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] networkd: Is auto-negotiation turned off when specifying parameters in a link file?

2015-04-03 Thread Tom Gundersen
On Fri, Apr 3, 2015 at 4:47 PM, Lennart Poettering
 wrote:
> On Thu, 02.04.15 13:11, Paul Menzel (paulepan...@users.sourceforge.net) wrote:
>
>> Dear systemd folks,
>>
>>
>> some network cards with certain cables and devices take up to five
>> seconds so that the link is up [1].
>>
>> $ sudo journalctl -u systemd-networkd
>> -- Logs begin at Fr 2015-03-20 17:39:31 CET, end at So 2015-03-22 
>> 08:39:39 CET. --
>> Mär 20 17:39:31 myhostname systemd-networkd[245]: lo  : 
>> gained carrier
>> Mär 20 17:39:31 myhostname systemd-networkd[245]: eth0: 
>> link configured
>> Mär 20 17:39:34 myhostname systemd-networkd[245]: eth0: 
>> gained carrier
>>
>> This is annoying if the system is up, but you cannot log into a server
>> because the NIC is so slow configuring the link.
>>
>> The Linux kernel module developers from the e1000-devel mailing list
>> suggested the following [2]:
>>
>> > Both sides either need to be forced or both sides need to be auto-neg.
>> > Otherwise the auto-negotiation process will usually detect link (the
>> > physical signal) but fail to find duplex (since one side is not
>> > talking) and default to the lowest common denominator, e.g. half
>> > duplex.  So, you could try forcing speed, it may look right on your
>> > end but if you have no visibility to the other end it could be running
>> > at half duplex.
>> >
>> > You may be able to speed up the auto negotiation process by
>> > exclusively advertising 1000 Mbps Full Duplex.
>> >
>> > # ethtool -s ethX advertise 0x20
>>
>> In the IRC channel #syst...@irc.freenode.net somebody told me to look
>> into systemd’s network link configuration (`man systemd.link`).
>>
>> Reading the manual page, it’s not clear to me, if auto-negotiation is
>> going to be disabled, if the following is set.
>>
>> BitsPerSecond=1G
>> Duplex=Full
>>
>> Is that equivalent to `ethtool -s ethX advertise 0x20`? If not, how
>> could I set that up?
>
> BitsPerSecond= and Duplex= are equivalent to "ethtool speed" and
> "ethtool duplex".
>
> We currently have no setting in .link files that was equivalent to
> "ethtool advertise". Maybe we could add that, Tom?
>
> Paul, are you sure that this will really cut 5s from the configuration
> time? Did you try this?

No objections from me if it really cuts 5s as described.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd from git doesn't boot properly in fedora rawhide

2015-04-03 Thread Tom Gundersen
On Fri, Apr 3, 2015 at 11:14 AM, Jan Synáček  wrote:
> From the following commit onward, systemd doesn't boot properly in
> Rawhide. Some device units time out and I'm then dropped into an
> emergency shell.
>
> commit f4ac4d1a82e2c468761fffa333323841ad886221
> Author: Tom Gundersen 
> Date:   Wed Apr 1 13:55:20 2015 +0200
>
> libudev: device - replace by a thin wrapper around sd-device
>
> All I can see is that a job is running for some of the device units and
> then it times out. Log from journalctl and one of the failed device
> units can be found at [1] and [2]. Any idea what might be wrong or how
> to debug this any further?
>
> [1] https://jsynacek.fedorapeople.org/systemd/journalctl.log
> [2] https://jsynacek.fedorapeople.org/systemd/device-unit.log

Thanks for the report. I'll try to reproduce.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC][PATCH 1/2] libsystemd: add sd-device library

2015-04-01 Thread Tom Gundersen
After discussing this with Kay, I went ahead and pushed this. Feedback
still welcome of course.

Cheers,

Tom

On Wed, Apr 1, 2015 at 3:00 PM, Tom Gundersen  wrote:
> This provides equivalent functionality to libudev-device, but in the
> systemd style. The public API only caters to creating sd_device objects
> from for devices that already exist in /sys, there is no support for
> listening for monitoring uevents or creating devices received over
> the udev netlink protocol.
>
> The private API contains the necessary functionality to make sd-device
> a drop-in replacement for libudev-device, but which we will not export
> publicly.
> ---
>
> Hi guys,
>
> This is another step on the way of ripping out the bits of libudev that will
> still be useful once the udev netlink transport has been replaced by kdbus.
>
> Feedback welcome.
>
> Cheers,
>
> Tom
>
>  Makefile.am|9 +-
>  src/libsystemd/sd-device/device-internal.h |  125 ++
>  src/libsystemd/sd-device/device-private.c  | 1101 +
>  src/libsystemd/sd-device/device-private.h  |   63 +
>  src/libsystemd/sd-device/device-util.h |   48 +
>  src/libsystemd/sd-device/sd-device.c   | 1812 
> 
>  src/systemd/sd-device.h|   77 ++
>  7 files changed, 3234 insertions(+), 1 deletion(-)
>  create mode 100644 src/libsystemd/sd-device/device-internal.h
>  create mode 100644 src/libsystemd/sd-device/device-private.c
>  create mode 100644 src/libsystemd/sd-device/device-private.h
>  create mode 100644 src/libsystemd/sd-device/device-util.h
>  create mode 100644 src/libsystemd/sd-device/sd-device.c
>  create mode 100644 src/systemd/sd-device.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 93fdbc2..9509247 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -231,6 +231,7 @@ AM_CPPFLAGS = \
> -I $(top_srcdir)/src/libsystemd/sd-rtnl \
> -I $(top_srcdir)/src/libsystemd/sd-network \
> -I $(top_srcdir)/src/libsystemd/sd-hwdb \
> +   -I $(top_srcdir)/src/libsystemd/sd-device \
> -I $(top_srcdir)/src/libsystemd-network \
> -I $(top_srcdir)/src/libsystemd-terminal \
> $(OUR_CPPFLAGS)
> @@ -2918,6 +2919,7 @@ libsystemd_internal_la_SOURCES = \
> src/systemd/sd-path.h \
> src/systemd/sd-network.h \
> src/systemd/sd-hwdb.h \
> +   src/systemd/sd-device.h \
> src/libsystemd/sd-bus/sd-bus.c \
> src/libsystemd/sd-bus/bus-control.c \
> src/libsystemd/sd-bus/bus-control.h \
> @@ -2981,7 +2983,12 @@ libsystemd_internal_la_SOURCES = \
> src/libsystemd/sd-network/network-util.c \
> src/libsystemd/sd-hwdb/sd-hwdb.c \
> src/libsystemd/sd-hwdb/hwdb-util.h \
> -   src/libsystemd/sd-hwdb/hwdb-internal.h
> +   src/libsystemd/sd-hwdb/hwdb-intenal.h \
> +   src/libsystemd/sd-device/device-internal.h \
> +   src/libsystemd/sd-device/device-util.h \
> +   src/libsystemd/sd-device/sd-device.c \
> +   src/libsystemd/sd-device/device-private.c \
> +   src/libsystemd/sd-device/device-private.h
>
>  nodist_libsystemd_internal_la_SOURCES = \
> src/libsystemd/libsystemd.sym
> diff --git a/src/libsystemd/sd-device/device-internal.h 
> b/src/libsystemd/sd-device/device-internal.h
> new file mode 100644
> index 000..59ec1a6
> --- /dev/null
> +++ b/src/libsystemd/sd-device/device-internal.h
> @@ -0,0 +1,125 @@
> +/***
> +  This file is part of systemd.
> +
> +  Copyright 2008-2012 Kay Sievers 
> +  Copyright 2014 Tom Gundersen 
> +
> +  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/>.
> +***/
> +
> +#pragma once
> +
> +#include "hashmap.h"
> +#include "set.h"
> +
> +struct sd_device {
> +uint64_t n_ref;
> +
> +sd_device *parent;
> +bool parent_set; /* no need to try to reload parent */
> +
> +OrderedHashmap *properties;
> +Iterator properties_iterator;
> +uint64_t properties_generation; /* changes whenever the prope

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

2015-04-01 Thread Tom Gundersen
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  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:
>   *   b -- BCMA bus core number
>   *   ccw -- CCW bus group name
> - *   o  -- on-board device index number
> + *   o[f][d]-- on-board device index number
>   *   s[f][d] -- hotplug slot index number
>   *   x-- MAC address
>   *   [P]ps[f][d]
> @@ -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


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

2015-04-01 Thread Tom Gundersen
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:
  *   b -- BCMA bus core number
  *   ccw -- CCW bus group name
- *   o  -- on-board device index number
+ *   o[f][d]-- on-board device index number
  *   s[f][d] -- hotplug slot index number
  *   x-- MAC address
  *   [P]ps[f][d]
@@ -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


[systemd-devel] [RFC][PATCH 2/2] libudev: device - replace by a thin wrapper around sd-device

2015-04-01 Thread Tom Gundersen
 = \
src/network/networkctl.c
 
 networkctl_LDADD = \
-   libsystemd-internal.la \
libudev-internal.la \
+   libsystemd-internal.la \
libsystemd-shared.la \
libsystemd-network.la
 
@@ -5905,8 +5918,8 @@ libsystemd_logind_core_la_SOURCES = \
 
 libsystemd_logind_core_la_LIBADD = \
libsystemd-label.la \
-   libsystemd-internal.la \
libudev-internal.la \
+   libsystemd-internal.la \
libsystemd-shared.la
 
 if HAVE_ACL
@@ -5929,10 +5942,10 @@ loginctl_SOURCES = \
src/login/sysfs-show.c
 
 loginctl_LDADD = \
+   libudev-internal.la \
libsystemd-internal.la \
libsystemd-logs.la \
libsystemd-journal-internal.la \
-   libudev-internal.la \
libsystemd-shared.la
 
 rootbin_PROGRAMS += \
diff --git a/src/libudev/libudev-device-internal.h 
b/src/libudev/libudev-device-internal.h
new file mode 100644
index 000..18ae7a9
--- /dev/null
+++ b/src/libudev/libudev-device-internal.h
@@ -0,0 +1,62 @@
+/***
+  This file is part of systemd.
+
+  Copyright 2008-2012 Kay Sievers 
+  Copyright 2015 Tom Gundersen 
+
+  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/>.
+***/
+
+#pragma once
+
+#include "libudev.h"
+#include "sd-device.h"
+
+/**
+ * udev_device:
+ *
+ * Opaque object representing one kernel sys device.
+ */
+struct udev_device {
+struct udev *udev;
+
+/* real device object */
+sd_device *device;
+
+/* legacy */
+int refcount;
+
+struct udev_device *parent;
+bool parent_set;
+
+struct udev_list properties;
+uint64_t properties_generation;
+struct udev_list tags;
+uint64_t tags_generation;
+struct udev_list devlinks;
+uint64_t devlinks_generation;
+struct udev_list sysattrs;
+bool sysattrs_read;
+};
+
+struct udev_device *udev_device_new(struct udev *udev);
+
+#define assert_return_errno(expr, r, err)   \
+do {\
+if (_unlikely_(!(expr))) {  \
+log_assert_failed_return(#expr, __FILE__, __LINE__, 
__PRETTY_FUNCTION__); \
+errno = err;\
+return (r); \
+}   \
+} while (false)
diff --git a/src/libudev/libudev-device-private.c 
b/src/libudev/libudev-device-private.c
index e590288..bb4d7e6 100644
--- a/src/libudev/libudev-device-private.c
+++ b/src/libudev/libudev-device-private.c
@@ -2,6 +2,7 @@
   This file is part of systemd.
 
   Copyright 2008-2012 Kay Sievers 
+  Copyright 2015 Tom Gundersen 
 
   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
@@ -17,172 +18,392 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
 #include "libudev.h"
 #include "libudev-private.h"
+#include "libudev-device-internal.h"
+
+#include "device-private.h"
+
+int udev_device_tag_index(struct udev_device *udev_device, struct udev_device 
*udev_device_old, bool add) {
+sd_device *device_old = NULL;
+int r;
+
+assert(udev_device);
+
+if (udev_device_old)
+device_old = udev_device_old->device;
+
+r = device_tag_index(udev_device->device, device_old, add);
+if (r < 0)
+return r;
+
+return 0;
+}
+
+int udev_device_update_db(struct udev_device *udev_device) {
+int r;
+
+assert(udev_device);
+
+r = device_update_db(udev_device->device);
+if (r < 0)
+return r;
+
+return 0;
+}
+
+int udev_device_delete_db(struct udev_device *udev_device) {
+int r;
+
+assert(udev_device);
+
+r = device_delete_db(udev_device->device);
+if (r < 0)
+return r;
+
+return 0;
+}
+
+int udev_device_get_ifindex(struct udev_device *udev_device) {
+int r, ifindex;
+
+assert(udev_device);
+
+r = sd_dev

[systemd-devel] [RFC][PATCH 1/2] libsystemd: add sd-device library

2015-04-01 Thread Tom Gundersen
This provides equivalent functionality to libudev-device, but in the
systemd style. The public API only caters to creating sd_device objects
from for devices that already exist in /sys, there is no support for
listening for monitoring uevents or creating devices received over
the udev netlink protocol.

The private API contains the necessary functionality to make sd-device
a drop-in replacement for libudev-device, but which we will not export
publicly.
---

Hi guys,

This is another step on the way of ripping out the bits of libudev that will
still be useful once the udev netlink transport has been replaced by kdbus.

Feedback welcome.

Cheers,

Tom

 Makefile.am|9 +-
 src/libsystemd/sd-device/device-internal.h |  125 ++
 src/libsystemd/sd-device/device-private.c  | 1101 +
 src/libsystemd/sd-device/device-private.h  |   63 +
 src/libsystemd/sd-device/device-util.h |   48 +
 src/libsystemd/sd-device/sd-device.c   | 1812 
 src/systemd/sd-device.h|   77 ++
 7 files changed, 3234 insertions(+), 1 deletion(-)
 create mode 100644 src/libsystemd/sd-device/device-internal.h
 create mode 100644 src/libsystemd/sd-device/device-private.c
 create mode 100644 src/libsystemd/sd-device/device-private.h
 create mode 100644 src/libsystemd/sd-device/device-util.h
 create mode 100644 src/libsystemd/sd-device/sd-device.c
 create mode 100644 src/systemd/sd-device.h

diff --git a/Makefile.am b/Makefile.am
index 93fdbc2..9509247 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -231,6 +231,7 @@ AM_CPPFLAGS = \
-I $(top_srcdir)/src/libsystemd/sd-rtnl \
-I $(top_srcdir)/src/libsystemd/sd-network \
-I $(top_srcdir)/src/libsystemd/sd-hwdb \
+   -I $(top_srcdir)/src/libsystemd/sd-device \
-I $(top_srcdir)/src/libsystemd-network \
-I $(top_srcdir)/src/libsystemd-terminal \
$(OUR_CPPFLAGS)
@@ -2918,6 +2919,7 @@ libsystemd_internal_la_SOURCES = \
src/systemd/sd-path.h \
src/systemd/sd-network.h \
src/systemd/sd-hwdb.h \
+   src/systemd/sd-device.h \
src/libsystemd/sd-bus/sd-bus.c \
src/libsystemd/sd-bus/bus-control.c \
src/libsystemd/sd-bus/bus-control.h \
@@ -2981,7 +2983,12 @@ libsystemd_internal_la_SOURCES = \
src/libsystemd/sd-network/network-util.c \
src/libsystemd/sd-hwdb/sd-hwdb.c \
src/libsystemd/sd-hwdb/hwdb-util.h \
-   src/libsystemd/sd-hwdb/hwdb-internal.h
+   src/libsystemd/sd-hwdb/hwdb-intenal.h \
+   src/libsystemd/sd-device/device-internal.h \
+   src/libsystemd/sd-device/device-util.h \
+   src/libsystemd/sd-device/sd-device.c \
+   src/libsystemd/sd-device/device-private.c \
+   src/libsystemd/sd-device/device-private.h
 
 nodist_libsystemd_internal_la_SOURCES = \
src/libsystemd/libsystemd.sym
diff --git a/src/libsystemd/sd-device/device-internal.h 
b/src/libsystemd/sd-device/device-internal.h
new file mode 100644
index 000..59ec1a6
--- /dev/null
+++ b/src/libsystemd/sd-device/device-internal.h
@@ -0,0 +1,125 @@
+/***
+  This file is part of systemd.
+
+  Copyright 2008-2012 Kay Sievers 
+  Copyright 2014 Tom Gundersen 
+
+  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/>.
+***/
+
+#pragma once
+
+#include "hashmap.h"
+#include "set.h"
+
+struct sd_device {
+uint64_t n_ref;
+
+sd_device *parent;
+bool parent_set; /* no need to try to reload parent */
+
+OrderedHashmap *properties;
+Iterator properties_iterator;
+uint64_t properties_generation; /* changes whenever the properties are 
changed */
+uint64_t properties_iterator_generation; /* generation when iteration 
was started */
+
+/* the subset of the properties that should be written to the db*/
+OrderedHashmap *properties_db;
+
+Hashmap *sysattr_values; /* cached sysattr values */
+
+Set *sysattrs; /* names of sysattrs */
+Iterator sysattrs_iterator;
+bool sysattrs_read; /* don't try to re-read sysattrs once read */
+
+Set *tags;
+Iterator tags_iterator;
+uint64_t tags_generation; /* changes whenever the tags are changed */
+uint64_t tags_iterator_generation; /* generation when iteration was 
started */
+bool property_tags_outdated; /

Re: [systemd-devel] [PATCH] nspawn: fallback on bind mount when mknod fails

2015-03-29 Thread Tom Gundersen
On Mar 29, 2015 5:18 PM, "Alban Crequy"  wrote:
>
> From: Alban Crequy 
>
> Some systems abusively restrict mknod, even when the device node already
> exists in /dev. This is unfortunate because it prevents systemd-nspawn
> from creating the basic devices in /dev in the container.
>
> This patch implements a workaround: when mknod fails, fallback on bind
> mounts.

Could we just always use bind mounts and avoid the two code paths?

Tom

> Additionally, /dev/console was created with a mknod with the same
> major/minor as /dev/null before bind mounting a pts on it. This patch
> removes the mknod and creates an empty regular file instead.
>
> In order to test this patch, I used the following configuration, which I
> think should replicate the system with the abusive restriction on mknod:
>
>   # grep devices /proc/self/cgroup
>   4:devices:/user.slice/restrict
>   # cat /sys/fs/cgroup/devices/user.slice/restrict/devices.list
>   c 1:9 r
>   c 5:2 rw
>   c 136:* rw
>   # systemd-nspawn --register=false -D .
> ---
>  src/nspawn/nspawn.c | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index 300b6df..09fff38 100644
> --- a/src/nspawn/nspawn.c
> +++ b/src/nspawn/nspawn.c
> @@ -1449,8 +1449,17 @@ static int copy_devnodes(const char *dest) {
>  return -r;
>  }
>
> -if (mknod(to, st.st_mode, st.st_rdev) < 0)
> -return log_error_errno(errno, "mknod(%s)
failed: %m", to);
> +if (mknod(to, st.st_mode, st.st_rdev) < 0) {
> +if (errno != EPERM)
> +return log_error_errno(errno,
"mknod(%s) failed: %m", to);
> +
> +/* Some systems abusively restrict mknod
but
> + * allow bind mounts. */
> +if (touch(to) < 0)
> +return log_error_errno(errno,
"touch (%s) failed: %m", to);
> +if (mount(from, to, "bind", MS_BIND,
NULL) < 0)
> +return log_error_errno(errno,
"both mknod and bind mount (%s) failed: %m", to);
> +}
>
>  if (arg_userns && arg_uid_shift != UID_INVALID)
>  if (lchown(to, arg_uid_shift,
arg_uid_shift) < 0)
> @@ -1481,7 +1490,6 @@ static int setup_ptmx(const char *dest) {
>  static int setup_dev_console(const char *dest, const char *console) {
>  _cleanup_umask_ mode_t u;
>  const char *to;
> -struct stat st;
>  int r;
>
>  assert(dest);
> @@ -1489,24 +1497,17 @@ static int setup_dev_console(const char *dest,
const char *console) {
>
>  u = umask();
>
> -if (stat("/dev/null", &st) < 0)
> -return log_error_errno(errno, "Failed to stat /dev/null:
%m");
> -
>  r = chmod_and_chown(console, 0600, 0, 0);
>  if (r < 0)
>  return log_error_errno(r, "Failed to correct access mode
for TTY: %m");
>
>  /* We need to bind mount the right tty to /dev/console since
>   * ptys can only exist on pts file systems. To have something
> - * to bind mount things on we create a device node first, and
> - * use /dev/null for that since we the cgroups device policy
> - * allows us to create that freely, while we cannot create
> - * /dev/console. (Note that the major minor doesn't actually
> - * matter here, since we mount it over anyway). */
> + * to bind mount things on we create a empty regular file. */
>
>  to = strjoina(dest, "/dev/console");
> -if (mknod(to, (st.st_mode & ~0) | 0600, st.st_rdev) < 0)
> -return log_error_errno(errno, "mknod() for /dev/console
failed: %m");
> +if (touch(to) < 0)
> +return log_error_errno(errno, "touch() for /dev/console
failed: %m");
>
>  if (mount(console, to, "bind", MS_BIND, NULL) < 0)
>  return log_error_errno(errno, "Bind mount for
/dev/console failed: %m");
> --
> 2.1.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


Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

2015-03-27 Thread Tom Gundersen
On Fri, Mar 27, 2015 at 2:04 PM, Djalal Harouni  wrote:
> Hi Shawn,
>
> On Thu, Mar 26, 2015 at 11:21:54PM -0700, Shawn Landden wrote:
>> On Thu, Mar 26, 2015 at 5:47 PM, Djalal Harouni  wrote:
>> > On Fri, Mar 27, 2015 at 12:30:53AM +0100, Tom Gundersen wrote:
>> >> On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
>> >>  wrote:
>> >> > On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>> >> >
>> >> >> Will result in slightly smaller binaries, and cuts out the branch, 
>> >> >> even if
>> >> >> the expression is still executed.
>> >> >
>> >> > I am sorry, but the whole point of assert_se() is that it has side
>> >> > effects. That's why it is called that way (the "se" suffix stands for
>> >> > "side effects"), and is different from assert(). You are supposed to
>> >> > use it whenever the code enclosed in it shall not be suppressed if
>> >> > NDEBUG is defined. This patch of yours breaks that whole logic!
>> >>
>> >> Hm, am I reading the patch wrong? It looks good to me. It is simply
>> >> dropping the branching and logging in the NDEBUG case, but the
>> >> expression itself is still evaluated.
>> > Yep but it seems that abort() will never be called,
>> > log_assert_failed() => abort()
>> >
>> > And the logging mechanism is also one of those side effects. IMO unless
>> > there are real valid reasons for any change to these macors... changing
>> > anything here will just bring more bugs that we may never notice.
>> >
>> Those are the side effects of assert(), the side effects of the
>> expression are still evaluated.
> Yes but there are also the following points:
> * assert() is simple, assert_se() is more complex and it may modify other
> global sate. I don't think that we can break down side effects of
> assert_se() into:
>  1) side effects of assert()
>  2) side effects of other expressions.
>
> And then remove that part 1)
>
>
> So given the fact that asser_se() do not depend on NDEBUG, did you
> consider the following:
>
> * You don't have control over what the expression do, it may just call
>   abort() or exit() in case of fatal errors, so even if you remove the
>   explicit abort() call, expressions may just call it.

If expr() aborts or exits, then it really does not matter whether we
abort() explicitly or not, so I don't see what this has to do with
this patch. Sure, even with NDEBUG the program may abort if that is
called explicitly...

> * Some expressions were perhaps depending on the logging mechanism which
>   is part of the side effect. Callers to assert_se() are perhaps using
>   it for a reason, are you sure that you are not introducing a
>   regression here ?! it will be difficult to debug the error if we don't
>   have logs.

As far as I can tell all the relevant logging functions are pure. They
should not interact with global state, but maybe I'm missing
something?

> * Current expression may modify/interact with a global state which may
>   cause a fatal error, and if the caller wants to know if that failed,
>   then abort(), your patch just introduced a regression, without the
>   explicit abort(), all callers now have to call abort().

Yeah, this is the point I think. I still think the patch makes sense
though, it really don't see why there should be a distinction between
assert_se() and assert() when it comes to logging and aborting.

If assert_se() fails it indicates we may have messed up the global
state, and if assert() fails it indicates that the global state may be
messed up (by someone else). Either way the global state is
potentially messed up and not logging/aborting seems as (un)safe in
both situations.

Another way of seeing it, intuitively I don't see why we should
distinguish between

assert_se(foo() >= 0);

and

r = foo();
assert(r >= 0);

> * Did you grep for assert_se() uses in the source ?
>
> I really do not think this is an improvement, we can't predict the
> semantics of expression here, perhaps we can with simple assert() but
> not with assert_se().
>
> --
> Djalal Harouni
> http://opendz.org
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Providing DNS for DHCPServer configuration

2015-03-27 Thread Tom Gundersen
Hi Ash,

We do want some functionality like this. However, I don't think we
should simply default to assuming the DHCP server is also a DNS
server. The way it should work is to pass to the client all the (non
restricted, i.e., where Domains= is not set, or includes the wildcard)
DNS servers configured for the host.

That has the restriction that any DNS server you want to pass on to
the client must also be a valid DNS server on the server. Which I
think is fine.

Makes sense?

Cheers,

Tom

On Wed, Mar 25, 2015 at 1:44 AM, Ash Charles  wrote:
> Hi,
>
> I'm using the DHCPServer option within systemd-networkd along with
> hostapd to provide a basic wireless access point on an embedded
> system.  When my client systems connect, they make a DHCP request that
> includes a parameter request for a Domain Name Server (option
> 6)---standard fare I think.  The DHCPAck/DHCPOffer from
> systemd-network doesn't provide a response to this part of the request
> so the client needs to manually configure a DNS server.  I created a
> rudimentary patch (attached) that uses the access point IP as the DNS
> server---seems to work okay for my particular use case though I
> recognize this is *far* from generic.
>
> Is there 1. a different/better way of doing this and, 2. if not, is
> such a feature desirable for systemd-networkd/what might a good
> implementation look like?
>
> Thanks for any inights!
> --Ash
>
> ___
> 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


  1   2   3   4   5   6   7   8   9   10   >