Re: [systemd-devel] networkd with radv advertised prefixes
2014-08-15 4:43 GMT+04:00 Lennart Poettering : > router advertisment allows passing along DNS server info. The kernel > can't make use of that, but we cetrainly should make use of it in > userspace. Yes, rdnss (DNS services) and dnssl (domain search list) -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru jabber: v...@selfip.ru ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: don't complain if cgroup property is missing
On Fri, Aug 15, 2014 at 2:45 AM, Lennart Poettering wrote: > On Fri, 15.08.14 02:42, Lennart Poettering (lenn...@poettering.net) wrote: > >> >> On Tue, 15.07.14 11:53, Umut Tezduyar Lindskog (umut.tezdu...@axis.com) >> wrote: >> >> Looks Ok, but doesn't apply to currently git anymore... > > (In case this wasn't clear, please rebase and I'll commit this. -- Or > actually, could you add a short comment explaining that EACCES is what > the kernel returns if CFS quotas are disabled?) Hi, I think there is a misunderstanding. Cgroup properties do not exist if they are turned off. And since your concern is EACCESS, I think a "file exists?" check inside cg_set_attribute before we call "write_string_file" is more proper in this case. Thanks, Umut Following is an example system with only few attributes enabled. [root@axis-00408cc0 /sys/fs/cgroup/cpu]26687# ls -al drwxr-xr-x2 root root 0 Aug 14 12:44 . drwxr-xr-x4 root root 120 Aug 14 12:44 .. -rw-r--r--1 root root 0 Aug 14 12:44 cgroup.clone_children -rw-r--r--1 root root 0 Aug 15 07:03 cgroup.procs -r--r--r--1 root root 0 Aug 14 12:44 cgroup.sane_behavior -rw-r--r--1 root root 0 Aug 14 12:44 cpu.shares -r--r--r--1 root root 0 Aug 14 12:44 cpuacct.stat -rw-r--r--1 root root 0 Aug 14 12:44 cpuacct.usage -r--r--r--1 root root 0 Aug 14 12:44 cpuacct.usage_percpu -rw-r--r--1 root root 0 Aug 14 12:44 notify_on_release -rw-r--r--1 root root 0 Aug 14 12:44 release_agent -rw-r--r--1 root root 0 Aug 14 12:44 tasks > > 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] Changing configurations with networkd
On Thu, Aug 14, 2014 at 05:38:05PM +0200, Lennart Poettering wrote: > On Fri, 25.07.14 09:48, Michael Olbrich (m.olbr...@pengutronix.de) wrote: > > > What I'm _not_ seeing, and what usually comes when anything else changes in > > the network configuration is: > > systemd-timesyncd[348]: Network configuration changed, trying to establish > > connection. > > > > I would expect, that systemd-timesyncd should be notified in this case as > > well, right? > > This should be fixed with current git. Could you please recheck? Indeed: Aug 01 00:20:15 BaseKit systemd-networkd[434]: eth0: removed address: 192.168.51.144/24 (valid for 0) Aug 01 00:20:15 BaseKit systemd-timesyncd[346]: Network configuration changed, trying to establish connection. Aug 01 00:20:15 BaseKit systemd-networkd[434]: eth0: added address: 192.168.51.145/24 (valid for 9min 30s) Aug 01 00:20:15 BaseKit systemd-timesyncd[346]: Network configuration changed, trying to establish connection. I'm not sure, why the new address is found again though. Note: this is with "net.ipv4.conf.all.promote_secondaries = 1". Setting just "net.ipv4.conf.default.promote_secondaries = 1", as it's currently done in /usr/lib/sysctl.d/50-default.conf is not always sufficient. I think the default only works for new interfaces that show up afterwards. Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Pairing udev's SYSTEMD_WANTS and systemd's templated units
On 14.08.2014 17:27, Lennart Poettering wrote:
> On Thu, 14.08.14 17:10, Harald Hoyer (harald.ho...@gmail.com) wrote:
>
>>
>> On 14.08.2014 13:00, Lennart Poettering wrote:
>>> On Thu, 14.08.14 10:02, Ivan Shapovalov (intelfx...@gmail.com) wrote:
>>>
The only thing: PROGRAM="...", ENV{SYSTEMD_WANTS}+="...%c..." idiom seems a
pretty ugly way to invoke systemd-escape. This looks like a pretty common
thing to do; shouldn't there be a shorthand or something? (just a
suggestion)
>>>
>>> Yeah, I agree, but I not entirely sure how this could look like in a
>>> nice way?
>>>
>>> Maybe add:
>>>
>>> ENV{SYSTEMD_WANTS_INSTANCE}="bar"
>>> ENV{SYSTEMD_WANTS_TEMPLATE}="foo@.service"
>>>
>>> or so, would escape "bar" and add it into foo@.service... But that's not
>>> particularly generic but focusses only on the instance/template case...
>>>
>>> Ideas?
>>>
>>> Lennart
>>>
>>
>> Why not extend udev with new % specifiers for the systemd escaped name?
>
> What syntax would you propose? Note that there are probably a couple of
> different strings people might want to have escaped?
>
> Lennart
>
We could probably make it $[], which would result in $[$devpath] $[%p]
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] eye of cylon / timeout timer is cut off
On 14.08.2014 19:20, Zbigniew Jędrzejewski-Szmek wrote: > On Thu, Aug 14, 2014 at 06:44:03PM +0200, Michael Biebl wrote: >> 2014-08-14 18:36 GMT+02:00 Zbigniew Jędrzejewski-Szmek : >>> It would be more useful to remove the "/ 1min 30s" part. Maybe the cylon >>> code could be smart enough for that. >> >> I think it's useful information to know that the timeout is 90s. >> An alternative could be, to simply use seconds and not convert that >> into h:m:s values. >> A "77s / 90s" display would be ok to me. > > What I meant is to skip the limit only where there is a shortage of space. > But thinking about it again, it is hard to say which is more important. So > maybe it is better to instead display the time remaining, like -33s, -32s, > -31s... > when columns() <= 80. > ^this :) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
wrote:
> On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:
>
>> 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
>> for looking up names) broke the build on clang.
>>
>> src/resolve/resolved-manager.c:553:43: error: non-const static data
>> member must be initialized out of line
>> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
>> in6_pktinfo)))
>>
>> Moving the MAX(...) to a separate line fixes that problem but another
>> error then happens:
>>
>> src/resolve/resolved-manager.c:554:25: error: fields must have a
>> constant size: 'variable length array in structure' extension will
>> never be supported
>> uint8_t buffer[CMSG_SPACE(size)
>>
>> We have encountered the same problem before and Lennart was able to
>> write the code in a different way. Would this be possible here too?
>
> My sugegstion here would be to maybe rewrite the MAX() macro to use
> __builtin_constant_p() so that it becomes constant if the params are
> constant, and only uses code block when it isn't. Or so...
>
> http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
Hm, I don't know whether that works. See the description here:
https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
What you propose is something like my attached patch, I guess? Along
the lines of:
(__builtin_constant_p(A) && __builtin_constant_p(B)) ?
((A) > (B) ? (A) : (B)) :
({ OLD_MAX })
Thing is, the ELSE case is not considered a compile-time constant by
LLVM. Therefore, the whole expression is not considered a compile-time
constant. I don't know whether conditions with __builtin_constant_p()
are evaluated at the parser-step. The GCC example replaces the ELSE
case with -1, effectively making both compile-time constants.
I also added a test-case to make sure __builtin_constant_p doesn't
evaluate the arguments.
Can someone with LLVM set up give this a spin?
Thanks
David
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 5619c32..18f5a79 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) {
_a > _b ? _a : _b; \
})
+#undef MAX
+#define MAX(_A, _B) \
+__extension__ ( \
+(__builtin_constant_p(_A) && __builtin_constant_p(_B)) \
+? (((_A) > (_B)) ? (_A) : (_B)) \
+: ({\
+typeof(_A) _a = (_A); \
+typeof(_B) _b = (_B); \
+_a > _b ? _a : _b; \
+}))
+
#define MAX3(x,y,z) \
__extension__ ({ \
typeof(x) _c = MAX(x,y); \
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 1850f97..d348ac5 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -70,6 +70,20 @@ static void test_align_power2(void) {
}
}
+static void test_max(void) {
+/* try triggering compile-errors for non-const initializers */
+static const struct {
+int a;
+} val1 = {
+.a = MAX(10, 100),
+};
+int d = 0;
+
+assert_se(val1.a == 100);
+assert_se(MAX(++d, 0) == 1);
+assert_se(d == 1);
+}
+
static void test_first_word(void) {
assert_se(first_word("Hello", ""));
assert_se(first_word("Hello", "Hello"));
@@ -926,6 +940,7 @@ int main(int argc, char *argv[]) {
test_streq_ptr();
test_align_power2();
+test_max();
test_first_word();
test_close_many();
test_parse_boolean();
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 5619c32..18f5a79 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) {
_a > _b ? _a : _b; \
})
+#undef MAX
+#define MAX(_A, _B) \
+__extension__ ( \
+(__builtin_constant_p(_A) && __builtin_constant_p(_B)) \
+? (((_A) > (_B)) ? (_A) : (_B)) \
+: ({\
+typeof(_A) _a = (_A); \
+typeof(_B) _b = (_B); \
+_a > _b ? _a : _b; \
+}))
+
#define MAX3(x,y,z)
Re: [systemd-devel] Implementing resume from hibernation as a systemd unit file
On Thursday 14 August 2014 at 12:56:10, Lennart Poettering wrote: > On Thu, 14.08.14 09:20, Ivan Shapovalov (intelfx...@gmail.com) wrote: > > > The udev rule should be possible (provided that udevd does not need rootfs > > remounted read-write -- I'd like to preserve some decency towards > > initrd-less > > systems), but udev is a framework for handling events, whereas we don't have > > any events here: such symlink can be derived from kernel command-line alone, > > statically. > > udev is totally fine with read-only everything. It just needs writable > /run. > > > Yes, a udev rule would allow to create a symlink which is tracked by > > systemd, > > so the dev-disk-resume.device appears and then it can be easily After='ed > > from the resume unit, but... really, is udev the proper tool for this? > > The symlink thing we already do in a very similar way for the gpt auto root > logic (see > 60-persistent-storage.rules) already, so there's prior art. Understood. So there's a helper (ata_id) ran by IMPORT which sets ID_PART_GPT_AUTO_ROOT, and then this variable is used to match against... Looks beautiful, but here's a slightly another case. In context of hibernation/resume, the path to device node is already explicitly specified as a kernel cmdline parameter, and it could be anything from /dev/sdXY to /dev/disk/by-label/foo, so we can't use a helper to translate that path into a per-device flag (pathes for devices are generated after running helpers). So, IIUC, using udev is not an option here. Did I miss anything? If I didn't, are you OK with the "generator + templated unit" approach? > > > Actually, the main question is how to order the resume unit. It needs to run > > before any real filesystems are mounted (speaking in terms of initrd) AND > > before > > rootfs is remounted read-write (speaking in terms of initrd-less system), > > but > > after whatever is needed to make the device node appear. > > You could turn this into a generator, that pulls the switch from the > kernel cmdline, and generates a service that orders itself before > local-fs-pre.taret and after the device you need. The device you need > you give a stable name via an udev rule. So is "Before=local-fs-pre.target" a sufficient ordering for such resume unit? Again, the resume unit must be started before any filesystems are (re)mounted read-write, either from initrd or not. Seems like there's at least systemd-remount-fs.service that also needs to be taken into account... Thanks, -- Ivan Shapovalov / intelfx / > > That's pretty much exactly how the got auto root thing works... > > Lennart > > signature.asc Description: This is a digitally signed message part. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] eye of cylon / timeout timer is cut off
On Fri, 15.08.14 10:22, Harald Hoyer (harald.ho...@gmail.com) wrote: > On 14.08.2014 19:20, Zbigniew Jędrzejewski-Szmek wrote: > > On Thu, Aug 14, 2014 at 06:44:03PM +0200, Michael Biebl wrote: > >> 2014-08-14 18:36 GMT+02:00 Zbigniew Jędrzejewski-Szmek : > >>> It would be more useful to remove the "/ 1min 30s" part. Maybe the cylon > >>> code could be smart enough for that. > >> > >> I think it's useful information to know that the timeout is 90s. > >> An alternative could be, to simply use seconds and not convert that > >> into h:m:s values. > >> A "77s / 90s" display would be ok to me. > > > > What I meant is to skip the limit only where there is a shortage of space. > > But thinking about it again, it is hard to say which is more important. So > > maybe it is better to instead display the time remaining, like -33s, -32s, > > -31s... > > when columns() <= 80. > > ^this :) Instead of making this context sensitive, for simplicity reasons: why not just always show the negative time? 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] Implementing resume from hibernation as a systemd unit file
On Fri, 15.08.14 13:02, Ivan Shapovalov (intelfx...@gmail.com) wrote: Ah, well, if the resume= switch is supposed to simply carry the finished device node path, and nothing we still have to translate into one, then of course the entire udev rules step is unnecessary, and you can just write this out directly from the generator. > > > Actually, the main question is how to order the resume unit. It needs to > > > run > > > before any real filesystems are mounted (speaking in terms of initrd) AND > > > before > > > rootfs is remounted read-write (speaking in terms of initrd-less system), > > > but > > > after whatever is needed to make the device node appear. > > > > You could turn this into a generator, that pulls the switch from the > > kernel cmdline, and generates a service that orders itself before > > local-fs-pre.taret and after the device you need. The device you need > > you give a stable name via an udev rule. > > So is "Before=local-fs-pre.target" a sufficient ordering for such resume unit? > > Again, the resume unit must be started before any filesystems are (re)mounted > read-write, either from initrd or not. Yes. That should be enough. > Seems like there's at least systemd-remount-fs.service that also needs to be > taken into account... That only runs after the transition into the host OS. 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] Pairing udev's SYSTEMD_WANTS and systemd's templated units
On Fri, 15.08.14 09:58, Harald Hoyer (harald.ho...@gmail.com) wrote:
>
> On 14.08.2014 17:27, Lennart Poettering wrote:
> > On Thu, 14.08.14 17:10, Harald Hoyer (harald.ho...@gmail.com) wrote:
> >
> >>
> >> On 14.08.2014 13:00, Lennart Poettering wrote:
> >>> On Thu, 14.08.14 10:02, Ivan Shapovalov (intelfx...@gmail.com) wrote:
> >>>
> The only thing: PROGRAM="...", ENV{SYSTEMD_WANTS}+="...%c..." idiom
> seems a
> pretty ugly way to invoke systemd-escape. This looks like a pretty common
> thing to do; shouldn't there be a shorthand or something? (just a
> suggestion)
> >>>
> >>> Yeah, I agree, but I not entirely sure how this could look like in a
> >>> nice way?
> >>>
> >>> Maybe add:
> >>>
> >>> ENV{SYSTEMD_WANTS_INSTANCE}="bar"
> >>> ENV{SYSTEMD_WANTS_TEMPLATE}="foo@.service"
> >>>
> >>> or so, would escape "bar" and add it into foo@.service... But that's not
> >>> particularly generic but focusses only on the instance/template case...
> >>>
> >>> Ideas?
> >>>
> >>> Lennart
> >>>
> >>
> >> Why not extend udev with new % specifiers for the systemd escaped name?
> >
> > What syntax would you propose? Note that there are probably a couple of
> > different strings people might want to have escaped?
> >
> > Lennart
> >
>
> We could probably make it $[], which would result in $[$devpath] $[%p]
Well, there are at least two modes how to escape strings for unit names
(one for general string, one for paths). This would become awfully
complex...
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] compile with clang broken
On Fri, Aug 15, 2014 at 10:55 AM, David Herrmann wrote:
> Hi
>
> On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
> wrote:
>> On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:
>>
>>> 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
>>> for looking up names) broke the build on clang.
>>>
>>> src/resolve/resolved-manager.c:553:43: error: non-const static data
>>> member must be initialized out of line
>>> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
>>> in6_pktinfo)))
>>>
>>> Moving the MAX(...) to a separate line fixes that problem but another
>>> error then happens:
>>>
>>> src/resolve/resolved-manager.c:554:25: error: fields must have a
>>> constant size: 'variable length array in structure' extension will
>>> never be supported
>>> uint8_t buffer[CMSG_SPACE(size)
>>>
>>> We have encountered the same problem before and Lennart was able to
>>> write the code in a different way. Would this be possible here too?
>>
>> My sugegstion here would be to maybe rewrite the MAX() macro to use
>> __builtin_constant_p() so that it becomes constant if the params are
>> constant, and only uses code block when it isn't. Or so...
>>
>> http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
>
> Hm, I don't know whether that works. See the description here:
> https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
>
> What you propose is something like my attached patch, I guess? Along
> the lines of:
> (__builtin_constant_p(A) && __builtin_constant_p(B)) ?
> ((A) > (B) ? (A) : (B)) :
> ({ OLD_MAX })
>
> Thing is, the ELSE case is not considered a compile-time constant by
> LLVM. Therefore, the whole expression is not considered a compile-time
> constant. I don't know whether conditions with __builtin_constant_p()
> are evaluated at the parser-step. The GCC example replaces the ELSE
> case with -1, effectively making both compile-time constants.
>
> I also added a test-case to make sure __builtin_constant_p doesn't
> evaluate the arguments.
>
> Can someone with LLVM set up give this a spin?
I still get:
src/resolve/resolved-manager.c:844:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
^
I tried moving that to a separate line but then I also still get:
src/resolve/resolved-manager.c:845:25: error: fields must have a
constant size: 'variable length array in structure' extension will
never be supported
uint8_t buffer[CMSG_SPACE(size)
^
I got the following to compile but I have not have time to test it at
all. Too ugly to live I guess...
diff --git a/src/resolve/resolved-dns-stream.c
b/src/resolve/resolved-dns-stream.c
index eb78587..1b415ae 100644
--- a/src/resolve/resolved-dns-stream.c
+++ b/src/resolve/resolved-dns-stream.c
@@ -62,10 +62,14 @@ static int dns_stream_complete(DnsStream *s, int error) {
}
static int dns_stream_identify(DnsStream *s) {
+const size_t size = __builtin_constant_p(
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) : 0;
union {
struct cmsghdr header; /* For alignment */
-uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
+uint8_t buffer[CMSG_SPACE(size)
+ EXTRA_CMSG_SPACE /* kernel appears
to require extra space */];
+
} control;
struct msghdr mh = {};
struct cmsghdr *cmsg;
@@ -73,6 +77,7 @@ static int dns_stream_identify(DnsStream *s) {
int r;
assert(s);
+assert(size > 0);
if (s->identified)
return 0;
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index bfbdc7d..1342fb1 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -839,9 +839,12 @@ fail:
int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
+const size_t size = __builtin_constant_p(
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) : 0;
union {
struct cmsghdr header; /* For alignment */
-uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
+uint8_t buffer[CMSG_SPACE(size)
+ CMSG_SPACE(int) /* ttl/hoplimit */
+ EXTRA_CMSG_SPACE /* kernel appears
to require extra buffer space */];
} control;
@@ -855,6 +858,7 @@ int manager_recv(Manager *m, int fd, DnsProtocol
protocol, DnsP
Re: [systemd-devel] compile with clang broken
On Fri, 15.08.14 10:55, David Herrmann (dh.herrm...@gmail.com) wrote:
> Hm, I don't know whether that works. See the description here:
> https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
>
> What you propose is something like my attached patch, I guess? Along
> the lines of:
> (__builtin_constant_p(A) && __builtin_constant_p(B)) ?
> ((A) > (B) ? (A) : (B)) :
> ({ OLD_MAX })
Yes, correct.
> Thing is, the ELSE case is not considered a compile-time constant by
> LLVM.
In that case __builtin_constant_p() would be entirely useless on LLVM,
right? And all uses by this construct in glibc would not work, right?
Thanks for looking into this!
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] Logind error - Failed to abandon session scope: Connection reset
On Fri, 15.08.14 15:04, Roger Qiu (roger@polycademy.com) wrote: > > Hello Lennart, > > Thanks for answering. > > Is there any way to enforce ordering to make this error not occur? You could order systemd-logind.service After= dbus.service. That way dbus is started first, and logind second. And since the top order is the reversed start order, this would result in logind being stopped first and dbus second. But honestly, I don't really like this, we should no add unnecessary ordering here... > Or what's the best way to hide that warning? We should probably just patch it out... > Or when is the ETA for kdbus? Well, "when it's ready"... 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: don't complain if cgroup property is missing
On Fri, 15.08.14 09:12, Umut Tezduyar Lindskog (u...@tezduyar.com) wrote: > > On Fri, Aug 15, 2014 at 2:45 AM, Lennart Poettering > wrote: > > On Fri, 15.08.14 02:42, Lennart Poettering (lenn...@poettering.net) wrote: > > > >> > >> On Tue, 15.07.14 11:53, Umut Tezduyar Lindskog (umut.tezdu...@axis.com) > >> wrote: > >> > >> Looks Ok, but doesn't apply to currently git anymore... > > > > (In case this wasn't clear, please rebase and I'll commit this. -- Or > > actually, could you add a short comment explaining that EACCES is what > > the kernel returns if CFS quotas are disabled?) > > Hi, > > I think there is a misunderstanding. Cgroup properties do not exist if > they are turned off. And since your concern is EACCESS, I think a > "file exists?" check inside cg_set_attribute before we call > "write_string_file" is more proper in this case. Hmm, OK, so I wonder if this is simply the case because we ask the file to be written with O_CREAT. Because you cannot create files on cgroups this will result in EACCES. Now, of course, we actually never really want to create the files, so if we drop the O_CREAT we should probably get ENOENT instead, which would be so much nicer to check for. The O_CREAT is done because opf fopen(.., "we") inside of write_string_file(), inside of cg_set_attribute(). Now, there's apparently no way to make fopen() open a file for writing without O_CREAT. We hence have to build our own version, by combining open() and fdopen()... Anyway, could you have a look at this? We probably want an entirely new function write_string_file_nocreate() or so, which does this, and then make the cgroup code use that... 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] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 11:35 AM, Thomas H.P. Andersen wrote:
> On Fri, Aug 15, 2014 at 10:55 AM, David Herrmann
> wrote:
>> Hi
>>
>> On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
>> wrote:
>>> On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:
>>>
1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
for looking up names) broke the build on clang.
src/resolve/resolved-manager.c:553:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
in6_pktinfo)))
Moving the MAX(...) to a separate line fixes that problem but another
error then happens:
src/resolve/resolved-manager.c:554:25: error: fields must have a
constant size: 'variable length array in structure' extension will
never be supported
uint8_t buffer[CMSG_SPACE(size)
We have encountered the same problem before and Lennart was able to
write the code in a different way. Would this be possible here too?
>>>
>>> My sugegstion here would be to maybe rewrite the MAX() macro to use
>>> __builtin_constant_p() so that it becomes constant if the params are
>>> constant, and only uses code block when it isn't. Or so...
>>>
>>> http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
>>
>> Hm, I don't know whether that works. See the description here:
>> https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
>>
>> What you propose is something like my attached patch, I guess? Along
>> the lines of:
>> (__builtin_constant_p(A) && __builtin_constant_p(B)) ?
>> ((A) > (B) ? (A) : (B)) :
>> ({ OLD_MAX })
>>
>> Thing is, the ELSE case is not considered a compile-time constant by
>> LLVM. Therefore, the whole expression is not considered a compile-time
>> constant. I don't know whether conditions with __builtin_constant_p()
>> are evaluated at the parser-step. The GCC example replaces the ELSE
>> case with -1, effectively making both compile-time constants.
>>
>> I also added a test-case to make sure __builtin_constant_p doesn't
>> evaluate the arguments.
>>
>> Can someone with LLVM set up give this a spin?
>
> I still get:
> src/resolve/resolved-manager.c:844:43: error: non-const static data
> member must be initialized out of line
> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
> in_pktinfo), sizeof(struct in6_pktinfo)))
> ^
Thanks for trying!
Result is as I expected. Evaluation takes place _after_ validating
compile-time constants, and thus __builtin_constant_p in combination
with ?: will not work if not both cases are constant. Maybe it works
with __builtin_choose_expr()?
Can you try the attached patch?
If that still doesn't work, I guess we're left with your proposed
solution below, or we add MAX_CONST() which just does (A > B)?A:B.
> I got the following to compile but I have not have time to test it at
> all. Too ugly to live I guess...
>
> diff --git a/src/resolve/resolved-dns-stream.c
> b/src/resolve/resolved-dns-stream.c
> index eb78587..1b415ae 100644
> --- a/src/resolve/resolved-dns-stream.c
> +++ b/src/resolve/resolved-dns-stream.c
> @@ -62,10 +62,14 @@ static int dns_stream_complete(DnsStream *s, int error) {
> }
>
> static int dns_stream_identify(DnsStream *s) {
> +const size_t size = __builtin_constant_p(
> +MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
> +MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) :
> 0;
No reason to make "size" constant. You can use:
size_t size = MAX();
uint8_t buffer[...];
This will be similar to alloca(), I think... or maybe I'm wrong..
Thanks
David
> union {
> struct cmsghdr header; /* For alignment */
> -uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
> in_pktinfo), sizeof(struct in6_pktinfo)))
> +uint8_t buffer[CMSG_SPACE(size)
> + EXTRA_CMSG_SPACE /* kernel appears
> to require extra space */];
> +
> } control;
> struct msghdr mh = {};
> struct cmsghdr *cmsg;
> @@ -73,6 +77,7 @@ static int dns_stream_identify(DnsStream *s) {
> int r;
>
> assert(s);
> +assert(size > 0);
>
> if (s->identified)
> return 0;
> diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
> index bfbdc7d..1342fb1 100644
> --- a/src/resolve/resolved-manager.c
> +++ b/src/resolve/resolved-manager.c
> @@ -839,9 +839,12 @@ fail:
>
> int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
> _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
> +const size_t size = __builtin_constant_p(
> +MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
> +MAX(sizeof(struct in_pktinfo), sizeo
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 11:38 AM, Lennart Poettering
wrote:
> On Fri, 15.08.14 10:55, David Herrmann (dh.herrm...@gmail.com) wrote:
>
>> Hm, I don't know whether that works. See the description here:
>> https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
>>
>> What you propose is something like my attached patch, I guess? Along
>> the lines of:
>> (__builtin_constant_p(A) && __builtin_constant_p(B)) ?
>> ((A) > (B) ? (A) : (B)) :
>> ({ OLD_MAX })
>
> Yes, correct.
>
>> Thing is, the ELSE case is not considered a compile-time constant by
>> LLVM.
>
> In that case __builtin_constant_p() would be entirely useless on LLVM,
> right? And all uses by this construct in glibc would not work, right?
No, it's just useless for our case.
glibc uses __builtin_constant_p() heavily to validate parameters. For
instance, it's very handy to verify length restrictions and so on. And
I think it was introduced mainly to allow optimizations, not to allow
conditional compilations. But maybe __builtin_choose_expr() works like
that.
Example: strlen() can use __builtin_constant_p() to use sizeof() - 1
on constant expressions (ok, it cannot, because there might be a NUL
in the middle, but I guess you get the idea?).
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Implementing resume from hibernation as a systemd unit file
On Friday 15 August 2014 at 11:32:59, Lennart Poettering wrote: > On Fri, 15.08.14 13:02, Ivan Shapovalov (intelfx...@gmail.com) wrote: > > Ah, well, if the resume= switch is supposed to simply carry the finished > device node path, and nothing we still have to translate into one, then > of course the entire udev rules step is unnecessary, and you can just > write this out directly from the generator. Yes, that's how it happens now. > > > > > Actually, the main question is how to order the resume unit. It needs > > > > to run > > > > before any real filesystems are mounted (speaking in terms of initrd) > > > > AND before > > > > rootfs is remounted read-write (speaking in terms of initrd-less > > > > system), but > > > > after whatever is needed to make the device node appear. > > > > > > You could turn this into a generator, that pulls the switch from the > > > kernel cmdline, and generates a service that orders itself before > > > local-fs-pre.taret and after the device you need. The device you need > > > you give a stable name via an udev rule. > > > > So is "Before=local-fs-pre.target" a sufficient ordering for such resume > > unit? > > > > Again, the resume unit must be started before any filesystems are > > (re)mounted > > read-write, either from initrd or not. > > Yes. That should be enough. > > > Seems like there's at least systemd-remount-fs.service that also needs to be > > taken into account... > > That only runs after the transition into the host OS. I'd like to make this work both with initramfs and without one (provided that the rootfs has been mounted read-only by using 'ro' kernel cmdline parameter). In this case, what are the needed orderings? Thanks, -- Ivan Shapovalov / intelfx / > > Lennart > > signature.asc Description: This is a digitally signed message part. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Pairing udev's SYSTEMD_WANTS and systemd's templated units
On Thursday 14 August 2014 at 13:00:50, Lennart Poettering wrote:
> On Thu, 14.08.14 10:02, Ivan Shapovalov (intelfx...@gmail.com) wrote:
>
> > The only thing: PROGRAM="...", ENV{SYSTEMD_WANTS}+="...%c..." idiom seems a
> > pretty ugly way to invoke systemd-escape. This looks like a pretty common
> > thing to do; shouldn't there be a shorthand or something? (just a
> > suggestion)
>
> Yeah, I agree, but I not entirely sure how this could look like in a
> nice way?
>
> Maybe add:
>
> ENV{SYSTEMD_WANTS_INSTANCE}="bar"
> ENV{SYSTEMD_WANTS_TEMPLATE}="foo@.service"
>
> or so, would escape "bar" and add it into foo@.service... But that's not
> particularly generic but focusses only on the instance/template case...
>
> Ideas?
>
> Lennart
How about $(/usr/bin/systemd-escape ...) shell-like syntax ? :)
--
Ivan Shapovalov / intelfx /
signature.asc
Description: This is a digitally signed message part.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Implementing resume from hibernation as a systemd unit file
On Fri, 15.08.14 13:54, Ivan Shapovalov (intelfx...@gmail.com) wrote: > On Friday 15 August 2014 at 11:32:59, Lennart Poettering wrote: > > On Fri, 15.08.14 13:02, Ivan Shapovalov (intelfx...@gmail.com) wrote: > > > > Ah, well, if the resume= switch is supposed to simply carry the finished > > device node path, and nothing we still have to translate into one, then > > of course the entire udev rules step is unnecessary, and you can just > > write this out directly from the generator. > > Yes, that's how it happens now. > > > > > > > > Actually, the main question is how to order the resume unit. It needs > > > > > to run > > > > > before any real filesystems are mounted (speaking in terms of initrd) > > > > > AND before > > > > > rootfs is remounted read-write (speaking in terms of initrd-less > > > > > system), but > > > > > after whatever is needed to make the device node appear. > > > > > > > > You could turn this into a generator, that pulls the switch from the > > > > kernel cmdline, and generates a service that orders itself before > > > > local-fs-pre.taret and after the device you need. The device you need > > > > you give a stable name via an udev rule. > > > > > > So is "Before=local-fs-pre.target" a sufficient ordering for such resume > > > unit? > > > > > > Again, the resume unit must be started before any filesystems are > > > (re)mounted > > > read-write, either from initrd or not. > > > > Yes. That should be enough. > > > > > Seems like there's at least systemd-remount-fs.service that also needs to > > > be > > > taken into account... > > > > That only runs after the transition into the host OS. > > I'd like to make this work both with initramfs and without one (provided that > the rootfs has been mounted read-only by using 'ro' kernel cmdline parameter). > > In this case, what are the needed orderings? Actually systemd-remount-fs.service uses After=local-fs-pre.target anyway, so ordering before l-f-p.t should be enough. 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: don't complain if cgroup property is missing
On Fri, 15.08.14 11:46, Lennart Poettering (lenn...@poettering.net) wrote: > > I think there is a misunderstanding. Cgroup properties do not exist if > > they are turned off. And since your concern is EACCESS, I think a > > "file exists?" check inside cg_set_attribute before we call > > "write_string_file" is more proper in this case. > > Hmm, OK, so I wonder if this is simply the case because we ask the file > to be written with O_CREAT. Because you cannot create files on cgroups > this will result in EACCES. Now, of course, we actually never really > want to create the files, so if we drop the O_CREAT we should probably > get ENOENT instead, which would be so much nicer to check for. > > The O_CREAT is done because opf fopen(.., "we") inside of > write_string_file(), inside of cg_set_attribute(). Now, there's > apparently no way to make fopen() open a file for writing without > O_CREAT. We hence have to build our own version, by combining open() and > fdopen()... > > Anyway, could you have a look at this? We probably want an entirely new > function write_string_file_nocreate() or so, which does this, and then > make the cgroup code use that... Don#t bother, I just decided to quickly hack that up myself. Does this make things work for you? Can you check? I left the log messages in btw, I just downgrade them to LOG_DEBUG now, if the files are missing. 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] Pairing udev's SYSTEMD_WANTS and systemd's templated units
On Friday 15 August 2014 at 11:34:28, Lennart Poettering wrote:
> On Fri, 15.08.14 09:58, Harald Hoyer (harald.ho...@gmail.com) wrote:
>
> >
> > On 14.08.2014 17:27, Lennart Poettering wrote:
> > > On Thu, 14.08.14 17:10, Harald Hoyer (harald.ho...@gmail.com) wrote:
> > >
> > >>
> > >> On 14.08.2014 13:00, Lennart Poettering wrote:
> > >>> On Thu, 14.08.14 10:02, Ivan Shapovalov (intelfx...@gmail.com) wrote:
> > >>>
> > The only thing: PROGRAM="...", ENV{SYSTEMD_WANTS}+="...%c..." idiom
> > seems a
> > pretty ugly way to invoke systemd-escape. This looks like a pretty
> > common
> > thing to do; shouldn't there be a shorthand or something? (just a
> > suggestion)
> > >>>
> > >>> Yeah, I agree, but I not entirely sure how this could look like in a
> > >>> nice way?
> > >>>
> > >>> Maybe add:
> > >>>
> > >>> ENV{SYSTEMD_WANTS_INSTANCE}="bar"
> > >>> ENV{SYSTEMD_WANTS_TEMPLATE}="foo@.service"
> > >>>
> > >>> or so, would escape "bar" and add it into foo@.service... But that's not
> > >>> particularly generic but focusses only on the instance/template case...
> > >>>
> > >>> Ideas?
> > >>>
> > >>> Lennart
> > >>>
> > >>
> > >> Why not extend udev with new % specifiers for the systemd escaped name?
> > >
> > > What syntax would you propose? Note that there are probably a couple of
> > > different strings people might want to have escaped?
> > >
> > > Lennart
> > >
> >
> > We could probably make it $[], which would result in $[$devpath] $[%p]
>
> Well, there are at least two modes how to escape strings for unit names
> (one for general string, one for paths). This would become awfully
> complex...
>
> Lennart
>
>
$[...] and ${...} maybe.
--
Ivan Shapovalov / intelfx /
signature.asc
Description: This is a digitally signed message part.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Implementing resume from hibernation as a systemd unit file
On Friday 15 August 2014 at 12:00:41, Lennart Poettering wrote: > [...] > > > > > > > > So is "Before=local-fs-pre.target" a sufficient ordering for such > > > > resume unit? > > > > > > > > Again, the resume unit must be started before any filesystems are > > > > (re)mounted > > > > read-write, either from initrd or not. > > > > > > Yes. That should be enough. > > > > > > > Seems like there's at least systemd-remount-fs.service that also needs > > > > to be > > > > taken into account... > > > > > > That only runs after the transition into the host OS. > > > > I'd like to make this work both with initramfs and without one (provided > > that > > the rootfs has been mounted read-only by using 'ro' kernel cmdline > > parameter). > > > > In this case, what are the needed orderings? > > Actually systemd-remount-fs.service uses After=local-fs-pre.target > anyway, so ordering before l-f-p.t should be nough. Hm. In git (v215-651-g41488fe), it is Before=local-fs-pre.target local-fs.target shutdown.target Wants=local-fs-pre.target -- Ivan Shapovalov / intelfx / signature.asc Description: This is a digitally signed message part. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Changing configurations with networkd
On Fri, 15.08.14 09:12, Michael Olbrich (m.olbr...@pengutronix.de) wrote: > > On Thu, Aug 14, 2014 at 05:38:05PM +0200, Lennart Poettering wrote: > > On Fri, 25.07.14 09:48, Michael Olbrich (m.olbr...@pengutronix.de) wrote: > > > > > What I'm _not_ seeing, and what usually comes when anything else changes > > > in > > > the network configuration is: > > > systemd-timesyncd[348]: Network configuration changed, trying to > > > establish connection. > > > > > > I would expect, that systemd-timesyncd should be notified in this case as > > > well, right? > > > > This should be fixed with current git. Could you please recheck? > > Indeed: > > Aug 01 00:20:15 BaseKit systemd-networkd[434]: eth0: removed > address: 192.168.51.144/24 (valid for 0) > Aug 01 00:20:15 BaseKit systemd-timesyncd[346]: Network configuration > changed, trying to establish connection. > Aug 01 00:20:15 BaseKit systemd-networkd[434]: eth0: added > address: 192.168.51.145/24 (valid for 9min 30s) > Aug 01 00:20:15 BaseKit systemd-timesyncd[346]: Network configuration > changed, trying to establish connection. > > I'm not sure, why the new address is found again though. > > Note: this is with "net.ipv4.conf.all.promote_secondaries = 1". Setting > just "net.ipv4.conf.default.promote_secondaries = 1", as it's currently > done in /usr/lib/sysctl.d/50-default.conf is not always sufficient. I think > the default only works for new interfaces that show up afterwards. This is in indeed a race. I changed the sysctl fragment now to write to both *.all.* and *.default.* for all IP related sysctls. 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] compile with clang broken
On Fri, Aug 15, 2014 at 10:55:57AM +0200, David Herrmann wrote:
> Hi
>
> On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
> wrote:
> > On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:
> >
> >> 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
> >> for looking up names) broke the build on clang.
> >>
> >> src/resolve/resolved-manager.c:553:43: error: non-const static data
> >> member must be initialized out of line
> >> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
> >> in6_pktinfo)))
> >>
> >> Moving the MAX(...) to a separate line fixes that problem but another
> >> error then happens:
> >>
> >> src/resolve/resolved-manager.c:554:25: error: fields must have a
> >> constant size: 'variable length array in structure' extension will
> >> never be supported
> >> uint8_t buffer[CMSG_SPACE(size)
> >>
> >> We have encountered the same problem before and Lennart was able to
> >> write the code in a different way. Would this be possible here too?
> >
> > My sugegstion here would be to maybe rewrite the MAX() macro to use
> > __builtin_constant_p() so that it becomes constant if the params are
> > constant, and only uses code block when it isn't. Or so...
> >
> > http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
>
> Hm, I don't know whether that works. See the description here:
> https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
>
> What you propose is something like my attached patch, I guess? Along
> the lines of:
> (__builtin_constant_p(A) && __builtin_constant_p(B)) ?
> ((A) > (B) ? (A) : (B)) :
> ({ OLD_MAX })
>
> Thing is, the ELSE case is not considered a compile-time constant by
> LLVM. Therefore, the whole expression is not considered a compile-time
> constant. I don't know whether conditions with __builtin_constant_p()
> are evaluated at the parser-step. The GCC example replaces the ELSE
> case with -1, effectively making both compile-time constants.
Sorry I didn't follow the thread, but:
Actually and IIRC it is more complicated, __builtin_constant_p() can
be computed during the SSA (optimization) passes on the GIMPLE form of
the code... so it depends on the code and parameters passed to
__builitin_constant_p(), not only preprocessor constants.
https://gcc.gnu.org/onlinedocs/gccint/Tree-SSA.html
--
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] Implementing resume from hibernation as a systemd unit file
On Fri, 15.08.14 14:07, Ivan Shapovalov (intelfx...@gmail.com) wrote: > On Friday 15 August 2014 at 12:00:41, Lennart Poettering wrote: > > [...] > > > > > > > > > > So is "Before=local-fs-pre.target" a sufficient ordering for such > > > > > resume unit? > > > > > > > > > > Again, the resume unit must be started before any filesystems are > > > > > (re)mounted > > > > > read-write, either from initrd or not. > > > > > > > > Yes. That should be enough. > > > > > > > > > Seems like there's at least systemd-remount-fs.service that also > > > > > needs to be > > > > > taken into account... > > > > > > > > That only runs after the transition into the host OS. > > > > > > I'd like to make this work both with initramfs and without one (provided > > > that > > > the rootfs has been mounted read-only by using 'ro' kernel cmdline > > > parameter). > > > > > > In this case, what are the needed orderings? > > > > Actually systemd-remount-fs.service uses After=local-fs-pre.target > > anyway, so ordering before l-f-p.t should be nough. > > Hm. In git (v215-651-g41488fe), it is > > Before=local-fs-pre.target local-fs.target shutdown.target > Wants=local-fs-pre.target Ah, right. This is actually correct here. We want to make sure that the root fs is remounted before we mount the other units, since this might required creating additional directories to mount things on... There are two more complications: a) if you want to make use of local-fs-pre.target you actually have to pull it in, it's a "passive" unit. See systemd.special(5). b) You want to run your stuff before fsck is run on the devices, so that you don't end up interrupting an fsck that is half in progress. To put this together, in your unit file you need: Before=local-fs-pre.target systemd-remount-fs.service systemd-fsck-root.service Wants=local-fs-pre.target That should be enough. (You don't need to individually order the systemd-fsck@.service instances for the other devices after your service, since they are already ordered after systemd-fsck-root.service, and you order yourself before that, so all is good). 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] compile with clang broken
On Fri, 15.08.14 11:49, David Herrmann (dh.herrm...@gmail.com) wrote: > If that still doesn't work, I guess we're left with your proposed > solution below, or we add MAX_CONST() which just does (A > B)?A:B. We could also just define MAX() differently if we detect we run on LLVM. There must be some macro how one can detect that... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] FW: systemd stopping daemons
Hi, Quoting a part of a discussion in debian-user that may be of interest to you . Bonno Bloksma -Oorspronkelijk bericht- Van: Bonno Bloksma Verzonden: vrijdag 15 augustus 2014 11:20 Aan: debian-u...@lists.debian.org Onderwerp: RE: systemd fails to poweroff - "A stop job is running for Session 2 of user $USER" Hi, > "A stop job is running for Session 2 of user $USER" [...] > > I interpret the quoted string in the Subject: header as being flawed > > use of English language. 'stop' should be 'stopped'. And, there is a > > That would definitely be clearer. > > I was interpreting it as some special systemd shutdown-ey thing which > runs around trying to stop things, and that there might be various of > these, and one of them has a problem. > Yes, I believe this is the correct interpretation. SystemD will tell > all services to "stop". It will then wait until all those sevices > have stopped. Some services will stop immediately, but some need a > little longer to flush logs, finish servicing a request or whatever. > After a period of seconds, it appears that SystemD will pop up a > message to the effect of "I'm still here, still responding. I'm > just waiting for service X to tell me it has stopped." > Given what was said earlier in the thread, I suspect this will continue > for 90 seconds until it finally gives up waiting. I wonder if the people developing this are paying attention to a development in de Windows environment where the latest thing is that de service can report back that it is indeed still trying to stop and not just hung and not reporting back. Windows will now kill a service after a certain time when shutting down, in some cases it was killing a database that took A LONG TIME to shut down and cause the database to become inconsistent. If systemd is trying to become smart about stopping services it might be a good idea to have this built in. Also not just have the service report back "I am still busy" but also with a progress indicator which NEEDS to increase at each report so system can detect whether the service is indeed progressing towards a stopped state or hung in the getting there. Bonno Bloksma ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi On Fri, Aug 15, 2014 at 12:29 PM, Thomas H.P. Andersen wrote: > On Fri, Aug 15, 2014 at 11:49 AM, David Herrmann > wrote: >> Thanks for trying! >> >> Result is as I expected. Evaluation takes place _after_ validating >> compile-time constants, and thus __builtin_constant_p in combination >> with ?: will not work if not both cases are constant. Maybe it works >> with __builtin_choose_expr()? >> >> Can you try the attached patch? > > This patch works. It also needs the change to do the calculation to a > seperate line. Also only if size is const, like so: > const size_t size = MAX(sizeof(struct in_pktinfo), sizeof(struct > in6_pktinfo)); Again, thanks for trying it out! I don't understand your comment, though. You're saying this works: const size_t size = MAX(...); uint8_t buffer[CMSG_SPACE(size) +...]; ...but this doesn't work: uint8_t buffer[CMSG_SPACE(MAX(...)) +...]; ...and this doesn't work either (mind the dropped 'const'): size_t size = MAX(...); uint8_t buffer[CMSG_SPACE(size) +...]; Hm. This is weird. Maybe CMSG_SPACE does something weird. I'll see. David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
On Fri, Aug 15, 2014 at 12:35 PM, David Herrmann wrote: > Hi > > On Fri, Aug 15, 2014 at 12:29 PM, Thomas H.P. Andersen > wrote: >> On Fri, Aug 15, 2014 at 11:49 AM, David Herrmann >> wrote: >>> Thanks for trying! >>> >>> Result is as I expected. Evaluation takes place _after_ validating >>> compile-time constants, and thus __builtin_constant_p in combination >>> with ?: will not work if not both cases are constant. Maybe it works >>> with __builtin_choose_expr()? >>> >>> Can you try the attached patch? >> >> This patch works. It also needs the change to do the calculation to a >> seperate line. Also only if size is const, like so: >> const size_t size = MAX(sizeof(struct in_pktinfo), sizeof(struct >> in6_pktinfo)); > > Again, thanks for trying it out! no problem. I have inserted the relevant error messages for the two non-working cases. > I don't understand your comment, though. You're saying this works: > > const size_t size = MAX(...); > uint8_t buffer[CMSG_SPACE(size) +...]; > > ...but this doesn't work: > > uint8_t buffer[CMSG_SPACE(MAX(...)) +...]; src/resolve/resolved-dns-stream.c:67:43: error: non-const static data member must be initialized out of line uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ^ > ...and this doesn't work either (mind the dropped 'const'): > > size_t size = MAX(...); > uint8_t buffer[CMSG_SPACE(size) +...]; src/resolve/resolved-dns-stream.c:68:25: error: fields must have a constant size: 'variable length array in structure' extension will never be supported uint8_t buffer[CMSG_SPACE(size) ^ > Hm. This is weird. Maybe CMSG_SPACE does something weird. I'll see. > David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: collapse JOB_RELOAD on an inactive unit into JOB_NOP
On Fri, 15.08.14 05:09, Uoti Urpala (uoti.urp...@pp1.inet.fi) wrote: > > > Before this commit systemctl reload on an inactive unit with a queued > > > start job would block until the unit had started if the unit supported > > > reload, but return failure immediately if the unit didn't. > > > > This sounds like correct behaviour to me. > > > > 1) I think the rule should be to return only after the unit is in the > > desired state, and the desired operation or something equivalent has > > been executed. This is useful, so that callers can rely on the fact that > > What is your "desired state" for reload then? *operating* with the new configuration loaded. I call this being "positive", i.e. try to make the best of things, and if we know things will improve, then wait for that, and return then. Or in other words: if there's the chance we can do something that is what the user asked for, or equivalent to it, then do that, but not just give up early, because right now we coudln't fulfill what is requested, even though we *know* that in a very short time we could. > To me the obvious natural meaning is that after reload has completed, > it is guaranteed the service will not process any requests with > outdated configuration from before reload was issued. Do you want to > have reload imply that AND "service is currently running"? If so, I > think such a combination is better done with "systemctl reload && > systemctl start"; getting the correct semantics for the reload part > alone is harder if systemctl only implements the combined operation. Hmm? That's what "systemctl reload-or-restart" does. But it's really not about this. it's about trying to give the caller guarantees that something is in a desired state after the operation returned, if there's any chance for it. > > > Additionally systemctl reload-or-try-restart (and systemctl force-reload) > > > would block until the unit had started if the unit supported reload, but > > > return *success* immediately if the unit didn't. > > > > This actually also sounds like correct behaviour to me. > > > > "try-restart" means that we try to restart a service, but only if it is > > already running. If we are not running this is not considered an > > error. If you then add the "reload-or-" to the mix, this means that we > > will try to reload it if the unit supports it, instead of restarting it. > > > > So, if a start is already queued, we'll wait for it to finish, so that > > the configuration is fully in effect afterwards. But if the thing is not > > running at all, then we will not consider that a problem at all and > > return success. > > I don't think this makes sense. Reload is a "softer" alternative; if I > say "reload-or-try-restart", that should be considered as asking less > from systemd - full restart is not necessary, just reload is enough if > that option is available. Asking for less should not be a reason to > block longer! "try-restart" also waits for any already queued start job to finish, if there's any, before it returns, hence "reload-or-try-restart" and "try-restart" actually block for the same time... Not following here... > > This is really what you want actually, because this allows admins (and > > scripts) to change some daemon configuration, and then finish off by > > issuing "reload-or-try-restart" on the daemon, so that the changes take > > effect, and then immediately talk to the daemon, knowing that if it was > > running, then the new configuration is definitely in effect. If it > > wasn't, then of course we can't talk to the service, but that's OK of > > course. > > Exactly, this is a common usecase. But note that this case supports the > changed semantics of the patch, and does not support your view! > > After an operation that changes configuration, > "reload-or-try-restart" (or just "reload" if we know reload is in fact > supported by the daemon and sufficient for the configuration change in > question) should guarantee that further requests use the new > configuration. It should not unnecessarily block until the daemon is > actually running if it's being started; anything which relies on it > being up should test for that separately, independently of configuration > change handling. And there's no reason why implementing lighter-weight > reloads to replace full restarts should lead to systemd blocking > longer. No. It *should* wait until any start request that is already queued is complete... Again, it's about the *outcome*, and about being *positive*, really. Nothing else. We should not make things fail, if we already *know* that there's alerady something going on that makes the thing we are trying to do succeed. I think most of the confusion here comes from the fact that sysv service restarts don't care about ordering at all, really, and we do. But the answer to that is not to weaken the current strong semantics of blocking, but simply not to request blocking operation at all, i.e. use --no-block, and just queue thi
Re: [systemd-devel] compile with clang broken
On Fri, 15.08.14 12:40, Thomas H.P. Andersen (pho...@gmail.com) wrote:
>
> On Fri, Aug 15, 2014 at 12:35 PM, David Herrmann
> wrote:
> > Hi
> >
> > On Fri, Aug 15, 2014 at 12:29 PM, Thomas H.P. Andersen
> > wrote:
> >> On Fri, Aug 15, 2014 at 11:49 AM, David Herrmann
> >> wrote:
> >>> Thanks for trying!
> >>>
> >>> Result is as I expected. Evaluation takes place _after_ validating
> >>> compile-time constants, and thus __builtin_constant_p in combination
> >>> with ?: will not work if not both cases are constant. Maybe it works
> >>> with __builtin_choose_expr()?
> >>>
> >>> Can you try the attached patch?
> >>
> >> This patch works. It also needs the change to do the calculation to a
> >> seperate line. Also only if size is const, like so:
> >> const size_t size = MAX(sizeof(struct in_pktinfo), sizeof(struct
> >> in6_pktinfo));
> >
> > Again, thanks for trying it out!
>
> no problem. I have inserted the relevant error messages for the two
> non-working cases.
>
> > I don't understand your comment, though. You're saying this works:
> >
> > const size_t size = MAX(...);
> > uint8_t buffer[CMSG_SPACE(size) +...];
> >
> > ...but this doesn't work:
> >
> > uint8_t buffer[CMSG_SPACE(MAX(...)) +...];
>
> src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
> member must be initialized out of line
> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
> in_pktinfo), sizeof(struct in6_pktinfo)))
> ^
>
> > ...and this doesn't work either (mind the dropped 'const'):
> >
> > size_t size = MAX(...);
> > uint8_t buffer[CMSG_SPACE(size) +...];
>
> src/resolve/resolved-dns-stream.c:68:25: error: fields must have a
> constant size: 'variable length array in structure' extension will
> never be supported
> uint8_t buffer[CMSG_SPACE(size)
> ^
>
> > Hm. This is weird. Maybe CMSG_SPACE does something weird. I'll see.
> > David
In this case I really think we should just detect LLVM and define MAX()
and MIN() to something weaker that doesn't use ({ ... }) expressions...
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] Thoughts about /etc/crypttab keyscript options
On Thu, Aug 14, 2014 at 08:18:10PM +0200, Lennart Poettering wrote: > On Thu, 14.08.14 20:10, Marc Haber (mh+systemd-de...@zugschlus.de) wrote: > > > Not aware of an C++ code. There's a vala one, and of course the one we > > > ship in systemd itself in C, but c++ i cannot help you with, sorry. > > > > Is it possible to write a PasswordAgent in shell? Example code please > > ;) > > Probably possible, after all bash allows you to talk to unix sockets and > stuff. And you could probably put the protocol together with carefully > crafted echo lines, but I know of nobody who has done that so far... There is also the daemonizing and inotify part... > > > I fear I don#t have an easy suggestion. What kind of device do you > > > actually want to make work here? some smartcard or so? > > > > That's the vision, yes. At the moment, my keyscript unlocks a small > > LUKS partition on the disk and takes the key for the root fs from > > there. That's just a placeholder for a future more complicated setup. > > Not following. You place a key for a LUKS partition on another LUKS > partition? What's the benefit of that? Inception? ;-) It's actually part of a two-factor-authentification for the poor. The part to know is the key to the LUKS parition, the part to have is an USB key. The key script hashes part of the key found on the USB key and part of the key found on the LUKS partition on the hard disk together to build the actual key to unlock the root fs. I use this scheme for so long that I don't even remember why I implemented it this way, but I guess it was because the logic to unlock a LUKS fs from initramfs was already in place and could be used as-is to unlock the key-part-holding LUKS partition instead of the actual root fs that it was intended for. But I also know of people who use a keyscript to unlock LUKS file systems with the key stored in the system's TPM or on a crypto card. I have never looked into the details of those implementations (having that saved for a long winter night), but I guess that those people will also be pretty hosed on a systemd-based Debian. > > First step to do that would be to implement support for the keyscript= > > option in /etc/crypttab as this is the canonical place to hook into on > > non-system systems. At least it's the case on Debian, I don't know > > about Red Hat, Fedora and other distributions. > > Well, I am really not convinced that this is a well-defined > interface. Even in your case you have to wait for two devices at least, > right? a synchronous shell callout won't solve that for you since > there's no guarantee that the second LUKS device has already shown up, > or am I missing something? It may be possible that /etc/crypttab is processed sequentially which would obviously not be the case with systemd. So you have a point here. > As mentioned we really should redesign the whole thing around the kernel > keyring anyway, I am really conservative in adding support for Debian's > keyscript thingy upstream now. (That of course shouldn't stop Debian > from adding this downstream) It didn't occur to me that /etc/crypttab's keyscript= option was a Debianism. I educated myself in the mean time. I'm going to yell at the Debian systemd team now ;-) > > The PasswordAgent stuff is really neat, but complicated due to the > > socket communication, and it's far from being a drop-in replacement. > > Well, it's really easy in C I figure, but admittedly not in shell... It would be significantly easier if there were boilerplate code to derive from. To a non-adept programmer, adapting the boilerplate would probably lead to enough buffer overflow vulnerabilities anyway ;-) Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Leimen, Germany| lose things."Winona Ryder | Fon: *49 6224 1600402 Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600420 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 12:40 PM, Thomas H.P. Andersen wrote:
> On Fri, Aug 15, 2014 at 12:35 PM, David Herrmann
> wrote:
>> Hi
>>
>> On Fri, Aug 15, 2014 at 12:29 PM, Thomas H.P. Andersen
>> wrote:
>>> On Fri, Aug 15, 2014 at 11:49 AM, David Herrmann
>>> wrote:
Thanks for trying!
Result is as I expected. Evaluation takes place _after_ validating
compile-time constants, and thus __builtin_constant_p in combination
with ?: will not work if not both cases are constant. Maybe it works
with __builtin_choose_expr()?
Can you try the attached patch?
>>>
>>> This patch works. It also needs the change to do the calculation to a
>>> seperate line. Also only if size is const, like so:
>>> const size_t size = MAX(sizeof(struct in_pktinfo), sizeof(struct
>>> in6_pktinfo));
>>
>> Again, thanks for trying it out!
>
> no problem. I have inserted the relevant error messages for the two
> non-working cases.
>
>> I don't understand your comment, though. You're saying this works:
>>
>> const size_t size = MAX(...);
>> uint8_t buffer[CMSG_SPACE(size) +...];
>>
>> ...but this doesn't work:
>>
>> uint8_t buffer[CMSG_SPACE(MAX(...)) +...];
>
> src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
> member must be initialized out of line
> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
> in_pktinfo), sizeof(struct in6_pktinfo)))
> ^
Ok, this can be fixed by adding "const" to the variables inside the ({
}) else-clause. But we then end up with:
error: statement expression not allowed at file scope
Hmm..
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
> > src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
> > member must be initialized out of line
> > uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
> > in_pktinfo), sizeof(struct in6_pktinfo)))
> > ^
>
> Ok, this can be fixed by adding "const" to the variables inside the ({
> }) else-clause. But we then end up with:
> error: statement expression not allowed at file scope
I wonder if there's *any* way how to implement a double-evalutation-free
all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
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] Thoughts about /etc/crypttab keyscript options
On Fri, 15.08.14 12:56, Marc Haber (mh+systemd-de...@zugschlus.de) wrote: > > > Is it possible to write a PasswordAgent in shell? Example code please > > > ;) > > > > Probably possible, after all bash allows you to talk to unix sockets and > > stuff. And you could probably put the protocol together with carefully > > crafted echo lines, but I know of nobody who has done that so far... > > There is also the daemonizing and inotify part... > > > > > I fear I don#t have an easy suggestion. What kind of device do you > > > > actually want to make work here? some smartcard or so? > > > > > > That's the vision, yes. At the moment, my keyscript unlocks a small > > > LUKS partition on the disk and takes the key for the root fs from > > > there. That's just a placeholder for a future more complicated setup. > > > > Not following. You place a key for a LUKS partition on another LUKS > > partition? What's the benefit of that? Inception? ;-) > > It's actually part of a two-factor-authentification for the poor. The > part to know is the key to the LUKS parition, the part to have is an > USB key. The part to have is trivially easy to copy, so why do the excercise at all? Sounds more like theatre to me... > But I also know of people who use a keyscript to unlock LUKS file > systems with the key stored in the system's TPM or on a crypto card. I > have never looked into the details of those implementations (having > that saved for a long winter night), but I guess that those people > will also be pretty hosed on a systemd-based Debian. I think supporting TPM or smartcards out of the box is very desirable to have upstream. I am not convinced though that Debian's keyscript= logic is really that well designed that I want to update it upstream. (But again, Debian can ship such a patch downstream) > > > First step to do that would be to implement support for the keyscript= > > > option in /etc/crypttab as this is the canonical place to hook into on > > > non-system systems. At least it's the case on Debian, I don't know > > > about Red Hat, Fedora and other distributions. > > > > Well, I am really not convinced that this is a well-defined > > interface. Even in your case you have to wait for two devices at least, > > right? a synchronous shell callout won't solve that for you since > > there's no guarantee that the second LUKS device has already shown up, > > or am I missing something? > > It may be possible that /etc/crypttab is processed sequentially which > would obviously not be the case with systemd. So you have a point > here. Modern design probing really doesn't work that way anymore. You never know when all devices have shown up. If you want to implement something like your USB key thing, then you'd have to explicitly wait for both devices. On old Debian and the other sysv distros, boot scripts generally included an "udev settle" call, which was supposed to wait until "all" devices have shown up. But that's not actually something that could work at all on many of the newer tehcnologies where the probing time is basically unbounded. One of those busses is actually USB, where there's no restriction made at all, when devices have to have told the OS about the storage devices they want to provide (and this fact is used by android phones, where you need to press OK on the phone before a USB device pops up in the system, after you connected the cable). So, nowadays no sane distribution uses "udev settle" anymoer, since it cannot deliver what people assume it delivers, and in particular not on USB, which you are looking for. Anyway, long story short: what you are doing worked mostly out of luck, not out of correctness.. > > > The PasswordAgent stuff is really neat, but complicated due to the > > > socket communication, and it's far from being a drop-in replacement. > > > > Well, it's really easy in C I figure, but admittedly not in shell... > > It would be significantly easier if there were boilerplate code to > derive from. To a non-adept programmer, adapting the boilerplate would > probably lead to enough buffer overflow vulnerabilities anyway ;-) Well, we have examples in our code, but really only in C, and they are not minimalist examples, but real code... 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] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
wrote:
> On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
>
>> > src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
>> > member must be initialized out of line
>> > uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
>> > in_pktinfo), sizeof(struct in6_pktinfo)))
>> > ^
>>
>> Ok, this can be fixed by adding "const" to the variables inside the ({
>> }) else-clause. But we then end up with:
>> error: statement expression not allowed at file scope
>
> I wonder if there's *any* way how to implement a double-evalutation-free
> all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
I looked around and it seems like there's nothing we can do. Weird
thing is, LLVM allows const-initialization but not member-definition
with that macro. I really don't understand why..
I somehow think adding MAX_CONST which uses __builtin_constant_p and
assert_cc() is the easiest way here. That is, we use MAX_CONST() for
all cases where MAX fails. I think this is the easiest way to
guarantee no-one else changes the code to use MAX() again.
Furthermore, it guarantees that MAX_CONST is *really* called with
constant arguments.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Thoughts about /etc/crypttab keyscript options
On Fri, Aug 15, 2014 at 01:30:32PM +0200, Lennart Poettering wrote: > On Fri, 15.08.14 12:56, Marc Haber (mh+systemd-de...@zugschlus.de) wrote: > > > > Is it possible to write a PasswordAgent in shell? Example code please > > > > ;) > > > > > > Probably possible, after all bash allows you to talk to unix sockets and > > > stuff. And you could probably put the protocol together with carefully > > > crafted echo lines, but I know of nobody who has done that so far... > > > > There is also the daemonizing and inotify part... > > > > > > > I fear I don#t have an easy suggestion. What kind of device do you > > > > > actually want to make work here? some smartcard or so? > > > > > > > > That's the vision, yes. At the moment, my keyscript unlocks a small > > > > LUKS partition on the disk and takes the key for the root fs from > > > > there. That's just a placeholder for a future more complicated setup. > > > > > > Not following. You place a key for a LUKS partition on another LUKS > > > partition? What's the benefit of that? Inception? ;-) > > > > It's actually part of a two-factor-authentification for the poor. The > > part to know is the key to the LUKS parition, the part to have is an > > USB key. > > The part to have is trivially easy to copy, so why do the excercise > at all? Sounds more like theatre to me... Because I still hope to have that in a more secure way in the near future. > > But I also know of people who use a keyscript to unlock LUKS file > > systems with the key stored in the system's TPM or on a crypto card. I > > have never looked into the details of those implementations (having > > that saved for a long winter night), but I guess that those people > > will also be pretty hosed on a systemd-based Debian. > > I think supporting TPM or smartcards out of the box is very desirable to > have upstream. Yes, and that should be done in a modular way so that even exotic (or broken) schemes can be plugged in. > I am not convinced though that Debian's keyscript= logic is really > that well designed that I want to update it upstream. You don't need to. I falsely thought that this was general functionality and not a Debianism. Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Leimen, Germany| lose things."Winona Ryder | Fon: *49 6224 1600402 Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600420 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
On Fri, 15.08.14 13:42, David Herrmann (dh.herrm...@gmail.com) wrote:
>
> Hi
>
> On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
> wrote:
> > On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
> >
> >> > src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
> >> > member must be initialized out of line
> >> > uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
> >> > in_pktinfo), sizeof(struct in6_pktinfo)))
> >> > ^
> >>
> >> Ok, this can be fixed by adding "const" to the variables inside the ({
> >> }) else-clause. But we then end up with:
> >> error: statement expression not allowed at file scope
> >
> > I wonder if there's *any* way how to implement a double-evalutation-free
> > all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
>
> I looked around and it seems like there's nothing we can do. Weird
> thing is, LLVM allows const-initialization but not member-definition
> with that macro. I really don't understand why..
>
> I somehow think adding MAX_CONST which uses __builtin_constant_p and
> assert_cc() is the easiest way here. That is, we use MAX_CONST() for
> all cases where MAX fails. I think this is the easiest way to
> guarantee no-one else changes the code to use MAX() again.
> Furthermore, it guarantees that MAX_CONST is *really* called with
> constant arguments.
If that is what it takes, go ahead.
Let it be known though for all future: I think LLVM is stupid here.
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] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 1:53 PM, Lennart Poettering
wrote:
> On Fri, 15.08.14 13:42, David Herrmann (dh.herrm...@gmail.com) wrote:
>
>>
>> Hi
>>
>> On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
>> wrote:
>> > On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
>> >
>> >> > src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
>> >> > member must be initialized out of line
>> >> > uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
>> >> > in_pktinfo), sizeof(struct in6_pktinfo)))
>> >> > ^
>> >>
>> >> Ok, this can be fixed by adding "const" to the variables inside the ({
>> >> }) else-clause. But we then end up with:
>> >> error: statement expression not allowed at file scope
>> >
>> > I wonder if there's *any* way how to implement a double-evalutation-free
>> > all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
>>
>> I looked around and it seems like there's nothing we can do. Weird
>> thing is, LLVM allows const-initialization but not member-definition
>> with that macro. I really don't understand why..
>>
>> I somehow think adding MAX_CONST which uses __builtin_constant_p and
>> assert_cc() is the easiest way here. That is, we use MAX_CONST() for
>> all cases where MAX fails. I think this is the easiest way to
>> guarantee no-one else changes the code to use MAX() again.
>> Furthermore, it guarantees that MAX_CONST is *really* called with
>> constant arguments.
>
> If that is what it takes, go ahead.
>
> Let it be known though for all future: I think LLVM is stupid here.
Meh, static_assert() is not allowed there. _Pragma(error) works, but
is always evaluated even with __builtin_choose_expr(). This is all
stupid... That means MAX_CONST really just becomes A>B?A:B.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 1:53 PM, Lennart Poettering
wrote:
> On Fri, 15.08.14 13:42, David Herrmann (dh.herrm...@gmail.com) wrote:
>
>>
>> Hi
>>
>> On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
>> wrote:
>> > On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
>> >
>> >> > src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
>> >> > member must be initialized out of line
>> >> > uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
>> >> > in_pktinfo), sizeof(struct in6_pktinfo)))
>> >> > ^
>> >>
>> >> Ok, this can be fixed by adding "const" to the variables inside the ({
>> >> }) else-clause. But we then end up with:
>> >> error: statement expression not allowed at file scope
>> >
>> > I wonder if there's *any* way how to implement a double-evalutation-free
>> > all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
>>
>> I looked around and it seems like there's nothing we can do. Weird
>> thing is, LLVM allows const-initialization but not member-definition
>> with that macro. I really don't understand why..
>>
>> I somehow think adding MAX_CONST which uses __builtin_constant_p and
>> assert_cc() is the easiest way here. That is, we use MAX_CONST() for
>> all cases where MAX fails. I think this is the easiest way to
>> guarantee no-one else changes the code to use MAX() again.
>> Furthermore, it guarantees that MAX_CONST is *really* called with
>> constant arguments.
>
> If that is what it takes, go ahead.
>
> Let it be known though for all future: I think LLVM is stupid here.
Ok, took me a while, but I now figured out how to cause compilation to
fail even in expressions that initialize types (_Static_assert is not
allowed there):
#define assert_const(expr)
((void)(__builtin_types_compatible_p(int[(expr) ? 1 : -1], int[1])))
Btw., I like that more than our current assert_cc() fallback. But I
leave it up to you to decide.
Anyhow, I found a way to make CONST_MAX work:
#define CONST_MAX(_A, _B)
(__builtin_choose_expr(__builtin_constant_p(_A) &&
__builtin_constant_p(_B), ((_A) > (_B)) ? (_A) : (_B), (void)0))
This will return (void) in case _A or _B is not constant. Works fine
on LLVM, I now have to test it on gcc. If it works, I will commit it
and fix resolvd.
Cheers
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] resolved: fix warnings
On Thu, 14.08.14 21:07, Daniel Buch (boogiewasth...@gmail.com) wrote: > I just hit this assert on my arch system with gcc 4.9, > > dbuch-laptop systemd-resolved[457]: Assertion 's->protocol == > DNS_PROTOCOL_LLMNR' failed at src/resolve/resolved-dns-scope.c:369 Yuck! Fixed now in git. 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] resolved: don't fail if IPv6 is not available
On Fri, 15.08.14 08:41, Michael Olbrich (m.olbr...@pengutronix.de) wrote: > > On Wed, Aug 13, 2014 at 03:04:20PM +0200, Lennart Poettering wrote: > > I applied a different patch now that makes sure we either get the full > > IPv6 support or none at all, and doesn't generate a warning. > > > > Please have a look, if this fixes things for you. > > This work now. However I had to revert > 5ba73e9b646af4d8109a5a633aa235665858144d (resolved: clarify that LLMNR > scopes must have a link assigned). dns_scope_llmnr_membership() is called > twice with s->protocol = DNS_PROTOCOL_DNS. dns_scope_new() calls it with > the specified link and protocol here: > > $ git grep dns_scope_new | grep AF_UNSPEC > src/resolve/resolved-link.c:r = > dns_scope_new(l->manager, &l->unicast_scope, l, DNS_PROTOCOL_DNS, AF_UNSPEC); > src/resolve/resolved-manager.c:r = dns_scope_new(m, > &m->unicast_scope, NULL, DNS_PROTOCOL_DNS, AF_UNSPEC); Fixed now in git! 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] compile with clang broken
Hi On Fri, Jul 18, 2014 at 4:02 PM, Thomas H.P. Andersen wrote: > 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support > for looking up names) broke the build on clang. > > src/resolve/resolved-manager.c:553:43: error: non-const static data > member must be initialized out of line > uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct > in6_pktinfo))) > > Moving the MAX(...) to a separate line fixes that problem but another > error then happens: > > src/resolve/resolved-manager.c:554:25: error: fields must have a > constant size: 'variable length array in structure' extension will > never be supported > uint8_t buffer[CMSG_SPACE(size) > > We have encountered the same problem before and Lennart was able to > write the code in a different way. Would this be possible here too? I now pushed 3 commits which should fix this. Works fine with LLVM+clang and GCC here. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add DEPLOYMENT to hostnamectl
On Tue, 08.07.14 16:52, David Timothy Strauss (da...@davidstrauss.net) wrote: > As someone who deploys developer VMs and production ones, this is > useful. Will it be possible to make units have ConditionDeployment=? > That would allow disabling, say, pushes of log messages to our log > aggregation servers from development and testing deployments. I am happy to take patches for btw. Should be pretty trivial. 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] Extending machine-info to include machine roles
On Tue, 08.07.14 23:07, Jóhann B. Guðmundsson (johan...@gmail.com) wrote: > Now let's start the dialog with machine roles and start by agreeing > what roles are What's the latest on this? I'd be willing to take a patch for this, but he same rules as for deployment should apply: no strict checks against a vocabulary, but a vocabulary suggestion in the man page. If anyone is still interested, ... 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] readahead: add option to create pack in directory other than root
On Tue, 08.07.14 14:38, Chaiken, Alison (alison_chai...@mentor.com) wrote: > >I am actually all for moving the file, though. But can't we find a more > >automatic solution for this, that doesn't > >require manual configuration. Maybe a scheme like this could work: > >a) readahead-reply would look for both /.readahead and > > /var/lib/systemd/readahead. If both files exist, it picks the newer > >one, if only one exists it picks that one. > >b) readahead-collect would check if /var/lib/systemd is on the same > >mount point as /. If so, it would store the file in > > /var/lib/systemd/readahead. Otherwise it would store the file in > > /.readahead, as before. > >This would move the file for most folks, while still allowing a split off > >/var -- but I figure this wouldn't solve your specific problem? > > In our case, /var/lib/ is part of / partition that is unwritable, but > /var/opt is not. How about a tristate option, where we implement the > solution you suggest above, but also allow runtime specification to > override the other two choices? I think we should cover the usual setups with systemd upstream only, where /var is variable, as the name suggest, and hence writable during runtime. It is OK to run things differently, but that would have to be a downstream change. That all said, we have the intention to remove systemd-readahead entirely from the package (see announcement the other day), hence I am not sure it's worth investing any more time in this anyway... 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] readahead: add option to create pack in directory other than root
On Tue, 08.07.14 19:02, Chaiken, Alison (alison_chai...@mentor.com) wrote: > > Lennart suggest: > >I am actually all for moving the file, though. But can't we find a more > >automatic solution for this, that doesn't > >require manual configuration. Maybe a scheme like this could work: > >a) readahead-reply would look for both /.readahead and > > /var/lib/systemd/readahead. If both files exist, it picks the newer > > one, if only one exists it picks that one. > > Upon further consideration, do we want a SUID program that is active > at early boot to *automatically* prefer a newer file on a writable > partition to an older one on a read-only partition? systemd-readahead is not SUID, it just runs as root... > Another aspect worth further consideration is, what is a graceful > fallback strategy if for some reason the pack file gets corrupted? > Might it prevent a board with systemd-readahead-replay enabled from > coming all the way up? Has anyone ever observed a hang due to a > corrupt pack? When a watchdog reboots a hung board, perhaps we want > to use a dynamic mechanism to point the pack file to /dev/null if we > don't come up far enough to run "systemctl disable > systemd-readahead-replay"? The tool is only an optimization anyway. if something goes wrong, we immediately terminate and do nothing. 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] User sessions: limit the ability to migrate cgroups
On 08/13/2014 12:11 PM, Alban Crequy wrote: > On Wed, 13 Aug 2014 16:37:17 +0200 > Lennart Poettering wrote: > >> On Thu, 07.08.14 15:19, Alban Crequy (alban.cre...@collabora.co.uk) >> wrote: >> >>> Hi, >>> >>> Should unprivileged processes be allowed to change cgroup? >> Well, they shouldn#t do it. But I think it's OK as long as this is >> only done within the specific user's hierarchies. >> >>> As I understand it, it is not possible to block processes to >>> leave a cgroup, but only to block processes to enter a cgroup. >> Correct. >> >>> In the following example, session-c4.scope/tasks belongs to >>> root:root with -rw-r--r-- and user@1000.service/tasks belongs to >>> user:user with -rw-r--r--. >> Yes, this is because systemd --user needs to be able to manage its own >> cgroup subtree, so we have to open this up for the user@1000.service >> service, but keep it restricted otherwise... > It makes sense. > >>> So processes can freely move from session-c4.scope to >>> user@1000.service. But not in the other direction. >> Correct. >>> $ systemd-cgls >>> Working Directory /sys/fs/cgroup/systemd/user.slice/user-1000.slice: >>> ├─session-c4.scope >>> │ ├─713 sshd: user [priv] >>> │ ├─722 sshd: user@pts/2 >>> │ ├─723 -bash >>> │ ├─732 systemd-cgls >>> │ └─733 pager >>> ├─user@1000.service >>> │ ├─406 /lib/systemd/systemd --user >>> >>> With user sessions managed by systemd, will it be possible to >>> restrict unprivileged users from migrating to other cgroups? >> Unlikely. Access control on Unix is generally bound to user IDs, not >> processes, and we really shouldn't start here with departing from >> that... > I tested SELinux and AppArmor to restrict /sys/fs/cgroup/. > > SELinux didn't help because the cgroup file system does not support > extended attributes such as "security.selinux", but AppArmor was able > to block an application from changing cgroup. > > Alban > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel You could mount the cgroup with a label that the intended domain could not write. Similarly you could just prevent the domain from writing to cgroupfs_t. Which is the default label of the cgroup file system. No confined/Few domains right now should be allowed to write to cgroupfs_t. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] How to suspend and wait for resume
On Mon, 07.07.14 21:17, Maciej Piechotka (uzytkown...@gmail.com) wrote: > Hi, > > I have following problem. I'd like to suspend and then after the > system resumes execute a command - problem is that systemctl suspend > finishes immediately, without waiting for the resume. Is there a way > of executing a command after the resume has happened as user or do I > need to add something as resume dispatcher? I think we should fix "systemctl suspend" to simply block until we come back. Added to the TODO list now. 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 v3 2/3] cxgb4: use module_long_probe_init()
On Fri, Aug 15, 2014 at 02:14:58AM +0200, Luis R. Rodriguez wrote: > This driver also uses module_pci_driver() so a module_long_probe_driver() > and respective module_long_probe_pci_driver() would need to be considered > if but easily implemented (sent to Alex to test). No, don't create bus-only versions of the "long probe" function, just unwrap the module_pci_driver() logic and use the module_long_probe() call, we want it to be "obvious" that something is odd here and needs to be fixed someday. thanks, greg k-h ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
On 15/08/2014 16:30, David Herrmann wrote:
> Ok, took me a while, but I now figured out how to cause compilation to
> fail even in expressions that initialize types (_Static_assert is not
> allowed there):
> #define assert_const(expr)
> ((void)(__builtin_types_compatible_p(int[(expr) ? 1 : -1], int[1])))
>
> Btw., I like that more than our current assert_cc() fallback. But I
> leave it up to you to decide.
>
> Anyhow, I found a way to make CONST_MAX work:
> #define CONST_MAX(_A, _B)
> (__builtin_choose_expr(__builtin_constant_p(_A) &&
> __builtin_constant_p(_B), ((_A) > (_B)) ? (_A) : (_B), (void)0))
>
> This will return (void) in case _A or _B is not constant. Works fine
> on LLVM, I now have to test it on gcc. If it works, I will commit it
> and fix resolvd.
Hello,
this may be completely stupid, but if the only use case you have for
CONST_MAX() is for computing the size of a data structure, I find
something like
#define MAXSIZE(A, B) sizeof(union { __typeof(A) a; __typeof(B) b;})
a little more clear and less magic, and I believe it has the same
guarantees that the solution you found.
In the specific case, the problematic line could then be written as:
uint8_t buffer[CMSG_SPACE(MAXSIZE(struct in_pktinfo,
struct in6_pktinfo)) \
+ EXTRA_CMSG_SPACE];
which IMHO reads a bit better.
Cheers,
Daniele
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Inconsistent processing of 'debug' and 'systemd.log_level' options from kernel command line
On Tue, 01.07.14 23:47, Mike Gilbert (flop...@gentoo.org) wrote: > I have noticed that when the 'debug' option is passed on the kernel > command line, it is impossible to override this using the > 'systemd.log_level' option. > > I also note that passing SYSTEMD_LOG_LEVEL on the kernel command line > DOES work; the kernel copies this to the environment block for init. > > Looking through the code, it looks like the 'debug' option is > processed twice by systemd[1] in src/core/main.c: > > 1. In parse_proc_cmdline(), 'debug' processed along with all other > kernel command line options including 'systemd.log_level'. > > 2. In log_parse_environment(), 'debug' is processed, but > systemd.log_level is ignored. > > Is this a bug? It seems quite strange. Yeah, I fucked that one up, as it appears. Fixed now. (It was even worse that that, we ended up parsing these kernel cmdline options three times, in the worst case, not just two...) 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] compile with clang broken
On Fri, 15.08.14 17:22, Daniele Nicolodi (dani...@grinta.net) wrote:
>
> On 15/08/2014 16:30, David Herrmann wrote:
> > Ok, took me a while, but I now figured out how to cause compilation to
> > fail even in expressions that initialize types (_Static_assert is not
> > allowed there):
> > #define assert_const(expr)
> > ((void)(__builtin_types_compatible_p(int[(expr) ? 1 : -1], int[1])))
> >
> > Btw., I like that more than our current assert_cc() fallback. But I
> > leave it up to you to decide.
> >
> > Anyhow, I found a way to make CONST_MAX work:
> > #define CONST_MAX(_A, _B)
> > (__builtin_choose_expr(__builtin_constant_p(_A) &&
> > __builtin_constant_p(_B), ((_A) > (_B)) ? (_A) : (_B), (void)0))
> >
> > This will return (void) in case _A or _B is not constant. Works fine
> > on LLVM, I now have to test it on gcc. If it works, I will commit it
> > and fix resolvd.
>
> Hello,
>
> this may be completely stupid, but if the only use case you have for
> CONST_MAX() is for computing the size of a data structure, I find
> something like
>
> #define MAXSIZE(A, B) sizeof(union { __typeof(A) a; __typeof(B) b;})
>
> a little more clear and less magic, and I believe it has the same
> guarantees that the solution you found.
>
> In the specific case, the problematic line could then be written as:
>
> uint8_t buffer[CMSG_SPACE(MAXSIZE(struct in_pktinfo,
> struct in6_pktinfo)) \
>+ EXTRA_CMSG_SPACE];
>
> which IMHO reads a bit better.
I'd be fine with that, if anyone wants to put together 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] Misleading udev error messages regarding virtual interfaces
On Sun, 06.07.14 12:43, Leonid Isaev (lis...@umail.iu.edu) wrote: > Hi, > > Sorry for a delayed reply. > > On Thu, Jul 03, 2014 at 01:46:53PM +0200, Lennart Poettering wrote: > > it would be good to know what the precise error output is you get now > > with this new change... > > With systemd-215 udevd still complains (and segfaults) about the virtual > interface: > -- > $ journalctl | grep udevd > Jul 06 12:21:05 hermes systemd-udevd[46]: starting version 215 > Jul 06 12:21:05 hermes systemd-udevd[226]: starting version 215 > Jul 06 12:21:06 hermes systemd-udevd[234]: renamed network interface wlan0 to > wlp1s0 > Jul 06 12:21:09 hermes systemd-udevd[226]: worker [233] terminated by signal > 11 (Segmentation fault) > Jul 06 12:21:09 hermes kernel: systemd-udevd[233]: segfault at 0 ip > 7ff524a0571a sp 7fffc8a69540 error 4 in > libc-2.19.so[7ff524907000+1a4000] > Jul 06 12:21:09 hermes systemd-udevd[226]: worker [233] failed while handling > '/devices/virtual/net/veth7DH07K' > -- > > As before, things seem to work i.e. I can still see servers inside containers. > The kernel is 3.15.3. Does this still occur with current systemd git? Any chance to get a backtrace for this with current git? 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] Work on adding polkit support to systemd1
On 13.08.2014 20:27, Lennart Poettering wrote: > On Wed, 06.08.14 13:23, Stef Walter (st...@redhat.com) wrote: > >> I've done initial work on adding polkit support to systemd1 DBus >> methods. You can see it here: Thanks for the review. Worked on this a bit more. I might drop off the face of the earth for a couple weeks. In case I do, I thought I'd update my public branch. But if I'm around, I'll test and prepare a patch set early next week. >> https://github.com/stefwalter/systemd/commits/polkit-systemd1 Updated this branch ^^ >> The way that each callback in sd-bus has to handle verification seems a >> bit risky to me. So I've only opened up the specific interfaces I >> touched in the DBus policy file. Your comments below address what I was worried about here ^^ > Regarding the fourth: please don't bind any checks against the UID, > check for CAP_SYS_ADMIN instead, we make it equally easy. Done. > Why is verify_root_sync() invoked that often? I mean, for these cases > the dbus1 policy should not grant access anyway, so we don't really need > to check for permissions here again. I'd really prefer if on dbus1 would > use the dbus1 policy for as much access control work as useful, and only > do it on our own if we really need to. Done. > reload (and probably also reexecute) should probably hook into polkit > too, with a new action? it sounds useful enough for people to invoke it > frequently. New action is called: org.freedesktop.systemd1.reload-daemon > Note that on kdbus access control works differently than on dbus1: it's > the client's responsibility to implement access control. To make this > easy our sd-bus library actually allows encoding in the method vtable > which calls are accessible for unpriviliged processes. That's what the > SD_BUS_VTABLE_UNPRIVILEGED flag in the object vtables is for (grep for > it). The flag (or its absence) has no effect on dbus1 daemons at all, > and only matters for kdbus. To make sure your changes also work > correctly and as intended on kdbus you need to add the flag to all > methods that you are now opening up via polkit. Because otherwise method > calls won't even get that far. > > Or in other words: everything that is opened up in the dbus1 policy also > needs to be opened up with SD_BUS_VTABLE_UNPRIVILIGED in the object > vtables... Makes sense ... but out of interest, why don't you just rely on this for dbus1 as well? It seems that maintaining these two distinct access control methods is risky. > Sometiems you added spurious newlines to the patches, please don't. Removed a couple extra newlines. I hope I found them all. > I'd prefer if the dbus1 policy would precisely list the method calls > that are now opened up... Done. One other thing is that it seems like the ofs.Manager.LoadUnit() DBus method call is wide open for anyone to call. Is this intentional? I don't know all the implications, but wanted to highlight it. Stef ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-hostnamed not shutting down when unused
On Mon, 07.07.14 09:46, Umut Tezduyar Lindskog (u...@tezduyar.com) wrote: > > Hi, > > I have tested it and it is working on git. > > I dug this up to figure few things out on shutting down dbus activated > services when they are idle. Is it possible that follow up activation > request on a service is being ignored by systemd because application > has just recently quit? > > I have this extreme case, > > - Service shuts down > - Dbus gets a signal up on service shutdown (systemd doesn't get > SIGCHLD yet) and acknowledges disappearance of the service > - An activation request comes up on the service > - Dbus sends a dbus signal to systemd to activate the service > - Systemd receives the request but is still thinking service is > active, so ignores the request > - Systemd receives the SIGCHLD and changes the state of service to inactive. > > This scenario is very unlikely but I am trying to figure out if there > are other possibilities. Thoughts? Hmm, interesting issue. Not entirely sure though what the right strategy could be here... Something like this could work: With sd_notify() hostnamed should tell PID 1 when it begins its shutdown. This should then be reflected in the service state PID 1 keeps track off. The sd_notify() socket should be processed at a higher priority than dbus, so that this is always processed first. With that in place, if dbus then sends an activation request it would be queued by PID, and be processed as soon as the service stop is complete, so that the service start is then done immediately after. Introducing sd_notify() messages that can notify PID 1 about daemons reloading or shutting down, has been on the TODO list for a while, I wanted that for mere prettiness, but now it turns out this might actually fix a race... I figure this problem will go away with kdbus, as the activation request stays around until it is processed (because it it is a durable POLLIN on the activation fd). But then again, being able to tell systemd about state changes in the daemon is still very useful. Internally, hostnamed will actually drop the name first, and then process requests as long as there's still something queued, and only then terminate. THis doesn't help here, but is supposed to at least fix the most obvious race, where bus messages might get stuck and dropped in the socket... 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] [RFC] [PATCH] Follow symlinks in /lib
On Mon, 03.03.14 10:09, Michael Stapelberg (stapelb...@debian.org) wrote:
Heya,
Hmm, I think I never reviewed this patch. Sorry for the delay! It
sometimes takes a while for me to review something, but this one is
bd... Sorry for that.
>
> +#define FOLLOW_MAX 8
> +
> static int unit_file_search(
> InstallContext *c,
> InstallInfo *info,
> @@ -1053,11 +1055,44 @@ static int unit_file_search(
> if (!path)
> return -ENOMEM;
>
> +int cnt = 0;
> +for (;;) {
> +if (cnt++ >= FOLLOW_MAX)
> +return -ELOOP;
> +
> +r = unit_file_load(c, info, path, allow_symlink);
> +
> +/* symlinks are always allowed for units in
> {/usr,}/lib/systemd so that
> + * one can alias units without using Alias= (the
> downside of Alias= is
> + * that the alias only exists when the unit is
> enabled). */
> +if (r >= 0)
> +break;
> +
> +if (r != -ELOOP)
> +break;
> +
> +if (allow_symlink)
> +break;
> +
> +if (!path_startswith(path, "/lib/systemd") &&
> +!path_startswith(path, "/usr/lib/systemd"))
> +break;
> +
> +char *target;
Even though C99 allows declaring variables in the middle of blocks, we
don't use that, and keep code and declarations separate.
> +r = readlink_and_make_absolute(path, &target);
> +if (r < 0)
> +return r;
> +free(path);
> +path = target;
> +}
> +
> r = unit_file_load(c, info, path, allow_symlink);
I'd prefer if this loop would be moved into a new function
unit_file_load_follow() or so, that is basically little more than a
wrapper around unit_file_load().
Otherwise looks pretty OK.
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] Work on adding polkit support to systemd1
On Fri, 15.08.14 18:25, Stef Walter (st...@redhat.com) wrote: > > On 13.08.2014 20:27, Lennart Poettering wrote: > > On Wed, 06.08.14 13:23, Stef Walter (st...@redhat.com) wrote: > > > >> I've done initial work on adding polkit support to systemd1 DBus > >> methods. You can see it here: > > Thanks for the review. Worked on this a bit more. > > I might drop off the face of the earth for a couple weeks. In case I do, > I thought I'd update my public branch. But if I'm around, I'll test and > prepare a patch set early next week. > > >> https://github.com/stefwalter/systemd/commits/polkit-systemd1 Hmm, yuck. There's a security issue here... Reading the capabilities from the sender on dbus1 is racy, since we have to read it from /proc/$PID/stat and don't get it sent along with the message, like we do on kdbus. A rogue client could send a message, quickly invoke some suid binary, and we'd consider the client trusted. Now for the low-level implementation of the vtable bit we are actually smart, and check by UID on dbus1, and by cap on kdbus, in order to avoid the vulnerability. Hmm, now I wonder how to best handle this for cases like this, we probably need some generic way how clients can make this decision in an always safe way... I need to think more about this... Patch set looks great otherwise. I'll come up with something for the security issue, then adapt your patch, and merge it. Thanks, 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: collapse JOB_RELOAD on an inactive unit into JOB_NOP
On Fri, 2014-08-15 at 12:50 +0200, Lennart Poettering wrote: > On Fri, 15.08.14 05:09, Uoti Urpala (uoti.urp...@pp1.inet.fi) wrote: > > > > > Before this commit systemctl reload on an inactive unit with a queued > > > > start job would block until the unit had started if the unit supported > > > > reload, but return failure immediately if the unit didn't. > > > > > > This sounds like correct behaviour to me. > > > > > > 1) I think the rule should be to return only after the unit is in the > > > desired state, and the desired operation or something equivalent has > > > been executed. This is useful, so that callers can rely on the fact that > > > > What is your "desired state" for reload then? > > *operating* with the new configuration loaded. The problem with this is that it's common for things updating configuration to be separate from things using the daemon. If something changes, the configuration update part wants to guarantee that subsequent requests, *if any*, use the new configuration, but does not itself make any such requests; as such, blocking for the service to be up only causes unnecessary delays and sometimes deadlocks. Ensuring that the service is up belongs to different code paths that actually make requests to the daemon. And they do that whether there's been a reload or not, so they need to handle it regardless of reload behavior anyway. > > > > Additionally systemctl reload-or-try-restart (and systemctl > > > > force-reload) > > > > would block until the unit had started if the unit supported reload, but > > > > return *success* immediately if the unit didn't. > > > > > > This actually also sounds like correct behaviour to me. > > > > > > "try-restart" means that we try to restart a service, but only if it is > > > already running. If we are not running this is not considered an > > > error. If you then add the "reload-or-" to the mix, this means that we > > > will try to reload it if the unit supports it, instead of restarting it. > > > > > > So, if a start is already queued, we'll wait for it to finish, so that > > > the configuration is fully in effect afterwards. But if the thing is not > > > running at all, then we will not consider that a problem at all and > > > return success. > > > > I don't think this makes sense. Reload is a "softer" alternative; if I > > say "reload-or-try-restart", that should be considered as asking less > > from systemd - full restart is not necessary, just reload is enough if > > that option is available. Asking for less should not be a reason to > > block longer! > > "try-restart" also waits for any already queued start job to finish, if > there's any, before it returns, hence "reload-or-try-restart" and > "try-restart" actually block for the same time... Not following here... "try-restart" does *not* wait for a queued start job to finish. job_type_collapse() changes JOB_TRY_RESTART to JOB_NOP if UNIT_IS_INACTIVE_OR_DEACTIVATING(), which is true if the job is just queued and not yet being executed. > > After an operation that changes configuration, > > "reload-or-try-restart" (or just "reload" if we know reload is in fact > > supported by the daemon and sufficient for the configuration change in > > question) should guarantee that further requests use the new > > configuration. It should not unnecessarily block until the daemon is > > actually running if it's being started; anything which relies on it > > being up should test for that separately, independently of configuration > > change handling. And there's no reason why implementing lighter-weight > > reloads to replace full restarts should lead to systemd blocking > > longer. > > No. It *should* wait until any start request that is already queued is > complete... Again, it's about the *outcome*, and about being *positive*, > really. Nothing else. We should not make things fail, if we already > *know* that there's alerady something going on that makes the thing we > are trying to do succeed. >From the configuration change point of view, the outcome *is* positive as soon as it's guaranteed that no further requests will be processed with the old configuration. So systemd should return *success* at that point. This does not "make things fail"; rather, it prevents failure from deadlocks caused by unnecessary blocking. > I think most of the confusion here comes from the fact that sysv service > restarts don't care about ordering at all, really, and we do. But the No, my objections to the current behavior are not related to sysv semantics at all. > answer to that is not to weaken the current strong semantics of > blocking, but simply not to request blocking operation at all, i.e. use > --no-block, and just queue things in, instead of waiting for them. If you want to block for things other than the configuration change itself, such as the service being up, that should be separate from changing configuration. > > > > Finaly reload on an inactive unit without a queued start job wou
Re: [systemd-devel] Work on adding polkit support to systemd1
On 15.08.2014 18:56, Lennart Poettering wrote: > On Fri, 15.08.14 18:25, Stef Walter (st...@redhat.com) wrote: > >> >> On 13.08.2014 20:27, Lennart Poettering wrote: >>> On Wed, 06.08.14 13:23, Stef Walter (st...@redhat.com) wrote: >>> I've done initial work on adding polkit support to systemd1 DBus methods. You can see it here: >> >> Thanks for the review. Worked on this a bit more. >> >> I might drop off the face of the earth for a couple weeks. In case I do, >> I thought I'd update my public branch. But if I'm around, I'll test and >> prepare a patch set early next week. >> https://github.com/stefwalter/systemd/commits/polkit-systemd1 > > Hmm, yuck. There's a security issue here... Reading the capabilities > from the sender on dbus1 is racy, since we have to read it from > /proc/$PID/stat and don't get it sent along with the message, like we do > on kdbus. A rogue client could send a message, quickly invoke some suid > binary, and we'd consider the client trusted. > > Now for the low-level implementation of the vtable bit we are actually > smart, and check by UID on dbus1, and by cap on kdbus, in order to avoid > the vulnerability. > > Hmm, now I wonder how to best handle this for cases like this, we > probably need some generic way how clients can make this decision in an > always safe way... > > I need to think more about this... By the way, there's some similar problematic code in the modified KillUnit() method implementation ... changed from specifying the CAP_KILL in the vtable, and now it does a manual check. > Patch set looks great otherwise. I'll come up with something for the > security issue, then adapt your patch, and merge it. I haven't tested the updated branch at all :) So it may go boom... Stef ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] bootchart: don't parse /proc/uptime, use CLOCK_BOOTTIME
thumbs up from me, thanks for sending this.
Auke
On Thu, Jul 31, 2014 at 1:15 AM, Karel Zak wrote:
> * systemd-bootchart always parses /proc/uptime, although the
> information is unnecessary when --rel specified
>
> * use /proc/uptime is overkill, since Linux 2.6.39 we have
> clock_gettime(CLOCK_BOOTTIME, ...). The backend on kernel side is
> get_monotonic_boottime() in both cases.
>
> * main() uses "if (graph_start <= 0.0)" to detect that /proc is
> available.
>
> This is fragile solution as graph_start is always smaller than zero
> on all systems after suspend/resume (e.g. laptops), because in this
> case the system uptime includes suspend time and uptime is always
> greater number than monotonic time. For example right now difference
> between uptime and monotonic time is 37 hours on my laptop.
>
> Note that main() calls log_uptime() (to parse /proc/uptime) for each
> sample when it believes that /proc is not available. So on my laptop
> systemd-boochars spends all live with /proc/uptime parsing +
> nanosleep(), try
>
> strace /usr/lib/systemd/systemd-bootchart
>
> to see the never ending loop.
>
> This patch uses access("/proc/vmstat", F_OK) to detect procfs.
> ---
> man/systemd-bootchart.xml | 4 +++-
> src/bootchart/bootchart.c | 11 +++
> src/bootchart/store.c | 29 -
> 3 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/man/systemd-bootchart.xml b/man/systemd-bootchart.xml
> index e19bbc1..150ca48 100644
> --- a/man/systemd-bootchart.xml
> +++ b/man/systemd-bootchart.xml
> @@ -131,7 +131,9 @@
> not graph the time elapsed since boot
> and before systemd-bootchart was
> started, as it may result in extremely
> -large graphs.
> +large graphs. The time elapsed since boot
> +might also include any time that the
> system
> +was suspended.
>
>
>
> diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
> index cbfc28d..909ef46 100644
> --- a/src/bootchart/bootchart.c
> +++ b/src/bootchart/bootchart.c
> @@ -310,6 +310,7 @@ int main(int argc, char *argv[]) {
> time_t t = 0;
> int r;
> struct rlimit rlim;
> +bool has_procfs = false;
>
> parse_conf();
>
> @@ -349,6 +350,8 @@ int main(int argc, char *argv[]) {
>
> log_uptime();
>
> +has_procfs = access("/proc/vmstat", F_OK) == 0;
> +
> LIST_HEAD_INIT(head);
>
> /* main program loop */
> @@ -385,11 +388,11 @@ int main(int argc, char *argv[]) {
> parse_env_file("/usr/lib/os-release",
> NEWLINE, "PRETTY_NAME", &build, NULL);
> }
>
> -/* wait for /proc to become available, discarding samples
> */
> -if (graph_start <= 0.0)
> -log_uptime();
> -else
> +if (has_procfs)
> log_sample(samples, &sampledata);
> +else
> +/* wait for /proc to become available, discarding
> samples */
> +has_procfs = access("/proc/vmstat", F_OK) == 0;
>
> sample_stop = gettime_ns();
>
> diff --git a/src/bootchart/store.c b/src/bootchart/store.c
> index e071983..cedcba8 100644
> --- a/src/bootchart/store.c
> +++ b/src/bootchart/store.c
> @@ -57,27 +57,22 @@ double gettime_ns(void) {
> return (n.tv_sec + (n.tv_nsec / 10.0));
> }
>
> -void log_uptime(void) {
> -_cleanup_fclose_ FILE *f = NULL;
> -char str[32];
> -double uptime;
> -
> -f = fopen("/proc/uptime", "re");
> -
> -if (!f)
> -return;
> -if (!fscanf(f, "%s %*s", str))
> -return;
> -
> -uptime = strtod(str, NULL);
> +static double gettime_up(void) {
> +struct timespec n;
>
> -log_start = gettime_ns();
> +clock_gettime(CLOCK_BOOTTIME, &n);
> +return (n.tv_sec + (n.tv_nsec / 10.0));
> +}
>
> -/* start graph at kernel boot time */
> +void log_uptime(void) {
> if (arg_relative)
> -graph_start = log_start;
> -else
> +graph_start = log_start = gettime_ns();
> +else {
> +double uptime = gettime_up();
> +
> +log_start = gettime_ns();
> graph_start = log_start - uptime;
> +}
> }
>
> static char *bufgetline(char *buf) {
> --
> 1.9.3
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
___
systemd-devel mailing
Re: [systemd-devel] [PATCH 2/2] bootchart: ask for --rel when failed to initialize graph start time
thanks!
Auke
On Sat, Aug 2, 2014 at 10:17 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:
> On Thu, Jul 31, 2014 at 10:15:40AM +0200, Karel Zak wrote:
> > This patch uses access("/proc/vmstat", F_OK) to detect procfs.
>
> > We always read system uptime before log start time. So the uptime
> > should be always smaller number, except it includes system suspend
> > time. It seems better to ask for --rel and exit() than try to be
> > smart and try to recovery from this situation or generate huge
> > messy graphs.
> Applied both.
>
> Zbyszek
> ___
> 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] [HEADS-UP] Intent to remove readahead from systemd
On Thu, Aug 14, 2014 at 10:16 AM, Lennart Poettering wrote: > Heya, > > Since its early days systemd contained the systemd-readahead tool, whose > job was to improve boot times by reading files in their order on disk, > before they would actually be needed by applications. In times of SSD > the benefit of systemd-readahead is much less convincing, in many case > it actually slows things down. > > The fact is now that nobody really cares about systemd-readahead much > anymore. Nobody in the systemd team still works on a laptop with > rotating media, hence nobody tries to optimize it in any way. And it > probably needs a lot of looking after and love to still be useful as > general purpose systems, instead of just slowing them down... > > So, I think with the release after the upcoming one we should just > remove it from the systemd package and just throw it on the pile of > historic cruft. So, yeah, here's the advance warning that this will be > happening... > > (Well, unless somebody from the community who cares and wants to invest > the necessary time in it steps up and gives it the love it really > needs. If nobody does until that release, I will delete the component > from systemd). > > I fully understand that not everybody uses SSDs yet, and also that > theoretically doign systemd-readahead on SSD could be beneficial still > (since RAM is still orders of magnitude faster than SSDs), but it's > really not about that, it's about maintainership and giving the tool the > love it needs. > > heh, ouch X_X I can understand your sentiment, though. I've identified plenty of cases where readahead just isn't working out well at all, and the constant tweaking has left it ... quite a bit messy. Auke ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: collapse JOB_RELOAD on an inactive unit into JOB_NOP
В Fri, 15 Aug 2014 20:25:57 +0300 Uoti Urpala пишет: > > > > > > What is your "desired state" for reload then? > > > > *operating* with the new configuration loaded. > > The problem with this is that it's common for things updating > configuration to be separate from things using the daemon. If something > changes, the configuration update part wants to guarantee that > subsequent requests, *if any*, use the new configuration, but does not > itself make any such requests; as such, blocking for the service to be > up only causes unnecessary delays and sometimes deadlocks. Ensuring that > the service is up belongs to different code paths that actually make > requests to the daemon. And they do that whether there's been a reload > or not, so they need to handle it regardless of reload behavior anyway. > It's not how I interpret "reload" and how "reload" was traditionally implemented by initscripts. "reload" means - request daemon to do whatever is necessary to start using new configuration. It never implied changing this configuration. This happens outside of scope of performing "reload" action. You seem to interpret "reload" as request to update static on-disk configuration of service. Am I right? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: collapse JOB_RELOAD on an inactive unit into JOB_NOP
On Fri, 2014-08-15 at 22:22 +0400, Andrei Borzenkov wrote: > В Fri, 15 Aug 2014 20:25:57 +0300 > Uoti Urpala пишет: > > The problem with this is that it's common for things updating > > configuration to be separate from things using the daemon. If something > > changes, the configuration update part wants to guarantee that > > subsequent requests, *if any*, use the new configuration, but does not > > itself make any such requests; as such, blocking for the service to be > > up only causes unnecessary delays and sometimes deadlocks. Ensuring that > > the service is up belongs to different code paths that actually make > > requests to the daemon. And they do that whether there's been a reload > > or not, so they need to handle it regardless of reload behavior anyway. > > > > It's not how I interpret "reload" and how "reload" was traditionally > implemented by initscripts. "reload" means - request daemon to do > whatever is necessary to start using new configuration. It never > implied changing this configuration. This happens outside of scope of > performing "reload" action. You seem to interpret "reload" as request to > update static on-disk configuration of service. Am I right? No, I didn't say anything about "systemctl reload" itself modifying on-disk configuration (if that's really what your "request to update static on-disk-configuration" meant). The basic difference in desired semantics seems to be: me: "reload" should ensure that system has switched to the new configuration. No other semantics, just that any configuration that is used after "reload" has returned is the new one. Lennart: "reload" should ensure the system has switched to the new configuration, *and* should also wait to ensure that the daemon is up and is currently responding to requests with the new configuration if possible. The latter semantics cause problems for any generic state change code which writes new configuration for a service, runs "systemctl reload", and then informs the caller that state was successfully changed. Changing configuration does not imply that you want the daemon to be ready to handle requests! In case this is still not clear, consider this division of code: 1) Event hook which runs in response to some external changes or admin requests. Writes new configuration for the daemon, and then runs "systemctl reload foo.service". Does not use --no-block, because it should be guaranteed that the new configuration is in effect before the hook returns. Does not itself make any requests to the daemon. 2) Code elsewhere that actually makes requests to the daemon. Code 1 can run early during the boot before the service itself starts. If "reload" blocks until the queued start of the daemon is executed, this causes a deadlock: the hook waits for the daemon to start, but boot can not progress to the point where the daemon starts because the part running the hook is blocked in systemctl. Having reload block until a starting service is really be up does not have any positive effect: code 2 has to depend on other ways ensure that the daemon is up before making requests anyway, because it can not assume that the reload hook has necessarily been triggered at any prior point (and even if it could make such an assumption, relying on that would seem like quite a hacky design - there are much better ways to ensure daemons you require are up). ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: collapse JOB_RELOAD on an inactive unit into JOB_NOP
2014-08-15 12:50 GMT+02:00 Lennart Poettering : > I think most of the confusion here comes from the fact that sysv service > restarts don't care about ordering at all, really, and we do. But the > answer to that is not to weaken the current strong semantics of > blocking, but simply not to request blocking operation at all, i.e. use > --no-block, and just queue things in, instead of waiting for them. > > Note that on FEdora the sysv /sbin/service glue actually adds in > --no-block for many cases, too, due to the stricter requirements of > systemd there. I just had a look at /sbin/service and/etc/init.d/functions as shipped by F20 and couldn't find any traces of --no-block. I'd be interested to know under what conditions you add --no-block. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(&conn->msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; mutex_unlock(&conn->lock); return mask; -- 1.9.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] bootchart: don't parse /proc/uptime, use CLOCK_BOOTTIME
2014-07-31 10:15 GMT+02:00 Karel Zak :
Hi,
> * systemd-bootchart always parses /proc/uptime, although the
> information is unnecessary when --rel specified
>
> * use /proc/uptime is overkill, since Linux 2.6.39 we have
> clock_gettime(CLOCK_BOOTTIME, ...). The backend on kernel side is
> get_monotonic_boottime() in both cases.
>
> * main() uses "if (graph_start <= 0.0)" to detect that /proc is
> available.
>
> This is fragile solution as graph_start is always smaller than zero
> on all systems after suspend/resume (e.g. laptops), because in this
> case the system uptime includes suspend time and uptime is always
> greater number than monotonic time. For example right now difference
> between uptime and monotonic time is 37 hours on my laptop.
>
> Note that main() calls log_uptime() (to parse /proc/uptime) for each
> sample when it believes that /proc is not available. So on my laptop
> systemd-boochars spends all live with /proc/uptime parsing +
> nanosleep(), try
>
> strace /usr/lib/systemd/systemd-bootchart
>
> to see the never ending loop.
>
> This patch uses access("/proc/vmstat", F_OK) to detect procfs.
> ---
> man/systemd-bootchart.xml | 4 +++-
> src/bootchart/bootchart.c | 11 +++
> src/bootchart/store.c | 29 -
> 3 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/man/systemd-bootchart.xml b/man/systemd-bootchart.xml
> index e19bbc1..150ca48 100644
> --- a/man/systemd-bootchart.xml
> +++ b/man/systemd-bootchart.xml
> @@ -131,7 +131,9 @@
> not graph the time elapsed since boot
> and before systemd-bootchart was
> started, as it may result in extremely
> -large graphs.
> +large graphs. The time elapsed since boot
> +might also include any time that the system
> +was suspended.
>
>
>
> diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
> index cbfc28d..909ef46 100644
> --- a/src/bootchart/bootchart.c
> +++ b/src/bootchart/bootchart.c
> @@ -310,6 +310,7 @@ int main(int argc, char *argv[]) {
> time_t t = 0;
> int r;
> struct rlimit rlim;
> +bool has_procfs = false;
>
> parse_conf();
>
> @@ -349,6 +350,8 @@ int main(int argc, char *argv[]) {
>
> log_uptime();
>
> +has_procfs = access("/proc/vmstat", F_OK) == 0;
> +
> LIST_HEAD_INIT(head);
>
> /* main program loop */
> @@ -385,11 +388,11 @@ int main(int argc, char *argv[]) {
> parse_env_file("/usr/lib/os-release",
> NEWLINE, "PRETTY_NAME", &build, NULL);
> }
>
> -/* wait for /proc to become available, discarding samples */
> -if (graph_start <= 0.0)
> -log_uptime();
> -else
> +if (has_procfs)
> log_sample(samples, &sampledata);
> +else
> +/* wait for /proc to become available, discarding
> samples */
> +has_procfs = access("/proc/vmstat", F_OK) == 0;
>
> sample_stop = gettime_ns();
>
> diff --git a/src/bootchart/store.c b/src/bootchart/store.c
> index e071983..cedcba8 100644
> --- a/src/bootchart/store.c
> +++ b/src/bootchart/store.c
> @@ -57,27 +57,22 @@ double gettime_ns(void) {
> return (n.tv_sec + (n.tv_nsec / 10.0));
> }
>
> -void log_uptime(void) {
> -_cleanup_fclose_ FILE *f = NULL;
> -char str[32];
> -double uptime;
> -
> -f = fopen("/proc/uptime", "re");
> -
> -if (!f)
> -return;
> -if (!fscanf(f, "%s %*s", str))
> -return;
> -
> -uptime = strtod(str, NULL);
> +static double gettime_up(void) {
> +struct timespec n;
>
> -log_start = gettime_ns();
> +clock_gettime(CLOCK_BOOTTIME, &n);
> +return (n.tv_sec + (n.tv_nsec / 10.0));
You can use NSEC_PER_SEC from src/shared/time-util.h instead of 10.0.
> +}
>
> -/* start graph at kernel boot time */
> +void log_uptime(void) {
> if (arg_relative)
> -graph_start = log_start;
> -else
> +graph_start = log_start = gettime_ns();
> +else {
> +double uptime = gettime_up();
> +
> +log_start = gettime_ns();
> graph_start = log_start - uptime;
> +}
> }
>
> static char *bufgetline(char *buf) {
> --
> 1.9.3
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd
Re: [systemd-devel] [PATCH 1/2] bootchart: don't parse /proc/uptime, use CLOCK_BOOTTIME
2014-08-15 22:02 GMT+02:00 Ronny Chevalier :
> 2014-07-31 10:15 GMT+02:00 Karel Zak :
> Hi,
>
>> * systemd-bootchart always parses /proc/uptime, although the
>> information is unnecessary when --rel specified
>>
>> * use /proc/uptime is overkill, since Linux 2.6.39 we have
>> clock_gettime(CLOCK_BOOTTIME, ...). The backend on kernel side is
>> get_monotonic_boottime() in both cases.
>>
>> * main() uses "if (graph_start <= 0.0)" to detect that /proc is
>> available.
>>
>> This is fragile solution as graph_start is always smaller than zero
>> on all systems after suspend/resume (e.g. laptops), because in this
>> case the system uptime includes suspend time and uptime is always
>> greater number than monotonic time. For example right now difference
>> between uptime and monotonic time is 37 hours on my laptop.
>>
>> Note that main() calls log_uptime() (to parse /proc/uptime) for each
>> sample when it believes that /proc is not available. So on my laptop
>> systemd-boochars spends all live with /proc/uptime parsing +
>> nanosleep(), try
>>
>> strace /usr/lib/systemd/systemd-bootchart
>>
>> to see the never ending loop.
>>
>> This patch uses access("/proc/vmstat", F_OK) to detect procfs.
>> ---
>> man/systemd-bootchart.xml | 4 +++-
>> src/bootchart/bootchart.c | 11 +++
>> src/bootchart/store.c | 29 -
>> 3 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/man/systemd-bootchart.xml b/man/systemd-bootchart.xml
>> index e19bbc1..150ca48 100644
>> --- a/man/systemd-bootchart.xml
>> +++ b/man/systemd-bootchart.xml
>> @@ -131,7 +131,9 @@
>> not graph the time elapsed since boot
>> and before systemd-bootchart was
>> started, as it may result in extremely
>> -large graphs.
>> +large graphs. The time elapsed since boot
>> +might also include any time that the system
>> +was suspended.
>>
>>
>>
>> diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
>> index cbfc28d..909ef46 100644
>> --- a/src/bootchart/bootchart.c
>> +++ b/src/bootchart/bootchart.c
>> @@ -310,6 +310,7 @@ int main(int argc, char *argv[]) {
>> time_t t = 0;
>> int r;
>> struct rlimit rlim;
>> +bool has_procfs = false;
>>
>> parse_conf();
>>
>> @@ -349,6 +350,8 @@ int main(int argc, char *argv[]) {
>>
>> log_uptime();
>>
>> +has_procfs = access("/proc/vmstat", F_OK) == 0;
>> +
>> LIST_HEAD_INIT(head);
>>
>> /* main program loop */
>> @@ -385,11 +388,11 @@ int main(int argc, char *argv[]) {
>> parse_env_file("/usr/lib/os-release",
>> NEWLINE, "PRETTY_NAME", &build, NULL);
>> }
>>
>> -/* wait for /proc to become available, discarding samples */
>> -if (graph_start <= 0.0)
>> -log_uptime();
>> -else
>> +if (has_procfs)
>> log_sample(samples, &sampledata);
>> +else
>> +/* wait for /proc to become available, discarding
>> samples */
>> +has_procfs = access("/proc/vmstat", F_OK) == 0;
>>
>> sample_stop = gettime_ns();
>>
>> diff --git a/src/bootchart/store.c b/src/bootchart/store.c
>> index e071983..cedcba8 100644
>> --- a/src/bootchart/store.c
>> +++ b/src/bootchart/store.c
>> @@ -57,27 +57,22 @@ double gettime_ns(void) {
>> return (n.tv_sec + (n.tv_nsec / 10.0));
>> }
>>
>> -void log_uptime(void) {
>> -_cleanup_fclose_ FILE *f = NULL;
>> -char str[32];
>> -double uptime;
>> -
>> -f = fopen("/proc/uptime", "re");
>> -
>> -if (!f)
>> -return;
>> -if (!fscanf(f, "%s %*s", str))
>> -return;
>> -
>> -uptime = strtod(str, NULL);
>> +static double gettime_up(void) {
>> +struct timespec n;
>>
>> -log_start = gettime_ns();
>> +clock_gettime(CLOCK_BOOTTIME, &n);
>> +return (n.tv_sec + (n.tv_nsec / 10.0));
> You can use NSEC_PER_SEC from src/shared/time-util.h instead of 10.0.
>
Oops it has already been applied, sorry.
>> +}
>>
>> -/* start graph at kernel boot time */
>> +void log_uptime(void) {
>> if (arg_relative)
>> -graph_start = log_start;
>> -else
>> +graph_start = log_start = gettime_ns();
>> +else {
>> +double uptime = gettime_up();
>> +
>> +log_start = gettime_ns();
>> graph_start = log_start - uptime;
>> +}
>> }
>>
>> static char *bufgetline(char *buf) {
>> --
>> 1.9.3
>>
>> ___
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi, On 08/15/2014 09:43 PM, Marcel Holtmann wrote: > To facility the feature of doing an asynchronous sending of messages > when the bus is idle, make sure to return POLLOUT | POLLWRNORM from > kdbus_handle_poll. > > Signed-off-by: Marcel Holtmann > --- > handle.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/handle.c b/handle.c > index ac6868133280..fc15d28351b3 100644 > --- a/handle.c > +++ b/handle.c > @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, > mask |= POLLERR | POLLHUP; > else if (!list_empty(&conn->msg_list)) > mask |= POLLIN | POLLRDNORM; > + else > + mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(&conn->msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... Anyway, I don't see a problem in that change, so I've applied it now. Thanks! Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi Daniel, >> To facility the feature of doing an asynchronous sending of messages >> when the bus is idle, make sure to return POLLOUT | POLLWRNORM from >> kdbus_handle_poll. >> >> Signed-off-by: Marcel Holtmann >> --- >> handle.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/handle.c b/handle.c >> index ac6868133280..fc15d28351b3 100644 >> --- a/handle.c >> +++ b/handle.c >> @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, >> mask |= POLLERR | POLLHUP; >> else if (!list_empty(&conn->msg_list)) >> mask |= POLLIN | POLLRDNORM; >> +else >> +mask |= POLLOUT | POLLWRNORM; > > Hmm, what's your use case here? list_empty(&conn->msg_list) only checks > the incoming list of the current connection for pending messages. That > doesn't tell you whether the bus is idle, or if your receiving end has > messages pending ... if you are a good client citizen, then you only send messages when POLLOUT gets signaled. That is what this is allowing now. Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Regards Marcel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Implementing resume from hibernation as a systemd unit file
On Friday 15 August 2014 at 12:19:50, Lennart Poettering wrote: > [...] > > > > > > > > I'd like to make this work both with initramfs and without one > > > > (provided that > > > > the rootfs has been mounted read-only by using 'ro' kernel cmdline > > > > parameter). > > > > > > > > In this case, what are the needed orderings? > > > > > > Actually systemd-remount-fs.service uses After=local-fs-pre.target > > > anyway, so ordering before l-f-p.t should be nough. > > > > Hm. In git (v215-651-g41488fe), it is > > > > Before=local-fs-pre.target local-fs.target shutdown.target > > Wants=local-fs-pre.target > > Ah, right. This is actually correct here. We want to make sure that the > root fs is remounted before we mount the other units, since this might > required creating additional directories to mount things on... > > There are two more complications: > > a) if you want to make use of local-fs-pre.target you actually have to >pull it in, it's a "passive" unit. See systemd.special(5). > > b) You want to run your stuff before fsck is run on the devices, so that >you don't end up interrupting an fsck that is half in progress. > > To put this together, in your unit file you need: > > Before=local-fs-pre.target systemd-remount-fs.service > systemd-fsck-root.service > Wants=local-fs-pre.target > > That should be enough. (You don't need to individually order the > systemd-fsck@.service instances for the other devices after your > service, since they are already ordered after systemd-fsck-root.service, > and you order yourself before that, so all is good). > > Lennart One more question. What about setups with no initrd and read-write rootfs? In such cases, the resume unit must silently skip itself. ConditionPathIsReadWrite=!/ doesn't seem to be useful here: with initramfs this check will yield a false-negative. This can be solved by introducing two resume units (say, systemd-resume@.service and initrd-resume@.service), first with Before=local-fs-pre.target systemd-remount-fs.service systemd-fsck-root.service Wants=local-fs-pre.target ConditionPathIsReadWrite=!/ and the second one with ConditionPathExists=/etc/initrd-release # something else ? BTW... are you sure that the second variant (in initramfs) does not require something to order before sysroot.mount and all fsck units?.. -- Ivan Shapovalov / intelfx / signature.asc Description: This is a digitally signed message part. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
