Re: [systemd-devel] [HEADS-UP] hwdb: change 60-keyboard.hwdb matches

2015-03-26 Thread Martin Pitt
Hey David,

(sorry for late reply, back from holidays)

David Herrmann [2015-03-16 12:31 +0100]:
 I've pushed some patches that change the hwdb 60-keyboard matching
 logic. This was needed to support bluetooth devices, and then I just
 went ahead and cleaned up the other rules. This email is meant as a
 heads-up to you guys, as you are the most frequent contributors to
 that file.

Thanks for the heads-up!

 Two notes:
 
 1) Both of you added keyboard:usb:* matches that match on the USB
 interface. This was now dropped in:
   
 http://cgit.freedesktop.org/systemd/systemd/commit/?id=b26e4ced91d0ac0eabdce1c505228ccafc65a23f
 If this was intentional, we can re-add it, but it looked like a
 safety-net to me, rather than an explicit interface match. Please let
 me know if it is required!

Some USB devices (Logitech and/or Microsoft, I don't remember any
more) do need an interface match; e. g. some USB keyboards also
include a mouse/touchpad and thus expose two input devices, and we
must ensure to apply the keymap to the keyboard, not to the mouse.
However, this does not apply to the Compaq ones, so this commit looks
ok.

It indeed makes much more sense to match on the input devices than on
the USB ones, so handling the multi-function device case should be
much easier now.

 2) The keyboard:dmi:* matches are now merged with the platform
 fallback, as the platform-fallback is a superset of the dmi-match:
   
 http://cgit.freedesktop.org/systemd/systemd/commit/?id=ba76ee29bc02879fb42c048132af8889b00220d5

Looks fine.

Thanks!

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Most important changes the last 15 months

2015-03-26 Thread Lennart Poettering
On Mon, 16.03.15 08:05, Cecil Westerhof (cldwester...@gmail.com) wrote:

 About 15 months ago I gave a presentation about systemd/journald. I am
 asked to give another presentation about systemd/journald. What are the
 most important changes I should integrate into my presentation?

http://cgit.freedesktop.org/systemd/systemd/tree/NEWS

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] parsing audit messages

2015-03-26 Thread Lennart Poettering
On Sun, 15.03.15 03:51, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 On Sun, Mar 15, 2015 at 03:49:07AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
  Hi,
  
  I was looking at some debug logs, and the audit messages are
  semi-useless in their current undecoded form:
  
  mar 14 22:24:02 fedora22 audit[1]: audit-1130 pid=1 uid=0 auid=4294967295 
  ses=4294967295 subj=system_u:system_r:init_t:s0 
  msg='unit=systemd-udev-trigger comm=systemd 
  exe=/usr/lib/systemd/systemd hostname=? addr=? terminal=? res=success'
  mar 14 22:24:05 fedora22 audit: audit-1327 
  proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D0069707461626C655F7365637572697479
  
  You added code to parse this, and I think we should make use of it and
  put msg= field as MESSAGE=, and maybe store the original message as
  _AUDIT= or something. If there's no msg field, like with proctitle,
  print all fields that are in the message, but using our cescape, and
  not this hexadecimal form which is unreadable for humans.
 
 I think we should also translate type= to names...
 
 https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Security_Guide/sec-Audit_Record_Types.html

Well, we don't translate MESSAGE_ID fields to strings either...

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] parsing audit messages

2015-03-26 Thread Lennart Poettering
On Sun, 15.03.15 03:49, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 Hi,
 
 I was looking at some debug logs, and the audit messages are
 semi-useless in their current undecoded form:
 
 mar 14 22:24:02 fedora22 audit[1]: audit-1130 pid=1 uid=0 auid=4294967295 
 ses=4294967295 subj=system_u:system_r:init_t:s0 
 msg='unit=systemd-udev-trigger comm=systemd exe=/usr/lib/systemd/systemd 
 hostname=? addr=? terminal=? res=success'
 mar 14 22:24:05 fedora22 audit: audit-1327 
 proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D0069707461626C655F7365637572697479
 
 You added code to parse this, and I think we should make use of it and
 put msg= field as MESSAGE=, and maybe store the original message as
 _AUDIT= or something. If there's no msg field, like with proctitle,
 print all fields that are in the message, but using our cescape, and
 not this hexadecimal form which is unreadable for humans.
 
 Thoughts?

Well msg= is just where they place the userspace message, if it is a
userspace generated message. It is little more than a separator
between the kernel generated and userspace generated parts of the
message. The userspace message is generally not more or less human
readable than the whole message I fear...

I am all for making the audit parsing logic smarter, but I don't see
how that's possible, the kernel generated format is a complete
disaster, the people who wrote that had no concept at all of computer
security, and its' impossible to parse fully correctly without
heuristics.

For example, if we encounter 2proctitle=41 in the message, we simply
don't know whether this is actually a process called 41, or just the
hex encoded process name A... The formatting is not reversible. It's
complete rubbish.

It's an embarassment for the kernel community that a technology like
audit -- that is supposed to improve security -- is so vulnerable to the
most trivial script-kiddy attacks!

I am not sure we can do much about this really...

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] CODING_STYLE: this also help with unaligned memory accesses

2015-03-26 Thread Lennart Poettering
On Tue, 24.03.15 11:16, Shawn Landden (sh...@churchofgit.com) wrote:

 And those arches don't get much testing too.
 ---
  CODING_STYLE | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/CODING_STYLE b/CODING_STYLE
 index b687e72..8934954 100644
 --- a/CODING_STYLE
 +++ b/CODING_STYLE
 @@ -14,7 +14,8 @@
  - The destructors always unregister the object from the next bigger
object, not the other way around
  
 -- To minimize strict aliasing violations, we prefer unions over casting
 +- To minimize strict aliasing violations and unaligned memory accesses,
 +  we prefer unions over casting

Unaligned memory accesses are an orthogonal problem really. I don't
see how the change above really makes sense?

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-commits] Makefile.am src/shared src/timedate

2015-03-26 Thread Lennart Poettering
On Tue, 24.03.15 07:04, Kay Sievers (k...@kemper.freedesktop.org) wrote:

  Makefile.am|2 
  src/shared/time-dst.c  |  329 
 -
  src/shared/time-dst.h  |   26 ---
  src/timedate/timedatectl.c |   56 ---
  4 files changed, 413 deletions(-)
 
 New commits:
 commit 16c6ea29348ddac73998f339166f863bee0dfef6
 Author: Kay Sievers k...@vrfy.org
 Date:   Tue Mar 24 13:52:04 2015 +0100
 
 timedate: remove daylight saving time handling and tzfile parser
 
 We planned to support (the conceptually broken) daylight saving
 time/local time features in the kernel, SCSI, networking, FAT
 filesystem, but it turned out to be a race we cannot win and do
 not want to get involved. Systemd should not fiddle with daylight
 saving time or parse timezone information itself.
 
 Leave everything to glibc or tools like date(1) and do not make any
 promises or raise expectations that systemd should handle anything
 like this.

For what's it worth, I strongly disagree with the removal of this. 

We do provide OnCalendar= triggers in timer units. They trigger on
calendar time, not on UTC time, and are hence subject to local clock
changes, to local timezone changes, and to the DST changes of the
local timezone. Their time scale is non-linear due to this: it
*mostly* is linear, but under any of these three conditions it will
not be linear anymore, it will jump.

Now, to make calendar time triggers complete I think we must enable
people to at least trigger on threse three kinds of anomalies in the
time scale. As such I think it would make a *ton* of sense to add:

 OnTimeZoneChange=
 OnClockChange=
 OnDSTChange=

settings to .timer units, so that users can explictly watch for any of
them, either separately of the actual calendar expressions, or in
addition to them. 

If we do have all four of OnCalendar=, OnTimeZoneChange=,
OnClockChange= and OnDSTChange= in place, then for the first time we
actually can express timer events in a complete way, and the anomalies
of the wallclock time scale becomes *manageable*, for the first time.

Just removing this code blindly appears misguided to me. It just means
sticking the head in the sand, ignoring completely the fact that we
*do* already provide OnCalendar= timer units, and ignoring that their
time scale is so weirdly non-monotonic. And we leave our users in the
cold, because we provide them with no way to manage the fuckup that
DST is.

I strongly believe that it is our duty to make the non-monotonicity of
the calendar time scale as managable as possible for admins, and that
not only includes triggers on DST changes, but in fact the DST changes
are probably the most important kind of anomaly on the calendar scale,
since even a completely NTP controlled, physically fixed system will
experience them regularly, in contrast to the other kinds of
anomalies.

So yeah, Kay, I think this should be readded and be made useful for
timer units. And if we make use of this for the timer units we might
as well show it in timedatectl...

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-commits] Makefile.am src/shared src/timedate

2015-03-26 Thread Kay Sievers
On Thu, Mar 26, 2015 at 11:48 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 24.03.15 07:04, Kay Sievers (k...@kemper.freedesktop.org) wrote:

  Makefile.am|2
  src/shared/time-dst.c  |  329 
 -
  src/shared/time-dst.h  |   26 ---
  src/timedate/timedatectl.c |   56 ---
  4 files changed, 413 deletions(-)

 New commits:
 commit 16c6ea29348ddac73998f339166f863bee0dfef6
 Author: Kay Sievers k...@vrfy.org
 Date:   Tue Mar 24 13:52:04 2015 +0100

 timedate: remove daylight saving time handling and tzfile parser

 We planned to support (the conceptually broken) daylight saving
 time/local time features in the kernel, SCSI, networking, FAT
 filesystem, but it turned out to be a race we cannot win and do
 not want to get involved. Systemd should not fiddle with daylight
 saving time or parse timezone information itself.

 Leave everything to glibc or tools like date(1) and do not make any
 promises or raise expectations that systemd should handle anything
 like this.

 For what's it worth, I strongly disagree with the removal of this.

 We do provide OnCalendar= triggers in timer units. They trigger on
 calendar time, not on UTC time, and are hence subject to local clock
 changes, to local timezone changes, and to the DST changes of the
 local timezone. Their time scale is non-linear due to this: it
 *mostly* is linear, but under any of these three conditions it will
 not be linear anymore, it will jump.

 Now, to make calendar time triggers complete I think we must enable
 people to at least trigger on threse three kinds of anomalies in the
 time scale. As such I think it would make a *ton* of sense to add:

  OnTimeZoneChange=
  OnClockChange=

These are useful, sure.

  OnDSTChange=

This is absolutely not needed and we should not get into that business.

 settings to .timer units, so that users can explictly watch for any of
 them, either separately of the actual calendar expressions, or in
 addition to them.

 If we do have all four of OnCalendar=, OnTimeZoneChange=,
 OnClockChange= and OnDSTChange= in place, then for the first time we
 actually can express timer events in a complete way, and the anomalies
 of the wallclock time scale becomes *manageable*, for the first time.

 Just removing this code blindly appears misguided to me. It just means
 sticking the head in the sand, ignoring completely the fact that we
 *do* already provide OnCalendar= timer units, and ignoring that their
 time scale is so weirdly non-monotonic. And we leave our users in the
 cold, because we provide them with no way to manage the fuckup that
 DST is.

 I strongly believe that it is our duty to make the non-monotonicity of
 the calendar time scale as managable as possible for admins, and that
 not only includes triggers on DST changes, but in fact the DST changes
 are probably the most important kind of anomaly on the calendar scale,
 since even a completely NTP controlled, physically fixed system will
 experience them regularly, in contrast to the other kinds of
 anomalies.

DST support works all fine already today. I don't see any need for
parsing the tz files here to solve an actual existing problem.

 So yeah, Kay, I think this should be readded and be made useful for
 timer units. And if we make use of this for the timer units we might
 as well show it in timedatectl...

No, it shouldn't.

We should stay out of the DST business. Glibc calculates everything
just fine for all needed tasks and we arm the timers accordingly.
There is no need to fiddle with the raw tzfile data here.

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


Re: [systemd-devel] [systemd-commits] Makefile.am src/shared src/timedate

2015-03-26 Thread Kay Sievers
On Thu, Mar 26, 2015 at 12:06 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Thu, 26.03.15 11:57, Kay Sievers (k...@vrfy.org) wrote:

  Now, to make calendar time triggers complete I think we must enable
  people to at least trigger on threse three kinds of anomalies in the
  time scale. As such I think it would make a *ton* of sense to add:
 
   OnTimeZoneChange=
   OnClockChange=

 These are useful, sure.

   OnDSTChange=

 This is absolutely not needed and we should not get into that
 business.

 That's just a statement of an opinion. I see no explanation for this.

I see only repeated made-up arguments to add an exotic feature, which
I don't see adding any real value.

DST for the machine is presentation only. Tools handling it turn the
presentation into the actual machine's time and be done with it.
Glibc does that for us today already and it already works sufficiently
for systemd's calendar time support.

If the system's time zone changes, or the time is adjusted manually,
we just re-arm all timers, and all should be fine.

I see no need or use to support explicit triggers on the event of DST
changes, the system or calendar time support just does not need them.

 There is no need to fiddle with the raw tzfile data here.

 Well, it's good that things are so simple for you.

I'm not willing to discuss things in that tone.

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


Re: [systemd-devel] [systemd-commits] Makefile.am src/shared src/timedate

2015-03-26 Thread Lennart Poettering
On Thu, 26.03.15 11:57, Kay Sievers (k...@vrfy.org) wrote:

  Now, to make calendar time triggers complete I think we must enable
  people to at least trigger on threse three kinds of anomalies in the
  time scale. As such I think it would make a *ton* of sense to add:
 
   OnTimeZoneChange=
   OnClockChange=
 
 These are useful, sure.
 
   OnDSTChange=
 
 This is absolutely not needed and we should not get into that
 business.

That's just a statement of an opinion. I see no explanation for this.

I think I gave a good explanation why I think we should cover all
three kinds of calendar anomalies. Can you explain why the one anomaly
that happens all the time, on pretty much everybody's system you
consider irrelevant?

  I strongly believe that it is our duty to make the non-monotonicity of
  the calendar time scale as managable as possible for admins, and that
  not only includes triggers on DST changes, but in fact the DST changes
  are probably the most important kind of anomaly on the calendar scale,
  since even a completely NTP controlled, physically fixed system will
  experience them regularly, in contrast to the other kinds of
  anomalies.
 
 DST support works all fine already today. I don't see any need for
 parsing the tz files here to solve an actual existing problem.

True. And sysvinit worked fine already today too.

It's not a question on wether something works fine or not, it's a
question of actually solving real problems.

If you think that DST handling for timer triggers is not a problem, then
you live in a very simple world.

  So yeah, Kay, I think this should be readded and be made useful for
  timer units. And if we make use of this for the timer units we might
  as well show it in timedatectl...
 
 No, it shouldn't.
 
 We should stay out of the DST business. Glibc calculates everything
 just fine for all needed tasks and we arm the timers accordingly.
 There is no need to fiddle with the raw tzfile data here.

Well, it's good that things are so simple for you. But calendar time
handling is really not that simple. You *have* to think of DST. And if
you don't you just avoid dealing with the complexity it brings.

You know, Kay, DST is a real thing. It's an ugly thing, and we all
wish it would not exist. But well, it does, and we better deal with it
and make it managable.

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] How to debug blocking service start?

2015-03-26 Thread Kai Hendry
First, thanks for trying to help me Kai. Awesome name btw.

On Fri, 27 Mar 2015, at 03:26 AM, Kai Krakow wrote:
 Try Type=simple to not let it wait. That is telling systemd, that the
 binary 
 will not daemonize - athough it should be default according to [1].

It's still getting stuck with Type=simple.

http://s.natalian.org/2015-03-27/simple.png

Isn't there a better way to debug than running journalctl -u service
-f in parallel?

The frustrating thing is that the SAME service file works fine on
another rpi2.

 However, I'm not sure whether the rest of the setup will work. You should 
 probably make it a user service, so that you won't have to pass DISPLAY. 
 Your special setup may be part of the problem. Also keep in mind that 
 After=graphical.target doesn't imply that X11 is ready to accept
 connections 

You mean systemctl --user right?
https://wiki.archlinux.org/index.php/Systemd/User

I don't like that since it seems like a lot of extra crud. What's the
big deal about passing in the DISPLAY environment?

I don't see the point of having a non-root user. Trying to keep things
as simple as possible to achieve my use case. Which is to start a
browser once online.

 - even worse: it doesn't imply that it is started late in the boot 
 process. Your service may start as early as graphical.target becomes 
 scheduled to start [3] - and that may be well before even basic system
 setup 
 is finished. Instead, I suggest using a DM with autologin, and then spawn
 a 
 user service from there. As an alternative, you may want to try working
 with 
 timer units [2] to activate surf.service a few seconds after X11 is 
 guarenteed to be up, but that would just be a bandaid.

Well, if X wasn't up up, I was hoping Restart=always would do the rest.

 [1]: man systemd.service
 [2]: man systemd.timer

Timers seem like a kluge, just to know when X is ready.

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


Re: [systemd-devel] How to debug blocking service start?

2015-03-26 Thread Kai Hendry


On Fri, 27 Mar 2015, at 12:14 PM, Daurnimator wrote:
 My first guess based on that screenshot is case: Simple vs simple.

No I fixed that problem. :) http://ix.io/h8U

Wish there was a service validator service.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] regarding to cgroup siblings mask

2015-03-26 Thread WaLyong Cho
On 03/27/2015 05:33 AM, David Timothy Strauss wrote:
 On Tue, Mar 24, 2015 at 4:29 AM, WaLyong Cho walyong@samsung.com wrote:
 Could anyone explain why?
 
 An admin using CPUShares= or a similar proportional CGroup controller
 probably assumes that setting the shares to twice the default (for
 example) increases the relative proportion of resources for that unit.
 However, that is only true if other units competing for that resource
 have the same controller(s) enabled so that the kernel knows to
 balance the resources accordingly.
 
 The code in systemd ensures that if any unit uses a proportional
 CGroups controller in a slice, all other units in that same slice
 enable that controller as well, usually with the default proportions.

Thanks, understood. But I think this propagation is needed only for
taking weight argument such like CPUShares=weight,
StartupCPUShares=weight, BlockIOWeight=weight,
StartupBlockIOWeight=weight, BlockIODeviceWeight=device weight. For
example, I don't think MemoryLimit= is not option of proportional. It
just only limit of its cgroup and does not race with other cgroup.

If I'm right, we need to modify unit_get_target_mask() to get only mask
for proportional properties.

 
 unit_get_target_mask() is part of an optimization I added so that
 initializing CGroups controllers for a given unit doesn't require
 iterating through every other unit in a slice to figure out the
 necessary controllers. It provides a bitmask indicating the
 controllers in use by its siblings so the unit can enable, say,
 CPUShares= if one of its siblings is doing so.
 ___
 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] How to debug blocking service start?

2015-03-26 Thread Daurnimator
On 27 March 2015 at 13:32, Kai Hendry hen...@webconverger.com wrote:
 It's still getting stuck with Type=simple.

 http://s.natalian.org/2015-03-27/simple.png

 Isn't there a better way to debug than running journalctl -u service
 -f in parallel?

 The frustrating thing is that the SAME service file works fine on
 another rpi2.

What does your .service file currently look like?
My first guess based on that screenshot is case: Simple vs simple.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] regarding to cgroup siblings mask

2015-03-26 Thread David Timothy Strauss
On Tue, Mar 24, 2015 at 4:29 AM, WaLyong Cho walyong@samsung.com wrote:
 If this can be configurable, how about add a configuration for cgroup
 mask propagation to siblings?

I believe the way to prevent propagation is to separate the units into
different slices.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] How to debug blocking service start?

2015-03-26 Thread Kai Krakow
Kai Hendry hen...@webconverger.com schrieb:

 Hi there,
 
 How do I figure out why or where something is stuck?
 http://s.natalian.org/2015-03-25/systemd-start-issue.png
 
 `journalctl -u surf -f` prints nothing.
 
 Binary surf runs fine when I run it manually.

It probably is not stuck, it's systemd waiting for the command to return. 
Try Type=simple to not let it wait. That is telling systemd, that the binary 
will not daemonize - athough it should be default according to [1].

However, I'm not sure whether the rest of the setup will work. You should 
probably make it a user service, so that you won't have to pass DISPLAY. 
Your special setup may be part of the problem. Also keep in mind that 
After=graphical.target doesn't imply that X11 is ready to accept connections 
- even worse: it doesn't imply that it is started late in the boot 
process. Your service may start as early as graphical.target becomes 
scheduled to start [3] - and that may be well before even basic system setup 
is finished. Instead, I suggest using a DM with autologin, and then spawn a 
user service from there. As an alternative, you may want to try working with 
timer units [2] to activate surf.service a few seconds after X11 is 
guarenteed to be up, but that would just be a bandaid.

[1]: man systemd.service
[2]: man systemd.timer

[3]: A target is, AFAIK, reached as soon as all services making up the 
target are scheduled to start - so you can rely on sockets being ready but 
nothing more. Targets are sets of functionality to offer, not points in time 
of the boot process - this is different to what you know of sysvinit. 
Multiple targets can be active at the same time, e.g. printer.target pulls 
in cups and becomes activated by udev if you plug in your printer.

-- 
Replies to list only preferred.

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


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

2015-03-26 Thread Djalal Harouni
On Fri, Mar 27, 2015 at 12:30:53AM +0100, Tom Gundersen wrote:
 On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
 
  Will result in slightly smaller binaries, and cuts out the branch, even if
  the expression is still executed.
 
  I am sorry, but the whole point of assert_se() is that it has side
  effects. That's why it is called that way (the se suffix stands for
  side effects), and is different from assert(). You are supposed to
  use it whenever the code enclosed in it shall not be suppressed if
  NDEBUG is defined. This patch of yours breaks that whole logic!
 
 Hm, am I reading the patch wrong? It looks good to me. It is simply
 dropping the branching and logging in the NDEBUG case, but the
 expression itself is still evaluated.
Yep but it seems that abort() will never be called,
log_assert_failed() = abort()

And the logging mechanism is also one of those side effects. IMO unless
there are real valid reasons for any change to these macors... changing
anything here will just bring more bugs that we may never notice.


 So in the NDEBUG case assert_se(foo()) becomes equivalent to
 foo(), which I guess makes sense (though I doubt it makes much of a
 difference).
 
 -t
 
  ---
   src/shared/macro.h | 12 ++--
   1 file changed, 6 insertions(+), 6 deletions(-)
 
  diff --git a/src/shared/macro.h b/src/shared/macro.h
  index 7f89951..02219ea 100644
  --- a/src/shared/macro.h
  +++ b/src/shared/macro.h
  @@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned 
  long u) {
   (__x / __y + !!(__x % __y));\
   })
 
  -#define assert_se(expr) \
  -do {\
  -if (_unlikely_(!(expr)))\
  -log_assert_failed(#expr, __FILE__, __LINE__, 
  __PRETTY_FUNCTION__); \
  -} while (false) \
  -
   /* We override the glibc assert() here. */
   #undef assert
   #ifdef NDEBUG
  +#define assert_se(expr) do {expr} while(false)
   #define assert(expr) do {} while(false)
   #else
  +#define assert_se(expr) \
  +do {\
  +if (_unlikely_(!(expr)))\
  +log_assert_failed(#expr, __FILE__, __LINE__, 
  __PRETTY_FUNCTION__); \
  +} while (false)
   #define assert(expr) assert_se(expr)
   #endif
 
  --
  2.2.1.209.g41e5f3a
 
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 
 
  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

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


Re: [systemd-devel] parsing audit messages

2015-03-26 Thread Zbigniew Jędrzejewski-Szmek
On Thu, Mar 26, 2015 at 09:42:45AM +0100, Lennart Poettering wrote:
 On Sun, 15.03.15 03:51, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
 
  On Sun, Mar 15, 2015 at 03:49:07AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
   Hi,
   
   I was looking at some debug logs, and the audit messages are
   semi-useless in their current undecoded form:
   
   mar 14 22:24:02 fedora22 audit[1]: audit-1130 pid=1 uid=0 
   auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 
   msg='unit=systemd-udev-trigger comm=systemd 
   exe=/usr/lib/systemd/systemd hostname=? addr=? terminal=? res=success'
   mar 14 22:24:05 fedora22 audit: audit-1327 
   proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D0069707461626C655F7365637572697479
   
   You added code to parse this, and I think we should make use of it and
   put msg= field as MESSAGE=, and maybe store the original message as
   _AUDIT= or something. If there's no msg field, like with proctitle,
   print all fields that are in the message, but using our cescape, and
   not this hexadecimal form which is unreadable for humans.
  
  I think we should also translate type= to names...
  
  https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Security_Guide/sec-Audit_Record_Types.html

 Well, we don't translate MESSAGE_ID fields to strings either...

Here the mapping is stable, and maintained in one place... I think it's more
like dns TYPE field, completely reversible, then MESSAGE_IDs.

I see your point about the format being too messy to parse
reliably. OTOH, currently, what we log is much harder to use than the
standard audit logs. Dunno.

Zbyszek

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


Re: [systemd-devel] regarding to cgroup siblings mask

2015-03-26 Thread David Timothy Strauss
On Tue, Mar 24, 2015 at 4:29 AM, WaLyong Cho walyong@samsung.com wrote:
 Could anyone explain why?

An admin using CPUShares= or a similar proportional CGroup controller
probably assumes that setting the shares to twice the default (for
example) increases the relative proportion of resources for that unit.
However, that is only true if other units competing for that resource
have the same controller(s) enabled so that the kernel knows to
balance the resources accordingly.

The code in systemd ensures that if any unit uses a proportional
CGroups controller in a slice, all other units in that same slice
enable that controller as well, usually with the default proportions.

unit_get_target_mask() is part of an optimization I added so that
initializing CGroups controllers for a given unit doesn't require
iterating through every other unit in a slice to figure out the
necessary controllers. It provides a bitmask indicating the
controllers in use by its siblings so the unit can enable, say,
CPUShares= if one of its siblings is doing so.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2015-03-26 Thread Tom Gundersen
On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:

 Will result in slightly smaller binaries, and cuts out the branch, even if
 the expression is still executed.

 I am sorry, but the whole point of assert_se() is that it has side
 effects. That's why it is called that way (the se suffix stands for
 side effects), and is different from assert(). You are supposed to
 use it whenever the code enclosed in it shall not be suppressed if
 NDEBUG is defined. This patch of yours breaks that whole logic!

Hm, am I reading the patch wrong? It looks good to me. It is simply
dropping the branching and logging in the NDEBUG case, but the
expression itself is still evaluated.

So in the NDEBUG case assert_se(foo()) becomes equivalent to
foo(), which I guess makes sense (though I doubt it makes much of a
difference).

-t

 ---
  src/shared/macro.h | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/src/shared/macro.h b/src/shared/macro.h
 index 7f89951..02219ea 100644
 --- a/src/shared/macro.h
 +++ b/src/shared/macro.h
 @@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long 
 u) {
  (__x / __y + !!(__x % __y));\
  })

 -#define assert_se(expr) \
 -do {\
 -if (_unlikely_(!(expr)))\
 -log_assert_failed(#expr, __FILE__, __LINE__, 
 __PRETTY_FUNCTION__); \
 -} while (false) \
 -
  /* We override the glibc assert() here. */
  #undef assert
  #ifdef NDEBUG
 +#define assert_se(expr) do {expr} while(false)
  #define assert(expr) do {} while(false)
  #else
 +#define assert_se(expr) \
 +do {\
 +if (_unlikely_(!(expr)))\
 +log_assert_failed(#expr, __FILE__, __LINE__, 
 __PRETTY_FUNCTION__); \
 +} while (false)
  #define assert(expr) assert_se(expr)
  #endif

 --
 2.2.1.209.g41e5f3a

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


 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] [PATCH v2] Add reboot to EFI support

2015-03-26 Thread Jan Janssen
---
 man/systemctl.xml |  6 +++-
 src/libsystemd/sd-bus/bus-common-errors.h |  1 +
 src/login/logind-dbus.c   | 49 +++--
 src/login/org.freedesktop.login1.conf |  8 +
 src/shared/efivars.c  | 52 +++
 src/shared/efivars.h  |  2 ++
 src/systemctl/systemctl.c | 16 --
 7 files changed, 127 insertions(+), 7 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 50e6bc9..eafdd73 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1538,7 +1538,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 systems. This may result in data loss./para
 
 paraIf the optional argument
-replaceablearg/replaceable is given, it will be passed
+replaceablearg/replaceable is given and is equal to
+literalefi/literal, the system will be rebooted to
+the EFI firmware interface on machines that support it.
+Note that this requires the system to be booted in EFI mode.
+Otherwise, the argument will be passed
 as the optional argument to the
 
citerefentryrefentrytitlereboot/refentrytitlemanvolnum2/manvolnum/citerefentry
 system call. The value is architecture and firmware
diff --git a/src/libsystemd/sd-bus/bus-common-errors.h 
b/src/libsystemd/sd-bus/bus-common-errors.h
index b17b62a..3019140 100644
--- a/src/libsystemd/sd-bus/bus-common-errors.h
+++ b/src/libsystemd/sd-bus/bus-common-errors.h
@@ -57,6 +57,7 @@
 #define BUS_ERROR_DEVICE_IS_TAKEN org.freedesktop.login1.DeviceIsTaken
 #define BUS_ERROR_DEVICE_NOT_TAKEN org.freedesktop.login1.DeviceNotTaken
 #define BUS_ERROR_OPERATION_IN_PROGRESS 
org.freedesktop.login1.OperationInProgress
+#define BUS_ERROR_REBOOT_TO_EFI_NOT_SUPPORTED 
org.freedesktop.login1.RebootToEfiNotSupported
 #define BUS_ERROR_SLEEP_VERB_NOT_SUPPORTED 
org.freedesktop.login1.SleepVerbNotSupported
 
 #define BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED 
org.freedesktop.timedate1.AutomaticTimeSyncEnabled
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index a3d49ef..8fec90f 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -38,8 +38,11 @@
 #include bus-common-errors.h
 #include udev-util.h
 #include selinux-util.h
+#include efivars.h
 #include logind.h
 
+#define SPECIAL_REBOOT_TO_EFI_TARGET x-logind-reboot-to-efi.target
+
 int manager_get_session_from_creds(Manager *m, sd_bus_message *message, const 
char *name, sd_bus_error *error, Session **ret) {
 _cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL;
 Session *session;
@@ -1422,6 +1425,13 @@ static int execute_shutdown_or_sleep(
 assert(w  _INHIBIT_WHAT_MAX);
 assert(unit_name);
 
+if (streq(unit_name, SPECIAL_REBOOT_TO_EFI_TARGET)) {
+unit_name = SPECIAL_REBOOT_TARGET;
+r = efi_indicate_reboot_to_fw();
+if (r  0)
+return r;
+}
+
 bus_manager_log_shutdown(m, w, unit_name);
 
 r = sd_bus_call_method(
@@ -1563,6 +1573,9 @@ static int method_do_shutdown_or_sleep(
 if (m-action_what)
 return sd_bus_error_setf(error, 
BUS_ERROR_OPERATION_IN_PROGRESS, There's already a shutdown or sleep operation 
in progress);
 
+if (streq(unit_name, SPECIAL_REBOOT_TO_EFI_TARGET)  
!is_efi_reboot_to_fw_supported())
+return sd_bus_error_setf(error, 
BUS_ERROR_REBOOT_TO_EFI_NOT_SUPPORTED, Reboot to EFI not supported);
+
 if (sleep_verb) {
 r = can_sleep(sleep_verb);
 if (r  0)
@@ -1648,6 +1661,21 @@ static int method_reboot(sd_bus *bus, sd_bus_message 
*message, void *userdata, s
 error);
 }
 
+static int method_reboot_to_efi(sd_bus *bus, sd_bus_message *message, void 
*userdata, sd_bus_error *error) {
+Manager *m = userdata;
+
+return method_do_shutdown_or_sleep(
+m, message,
+SPECIAL_REBOOT_TO_EFI_TARGET,
+INHIBIT_SHUTDOWN,
+org.freedesktop.login1.reboot,
+org.freedesktop.login1.reboot-multiple-sessions,
+org.freedesktop.login1.reboot-ignore-inhibit,
+NULL,
+method_reboot_to_efi,
+error);
+}
+
 static int method_suspend(sd_bus *bus, sd_bus_message *message, void 
*userdata, sd_bus_error *error) {
 Manager *m = userdata;
 
@@ -1700,7 +1728,7 @@ static int method_can_shutdown_or_sleep(
 const char *action,
 const char *action_multiple_sessions,
 const char *action_ignore_inhibit,
-const char *sleep_verb,
+const char *arg,
 sd_bus_error *error) {