Re: [systemd-devel] [PATCH] CODING_STYLE: this also help with unaligned memory accesses

2015-03-26 Thread Shawn Landden
On Thu, Mar 26, 2015 at 1:31 AM, Lennart Poettering
 wrote:
> 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?
casting often leads to unaligned memory accesses. Without casting the
compiler usually gets it right, but yeah this has little to do with
unions. I was confusing with having a packed and unpacked struct and
converting between the two like thus:

typedef struct {
uint8_t a
uint32_t b
} packed __attribute__((packed));

#if HAVE_FAST_UNALIGNED
#define unpacked packed
#elsif
typedef struct {
uint8_t a
// uint8_t __padding[3]; gcc will do this for you
uint32_t b
} unpacked;
#endif

foo get_foo(p *void) {
  unpacked e;
  e.a = (*packed)->a;
  e.b = (*packed)->b;
  return e;
}
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
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 Martin Pitt
Hello Kai,

Kai Hendry [2015-03-27 12:18 +0800]:
> Wish there was a service validator service.

Not sure what you changed, but "systemd-analyze verify foo.service"
might be a good start?

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] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

2015-03-26 Thread Shawn Landden
On Thu, Mar 26, 2015 at 5:47 PM, Djalal Harouni  wrote:
> On Fri, Mar 27, 2015 at 12:30:53AM +0100, Tom Gundersen wrote:
>> On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
>>  wrote:
>> > On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>> >
>> >> Will result in slightly smaller binaries, and cuts out the branch, even if
>> >> the expression is still executed.
>> >
>> > I am sorry, but the whole point of assert_se() is that it has side
>> > effects. That's why it is called that way (the "se" suffix stands for
>> > "side effects"), and is different from assert(). You are supposed to
>> > use it whenever the code enclosed in it shall not be suppressed if
>> > NDEBUG is defined. This patch of yours breaks that whole logic!
>>
>> Hm, am I reading the patch wrong? It looks good to me. It is simply
>> dropping the branching and logging in the NDEBUG case, but the
>> expression itself is still evaluated.
> Yep but it seems that abort() will never be called,
> log_assert_failed() => abort()
>
> And the logging mechanism is also one of those side effects. IMO unless
> there are real valid reasons for any change to these macors... changing
> anything here will just bring more bugs that we may never notice.
>
Those are the side effects of assert(), the side effects of the
expression are still evaluated.
>
>> 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



-- 
Shawn Landden
___
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 Shawn Landden
On Thu, Mar 26, 2015 at 1:19 AM, Lennart Poettering
 wrote:
> On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>
>> Will result in slightly smaller binaries, and cuts out the branch, even if
>> the expression is still executed.
>
> I am sorry, but the whole point of assert_se() is that it has side
> effects. That's why it is called that way (the "se" suffix stands for
> "side effects"), and is different from assert(). You are supposed to
> use it whenever the code enclosed in it shall not be suppressed if
> NDEBUG is defined. This patch of yours breaks that whole logic!

+#define assert_se(expr) do {expr} while(false)
 #define assert(expr) do {} while(false)

No it does not. see "{expr}", it still is doing the side effect. It
just doesn't check if the return value is as expected.
>
> Lennart
>
>> ---
>>  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



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


[systemd-devel] [PATCH] cgroup: propagate cgroup mask only for proportional properties

2015-03-26 Thread WaLyong Cho
Some of cgroup properties does not affect to sibling
cgroups. CPUShares and BlockIOWeight are only needed to be propagated.
---
 src/core/cgroup.c | 29 -
 src/core/cgroup.h |  2 ++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 6b8abb4..b4b9678 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -458,6 +458,23 @@ void cgroup_context_apply(CGroupContext *c, 
CGroupControllerMask mask, const cha
 }
 }
 
+CGroupControllerMask cgroup_context_get_proportional_mask(CGroupContext *c) {
+CGroupControllerMask mask = 0;
+
+/* Get only proportional mask */
+
+if (c->cpu_shares != (unsigned long) -1 ||
+c->startup_cpu_shares != (unsigned long) -1)
+mask |= CGROUP_CPU;
+
+if (c->blockio_weight != (unsigned long) -1 ||
+c->startup_blockio_weight != (unsigned long) -1 ||
+c->blockio_device_weights)
+mask |= CGROUP_BLKIO;
+
+return mask;
+}
+
 CGroupControllerMask cgroup_context_get_mask(CGroupContext *c) {
 CGroupControllerMask mask = 0;
 
@@ -487,6 +504,16 @@ CGroupControllerMask cgroup_context_get_mask(CGroupContext 
*c) {
 return mask;
 }
 
+CGroupControllerMask unit_get_cgroup_proportional_mask(Unit *u) {
+CGroupContext *c;
+
+c = unit_get_cgroup_context(u);
+if (!c)
+return 0;
+
+return cgroup_context_get_proportional_mask(c);
+}
+
 CGroupControllerMask unit_get_cgroup_mask(Unit *u) {
 CGroupContext *c;
 
@@ -531,7 +558,7 @@ CGroupControllerMask unit_get_members_mask(Unit *u) {
 continue;
 
 u->cgroup_members_mask |=
-unit_get_cgroup_mask(member) |
+unit_get_cgroup_proportional_mask(member) |
 unit_get_members_mask(member);
 }
 }
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
index 869ddae..2371158 100644
--- a/src/core/cgroup.h
+++ b/src/core/cgroup.h
@@ -98,12 +98,14 @@ void cgroup_context_done(CGroupContext *c);
 void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix);
 void cgroup_context_apply(CGroupContext *c, CGroupControllerMask mask, const 
char *path, ManagerState state);
 
+CGroupControllerMask cgroup_context_get_proportional_mask(CGroupContext *c);
 CGroupControllerMask cgroup_context_get_mask(CGroupContext *c);
 
 void cgroup_context_free_device_allow(CGroupContext *c, CGroupDeviceAllow *a);
 void cgroup_context_free_blockio_device_weight(CGroupContext *c, 
CGroupBlockIODeviceWeight *w);
 void cgroup_context_free_blockio_device_bandwidth(CGroupContext *c, 
CGroupBlockIODeviceBandwidth *b);
 
+CGroupControllerMask unit_get_cgroup_proportional_mask(Unit *u);
 CGroupControllerMask unit_get_cgroup_mask(Unit *u);
 CGroupControllerMask unit_get_siblings_mask(Unit *u);
 CGroupControllerMask unit_get_members_mask(Unit *u);
-- 
1.9.3

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

2015-03-26 Thread Daurnimator
On 27 March 2015 at 13:32, Kai Hendry  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 
> -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 WaLyong Cho
On 03/27/2015 05:33 AM, David Timothy Strauss wrote:
> On Tue, Mar 24, 2015 at 4:29 AM, WaLyong Cho  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 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 
-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] [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
>  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] [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
 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


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  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] regarding to cgroup siblings mask

2015-03-26 Thread David Timothy Strauss
On Tue, Mar 24, 2015 at 4:29 AM, WaLyong Cho  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] How to debug blocking service start?

2015-03-26 Thread Kai Krakow
Kai Hendry  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


[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.
 
 If the optional argument
-arg is given, it will be passed
+arg is given and is equal to
+efi, 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
 
reboot2
 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) {
 
 _cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL;
@@ -1717,8 +1745,8 @@ static int method_ca

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]:  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:  
> > > 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] [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
 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] [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
 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 
>> 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 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 
> 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] 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]:  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:  
> > 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]:  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:  
> 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] [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] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

2015-03-26 Thread Lennart Poettering
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!

Lennart

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