Re: [systemd-devel] Thawing status in a service

[Michal Sekletar]

On Wed, Oct 14, 2020 at 11:21 AM Srijan Sivakumar 
wrote:

> Hello there SMEs,
>
> I'm contributing to glusterfs project and found the service to be in the
> state of
> *Active: active (running) (thawing) *sometimes.
>
> Now, I tried looking up what is the thawing state but couldn't get
> anything concrete on it. Wondering if I could get some help from the actual
> devs working on systemd.
>

Hi Srijan,

Thawing state is related to cgroup freezer implementation.
https://github.com/systemd/systemd/pull/13512

Issue you are referring to is tracked here:
https://bugzilla.redhat.com/show_bug.cgi?id=1868831

Michal



> Thank you in advance :)
> Srijan
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd.net-naming-scheme change after update

[Michal Sekletar]

On Wed, Aug 5, 2020 at 4:12 PM Thomas HUMMEL 
wrote:
>
>
> What I understand here in my case is that NAME is not empty (because of
> biosdevname step) so I don't understand why I don't end up with em1
> instead of the
>   onboard style name. This would mean ID_NET_NAME has been set in a
> previous step ? What was the use of the biosdevname stop then ?

On RHEL/CentOS 8 biosdevname naming is not used unless it is explicitly
enabled on the kernel command line using biosdevname=1. Since you didn't
have an interface named using biosdevname to begin with I'd assume that
this rule is always skipped (which is OK unless you have biosdevname=1 on
cmdline)

In the case of an updated system net_id failed to generate a name based on
an on board index provided by the firmware. Hence naming falls back to the
next naming scheme which is based on PCI topology. I can't explain the
difference in names between updated and newly provisioned system (provided
they are exactly identical in terms of HW, firmware, ...).  Maybe due to
some race we assign a PCI path based name because the sysfs attribute that
is used to construct an on board name (enoX) becomes available only later
on. If that is the case then it would be a driver bug. To prove this
hypothesis you need to modify net_id so that it would log about missing
attributes. Roughly here,

https://github.com/systemd-rhel/rhel-8/blob/master/src/udev/udev-builtin-net_id.c#L228

you need to call log_error() or something like that only then return
-ENOENT.

>
>
> finally, what does "If the kernel claims that the name it has set for a
> device is predictable" mean
> (https://www.freedesktop.org/software/systemd/man/systemd.link.html#) ?
>
> And what is the kernel name (%k) : is it always ethX ?

Kernel can indicate via value of name_assign_type sysfs attribute that the
already assigned name is predictable.

More details in commit message,

https://patchwork.kernel.org/patch/3733841/

Cheers,

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

Re: [systemd-devel] What is the correct value for NUMAMask to achieve all?

[Michal Sekletar]

On Fri, Nov 22, 2019 at 4:09 PM Mathew Robinson  wrote:
>
> Hey all,
>
> With the new NUMAPolicy/NUMAMask options as a service author I want to 
> specify a NUMAPolicy=interleave what mask can I apply for NUMAMask that is 
> equivalent to all? We tried blank but this appears to not work and you have 
> to specify a NUMAMask for interleave.
>
> It appears that setting some arbitrarily high range like 0-1000 works but 
> doesn't feel like the right answer.

Idea was that NUMA settings are very system specific so it is up to
sysadmin to decide what is the right value for NUMAMAsk=. On the other
hand, we could have a shorthand to mean "all NUMA nodes" to make
configuration slightly easier. Please file RFE issue on Github.

Cheers,
Michal

https://github.com/systemd/systemd/issues

>
> - Mathew Robinson @chasinglogic
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

Re: [systemd-devel] udev can fail to read stdout of processes spwaned in udev_event_spawn

[Michal Sekletar]

On Fri, Nov 1, 2019 at 1:49 AM Paul Davey
 wrote:

> What is the best way to fix this issue?  I have locally had success
> just calling the on_spawn_io callback in the process success branch of
> on_spawn_sigchld, but I am unsure if this is an acceptable fix.

In the callback, we call read() only once and I don't think we have
any guarantee that kernel will give us the entire output of the child
process in one go. I figure we should drain the file descriptor in a
loop.

Michal

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

Re: [systemd-devel] systemctl show outputs incorrect MemoryCurrent value

[Michal Sekletar]

On Wed, Jul 25, 2018 at 5:25 AM George Xie  wrote:
>
> thanks for your reply.
>
> odds enough, on both aforementioned boxes, MemoryAccounting is set to no:

There is probably some other service that had MemoryAccounting=yes
which in turn effectively (even though dbus property doesn't reflect
that) enabled MemoryAccounting on all "sibling" services.

> the good news is, after setting `DefaultMemoryAccounting=yes` in 
> /etc/systemd/system.conf, and a `systemctl daemon-reexec`, all units have 
> correct memory usage info.

Tasks and memory accounting is now enabled by default in upstream so I
figure these weird issues are more visible on CentOS where accounting
still defaults to "no".

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

Re: [systemd-devel] Failed to start Apache Tomcat Web Application Container

[Michal Sekletar]

On Tue, Jul 24, 2018 at 5:04 PM Mark Huggins  wrote:
> Question:
> Is there way to modify the template file: tomcat.service.erb to include 
> creating ‘/opt/tomcat/logs/catalina.out' file prior to execution of the 'sudo 
> systemctl daemon-reload'command? Also, I'm unsure how long  the command: 
> 'sudo systemctl daemon-reload'  takes and perhaps is a causing the failed 
> state of the tomcat.service since it times out?. If there's another 
> resolution to this issue or the path I'm leading towards is the wrong 
> direction, please advise.

In order to run some helper script prior to starting your actual
service, you can add ExecStartPre= stanza to your tomcat.service.erb
template. However, this seems more like SELinux issue instead of the
service issue. Please try to use distribution rpm instead of the
upstream download. Alternatively, you can set SELinux to permissive
mode.

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

Re: [systemd-devel] `Found ordering cycle on _SERVICE_` loops due to `local-fs.target` with `Requires` instead of `Wants` for mounts generated from `/etc/fstab`

[Michal Sekletar]

On Wed, May 9, 2018 at 9:42 PM, Uoti Urpala  wrote:

> What information would the graph contain? The basic structure of a
> cycle is always just a simple ring, and I don't see what benefit making
> a graph of that would give over just listing the nodes in order.

The simple transformation of the current text output to graph form is
not very helpful, but I think we could provide more than that. For
example, highlighting a job that systemd removed to break the cycle,
annotating edges with dependency origin (e.g. implicit dependencies
colour-coded differently), highlighting anchor job of the transaction
if it is present in the cycle.

Basically, I am trying to figure out what relevant pieces of the
runtime state could be captured in such graph and help admins (or
maintainers) to understand and fix the dependency cycle in their
environment.

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

Re: [systemd-devel] `Found ordering cycle on _SERVICE_` loops due to `local-fs.target` with `Requires` instead of `Wants` for mounts generated from `/etc/fstab`

[Michal Sekletar]

On Fri, Apr 27, 2018 at 6:01 AM, Andrei Borzenkov  wrote:

> апр 27 06:54:14 bor-Latitude-E5450 systemd[1582]: foo.service: Found
> ordering cycle on foo.service/start
> апр 27 06:54:14 bor-Latitude-E5450 systemd[1582]: foo.service: Found
> dependency on bar.service/start
> апр 27 06:54:14 bor-Latitude-E5450 systemd[1582]: foo.service: Found
> dependency on baz.service/start
> апр 27 06:54:14 bor-Latitude-E5450 systemd[1582]: foo.service: Found
> dependency on foo.service/start
> апр 27 06:54:14 bor-Latitude-E5450 systemd[1582]: foo.service: Breaking
> ordering cycle by deleting job baz.service/start
>
>
> It would be helpful it if additionally printed kind of dependency (like
> foo.service is After bar.service) because this is by far not obvious for
> average user.

I was thinking that in addition to better log messages we could
generate dot graph of the cycle and dump it to journal as a custom
field. So people can then turn it into the picture and get a better
understanding of the cycle. Do you think it would be helpful?

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

Re: [systemd-devel] Create a target unit to start & stop a group of services

[Michal Sekletar]

On Mon, Feb 26, 2018 at 10:24 AM, 林自均  wrote:

> However, it is a little bit tedious to write drop-ins for each service
> units. Is there a directive for "my-apps.target" to propagate "stop"
> operation to the 3 services? I was expecting something like
> "PropagateStopTo=docker.service sshd.service mongodb.service".

I agree that preparation steps are tedious for your scenario, but this
is because the original use-case behind PartOf was a bit different. We
needed a way to stop a group of services at the same time. Think about
foo.target and then imagine a lot of instances of foo@.service which
all have PartOf=foo.target (because of configuration in the template
unit file). Previously you couldn't really stop the instances with a
single command.

Unfortunately, we don't have a dependency (AFAIK) that only propagates
stop actions.

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

Re: [systemd-devel] Conditional clean up action

[Michal Sekletar]

On Thu, Jan 4, 2018 at 9:50 PM, Mircea Husz  wrote:
> Migrating an init script to systemd, and need to archive logs only if the
> service exited normally with a return code 0. I found $SERVICE_RESULT,
> $EXIT_CODE, and $EXIT_STATUS  but they are in a newer version of systemd, I
> am stuck with version 219 on CentOS 7.
>
> I'd love to run ExecStopPost on a conditional exit status but that's just
> not available. Before I get creative with Pre, Start, and Post bash scripts,
> is there any straightforward way of handling this kind of thing with systemd
> ?

You could query the exit code in post script via systemctl show, the
property is called ExecMainCode. Btw, in case you have RH subscription
then please open a support case and we can backport this functionality
to RHEL/CentOS.

Cheers,

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

Re: [systemd-devel] How to stop systemd-udevd reading a device after dd

[Michal Sekletar]

On Thu, Oct 12, 2017 at 6:01 PM, Akira Hayakawa  wrote:

> And I want to stop the read request because it makes it difficult to test 
> kernel code.
> So the second question is: how can I stop the read request?

You can install local udev rule that will disable udev's monitoring of
the block device.

# cat /etc/udev/rules.d/61-no-watch-sdb.rules
ACTION!="remove", SUBSYSTEM=="block", KERNEL=="sdb*", OPTIONS:="nowatch"
# udevadm control --reload-rules

Udev watches exist because block layer doesn't have any generic
mechanism that could be used by the tools (e2fsprogs and friends) to
make sure that udev picks up changes that they did, e.g. change in
filesystem label.

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

[systemd-devel] Umount of network filesystems and rescue.target

[Michal Sekletar]

Hi,

For RHEL7 we have received complains from users that network
filesystems are not unmounted before entering rescue mode. I guess
this is because mount units have IgnoreOnIsolate=yes set by default. I
think this is fine and actually desired for local filesystems, but
there is little sense in keeping network filesystem mounted while
possibly cutting of network connections.

I know that this is may not be the case for all network configuration
daemon/tools, i.e. network configuration daemon may decide to leave
network interfaces configured upon exit. However, this is definitely
not the case for CentOS/RHEL.

In the bugzilla related to this issue, it has been proposed to make
network mounts PartOf remote-fs.target. IOW, these mount units will be
stopped when we stop remote-fs.target (which should also happen upon
entering rescue mode -- isolating rescue.target).

Is this something that we want to address upstream? Does the above
solution make sense? (FWIW, at least I don't see anything terribly
wrong with it)

I appreciate any suggestions or feedback.

Thanks,

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

Re: [systemd-devel] socket unit refusing connection when JOB_STOP is pending

[Michal Sekletar]

On Wed, May 31, 2017 at 3:43 PM, Moravec, Stanislav (ERT)
 wrote:
> FYI:
> I tried to simply bypass the pending job check:
> +int ignore_stop_pending = true;
> static void socket_enter_running(Socket *s, int cfd) {
> ...
> -if (unit_stop_pending(UNIT(s))) {
> +if (!ignore_stop_pending && unit_stop_pending(UNIT(s))) {
>
> But, as expected, it's not as that easy - the startup of the service fails to 
> get queued.


This is because, stop jobs queued on shutdown have special job mode
that doesn't allow them to be replaced. When you removed the check you
caused activation to go through and that generated start jobs that
would normally replace pending stop jobs. But like I said, on shutdown
those stop job objects have the special job mode (flag) that prohibits
this.

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

Re: [systemd-devel] socket unit refusing connection when JOB_STOP is pending

[Michal Sekletar]

On Mon, May 29, 2017 at 5:44 PM, Lennart Poettering
 wrote:

> This is indeed a shortcoming in systemd's model right now: we don't
> permit a start and a stop job to be enqueued for the same unit at the
> same time. But to do what you want to do we'd need to permit that: the
> service is supposed to stop, but also temporarily start.

AFAIU, this is not exactly the case Stanislav is talking about. He
wants systemd to activate instance of a service during shutdown while
stop job is already enqueued for respective socket unit (which is
different unit). At that time there can't be any stop job enqueued for
service instance since that isn't running yet. Hence there is no
conflict between start and stop jobs. *But* this is only true when we
talk about the service instance itself. That instance can have
dependencies that are already running and are scheduled to be stopped,
and here we have the problem that Lennart is talking about.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Ordering (apt) timer services to not run at the same time

[Michal Sekletar]

On Fri, Apr 28, 2017 at 11:05 AM, Julian Andres Klode  wrote:
> From my testing, if B has After=A, and A is already started, the
> startup of B is delayed until A has completed - do you mean that
> with run queue, or is that merely by accident somehow?

Like I said, we can't do anything about job that is already dispatched.

Consider, following simple units:

# /etc/systemd/system/a.service
[Service]
ExecStart=/bin/sleep 60

# /etc/systemd/system/b.service
[Unit]
After=a.service

[Service]
ExecStart=/bin/sleep 60

When I start a.service first and then I invoke systemctl start second
time for b.service, it will start b.service immediately. Since start
job for a.service is already gone and a.service is already running.

Now consider second scenario. b.service stays unchanged and a.service
looks like this,

# /etc/systemd/system/a.service
[Service]
Type=oneshot
ExecStart=/bin/sleep 60

When I start a.service in non-blocking mode (systemctl by default
blocks until job it queued finishes) by "systemctl --no-block start
a.service" and then I list jobs via "systemctl list-jobs" I can see
following jobs,

JOB UNIT  TYPE  STATE
441 a.service start running

Then I start b.service. This time systemctl will block because start
job for b.service is waiting on start job for a.service and that is
still in the job queue. List of jobs then looks like this,

-bash-4.4# systemctl list-jobs
JOB UNIT  TYPE  STATE
631 a.service start running
667 b.service start waiting

Job for b.service can't run because there is running job for a.service
and a.service is still in activating state. That is not the case in
first scenario, due to different type of service a.

I looked over your unit files once again and I see your services are
indeed oneshots. So this should actually work for you. I don't know
why it doesn't.

>> Indeed, seems like lockfile + condition in other unit is the simplest way 
>> out.
>
> How unfortunate.

Scratch that, I've missed that you are using oneshot services.

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

[systemd-devel] Best way to configure longer start timeout for .device units?

[Michal Sekletar]

Hi,

On big setups (read: a lot of multipathed disks), probing and
assembling storage may take significant amount of time. However, by
default systemd waits only 90s (DefaultTimeoutStartSec) for
"top-level" device unit to show up, i.e. one that is referenced in
/etc/fstab.

One possible solution is to change JobTimeout for device unit by
adding x-systemd.device-timeout= option to fstab entries. This is
kinda ugly.

Another option is to bump value of DefaultTimeoutStartSec, since that
is what systemd uses as default timeout for device's unit start job.
However, this has possible negative effect on all other units as well,
e.g. service Exec* timeouts will be affected by this change.

I am looking for elegant solution that doesn't involve rewriting
automation scripts that manage /etc/fstab.

Is there any other way how to configure the timeout? Can't we
introduce new timeout value specifically for device units?

Any advice is much appreciated, thanks.

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

Re: [systemd-devel] Ordering (apt) timer services to not run at the same time

[Michal Sekletar]

On Thu, Apr 27, 2017 at 11:30 PM, Julian Andres Klode  wrote:

> Now, we seem to be missing one bit: If daily-upgrade is already
> running, and daily is about to start, daily should wait for
> daily-upgrade to finish. I had hoped that maybe that works
> automatically given that there is some ordering relation between the
> two, but that did not work out. I tried adding Conflicts, but systemd
> then said "loop to fast" and became unresponsive (not sure if caused
> by this, but maybe).

After/Before dependencies ensure ordering between respective jobs in a
transaction (actually between both jobs in a single transaction and
between jobs that are already in run queue). However, ordering doesn't
affect jobs that we've already dispatched, since they are already
running we can't do much about them.

AFAICT, you are asking for the opposite of Requisite dependency and we
don't have such dependency.

"Looping too fast" message usually indicates bug of some kind. Please
try to reproduce with latest upstream and file an issue in case this
is still reproducible.

>
> Is there a way to make this work in systemd, or do we need to add
> locking to the script invocation (like
> s#ExecStart=#ExecStart=/usr/bin/flock /path/to/lock#)?

Indeed, seems like lockfile + condition in other unit is the simplest way out.

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

[systemd-devel] Why journald has NotifyAccess=all set in the unit file?

[Michal Sekletar]

Hi everyone,

I was asked today about $subject. I quickly skimmed trough the
relevant parts of the code and current default looks like an
oversight. I think there are no processes other than journald involved
in notification handling. I think it would be nice if drop the setting
and rely on default NotifyAccess=main.

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

Re: [systemd-devel] systemd mount point disappearing on underlying device loss?

[Michal Sekletar]

On Sat, Apr 1, 2017 at 6:46 AM, Andrei Borzenkov  wrote:
> As far as I can tell, current systemd should behave as you want since
> commit 9d06297. If you want this commit to be backported, you need to
> contact your distribution.

9d06297 changed behavior only for mount units created from
/proc/mountinfo notifications. For normal mount units there is still
BindsTo dependency to respective device unit.

Also commit 9d06297 was backported to CentOS 7.2.

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

Re: [systemd-devel] F25: NAMESPACE spawning: Too many levels of symbolic links

[Michal Sekletar]

On Thu, Mar 16, 2017 at 4:29 PM, Reindl Harald  wrote:
> with systemd-229-18.fc24.x86_64 no problem at all - after upgrade to F25
> "/usr/bin/vmware-networks" while this is just a phyiscal file and was not
> touched
>
> [root@rh:~]$ rpm -q systemd
> systemd-231-14.fc25.x86_64
>
> Mar 16 16:25:23 rh systemd: vmware-vmnet.service: Failed at step NAMESPACE
> spawning /usr/bin/vmware-networks: Too many levels of symbolic links
> Mar 16 16:25:24 rh systemd: vmware-vmnet.service: Control process exited,
> code=exited status=226
> Mar 16 16:25:24 rh systemd: Failed to start VMware Virtual Machine Ethernet.
> Mar 16 16:25:24 rh systemd: vmware-vmnet.service: Unit entered failed state.
> Mar 16 16:25:24 rh systemd: vmware-vmnet.service: Failed with result
> 'exit-code'.
>
> [root@rh:~]$ stat /usr/bin/vmware-networks
>   File: '/usr/bin/vmware-networks'
>   Size: 1189920 Blocks: 2328   IO Block: 4096   regular file
> Device: 901h/2305d  Inode: 1308258 Links: 1
> Access: (0755/-rwxr-xr-x)  Uid: (0/root)   Gid: (0/root)
> Access: 2017-03-13 13:50:05.693010420 +0100
> Modify: 2017-03-13 13:50:05.734010674 +0100
> Change: 2017-03-13 13:50:05.764010860 +0100
>  Birth: -
>
> [root@rh:~]$ cat /etc/systemd/system/vmware-vmnet.service
> [Unit]
> Description=VMware Virtual Machine Ethernet
> After=vmware-modules.service
> Requires=vmware-modules.service
> Before=network.service systemd-networkd.service
>
> [Service]
> Type=forking
> ExecStart=/usr/bin/vmware-networks --start
> ExecStartPost=-/usr/sbin/sysctl -e -w net.ipv4.conf.vmnet8.forwarding=1
> ExecStartPost=-/usr/sbin/sysctl -e -w net.ipv4.conf.vmnet8.log_martians=0
> ExecStop=/usr/bin/vmware-networks --stop
>
> ReadOnlyDirectories=/usr
> InaccessibleDirectories=-/boot
> InaccessibleDirectories=-/home
> InaccessibleDirectories=-/media
> InaccessibleDirectories=-/data
> InaccessibleDirectories=-/mnt
> InaccessibleDirectories=-/mnt/data
> InaccessibleDirectories=-/root
>
> [Install]
> WantedBy=multi-user.target


Is any of inaccessible directories actually symlink? If so then I
believe you are hitting,

https://github.com/systemd/systemd/issues/3867

This is fixed upstream, but patches for this were not backported to
Fedora 25 yet. Here are Fedora bugs mentioning similar symptoms,

https://bugzilla.redhat.com/show_bug.cgi?id=131
https://bugzilla.redhat.com/show_bug.cgi?id=1414157

Michal


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

Re: [systemd-devel] systemd debug out of memory

[Michal Sekletar]

On Sun, Mar 5, 2017 at 3:59 PM, Pascal Kolijn  wrote:
> Peace,
>
> On 28/02/2017 16:00, Lennart Poettering wrote:
>> On Tue, 28.02.17 13:26, Pascal kolijn (p.kol...@vu.nl) wrote:
>>
>>> Hi List,
>>>
>>> I've subscribed to this list to ask for help in debugging a problem we
>>> seem to have with the socket activated telnetd on a rhel7 system.
>>>
>>> A default install of telnetd collects data from some small boxes
>>> deployed in the field. It works for a long time and then suddenly:
>>>
>>> Feb 26 17:46:53 bibr systemd: Created slice user-6006.slice.
>>> Feb 26 17:46:53 bibr systemd: Starting user-6006.slice.
>>> Feb 26 17:46:53 bibr systemd: Started Session 2223341 of user .
>>> Feb 26 17:46:53 bibr systemd-logind: New session 2223341 of user .
>>> Feb 26 17:46:53 bibr systemd: Starting Session 2223341 of user .
>>> Feb 26 17:46:53 bibr systemd: Started Telnet Server (:28830).
>>> Feb 26 17:46:53 pbibr001 systemd: Starting Telnet Server (:28830)...
>>> Feb 26 17:46:57 bibr systemd: Failed to fork: Cannot allocate memory
>>
>> Hmm, Linux fork() returns ENOMEM if the maximum number of tasks on the
>> system is hit (yes this is a bit misleading, but that's how it is).
>> That max number of tasks is limited for example by the max number of
>> assignable pids as configured in /proc/sys/kernel/pid_max? Maybe you
>> hit that limit? Maybe something is leaking pids on your system? not
>> reaping zombies properly?
>
> As far as I can determine running out of pids is not the issue, as I can
> see pids being reused in a day, which will not say that some may still
> go missing over time, but how do I determine if that is the case...?
>
> What I do see is that the rss of the systemd process is slowly growing
> over time in the production environment. I've not been able (yet) to
> reproduce the situation in a test environment, which is a pity. I think
> I can simulate the telnet connects more accurately after I speak with
> the developer of the said boxes, and see if I can create a reproducible
> situation.
>
>>> Feb 26 17:46:57 bibr systemd: Assertion 'pid >= 1' failed at
>>> src/core/unit.c:1996, function unit_watch_pid(). Aborting.
>>> Feb 26 17:46:57 bibr001 systemd: Caught , cannot fork for core
>>> dump: Cannot allocate memory
>>> Feb 26 17:46:57 bibr systemd: Freezing execution.
>>
>> So this is definitely a bug. If the limit is hit, we hould certainly
>> not hit an assert. I tried to figure out how this could ever happen,
>> but afaics this should not be possible on current git at least. Any
>> chance you can try to reproduce this isue with something more recent
>> than a rhel7 box?
>
> Hmmm, the version we currently use in production is:
>
> # rpm -qa | grep systemd
> systemd-libs-219-19.el7_2.13.x86_64
> systemd-219-19.el7_2.13.x86_64
> systemd-sysv-219-19.el7_2.13.x86_64

I've backported bunch of fixes for memory leaks to
systemd-219-19.el7_2.14. From changelog,

* Mon Aug 22 2016 Lukas Nykryn  - 219-19.14
- core: fix memory leak on set-default, enable, disable etc (#1331667)
- nspawn: fix minor memory leak (#1331667)
- basic: fix error/memleak in socket-util (#1331667)
- core: fix memory leak in manager_run_generators() (#1331667)
- modules-load: fix memory leak (#1331667)
- core: fix memory leak on failed preset-all (#1331667)
- sd-bus: fix memory leak in test-bus-chat (#1331667)
- core: fix memory leak in transient units (#1331667)

Fix is in the code path that is hit everytime you log onto the box,
because every session has its own scope unit.

- bus: fix leak in error path (#1331667)
- shared/logs-show: fix memleak in add_matches_for_unit (#1331667)

>
> I think I can update it to the current state in 7.3 for the production
> machine, but will be reluctant to go for a more recent version...

Those fixes are of course included in 7.3 as well.

Michal

>
> Maybe in the test env, if I can reproduce it there.
>
>> Either way it appears that there's both a bug on your setup and in
>> systemd: something leaks processes (which is bug #1, in your setup)
>> and then systemd doesn't deal properly with that (which is bug #2, in
>> systemd upstream)...
>>
>> Lennart
>>
>
> Pascal Kolijn
> Vrije Universiteit Amsterdam
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] How to use machinectl to get a running centos container?

[Michal Sekletar]

On Fri, Mar 3, 2017 at 4:09 PM, Lennart Poettering
 wrote:
> On Sat, 04.03.17 01:38, Daurnimator (q...@daurnimator.com) wrote:
>
>> On 3 March 2017 at 20:58, Lennart Poettering  wrote:
>> > On Fri, 03.03.17 12:34, Daurnimator (q...@daurnimator.com) wrote:
>> >
>> >> I'm trying to set up a centos 7 container with machinectl.
>> >> I've tried to run:
>> >>
>> >> machinectl pull-raw --verify=no
>> >> http://cloud.centos.org/centos/7/images/CentOS-7-x86_64-GenericCloud-1701.raw.tar.gz
>> >
>> > Hmm, what is a ".raw.tar.gz" file? That suffix makes no sense to me...
>>
>> *shrugs* it's what I saw available for download from
>> http://cloud.centos.org/centos/7/images/
>>
>> Apparently it's a gziped tar with a single file inside:
>> CentOS-7-x86_64-GenericCloud-20170131_01.raw
>> This .raw file is a disk image.
>
> That appears a bit redundant, and importd/machinectl pull-raw is not
> able to handle this.
>
>
>> > We support raw disk images and tarballs with OS trees in them, both
>> > compressed and non-compressed.
>> >
>> > There's currently a safety limit against overly large images enforced,
>> > of 8GiB. If the indicated image is larger than that, and that's
>> > intended we should probably bump this safety limit substantially (32G?
>> > 64G?), please file a github issue asking for this if this is the
>> > case. Or even better prep a PR, the fix is trivial:
>> >
>> > https://github.com/systemd/systemd/blob/master/src/import/pull-job.c#L530
>>
>> Looks like it's *equal* to the limit.
>>
>> Before I make a PR here, am I going about running a centos container
>> with machinectl the best way here?
>> How are other people doing this?
>
> I don't think many people are using CentOS caontainers with
> nspawn... That said, there's a good chance that it works OKish.

I use them regularly and they work just fine (well I use RHEL7 but
that should not matter). However I don't download images from
anywhere. I install distro trees to /var/lib/machines/ manually using
dnf.

>
> Note that "machinectl pull-raw" is just a helper to make downloading
> easy. But if you have images in weird formats, you can download them
> and place them in /var/lib/machines (with the .raw suffix), and
> machined/nspawn is happy. It doesn't really matter how the image gets
> there as long as it gets there, and "machinectl pull-raw" is just one
> way.

That is what I also recommend. Installing from repo always worked for
me. For basic system container I just use example from nspawn manpage.

Michal

>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] mount-on-demand for backups; hooks for indicating success/failure

[Michal Sekletar]

On Thu, Mar 9, 2017 at 4:53 PM, Jonathan Dowland
 wrote:
> Hey,
>
> I have some backup services which depend on mounts. I want those
> filesystems unmounted when the backup jobs are not running. This is
> easily achieved with StopWhenUnneeded.
>
> I also want to trigger the backup jobs to start when I attach my
> external HDD. This is reasonably simple by adding WantedBy=
> to the backup service (*not* the mount unit, or it will never be
> auto-unmounted).

WantedBy=device sounds like a weird hack to me. I think that it would
be better to use SYSTEM_WANTS in udev rules instead.

>
> So far so good!
>
> However, this is a headless box and I want to blink an LED when certain
> jobs are running, finish or fail. So I need to execute commands in
> certain situations. Job failure is easy, I can use OnFailure= and set up
> a oneshot service. Job started isn't too bad, I add a ExecStart=-
> to my backup service before the real one.

OnFailure is fine and ExecStart before starting backup also sounds reasonable.

>
> Signalling "device is safe to remove" I have not figured out.
> Using a chain of blah.device -> blah.mount -> backup-blah.service units,
> "safe to remove" LED should only happen after the mount has completed
> unmounting, successfully. Is there an existing "hook" or place that I
> could put that in this situation?

It should be possible to achieve this with normal dependencies. For
example, you would have blink-successful.service that would Require
backup service and would be ordered after it.

Dependencies should then look like this,
# backup.service
[Unit]
Wants=blink-successful.service

# blink-successful.service,
[Unit]
Requires=backup.service
After=backup.service

backup.service pulls in blink service and that will run only when
backup succeeded.

>
> My temporary solution is to remove the mount unit entirely, define the
> mount point in /etc/fstab and use ExecStart and ExecStop in the backup
> service to mount and umount before and after the job runs:
>
> [Unit]
> OnFailure=blinkstick-fail.service
> [Service]
> ExecStart=/bin/mount /media/ipod

I'd leave fstab entry in place and replace ExecStart=/bin/mount with
RequiresMountsFor=/media/ipod (this belongs to [Unit] section). So
when this service is started it will pull in mount unit to transaction
and gets ordered after the mount unit.

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

Re: [systemd-devel] udev virtio by-path naming

[Michal Sekletar]

On Fri, Feb 24, 2017 at 10:56 AM, Viktor Mihajlovski
 wrote:
> On 20.02.2017 17:00, Cornelia Huck wrote:
>> On Mon, 20 Feb 2017 15:34:49 +0100
>> Viktor Mihajlovski  wrote:
>>
>>> Hi,
>>>
>>> with systemd > v229 all virtio block devices will receive persistent
>>> device names in the format /dev/disk-by/virtio-pci-, the
>>> last component being the udev built-in path_id.
>>>
>>> This naming introduces some issues.
>>>
>>> First and obvious, there are virtio implementations not based
>>> on PCI, like virtio-ccw (currently only on s390) and virtio-mmio
>>> (for which I can't speak). This results in persistent names like
>>> /dev/disk-by/virtio-pci-0.0.0001, where the bus id is a CCW id.
>>> One seemingly obvious remedy would be to make the path_id return
>>> virtio-ccw- or more generally virtio--,
>>> both easily done with small patches to systemd-udev.
>>>
>>> But then, I find this naming scheme somewhat weird.
>>> A virtio disk shows up as a regular PCI function on the PCI
>>> bus side by side with other (non-virtio) devices. The naming otoh
>>> suggests that virtio-pci is a subsystem of its own, which is simply
>>> incorrect from a by-path perspective.
>>
>> From the ccw perspective, this is quite similar: The virtio proxy
>> device shows up on the ccw bus, just like e.g. a dasd device shows up
>> on the ccw bus.
>>
>>>
>>> Using just the plain PCI path id is actually sufficient to identify
>>> a virtio disk by its path. This would be in line with virtio
>>> network interface path names which use the plain PCI naming.
>>
>> Same for ccw: The id on the ccw bus (devno) is already unique and
>> persistent.
>>
>>>
>>> One could argue about back-level compatibility, but virtio by-path
>>> naming has changed multiple times. We have seen virtio-pci-virtio
>>> (not predictable), pci- and virtio-pci- already. It
>>> might be a good time now to settle on a common approach for all
>>> virtio types.
>>>
>>> For the reasons above, I'd vote for -, which
>>> would work for PCI and CCW, not sure about ARM MMIO though.
>>> Opinions?
>>
>> I'm not sure whether there is any reason to make virtio special,
>> although this depends upon what virtio-mmio looks like in the Linux
>> device model (any arm folks here?)
>>
>> In the end, I'd be happy with any naming scheme that does not include
>> 'pci' for non-pci devices.
>>
> Michal, as author of commit f073b1b3c0f4f0df1b0bd61042ce85fb5d27d407
> that introduced this behavior: can you comment on the reasoning for
> prepending virtio- to the bus, i.e. why would pci- not
> sufficiently identify the device?

As with many things, it looked like a good idea at the time. In commit
message I referenced discussion on upstream mailing list. In that
thread we got reassured that there is one-to-one correspondence
between virtio and pci devices. IIRC, we had bugs for RHEL and CentOS
7 were users requested these symlinks. Hence we went ahead with above
naming scheme.

Your argument about virtio being first component of the path makes
sense to me, but then again, we wanted to distinguish these devices
(because of user demands).

In retrospect what we did wasn't probably the best we could.
Nevertheless, I'd not change the naming. After all, these symlinks are
for users and since then I didn't hear complains about them. I'd only
adjust the code so that we produce correctly named symlinks for
different parent buses (pci, ccw, mmio). IOW we would remove hardcoded
"pci" string.

Michal
>
> --
>
> Mit freundlichen Grüßen/Kind Regards
>Viktor Mihajlovski
>
> IBM Deutschland Research & Development GmbH
> Vorsitzender des Aufsichtsrats: Martina Köderitz
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [libvirt] How to make udev not touch my device?

[Michal Sekletar]

On Fri, Nov 11, 2016 at 2:20 PM, Daniel P. Berrange  wrote:

> What kind of issues ?

General problem with manually created device nodes is that udev and
systemd do not know about them. Device units do not exist for these
device nodes. Hence these device units can not be a dependency of some
other unit. Typical example is manually created device node referenced
from /etc/fstab. Then corresponding mount unit is bound to a device
that never shows up and hence it always fails to mount even tough
device node is there.

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

Re: [systemd-devel] [libvirt] How to make udev not touch my device?

[Michal Sekletar]

On Mon, Nov 7, 2016 at 1:20 PM, Daniel P. Berrange  wrote:

> So if libvirt creates a private mount namespace for each QEMU and mounts
> a custom /dev there, this is invisible to udev, and thus udev won't/can't
> mess with permissions we set in our private /dev.
>
> For hotplug, the libvirt QEMU would do the same as the libvirt LXC driver
> currently does. It would fork and setns() into the QEMU mount namespace
> and run mknod()+chmod() there, before doing the rest of its normal hotplug
> logic. See lxcDomainAttachDeviceMknodHelper() for what LXC does.

We try to migrate people away from using mknod and messing with /dev/
from user-space. For example, we had to deal with non-trivial problems
wrt. mknod and Veritas storage stack in the past (most of these issues
remain unsolved to date). I don't like to hear that you plan to get
into /dev management business in libvirt too. I am judging based on
past experiences, nevertheless, I don't like this plan.

Also, managing separate mount namespace for each qemu process and
forking helper that joins the namespace to do some work seems quite
complex too.

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

Re: [systemd-devel] systemd-222 mount issues on CentOS 7

[Michal Sekletar]

On Tue, Sep 27, 2016 at 5:05 PM, Lokesh Mandvekar
 wrote:
> Now, I can mount these partitions with:
>
> # lvm vgchange -ay
>
> but this still doesn't automount succesfully on a reboot.
>
> Did I miss something here?

I'd check from emergency shell whether lvm2-pvscan@.service was run.
This instantiated systemd service is responsible for scaning LVM PVs
and auto-activating LVs on them. Note that it is spawned from udev
rules in case when certain conditions are met, e.g. block device is
identified as LVM2_member and udev event doesn't have SYSTEMD_READY=0
property set.

Also, there has been couple of bugfixes since systemd-222 that are
maybe related. We backported them to RHEL/CentOS 7 (systemd-219).

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

Re: [systemd-devel] UEFI menu entries wiped from BIOS after power off at dm-crypt boot prompt

[Michal Sekletar]

On Sat, May 21, 2016 at 10:43 PM, Jamie Kitson  wrote:
> if I power off my computer at the dm-crypt boot password prompt my UEFI
> menu entries get wiped from the BIOS and reset to the single default
> Windows option.

What do you exactly mean by "menu entries get wiped from the BIOS"?
Does BootOrder variable have an unexpected content on next boot?

>
> This is with an Asus UX32VD laptop, Grub UEFI and systemd and sd-encrypt
> mkinitcpio hooks.

That means you are not using systemd-boot at all?
>
> If this isn't a systemd issue could anyone have a guess as to where the
> issue might lie?

If this is really about boot loader entries and not about BootOrder,
then I'd guess that you are probably hitting some weird bug in
firmware's FAT driver.

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

Re: [systemd-devel] Transaction contains conflicting jobs 'restart' and 'stop'

[Michal Sekletar]

On Thu, Mar 10, 2016 at 10:11 PM, Orion Poplawski  wrote:

> Can't the stop of iptables be dropped because the service is already stopped
> (or more likely not even present)?

Isn't this the case already? I simplified your scenario, i.e. A
conflicts B and C is part of both A and B. If I first start B and C
and then issue stop for B, then follow-up restart of A doesn't produce
an error. I observed the problem only after trying to restart A when B
and C were running.

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

Re: [systemd-devel] centos-ci

[Michal Sekletar]

On Tue, Apr 12, 2016 at 5:58 PM, Lennart Poettering
 wrote:

> The rhel-ci people offered us to use their spare machines, that's
> all. And Daniel took them up on it, and is now working on getting this
> hooked up.

It is not even rhel-ci (as in Red Hat internal), rather CentOS CI,
which is community service available to other free/open source
projects as well.

Michal

https://ci.centos.org/
https://fosdem.org/2016/schedule/event/centos_ci_getting_started/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd efi boot and default entry

[Michal Sekletar]

On Thu, Mar 31, 2016 at 11:10 AM, Jóhann B. Guðmundsson
<johan...@gmail.com> wrote:
>
>
> On 03/30/2016 03:49 PM, Michal Sekletar wrote:
>>
>> On Mon, Mar 21, 2016 at 1:42 PM, Vasiliy Tolstov <v.tols...@selfip.ru>
>> wrote:
>>
>>> Now i want to have two entries and assign priority to it via systemd,
>>> in my use-case i want to know last succeseful boot entry and use it.
>>> After upgrade i want to boot from new antry and if it fails - change
>>> priority to lower level...
>>
>> I don't believe this is currently possible. I've tried to implement
>> similar scheme in the past. I should probably resurrect that effort,
>>
>
> Had you finished writing the kernel driver that implements some kind of (
> sysfs? ) boot counting scheme?
> ( There is no  point in implementing something in systemd until that is in
> place )

We don't need to extend the kernel in order to implement this
particular mechanism. After new kernel is installed, you make it
default and mark as "tentative". Then, after first successful boot of
newly added bootloader entry you just remove the flag, because it is
known to work.

I withdrew my PR because we discussed this with Kay and we were not
sure we liked proposed scheme which uses file on disk as "tentative"
marker.

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

Re: [systemd-devel] [networkd] Set MTU of disconnected OVS bridge

[Michal Sekletar]

On Thu, Mar 24, 2016 at 3:52 PM, Ian Pilcher  wrote:
> I haven't been able to figure out a way to get systemd-networkd to do
> this.  I tried creating /etc/systemd/network/ovs1.link, but it had no
> effect:
>
>   [Match]
>   OriginalName=ovs1
>
>   [Link]
>   MTUBytes=9000
>

Are you sure that bridge is really named ovs1 first time it appears on
the system? To make sure you can run udevadm monitor -k -p and then
add the bridge. Look for value of INTERFACE property.

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

Re: [systemd-devel] systemd efi boot and default entry

[Michal Sekletar]

On Mon, Mar 21, 2016 at 1:42 PM, Vasiliy Tolstov  wrote:

> Now i want to have two entries and assign priority to it via systemd,
> in my use-case i want to know last succeseful boot entry and use it.
> After upgrade i want to boot from new antry and if it fails - change
> priority to lower level...

I don't believe this is currently possible. I've tried to implement
similar scheme in the past. I should probably resurrect that effort,

https://github.com/systemd/systemd/pull/1894

In the meantime, you can change default boot entry manually by
selecting it in the menu and pressing 'd' key.

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

Re: [systemd-devel] Issues with docker systemd cgroups integration

[Michal Sekletar]

On Tue, Mar 15, 2016 at 5:42 PM, Mrunal Patel  wrote:
>
>
>
>> >
>> > docker.io did that too three weeks ago:
>> >
>> >   https://github.com/docker/docker/commit/65820132
>>
>> This seems odd. I thought you primarily want to enable delegation for
>> scope units wrapping processes which constitute your container and not
>> for the container manager itself. For example, on Fedora 23
>> libvirtd.service doesn't have Delegate=yes. However libvirtd registers
>> VMs as machines in machined. And machined will take care for creating
>> scope unit and it also sets Delegate=yes for the scope.
>>
> Yes, this is the model we have. So, it looks like we should set Delegate=yes
> for the scope. Last time I tried, I got some errors from the API.
> I will try again and respond back on this thread.

You can have a look at our C implementation and re-implement it in Go.

https://github.com/systemd/systemd/blob/master/src/machine/machined-dbus.c#L1277

Michal

>
> Thanks,
> Mrunal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Issues with docker systemd cgroups integration

[Michal Sekletar]

On Tue, Mar 15, 2016 at 4:56 PM, Martin Pitt <martin.p...@ubuntu.com> wrote:
> Michal Sekletar [2016-03-15 16:06 +0100]:
>> We had similar problem in the past with libvirtd and it got solved by
>> introducing Delegate option (man systemd.resource-control).
>
> docker.io did that too three weeks ago:
>
>   https://github.com/docker/docker/commit/65820132

This seems odd. I thought you primarily want to enable delegation for
scope units wrapping processes which constitute your container and not
for the container manager itself. For example, on Fedora 23
libvirtd.service doesn't have Delegate=yes. However libvirtd registers
VMs as machines in machined. And machined will take care for creating
scope unit and it also sets Delegate=yes for the scope.

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

Re: [systemd-devel] Issues with docker systemd cgroups integration

[Michal Sekletar]

On Mon, Mar 14, 2016 at 6:42 PM, Daniel J Walsh  wrote:
>> To see the code you can refer to
>> https://github.com/projectatomic/docker/pull/71/files (In this PR, I made
>> the change to always join all the subsystems after creating the scope to
>> help with first issue below).

Why do you touch cgroup hierarchies which systemd manages? This is not
supported. I think you should consider applying following patch,

@@ -375,7 +375,7 @@ func setKernelMemory(c *configs.Cgroup) error {
 func joinCgroups(c *configs.Cgroup, pid int) error {
for name, sys := range subsystems {
switch name {
-   case "name=systemd":
+   case "name=systemd", "devices", "memory", "cpu",
"cpuacct", "blkio":
// let systemd handle this
break
case "cpuset":


>> There are two classes of issues that are cropping up (especially under
>> load):
>> 1. Sometimes a cgroup isn't joined even though it is passed as a property
>> while creating a Transient Unit. For e.g. CPUShares are specified, however
>> the transient unit doesn't seem to join
>> the cpu,cpuacct subsystem.

This is probably a bug.

>> 2. cpu.proc file itself goes missing especitally with the devices
>> subsystemd even thought we mkdir the directory and write to the file. It is
>> as if the file gets deleted by some other process.

We had similar problem in the past with libvirtd and it got solved by
introducing Delegate option (man systemd.resource-control). Setting
Delegate=yes for unit will cause that cgroup hierarchy for this unit
will be created in all controllers. Then you can manage cgroup
settings for controllers not managed by systemd.

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

Re: [systemd-devel] Support for large applications

[Michal Sekletar]

On Fri, Feb 19, 2016 at 1:49 PM, Zbigniew Jędrzejewski-Szmek
 wrote:

> I don't think there's a way around the issue short of allowing
> watchdog during startup. Databases which do long recovery are a bit
> special, most programs don't exhibit this kind of behaviour, but maybe
> this case is important enough to add support for it.

Rather than configuring huge start timeout for the service, firing it
up and hoping for the best I was thinking about disabling start
timeout entirely, make service of type "notify" and signal service
readiness via sd_notify. Also update progress by sending STATUS
message when some significant stage of DB startup procedure finishes.
This is not solving watchdog problem, but I think it would be an
improvement anyway. At least, it would provide better debugability of
start-up of this particular service.

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

Re: [systemd-devel] Support for large applications

[Michal Sekletar]

On Wed, Feb 17, 2016 at 1:35 PM, Avi Kivity  wrote:

> 3. watchdog during startup
>
> Sometimes we need to perform expensive operations during startup (log
> replay, rebuild from network replica) before we can start serving. Rather
> than configure a huge start timeout, I'd prefer to have the service report
> progress to systemd so that it knows that startup is still in progress.
>

Did you have a look at sd_notify (man 3 sd_notify)? Basically, you can
easily patch your service to report status to systemd and tell to
systemd exactly when it is ready to serve the clients. Thus you can
avoid hacks like huge start timeout you've mentioned.

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

Re: [systemd-devel] I want to run systemd inside of a locked down base docker container

[Michal Sekletar]

On Thu, Feb 11, 2016 at 2:48 PM, Daniel J Walsh  wrote:

> I am now masking nothing, just removing /etc/fstab.   We will probably
> need to back port the dev-hugepages.mount fix
> to rhel7 at some point.

On RHEL-7.2 dev-hugepages.mount already has ConditionCapability=CAP_SYS_ADMIN.

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

Re: [systemd-devel] [packaging] split of systemd package

[Michal Sekletar]

On Wed, Nov 11, 2015 at 11:52 AM, Jóhann B. Guðmundsson
 wrote:
>
> I thought the conscious was not recommending downstream to split systemd
> into subpackages?
>

This decision was recently (at systemd.conf) reevaluated :)

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

[systemd-devel] [PATCH] install: follow symlinks when enabling unit files from /usr/

[Michal Sekletar]

Right now it is difficult for distros to ship convenience/compat alias for some
service, e.g. mariadb aliased to mysql or nfs-server to nfs. If service which
comes with alias is not enabled by default then user must refer to its new unit
file name when trying to enable the service. Contrary, using alias for
start or stop works fine.

This patch introduces exception to current handling of enable requests. If we
find symlink instead of the unit file under /usr such that it has target in the
same directory, we then assume this is a convenience alias and we will follow
the symlink regardless the value of allow_symlink parameter.
---

Caveat: Patch doesn't cover case when full-path to symlink in /usr is given.
Not sure we want to support this though.

 src/shared/install.c | 84 ++--
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index 6172c42..18cc63e 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -824,12 +824,16 @@ static void install_info_free(UnitFileInstallInfo *i) {
 
 static void install_info_hashmap_free(OrderedHashmap *m) {
 UnitFileInstallInfo *i;
+char *name;
 
 if (!m)
 return;
 
-while ((i = ordered_hashmap_steal_first(m)))
+while ((name = ordered_hashmap_first_key(m))) {
+i = ordered_hashmap_steal_first(m);
 install_info_free(i);
+free(name);
+}
 
 ordered_hashmap_free(m);
 }
@@ -849,6 +853,7 @@ static int install_info_add(
 const char *path) {
 UnitFileInstallInfo *i = NULL;
 int r;
+char *n = NULL;
 
 assert(c);
 assert(name || path);
@@ -885,7 +890,18 @@ static int install_info_add(
 }
 }
 
-r = ordered_hashmap_put(c-will_install, i-name, i);
+n = strdup(name);
+if (!n) {
+r = -ENOMEM;
+goto fail;
+}
+
+/*
+ *  There is a possibility that i-name will be changed if we figure 
out that we are not
+ *  handling unit file but rather symlink to another unit file in 
/usr. Thus we can't use
+ *  i-name as a key.
+ */
+r = ordered_hashmap_put(c-will_install, n, i);
 if (r  0)
 goto fail;
 
@@ -895,6 +911,8 @@ fail:
 if (i)
 install_info_free(i);
 
+free(n);
+
 return r;
 }
 
@@ -1108,6 +1126,68 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
+if (!path_startswith(path, /etc)  !path_startswith(path, 
/run)) {
+_cleanup_free_ char *root_target = NULL, *root_path = 
NULL;
+
+if (root_dir)
+root_path = path_join(root_dir, path, NULL);
+else
+root_path = strdup(path);
+
+if (!root_path)
+return -ENOMEM;
+
+r = readlink_and_canonicalize(root_path, root_target);
+if (r = 0) {
+_cleanup_free_ char *root_target_prefix= NULL, 
*target = NULL;
+
+root_target_prefix = path_join(root_dir, *p, 
NULL);
+if (!root_target_prefix)
+return -ENOMEM;
+
+if (path_startswith(root_target, *p) ||
+ path_startswith(root_target, 
root_target_prefix)) {
+char *name, *n = NULL;
+
+name = basename(root_target);
+target = path_join(*p, name, NULL);
+if (!target)
+return -ENOMEM;
+
+free(path);
+path = target;
+target = NULL;
+
+if (unit_name_is_valid(info-name, 
UNIT_NAME_INSTANCE)) {
+_cleanup_free_ char *instance 
= NULL;
+char *at;
+
+at = strchr(info-name, '@');
+
+assert(at);
+
+instance = strdup(++at);
+if (!instance)
+return -ENOMEM;
+
+at = strchr(name, '@');
+if (at) {
+

Re: [systemd-devel] [PATCH v2] log: be more verbose if dbus job fails

[Michal Sekletar]

On Thu, Apr 09, 2015 at 02:44:38PM +, Zbigniew Jędrzejewski-Szmek wrote:
 On Thu, Apr 09, 2015 at 04:35:53PM +0200, Michal Sekletar wrote:
  On Thu, Apr 09, 2015 at 02:10:14PM +, Zbigniew Jędrzejewski-Szmek wrote:
   On Thu, Apr 09, 2015 at 03:20:02PM +0200, Michal Sekletar wrote:
Users might have hard time figuring out why exactly their systemctl 
request
failed. If dbus job fails try to figure out more details about failure 
by
examining Result property of the service.

https://bugzilla.redhat.com/show_bug.cgi?id=1016680
---
 src/libsystemd/sd-bus/bus-util.c | 41 +++---
 src/shared/log.c | 76 

 src/shared/log.h |  2 ++
 3 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-util.c 
b/src/libsystemd/sd-bus/bus-util.c
index 6498769..bb4c993 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/sd-bus/bus-util.c
@@ -30,6 +30,7 @@
 #include path-util.h
 #include missing.h
 #include set.h
+#include unit-name.h
 
 #include sd-bus.h
 #include bus-error.h
@@ -1716,6 +1717,35 @@ static int bus_process_wait(sd_bus *bus) {
 }
 }
 
+static int bus_job_get_service_result(BusWaitForJobs *d, char 
**result) {
+int r = 0;
+_cleanup_free_ char *dbus_path = NULL, *p = NULL;
+
+assert(d);
+assert(d-result);
+assert(d-name);
+assert(result);
+
+dbus_path = unit_dbus_path_from_name(d-name);
+if (!dbus_path)
+return -ENOMEM;
+
+r = sd_bus_get_property_string(d-bus,
+   org.freedesktop.systemd1,
+   dbus_path,
+   
org.freedesktop.systemd1.Service,
+   Result,
+   NULL,
+   p);
+if (r  0)
+return r;
+
+*result = p;
+p = NULL;
+
+return 0;
+}
+
 static int check_wait_response(BusWaitForJobs *d, bool quiet) {
 int r = 0;
 
@@ -1736,13 +1766,14 @@ static int check_wait_response(BusWaitForJobs 
*d, bool quiet) {
 log_error(Operation on or unit type of %s not 
supported on this system., strna(d-name));
 else if (!streq(d-result, done)  
!streq(d-result, skipped)) {
 if (d-name) {
-bool quotes;
+int q;
+_cleanup_free_ char *result = NULL;
 
-quotes = chars_intersect(d-name, 
SHELL_NEED_QUOTES);
+q = bus_job_get_service_result(d, 
result);
+if (q  0)
+log_debug_errno(q, Failed to 
get Result property of service %s: %m, d-name);
 
-log_error(Job for %s failed. See 
\systemctl status %s%s%s\ and \journalctl -xe\ for details.,
-  d-name,
-  quotes ? ' : , d-name, 
quotes ? ' : );
+
log_job_error_with_service_result(d-name, result);
 } else
 log_error(Job failed. See 
\journalctl -xe\ for details.);
 }
diff --git a/src/shared/log.c b/src/shared/log.c
index 646a1d6..3219756 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -1061,3 +1061,79 @@ void log_received_signal(int level, const struct 
signalfd_siginfo *si) {
 void log_set_upgrade_syslog_to_journal(bool b) {
 upgrade_syslog_to_journal = b;
 }
+
+typedef enum ServiceResult {
+SERVICE_SUCCESS,
+SERVICE_FAILURE_RESOURCES,
+SERVICE_FAILURE_TIMEOUT,
+SERVICE_FAILURE_EXIT_CODE,
+SERVICE_FAILURE_SIGNAL,
+SERVICE_FAILURE_CORE_DUMP,
+SERVICE_FAILURE_WATCHDOG,
+SERVICE_FAILURE_START_LIMIT,
+_SERVICE_RESULT_MAX,
+_SERVICE_RESULT_INVALID = -1
+} ServiceResult;
   
   dbus property returns the result as a string. So we need to map this 
   string
   to an explanation. Going through the intermediate ServiceResult int does
   not really gain anything... I think things would be simpler if you added a
   table mapping result to explanation:
   
   static const struct {
 const char *result, *explanation;
   } explanations

Re: [systemd-devel] [PATCH v2] log: be more verbose if dbus job fails

[Michal Sekletar]

On Thu, Apr 09, 2015 at 02:10:14PM +, Zbigniew Jędrzejewski-Szmek wrote:
 On Thu, Apr 09, 2015 at 03:20:02PM +0200, Michal Sekletar wrote:
  Users might have hard time figuring out why exactly their systemctl request
  failed. If dbus job fails try to figure out more details about failure by
  examining Result property of the service.
  
  https://bugzilla.redhat.com/show_bug.cgi?id=1016680
  ---
   src/libsystemd/sd-bus/bus-util.c | 41 +++---
   src/shared/log.c | 76 
  
   src/shared/log.h |  2 ++
   3 files changed, 114 insertions(+), 5 deletions(-)
  
  diff --git a/src/libsystemd/sd-bus/bus-util.c 
  b/src/libsystemd/sd-bus/bus-util.c
  index 6498769..bb4c993 100644
  --- a/src/libsystemd/sd-bus/bus-util.c
  +++ b/src/libsystemd/sd-bus/bus-util.c
  @@ -30,6 +30,7 @@
   #include path-util.h
   #include missing.h
   #include set.h
  +#include unit-name.h
   
   #include sd-bus.h
   #include bus-error.h
  @@ -1716,6 +1717,35 @@ static int bus_process_wait(sd_bus *bus) {
   }
   }
   
  +static int bus_job_get_service_result(BusWaitForJobs *d, char **result) {
  +int r = 0;
  +_cleanup_free_ char *dbus_path = NULL, *p = NULL;
  +
  +assert(d);
  +assert(d-result);
  +assert(d-name);
  +assert(result);
  +
  +dbus_path = unit_dbus_path_from_name(d-name);
  +if (!dbus_path)
  +return -ENOMEM;
  +
  +r = sd_bus_get_property_string(d-bus,
  +   org.freedesktop.systemd1,
  +   dbus_path,
  +   org.freedesktop.systemd1.Service,
  +   Result,
  +   NULL,
  +   p);
  +if (r  0)
  +return r;
  +
  +*result = p;
  +p = NULL;
  +
  +return 0;
  +}
  +
   static int check_wait_response(BusWaitForJobs *d, bool quiet) {
   int r = 0;
   
  @@ -1736,13 +1766,14 @@ static int check_wait_response(BusWaitForJobs *d, 
  bool quiet) {
   log_error(Operation on or unit type of %s not 
  supported on this system., strna(d-name));
   else if (!streq(d-result, done)  !streq(d-result, 
  skipped)) {
   if (d-name) {
  -bool quotes;
  +int q;
  +_cleanup_free_ char *result = NULL;
   
  -quotes = chars_intersect(d-name, 
  SHELL_NEED_QUOTES);
  +q = bus_job_get_service_result(d, result);
  +if (q  0)
  +log_debug_errno(q, Failed to get 
  Result property of service %s: %m, d-name);
   
  -log_error(Job for %s failed. See 
  \systemctl status %s%s%s\ and \journalctl -xe\ for details.,
  -  d-name,
  -  quotes ? ' : , d-name, 
  quotes ? ' : );
  +log_job_error_with_service_result(d-name, 
  result);
   } else
   log_error(Job failed. See \journalctl 
  -xe\ for details.);
   }
  diff --git a/src/shared/log.c b/src/shared/log.c
  index 646a1d6..3219756 100644
  --- a/src/shared/log.c
  +++ b/src/shared/log.c
  @@ -1061,3 +1061,79 @@ void log_received_signal(int level, const struct 
  signalfd_siginfo *si) {
   void log_set_upgrade_syslog_to_journal(bool b) {
   upgrade_syslog_to_journal = b;
   }
  +
  +typedef enum ServiceResult {
  +SERVICE_SUCCESS,
  +SERVICE_FAILURE_RESOURCES,
  +SERVICE_FAILURE_TIMEOUT,
  +SERVICE_FAILURE_EXIT_CODE,
  +SERVICE_FAILURE_SIGNAL,
  +SERVICE_FAILURE_CORE_DUMP,
  +SERVICE_FAILURE_WATCHDOG,
  +SERVICE_FAILURE_START_LIMIT,
  +_SERVICE_RESULT_MAX,
  +_SERVICE_RESULT_INVALID = -1
  +} ServiceResult;
 
 dbus property returns the result as a string. So we need to map this string
 to an explanation. Going through the intermediate ServiceResult int does
 not really gain anything... I think things would be simpler if you added a
 table mapping result to explanation:
 
 static const struct {
   const char *result, *explanation;
 } explanations[] = {
   {timeout, start timeout was exceeded},
   ...
   {}
 };
 
 and then simply looped over this table. This way it won't be necessary to
 have three different lists.

I guess that what you are suggesting will remove a bit of boilerplate code, will
do that.

 
 This could live simply in systemctl.c. Those are only parts of
 sentences, nothing generally useful.

I'd say that having general function for logging about failed dbus

[systemd-devel] [PATCH v2] log: be more verbose if dbus job fails

[Michal Sekletar]

Users might have hard time figuring out why exactly their systemctl request
failed. If dbus job fails try to figure out more details about failure by
examining Result property of the service.

https://bugzilla.redhat.com/show_bug.cgi?id=1016680
---
 src/libsystemd/sd-bus/bus-util.c | 41 +++---
 src/shared/log.c | 76 
 src/shared/log.h |  2 ++
 3 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
index 6498769..bb4c993 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/sd-bus/bus-util.c
@@ -30,6 +30,7 @@
 #include path-util.h
 #include missing.h
 #include set.h
+#include unit-name.h
 
 #include sd-bus.h
 #include bus-error.h
@@ -1716,6 +1717,35 @@ static int bus_process_wait(sd_bus *bus) {
 }
 }
 
+static int bus_job_get_service_result(BusWaitForJobs *d, char **result) {
+int r = 0;
+_cleanup_free_ char *dbus_path = NULL, *p = NULL;
+
+assert(d);
+assert(d-result);
+assert(d-name);
+assert(result);
+
+dbus_path = unit_dbus_path_from_name(d-name);
+if (!dbus_path)
+return -ENOMEM;
+
+r = sd_bus_get_property_string(d-bus,
+   org.freedesktop.systemd1,
+   dbus_path,
+   org.freedesktop.systemd1.Service,
+   Result,
+   NULL,
+   p);
+if (r  0)
+return r;
+
+*result = p;
+p = NULL;
+
+return 0;
+}
+
 static int check_wait_response(BusWaitForJobs *d, bool quiet) {
 int r = 0;
 
@@ -1736,13 +1766,14 @@ static int check_wait_response(BusWaitForJobs *d, bool 
quiet) {
 log_error(Operation on or unit type of %s not 
supported on this system., strna(d-name));
 else if (!streq(d-result, done)  !streq(d-result, 
skipped)) {
 if (d-name) {
-bool quotes;
+int q;
+_cleanup_free_ char *result = NULL;
 
-quotes = chars_intersect(d-name, 
SHELL_NEED_QUOTES);
+q = bus_job_get_service_result(d, result);
+if (q  0)
+log_debug_errno(q, Failed to get 
Result property of service %s: %m, d-name);
 
-log_error(Job for %s failed. See \systemctl 
status %s%s%s\ and \journalctl -xe\ for details.,
-  d-name,
-  quotes ? ' : , d-name, quotes ? 
' : );
+log_job_error_with_service_result(d-name, 
result);
 } else
 log_error(Job failed. See \journalctl -xe\ 
for details.);
 }
diff --git a/src/shared/log.c b/src/shared/log.c
index 646a1d6..3219756 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -1061,3 +1061,79 @@ void log_received_signal(int level, const struct 
signalfd_siginfo *si) {
 void log_set_upgrade_syslog_to_journal(bool b) {
 upgrade_syslog_to_journal = b;
 }
+
+typedef enum ServiceResult {
+SERVICE_SUCCESS,
+SERVICE_FAILURE_RESOURCES,
+SERVICE_FAILURE_TIMEOUT,
+SERVICE_FAILURE_EXIT_CODE,
+SERVICE_FAILURE_SIGNAL,
+SERVICE_FAILURE_CORE_DUMP,
+SERVICE_FAILURE_WATCHDOG,
+SERVICE_FAILURE_START_LIMIT,
+_SERVICE_RESULT_MAX,
+_SERVICE_RESULT_INVALID = -1
+} ServiceResult;
+
+static const char* const service_result_table[_SERVICE_RESULT_MAX] = {
+[SERVICE_SUCCESS] = success,
+[SERVICE_FAILURE_RESOURCES] = resources,
+[SERVICE_FAILURE_TIMEOUT] = timeout,
+[SERVICE_FAILURE_EXIT_CODE] = exit-code,
+[SERVICE_FAILURE_SIGNAL] = signal,
+[SERVICE_FAILURE_CORE_DUMP] = core-dump,
+[SERVICE_FAILURE_WATCHDOG] = watchdog,
+[SERVICE_FAILURE_START_LIMIT] = start-limit
+};
+
+DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(service_result, ServiceResult);
+
+static const char* const 
service_result_error_message_table[_SERVICE_RESULT_MAX] = {
+[SERVICE_FAILURE_RESOURCES] = configured resource limit was exceeded,
+[SERVICE_FAILURE_TIMEOUT] = start timeout was exceeded,
+[SERVICE_FAILURE_EXIT_CODE] = ExecStart process exited with error 
code,
+[SERVICE_FAILURE_SIGNAL] = fatal signal was delivered to ExecStart 
process,
+[SERVICE_FAILURE_CORE_DUMP] = fatal signal was delivered to ExecStart 
process. Core dumped,
+[SERVICE_FAILURE_WATCHDOG] = service failed to sent watchdog ping,
+

[systemd-devel] [PATCH] bus-util: be more verbose if bus job failed because of start limit

[Michal Sekletar]

Users might have hard time figuring out that they need to call systemctl
reset-failed, before they are allowed to start the service again, after service
ended up in failed state because start job rate limiting. Let's be nice and
print better error message.

https://bugzilla.redhat.com/show_bug.cgi?id=1016680
---
 src/libsystemd/sd-bus/bus-util.c | 49 +---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
index dcad701..27217d9 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/sd-bus/bus-util.c
@@ -30,6 +30,7 @@
 #include path-util.h
 #include missing.h
 #include set.h
+#include unit-name.h
 
 #include sd-bus.h
 #include bus-error.h
@@ -1721,6 +1722,35 @@ static int bus_process_wait(sd_bus *bus) {
 }
 }
 
+static int bus_job_get_service_result(BusWaitForJobs *d, char **result) {
+int r = 0;
+_cleanup_free_ char *dbus_path = NULL, *p = NULL;
+
+assert(d);
+assert(d-result);
+assert(d-name);
+assert(result);
+
+dbus_path = unit_dbus_path_from_name(d-name);
+if (!dbus_path)
+return -ENOMEM;
+
+r = sd_bus_get_property_string(d-bus,
+   org.freedesktop.systemd1,
+   dbus_path,
+   org.freedesktop.systemd1.Service,
+   Result,
+   NULL,
+   p);
+if (r  0)
+return r;
+
+*result = p;
+p = NULL;
+
+return 0;
+}
+
 static int check_wait_response(BusWaitForJobs *d, bool quiet) {
 int r = 0;
 
@@ -1741,13 +1771,26 @@ static int check_wait_response(BusWaitForJobs *d, bool 
quiet) {
 log_error(Operation on or unit type of %s not 
supported on this system., strna(d-name));
 else if (!streq(d-result, done)  !streq(d-result, 
skipped)) {
 if (d-name) {
+int q;
 bool quotes;
+_cleanup_free_ char *result = NULL;
 
 quotes = chars_intersect(d-name, 
SHELL_NEED_QUOTES);
 
-log_error(Job for %s failed. See \systemctl 
status %s%s%s\ and \journalctl -xe\ for details.,
-  d-name,
-  quotes ? ' : , d-name, quotes ? 
' : );
+q = bus_job_get_service_result(d, result);
+if (q  0)
+log_debug_errno(q, Failed to get 
Result property of service %s: %m, d-name);
+
+if (streq_ptr(result, start-limit))
+log_error(Starting %s has been 
attempted too often too quickly, the repeated start of the unit has been 
refused.\n
+  To force a start please 
invoke \systemctl reset-failed %s%s%s\ followed by \systemctl start %s%s%s\ 
again.,
+  d-name,
+  quotes ? ' : , d-name, 
quotes ? ' : ,
+  quotes ? ' : , d-name, 
quotes ? ' : );
+else
+log_error(Job for %s failed. See 
\systemctl status %s%s%s\ and \journalctl -xe\ for details.,
+  d-name,
+  quotes ? ' : , d-name, 
quotes ? ' : );
 } else
 log_error(Job failed. See \journalctl -xe\ 
for details.);
 }
-- 
2.3.4

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

Re: [systemd-devel] is there a plan for NIC teaming support ?

[Michal Sekletar]

On Thu, Mar 05, 2015 at 05:41:55AM +0100, Branko wrote:
 I have a need to put my NICs in RAID0 so to speak, but according
 to materials I have found on net I can't use NIC bonding driver
 because I would need LACP (IEEE whatever) aware L2 switch, so I was
 refered to teaming driver, which should be ( if those folks is to be
 believed) replacing bond driver in future.
 
 Since systemd doesn't support teaming ( teaming module + libeteam),
 I had to scotchstape it together under systemd and it was major
 PITA.
 
 libteam doesn't seem to complicated to be included as a module in
 systemd, which should take away most of the headaches...

I started looking into providing teaming support in networkd. What I am
currently able to do (not much btw) is to create team netdev and
enslave interfaces. Everything else is done via GeNetlink and sd-rtnl doesn't
support that. Also even the most basic balance-rr mode doesn't work without
teamd instance running. Thus naturally I wanted to use libteam, problem is that
still quite a lot of logic is implemented in teamd daemon and not in
library.

I envision this to work in a following way. networkd will create netdev and
enslave interfaces, then it will call systemd dbus API to create
instance of teamd for the team device. networkd will then communicate with teamd
instance over dbus. Problem I am facing here is not very well designed teamd
dbus interface, i.e. you have to pass json config string as parameter. I am not
sure we want to get into business of stamping out some json to variable
and then passing that string to teamd. It would be much nicer to have v2 of
teamd dbus interface. I had a brief discussion about it with teamd maintainer
and as always patches are welcome.

Michal

 
 
 
 ___
 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] persisting sriov_numvfs

[Michal Sekletar]

On Sat, Feb 14, 2015 at 12:47:54AM +0100, Tom Gundersen wrote:
 On Tue, Jan 27, 2015 at 5:49 PM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Tue, 27.01.15 08:41, Martin Polednik (mpoled...@redhat.com) wrote:
 
   b) Expose this via udev .link files. This would be appropriate if
  adding/removing VFs is a one-time thing, when a device pops
  up. This would be networking specific, not cover anything else like
  GPU or storage or so. Would still be quite nice. Would probably the
  best option, after a), if VFs cannot be added/removed dynamically
  all the time without affecting the other VFs.
  
   c) Expose this via udev rules files. This would be generic, would work
  for networking as well as GPUs or storage. This would entail
  writing our rules files when you want to configure the number of
  VFs. Care needs to be taken to use the right way to identify
  devices as they come and go, so that you can apply configuration to
  them in a stable way. This is somewhat uglier, as we don't really
  think that udev rules should be used that much for configuration,
  especially not for configuration written out by programs, rather
  than manually. However, logind already does this, to assign seat
  identifiers to udev devices to enable multi-seat support.
  
   A combination of b) for networking and c) for the rest might be an
   option too.
 
  I myself would vote for b) + c) since we want to cover most of the
  possible use cases for SR-IOV and MR-IOV, which hopefully shares
  the interface; adding Dan back to CC as he is the one to speak for network.
 
  I have added b) to our TODO list for networkd/udev .link files.
 
 I discussed this with Michal Sekletar who has been looking at this. It
 appears that the sysfs attribute can only be set after the underlying
 netdev is IFF_UP. Is that expected? If so, I don't think it is
 appropriate for udev to deal with this. If anything it should be
 networkd (who is responsible for bringing the links up), but I must
 say I don't think this kernel API makes much sense, so hopefully we
 can come up with something better...

I tried this only with hardware using bnx2x driver but I don't assume that other
hardware will behave any different. Anyway, so far it *seems* like udev is not
the right place to implement this.

Michal

 
  c) should probably be done outside of systemd/udev. Just write a tool
  (or even documenting this might suffice), that creates udev rules in
  /etc/udev/rules.d, matches against ID_PATH and then sets the right
  attribute.
 
  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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/3] systemd: Bind rpc.idmapd to the nfs-server service

[Michal Sekletar]

On Tue, Jan 13, 2015 at 03:37:35PM -0500, Steve Dickson wrote:
 Since rpc.idmapd is only used by the nfs server, to do 
 its id mapping, bind the nfs-idmapd service to the 
 nfs-server service so rpc.idmapd will be started 
 and stopped with the nfs server.
 
 Signed-off-by: Steve Dickson ste...@redhat.com
 ---
  systemd/nfs-idmapd.service | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/systemd/nfs-idmapd.service b/systemd/nfs-idmapd.service
 index 11895e2..61c9a64 100644
 --- a/systemd/nfs-idmapd.service
 +++ b/systemd/nfs-idmapd.service
 @@ -1,7 +1,7 @@
  [Unit]
  Description=NFSv4 ID-name mapping service
  
 -PartOf=nfs-utils.service
 +BindTo=nfs-server.service

Please note that actual name of the dependency is BindsTo.

Michal

  
  Wants=nfs-config.service
  After=nfs-config.service
 -- 
 2.1.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] [WIP PATCH] Do not realize and migrate cgroups multiple times

[Michal Sekletar]

On Mon, Dec 01, 2014 at 12:06:03PM +0100, Martin Pitt wrote:
 Hello all,
 
 In my efforts to make user LXC containers work I noticed that under a
 real desktop (not just nspawn with VT login or ssh logins) my
 carefully set up cgroups in the non-systemd controllers get reverted.
 I. e. I put the session leader (and all other pids) of logind sessions
 (/user.slice/user-1000.slice/session-XX.scope) into all cgroup
 controllers, but a second after they are all back in the / cgroups (or
 sometimes in /user.slice). After some days of debugging (during which
 I learned a lot about the innards of systemd :-) I finally found out
 why:
 
 During unit cgroup initialization (unit_realize_cgroup), sibling
 cgroups are queued instead of initialized immediately.
 unit_create_cgroups() makes an attempt to avoid initializing and
 migrating a cgroup more than once:
 
path = unit_default_cgroup_path(u);
[...]
r = hashmap_put(u-manager-cgroup_unit, path, u);
 if (r  0) {
 log_error(r == -EEXIST ? cgroup %s exists already: %s : 
 hashmap_put failed for %s: %s, path, strerror(-r));
 return r;
 }
 
 But this doesn't work: path always gets allocated freshly in that
 function, so the pointer is never in the hashmap and the -EEXISTS
 never actually hits. This causes -.slice to be initialized and
 recursively migrated a gazillion times, which explains the random
 punting of sub-cgroup PIDs back to /.
 
 I fixed this with an explicit hashmap_get() call, which compares
 values instead of pointers.
 
 I also added some debugging to that function:
 
   log_debug(unit_create_cgroups %s: path=%s realized %i hashmap %p, u-id, 
 path, u-cgroup_realized, hashmap_get(u-manager-cgroup_unit, path));
 
 (right before the hashmap_put() above), which illustrates the
 problem:
 
 systemd[1]: Starting Root Slice.
 systemd[1]: unit_create_cgroups -.slice: path= realized 0 hashmap (nil)
 systemd[1]: Created slice Root Slice.
 [...]
 pid1 systemd[1]: unit_create_cgroups session-c1.scope: 
 path=/user.slice/user-1000.slice/session-c1.scope realized 0 hashmap (nil)
 systemd[1]: Started Session c1 of user martin.
 [... later on when most things have been started ...]
 systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 
 0x7f0e14aa4850
 systemd[1]: unit_create_cgroups -.slice: cgroup  exists already
 systemd[1]: Failed to realize cgroups for queued unit user.slice: File exists
 systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 
 0x7f0e14aa4850
 systemd[1]: unit_create_cgroups -.slice: cgroup  exists already
 systemd[1]: Failed to realize cgroups for queued unit grub-common.slice: File 
 exists
 systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 
 0x7f0e14aa4850
 systemd[1]: unit_create_cgroups -.slice: cgroup  exists already
 systemd[1]: Failed to realize cgroups for queued unit networking.slice: File 
 exists
 
 ... and so on, basically once for each .service.
 
 Initializing -.slice so many times is certainly an unintended effect
 of the peer cgroup creation. Thus the patch fixes the multiple
 initialization/creation, but a proper fix should also quiesce these
 error messages. The condition could be checked explicitly, i. e. we
 skip the Failed to realize... error for EEXISTS, or we generally
 tone this down to log_debug. I'm open to suggestions. And of course
 the log_debug should be removed; it's nice to illustrate the problem,
 but doesn't really belong into production code.
 
 Thanks,
 
 Martin

Also this looks like a possible fix to the problem I tried to describe in,

http://lists.freedesktop.org/archives/systemd-devel/2014-November/025607.html

Michal

 
 -- 
 Martin Pitt| http://www.piware.de
 Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

 From fd2f8a444c9c644a39dd3b619934e8768e03c2a3 Mon Sep 17 00:00:00 2001
 From: Martin Pitt martin.p...@ubuntu.com
 Date: Mon, 1 Dec 2014 10:50:06 +0100
 Subject: [PATCH] Do not realize and migrate cgroups multiple times
 
 unit_create_cgroups() tries to check if a cgroup already exists. But has the
 destination path is always allocated dynamically as a new string, that pointer
 will never already be in the hashmap, thus hashmap_put() will never actually
 fail with EEXISTS. Thus check for the existance of the cgroup path explicitly.
 
 Before this, -.slice got initialized and its child PIDs migrated many times
 through queuing the realization of sibling units; thiss caused any cgroup
 controller layout from sub-cgroups to be reverted and their pids moved back to
 the root cgroup in all controllers (except systemd).
 ---
  src/core/cgroup.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/src/core/cgroup.c b/src/core/cgroup.c
 index 8820bee..3d36080 100644
 --- a/src/core/cgroup.c
 +++ b/src/core/cgroup.c
 @@ -614,6 +614,13 @@ static int unit_create_cgroups(Unit *u, 
 CGroupControllerMask mask) {
  if (!path)
 

[systemd-devel] [PATCH] selinux: figure out selinux context applied on exec() before closing all fds

[Michal Sekletar]

We need original socket_fd around otherwise mac_selinux_get_child_mls_label
fails with -EINVAL return code. Also don't call setexeccon twice but rather pass
context value of SELinuxContext option as an extra argument.
---
 src/core/execute.c| 31 ++-
 src/shared/selinux-util.c |  8 ++--
 src/shared/selinux-util.h |  2 +-
 3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/src/core/execute.c b/src/core/execute.c
index 5cfd4a1..e8ee0e7 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1236,6 +1236,7 @@ static int exec_child(ExecCommand *command,
   int *error) {
 
 _cleanup_strv_free_ char **our_env = NULL, **pam_env = NULL, 
**final_env = NULL, **final_argv = NULL;
+_cleanup_free_ char *mac_selinux_context_net = NULL;
 const char *username = NULL, *home = NULL, *shell = NULL;
 unsigned n_dont_close = 0;
 int dont_close[n_fds + 4];
@@ -1582,6 +1583,16 @@ static int exec_child(ExecCommand *command,
 }
 }
 
+#ifdef HAVE_SELINUX
+if (params-apply_permissions  mac_selinux_use()  
params-selinux_context_net  socket_fd = 0) {
+err = mac_selinux_get_child_mls_label(socket_fd, 
command-path, context-selinux_context, mac_selinux_context_net);
+if (err  0) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
+}
+#endif
+
 /* We repeat the fd closing here, to make sure that
  * nothing is leaked from the PAM modules. Note that
  * we are more aggressive this time since socket_fd
@@ -1671,24 +1682,10 @@ static int exec_child(ExecCommand *command,
 
 #ifdef HAVE_SELINUX
 if (mac_selinux_use()) {
-if (context-selinux_context) {
-err = setexeccon(context-selinux_context);
-if (err  0  
!context-selinux_context_ignore) {
-*error = EXIT_SELINUX_CONTEXT;
-return err;
-}
-}
-
-if (params-selinux_context_net  socket_fd = 0) {
-_cleanup_free_ char *label = NULL;
-
-err = 
mac_selinux_get_child_mls_label(socket_fd, command-path, label);
-if (err  0) {
-*error = EXIT_SELINUX_CONTEXT;
-return err;
-}
+char *exec_context = mac_selinux_context_net ?: 
context-selinux_context;
 
-err = setexeccon(label);
+if (exec_context) {
+err = setexeccon(exec_context);
 if (err  0) {
 *error = EXIT_SELINUX_CONTEXT;
 return err;
diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c
index 6bd3bf1..a2233e0 100644
--- a/src/shared/selinux-util.c
+++ b/src/shared/selinux-util.c
@@ -233,7 +233,7 @@ int mac_selinux_get_our_label(char **label) {
 return r;
 }
 
-int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, char 
**label) {
+int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char 
*exec_label, char **label) {
 int r = -EOPNOTSUPP;
 
 #ifdef HAVE_SELINUX
@@ -257,11 +257,7 @@ int mac_selinux_get_child_mls_label(int socket_fd, const 
char *exe, char **label
 if (r  0)
 return -errno;
 
-r = getexeccon(fcon);
-if (r  0)
-return -errno;
-
-if (!fcon) {
+if (!exec_label) {
 /* If there is no context set for next exec let's use context
of target executable */
 r = getfilecon(exe, fcon);
diff --git a/src/shared/selinux-util.h b/src/shared/selinux-util.h
index 7ff8c60..a694441 100644
--- a/src/shared/selinux-util.h
+++ b/src/shared/selinux-util.h
@@ -36,7 +36,7 @@ int mac_selinux_apply(const char *path, const char *label);
 
 int mac_selinux_get_create_label_from_exe(const char *exe, char **label);
 int mac_selinux_get_our_label(char **label);
-int mac_selinux_get_child_mls_label(int socket_fd, const char *exec, char 
**label);
+int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char 
*exec_label, char **label);
 void mac_selinux_free(char *label);
 
 int mac_selinux_create_file_prepare(const char *path, mode_t mode);
-- 
1.8.3.1

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

[systemd-devel] [PATCH] units: skip mounting /dev/hugepages if we don't have CAP_SYS_ADMIN

[Michal Sekletar]

---
 units/dev-hugepages.mount | 1 +
 1 file changed, 1 insertion(+)

diff --git a/units/dev-hugepages.mount b/units/dev-hugepages.mount
index d711fae..882adb4 100644
--- a/units/dev-hugepages.mount
+++ b/units/dev-hugepages.mount
@@ -12,6 +12,7 @@ 
Documentation=http://www.freedesktop.org/wiki/Software/systemd/APIFileSystems
 DefaultDependencies=no
 Before=sysinit.target
 ConditionPathExists=/sys/kernel/mm/hugepages
+ConditionCapability=CAP_SYS_ADMIN
 
 [Mount]
 What=hugetlbfs
-- 
1.8.3.1

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

Re: [systemd-devel] [RFC] [PATCH] cgroup: don't trim cgroup trees created by someone else

[Michal Sekletar]

On Tue, Oct 21, 2014 at 09:16:16PM +0200, Lennart Poettering wrote:
 On Fri, 19.09.14 17:14, Michal Sekletar (msekl...@redhat.com) wrote:
 
snip 
 I do see the usecase though for those projects. I'd probably suggest
 not to merge it for RHEL either. But instead I'd propose a different
 solution for software: make sure sure to always move a PID into a
 cgroup as soon as it is created, so that its removal is blocked. Or in
 other words: right before you need a cgroup to add yourself to it,
 create it, and expect that it might go away while you are not using
 it. To deal with the possible race condition of creating a cgroup
 which is immediately cleaned up by somebody else, try doing this in a
 loop, until you succeeded. 

I think I grok what are you proposing, however according to developments in 

https://bugzilla.redhat.com/show_bug.cgi?id=1139223

it doesn't seem to be correct solution either. systemd will happily remove 
cgroup
in which there are processes.

 
 That way things can always stay nicely cleaned up and things are
 robust towards other processing playing around in the cgroup tree.

I am not sure how can we make that possible (playing nicely in cgroup tree) for
the time beeing until we are the only writer.

 
 Hope that makes sense?
 
 Lennart

Michal

 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] util: introduce sethostname_idempotent

[Michal Sekletar]

On Tue, Oct 21, 2014 at 07:29:31PM +0200, Lennart Poettering wrote:
 On Tue, 21.10.14 18:32, Michal Sekletar (msekl...@redhat.com) wrote:
snip
 Go ahead and commit. Ideally with those two nitpicks fixed, but even
 if you don't it's OK.

sethostname_idempotent now returns 1 when hostname was changed. Also, I've
pushed follow-up fix-up for copy paste error, so we actually call sethostname()
on s not on buf which contains old hostname.

Michal

 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 3/4] shutdown: don't do final unmounting when inside the container and running without CAP_SYS_ADMIN

[Michal Sekletar]

On Wed, Oct 08, 2014 at 04:54:59PM +0200, Lennart Poettering wrote:
 On Wed, 08.10.14 16:49, Michal Sekletar (msekl...@redhat.com) wrote:
 
 Hmm, I think we should just do need_umount = !in_container, like we
 do for the other things like loopback detaching, dm detaching or
 swapoff. After all, if we run in a container we run in a mount
 namespace anyway, so unmounting things is done by the kernel
 implicitly if the namespace dies. At least in theory this means we can
 simply skip the unmounting in all containers, but I must admit that I
 am not entirely clear on this one, so this needs to be tested in the
 common container managers really, I figure...

Do you mind if I push just need_umount = !in_container then?
   
   Well, yes.
   
   I'd be thankful if you'd test this a bit first, so that this doesn't
   break anything. Testing nspawn and on bare-metal should be enough.
  
  Works just fine on F21 KVM guest and in rawhide nspawn container.
 
 THen please, go ahead, commit with a good commit msg explaining things,
 maybe even referencing this discussion.

Pushed with better explanation. Hope that commit message makes sense.

Michal

 
 Thanks,
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] util: introduce sethostname_idempotent

[Michal Sekletar]

Function queries system hostname and applies changes only when necessary. Also,
migrate all client of sethostname to sethostname_idempotent while at it.
---
 src/core/hostname-setup.c |  2 +-
 src/hostname/hostnamed.c  |  2 +-
 src/nspawn/nspawn.c   |  2 +-
 src/shared/util.c | 20 
 src/shared/util.h |  2 ++
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/core/hostname-setup.c b/src/core/hostname-setup.c
index 8aa1cff..57baa79 100644
--- a/src/core/hostname-setup.c
+++ b/src/core/hostname-setup.c
@@ -82,7 +82,7 @@ int hostname_setup(void) {
 hn = localhost;
 }
 
-if (sethostname(hn, strlen(hn))  0) {
+if (sethostname_idempotent(hn)  0) {
 log_warning(Failed to set hostname to %s: %m, hn);
 return -errno;
 }
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index b6b5d52..3e5d434 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -287,7 +287,7 @@ static int context_update_kernel_hostname(Context *c) {
 else
 hn = localhost;
 
-if (sethostname(hn, strlen(hn))  0)
+if (sethostname_idempotent(hn)  0)
 return -errno;
 
 return 0;
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index c567c8d..b6d9bc6 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1289,7 +1289,7 @@ static int setup_hostname(void) {
 if (arg_share_system)
 return 0;
 
-if (sethostname(arg_machine, strlen(arg_machine))  0)
+if (sethostname_idempotent(arg_machine)  0)
 return -errno;
 
 return 0;
diff --git a/src/shared/util.c b/src/shared/util.c
index 5f6249e..cb3bd2f 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7164,3 +7164,23 @@ int free_and_strdup(char **p, const char *s) {
 
 return 0;
 }
+
+int sethostname_idempotent(const char *s) {
+int r = 0;
+char buf[HOST_NAME_MAX + 1] = {};
+
+assert(s);
+
+r = gethostname(buf, sizeof(buf));
+if (r  0)
+return -errno;
+
+if (streq(buf, s))
+return 0;
+
+r = sethostname(buf, strlen(buf));
+if (r  0)
+return -errno;
+
+return 0;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 21a90a4..10cdc7b 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -996,3 +996,5 @@ int unquote_first_word(const char **p, char **ret);
 int unquote_many_words(const char **p, ...) _sentinel_;
 
 int free_and_strdup(char **p, const char *s);
+
+int sethostname_idempotent(const char *s);
-- 
1.8.3.1

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

Re: [systemd-devel] Unicode support in console after boot

[Michal Sekletar]

On Tue, Oct 14, 2014 at 09:04:56AM +0200, Jan Synacek wrote:
 Michal Sekletar msekl...@redhat.com writes:
  On Mon, Oct 13, 2014 at 09:36:16AM +0200, Jan Synacek wrote:
  Hello,
  
  currently, unicode characters are not correctly displayed in the
  console. After login, when I run /usr/bin/unicode_start, unicode works
  fine. I tried to create a service file that runs this script, linking
  tty to stdout and stderr, but that didn't work. Is there a way how to
  turn on unicode support in console after boot using a service file? Or
  any other type of unit? Or is this something that has to be patched in
  the source (logind perhaps?)?
 
  Please try editing /usr/lib/systemd/system/systemd-vconsole-setup.service 
  and
  remove RemainAfterExit=yes, then regenerate your initramfs image by running
  dracut command. Add back RemainAfterExit=yes to service file. Reboot.
 
 Yep, this helped. Could you please explain why? Also, I believe this
 should be fixed in all Fedora versions.

I must admit I'm not sure why this workaround works. Maybe there is some race
condition with some kernel initialization or settings get unapplied because of
switch-root.

Also, if we go with workaround in Fedora I think dracut needs to fixed to
include version on unit file *without* RemainAfterExit=yes.

Michal

 
  Michal
 
  
  Cheers,
  -- 
  Jan Synacek
  Software Engineer, Red Hat
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 
 -- 
 Jan Synacek
 Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Unicode support in console after boot

[Michal Sekletar]

On Tue, Oct 21, 2014 at 09:39:46PM +0400, Ivan Shapovalov wrote:
 On Tuesday 21 October 2014 at 19:03:17, Michal Sekletar wrote:
  On Tue, Oct 14, 2014 at 09:04:56AM +0200, Jan Synacek wrote:
   Michal Sekletar msekl...@redhat.com writes:
On Mon, Oct 13, 2014 at 09:36:16AM +0200, Jan Synacek wrote:
Hello,

currently, unicode characters are not correctly displayed in the
console. After login, when I run /usr/bin/unicode_start, unicode works
fine. I tried to create a service file that runs this script, linking
tty to stdout and stderr, but that didn't work. Is there a way how to
turn on unicode support in console after boot using a service file? Or
any other type of unit? Or is this something that has to be patched in
the source (logind perhaps?)?
   
Please try editing 
/usr/lib/systemd/system/systemd-vconsole-setup.service and
remove RemainAfterExit=yes, then regenerate your initramfs image by 
running
dracut command. Add back RemainAfterExit=yes to service file. Reboot.
   
   Yep, this helped. Could you please explain why? Also, I believe this
   should be fixed in all Fedora versions.
  
  I must admit I'm not sure why this workaround works. Maybe there is some 
  race
  condition with some kernel initialization or settings get unapplied because 
  of
  switch-root.
  
  Also, if we go with workaround in Fedora I think dracut needs to fixed to
  include version on unit file *without* RemainAfterExit=yes.
 
 IIUC, this makes unit to be re-run outside of initramfs, so the VT is set up 
 twice,
 second time after switching the framebuffer driver.
 
 And the latter condition is not mandated by anything, it's just a
 coincidence...

I guess that what you are saying pretty much summarizes the situation here. Can
we make something about framebuffer driver, thus settings applied in initramfs
are not thrashed?

Michal

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

Re: [systemd-devel] [PATCH 1/2] selinux: fix potential double free crash in child process

[Michal Sekletar]

On Mon, Oct 13, 2014 at 05:14:24PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
 On Mon, Oct 13, 2014 at 04:57:12PM +0200, Michal Sekletar wrote:
  Before returning from function we should reset ret to NULL, thus cleanup
  function is nop.
  
  Also context_str() returns pointer to a string containing context but not a
  copy, hence we must make copy it explicitly.
  ---
   src/shared/label.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
  
  diff --git a/src/shared/label.c b/src/shared/label.c
  index b6af38d..89fb49e 100644
  --- a/src/shared/label.c
  +++ b/src/shared/label.c
  @@ -334,7 +334,8 @@ int label_get_child_mls_label(int socket_fd, const char 
  *exe, char **label) {
   }
   
   freecon(mycon);
  -mycon = context_str(bcon);
  +mycon = NULL;
 This line doesn't make sense.

Will remove it.

 
  +mycon = strdup(context_str(bcon));
   if (!mycon) {
   r = -errno;
   goto out;
  @@ -348,6 +349,7 @@ int label_get_child_mls_label(int socket_fd, const char 
  *exe, char **label) {
   }
   
   *label = ret;
  +ret = NULL;
   r = 0;
 Otherwise looks good.

I will amend and push then.

Thanks,

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

Re: [systemd-devel] Unicode support in console after boot

[Michal Sekletar]

On Mon, Oct 13, 2014 at 09:36:16AM +0200, Jan Synacek wrote:
 Hello,
 
 currently, unicode characters are not correctly displayed in the
 console. After login, when I run /usr/bin/unicode_start, unicode works
 fine. I tried to create a service file that runs this script, linking
 tty to stdout and stderr, but that didn't work. Is there a way how to
 turn on unicode support in console after boot using a service file? Or
 any other type of unit? Or is this something that has to be patched in
 the source (logind perhaps?)?

Please try editing /usr/lib/systemd/system/systemd-vconsole-setup.service and
remove RemainAfterExit=yes, then regenerate your initramfs image by running
dracut command. Add back RemainAfterExit=yes to service file. Reboot.

Michal

 
 Cheers,
 -- 
 Jan Synacek
 Software Engineer, 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

[systemd-devel] [PATCH 1/2] selinux: fix potential double free crash in child process

[Michal Sekletar]

Before returning from function we should reset ret to NULL, thus cleanup
function is nop.

Also context_str() returns pointer to a string containing context but not a
copy, hence we must make copy it explicitly.
---
 src/shared/label.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/shared/label.c b/src/shared/label.c
index b6af38d..89fb49e 100644
--- a/src/shared/label.c
+++ b/src/shared/label.c
@@ -334,7 +334,8 @@ int label_get_child_mls_label(int socket_fd, const char 
*exe, char **label) {
 }
 
 freecon(mycon);
-mycon = context_str(bcon);
+mycon = NULL;
+mycon = strdup(context_str(bcon));
 if (!mycon) {
 r = -errno;
 goto out;
@@ -348,6 +349,7 @@ int label_get_child_mls_label(int socket_fd, const char 
*exe, char **label) {
 }
 
 *label = ret;
+ret = NULL;
 r = 0;
 
 out:
-- 
1.8.3.1

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

[systemd-devel] [PATCH 2/2] selinux: set selinux context applied on exec() before closing all fds

[Michal Sekletar]

We need original socket_fd around otherwise label_get_child_mls_label fails with
-EINVAL return code.
---
 src/core/execute.c | 58 --
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/core/execute.c b/src/core/execute.c
index b165b33..d64fa7c 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1578,6 +1578,36 @@ static int exec_child(ExecCommand *command,
 }
 }
 
+#ifdef HAVE_SELINUX
+if (params-apply_permissions) {
+if (use_selinux()) {
+if (context-selinux_context) {
+err = setexeccon(context-selinux_context);
+if (err  0  
!context-selinux_context_ignore) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
+}
+
+if (params-selinux_context_net  socket_fd = 0) {
+_cleanup_free_ char *label = NULL;
+
+err = label_get_child_mls_label(socket_fd, 
command-path, label);
+if (err  0) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
+
+err = setexeccon(label);
+if (err  0) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
+}
+}
+}
+#endif
+
 /* We repeat the fd closing here, to make sure that
  * nothing is leaked from the PAM modules. Note that
  * we are more aggressive this time since socket_fd
@@ -1665,34 +1695,6 @@ static int exec_child(ExecCommand *command,
 }
 #endif
 
-#ifdef HAVE_SELINUX
-if (use_selinux()) {
-if (context-selinux_context) {
-err = setexeccon(context-selinux_context);
-if (err  0  
!context-selinux_context_ignore) {
-*error = EXIT_SELINUX_CONTEXT;
-return err;
-}
-}
-
-if (params-selinux_context_net  socket_fd = 0) {
-_cleanup_free_ char *label = NULL;
-
-err = label_get_child_mls_label(socket_fd, 
command-path, label);
-if (err  0) {
-*error = EXIT_SELINUX_CONTEXT;
-return err;
-}
-
-err = setexeccon(label);
-if (err  0) {
-*error = EXIT_SELINUX_CONTEXT;
-return err;
-}
-}
-}
-#endif
-
 #ifdef HAVE_APPARMOR
 if (context-apparmor_profile  use_apparmor()) {
 err = aa_change_onexec(context-apparmor_profile);
-- 
1.8.3.1

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

Re: [systemd-devel] [PATCH 3/4] shutdown: don't do final unmounting when inside the container and running without CAP_SYS_ADMIN

[Michal Sekletar]

On Wed, Oct 08, 2014 at 01:41:16PM +0200, Lennart Poettering wrote:
 On Tue, 07.10.14 14:17, Michal Sekletar (msekl...@redhat.com) wrote:
 
  On Thu, Oct 02, 2014 at 12:04:02PM +0200, Lennart Poettering wrote:
   On Thu, 02.10.14 09:57, Michal Sekletar (msekl...@redhat.com) wrote:
   
 #define FINALIZE_ATTEMPTS 50
 
@@ -207,7 +208,11 @@ int main(int argc, char *argv[]) {
 
 in_container = detect_container(NULL)  0;
 
-need_umount = true;
+if (in_container  !have_effective_cap(CAP_SYS_ADMIN))
+need_umount = false;
+else
+need_umount = true;
+
 need_swapoff = !in_container;
 need_loop_detach = !in_container;
 need_dm_detach = !in_container;
   
   Hmm, I think we should just do need_umount = !in_container, like we
   do for the other things like loopback detaching, dm detaching or
   swapoff. After all, if we run in a container we run in a mount
   namespace anyway, so unmounting things is done by the kernel
   implicitly if the namespace dies. At least in theory this means we can
   simply skip the unmounting in all containers, but I must admit that I
   am not entirely clear on this one, so this needs to be tested in the
   common container managers really, I figure...
  
  Do you mind if I push just need_umount = !in_container then?
 
 Well, yes.
 
 I'd be thankful if you'd test this a bit first, so that this doesn't
 break anything. Testing nspawn and on bare-metal should be enough.

Works just fine on F21 KVM guest and in rawhide nspawn container.

Michal
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()

[Michal Sekletar]

On Thu, Oct 02, 2014 at 11:43:22AM +0200, Lennart Poettering wrote:
 On Thu, 02.10.14 09:57, Michal Sekletar (msekl...@redhat.com) wrote:
 
  In cases when we are running as system manager, but we don't have the
  capability to mount filesystems don't call mount_setup(). However we
  assume that some directories (e.g. /run/systemd) are always
  around. Hence don't create those directories in mount_setup().
  ---
   src/core/main.c|  7 ++-
   src/core/mount-setup.c | 20 
   src/core/mount-setup.h |  1 +
   3 files changed, 19 insertions(+), 9 deletions(-)
  
  diff --git a/src/core/main.c b/src/core/main.c
  index 1a62e04..fcd9471 100644
  --- a/src/core/main.c
  +++ b/src/core/main.c
  @@ -1393,10 +1393,15 @@ int main(int argc, char *argv[]) {
   
   /* Mount /proc, /sys and friends, so that /proc/cmdline and
* /proc/$PID/fd is available. */
  -if (getpid() == 1) {
  +if (getpid() == 1  have_effective_cap(CAP_SYS_ADMIN)) {
   r = mount_setup(loaded_policy);
   if (r  0)
   goto finish;
 
 Hmm, is this really necessary? I mean, the code in mount_setup() will
 anyway only mount what is missing, but not overmount what is already
 mounted.

I think it is necessary to make possible to run systemd in a docker container.

 Hence, if a container manager mounts everything properly, then mount_setup()
 should be a NOP anyway... 

In theory yes, but in fact not having /run mounted as tmpfs is default in the 
docker
container. I have no strong opinion on whether this is sensible or not, however
I think that systemd can be made more resilient and handle such cases. 

Now systemd will try to mount /run on tmpfs, such attempt will fail because of
missing capability and then systemd will just hang.


Michal
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 3/4] shutdown: don't do final unmounting when inside the container and running without CAP_SYS_ADMIN

[Michal Sekletar]

On Thu, Oct 02, 2014 at 12:04:02PM +0200, Lennart Poettering wrote:
 On Thu, 02.10.14 09:57, Michal Sekletar (msekl...@redhat.com) wrote:
 
   #define FINALIZE_ATTEMPTS 50
   
  @@ -207,7 +208,11 @@ int main(int argc, char *argv[]) {
   
   in_container = detect_container(NULL)  0;
   
  -need_umount = true;
  +if (in_container  !have_effective_cap(CAP_SYS_ADMIN))
  +need_umount = false;
  +else
  +need_umount = true;
  +
   need_swapoff = !in_container;
   need_loop_detach = !in_container;
   need_dm_detach = !in_container;
 
 Hmm, I think we should just do need_umount = !in_container, like we
 do for the other things like loopback detaching, dm detaching or
 swapoff. After all, if we run in a container we run in a mount
 namespace anyway, so unmounting things is done by the kernel
 implicitly if the namespace dies. At least in theory this means we can
 simply skip the unmounting in all containers, but I must admit that I
 am not entirely clear on this one, so this needs to be tested in the
 common container managers really, I figure...

Do you mind if I push just need_umount = !in_container then?

Michal
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] localectl: print warning when there are options given on kernel cmdline

[Michal Sekletar]

On Thu, Oct 02, 2014 at 02:39:10PM +0200, Lennart Poettering wrote:
 On Wed, 24.09.14 17:18, Michal Sekletar (msekl...@redhat.com) wrote:
 
 Heya,
 
  +for (j = VARIABLE_LANG; j  _VARIABLE_LC_MAX; j++)
 
 I think it is much nicer to count from an explicit 0 on here,
 instead of VARIABLE_LANG, since this makes the loop work correctly
 even if the order of the enum is changed.
 
  +if (variables[j]) {
  +if (print_warning) {
  +printf(Warning: Settings on Kernel 
  Command Line override system locale settings in /etc/locale.conf\n);
  +printf(Command Line: %s=%s\n, 
  locale_variable_to_string(j), variables[j]);
 
 Warnings should be printed with log_warning() and not printf() so that they 
 end up on stderr, not stdout.
 
 I fixed this now in git.

Thanks, for clean-ups. Much appreciated.

Michal

 
 Thanks,
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/4] virt: detect that we are running inside the docker container

[Michal Sekletar]

---
 src/shared/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/shared/virt.c b/src/shared/virt.c
index b436895..f9c4e67 100644
--- a/src/shared/virt.c
+++ b/src/shared/virt.c
@@ -310,6 +310,8 @@ int detect_container(const char **id) {
 _id = lxc-libvirt;
 else if (streq(e, systemd-nspawn))
 _id = systemd-nspawn;
+else if (streq(e, docker))
+_id = docker;
 else
 _id = other;
 
-- 
2.0.1

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

[systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()

[Michal Sekletar]

In cases when we are running as system manager, but we don't have the
capability to mount filesystems don't call mount_setup(). However we
assume that some directories (e.g. /run/systemd) are always
around. Hence don't create those directories in mount_setup().
---
 src/core/main.c|  7 ++-
 src/core/mount-setup.c | 20 
 src/core/mount-setup.h |  1 +
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index 1a62e04..fcd9471 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1393,10 +1393,15 @@ int main(int argc, char *argv[]) {
 
 /* Mount /proc, /sys and friends, so that /proc/cmdline and
  * /proc/$PID/fd is available. */
-if (getpid() == 1) {
+if (getpid() == 1  have_effective_cap(CAP_SYS_ADMIN)) {
 r = mount_setup(loaded_policy);
 if (r  0)
 goto finish;
+} else if (getpid() == 1  detect_container(NULL)  0) {
+/* Running inside the container as PID 1 but without capability
+   to mount filesystems. Create at least directories we always
+   expect to be around */
+mount_setup_run_dirs();
 }
 
 /* Reset all signal handlers. */
diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index 23a66d2..cd2991d 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -373,6 +373,17 @@ static int nftw_cb(
 return FTW_CONTINUE;
 };
 
+void mount_setup_run_dirs(void) {
+/* Create a few directories we always want around, Note that
+ * sd_booted() checks for /run/systemd/system, so this mkdir
+ * really needs to stay for good, otherwise software that
+ * copied sd-daemon.c into their sources will misdetect
+ * systemd. */
+mkdir_label(/run/systemd, 0755);
+mkdir_label(/run/systemd/system, 0755);
+mkdir_label(/run/systemd/inaccessible, );
+}
+
 int mount_setup(bool loaded_policy) {
 int r;
 unsigned i;
@@ -418,14 +429,7 @@ int mount_setup(bool loaded_policy) {
 if (mount(NULL, /, NULL, MS_REC|MS_SHARED, NULL)  0)
 log_warning(Failed to set up the root directory for 
shared mount propagation: %m);
 
-/* Create a few directories we always want around, Note that
- * sd_booted() checks for /run/systemd/system, so this mkdir
- * really needs to stay for good, otherwise software that
- * copied sd-daemon.c into their sources will misdetect
- * systemd. */
-mkdir_label(/run/systemd, 0755);
-mkdir_label(/run/systemd/system, 0755);
-mkdir_label(/run/systemd/inaccessible, );
+mount_setup_run_dirs();
 
 return 0;
 }
diff --git a/src/core/mount-setup.h b/src/core/mount-setup.h
index 4b521ad..bfe92b1 100644
--- a/src/core/mount-setup.h
+++ b/src/core/mount-setup.h
@@ -25,6 +25,7 @@
 
 int mount_setup_early(void);
 
+void mount_setup_run_dirs(void);
 int mount_setup(bool loaded_policy);
 
 int mount_cgroup_controllers(char ***join_controllers);
-- 
2.0.1

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

[systemd-devel] [PATCH 3/4] shutdown: don't do final unmounting when inside the container and running without CAP_SYS_ADMIN

[Michal Sekletar]

---
 Makefile.am | 3 ++-
 src/core/shutdown.c | 7 ++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 5033028..f8104bc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1925,7 +1925,8 @@ systemd_shutdown_SOURCES = \
 systemd_shutdown_LDADD = \
libsystemd-label.la \
libudev-internal.la \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-capability.la
 
 # 
--
 if HAVE_KMOD
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 1e88b05..5b539f0 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -49,6 +49,7 @@
 #include cgroup-util.h
 #include def.h
 #include switch-root.h
+#include capability.h
 
 #define FINALIZE_ATTEMPTS 50
 
@@ -207,7 +208,11 @@ int main(int argc, char *argv[]) {
 
 in_container = detect_container(NULL)  0;
 
-need_umount = true;
+if (in_container  !have_effective_cap(CAP_SYS_ADMIN))
+need_umount = false;
+else
+need_umount = true;
+
 need_swapoff = !in_container;
 need_loop_detach = !in_container;
 need_dm_detach = !in_container;
-- 
2.0.1

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

[systemd-devel] [PATCH 4/4] hostname-setup: try to set hostname only when necessary

[Michal Sekletar]

When a system already has hostname set to the configured value
don't try to set it again. This will prevent an error being reported when
running inside an unprivileged container, i.e. in an environment where we
don't have CAP_SYS_ADMIN capability and therefore the container manager is
responsible for setting hostname for us.
---
 src/core/hostname-setup.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/core/hostname-setup.c b/src/core/hostname-setup.c
index 8aa1cff..1df7a56 100644
--- a/src/core/hostname-setup.c
+++ b/src/core/hostname-setup.c
@@ -80,6 +80,15 @@ int hostname_setup(void) {
 log_info(No hostname configured.);
 
 hn = localhost;
+} else {
+char old_hn[HOST_NAME_MAX + 1] = {};
+
+r = gethostname(old_hn, sizeof(old_hn));
+if (!r) {
+if (streq(hn, old_hn))
+return 0;
+} else
+log_warning(Failed to get hostname: %m);
 }
 
 if (sethostname(hn, strlen(hn))  0) {
-- 
2.0.1

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

Re: [systemd-devel] [PATCH 2/2] localectl: print warning when there are options given on kernel cmdline

[Michal Sekletar]

On Thu, Sep 25, 2014 at 04:47:58AM +0200, Zbigniew Jędrzejewski-Szmek wrote:
 On Wed, Sep 24, 2014 at 05:18:47PM +0200, Michal Sekletar wrote:
  ---
   src/core/locale-setup.c  | 47 +
   src/locale/localectl.c   | 50 
  
   src/shared/locale-util.c | 20 +++
   src/shared/locale-util.h | 25 
   4 files changed, 100 insertions(+), 42 deletions(-)
 
 Both patches look good. Please push.

Pushed now.

Michal
 
 Zbyszek
 
  diff --git a/src/core/locale-setup.c b/src/core/locale-setup.c
  index 7a41035..5177dbf 100644
  --- a/src/core/locale-setup.c
  +++ b/src/core/locale-setup.c
  @@ -30,48 +30,11 @@
   #include fileio.h
   #include strv.h
   #include env-util.h
  -
  -enum {
  -/* We don't list LC_ALL here on purpose. People should be
  - * using LANG instead. */
  -
  -VARIABLE_LANG,
  -VARIABLE_LANGUAGE,
  -VARIABLE_LC_CTYPE,
  -VARIABLE_LC_NUMERIC,
  -VARIABLE_LC_TIME,
  -VARIABLE_LC_COLLATE,
  -VARIABLE_LC_MONETARY,
  -VARIABLE_LC_MESSAGES,
  -VARIABLE_LC_PAPER,
  -VARIABLE_LC_NAME,
  -VARIABLE_LC_ADDRESS,
  -VARIABLE_LC_TELEPHONE,
  -VARIABLE_LC_MEASUREMENT,
  -VARIABLE_LC_IDENTIFICATION,
  -_VARIABLE_MAX
  -};
  -
  -static const char * const variable_names[_VARIABLE_MAX] = {
  -[VARIABLE_LANG] = LANG,
  -[VARIABLE_LANGUAGE] = LANGUAGE,
  -[VARIABLE_LC_CTYPE] = LC_CTYPE,
  -[VARIABLE_LC_NUMERIC] = LC_NUMERIC,
  -[VARIABLE_LC_TIME] = LC_TIME,
  -[VARIABLE_LC_COLLATE] = LC_COLLATE,
  -[VARIABLE_LC_MONETARY] = LC_MONETARY,
  -[VARIABLE_LC_MESSAGES] = LC_MESSAGES,
  -[VARIABLE_LC_PAPER] = LC_PAPER,
  -[VARIABLE_LC_NAME] = LC_NAME,
  -[VARIABLE_LC_ADDRESS] = LC_ADDRESS,
  -[VARIABLE_LC_TELEPHONE] = LC_TELEPHONE,
  -[VARIABLE_LC_MEASUREMENT] = LC_MEASUREMENT,
  -[VARIABLE_LC_IDENTIFICATION] = LC_IDENTIFICATION
  -};
  +#include locale-util.h
   
   int locale_setup(char ***environment) {
   char **add;
  -char *variables[_VARIABLE_MAX] = {};
  +char *variables[_VARIABLE_LC_MAX] = {};
   int r = 0, i;
   
   if (detect_container(NULL) = 0) {
  @@ -121,13 +84,13 @@ int locale_setup(char ***environment) {
   }
   
   add = NULL;
  -for (i = 0; i  _VARIABLE_MAX; i++) {
  +for (i = 0; i  _VARIABLE_LC_MAX; i++) {
   char *s;
   
   if (!variables[i])
   continue;
   
  -s = strjoin(variable_names[i], =, variables[i], NULL);
  +s = strjoin(locale_variable_to_string(i), =, 
  variables[i], NULL);
   if (!s) {
   r = -ENOMEM;
   goto finish;
  @@ -157,7 +120,7 @@ int locale_setup(char ***environment) {
   finish:
   strv_free(add);
   
  -for (i = 0; i  _VARIABLE_MAX; i++)
  +for (i = 0; i  _VARIABLE_LC_MAX; i++)
   free(variables[i]);
   
   return r;
  diff --git a/src/locale/localectl.c b/src/locale/localectl.c
  index bf8b7b2..5917364 100644
  --- a/src/locale/localectl.c
  +++ b/src/locale/localectl.c
  @@ -43,6 +43,8 @@
   #include path-util.h
   #include utf8.h
   #include def.h
  +#include virt.h
  +#include fileio.h
   #include locale-util.h
   
   static bool arg_no_pager = false;
  @@ -81,6 +83,53 @@ typedef struct StatusInfo {
   const char *x11_options;
   } StatusInfo;
   
  +static void print_overriden_variables(void) {
  +int r;
  +char *variables[_VARIABLE_LC_MAX] = {};
  +LocaleVariable j;
  +bool print_warning = true;
  +
  +if (detect_container(NULL)  0 || arg_host)
  +return;
  +
  +r = parse_env_file(/proc/cmdline, WHITESPACE,
  +   locale.LANG,  
  variables[VARIABLE_LANG],
  +   locale.LANGUAGE,  
  variables[VARIABLE_LANGUAGE],
  +   locale.LC_CTYPE,  
  variables[VARIABLE_LC_CTYPE],
  +   locale.LC_NUMERIC,
  variables[VARIABLE_LC_NUMERIC],
  +   locale.LC_TIME,   
  variables[VARIABLE_LC_TIME],
  +   locale.LC_COLLATE,
  variables[VARIABLE_LC_COLLATE],
  +   locale.LC_MONETARY,   
  variables[VARIABLE_LC_MONETARY],
  +   locale.LC_MESSAGES,   
  variables[VARIABLE_LC_MESSAGES],
  +   locale.LC_PAPER,  
  variables[VARIABLE_LC_PAPER],
  +   locale.LC_NAME,   
  variables[VARIABLE_LC_NAME],
  +   locale.LC_ADDRESS,
  variables

[systemd-devel] [PATCH 1/2] fileio: make parse_env_file() return number of parsed items

[Michal Sekletar]

This commit introduces possibility to call parse_env_file_internal() and hand
over extra argument where we will accumulate how many items were successfully
parsed and pushed by callback. We make use of this in parse_env_file() and
return number of parsed items on success instead of always returning zero.

As a side-effect this commit should fix bug that locale settings in
/etc/locale.conf are not overriden by options passed via kernel command line.
---
 src/shared/fileio.c | 40 +++-
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index 18960ab..38028b9 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -296,8 +296,9 @@ static int parse_env_file_internal(
 const char *fname,
 const char *newline,
 int (*push) (const char *filename, unsigned line,
- const char *key, char *value, void *userdata),
-void *userdata) {
+ const char *key, char *value, void *userdata, int 
*n_pushed),
+void *userdata,
+int *n_pushed) {
 
 _cleanup_free_ char *contents = NULL, *key = NULL;
 size_t key_alloc = 0, n_key = 0, value_alloc = 0, n_value = 0, 
last_value_whitespace = (size_t) -1, last_key_whitespace = (size_t) -1;
@@ -386,7 +387,7 @@ static int parse_env_file_internal(
 if (last_key_whitespace != (size_t) -1)
 key[last_key_whitespace] = 0;
 
-r = push(fname, line, key, value, userdata);
+r = push(fname, line, key, value, userdata, 
n_pushed);
 if (r  0)
 goto fail;
 
@@ -431,7 +432,7 @@ static int parse_env_file_internal(
 if (last_key_whitespace != (size_t) -1)
 key[last_key_whitespace] = 0;
 
-r = push(fname, line, key, value, userdata);
+r = push(fname, line, key, value, userdata, 
n_pushed);
 if (r  0)
 goto fail;
 
@@ -566,7 +567,7 @@ static int parse_env_file_internal(
 if (last_key_whitespace != (size_t) -1)
 key[last_key_whitespace] = 0;
 
-r = push(fname, line, key, value, userdata);
+r = push(fname, line, key, value, userdata, n_pushed);
 if (r  0)
 goto fail;
 }
@@ -581,7 +582,8 @@ fail:
 static int parse_env_file_push(
 const char *filename, unsigned line,
 const char *key, char *value,
-void *userdata) {
+void *userdata,
+int *n_pushed) {
 
 const char *k;
 va_list aq, *ap = userdata;
@@ -613,6 +615,10 @@ static int parse_env_file_push(
 va_end(aq);
 free(*v);
 *v = value;
+
+if (n_pushed)
+(*n_pushed)++;
+
 return 1;
 }
 }
@@ -628,22 +634,23 @@ int parse_env_file(
 const char *newline, ...) {
 
 va_list ap;
-int r;
+int r, n_pushed = 0;
 
 if (!newline)
 newline = NEWLINE;
 
 va_start(ap, newline);
-r = parse_env_file_internal(NULL, fname, newline, parse_env_file_push, 
ap);
+r = parse_env_file_internal(NULL, fname, newline, parse_env_file_push, 
ap, n_pushed);
 va_end(ap);
 
-return r;
+return r  0 ? r : n_pushed;
 }
 
 static int load_env_file_push(
 const char *filename, unsigned line,
 const char *key, char *value,
-void *userdata) {
+void *userdata,
+int *n_pushed) {
 char ***m = userdata;
 char *p;
 int r;
@@ -670,6 +677,9 @@ static int load_env_file_push(
 if (r  0)
 return r;
 
+if (n_pushed)
+(*n_pushed)++;
+
 free(value);
 return 0;
 }
@@ -681,7 +691,7 @@ int load_env_file(FILE *f, const char *fname, const char 
*newline, char ***rl) {
 if (!newline)
 newline = NEWLINE;
 
-r = parse_env_file_internal(f, fname, newline, load_env_file_push, m);
+r = parse_env_file_internal(f, fname, newline, load_env_file_push, m, 
NULL);
 if (r  0) {
 strv_free(m);
 return r;
@@ -694,7 +704,8 @@ int load_env_file(FILE *f, const char *fname, const char 
*newline, char ***rl) {
 static int load_env_file_push_pairs(
 const char *filename, unsigned line,
 const char *key, 

[systemd-devel] [PATCH 2/2] localectl: print warning when there are options given on kernel cmdline

[Michal Sekletar]

---
 src/core/locale-setup.c  | 47 +
 src/locale/localectl.c   | 50 
 src/shared/locale-util.c | 20 +++
 src/shared/locale-util.h | 25 
 4 files changed, 100 insertions(+), 42 deletions(-)

diff --git a/src/core/locale-setup.c b/src/core/locale-setup.c
index 7a41035..5177dbf 100644
--- a/src/core/locale-setup.c
+++ b/src/core/locale-setup.c
@@ -30,48 +30,11 @@
 #include fileio.h
 #include strv.h
 #include env-util.h
-
-enum {
-/* We don't list LC_ALL here on purpose. People should be
- * using LANG instead. */
-
-VARIABLE_LANG,
-VARIABLE_LANGUAGE,
-VARIABLE_LC_CTYPE,
-VARIABLE_LC_NUMERIC,
-VARIABLE_LC_TIME,
-VARIABLE_LC_COLLATE,
-VARIABLE_LC_MONETARY,
-VARIABLE_LC_MESSAGES,
-VARIABLE_LC_PAPER,
-VARIABLE_LC_NAME,
-VARIABLE_LC_ADDRESS,
-VARIABLE_LC_TELEPHONE,
-VARIABLE_LC_MEASUREMENT,
-VARIABLE_LC_IDENTIFICATION,
-_VARIABLE_MAX
-};
-
-static const char * const variable_names[_VARIABLE_MAX] = {
-[VARIABLE_LANG] = LANG,
-[VARIABLE_LANGUAGE] = LANGUAGE,
-[VARIABLE_LC_CTYPE] = LC_CTYPE,
-[VARIABLE_LC_NUMERIC] = LC_NUMERIC,
-[VARIABLE_LC_TIME] = LC_TIME,
-[VARIABLE_LC_COLLATE] = LC_COLLATE,
-[VARIABLE_LC_MONETARY] = LC_MONETARY,
-[VARIABLE_LC_MESSAGES] = LC_MESSAGES,
-[VARIABLE_LC_PAPER] = LC_PAPER,
-[VARIABLE_LC_NAME] = LC_NAME,
-[VARIABLE_LC_ADDRESS] = LC_ADDRESS,
-[VARIABLE_LC_TELEPHONE] = LC_TELEPHONE,
-[VARIABLE_LC_MEASUREMENT] = LC_MEASUREMENT,
-[VARIABLE_LC_IDENTIFICATION] = LC_IDENTIFICATION
-};
+#include locale-util.h
 
 int locale_setup(char ***environment) {
 char **add;
-char *variables[_VARIABLE_MAX] = {};
+char *variables[_VARIABLE_LC_MAX] = {};
 int r = 0, i;
 
 if (detect_container(NULL) = 0) {
@@ -121,13 +84,13 @@ int locale_setup(char ***environment) {
 }
 
 add = NULL;
-for (i = 0; i  _VARIABLE_MAX; i++) {
+for (i = 0; i  _VARIABLE_LC_MAX; i++) {
 char *s;
 
 if (!variables[i])
 continue;
 
-s = strjoin(variable_names[i], =, variables[i], NULL);
+s = strjoin(locale_variable_to_string(i), =, variables[i], 
NULL);
 if (!s) {
 r = -ENOMEM;
 goto finish;
@@ -157,7 +120,7 @@ int locale_setup(char ***environment) {
 finish:
 strv_free(add);
 
-for (i = 0; i  _VARIABLE_MAX; i++)
+for (i = 0; i  _VARIABLE_LC_MAX; i++)
 free(variables[i]);
 
 return r;
diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index bf8b7b2..5917364 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -43,6 +43,8 @@
 #include path-util.h
 #include utf8.h
 #include def.h
+#include virt.h
+#include fileio.h
 #include locale-util.h
 
 static bool arg_no_pager = false;
@@ -81,6 +83,53 @@ typedef struct StatusInfo {
 const char *x11_options;
 } StatusInfo;
 
+static void print_overriden_variables(void) {
+int r;
+char *variables[_VARIABLE_LC_MAX] = {};
+LocaleVariable j;
+bool print_warning = true;
+
+if (detect_container(NULL)  0 || arg_host)
+return;
+
+r = parse_env_file(/proc/cmdline, WHITESPACE,
+   locale.LANG,  
variables[VARIABLE_LANG],
+   locale.LANGUAGE,  
variables[VARIABLE_LANGUAGE],
+   locale.LC_CTYPE,  
variables[VARIABLE_LC_CTYPE],
+   locale.LC_NUMERIC,
variables[VARIABLE_LC_NUMERIC],
+   locale.LC_TIME,   
variables[VARIABLE_LC_TIME],
+   locale.LC_COLLATE,
variables[VARIABLE_LC_COLLATE],
+   locale.LC_MONETARY,   
variables[VARIABLE_LC_MONETARY],
+   locale.LC_MESSAGES,   
variables[VARIABLE_LC_MESSAGES],
+   locale.LC_PAPER,  
variables[VARIABLE_LC_PAPER],
+   locale.LC_NAME,   
variables[VARIABLE_LC_NAME],
+   locale.LC_ADDRESS,
variables[VARIABLE_LC_ADDRESS],
+   locale.LC_TELEPHONE,  
variables[VARIABLE_LC_TELEPHONE],
+   locale.LC_MEASUREMENT,
variables[VARIABLE_LC_MEASUREMENT],
+   locale.LC_IDENTIFICATION, 
variables[VARIABLE_LC_IDENTIFICATION],
+   NULL);
+
+if (r  0  r != -ENOENT) {
+log_warning(Failed to read /proc/cmdline: %s, strerror(-r));
+goto finish;
+}
+
+for (j 

Re: [systemd-devel] [PATCH v5] socket: introduce SELinuxContextFromNet option

[Michal Sekletar]

On Fri, Sep 19, 2014 at 12:13:18PM +0200, Tom Gundersen wrote:
 On Mon, Sep 8, 2014 at 3:42 PM, Michal Sekletar msekl...@redhat.com wrote:
  This makes possible to spawn service instances triggered by socket with
  MLS/MCS SELinux labels which are created based on information provided by
  connected peer.
 
  Implementation of label_get_child_mls_label derived from xinetd.
 
  Reviewed-by: Paul Moore pmo...@redhat.com
  ---
 
  Changes in v5:
  * removed unneeded #include of libselinux headers from socket.c
  * fixed white-space issue in service_set_socket_fd
 
 As all the comments from v4 has been fixed, please go ahead and commit this.

Rebased and pushed.

Regards,

Michal

 
 Cheers,
 
 Tom
 
   man/systemd.socket.xml|  26 
   src/core/execute.c|  29 +++--
   src/core/execute.h|   1 +
   src/core/load-fragment-gperf.gperf.m4 |   3 +
   src/core/service.c|  10 +--
   src/core/service.h|   3 +-
   src/core/socket.c |  16 +++--
   src/core/socket.h |   2 +
   src/shared/label.c| 113 
  ++
   src/shared/label.h|   2 +
   10 files changed, 190 insertions(+), 15 deletions(-)
 
  diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
  index 7a63348..dad0267 100644
  --- a/man/systemd.socket.xml
  +++ b/man/systemd.socket.xml
  @@ -676,6 +676,32 @@
   /varlistentry
 
   varlistentry
  +  
  termvarnameSELinuxContextFromNet=/varname/term
  + listitemparaTakes a boolean
  + argument. When true systemd will attempt
  + to figure out the SELinux label used
  + for the instantiated service from the
  + information handed by the peer over the
  + network. Note that only the security
  + level is used from the information
  + provided by the peer. Other parts of
  + the resulting SELinux context originate
  + from either the target binary that is
  + effectively triggered by socket unit
  + are taken from the value of the
  + varnameSELinuxContext=/varname
  + option.This configuration option only
  + affects sockets with
  + varnameAccept=/varname mode set to
  + literaltrue/literal. Also note that
  + this option is useful only when
  + MLS/MCS SELinux policy is
  + deployed. Defaults to
  + literalfalse/literal.
  + /para/listitem
  +/varlistentry
  +
  +varlistentry
   termvarnamePipeSize=/varname/term
   listitemparaTakes a size in
   bytes. Controls the pipe buffer size
  diff --git a/src/core/execute.c b/src/core/execute.c
  index 0a59147..37b9ed4 100644
  --- a/src/core/execute.c
  +++ b/src/core/execute.c
  @@ -83,6 +83,7 @@
   #include af-list.h
   #include mkdir.h
   #include apparmor-util.h
  +#include label.h
 
   #ifdef HAVE_SECCOMP
   #include seccomp-util.h
  @@ -1646,11 +1647,29 @@ static int exec_child(ExecCommand *command,
   #endif
 
   #ifdef HAVE_SELINUX
  -if (context-selinux_context  use_selinux()) {
  -err = setexeccon(context-selinux_context);
  -if (err  0  !context-selinux_context_ignore) {
  -*error = EXIT_SELINUX_CONTEXT;
  -return err;
  +if (use_selinux()) {
  +if (context-selinux_context) {
  +err = setexeccon(context-selinux_context);
  +if (err  0  
  !context-selinux_context_ignore) {
  +*error = EXIT_SELINUX_CONTEXT;
  +return err;
  +}
  +}
  +
  +if (params-selinux_context_net  socket_fd = 0) 
  {
  +_cleanup_free_ char *label = NULL;
  +
  +err = label_get_child_mls_label(socket_fd, 
  command-path, label);
  +if (err  0

[systemd-devel] [RFC] [PATCH] cgroup: don't trim cgroup trees created by someone else

[Michal Sekletar]

In cases when there is a cgroup tree in a controller hierarchy which was
not created by us, but it looks like it was (i.e. cgroup path is the
same as the one in systemd's named hierarchy) we shouldn't delete it.
---

Reproducer:
1) start qemu-kvm VM via virsh/virt-manager
2) ls /sys/fs/cgroup/memory /* see machine.slice cgroup */
3) verify that scope unit for VM has MemoryAccounting=no
4) systemctl daemon-reload
5) systemctl restart some_service
6) ls /sys/fs/cgroup/memory /* see machine.slice cgroup gone */

To be honest I have no idea if the approach taken here is correct, however
it fixes the bug at least on my machine.


 src/core/cgroup.c| 2 +-
 src/shared/cgroup-util.c | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 6c6e4f5..8ede79a 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -780,7 +780,7 @@ void unit_destroy_cgroup(Unit *u) {
 if (!u-cgroup_path)
 return;
 
-r = cg_trim_everywhere(u-manager-cgroup_supported, u-cgroup_path, 
!unit_has_name(u, SPECIAL_ROOT_SLICE));
+r = cg_trim_everywhere(u-cgroup_realized_mask, u-cgroup_path, 
!unit_has_name(u, SPECIAL_ROOT_SLICE));
 if (r  0)
 log_debug(Failed to destroy cgroup %s: %s, u-cgroup_path, 
strerror(-r));
 
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index da8e885..a19b5b4 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1615,8 +1615,6 @@ int cg_create_everywhere(CGroupControllerMask supported, 
CGroupControllerMask ma
 NULSTR_FOREACH(n, mask_names) {
 if (mask  bit)
 cg_create(n, path);
-else if (supported  bit)
-cg_trim(n, path, true);
 
 bit = 1;
 }
-- 
2.0.1

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

[systemd-devel] [PATCH v5] socket: introduce SELinuxContextFromNet option

[Michal Sekletar]

This makes possible to spawn service instances triggered by socket with
MLS/MCS SELinux labels which are created based on information provided by
connected peer.

Implementation of label_get_child_mls_label derived from xinetd.

Reviewed-by: Paul Moore pmo...@redhat.com
---

Changes in v5:
* removed unneeded #include of libselinux headers from socket.c
* fixed white-space issue in service_set_socket_fd

 man/systemd.socket.xml|  26 
 src/core/execute.c|  29 +++--
 src/core/execute.h|   1 +
 src/core/load-fragment-gperf.gperf.m4 |   3 +
 src/core/service.c|  10 +--
 src/core/service.h|   3 +-
 src/core/socket.c |  16 +++--
 src/core/socket.h |   2 +
 src/shared/label.c| 113 ++
 src/shared/label.h|   2 +
 10 files changed, 190 insertions(+), 15 deletions(-)

diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 7a63348..dad0267 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -676,6 +676,32 @@
 /varlistentry
 
 varlistentry
+  
termvarnameSELinuxContextFromNet=/varname/term
+ listitemparaTakes a boolean
+ argument. When true systemd will attempt
+ to figure out the SELinux label used
+ for the instantiated service from the
+ information handed by the peer over the
+ network. Note that only the security
+ level is used from the information
+ provided by the peer. Other parts of
+ the resulting SELinux context originate
+ from either the target binary that is
+ effectively triggered by socket unit
+ are taken from the value of the
+ varnameSELinuxContext=/varname
+ option.This configuration option only
+ affects sockets with
+ varnameAccept=/varname mode set to
+ literaltrue/literal. Also note that
+ this option is useful only when
+ MLS/MCS SELinux policy is
+ deployed. Defaults to
+ literalfalse/literal.
+ /para/listitem
+/varlistentry
+
+varlistentry
 termvarnamePipeSize=/varname/term
 listitemparaTakes a size in
 bytes. Controls the pipe buffer size
diff --git a/src/core/execute.c b/src/core/execute.c
index 0a59147..37b9ed4 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -83,6 +83,7 @@
 #include af-list.h
 #include mkdir.h
 #include apparmor-util.h
+#include label.h
 
 #ifdef HAVE_SECCOMP
 #include seccomp-util.h
@@ -1646,11 +1647,29 @@ static int exec_child(ExecCommand *command,
 #endif
 
 #ifdef HAVE_SELINUX
-if (context-selinux_context  use_selinux()) {
-err = setexeccon(context-selinux_context);
-if (err  0  !context-selinux_context_ignore) {
-*error = EXIT_SELINUX_CONTEXT;
-return err;
+if (use_selinux()) {
+if (context-selinux_context) {
+err = setexeccon(context-selinux_context);
+if (err  0  
!context-selinux_context_ignore) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
+}
+
+if (params-selinux_context_net  socket_fd = 0) {
+_cleanup_free_ char *label = NULL;
+
+err = label_get_child_mls_label(socket_fd, 
command-path, label);
+if (err  0) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
+
+err = setexeccon(label);
+if (err  0) {
+*error = EXIT_SELINUX_CONTEXT;
+return err;
+}
 }
 }
 #endif
diff --git 

Re: [systemd-devel] [PATCH 2/2 v3] socket: introduce SELinuxContextFromNet option

[Michal Sekletar]

On Wed, Aug 27, 2014 at 04:45:32AM +0200, Zbigniew Jędrzejewski-Szmek wrote:
 On Mon, Aug 25, 2014 at 10:02:58AM +0200, Michal Sekletar wrote:
   varlistentry
  +  
  termvarnameSELinuxContextFromNet=/varname/term
  +listitemparaTakes a boolean
  +value. Controls whether systemd attempts 
  to figure out
  +SELinux label used for instantiated 
  service from
  +information handed by peer over the
  +network. From information provided by
  +peer we actually use only security level.
  +Other parts of resulting SELinux context
  +originate from either the target binary
  +effectively triggered by socket unit or
  +it is the value of
  +varnameSELinuxContext/varname
  +option. Configuration option has effect
  +only on sockets with
  +varnameAccept/varname mode set to
  +literalyes/literal. Also note that
  +this option is usefull only when MLS/MCS
  +SELinux policy is deployed. Defaults to
  +literalfalse/literal.
  +/para/listitem
 I think some prepositions are missing. Maybe something like this:
 
 Takes a boolean argument. When true systemd will attempt to figure
 out the SELinux label used for the instantiated service from
 the information handed by the peer over the network. Note
 that only the security label is used in the information provided
 by the peer. Other parts of the SELinux context originate from either
 the target binary that is effectively triggered by the socket unit
 or are taken from the value of the varnameSELinuxContext=/varname
 option. This configuration option only affects sockets with
 varnameAccept=/varname mode set to true. Also note that this
 option is useful only when MLS/MCS SELinux policy is deployed.
 Defaults to false.

Thanks for corrections, sounds a bit better than my original version.

 
  +/varlistentry
  +
  +varlistentry
   termvarnamePipeSize=/varname/term
   listitemparaTakes a size in
   bytes. Controls the pipe buffer size
  diff --git a/src/core/execute.c b/src/core/execute.c
  index b5b2247..0e583ed 100644
  --- a/src/core/execute.c
  +++ b/src/core/execute.c
  @@ -83,6 +83,7 @@
   #include af-list.h
   #include mkdir.h
   #include apparmor-util.h
  +#include label.h
   
   #ifdef HAVE_SECCOMP
   #include seccomp-util.h
  @@ -1232,6 +1233,7 @@ int exec_spawn(ExecCommand *command,
  bool apply_chroot,
  bool apply_tty_stdin,
  bool confirm_spawn,
  +   bool selinux_context_net,
  CGroupControllerMask cgroup_supported,
  const char *cgroup_path,
  const char *runtime_prefix,
  @@ -1724,11 +1726,29 @@ int exec_spawn(ExecCommand *command,
   #endif
   
   #ifdef HAVE_SELINUX
  -if (context-selinux_context  use_selinux()) {
  -err = setexeccon(context-selinux_context);
  -if (err  0  
  !context-selinux_context_ignore) {
  -r = EXIT_SELINUX_CONTEXT;
  -goto fail_child;
  +if (use_selinux()) {
  +if (context-selinux_context) {
  +err = 
  setexeccon(context-selinux_context);
  +if (err  0  
  !context-selinux_context_ignore) {
  +r = EXIT_SELINUX_CONTEXT;
  +goto fail_child;
  +}
  +}
  +
  +if (selinux_context_net  socket_fd = 0) 
  {
  +_cleanup_free_ char *label = NULL;
  +
  +err = 
  label_get_child_mls_label(socket_fd, command-path, label);
  +if (err  0) {
  +r = EXIT_SELINUX_CONTEXT;
  +goto fail_child;
  +}
  +
  +err = setexeccon(label

[systemd-devel] [PATCH v4] socket: introduce SELinuxContextFromNet option

[Michal Sekletar]

This makes possible to spawn service instances triggered by socket with
MLS/MCS SELinux labels which are created based on information provided by
connected peer.

Implementation of label_get_child_mls_label derived from xinetd.

Reviewed-by: Paul Moore pmo...@redhat.com
---

Changes in v4:
* fixes in man page suggested by Zbyszek
* cleanup macros definitions for security context moved to label.c
* label_get_our_label now returns -EOPNOTSUPP if SELinux support is 
disabled
* more robust error checking in label_get_child_mls_label

 man/systemd.socket.xml|  26 
 src/core/execute.c|  30 +++--
 src/core/execute.h|   1 +
 src/core/load-fragment-gperf.gperf.m4 |   3 +
 src/core/mount.c  |   1 +
 src/core/service.c|   5 +-
 src/core/service.h|   3 +-
 src/core/socket.c |  21 +--
 src/core/socket.h |   2 +
 src/core/swap.c   |   1 +
 src/shared/label.c| 113 ++
 src/shared/label.h|   2 +
 12 files changed, 196 insertions(+), 12 deletions(-)

diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 8394fa8..60f5c48 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -676,6 +676,32 @@
 /varlistentry
 
 varlistentry
+  
termvarnameSELinuxContextFromNet=/varname/term
+ listitemparaTakes a boolean
+ argument. When true systemd will attempt
+ to figure out the SELinux label used
+ for the instantiated service from the
+ information handed by the peer over the
+ network. Note that only the security
+ level is used from the information
+ provided by the peer. Other parts of
+ the resulting SELinux context originate
+ from either the target binary that is
+ effectively triggered by socket unit
+ are taken from the value of the
+ varnameSELinuxContext=/varname
+ option.This configuration option only
+ affects sockets with
+ varnameAccept=/varname mode set to
+ literaltrue/literal. Also note that
+ this option is useful only when
+ MLS/MCS SELinux policy is
+ deployed. Defaults to
+ literalfalse/literal.
+ /para/listitem
+/varlistentry
+
+varlistentry
 termvarnamePipeSize=/varname/term
 listitemparaTakes a size in
 bytes. Controls the pipe buffer size
diff --git a/src/core/execute.c b/src/core/execute.c
index 066efd6..84d6bec 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -83,6 +83,7 @@
 #include af-list.h
 #include mkdir.h
 #include apparmor-util.h
+#include label.h
 
 #ifdef HAVE_SECCOMP
 #include seccomp-util.h
@@ -1232,6 +1233,7 @@ int exec_spawn(ExecCommand *command,
bool apply_chroot,
bool apply_tty_stdin,
bool confirm_spawn,
+   bool selinux_context_net,
CGroupControllerMask cgroup_supported,
const char *cgroup_path,
const char *runtime_prefix,
@@ -1722,11 +1724,29 @@ int exec_spawn(ExecCommand *command,
 #endif
 
 #ifdef HAVE_SELINUX
-if (context-selinux_context  use_selinux()) {
-err = setexeccon(context-selinux_context);
-if (err  0  
!context-selinux_context_ignore) {
-r = EXIT_SELINUX_CONTEXT;
-goto fail_child;
+if (use_selinux()) {
+if (context-selinux_context) {
+err = 
setexeccon(context-selinux_context);
+if (err  0  
!context-selinux_context_ignore) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+}
+
+if (selinux_context_net  socket_fd = 0) {
+

[systemd-devel] [PATCH 2/2 v3] socket: introduce SELinuxContextFromNet option

[Michal Sekletar]

This makes possible to spawn service instances triggered by socket with
MLS/MCS SELinux labels which are created based on information provided by
connected peer.

Implementation of label_get_child_mls_label derived from xinetd.

Reviewed-by: Paul Moore pmo...@redhat.com
---

Changes in v3:
* renamed option to SELinuxContextFromNet
* removed call to getcon() from socket.c,
* introduced new function label_get_our_label
* introduced cleanup macros for context_t and security_context_t

Patch was compile tested, Miro it would be greatly appreciated if you could 
test it.

Thanks!

 man/systemd.socket.xml| 23 ++
 src/core/execute.c| 30 ++--
 src/core/execute.h|  1 +
 src/core/load-fragment-gperf.gperf.m4 |  3 ++
 src/core/mount.c  |  1 +
 src/core/service.c|  5 +-
 src/core/service.h|  3 +-
 src/core/socket.c | 21 +++--
 src/core/socket.h |  2 +
 src/core/swap.c   |  1 +
 src/shared/label.c| 86 +++
 src/shared/label.h| 17 +++
 12 files changed, 181 insertions(+), 12 deletions(-)

diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 238029a..79ed40d 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -676,6 +676,29 @@
 /varlistentry
 
 varlistentry
+  
termvarnameSELinuxContextFromNet=/varname/term
+listitemparaTakes a boolean
+value. Controls whether systemd attempts to 
figure out
+SELinux label used for instantiated service 
from
+information handed by peer over the
+network. From information provided by
+peer we actually use only security level.
+Other parts of resulting SELinux context
+originate from either the target binary
+effectively triggered by socket unit or
+it is the value of
+varnameSELinuxContext/varname
+option. Configuration option has effect
+only on sockets with
+varnameAccept/varname mode set to
+literalyes/literal. Also note that
+this option is usefull only when MLS/MCS
+SELinux policy is deployed. Defaults to
+literalfalse/literal.
+/para/listitem
+/varlistentry
+
+varlistentry
 termvarnamePipeSize=/varname/term
 listitemparaTakes a size in
 bytes. Controls the pipe buffer size
diff --git a/src/core/execute.c b/src/core/execute.c
index b5b2247..0e583ed 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -83,6 +83,7 @@
 #include af-list.h
 #include mkdir.h
 #include apparmor-util.h
+#include label.h
 
 #ifdef HAVE_SECCOMP
 #include seccomp-util.h
@@ -1232,6 +1233,7 @@ int exec_spawn(ExecCommand *command,
bool apply_chroot,
bool apply_tty_stdin,
bool confirm_spawn,
+   bool selinux_context_net,
CGroupControllerMask cgroup_supported,
const char *cgroup_path,
const char *runtime_prefix,
@@ -1724,11 +1726,29 @@ int exec_spawn(ExecCommand *command,
 #endif
 
 #ifdef HAVE_SELINUX
-if (context-selinux_context  use_selinux()) {
-err = setexeccon(context-selinux_context);
-if (err  0  
!context-selinux_context_ignore) {
-r = EXIT_SELINUX_CONTEXT;
-goto fail_child;
+if (use_selinux()) {
+if (context-selinux_context) {
+err = 
setexeccon(context-selinux_context);
+if (err  0  
!context-selinux_context_ignore) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+}
+
+if (selinux_context_net  socket_fd = 0) {
+_cleanup_free_ char *label = NULL;
+
+err = 

[systemd-devel] [PATCH 1/2] everywhere: don't use uprefixed word context in struct and function names

[Michal Sekletar]

We use libselinux which exports context_free function. To prevent name
clashes rename our internal APIs and don't use context_* but rather
appropriately prefixed names.
---
 src/hostname/hostnamed.c |  72 +--
 src/journal/mmap-cache.c | 102 +++
 src/journal/mmap-cache.h |   2 +-
 src/locale/localed.c |  84 
 src/socket-proxy/socket-proxyd.c |  18 +++
 src/timedate/timedated.c |  54 ++---
 6 files changed, 166 insertions(+), 166 deletions(-)

diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index d31fef7..edd9d9b 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -53,12 +53,12 @@ enum {
 _PROP_MAX
 };
 
-typedef struct Context {
+typedef struct HostnamedContext {
 char *data[_PROP_MAX];
 Hashmap *polkit_registry;
-} Context;
+} HostnamedContext;
 
-static void context_reset(Context *c) {
+static void hostnamed_context_reset(HostnamedContext *c) {
 int p;
 
 assert(c);
@@ -69,20 +69,20 @@ static void context_reset(Context *c) {
 }
 }
 
-static void context_free(Context *c) {
+static void hostnamed_context_free(HostnamedContext *c) {
 assert(c);
 
-context_reset(c);
+hostnamed_context_reset(c);
 bus_verify_polkit_async_registry_free(c-polkit_registry);
 }
 
-static int context_read_data(Context *c) {
+static int hostnamed_context_read_data(HostnamedContext *c) {
 int r;
 struct utsname u;
 
 assert(c);
 
-context_reset(c);
+hostnamed_context_reset(c);
 
 assert_se(uname(u) = 0);
 c-data[PROP_KERNEL_NAME] = strdup(u.sysname);
@@ -242,7 +242,7 @@ try_dmi:
 return NULL;
 }
 
-static char* context_fallback_icon_name(Context *c) {
+static char* hostnamed_context_fallback_icon_name(HostnamedContext *c) {
 const char *chassis;
 
 assert(c);
@@ -262,7 +262,7 @@ static bool hostname_is_useful(const char *hn) {
 return !isempty(hn)  !is_localhost(hn);
 }
 
-static int context_update_kernel_hostname(Context *c) {
+static int hostnamed_context_update_kernel_hostname(HostnamedContext *c) {
 const char *static_hn;
 const char *hn;
 
@@ -293,7 +293,7 @@ static int context_update_kernel_hostname(Context *c) {
 return 0;
 }
 
-static int context_write_data_static_hostname(Context *c) {
+static int hostnamed_context_write_data_static_hostname(HostnamedContext *c) {
 
 assert(c);
 
@@ -307,7 +307,7 @@ static int context_write_data_static_hostname(Context *c) {
 return write_string_file_atomic_label(/etc/hostname, 
c-data[PROP_STATIC_HOSTNAME]);
 }
 
-static int context_write_data_machine_info(Context *c) {
+static int hostnamed_context_write_data_machine_info(HostnamedContext *c) {
 
 static const char * const name[_PROP_MAX] = {
 [PROP_PRETTY_HOSTNAME] = PRETTY_HOSTNAME,
@@ -369,11 +369,11 @@ static int property_get_icon_name(
 sd_bus_error *error) {
 
 _cleanup_free_ char *n = NULL;
-Context *c = userdata;
+HostnamedContext *c = userdata;
 const char *name;
 
 if (isempty(c-data[PROP_ICON_NAME]))
-name = n = context_fallback_icon_name(c);
+name = n = hostnamed_context_fallback_icon_name(c);
 else
 name = c-data[PROP_ICON_NAME];
 
@@ -392,7 +392,7 @@ static int property_get_chassis(
 void *userdata,
 sd_bus_error *error) {
 
-Context *c = userdata;
+HostnamedContext *c = userdata;
 const char *name;
 
 if (isempty(c-data[PROP_CHASSIS]))
@@ -404,7 +404,7 @@ static int property_get_chassis(
 }
 
 static int method_set_hostname(sd_bus *bus, sd_bus_message *m, void *userdata, 
sd_bus_error *error) {
-Context *c = userdata;
+HostnamedContext *c = userdata;
 const char *name;
 int interactive;
 char *h;
@@ -439,7 +439,7 @@ static int method_set_hostname(sd_bus *bus, sd_bus_message 
*m, void *userdata, s
 free(c-data[PROP_HOSTNAME]);
 c-data[PROP_HOSTNAME] = h;
 
-r = context_update_kernel_hostname(c);
+r = hostnamed_context_update_kernel_hostname(c);
 if (r  0) {
 log_error(Failed to set host name: %s, strerror(-r));
 return sd_bus_error_set_errnof(error, r, Failed to set 
hostname: %s, strerror(-r));
@@ -453,7 +453,7 @@ static int method_set_hostname(sd_bus *bus, sd_bus_message 
*m, void *userdata, s
 }
 
 static int method_set_static_hostname(sd_bus *bus, sd_bus_message *m, void 
*userdata, sd_bus_error *error) {
-Context *c = userdata;
+HostnamedContext *c = userdata;
 const char *name;
 int interactive;
 int r;
@@ -491,13 +491,13 @@ static int method_set_static_hostname(sd_bus *bus, 

Re: [systemd-devel] [PATCH] missing: add BPF_XOR

[Michal Sekletar]

On Thu, Aug 21, 2014 at 12:38:08PM +0200, Michael Olbrich wrote:
 BPF_XOR was introduced in kernel 3.7
 ---
 
 This fixes compiling systemd for me. I'm not sure about the implications of
 this. I'm not sure what happens if the code using it is executed on a linux
 kernel  3.7

I don't think that merging this patch makes sense since we don't really support
combination of old kernel and new systemd. Both components are so centric these
days that they should be updated in lockstep. Hence allowing people to compile
against very old kernels shouldn't be possible really.

AFAIK filter using unsupported BPF instruction will be rejected by kernel's
static analyzer for BPF code.

Michal

 
 Michael
 
  src/shared/missing.h | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/src/shared/missing.h b/src/shared/missing.h
 index 3ff1a21..1321db1 100644
 --- a/src/shared/missing.h
 +++ b/src/shared/missing.h
 @@ -589,3 +589,7 @@ static inline int setns(int fd, int nstype) {
  #ifndef NET_NAME_RENAMED
  #  define NET_NAME_RENAMED 4
  #endif
 +
 +#ifndef BPF_XOR
 +#  define BPF_XOR 0xa0
 +#endif
 -- 
 2.1.0.rc1
 
 ___
 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] socket: introduce SELinuxLabeledNet option

[Michal Sekletar]

On Wed, Aug 13, 2014 at 09:42:14PM +0200, Lennart Poettering wrote:
 On Tue, 05.08.14 13:46, Michal Sekletar (msekl...@redhat.com) wrote:
 
  This makes possible to spawn service instances triggered by socket with
  MLS/MCS SELinux labels which are created based on information provided by
  connected peer.
  
  Implementation of label_get_socket_label derived from xinetd.
  ---
   man/systemd.socket.xml| 11 ++
   src/core/execute.c| 23 +++-
   src/core/execute.h|  1 +
   src/core/load-fragment-gperf.gperf.m4 |  3 ++
   src/core/socket.c | 18 +++--
   src/core/socket.h |  2 +
   src/shared/label.c| 69 
  +++
   src/shared/label.h|  1 +
   8 files changed, 123 insertions(+), 5 deletions(-)
  
  diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
  index 09a7311..959f46f 100644
  --- a/man/systemd.socket.xml
  +++ b/man/systemd.socket.xml
  @@ -588,6 +588,17 @@
   /varlistentry
   
   varlistentry
  +  
  termvarnameSELinuxLabeledNet=/varname/term
  +  listitemparaTakes a boolean
  +  value. Controls whether systemd attempts to 
  figure out
  +  SELinux label used for instantiated service from
  +  information handed by peer over the
  +  network. Configuration option has effect only
  +  on sockets with literalAccept=/literal
  +  mode set to 
  literalyes/literal./para/listitem
  +/varlistentry
 
 Could you please indent the same way as the other settings?

Will do.

 
   
   #ifdef HAVE_SECCOMP
   #include seccomp-util.h
  @@ -1729,6 +1730,22 @@ int exec_spawn(ExecCommand *command,
   goto fail_child;
   }
   }
  +
  +if (context-selinux_labeled_net  use_selinux()) 
  {
  +_cleanup_free_ char *label = NULL;
  +
  +err = label_get_socket_label(socket_fd, 
  command-path, label);
  +if (err  0) {
  +r = EXIT_SELINUX_CONTEXT;
  +goto fail_child;
  +}
  +
  +err = setexeccon(label);
  +if (err  0) {
  +r = EXIT_SELINUX_CONTEXT;
  +goto fail_child;
  +}
  +}
 
 If both SELinuxContext= and SELinuxLabeledNet= are set we should
 probably not execute one after the other, but only one of them.

I think that it makes sense to set both and resulting label will be combination
of both. Note that from SELinux label we acquire from network we are using only
security level part.

 
 Also, before applying selinux_labeled_net you really need to check for
 socket_fd if it is = 0...

Agreed.

 
 I think it would be good to enclose processing of SELinuxContext= and
 SELinuxLabeledNet= processing in single if (use_selinux()) check, and then
 use SELinuxLabeledNet= if it is set and if socket_fd = 0, and otherwise
 apply SELinuxContext= if it is set.
 
 I have no understanding of the actual selinux logic here, but it looks
 right, I hope this is blessed by dwalsh or someome else knowledgable in
 these things?

I will change it in a way such that resulting label will be combination of
SELinuxContext and whatever security level is handed to us from network. In most
cases we will combine label from target executable and security label from
network tough.

 
 
  -
  +#ifdef HAVE_SELINUX
  +if (!know_label  s-selinux_labeled_net) {
  +r = getcon(label);
  +if (r  0)
  +return r;
  +know_label = true;
  +}
  +#endif
   if (!know_label) {
   
 
 Can you explain this bit? Why would we label the listening socket with our own
 label here?

This is because of MLS SELinux policy implementation details. If we relabel to
the context acquired from the target binary then it if not possible to connect 
to
the socket because SELinux denies a packet receive on the socket.

https://github.com/selinux-policy/selinux-policy/blob/rawhide-base/policy/mls#L361

 
   r = socket_instantiate_service(s);
  @@ -1773,6 +1782,9 @@ static void socket_enter_running(Socket *s, int cfd) {
   cfd = -1;
   s-n_connections

Re: [systemd-devel] [PATCH] socket: introduce SELinuxLabeledNet option

[Michal Sekletar]

On Wed, Aug 20, 2014 at 03:29:37PM +0200, Lennart Poettering wrote:
 On Wed, 20.08.14 12:01, Michal Sekletar (msekl...@redhat.com) wrote:
 
+if (context-selinux_labeled_net  
use_selinux()) {
+_cleanup_free_ char *label = NULL;
+
+err = 
label_get_socket_label(socket_fd, command-path, label);
+if (err  0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+
+err = setexeccon(label);
+if (err  0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+}
   
   If both SELinuxContext= and SELinuxLabeledNet= are set we should
   probably not execute one after the other, but only one of them.
  
  I think that it makes sense to set both and resulting label will be 
  combination
  of both. Note that from SELinux label we acquire from network we are using 
  only
  security level part.
 
 Hmm? But in both cases we just execute setexeccon()? Are you saying that
 if we invoke setexeccon() twice with a specific combination of
 parameters then it leads to different results than just doing the second
 invocation and leaving the first one out?

Sorry, I've phrased what I meant poorly. If both are set we will call 
setexeccon()
to set whatever user has given us via SELinuxContext and then we will figure out
resulting context from context which we set already and from information
provided by connected peer. If no context is explicitly required by user then we
will figure out resulting context from target binary and from information
acquired from network. In v2 of the patch docs will explain how combination of
two options work in greater detail.

 
 The documentation doesn't mention that. 
 
 Obviously, I have no understanding of SELinux as it appears, but this
 sounds so werid to me.
 
 Dan, what's the story here?
 
 
+#ifdef HAVE_SELINUX
+if (!know_label  s-selinux_labeled_net) {
+r = getcon(label);
+if (r  0)
+return r;
+know_label = true;
+}
+#endif
 if (!know_label) {
 
   
   Can you explain this bit? Why would we label the listening socket with 
   our own
   label here?
  
  This is because of MLS SELinux policy implementation details. If we relabel 
  to
  the context acquired from the target binary then it if not possible to 
  connect to
  the socket because SELinux denies a packet receive on the socket.
  
  https://github.com/selinux-policy/selinux-policy/blob/rawhide-base/policy/mls#L361
 
 I don't understand a word of this, I mus say. 
 
 But is it really the intention here to take the current process label
 and apply it directly to the socket fd?
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] socket: introduce SELinuxLabeledNet option

[Michal Sekletar]

On Wed, Aug 13, 2014 at 09:42:14PM +0200, Lennart Poettering wrote:
snip
  @@ -1773,6 +1782,9 @@ static void socket_enter_running(Socket *s, int cfd) {
   cfd = -1;
   s-n_connections ++;
   
  +if (s-selinux_labeled_net)
  +service-exec_context.selinux_labeled_net = true;
  +
 
 This I don't like. We shouldn#t make permanent changes here... I'd
 prefer if we could pass this somehow else, so that the service isn't
 changed permanently...

Well I don't like this either but I don't know about any other way how to pass
that flag all the way down to exec_spawn. However, is this really an issue if
the new option will work only for Accept=true services?

 
 I must say I feel a bit uneasy about the naming of SELinuxContext= and
 SELinuxLabeledNet=... One uses the term context, the other one
 label. afaiu that's actually the same thing, no? If it is, can we use
 the same terminology here? (which would mean sticking to context since
 that's what we already are using...)
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2] socket: introduce SELinuxContextViaNet option

[Michal Sekletar]

This makes possible to spawn service instances triggered by socket with
MLS/MCS SELinux labels which are created based on information provided by
connected peer.

Implementation of label_get_child_label derived from xinetd.

Reviewed-by: Paul Moore pmo...@redhat.com
---

Changes in v2:
* make possible to use both SELinuxContextViaNet and SELinuxContext at 
  the same time
* provide better explanation how is resulting SELinux context figured 
out
* fix some of the issues pointed out by Lennart

Future work:
* find out nicer way how to pass information to the triggered service 
  unit that SELinuxContextViaNet is enabled on the socket
  

 man/systemd.socket.xml| 23 +++
 src/core/execute.c| 35 
 src/core/execute.h|  1 +
 src/core/load-fragment-gperf.gperf.m4 |  3 ++
 src/core/socket.c | 22 --
 src/core/socket.h |  2 +
 src/shared/label.c| 77 +++
 src/shared/label.h|  1 +
 8 files changed, 154 insertions(+), 10 deletions(-)

diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 238029a..0ff4ef4 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -676,6 +676,29 @@
 /varlistentry
 
 varlistentry
+  termvarnameSELinuxContextViaNet=/varname/term
+listitemparaTakes a boolean
+value. Controls whether systemd attempts to 
figure out
+SELinux label used for instantiated service 
from
+information handed by peer over the
+network. From information provided by
+peer we actually use only security level.
+Other parts of resulting SELinux context
+originate from either the target binary
+effectively triggered by socket unit or
+it is the value of
+varnameSELinuxContext/varname
+option. Configuration option has effect
+only on sockets with
+varnameAccept/varname mode set to
+literalyes/literal. Also note that
+this option is usefull only when MLS/MCS
+SELinux policy is deployed. Defaults to
+literalfalse/literal.
+/para/listitem
+/varlistentry
+
+varlistentry
 termvarnamePipeSize=/varname/term
 listitemparaTakes a size in
 bytes. Controls the pipe buffer size
diff --git a/src/core/execute.c b/src/core/execute.c
index d8452a6..e006fdb 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -83,6 +83,7 @@
 #include af-list.h
 #include mkdir.h
 #include apparmor-util.h
+#include label.h
 
 #ifdef HAVE_SECCOMP
 #include seccomp-util.h
@@ -1722,11 +1723,29 @@ int exec_spawn(ExecCommand *command,
 #endif
 
 #ifdef HAVE_SELINUX
-if (context-selinux_context  use_selinux()) {
-err = setexeccon(context-selinux_context);
-if (err  0  
!context-selinux_context_ignore) {
-r = EXIT_SELINUX_CONTEXT;
-goto fail_child;
+if (use_selinux()) {
+if (context-selinux_context) {
+err = 
setexeccon(context-selinux_context);
+if (err  0  
!context-selinux_context_ignore) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+}
+
+if (context-selinux_context_via_net  
socket_fd = 0) {
+_cleanup_free_ char *label = NULL;
+
+err = label_get_child_label(socket_fd, 
command-path, label);
+if (err  0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+
+err = setexeccon(label);
+if (err  0) {
+r = 

Re: [systemd-devel] Condition* options linked by AND if stated more than once

[Michal Sekletar]

On Thu, Aug 07, 2014 at 01:23:34PM +0200, Peter Mattern wrote:
 If one of these options gets stated more than once the different
 instances seem to be linked by a logical AND, too. This prevents
 overwriting these options via snippets in /etc, e. g.
 systemd-timesyncd.service still won't run in KVM with a snippet
 /etc/systemd/system/systemd-timesyncd.service.d/do-run-in-kvm.conf
 stating
  [Unit]
  ConditionVirtualization=kvm

What you could do in a snippet is to cancel the previous assignment and define
new value for the key. Following should work

[Unit]
ConditionVirtualization=
ConditionVirtualization=kvm

 
 Seen on Arch Linux, systemd 215-4, tested
 Condition{Architecure,Host,Virtualization}.
 
 Any thoughts?
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] socket: introduce SELinuxLabeledNet option

[Michal Sekletar]

This makes possible to spawn service instances triggered by socket with
MLS/MCS SELinux labels which are created based on information provided by
connected peer.

Implementation of label_get_socket_label derived from xinetd.
---
 man/systemd.socket.xml| 11 ++
 src/core/execute.c| 23 +++-
 src/core/execute.h|  1 +
 src/core/load-fragment-gperf.gperf.m4 |  3 ++
 src/core/socket.c | 18 +++--
 src/core/socket.h |  2 +
 src/shared/label.c| 69 +++
 src/shared/label.h|  1 +
 8 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 09a7311..959f46f 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -588,6 +588,17 @@
 /varlistentry
 
 varlistentry
+  termvarnameSELinuxLabeledNet=/varname/term
+  listitemparaTakes a boolean
+  value. Controls whether systemd attempts to figure 
out
+  SELinux label used for instantiated service from
+  information handed by peer over the
+  network. Configuration option has effect only
+  on sockets with literalAccept=/literal
+  mode set to literalyes/literal./para/listitem
+/varlistentry
+
+varlistentry
 termvarnamePipeSize=/varname/term
 listitemparaTakes a size in
 bytes. Controls the pipe buffer size
diff --git a/src/core/execute.c b/src/core/execute.c
index d8452a6..ec5f878 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -83,6 +83,7 @@
 #include af-list.h
 #include mkdir.h
 #include apparmor-util.h
+#include label.h
 
 #ifdef HAVE_SECCOMP
 #include seccomp-util.h
@@ -1729,6 +1730,22 @@ int exec_spawn(ExecCommand *command,
 goto fail_child;
 }
 }
+
+if (context-selinux_labeled_net  use_selinux()) {
+_cleanup_free_ char *label = NULL;
+
+err = label_get_socket_label(socket_fd, 
command-path, label);
+if (err  0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+
+err = setexeccon(label);
+if (err  0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+}
 #endif
 
 #ifdef HAVE_APPARMOR
@@ -2112,7 +2129,8 @@ void exec_context_dump(ExecContext *c, FILE* f, const 
char *prefix) {
 %sPrivateDevices: %s\n
 %sProtectHome: %s\n
 %sProtectSystem: %s\n
-%sIgnoreSIGPIPE: %s\n,
+%sIgnoreSIGPIPE: %s\n
+%sSELinuxLabeledNet: %s\n,
 prefix, c-umask,
 prefix, c-working_directory ? c-working_directory : /,
 prefix, c-root_directory ? c-root_directory : /,
@@ -2122,7 +2140,8 @@ void exec_context_dump(ExecContext *c, FILE* f, const 
char *prefix) {
 prefix, yes_no(c-private_devices),
 prefix, protect_home_to_string(c-protect_home),
 prefix, protect_system_to_string(c-protect_system),
-prefix, yes_no(c-ignore_sigpipe));
+prefix, yes_no(c-ignore_sigpipe),
+prefix, yes_no(c-selinux_labeled_net));
 
 STRV_FOREACH(e, c-environment)
 fprintf(f, %sEnvironment: %s\n, prefix, *e);
diff --git a/src/core/execute.h b/src/core/execute.h
index 9d05d3a..e7d0bdd 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -136,6 +136,7 @@ struct ExecContext {
 
 bool selinux_context_ignore;
 char *selinux_context;
+bool selinux_labeled_net;
 
 bool apparmor_profile_ignore;
 char *apparmor_profile;
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index d70f9ee..3c3f408 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -257,6 +257,9 @@ Socket.SmackLabelIPOut,  config_parse_string,   
 0,
 `Socket.SmackLabel,  config_parse_warn_compat,   0,
 0
 Socket.SmackLabelIPIn,   config_parse_warn_compat,   0,
 0
 Socket.SmackLabelIPOut,  

Re: [systemd-devel] Support for pre-restart check

[Michal Sekletar]

On Wed, Jul 30, 2014 at 11:26:38AM +, Jóhann B. Guðmundsson wrote:
 
 On 07/30/2014 10:22 AM, Ansgar Burchardt wrote:
 Hi,
 
 while looking at how to convert sysvinit scripts to service files in
 Debian, I noticed that some scripts do an additional sanity check during
 the restart action. That is, they run an extra command to validate the
 configuration and only restart the service on success.
 
 Systemd doesn't seem to provide such an option so far. Would you think
 this is something one would like to see? If yes, what about the
 following idea:
 
 Add an ExecRestartPre option in the [Service] section. These commands
 are executed when a restart is requested, before the service is stopped.
 If they fail, the restart is aborted with an error.
 
 
 Re-sending to list due to simple clicking reply not reply-list mistake.
 
 This has been discussed before and check the archive for that
 discussion but the bottom line is that this is not needed we already
 have ExecStartPre=, ExecStartPost=, ExecStop=, ExecStopPost=, or
 ExecReload= which cover those usecases

If that is the case, we should really delete TODO item:

* maybe introduce ExecRestartPre

Michal
 
 JBG
 ___
 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] journald: fix syslog facility for messages coming from kmsg

[Michal Sekletar]

On Sun, Jul 27, 2014 at 07:57:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
 On Sun, Jul 27, 2014 at 01:11:07PM +0200, Michal Sekletar wrote:
  On Sat, Jul 26, 2014 at 09:11:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
   Hm, what was wrong with the facility before?
  
  I think that we should always add SYSLOG_FACILITY field. Now, if (priority 
  LOG_FACMASK) == LOG_KERN we don't do that. We set SYSLOG_IDENTIFIER to 
  kernel
  tough. But missing SYSLOG_FACILITY field confuses some tools, most notably
  rsyslog, thus rsyslog filters like kern.* /var/log/kernel.log doesn't 
  work and
  rsyslog don't output kernel log messages to /var/log/kernel.log.
 
 Agreed, even though for slightly different reason :)
 syslog(2) describes LOG_KERN as the facility for kernel messages, so we should
 set it so. So I'm +1 for pushing this change, but *please* add the explanation
 to the commit message. It makes it much easier for downstreams, especially for
 people who see the commit in isolation on some stable branch a few months down
 the road.

Pushed, with explanation. Thanks!

 
 Zbyszek
 
   On Fri, Jul 25, 2014 at 03:04:44PM +0200, Michal Sekletar wrote:
---
 src/journal/journald-kmsg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
index 12992e7..8d24344 100644
--- a/src/journal/journald-kmsg.c
+++ b/src/journal/journald-kmsg.c
@@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, 
size_t l) {
 if (asprintf(syslog_priority, PRIORITY=%i, priority  
LOG_PRIMASK) = 0)
 IOVEC_SET_STRING(iovec[n++], syslog_priority);
 
+if (asprintf(syslog_facility, SYSLOG_FACILITY=%i, priority 
 LOG_FACMASK) = 0)
+IOVEC_SET_STRING(iovec[n++], syslog_facility);
+
 if ((priority  LOG_FACMASK) == LOG_KERN)
 IOVEC_SET_STRING(iovec[n++], 
SYSLOG_IDENTIFIER=kernel);
 else {
@@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, 
size_t l) {
 if (syslog_pid)
 IOVEC_SET_STRING(iovec[n++], 
syslog_pid);
 }
-
-if (asprintf(syslog_facility, SYSLOG_FACILITY=%i, 
LOG_FAC(priority)) = 0)
-IOVEC_SET_STRING(iovec[n++], syslog_facility);
 }
 
 message = cunescape_length_with_prefix(p, pl, MESSAGE=);
-- 
2.0.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] journald: fix syslog facility for messages coming from kmsg

[Michal Sekletar]

On Sat, Jul 26, 2014 at 09:11:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
 Hm, what was wrong with the facility before?

I think that we should always add SYSLOG_FACILITY field. Now, if (priority 
LOG_FACMASK) == LOG_KERN we don't do that. We set SYSLOG_IDENTIFIER to kernel
tough. But missing SYSLOG_FACILITY field confuses some tools, most notably
rsyslog, thus rsyslog filters like kern.* /var/log/kernel.log doesn't work and
rsyslog don't output kernel log messages to /var/log/kernel.log.

Michal

 Zbyszek
 
 On Fri, Jul 25, 2014 at 03:04:44PM +0200, Michal Sekletar wrote:
  ---
   src/journal/journald-kmsg.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
  
  diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
  index 12992e7..8d24344 100644
  --- a/src/journal/journald-kmsg.c
  +++ b/src/journal/journald-kmsg.c
  @@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, size_t 
  l) {
   if (asprintf(syslog_priority, PRIORITY=%i, priority  
  LOG_PRIMASK) = 0)
   IOVEC_SET_STRING(iovec[n++], syslog_priority);
   
  +if (asprintf(syslog_facility, SYSLOG_FACILITY=%i, priority  
  LOG_FACMASK) = 0)
  +IOVEC_SET_STRING(iovec[n++], syslog_facility);
  +
   if ((priority  LOG_FACMASK) == LOG_KERN)
   IOVEC_SET_STRING(iovec[n++], SYSLOG_IDENTIFIER=kernel);
   else {
  @@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, size_t 
  l) {
   if (syslog_pid)
   IOVEC_SET_STRING(iovec[n++], syslog_pid);
   }
  -
  -if (asprintf(syslog_facility, SYSLOG_FACILITY=%i, 
  LOG_FAC(priority)) = 0)
  -IOVEC_SET_STRING(iovec[n++], syslog_facility);
   }
   
   message = cunescape_length_with_prefix(p, pl, MESSAGE=);
  -- 
  2.0.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

[systemd-devel] [PATCH] journald: fix syslog facility for messages coming from kmsg

[Michal Sekletar]

---
 src/journal/journald-kmsg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
index 12992e7..8d24344 100644
--- a/src/journal/journald-kmsg.c
+++ b/src/journal/journald-kmsg.c
@@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, size_t l) {
 if (asprintf(syslog_priority, PRIORITY=%i, priority  LOG_PRIMASK) 
= 0)
 IOVEC_SET_STRING(iovec[n++], syslog_priority);
 
+if (asprintf(syslog_facility, SYSLOG_FACILITY=%i, priority  
LOG_FACMASK) = 0)
+IOVEC_SET_STRING(iovec[n++], syslog_facility);
+
 if ((priority  LOG_FACMASK) == LOG_KERN)
 IOVEC_SET_STRING(iovec[n++], SYSLOG_IDENTIFIER=kernel);
 else {
@@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, size_t l) {
 if (syslog_pid)
 IOVEC_SET_STRING(iovec[n++], syslog_pid);
 }
-
-if (asprintf(syslog_facility, SYSLOG_FACILITY=%i, 
LOG_FAC(priority)) = 0)
-IOVEC_SET_STRING(iovec[n++], syslog_facility);
 }
 
 message = cunescape_length_with_prefix(p, pl, MESSAGE=);
-- 
2.0.1

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

Re: [systemd-devel] [PATCH] units: make ExecStopPost action part of ExecStart

[Michal Sekletar]

If there are no further objections I will push the patch as is.

Michal

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

[systemd-devel] [PATCH] units: make ExecStopPost action part of ExecStart

[Michal Sekletar]

Currently after exiting rescue shell we isolate default target. User
might want to isolate to some other target than default one. However
issuing systemctl isolate command to desired target would bring system
to default target as a consequence of running ExecStopPost action.

Having common ancestor for rescue shell and possible followup systemctl
default command should fix this. If user exits rescue shell we will
proceed with isolating default target, otherwise, on manual isolate,
parent shell process is terminated and we don't isolate default target,
but target chosen by user.

Suggested-by: Michal Schmidt mschm...@redhat.com
---
 units/emergency.service.in | 3 +--
 units/rescue.service.m4.in | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/units/emergency.service.in b/units/emergency.service.in
index 94c090f..91fc1bb 100644
--- a/units/emergency.service.in
+++ b/units/emergency.service.in
@@ -17,8 +17,7 @@ Environment=HOME=/root
 WorkingDirectory=/root
 ExecStartPre=-/bin/plymouth quit
 ExecStartPre=-/bin/echo -e 'Welcome to emergency mode! After logging in, type 
journalctl -xb to view\\nsystem logs, systemctl reboot to reboot, 
systemctl default to try again\\nto boot into default mode.'
-ExecStart=-/sbin/sulogin
-ExecStopPost=@SYSTEMCTL@ --fail --no-block default
+ExecStart=-/bin/sh -c /sbin/sulogin; @SYSTEMCTL@ --fail --no-block default
 Type=idle
 StandardInput=tty-force
 StandardOutput=inherit
diff --git a/units/rescue.service.m4.in b/units/rescue.service.m4.in
index 552ef89..ef54369 100644
--- a/units/rescue.service.m4.in
+++ b/units/rescue.service.m4.in
@@ -18,8 +18,7 @@ Environment=HOME=/root
 WorkingDirectory=/root
 ExecStartPre=-/bin/plymouth quit
 ExecStartPre=-/bin/echo -e 'Welcome to rescue mode! Type systemctl default 
or ^D to enter default mode.\\nType journalctl -xb to view system logs. Type 
systemctl reboot to reboot.'
-ExecStart=-/sbin/sulogin
-ExecStopPost=-@SYSTEMCTL@ --fail --no-block default
+ExecStart=-/bin/sh -c /sbin/sulogin; @SYSTEMCTL@ --fail --no-block default
 Type=idle
 StandardInput=tty-force
 StandardOutput=inherit
-- 
2.0.0

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

Re: [systemd-devel] [PATCH 1/2] dhcp-network: add check for DHCP.chaddr

[Michal Sekletar]

On Tue, Jun 24, 2014 at 03:54:54PM +0200, Tom Gundersen wrote:
 On Tue, Jun 24, 2014 at 12:58 PM, Michal Sekletar msekl...@redhat.com wrote:
  On Thu, Jun 19, 2014 at 12:02:39PM -0500, Dan Williams wrote:
  On Thu, 2014-06-19 at 18:59 +0200, Michal Sekletar wrote:
   Check that received DHCP packets actually include our MAC address in
   chaddr field. BPF interpreter has 32 bit wide registers but MAC address
   is 48 bits long so we have to do check in two steps.
 
  And if the MAC address is not 48 bits (Infiniband) or the interface type
  doesn't really have a MAC address (PPP/PPPoE)?  DHCP can be used in both
  cases...  If anything, the code that uses the MAC address should be
  if-ed so it only executes when a MAC address is provided.
 
  I reconsidered. In filter code I can very easily find out type of device to 
  which
  filter is attached and skip ethernet specific bits or fallback to something
  else. However filter already assumes ethernet data-link type moreover there 
  is a ton of
  code in networkd which does likewise.
 
  I think we should merge patch as is right now. If somebody will hack up 
  patches
  for supporting various data-link types I will be happy to patch filter
  accordingly.
 
 Ok. I'm fine with that. Please push.

Applied!

 
 Just to make clear: I'd be happy if anyone wanted to turn all our libs
 to work also on non-ethernet (and test it!), and would merge such
 patches.
 
 Cheers,
 
 Tom
 
src/libsystemd-network/dhcp-internal.h|  3 ++-
src/libsystemd-network/dhcp-network.c | 15 +--
src/libsystemd-network/sd-dhcp-client.c   |  4 ++--
src/libsystemd-network/test-dhcp-client.c |  2 +-
4 files changed, 18 insertions(+), 6 deletions(-)
  
   diff --git a/src/libsystemd-network/dhcp-internal.h 
   b/src/libsystemd-network/dhcp-internal.h
   index 03cc824..fd86a70 100644
   --- a/src/libsystemd-network/dhcp-internal.h
   +++ b/src/libsystemd-network/dhcp-internal.h
   @@ -24,12 +24,13 @@
  
#include stdint.h
#include linux/if_packet.h
   +#include net/ethernet.h
  
#include socket-util.h
  
#include dhcp-protocol.h
  
   -int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, 
   uint32_t xid);
   +int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, 
   uint32_t xid, struct ether_addr mac_addr);
int dhcp_network_bind_udp_socket(be32_t address, uint16_t port);
int dhcp_network_send_raw_socket(int s, const union sockaddr_union 
   *link,
 const void *packet, size_t len);
   diff --git a/src/libsystemd-network/dhcp-network.c 
   b/src/libsystemd-network/dhcp-network.c
   index d7ba0ef..e5fd6a8 100644
   --- a/src/libsystemd-network/dhcp-network.c
   +++ b/src/libsystemd-network/dhcp-network.c
   @@ -33,7 +33,7 @@
#include dhcp-internal.h
  
int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link,
   - uint32_t xid) {
   + uint32_t xid, struct ether_addr 
   mac_addr) {
struct sock_filter filter[] = {
BPF_STMT(BPF_LD + BPF_W + BPF_LEN, 0),  
  /* A - packet length */
BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(DHCPPacket), 1, 
   0), /* packet = DHCPPacket ? */
   @@ -56,7 +56,18 @@ int dhcp_network_bind_raw_socket(int index, union 
   sockaddr_union *link,
BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, 
   dhcp.xid)),/* A - client identifier */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, xid, 1, 0), 
  /* client identifier == xid ? */
BPF_STMT(BPF_RET + BPF_K, 0),   
  /* ignore */
   -/* TODO: match chaddr */
   +BPF_STMT(BPF_LD + BPF_IMM, htobe32(*((unsigned int *) 
   mac_addr))),/* A - 4 bytes of client's MAC */
   +BPF_STMT(BPF_MISC + BPF_TAX, 0),
  /* X - A */
   +BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, 
   dhcp.chaddr)), /* A - 4 bytes of MAC from dhcp.chaddr */
   +BPF_STMT(BPF_ALU + BPF_XOR + BPF_X, 0), 
  /* A xor X */
   +BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),   
  /* A == 0 ? */
   +BPF_STMT(BPF_RET + BPF_K, 0),   
  /* ignore */
   +BPF_STMT(BPF_LD + BPF_IMM, htobe16(*((unsigned short *) 
   (((char *) mac_addr) + 4, /* A - remainder of client's MAC */
   +BPF_STMT(BPF_MISC + BPF_TAX, 0),
  /* X - A */
   +BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, 
   dhcp.chaddr) + 4), /* A - remainder of MAC

[systemd-devel] [PATCH] dhcp-network: ignore IP packets with More Fragments (MF) flag set

[Michal Sekletar]

We already ignore IP fragments, because we expect that Fragment
offset (FO) field is not set. However first fragment in a fragmented IP
flow will have all zeroes in FO field. We should ignore such packet as
well, thus we need to look at MF flag in the IP header. Checking MF flag
will filter out all except last packet in fragmented flows. Last one
will be ruled out by next check for value of FO.
---
 src/libsystemd-network/dhcp-network.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/libsystemd-network/dhcp-network.c 
b/src/libsystemd-network/dhcp-network.c
index f119cae..455d5a8 100644
--- a/src/libsystemd-network/dhcp-network.c
+++ b/src/libsystemd-network/dhcp-network.c
@@ -41,6 +41,10 @@ int dhcp_network_bind_raw_socket(int index, union 
sockaddr_union *link,
 BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, 
ip.protocol)), /* A - IP protocol */
 BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 1, 0),
/* IP protocol == UDP ? */
 BPF_STMT(BPF_RET + BPF_K, 0),  
/* ignore */
+BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, 
ip.frag_off)), /* A - Flags */
+BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x20), 
/* A - A  0x20 */
+BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),  
/* A == 0 ? */
+BPF_STMT(BPF_RET + BPF_K, 0),  
/* ignore */
 BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, 
ip.frag_off)), /* A - Flags + Fragment offset */
 BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x1fff),   
/* A - A  0x1fff */
 BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),  
/* A == 0 ? */
-- 
2.0.0

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

Re: [systemd-devel] [PATCH] localed: search locale settings on kernel cmdline first

[Michal Sekletar]

On Wed, Jul 02, 2014 at 03:05:36AM +0200, Zbigniew Jędrzejewski-Szmek wrote:
 On Tue, Jul 01, 2014 at 04:36:47PM +0200, Lennart Poettering wrote:
  On Tue, 01.07.14 16:47, microcai (micro...@fedoraproject.org) wrote:
  
Maybe another option is to improve localectl on the client side to
simply warn the user if there's something on the kernel cmdline
specified? (this check should probably test this directly, bypassing
localed, and thus get skipped if localectl is used on a remote host).

i.e. just a simple warning if you type localectl that says: Warning:
Locale configuration has been specified on the kernel command line,
overriding the system settings below. or so...
   
   kernel has BOOT time, etc settings has modified time, which one is newer, 
   take 
   that ?
  
  I am pretty sure we should stay with the simple logic of kernel
  cmdline overrides /etc. Trying to be smarter and saying newer /etc
  overrides older /proc/cmdline will just be confusing, and has not been
  used anywhere so far...
 One of the nice things about localectl set-locale was that it acted
 immediately. We should keep this property. So I think that the patch
 as is causes a usability regression, for things which are not started
 by systemd, and use /etc/locale.conf instead.
 
 OTOH, the difference in behaviour between systemd and localed is annoying
 and potentially confusing.
 
 I think we should
 
 1. make systemd and localed use the same logic, which follows the line
 proposed by microcai, so that a newer /etc/locale.conf wins over the
 commandline, which wins over an older /etc/locale.conf.
 
 2. make localectl generate a warning if anything is set on the commandline,
 that those settings will take precedence after reboot.
 
 This will keep the possibility to use dynamic configuration, and give
 the admin a hint and a chance to update or clean up the commandline.
 
 I don't think that the argument that /proc/cmdline always wins is valid.
 For example root filesystem settings, logical volumes, the default unit,
 additional units to get started, filesystems to be mounted, are all things
 which are often dynamically altered at runtime. I don't think we should
 sacrifice usability for narrow consistency.

I agree with above points.

 
 Zbyszek

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

Re: [systemd-devel] [PATCH 1/2] dhcp-network: add check for DHCP.chaddr

[Michal Sekletar]

On Thu, Jun 19, 2014 at 12:02:39PM -0500, Dan Williams wrote:
 On Thu, 2014-06-19 at 18:59 +0200, Michal Sekletar wrote:
  Check that received DHCP packets actually include our MAC address in
  chaddr field. BPF interpreter has 32 bit wide registers but MAC address
  is 48 bits long so we have to do check in two steps.
 
 And if the MAC address is not 48 bits (Infiniband) or the interface type
 doesn't really have a MAC address (PPP/PPPoE)?  DHCP can be used in both
 cases...  If anything, the code that uses the MAC address should be
 if-ed so it only executes when a MAC address is provided.

I reconsidered. In filter code I can very easily find out type of device to 
which
filter is attached and skip ethernet specific bits or fallback to something
else. However filter already assumes ethernet data-link type moreover there is 
a ton of
code in networkd which does likewise.

I think we should merge patch as is right now. If somebody will hack up patches
for supporting various data-link types I will be happy to patch filter
accordingly.
 
 
 Dan

Michal

 
  ---
   src/libsystemd-network/dhcp-internal.h|  3 ++-
   src/libsystemd-network/dhcp-network.c | 15 +--
   src/libsystemd-network/sd-dhcp-client.c   |  4 ++--
   src/libsystemd-network/test-dhcp-client.c |  2 +-
   4 files changed, 18 insertions(+), 6 deletions(-)
  
  diff --git a/src/libsystemd-network/dhcp-internal.h 
  b/src/libsystemd-network/dhcp-internal.h
  index 03cc824..fd86a70 100644
  --- a/src/libsystemd-network/dhcp-internal.h
  +++ b/src/libsystemd-network/dhcp-internal.h
  @@ -24,12 +24,13 @@
   
   #include stdint.h
   #include linux/if_packet.h
  +#include net/ethernet.h
   
   #include socket-util.h
   
   #include dhcp-protocol.h
   
  -int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, 
  uint32_t xid);
  +int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, 
  uint32_t xid, struct ether_addr mac_addr);
   int dhcp_network_bind_udp_socket(be32_t address, uint16_t port);
   int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
const void *packet, size_t len);
  diff --git a/src/libsystemd-network/dhcp-network.c 
  b/src/libsystemd-network/dhcp-network.c
  index d7ba0ef..e5fd6a8 100644
  --- a/src/libsystemd-network/dhcp-network.c
  +++ b/src/libsystemd-network/dhcp-network.c
  @@ -33,7 +33,7 @@
   #include dhcp-internal.h
   
   int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link,
  - uint32_t xid) {
  + uint32_t xid, struct ether_addr mac_addr) 
  {
   struct sock_filter filter[] = {
   BPF_STMT(BPF_LD + BPF_W + BPF_LEN, 0), 
  /* A - packet length */
   BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(DHCPPacket), 1, 0), 
  /* packet = DHCPPacket ? */
  @@ -56,7 +56,18 @@ int dhcp_network_bind_raw_socket(int index, union 
  sockaddr_union *link,
   BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, 
  dhcp.xid)),/* A - client identifier */
   BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, xid, 1, 0),
  /* client identifier == xid ? */
   BPF_STMT(BPF_RET + BPF_K, 0),  
  /* ignore */
  -/* TODO: match chaddr */
  +BPF_STMT(BPF_LD + BPF_IMM, htobe32(*((unsigned int *) 
  mac_addr))),/* A - 4 bytes of client's MAC */
  +BPF_STMT(BPF_MISC + BPF_TAX, 0),   
  /* X - A */
  +BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, 
  dhcp.chaddr)), /* A - 4 bytes of MAC from dhcp.chaddr */
  +BPF_STMT(BPF_ALU + BPF_XOR + BPF_X, 0),
  /* A xor X */
  +BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),  
  /* A == 0 ? */
  +BPF_STMT(BPF_RET + BPF_K, 0),  
  /* ignore */
  +BPF_STMT(BPF_LD + BPF_IMM, htobe16(*((unsigned short *) 
  (((char *) mac_addr) + 4, /* A - remainder of client's MAC */
  +BPF_STMT(BPF_MISC + BPF_TAX, 0),   
  /* X - A */
  +BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, 
  dhcp.chaddr) + 4), /* A - remainder of MAC from dhcp.chaddr */
  +BPF_STMT(BPF_ALU + BPF_XOR + BPF_X, 0),
  /* A xor X */
  +BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),  
  /* A == 0 ? */
  +BPF_STMT(BPF_RET + BPF_K, 0),  
  /* ignore */
   BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof

[systemd-devel] [PATCH] systemctl: don't return success if we refused to execute user's request

[Michal Sekletar]

---
 src/systemctl/systemctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 727be58..2dac67c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -6644,7 +6644,7 @@ found:
 } else {
 if (running_in_chroot()  0) {
 log_info(Running in chroot, ignoring request.);
-return 0;
+return -ENOTSUP;
 }
 
 if ((verb-bus != FORCE || arg_force = 0)  !bus) {
@@ -6895,7 +6895,7 @@ int main(int argc, char*argv[]) {
 
 if (running_in_chroot()  0  arg_action != ACTION_SYSTEMCTL) {
 log_info(Running in chroot, ignoring request.);
-r = 0;
+r = -ENOTSUP;
 goto finish;
 }
 
-- 
2.0.0

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

Re: [systemd-devel] [PATCH] systemctl: don't return success if we refused to execute user's request

[Michal Sekletar]

On Mon, Jun 23, 2014 at 02:46:42PM +0200, Lennart Poettering wrote:
 On Mon, 23.06.14 13:34, Michal Sekletar (msekl...@redhat.com) wrote:
 
 What's the rationale?

This came up while discussing the issue when some script saved list of actions
it took and same script could be called to undo those actions on the system. Of
course reporter of this issue saw logs that some of undo actions are failing
because they actually never succeeded because script in question was called from
%post during kickstarted installation. Related bugzilla where they are
requesting proposed change:

https://bugzilla.redhat.com/show_bug.cgi?id=1110675

 
 So far the logic always was to turn systemctl invocations into NOPs so
 that packaging scripts in invoked in chroot() environments won't
 fail. Why do you want them to fail? This sounds wrong?

Thinking about this once again it doesn't seem as such a good idea now. Not
sure really...

 
  ---
   src/systemctl/systemctl.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
  index 727be58..2dac67c 100644
  --- a/src/systemctl/systemctl.c
  +++ b/src/systemctl/systemctl.c
  @@ -6644,7 +6644,7 @@ found:
   } else {
   if (running_in_chroot()  0) {
   log_info(Running in chroot, ignoring request.);
  -return 0;
  +return -ENOTSUP;
   }
   
   if ((verb-bus != FORCE || arg_force = 0)  !bus) {
  @@ -6895,7 +6895,7 @@ int main(int argc, char*argv[]) {
   
   if (running_in_chroot()  0  arg_action != ACTION_SYSTEMCTL) {
   log_info(Running in chroot, ignoring request.);
  -r = 0;
  +r = -ENOTSUP;
   goto finish;
   }
   
 
 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd-networkd: no network connectivity with 214/master due to 63a070415db09f5b5bcc5c

[Michal Sekletar]

On Wed, Jun 18, 2014 at 09:51:02PM +0200, Friedrich Kröner wrote:
 Hello,
 
 when trying systemd-networkd with =214 and the following config:
 
 [Match]
 Name=eth*
 
 [Network]
 DHCP=yes
 
 Address=2001:db8::1234:5678/64
 DNS=8.8.8.8
 DNS=2001:db8:1::ab9:C0A8:102
 
 
 I get lots of DHCP DISCOVER events and it doesn't aquire an IPv4 address, nor 
 sets the configured IPv6.
 
 Jun 18 18:44:31 localhost systemd[1]: Starting Network Service...
 Jun 18 18:44:31 localhost systemd-networkd[19468]: timestamp of 
 '/etc/systemd/network' changed
 Jun 18 18:44:31 localhost systemd-networkd[19468]: sd-rtnl: discarding 20 
 bytes of incoming message
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: link 3 
 added
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: udev 
 initialized link
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: flags 
 change: +UP +LOWER_UP +RUNNING +MULTICAST +BROADCAST
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: gained 
 carrier
 Jun 18 18:44:31 localhost systemd-networkd[19468]: could not add new link
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : link 1 
 added
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : udev 
 initialized link
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : flags 
 change: +LOOPBACK +UP +LOWER_UP +RUNNING
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : gained 
 carrier
 Jun 18 18:44:31 localhost systemd[1]: Started Network Service.
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: link 
 state is up-to-date
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: found 
 matching network '/etc/systemd/network/80-dhcp.network'
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: 
 acquiring 
 DHCPv4 lease
 Jun 18 18:44:31 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 STARTED on ifindex 3 with address 52:54:e4:d2:24:44
 Jun 18 18:44:31 localhost systemd-networkd[19468]: Sent message 
 type=method_call sender=n/a destination=org.freedesktop.DBus 
 object=/org/freedesktop/DBus interface=org.freedesktop.DBus member=Hello 
 cookie=1 reply_cookie=0 error=n/a
 Jun 18 18:44:31 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : getting 
 address failed: Device or resource busy
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : link 
 state is up-to-date
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : unmanaged
 Jun 18 18:44:31 localhost systemd-networkd[19468]: sd-rtnl: discarding 20 
 bytes of incoming message
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: added 
 address: fe80::5054:e4ff:fed2:2444/64
 Jun 18 18:44:31 localhost systemd-networkd[19468]: rtnl: received address for 
 a nonexistent link, ignoring
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : added 
 address: ::1/128
 Jun 18 18:44:31 localhost systemd-networkd[19468]: eth0: added 
 address: 123.23.23.21/22
 Jun 18 18:44:31 localhost systemd-networkd[19468]: lo  : added 
 address: 127.0.0.1/8
 Jun 18 18:44:31 localhost systemd-networkd[19468]: Got message 
 type=method_return sender=org.freedesktop.DBus destination=:1.45 object=n/a 
 interface=n/a member=n/a cookie=1 reply_cookie=1 error=n/a 
 Jun 18 18:44:31 localhost systemd-networkd[19468]: Got message type=signal 
 sender=org.freedesktop.DBus destination=:1.45 object=/org/freedesktop/DBus 
 interface=org.freedesktop.DBus member=NameAcquired cookie=2 reply_cookie=0 
 error=n/a
 Jun 18 18:44:33 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:44:34 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:44:38 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:44:46 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:45:02 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:45:33 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:46:36 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:47:41 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 Jun 18 18:48:44 localhost systemd-networkd[19468]: DHCP CLIENT (0x3ddf8f7): 
 DISCOVER
 
 reverting commit 63a070415db09f5b5bcc5c sd-dhcp-client: allways request 
 broadcast 
 restores the previous behavior. Upon restart of systemd-networkd I get the 
 usual OFFER, REQUEST, ACK confirmation and the IPv6 gets set as well.
 
 This is on a kvm-machine with virtio_net as module.
 
 Please let me know if you want me to test anything or need further 
 information.

What DHCP server do you use? I was running networkd with 63a070415db09f5b5bcc5c
included and was able to 

Re: [systemd-devel] [PATCH] localed: consider an unset model as a wildcard

[Michal Sekletar]

On Fri, May 30, 2014 at 06:20:16PM +0200, Michal Sekletar wrote:

ping?

 ---
  src/locale/localed.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)
 
 diff --git a/src/locale/localed.c b/src/locale/localed.c
 index e3061c8..358f6c2 100644
 --- a/src/locale/localed.c
 +++ b/src/locale/localed.c
 @@ -712,15 +712,16 @@ static int find_legacy_keymap(Context *c, char 
 **new_keymap) {
  }
  }
  
 -if (matching  0 
 -streq_ptr(c-x11_model, a[2])) {
 -matching++;
 -
 -if (streq_ptr(c-x11_variant, a[3])) {
 +if (matching  0) {
 +if (isempty(c-x11_model) || streq_ptr(c-x11_model, 
 a[2])) {
  matching++;
  
 -if (streq_ptr(c-x11_options, a[4]))
 +if (streq_ptr(c-x11_variant, a[3])) {
  matching++;
 +
 +if (streq_ptr(c-x11_options, a[4]))
 +matching++;
 +}
  }
  }
  
 -- 
 1.9.0
 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel