Re: [systemd-devel] systemd-networkd with 802.1x

2013-11-25 Thread David Timothy Strauss
On Mon, Nov 25, 2013 at 5:38 PM, Umut Tezduyar Lindskog
umut.tezdu...@axis.com wrote:
 I would think port access protocols would be needed for servers even before 
 sending a single IP package.

I've never used a server that required 802.1x. I've only encountered
it in places where physical network access is widespread among people
who aren't trusted, like offices and universities.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd-networkd with 802.1x

2013-11-25 Thread Tomasz Torcz
On Mon, Nov 25, 2013 at 06:20:09PM +1000, David Timothy Strauss wrote:
 On Mon, Nov 25, 2013 at 5:38 PM, Umut Tezduyar Lindskog
 umut.tezdu...@axis.com wrote:
  I would think port access protocols would be needed for servers even before 
  sending a single IP package.
 
 I've never used a server that required 802.1x. I've only encountered
 it in places where physical network access is widespread among people
 who aren't trusted, like offices and universities.

  It's utilized with certain high-security environments. Data centers
hosting clouds with secure as selling points.  It's roughly
the same level of paranoia like using SecureBoot on servers,
some customers are paying extra for that.

-- 
Tomasz   .. oo o.   oo o. .o   .o o. o. oo o.   ..
Torcz.. .o .o   .o .o oo   oo .o .. .. oo   oo
o.o.o.   .o .. o.   o. o. o.   o. o. oo .. ..   o.

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


[systemd-devel] Italian translation + revision proposals (wording) on english labels

2013-11-25 Thread Daniele Medri
Dear systemd maintainers,

I've made a pull request on github with some patches:

- Add new po/it.po (Italian translation)
- Revision proposals (wording) on english labels

http://github.com/systemd/systemd/pull/7


Best Regards

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


[systemd-devel] [PATCH] build: fix problems with undefined references in tests

2013-11-25 Thread Lukasz Skalski
Please find patch in attachement for review - it fix problem
with undefined references in some test-libsystemd-*-sym.c tests.
---
 Makefile.am |5 +
 1 file changed, 5 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 728b860..9e03f0a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2257,6 +2257,7 @@ libudev_la_LDFLAGS = \
 
 libudev_la_LIBADD = \
libsystemd-daemon-internal.la \
+   libsystemd-bus-internal.la \
libsystemd-id128-internal.la \
libsystemd-shared.la
 
@@ -2826,6 +2827,7 @@ libsystemd_id128_la_LDFLAGS = \

-Wl,--version-script=$(top_srcdir)/src/libsystemd-id128/libsystemd-id128.sym
 
 libsystemd_id128_la_LIBADD = \
+   libsystemd-bus-internal.la \
libsystemd-daemon-internal.la \
libsystemd-shared.la
 
@@ -3027,6 +3029,7 @@ libsystemd_journal_la_LDFLAGS = \
 
 libsystemd_journal_la_LIBADD = \
libsystemd-label.la \
+   libsystemd-bus-internal.la \
libsystemd-daemon-internal.la \
libsystemd-id128-internal.la \
libsystemd-shared.la
@@ -4038,6 +4041,8 @@ libsystemd_login_la_LDFLAGS = \
-Wl,--version-script=$(top_srcdir)/src/login/libsystemd-login.sym
 
 libsystemd_login_la_LIBADD = \
+   libsystemd-bus-internal.la \
+   libsystemd-id128-internal.la \
libsystemd-daemon-internal.la \
libsystemd-shared.la
 
-- 
1.7.9.5

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


Re: [systemd-devel] [PATCH] build: fix problems with undefined references in tests

2013-11-25 Thread Kay Sievers
On Mon, Nov 25, 2013 at 1:56 PM, Lukasz Skalski
l.skal...@partner.samsung.com wrote:
 Please find patch in attachement for review - it fix problem
 with undefined references in some test-libsystemd-*-sym.c tests.

It looks a bit weird to add library requirements, theses libs should
not need the other libraries. What exactly fails on your side?

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


Re: [systemd-devel] [PATCH] build: fix problems with undefined references in tests

2013-11-25 Thread Lukasz Skalski

On 11/25/2013 02:03 PM, Kay Sievers wrote:

On Mon, Nov 25, 2013 at 1:56 PM, Lukasz Skalski
l.skal...@partner.samsung.com wrote:

Please find patch in attachement for review - it fix problem
with undefined references in some test-libsystemd-*-sym.c tests.


It looks a bit weird to add library requirements, theses libs should
not need the other libraries. What exactly fails on your side?

Kay



Hi,

Yes I know, but otherwise:

  CCLD   test-libsystemd-daemon-sym
  CC test-libsystemd-id128-sym.o
  CCLD   test-libsystemd-id128-sym
./.libs/libsystemd-id128.so: undefined reference to `sd_bus_label_escape'
./.libs/libsystemd-id128.so: undefined reference to `sd_bus_label_unescape'
collect2: ld returned 1 exit status
make[2]: *** [test-libsystemd-id128-sym] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2


--
Lukasz Skalski
Samsung RD Institute Poland
Samsung Electronics
l.skal...@partner.samsung.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] build: fix problems with undefined references in tests

2013-11-25 Thread Kay Sievers
On Mon, Nov 25, 2013 at 2:08 PM, Lukasz Skalski
l.skal...@partner.samsung.com wrote:
 On 11/25/2013 02:03 PM, Kay Sievers wrote:

 On Mon, Nov 25, 2013 at 1:56 PM, Lukasz Skalski
 l.skal...@partner.samsung.com wrote:

 Please find patch in attachement for review - it fix problem
 with undefined references in some test-libsystemd-*-sym.c tests.


 It looks a bit weird to add library requirements, theses libs should
 not need the other libraries. What exactly fails on your side?

 Yes I know, but otherwise:

   CCLD   test-libsystemd-daemon-sym
   CC test-libsystemd-id128-sym.o
   CCLD   test-libsystemd-id128-sym
 ./.libs/libsystemd-id128.so: undefined reference to `sd_bus_label_escape'
 ./.libs/libsystemd-id128.so: undefined reference to `sd_bus_label_unescape'
 collect2: ld returned 1 exit status
 make[2]: *** [test-libsystemd-id128-sym] Error 1
 make[1]: *** [all-recursive] Error 1
 make: *** [all] Error 2

That's caused by a broken code layout in the systemd source tree,
src/shared/ should not use the code of the shared libs directly, it's
a cyclic dep, which your simpler build-sys seem unable to handle:
  src/shared/unit-name.c:e = sd_bus_label_escape(name);

This should not be fixed by adding the bus lib to all tools that use
shared/ code, but the code currently the lib should move to shared/ if
it's needed in shared/.

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


[systemd-devel] [PATCH] conf-parser: fix memory realloc error

2013-11-25 Thread Yin Kangkai
Otherwise there is some memory corruption and undefined behavior,
e.g., in my case systemd-udev was always aborted at the
_cleanup_freep_ around that code blocks.

Signed-off-by: Yin Kangkai kangkai@intel.com
---
 src/shared/conf-parser.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
index 312315b..c194a07 100644
--- a/src/shared/conf-parser.h
+++ b/src/shared/conf-parser.h
@@ -199,7 +199,7 @@ int log_syntax_internal(const char *unit, int level,
 continue;  
\

\
 *(xs + i) = x; 
\
-xs = realloc(xs, ++i + 1); 
\
+xs = realloc(xs, (++i + 1) * sizeof(type));
\
 if (!xs)   
\
 return -ENOMEM;
\
 *(xs + i) = invalid;   
\
-- 
1.8.2.1

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


Re: [systemd-devel] Thread level resource management

2013-11-25 Thread Umut Tezduyar Lindskog

Hi,

On Nov 24, 2013, at 4:39 PM, Kay Sievers k...@vrfy.org wrote:

 On Sun, Nov 24, 2013 at 12:33 PM, Umut Tezduyar Lindskog
 umut.tezdu...@axis.com wrote:
 How do we support thread level resource management with the new cgroup 
 abstraction?
How do we do it in the process level then. Lets say a service has 5 processes 
under and 1 of them needs to be in a different slice. Any example?
 
 Can we use scopes with task ids of threads? If so, what is the API to put 
 the task id into its own scope unit?
 
 There is no plan at the moment to support thread-granular resource
 settings. The feature is expected to be removed from the kernel too,
 when we switch to the unified cgroup hierarchy.
Any plans to support existing applications that are making use of thread level 
resource management? If not, what are we left with then, posix thread 
priorities?
Is there an announcement of dropping thread level resource management support 
from kernel somewhere?
 
 Kay

Thanks,
Umut

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


Re: [systemd-devel] Thread level resource management

2013-11-25 Thread Kay Sievers
On Mon, Nov 25, 2013 at 5:28 PM, Umut Tezduyar Lindskog
umut.tezdu...@axis.com wrote:
 On Nov 24, 2013, at 4:39 PM, Kay Sievers k...@vrfy.org wrote:
 On Sun, Nov 24, 2013 at 12:33 PM, Umut Tezduyar Lindskog
 umut.tezdu...@axis.com wrote:
 How do we support thread level resource management with the new cgroup 
 abstraction?
 How do we do it in the process level then. Lets say a service has 5 processes 
 under and 1 of them needs to be in a different slice. Any example?

 Can we use scopes with task ids of threads? If so, what is the API to put 
 the task id into its own scope unit?

 There is no plan at the moment to support thread-granular resource
 settings. The feature is expected to be removed from the kernel too,
 when we switch to the unified cgroup hierarchy.
 Any plans to support existing applications that are making use of thread 
 level resource management?

No, the current idea is that there will be no replacement.

 If not, what are we left with then, posix thread priorities?

Something else than cgroups, yes. Thread prios could work if it fits
the way your tool works. And there are some ideas of adding new
interfaces to /proc/$PID/, to provide these thread-level knobs.

We will see when we get there. Only one thing seems clear, threads are
to be managed by the process, not by cgroups.

 Is there an announcement of dropping thread level resource management support 
 from kernel somewhere?

Nothing official, there are just discussions about the unified hierarchy.

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


[systemd-devel] [PATCH v2] build: fix problems with undefined references in tests

2013-11-25 Thread Lukasz Skalski
Now code form src/shared don't use code of shared libs directly.
It solves cyclic dependencies in the same way as sd_utf_is_valid()
from src/libsystemd-bus/sd-utf8.c and src/shared/utf8.c.
---
 src/libsystemd-bus/sd-bus.c   |   64 +
 src/libsystemd-bus/test-bus-marshal.c |   25 
 src/shared/unit-name.c|5 +--
 src/shared/util.c |   72 +
 src/shared/util.h |3 ++
 src/test/test-util.c  |   24 +++
 6 files changed, 103 insertions(+), 90 deletions(-)

diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c
index 1207d5a..3042802 100644
--- a/src/libsystemd-bus/sd-bus.c
+++ b/src/libsystemd-bus/sd-bus.c
@@ -2741,73 +2741,13 @@ _public_ int sd_bus_get_tid(sd_bus *b, pid_t *tid) {
 }
 
 _public_ char *sd_bus_label_escape(const char *s) {
-char *r, *t;
-const char *f;
-
 assert_return(s, NULL);
 
-/* Escapes all chars that D-Bus' object path cannot deal
- * with. Can be reversed with bus_path_unescape(). We special
- * case the empty string. */
-
-if (*s == 0)
-return strdup(_);
-
-r = new(char, strlen(s)*3 + 1);
-if (!r)
-return NULL;
-
-for (f = s, t = r; *f; f++) {
-
-/* Escape everything that is not a-zA-Z0-9. We also
- * escape 0-9 if it's the first character */
-
-if (!(*f = 'A'  *f = 'Z') 
-!(*f = 'a'  *f = 'z') 
-!(f  s  *f = '0'  *f = '9')) {
-*(t++) = '_';
-*(t++) = hexchar(*f  4);
-*(t++) = hexchar(*f);
-} else
-*(t++) = *f;
-}
-
-*t = 0;
-
-return r;
+return bus_path_escape(s);
 }
 
 _public_ char *sd_bus_label_unescape(const char *f) {
-char *r, *t;
-
 assert_return(f, NULL);
 
-/* Special case for the empty string */
-if (streq(f, _))
-return strdup();
-
-r = new(char, strlen(f) + 1);
-if (!r)
-return NULL;
-
-for (t = r; *f; f++) {
-
-if (*f == '_') {
-int a, b;
-
-if ((a = unhexchar(f[1]))  0 ||
-(b = unhexchar(f[2]))  0) {
-/* Invalid escape code, let's take it literal 
then */
-*(t++) = '_';
-} else {
-*(t++) = (char) ((a  4) | b);
-f += 2;
-}
-} else
-*(t++) = *f;
-}
-
-*t = 0;
-
-return r;
+return bus_path_unescape(f);
 }
diff --git a/src/libsystemd-bus/test-bus-marshal.c 
b/src/libsystemd-bus/test-bus-marshal.c
index da072c7..b6e2c7b 100644
--- a/src/libsystemd-bus/test-bus-marshal.c
+++ b/src/libsystemd-bus/test-bus-marshal.c
@@ -39,29 +39,6 @@
 #include bus-util.h
 #include bus-dump.h
 
-static void test_bus_label_escape_one(const char *a, const char *b) {
-_cleanup_free_ char *t = NULL, *x = NULL, *y = NULL;
-
-assert_se(t = sd_bus_label_escape(a));
-assert_se(streq(t, b));
-
-assert_se(x = sd_bus_label_unescape(t));
-assert_se(streq(a, x));
-
-assert_se(y = sd_bus_label_unescape(b));
-assert_se(streq(a, y));
-}
-
-static void test_bus_label_escape(void) {
-test_bus_label_escape_one(foo123bar, foo123bar);
-test_bus_label_escape_one(foo.bar, foo_2ebar);
-test_bus_label_escape_one(foo_2ebar, foo_5f2ebar);
-test_bus_label_escape_one(, _);
-test_bus_label_escape_one(_, _5f);
-test_bus_label_escape_one(1, _31);
-test_bus_label_escape_one(:1, _3a1);
-}
-
 int main(int argc, char *argv[]) {
 _cleanup_bus_message_unref_ sd_bus_message *m = NULL, *copy = NULL;
 int r, boolean;
@@ -317,7 +294,5 @@ int main(int argc, char *argv[]) {
 assert_se(streq(c, ccc));
 assert_se(streq(d, 3));
 
-test_bus_label_escape();
-
 return 0;
 }
diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index 2335463..bc8094d 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -23,7 +23,6 @@
 #include string.h
 #include assert.h
 
-#include sd-bus.h
 #include path-util.h
 #include util.h
 #include unit-name.h
@@ -460,7 +459,7 @@ char *unit_dbus_path_from_name(const char *name) {
 
 assert(name);
 
-e = sd_bus_label_escape(name);
+e = bus_path_escape(name);
 if (!e)
 return NULL;
 
@@ -475,7 +474,7 @@ int unit_name_from_dbus_path(const char *path, char **name) 
{
 if (!e)
 return -EINVAL;
 
-n = sd_bus_label_unescape(e);
+n 

Re: [systemd-devel] [PATCH v2] build: fix problems with undefined references in tests

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 18:26, Lukasz Skalski (l.skal...@partner.samsung.com) wrote:

 Now code form src/shared don't use code of shared libs directly.
 It solves cyclic dependencies in the same way as sd_utf_is_valid()
 from src/libsystemd-bus/sd-utf8.c and src/shared/utf8.c.

Hmm, I'd prefer just moving the code in unit-names.[ch] that needs this
into some new convenience library that links against
libsystemd-bus. Using these calls without linking against libsystem-bus
anyway makes little sense, hence that sounds like the better approach to
me...

 ---
  src/libsystemd-bus/sd-bus.c   |   64 +
  src/libsystemd-bus/test-bus-marshal.c |   25 
  src/shared/unit-name.c|5 +--
  src/shared/util.c |   72 
 +
  src/shared/util.h |3 ++
  src/test/test-util.c  |   24 +++
  6 files changed, 103 insertions(+), 90 deletions(-)
 
 diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c
 index 1207d5a..3042802 100644
 --- a/src/libsystemd-bus/sd-bus.c
 +++ b/src/libsystemd-bus/sd-bus.c
 @@ -2741,73 +2741,13 @@ _public_ int sd_bus_get_tid(sd_bus *b, pid_t *tid) {
  }
  
  _public_ char *sd_bus_label_escape(const char *s) {
 -char *r, *t;
 -const char *f;
 -
  assert_return(s, NULL);
  
 -/* Escapes all chars that D-Bus' object path cannot deal
 - * with. Can be reversed with bus_path_unescape(). We special
 - * case the empty string. */
 -
 -if (*s == 0)
 -return strdup(_);
 -
 -r = new(char, strlen(s)*3 + 1);
 -if (!r)
 -return NULL;
 -
 -for (f = s, t = r; *f; f++) {
 -
 -/* Escape everything that is not a-zA-Z0-9. We also
 - * escape 0-9 if it's the first character */
 -
 -if (!(*f = 'A'  *f = 'Z') 
 -!(*f = 'a'  *f = 'z') 
 -!(f  s  *f = '0'  *f = '9')) {
 -*(t++) = '_';
 -*(t++) = hexchar(*f  4);
 -*(t++) = hexchar(*f);
 -} else
 -*(t++) = *f;
 -}
 -
 -*t = 0;
 -
 -return r;
 +return bus_path_escape(s);
  }
  
  _public_ char *sd_bus_label_unescape(const char *f) {
 -char *r, *t;
 -
  assert_return(f, NULL);
  
 -/* Special case for the empty string */
 -if (streq(f, _))
 -return strdup();
 -
 -r = new(char, strlen(f) + 1);
 -if (!r)
 -return NULL;
 -
 -for (t = r; *f; f++) {
 -
 -if (*f == '_') {
 -int a, b;
 -
 -if ((a = unhexchar(f[1]))  0 ||
 -(b = unhexchar(f[2]))  0) {
 -/* Invalid escape code, let's take it 
 literal then */
 -*(t++) = '_';
 -} else {
 -*(t++) = (char) ((a  4) | b);
 -f += 2;
 -}
 -} else
 -*(t++) = *f;
 -}
 -
 -*t = 0;
 -
 -return r;
 +return bus_path_unescape(f);
  }
 diff --git a/src/libsystemd-bus/test-bus-marshal.c 
 b/src/libsystemd-bus/test-bus-marshal.c
 index da072c7..b6e2c7b 100644
 --- a/src/libsystemd-bus/test-bus-marshal.c
 +++ b/src/libsystemd-bus/test-bus-marshal.c
 @@ -39,29 +39,6 @@
  #include bus-util.h
  #include bus-dump.h
  
 -static void test_bus_label_escape_one(const char *a, const char *b) {
 -_cleanup_free_ char *t = NULL, *x = NULL, *y = NULL;
 -
 -assert_se(t = sd_bus_label_escape(a));
 -assert_se(streq(t, b));
 -
 -assert_se(x = sd_bus_label_unescape(t));
 -assert_se(streq(a, x));
 -
 -assert_se(y = sd_bus_label_unescape(b));
 -assert_se(streq(a, y));
 -}
 -
 -static void test_bus_label_escape(void) {
 -test_bus_label_escape_one(foo123bar, foo123bar);
 -test_bus_label_escape_one(foo.bar, foo_2ebar);
 -test_bus_label_escape_one(foo_2ebar, foo_5f2ebar);
 -test_bus_label_escape_one(, _);
 -test_bus_label_escape_one(_, _5f);
 -test_bus_label_escape_one(1, _31);
 -test_bus_label_escape_one(:1, _3a1);
 -}
 -
  int main(int argc, char *argv[]) {
  _cleanup_bus_message_unref_ sd_bus_message *m = NULL, *copy = NULL;
  int r, boolean;
 @@ -317,7 +294,5 @@ int main(int argc, char *argv[]) {
  assert_se(streq(c, ccc));
  assert_se(streq(d, 3));
  
 -test_bus_label_escape();
 -
  return 0;
  }
 diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
 index 2335463..bc8094d 100644
 --- a/src/shared/unit-name.c
 +++ b/src/shared/unit-name.c
 @@ -23,7 +23,6 @@
  #include string.h
  

Re: [systemd-devel] [PATCH] conf-parser: fix memory realloc error

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 23:14, Yin Kangkai (kangkai@intel.com) wrote:

 Otherwise there is some memory corruption and undefined behavior,
 e.g., in my case systemd-udev was always aborted at the
 _cleanup_freep_ around that code blocks.

Thanks! Applied!

 Signed-off-by: Yin Kangkai kangkai@intel.com

We don't do S-o-b in systemd, dropped this bit.

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 v2 00/26] Initial DHCP v4 library implementation

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 After that I noticed someting interesting when setting a monotonic timer
 event to go off at time 0. It seems to repeatedly call the timer event
 without stopping, either due to the next timer computation going wrong in
 my code or then the reshuffling of the event queue in sd-event. As I
 couldn't figure out the root cause, I left the timer setting to
 now(CLOCK_MONOTONIC) instead of the proposed value 0.

Hmm, are you aware of SD_EVENT_SOURCE_ON vs. SD_EVENT_SOURCE_ONESHOT?
The former leaves the event source on forever, and that means a timer is
dispatched over and over and over again (immediately, since the rule is
to execute a timer as soon as the configured time is passed, and that it
will be continously after it elapsed once), until it is explicitly
disabled. The latter is turned off automatically when it is first
dispatched. Of course, timer callbacks should only be called once, ence
ONESHOT mode is the default -- unless you change that explicitly.

If this doesn't solve the issue, I wouldn't be surprised if there was
still a bug lurking with the sd-event code. Note that I fixed a bug like
this last week, did you check with currentl git of sd-event if the issue
persists?

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 v2 02/26] dhcp: Add DHCP client initialization

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 +
 +DHCPClient *sd_dhcp_client_new(void)
 +{
 +DHCPClient *client;
 +
 +client = new0(DHCPClient, 1);
 +if (!client)
 +return NULL;
 +
 +client-state = DHCP_STATE_INIT;
 +client-index = -1;
 +
 +client-req_opts_size = ELEMENTSOF(default_req_opts);
 +
 +client-req_opts = memdup(default_req_opts,
 client-req_opts_size);

Missing OOM check?

 +
 +return client;
 +}
 diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h
 new file mode 100644
 index 000..65caf59
 --- /dev/null
 +++ b/src/systemd/sd-dhcp-client.h
 @@ -0,0 +1,33 @@
 +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
 +
 +#pragma once

If this is supposed to be public API, then it should not use the pragma
once stuff. That's a gcc'ism, which is fine for internal code, but for
external code we try hard to stay with C89. Please use #ifdef checks for
the public headers hence.

 +#include netinet/in.h
 +
 +typedef struct DHCPClient DHCPClient;

This also needs prefixing. The other public structures like this were
usually called in the style sd_dhcp_client.

 +int sd_dhcp_client_set_request_option(DHCPClient *client, const
 uint8_t option);

Hmm, what's a const uint8_t parameter supposed to be? const only
really makes sense with pointer parameters, but this ins't one...

 +int sd_dhcp_client_set_request_address(DHCPClient *client,
 +   const struct in_addr
 *last_address);

struct in_addr? Didn't you plan to use simple uint32_t for this?

Also, if this is supposed to cover ipv6 one day too, maybe call this
sd_dhcp_client_set_request_address4() or so?

 +int sd_dhcp_client_set_index(DHCPClient *client, const int interface_index);
 +
 +DHCPClient *sd_dhcp_client_new(void);

Hmm, for the public APIs we prefer returning newly allocated objects via
a call-by-ref pointer, and return an error code as return code. i.e.:

int sd_dhcp_client_new(sd_dhcp_client *ret);

or something similar. This leaves more room open to indicate more than
just OOM errors to the caller.

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 v2 05/26] dhcp: Add option appending and parsing

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 +#include stdint.h
 +
 +#include protocol.h
 +
 +int dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,

 

This is a memory size, right? It should be size_t rather than int, then, no?

 +static int parse_options(uint8_t *buf, int32_t buflen, int *overload,

^^ const missing?

 + int *message_type, dhcp_option_cb_t cb,
 + void *user_data)
 +{
 +uint8_t *code = buf;
 +uint8_t *len;
 +
 +while (buflen  0) {
 +switch (*code) {
 +case DHCP_OPTION_PAD:
 +buflen -= 1;
 +code++;
 +break;
 +
 +case DHCP_OPTION_END:
 +return 0;
 +
 +case DHCP_OPTION_MESSAGE_TYPE:
 +buflen -= 3;
 +if (buflen  0)
 +return -ENOBUFS;
 +
 +len = code + 1;
 +if (*len != 1)
 +return -EINVAL;
 +
 +if (message_type)
 +*message_type = *(len + 1);

Feels weird that message_type is an int, but you fill an uint8_t into
it, no?

 +if (buflen  0)
 +return -EINVAL;
 +
 +if (cb)
 +cb(*code, *len, len + 1, user_data);

I'd always be careful with callback functions like this, since people
might alter the objects you rely on while you iterate through things and
then you are seriously confused... I'd always do iterator-like
interfaces instead. (Doesn't matter here though, as this is an internal
interface only...)

 +int dhcp_option_parse(DHCPMessage *message, int32_t len,

Hmm, an int32_t length? Signed? And shouldn't it be size_t again?

 +  dhcp_option_cb_t cb, void *user_data)
 +{
 +int overload = 0;
 +int message_type = -ENOMSG;
 +uint8_t *opt = (uint8_t *)(message + 1);
 +int res;
 +
 +if (message == NULL || len  0)
 +return -EINVAL;
 +
 +len -= sizeof(DHCPMessage);
 +len -= 4;
 +if (len  0)
 +return -EINVAL;

Wouldn't it be better to check len = sizeof(DHCPMessage) + 4 first?
Sounds more logical than relying on a signed size...

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 v2 12/26] dhcp: Add function for sending a raw packet

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 +int dhcp_network_send_raw_packet(int index, void *packet, int len);

Should be const void* packet, no?

And size_t len, no?

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 v2 19/26] dhcp: Add timeout and main loop support

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 -DHCPClient *sd_dhcp_client_new(void)
 +DHCPClient *sd_dhcp_client_new(sd_event *event)
  {
  DHCPClient *client;
  
 +assert_return(event, NULL);
 +
  client = new0(DHCPClient, 1);
  if (!client)
  return NULL;
  
 +client-event = event;

You should probably take a reference here with sd_event_ref(), and then
unref it again in your destructor.

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 v2 20/26] dhcp: Handle received DHCP Offer message

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 +static int client_receive_raw_message(sd_event_source *s, int fd,
 +  uint32_t revents, void *userdata)
 +{
 +DHCPClient *client = userdata;
 +int len, buflen;
 +uint8_t *buf;
 +uint8_t tmp;
 +DHCPPacket *message;
 +
 +buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
 +buf = malloc0(buflen);
 +if (!buf) {
 +read(fd, tmp, 1);
 +return 0;
 +}

Hmm, how long is this message? Given that you don't keep the memory
around, why not just allocate this on the stack? Either with alloca, or
even directly as an uint8_t array? Given that this seems to be
relatively short, this should not be an issue and is one error source
less...

 +
 +len = read(fd, buf, buflen);
 +if (len  0)
 +goto error;
 +
 +message = (DHCPPacket *)buf;
 +
 +switch (client-state) {
 +case DHCP_STATE_SELECTING:
 +
 +if (client_receive_offer(client, message, len) = 0) {
 +
 +close(client-fd);
 +client-fd = -1;
 +client-receive_message =
 +
 sd_event_source_unref(client-receive_message);


You should unref the source before closing the fd, as this internally
still references the fd.

 -int dhcp_network_send_raw_packet(int index, void *packet, int len)
 +int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
 + void *packet, int len)

Hmm, what's the story regarding blocking here? What happens if the
socket is full? This call will block then, which sucks. Also, if you are
not using SOCK_NONBLOCK on these sockets, epoll behaviour can be weird,
as the socket layer takes the freedom to wake you up sometimes without
any packet to read. (We had the issue in Avahi).

It sounds to me as if you should set SOCK_NONBLOCK and then set a socket
send buffer (SO_SNDBUF) as large as useful, and simply consider it a
real error if you the ever get EAGAIN on sending...

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 v2 23/26] dhcp: Process DHCP Ack/Nak message

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 -static int client_receive_offer(DHCPClient *client,
 -DHCPPacket *offer, int len)
 +static int client_verify_headers(DHCPClient *client,
 + DHCPPacket *message, int len)

Thhis all looks like size_t len is the right param here, not int.

 +lease = new0(DHCPLease, 1);
 +if (!lease)
 +return -ENOBUFS;

OOM should result in ENOMEM, nothing else please. ENOBUFS is something
else: it indicates some local limit to be hit, not a global memory
shortage.

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] src/network

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 15:20, Dave Reisner (dreis...@kemper.freedesktop.org) wrote:

 uint64_t can be formatted correctly with %ju, rather than casting to
 unsigned and potentially losing accuracy.

Oh, shouldn't we be careful with that? %j is for intmax_t. Which might
or might not be int64_t. Given that int128_t is already on the horizon
(newer gcc already support __int128 on 64bit machines...), I wouldn't be
surprised if intmax_t is growing to 128bit eventually.

Format strings don't really have a nice way to print fixed-size
integers I fear... the only stuff that is correct is the PRIu64 macro,
but that's fricking ugly...

I think PRIu64 is still a better, more future-proof option than %j though...

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] syslog makes impossible to enter emergency mode

2013-11-25 Thread Lennart Poettering
On Sun, 24.11.13 22:36, Andrey Borzenkov (arvidj...@gmail.com) wrote:

 Interesting case (https://bugzilla.novell.com/show_bug.cgi?id=852021).
 Systemd enters emergency due to failed mount. At the same time syslog
 socket triggers syslog.service. Due to implicit Requires on
 basic.target which Requires sysinit.target which conflicts with
 emergency.{service,target} syslog.service tries to start basic.target
 (it is not there yet ...) which apparently kills emergency shell.

This was probably introduced by
80cfe9e163b1c92f917e0a5e053b148fca790677.

I figure we should find something in the middle of
OnActivationIsolate=yes and OnActivationIsolate=no. i.e. make use of
the replace-irreversible job mode which will allow the emergency jobs
to be queued without being reversible by later requests until they are
finished or explicitly flushed out with systemctl cancel.

I figure I'll replace OnActivationIsolate=yes by OnActivationMode= which
takes the full range of job modes, and then turn OnActiveIsolate= into a
hidden compat switch

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] src/network

2013-11-25 Thread Tom Gundersen
On Tue, Nov 26, 2013 at 12:40 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Mon, 25.11.13 15:20, Dave Reisner (dreis...@kemper.freedesktop.org) wrote:

 uint64_t can be formatted correctly with %ju, rather than casting to
 unsigned and potentially losing accuracy.

 Oh, shouldn't we be careful with that? %j is for intmax_t. Which might
 or might not be int64_t. Given that int128_t is already on the horizon
 (newer gcc already support __int128 on 64bit machines...), I wouldn't be
 surprised if intmax_t is growing to 128bit eventually.

 Format strings don't really have a nice way to print fixed-size
 integers I fear... the only stuff that is correct is the PRIu64 macro,
 but that's fricking ugly...

 I think PRIu64 is still a better, more future-proof option than %j though...

I'm about to change all this stuff to print the ifname rather than the
index, so please leave it as is for now.

Cheers,

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


[systemd-devel] [PATCH] nspawn: Give a more helpful error message when -D argument is bogus.

2013-11-25 Thread Shawn Landden
---
 src/nspawn/nspawn.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0973a00..1ee4ab3 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -200,6 +200,11 @@ static int parse_argv(int argc, char *argv[]) {
 
 case 'D':
 free(arg_directory);
+if (access(optarg, F_OK)  0) {
+log_error(Root directory %s is not 
accessable: %m, optarg);
+return -EACCES;
+}
+
 arg_directory = canonicalize_file_name(optarg);
 if (!arg_directory) {
 log_error(Failed to canonicalize root 
directory.);
-- 
1.8.4.4

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


Re: [systemd-devel] [systemd-commits] src/network

2013-11-25 Thread Dave Reisner
On Tue, Nov 26, 2013 at 12:40:05AM +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 15:20, Dave Reisner (dreis...@kemper.freedesktop.org) wrote:
 
  uint64_t can be formatted correctly with %ju, rather than casting to
  unsigned and potentially losing accuracy.
 
 Oh, shouldn't we be careful with that? %j is for intmax_t. Which might
 or might not be int64_t. Given that int128_t is already on the horizon
 (newer gcc already support __int128 on 64bit machines...), I wouldn't be
 surprised if intmax_t is growing to 128bit eventually.

How do you change sizeof(uintmax_t) without breaking ABI?

 Format strings don't really have a nice way to print fixed-size
 integers I fear... the only stuff that is correct is the PRIu64 macro,
 but that's fricking ugly...
 
 I think PRIu64 is still a better, more future-proof option than %j though...

Assuming uintmax_t really does increase in size, no type promotion, and
someone actually manages to conjure up a system with 4 billion
interfaces...
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] nspawn: Give a more helpful error message when -D argument is bogus.

2013-11-25 Thread Michael Biebl
2013/11/26 Shawn Landden sh...@churchofgit.com:
 +log_error(Root directory %s is not 
 accessable: %m, optarg);

s/accessable/accessible/




-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] src/network

2013-11-25 Thread Tom Gundersen
On Tue, Nov 26, 2013 at 1:05 AM, Tom Gundersen t...@jklm.no wrote:
 On Tue, Nov 26, 2013 at 12:40 AM, Lennart Poettering
 lenn...@poettering.net wrote:
 On Mon, 25.11.13 15:20, Dave Reisner (dreis...@kemper.freedesktop.org) wrote:

 uint64_t can be formatted correctly with %ju, rather than casting to
 unsigned and potentially losing accuracy.

 Oh, shouldn't we be careful with that? %j is for intmax_t. Which might
 or might not be int64_t. Given that int128_t is already on the horizon
 (newer gcc already support __int128 on 64bit machines...), I wouldn't be
 surprised if intmax_t is growing to 128bit eventually.

 Format strings don't really have a nice way to print fixed-size
 integers I fear... the only stuff that is correct is the PRIu64 macro,
 but that's fricking ugly...

 I think PRIu64 is still a better, more future-proof option than %j though...

 I'm about to change all this stuff to print the ifname rather than the
 index, so please leave it as is for now.

Done.

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


Re: [systemd-devel] syslog makes impossible to enter emergency mode

2013-11-25 Thread Lennart Poettering
On Tue, 26.11.13 01:00, Lennart Poettering (lenn...@poettering.net) wrote:

 
 On Sun, 24.11.13 22:36, Andrey Borzenkov (arvidj...@gmail.com) wrote:
 
  Interesting case (https://bugzilla.novell.com/show_bug.cgi?id=852021).
  Systemd enters emergency due to failed mount. At the same time syslog
  socket triggers syslog.service. Due to implicit Requires on
  basic.target which Requires sysinit.target which conflicts with
  emergency.{service,target} syslog.service tries to start basic.target
  (it is not there yet ...) which apparently kills emergency shell.
 
 This was probably introduced by
 80cfe9e163b1c92f917e0a5e053b148fca790677.
 
 I figure we should find something in the middle of
 OnActivationIsolate=yes and OnActivationIsolate=no. i.e. make use of
 the replace-irreversible job mode which will allow the emergency jobs
 to be queued without being reversible by later requests until they are
 finished or explicitly flushed out with systemctl cancel.
 
 I figure I'll replace OnActivationIsolate=yes by OnActivationMode= which
 takes the full range of job modes, and then turn OnActiveIsolate= into a
 hidden compat switch

Done.

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] src/network

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 19:12, Dave Reisner (d...@falconindy.com) wrote:

 
 On Tue, Nov 26, 2013 at 12:40:05AM +0100, Lennart Poettering wrote:
  On Mon, 25.11.13 15:20, Dave Reisner (dreis...@kemper.freedesktop.org) 
  wrote:
  
   uint64_t can be formatted correctly with %ju, rather than casting to
   unsigned and potentially losing accuracy.
  
  Oh, shouldn't we be careful with that? %j is for intmax_t. Which might
  or might not be int64_t. Given that int128_t is already on the horizon
  (newer gcc already support __int128 on 64bit machines...), I wouldn't be
  surprised if intmax_t is growing to 128bit eventually.
 
 How do you change sizeof(uintmax_t) without breaking ABI?

Well, there's prior art for botching such type size changes up, they
could just take inspiration from that and fail awesomely there too:
off_t...

  Format strings don't really have a nice way to print fixed-size
  integers I fear... the only stuff that is correct is the PRIu64 macro,
  but that's fricking ugly...
  
  I think PRIu64 is still a better, more future-proof option than %j though...
 
 Assuming uintmax_t really does increase in size, no type promotion, and
 someone actually manages to conjure up a system with 4 billion
 interfaces...

Well, the latter only matters on LE machines. On BE you'd be immediately
broken...

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] Thread level resource management

2013-11-25 Thread David Timothy Strauss
On Tue, Nov 26, 2013 at 2:28 AM, Umut Tezduyar Lindskog
umut.tezdu...@axis.com wrote:
 Any plans to support existing applications that are making use of thread 
 level resource management? If not, what are we left with then, posix thread 
 priorities?

Kay is right; there is no direct replacement. However, it's possible
to fork processes on Linux with many of the same properties as threads
(shared memory and file descriptors). Originally, all POSIX-like
(loosely using like here) threads used this capability back with
LinuxThreads. Using clone() that way would allow continued use of
cgroups resources.

Or, perhaps it's a good time to consider a more loosely coupled
architecture with multiple processes or daemons.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Improving the results shown with systemd-analyze blame

2013-11-25 Thread Lennart Poettering
On Sun, 24.11.13 12:44, Umut Tezduyar Lindskog (umut.tezdu...@axis.com) wrote:

 Hi,
 
 I think we can make the output of systemd-analyze blame more useful with cpu 
 accounting information.
 
 The output of systemd-analyze blame is not making so much sense when
 lots of programs are tried to be started at the same time since it is
 not known which program actually uses the cpu at any time. My propose
 is saving the cpuacct (if it is enabled of course) information
 throughout service activation and presenting the cpu usage information
 in systemd-analyze blame. We could even present the time it takes to
 activate a service / cpu time that is used to have information about
 overall cpu utilization.
 
 Thoughts?

What to do about accounting of cgroups is a big open question. In the
long run thre should probably be some infrastructure that allows
integration with RRD tools in one way or another.

In the short-term it probably makes sense to expose some of cgroup
accounting parameters in some minimal way on the bus, and possibly take
a snapshot of them automatically at appropriate times (i.e. when the
state of a unit changes), and all that in a somewhat atomic way.

Maybe something like this could work:

For each Unit with cgroups add four new properties and one new
method. The props are:

a{sv} InactiveExitAccounting
a{sv} ActiveEnterAccounting
a{sv} ActiveExitAccounting
a{sv} InactiveEnterAccounting

The methods is:

a{sv} GetCurrentAccounting()

The returned dictionaries would contain the names of a selected few
accounting parameters plus their values. Which ones are included depends
on the existing CPUAccounting, MemoryAccounting, BlockIOAccounting
fields. We'd only include values that according to Tejun are here to
stay.

The properties would reflect the accounting data of the four timestamps
we already take. All values would be normalized, and relative to the
data from InactiveExitTimestamp.

Hope that makes some sense? Would be happy to take a patch!

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] Restart instantiated services after suspend

2013-11-25 Thread Lennart Poettering
On Sat, 23.11.13 18:51, Marcos Felipe Rasia de Mello (marcos...@gmail.com) 
wrote:

 
 2013/11/20 Lennart Poettering lenn...@poettering.net:
  On Tue, 19.11.13 14:18, Marcos Felipe Rasia de Mello (marcos...@gmail.com) 
  wrote:
 
  Hi folks,
 
  I am trying to disable HDDs power management in a systemd way (aka no
  shell scripts :)
 
  /etc/udev/rules.d/99-hdparm.rules
 
  SUBSYSTEM==block, KERNEL==sd*, ATTR{removable}==0,
  TAG+=systemd, ENV{SYSTEMD_WANTS}+=hdparm@%k.service
 
  Why do you do this asyncrhonously via a systemd service? Given that this
  tool runs and terminates quickly invokign this with RUN inside of the
  udev rule itself sounds like the best approach.
 
 
 If suspend/resume did not reset HDD power setting, sure, better run
 hdparm directly from the udev rule.
 
 My initial idea was rerun hdparm@ transient units after suspend with
 something like what you have suggested:
 
 mkdir /etc/systemd/system/sleep.target.wants
 ln -s /etc/systemd/system/hdparm@.service
 /etc/systemd/system/sleep.target.wants/
 
 but it did not work so here I am. :)

Maybe you forget systemctl daemon-reload?

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] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]

2013-11-25 Thread Lennart Poettering
On Thu, 21.11.13 00:36, Lennart Poettering (lenn...@poettering.net) wrote:

 I do like Martin's original patch, since by unsetting XDG_RUNTIME_DIR it
 basically tells apps Hey, all bets are off, you are fucked, and
 doesn't pretend XDG_RUNTIME_DIR would still work. Because it doesn't.

I implemented this now, using a different approach than Martin's
original patch (i.e. I don't think it is a good idea to involve stat()
here, instead let's just let logind pass all information to
pam_systemd).

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] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]

2013-11-25 Thread Lennart Poettering
On Wed, 20.11.13 19:19, Colin Walters (walt...@verbum.org) wrote:

  So yeah, there your mix
  and match is broken: 
 
 I'm proposing a simple goal: XDG_RUNTIME_DIR should always be that
 matching the current uid.  I can't think of any case where you'd
 want it otherwise.

That can't work. As the directory only exists when a real login session
is around. su/sudo don't get their own login sessins, hence the dir
doesn't necessarily exist and from the perspective of the code running
in su/sudo the lifetime semantics of the dir wouldn't match any
expections...

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] Fix PAM module to not clobber XDG_RUNTIME_DIR with su

2013-11-25 Thread Lennart Poettering
On Thu, 21.11.13 07:55, Martin Pitt (martin.p...@ubuntu.com) wrote:

  So, what's the intention here? That XDG_RUNTIME_DIR is entirely unset
  after su? That sounds kinda acceptable to me.
 Yes, for su - and pkexec. It might not work for su as that is
 likely configured to not run PAM, see above. 
 
 That's what my patch is doing and what would prevent the damaging of
 runtime dirs. It's not what I consider ideal (I prefer Colin's
 approach of giving him the *correct* user's runtime dir), but if we
 can't have that, let's at least not pass the wrong one.

Due to the lifecycle guarantees on XDG_RUNTIME_DIR we cannot hand out a
correct (correct by what I assume you mean by correct) instance of
it wihtout also setting up a full new session, and keeping it around and
ref counting it.

Anyway, as mentioned elsewhere, git now will not set XDG_RUNTIME_DIR in
su instances, as per your original patch.

Thanks for the patch,

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 v2 19/26] dhcp: Add timeout and main loop support

2013-11-25 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Nov 25, 2013 at 09:13:35AM +0200, Patrik Flykt wrote:
 Require a main loop to be set when creating a DHCP client. Set up
 a timer to resend DHCP Discover messages and add a 0-2 second
 delay to the timeout value. Move to state Selecting after successful
 sending of a Discover message.
 ---
 v2: - previous 20/28
 - use usec_t
 - adding monotonic event with timeout 0 seems to keep calling the
   timeout function continuosly, use now() for the time being
 
  src/dhcp/client.c|   74 
 --
  src/dhcp/test-dhcp-client.c  |   19 +++
  src/systemd/sd-dhcp-client.h |4 ++-
  3 files changed, 87 insertions(+), 10 deletions(-)
 
 diff --git a/src/dhcp/client.c b/src/dhcp/client.c
 index 8735efe..7af0075 100644
 --- a/src/dhcp/client.c
 +++ b/src/dhcp/client.c
 @@ -34,12 +34,15 @@
  
  struct DHCPClient {
  DHCPState state;
 +sd_event *event;
 +sd_event_source *timeout_resend;
  int index;
  uint8_t *req_opts;
  size_t req_opts_size;
  uint32_t last_addr;
  struct ether_addr mac_addr;
  uint32_t xid;
 +usec_t start_time;
  };
  
  static const uint8_t default_req_opts[] = {
 @@ -124,6 +127,8 @@ static int client_stop(DHCPClient *client, int error)
  assert_return(client-state != DHCP_STATE_INIT 
client-state != DHCP_STATE_INIT_REBOOT, -EALREADY);
  
 +client-timeout_resend = 
 sd_event_source_unref(client-timeout_resend);
 +
  switch (client-state) {
  
  case DHCP_STATE_INIT:
 @@ -277,8 +282,59 @@ static int client_send_discover(DHCPClient *client, 
 uint16_t secs)
  return 0;
  }
  
 +static int client_timeout_resend(sd_event_source *s, uint64_t usec,
 + void *userdata)
 +{
 +DHCPClient *client = userdata;
 +usec_t next_timeout;
 +uint16_t secs;
 +int err = 0;
 +
 +switch (client-state) {
 +case DHCP_STATE_INIT:
 +case DHCP_STATE_SELECTING:
 +
 +if (!client-start_time)
 +client-start_time = usec;
 +
 +secs = (usec - client-start_time) / USEC_PER_SEC;
 +
 +next_timeout = usec + 2 * USEC_PER_SEC + (random()  
 0x1f);
 +
 +err = sd_event_add_monotonic(client-event, next_timeout,
 + 10 * USEC_PER_MSEC,
 + client_timeout_resend, client,
 + client-timeout_resend);
 +if (err  0)
 +goto error;
 +
 +if (client_send_discover(client, secs) = 0)
 +client-state = DHCP_STATE_SELECTING;
 +
 +break;
 +
 +case DHCP_STATE_INIT_REBOOT:
 +case DHCP_STATE_REBOOTING:
 +case DHCP_STATE_REQUESTING:
 +case DHCP_STATE_BOUND:
 +case DHCP_STATE_RENEWING:
 +case DHCP_STATE_REBINDING:
 +
 +break;
 +}
 +
 +return 0;
 +
 +error:
 +client_stop(client, err);
 +
 +return 0;
Return err?

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


Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets

2013-11-25 Thread Lennart Poettering
On Fri, 22.11.13 18:56, Shawn Landden (sh...@churchofgit.com) wrote:

 With EPOLLONESHOT we are guaranteed to only recieve one event
 until we reload with socket_enter_listening(s), otherwise
 multiple events can be generated upon receipt of multiple chunks of
 data.

Not following here. What precisely does this fix, can you elaborate?

We currently turn off the poll for the socket fds as soon as we queued
the service socket, so that we don't get woken up anymore. What would
EPOLLET do good here?

 
 We also only want wake-ups when external events happen, i.e.
 edge-triggered wakeups.
 ---
  src/core/socket.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/src/core/socket.c b/src/core/socket.c
 index 5fa4a5a..2f860a4 100644
 --- a/src/core/socket.c
 +++ b/src/core/socket.c
 @@ -1119,7 +1119,9 @@ static int socket_watch_fds(Socket *s) {
  if (p-event_source)
  r = sd_event_source_set_enabled(p-event_source, 
 SD_EVENT_ON);
  else
 -r = sd_event_add_io(UNIT(s)-manager-event, p-fd, 
 EPOLLIN, socket_dispatch_io, p, p-event_source);
 +r = sd_event_add_io(UNIT(s)-manager-event, p-fd,
 +EPOLLIN|EPOLLET|((s-accept || 
 s-distribute) ? 0 : EPOLLONESHOT),
 +socket_dispatch_io, p, 
 p-event_source);
  
  if (r  0) {
  log_warning_unit(UNIT(s)-id, Failed to watch 
 listening fds: %s, strerror(-r));


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] core/socket: we only want one event on standard sockets

2013-11-25 Thread David Timothy Strauss
On Tue, Nov 26, 2013 at 2:35 PM, Lennart Poettering
lenn...@poettering.net wrote:
 Not following here. What precisely does this fix, can you elaborate?

 We currently turn off the poll for the socket fds as soon as we queued
 the service socket, so that we don't get woken up anymore. What would
 EPOLLET do good here?

I think he's been working on having the distribute pool scale up
on-demand, which would involve systemd getting events on new
connections coming into the listener socket. More specifically, I
think he intends to, as each new connection comes in, check if we've
maxed out the number of processes. If not, spin another one up.
Presumably, we'd drop the listener once the max-size pool is running.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]

2013-11-25 Thread Martin Pitt
Hey Lennart,

Lennart Poettering [2013-11-26  5:12 +0100]:
 I implemented this now, using a different approach than Martin's
 original patch (i.e. I don't think it is a good idea to involve stat()
 here, instead let's just let logind pass all information to
 pam_systemd).

Thanks!

Lennart Poettering [2013-11-26  5:17 +0100]:
 That can't work. As the directory only exists when a real login session
 is around. su/sudo don't get their own login sessins, hence the dir
 doesn't necessarily exist and from the perspective of the code running
 in su/sudo the lifetime semantics of the dir wouldn't match any
 expections...

Right, as long as they don't actually get one. I (and I think Colin)
argued that su -/pkexec should (just like ssh localhost), as they
run a full PAM stack which is like logging in. But let's agree to
disagree at this point.

I'm happy that the not your own runtime dir issue is fixed now at
least.

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