[systemd-devel] fstab, rootfs on btrfs

2013-11-26 Thread Chris Murphy

In Fedora 20, by default anaconda sets fs_passno in fstab to 1 for / on btrfs. 
During offline updates, this is causing systemd-fstab-generator to freak out 
not finding fsck.btrfs.

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

For some time I've been suggesting that fstab should use fs_passno 0 for btrfs 
file systems:
https://bugzilla.redhat.com/show_bug.cgi?id=862871

But because of this suggestion by an XFS dev, I'm wondering if that's not a 
good idea. Or if we should expect some smarter behavior on the part of systemd 
(now or in the future) when it comes to devices that take a long time to appear?
http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg29231.html

It doesn't seem to me that for file systems that don't require an fs check, 
that fstab should indicate it does require an fs check, just to inhibit hissy 
fits by other processes not liking that the device is missing. But maybe I'm 
missing something.


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


Re: [systemd-devel] [systemd-commits] man/systemd-socket-proxyd.xml src/socket-proxy TODO

2013-11-26 Thread David Timothy Strauss
On Wed, Nov 27, 2013 at 12:03 PM, Lennart Poettering
 wrote:
> Could you please explain what the usecase here is? Why is this better
> than having two socket units with two proxy services?

Right now, it's because separate services cannot exist in the same
network namespace with another service. The main motivation for this
change was a desire within CoreOS to run etcd within a
network-namespaced service. With the need to only map one port in,
that would be equivalent to the network namespaced nginx example. But,
they need to map *two* ports in and to distinct destination ports
within the namespace. That was actually impossible before, where
socket-proxyd would map all inherited sockets to the same destination.

This change allows forcing a single socket-proxyd to inherit only one
socket each.

>> +/usr/bin/systemd-socket-proxyd --listener=3 localhost:8080 &
>> +/usr/bin/systemd-socket-proxyd --listener=4 localhost:8443 &
>> +wait]]>
>
> If our examples suggest that people should write shell scripts that fork
> things, then this is usually an indication that we need to rethink what
> we are doing here. In fact, running multiple manually forked off
> processes inside the same service already sounds very wrong. We should
> try to keep a 1:1 mapping between processes we fork and services they
> run it.

Agreed. I really don't want socket-proxyd directly involved in
exec'ing of things, either.

> What's the usecase here?
>
> If this is about running multiple things in the same Network namespace,
> then there are certainly other, better ways to achieve the same. For
> example, we could introduce JoinPrivateNetwork=$SERVICE or so which
> would allow one service to join the network namespace of another. That
> makes this much smoother and more powerful, too.

That would be wonderful. I'd love to shed the shell scripts.

> With that in place you could simply have three proxy instances, and make
> them join the private network of your nginx instance and you should be
> set?

I think that would do exactly the trick. I didn't know it was possible
to join into an existing namespace that way.

It would be my pleasure to update the documentation to use something
like that if it becomes available. I'd also enjoy dropping the
--listener option. :-)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-26 Thread David Timothy Strauss
On Wed, Nov 27, 2013 at 7:57 AM, Shawn Landden  wrote:
> Are you sure applications can handle the extra file descriptor of
> passing both the sockfd
> and the acceptfd in this case? I don't see why they wouldn't just do
> the accept() themselves?
>
> Can you explain what you mean here, and how it differs from the
> existing MaxConnections.

Actually, MaxConnections was exactly what I was thinking of. I agree
with you now on Distribute=true only being useful with Accept=false.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] man/systemd-socket-proxyd.xml src/socket-proxy TODO

2013-11-26 Thread Lennart Poettering
On Sun, 24.11.13 16:47, David Strauss (strau...@kemper.freedesktop.org) wrote:

>  TODO |1 
>  man/systemd-socket-proxyd.xml|   80 
> ++-
>  src/socket-proxy/socket-proxyd.c |   63 --
>  3 files changed, 121 insertions(+), 23 deletions(-)
> 
> New commits:
> commit adcf4c81c58511b67644e17fa743d1729d3c9ccf
> Author: David Strauss 
> Date:   Mon Nov 25 10:44:48 2013 +1000
> 
> socket-proxyd: Add --listener option for listener/destination
> pairs.

Could you please explain what the usecase here is? Why is this better
than having two socket units with two proxy services?

> +/usr/bin/systemd-socket-proxyd --listener=3 localhost:8080 &
> +/usr/bin/systemd-socket-proxyd --listener=4 localhost:8443 &
> +wait]]>

If our examples suggest that people should write shell scripts that fork
things, then this is usually an indication that we need to rethink what
we are doing here. In fact, running multiple manually forked off
processes inside the same service already sounds very wrong. We should
try to keep a 1:1 mapping between processes we fork and services they
run it.

What's the usecase here? 

If this is about running multiple things in the same Network namespace,
then there are certainly other, better ways to achieve the same. For
example, we could introduce JoinPrivateNetwork=$SERVICE or so which
would allow one service to join the network namespace of another. That
makes this much smoother and more powerful, too. 

With that in place you could simply have three proxy instances, and make
them join the private network of your nginx instance and you should be
set?


Lennart

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


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

2013-11-26 Thread Lennart Poettering
On Wed, 27.11.13 07:48, David Timothy Strauss (da...@davidstrauss.net) wrote:

> 
> On Wed, Nov 27, 2013 at 6:23 AM, Shawn Landden  wrote:
> > I was worried that the fact that we never accept() the socket when using
> > distribute (now I am convinced we shouldn't use it otherwise)
> 
> I'm not sure what you mean here. Distribute-style functionality is
> absolutely useful with Accept=true (to cap the number of simultaneous
> connections) as well as Accept=false (to allow running of a process
> pool of self-accepting, long-running workers).

We already enforce a connection limit (MaxConnections=) for Accept=yes
sockets... We did this since day one.

Lennart

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


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

2013-11-26 Thread Shawn Landden
On Tue, Nov 26, 2013 at 1:48 PM, David Timothy Strauss
 wrote:
> On Wed, Nov 27, 2013 at 6:23 AM, Shawn Landden  wrote:
>> I was worried that the fact that we never accept() the socket when using
>> distribute (now I am convinced we shouldn't use it otherwise)
>
> I'm not sure what you mean here. Distribute-style functionality is
> absolutely useful with Accept=true (to cap the number of simultaneous
> connections)
Are you sure applications can handle the extra file descriptor of
passing both the sockfd
and the acceptfd in this case? I don't see why they wouldn't just do
the accept() themselves?

Can you explain what you mean here, and how it differs from the
existing MaxConnections.

> as well as Accept=false (to allow running of a process
> pool of self-accepting, long-running workers).
>
>> would cause it to trigger multiple times for only one incoming connection,
>> if the spawned thread never entered accept() (or we raced it),
>> but reading this thread makes me think I don't fully understand the
>> semantics of EPOLLET.
>
> There are some decent examples on the man page: 
> http://linux.die.net/man/7/epoll
Yeah I had read that, and seen it in the kernel source. I am just
confused and need to think about it some more.

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


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

2013-11-26 Thread David Timothy Strauss
On Wed, Nov 27, 2013 at 6:23 AM, Shawn Landden  wrote:
> I was worried that the fact that we never accept() the socket when using
> distribute (now I am convinced we shouldn't use it otherwise)

I'm not sure what you mean here. Distribute-style functionality is
absolutely useful with Accept=true (to cap the number of simultaneous
connections) as well as Accept=false (to allow running of a process
pool of self-accepting, long-running workers).

> would cause it to trigger multiple times for only one incoming connection,
> if the spawned thread never entered accept() (or we raced it),
> but reading this thread makes me think I don't fully understand the
> semantics of EPOLLET.

There are some decent examples on the man page: http://linux.die.net/man/7/epoll
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-26 Thread Shawn Landden
As we only reciece one event at a time, and dequeue it in the same go,
yeah ONESHOT won't change anything.

On Tue, Nov 26, 2013 at 9:33 AM, David Timothy Strauss
 wrote:
> On Wed, Nov 27, 2013 at 2:32 AM, Lennart Poettering
>  wrote:
>> Well, but EPOLLET only works correctly if each time an event is
>> triggered we dispatch *all* possibly queued events on the fd, until
>> EAGAIN is read again. But we don't do that, heck, if Listen=no is used
>> we don''t even read a single incoming connection of the scket, we leave
>> that to the daemon we spawn, but we cannot trust that. So, I don't get
>> what EPOLLET is supposed to do good here?
>
> I should have more selectively quoted before answering. I was only
> trying to answer why we'd continue having poll for the socket fds
> after queuing the initial service. I was not trying to suggest EPOLLET
> is appropriate for the goal of lazy "distribute" process pool
> expansion. You've clearly correct about that.
I was worried that the fact that we never accept() the socket when using
distribute (now I am convinced we shouldn't use it otherwise)
would cause it to trigger multiple times for only one incoming connection,
if the spawned thread never entered accept() (or we raced it),
but reading this thread makes me think I don't fully understand the
semantics of EPOLLET.
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-26 Thread David Timothy Strauss
On Wed, Nov 27, 2013 at 2:32 AM, Lennart Poettering
 wrote:
> Well, but EPOLLET only works correctly if each time an event is
> triggered we dispatch *all* possibly queued events on the fd, until
> EAGAIN is read again. But we don't do that, heck, if Listen=no is used
> we don''t even read a single incoming connection of the scket, we leave
> that to the daemon we spawn, but we cannot trust that. So, I don't get
> what EPOLLET is supposed to do good here?

I should have more selectively quoted before answering. I was only
trying to answer why we'd continue having poll for the socket fds
after queuing the initial service. I was not trying to suggest EPOLLET
is appropriate for the goal of lazy "distribute" process pool
expansion. You've clearly correct about that.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC][PATCH] conf-parser: distinguish between multiple sections with the same name

2013-11-26 Thread Lennart Poettering
On Thu, 21.11.13 02:27, Tom Gundersen (t...@jklm.no) wrote:

> > Maybe then back to labelled sections:
> >
> > [Address:foobar]
> > Label=waldo
> > Address=1.1.1.1
> >
> > or so, so that the suffix "foobar" is purely an id that is by default
> > disconnected from any setting? And then maybe optionally inherit it into
> > Label= if Label= is not explicitly set?
> 
> I think the section name should not have any other function/meaning. I
> first tried to make 'section name'='Label', but I'm worried it might
> be confusing: If section names are required, it means we now require
> labeling, or force the admin to set "Label=" to disable it, which
> seems a bit weird. 

It hink that would actually be OK. Doing something useful without
explicit configuration sounds like a good approach. Inheriting the
Label= from the section name if no explicit Label= is specified sounds
quite OK to me...

> Also, I find it asymmetric the way section names
> for Addresses have this extra meaning, but for Routes or other types
> of sections where there is no natural equivalent to Label=, they have
> no meaning apart from a name.

Well, we wouldn't really bind the section name only and exclusively to
the label. It's just that when fields are not explicitly set they might
inherit the section name, in some field-specific way. For Label= it
would be quiet obvious, but for other things this might work too. For
example, for the ipv6 tunnel stuff we could inherit the section name
into the tunnel iface name or so, whatever comes up...

> I'm really not convinced one way or the other, but I think my
> preferred solution is: go with unnamed sections now, and if ever it
> becomes necessary, introduce named ones additionally.

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

2013-11-26 Thread Lennart Poettering
On Tue, 26.11.13 09:53, Colin Guthrie (gm...@colin.guthr.ie) wrote:

> >> I'm proposing a simple goal: XDG_RUNTIME_DIR should always be that
> >> matching the current uid.  I can't think of any case where you'd
> >> want it otherwise.
> > 
> > That can't work. As the directory only exists when a real login session
> > is around. su/sudo don't get their own login sessins, hence the dir
> > doesn't necessarily exist and from the perspective of the code running
> > in su/sudo the lifetime semantics of the dir wouldn't match any
> > expections...
> 
> Colin W's later patch did implement these semantics for the root user's
> XDG_RUNTIME_DIR. It kept it around and didn't tidy it up. Doesn't this
> solve the problem for the root user nicely (which is the primary
> problem)?

I am pretty sure we shouldn't exclude the root user's XDG_RUNTIME_DIR
from the usual clean-up logic.

Also, as mentioned, if you want su-l/sudo-i work, then please add the
"force-new-session=1" switch to pam_systemd, that I offered to merge.

> But regardless, why doesn't this code create the dirs if they don't
> exist anyway? Sure, this doesn't deal with lifetime problem (i.e.
> knowing when to delete the dirs again) which is highlighted when su'ing
> to another non-root user, but I think there was some talk in this thread
> of adding some kind of refcounting here to make this behave better.

We should avoid creating dirs we don't know are cleaned up again. logind
is the component that cleans up those dirs, hence it should also create
them.

> Could logind not learn to track these "nested-mini-sessions"? They all

That's what "force-new-session=1" would do.

Lennart

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


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

2013-11-26 Thread Lennart Poettering
On Tue, 26.11.13 07:19, Martin Pitt (martin.p...@ubuntu.com) wrote:

Heya,

> Lennart Poettering [2013-11-26  5:17 +0100]:
> > That can't work. As the directory only exists when a real login session
> > is around. su/sudo don't get their own login sessins, hence the dir
> > doesn't necessarily exist and from the perspective of the code running
> > in su/sudo the lifetime semantics of the dir wouldn't match any
> > expections...
> 
> Right, as long as they don't actually get one. I (and I think Colin)
> argued that "su -"/"pkexec" should (just like ssh localhost), as they
> run a full PAM stack which is like logging in. But let's agree to
> disagree at this point.

Disagree? I already offered that we add a new switch to pam_logind which
is only specified in the su-l/sudo-i PAM snippets which tells logind to
create an entirely new session, ignoring that the calling su is already
part of an existing session.

Let's say we call it "force-new-session=1" or so on the pam_systemd
configuration like, and we set that for /etc/pam.d/su-l and
/etc/pam.d/sudo-i but not in /etc/pam.d/su and /etc/pam.d/sudo. (Yes,
Debian/Ubuntu would have to gain separate config snippet for the two
cases to make this work, the way Fedora already has). Then, doing "su -"
would basically get you an entirely new session, only access to the TTY
you'd keep.

Doing this correctly is not entirely trivial though as these new
"limited sessions" should list the original tty device as their tty, but
probably not the seat/vtnr info, since they still should never get
access to any of the seats hardware...

Again, the offer stands: I'd be willing to merge such a patch. That way
the story is somewhat clear:

a) by default audit and logind session ids match
b) su/sudo will inherit almost all state from the parent with the
   exception of uid/gis, and get XDG_RUNTIME_DIR unset in order not to
   fuck things up 
c) su-l/sudo-i will get an almost entirely new session with no access to
   hardware but with access to the original TTY, and that's it.

Lennart

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


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

2013-11-26 Thread Lennart Poettering
On Tue, 26.11.13 14:44, David Timothy Strauss (da...@davidstrauss.net) wrote:

> 
> On Tue, Nov 26, 2013 at 2:35 PM, Lennart Poettering
>  wrote:
> > Not following here. What precisely does this fix, can you elaborate?
> >
> > We currently turn off the poll for the socket fds as soon as we queued
> > the service socket, so that we don't get woken up anymore. What would
> > EPOLLET do good here?
> 
> I think he's been working on having the "distribute" pool scale up
> on-demand, which would involve systemd getting events on new
> connections coming into the listener socket. More specifically, I
> think he intends to, as each new connection comes in, check if we've
> maxed out the number of processes. If not, spin another one up.
> Presumably, we'd drop the listener once the max-size pool is running.

Well, but EPOLLET only works correctly if each time an event is
triggered we dispatch *all* possibly queued events on the fd, until
EAGAIN is read again. But we don't do that, heck, if Listen=no is used
we don''t even read a single incoming connection of the scket, we leave
that to the daemon we spawn, but we cannot trust that. So, I don't get
what EPOLLET is supposed to do good 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] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]

2013-11-26 Thread Colin Walters
On Tue, 2013-11-26 at 09:53 +, Colin Guthrie wrote:

> Colin W's later patch did implement these semantics for the root user's
> XDG_RUNTIME_DIR. It kept it around and didn't tidy it up. Doesn't this
> solve the problem for the root user nicely (which is the primary problem)?

Yes, I run "pkexec" to root every day, and would like if code that ran
as root could just assume it was there.

I may look at rebasing my patch on top of master again as a followup.

> But regardless, why doesn't this code create the dirs if they don't
> exist anyway? Sure, this doesn't deal with lifetime problem (i.e.
> knowing when to delete the dirs again) 

Because the lifetime problem is important; daemons and the like use it
to implement "run only once" semantics.  At least if it's unset your
daemon can say "Sorry" and abort.  If we cleaned it up while the daemon
was running, you could get two, and that could lead to data loss.

> Could logind not learn to track these "nested-mini-sessions"? 

I think so, but:

> Perhaps it is a problem tracking when one of these nested sessions
> actually logs out? Or perhaps the a problem would be doing something
> like su[mini-session]+screen+detatch+logout[mini-session]? The
> mini-session would be closed and the runtime dir cleaned up even tho'
> processes in screen were still needing it. 

Exactly.  If we were going to claim to provide XDG_RUNTIME_DIR in these
types of scenarios, we need to avoid it being cleaned up too early.


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


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

2013-11-26 Thread Dr. Werner Fink
On Tue, Nov 26, 2013 at 02:39:49PM +, Colin Guthrie wrote:
> 'Twas brillig, and Dr. Werner Fink at 26/11/13 14:21 did gyre and gimble:
> > On Tue, Nov 26, 2013 at 10:41:36AM +, Colin Guthrie wrote:
> >> 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble:
> >>> Hey Lennart,
> >>>
> >>> Lennart Poettering [2013-11-26  5:12 +0100]:
>  I implemented this now, using a different approach than Martin's
>  original patch (i.e. I don't think it is a good idea to involve stat()
>  here, instead let's just let logind pass all information to
>  pam_systemd).
> >>>
> >>> Thanks!
> >>
> >> Indeed, thanks for this!
> >>
> >> If anyone backports this fix to v208 (i.e. pre sd-bus) please share it
> >> here. I'll likely do it just to have the "upstream-blessed" fix, but
> >> doubt I'll get around it it until later in the week.
> > 
> > I've backported it.
> 
> Can you link to it or attach it please?

Yep ...

> > But during tests I've found that it does not help
> > if the environment variable XDG_RUNTIME_DIR already exists before doing
> > su.  It will not unset but exported.
> 
> That's expected.
> 
> su does not do any env cleaning, su - does, sudo does, pkexec does.

I'm aware ;) neverthelesss ...

> 
> su's behaviour is to always not touch stuff and thus this is known and
> expected and has always been a problem.
> 
> Longer term we need to solve this more holistically (hence why I've
> tried to get information about "how things should be done" in the future
> to start helping lay the ground work for thise bits), but this is still
> the best fix we have just now.
> 
> For the specific case of su'ing to root, (which is the most common and
> potentially problematic one), I will probably use Colin W's most recent
> patch to have a static root runtime dir and for logind to set it. This
> should fix XDG_RUNTIME_DIR when su'ing to root. I'm not so worried about
> su'ing to other users (the damage that can be done is much more
> limited), but longer term we do need to address that nicely too IMO
> (which will likely need changes in su itself and a number of other places)

... I've a bug report here which indeed shows exactly the ``other'' problem.
If you do not like the enhancement in my backport you may replace

+} else {
+(void) unsetenv("XDG_RUNTIME_DIR");
+r = pam_putenv(handle, "XDG_RUNTIME_DIR");
+if (r != PAM_SUCCESS && r != PAM_BAD_ITEM) {
+ pam_syslog(handle, LOG_ERR, "Failed to unset runtime 
dir.");
+}

with `}'  ... otherwise both the pam_putenv() and unsetenv() are required
to make sure that XDG_RUNTIME_DIR is removed.

Werner

-- 
  "Having a smoking section in a restaurant is like having
  a peeing section in a swimming pool." -- Edward Burr
Based on upstream baae0358f349870544884e405e82e4be7d8add9f
| From: Lennart Poettering 
| Date: Tue, 26 Nov 2013 04:05:00 +
| Subject: pam_systemd: do not set XDG_RUNTIME_DIR if the session's original user is not the same as the newly logged in one
| It's better not to set any XDG_RUNTIME_DIR at all rather than one of a
| different user. So let's do this.
--- systemd-208/src/login/logind-dbus.c
+++ systemd-208/src/login/logind-dbus.c	2013-11-26 13:37:05.730735774 +
@@ -523,6 +523,7 @@ static int bus_manager_create_session(Ma
 DBUS_TYPE_OBJECT_PATH, &path,
 DBUS_TYPE_STRING, &session->user->runtime_path,
 DBUS_TYPE_UNIX_FD, &fifo_fd,
+DBUS_TYPE_UINT32, &session->user->uid,
 DBUS_TYPE_STRING, &cseat,
 DBUS_TYPE_UINT32, &vtnr,
 DBUS_TYPE_BOOLEAN, &exists,
--- systemd-208/src/login/logind-session-dbus.c
+++ systemd-208/src/login/logind-session-dbus.c	2013-11-26 13:36:07.478236401 +
@@ -755,6 +755,7 @@ int session_send_create_reply(Session *s
 DBUS_TYPE_OBJECT_PATH, &path,
 DBUS_TYPE_STRING, &s->user->runtime_path,
 DBUS_TYPE_UNIX_FD, &fifo_fd,
+DBUS_TYPE_UINT32, &s->user->uid,
 DBUS_TYPE_STRING, &cseat,
 DBUS_TYPE_UINT32, &vtnr,
 DBUS_TYPE_BOOLEAN, &exists,
--- systemd-208/src/login/pam-module.c
+++ systemd-208/src/login/pam-module.c	2013-11-26 14:32:20.194235777 +
@@ -93,24 +93,18 @@ static int get_user_data(
 assert(ret_username);
 assert(ret_pw);
 
-r = audit_loginuid_from_pid(0, &uid);
-if (r >= 0)
-pw = pam_modutil_getpwuid(handle, uid);
-else {
-r = pam_get_user(handle, &username, NULL);
-if (r != PAM_SUCCESS) {
-pam_syslog(handle, LOG_E

Re: [systemd-devel] Question regarding the NotifyAccess parameter

2013-11-26 Thread Hoyer, Marko (ADITG/SW2)
> >One more issue I observed is - if I specify Restart=on-failure, if
> > watchdog timer expire, it restart the service. But I can see that it
> > create two processes rather than restarting the process. But if I do
> > systemctl restart Myservice , it kills the previous instance of
> > service and start a new service. Any pointers on why it happens so.

This part has been already reported as a bug in May:
http://lists.freedesktop.org/archives/systemd-devel/2013-May/011030.html

Best to my knowledge, this has been fixed in systemd 203, 204, or 205 ...
Please note that the link above does not contain the final bug fix. Some 
discussions followed which led to the final solution at a certain point. Follow 
the threads, you'll find it ...


Best regards

Marko Hoyer
Software Group II (ADITG/SW2)

Tel. +49 5121 49 6948
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-26 Thread Colin Guthrie
'Twas brillig, and Dr. Werner Fink at 26/11/13 14:21 did gyre and gimble:
> On Tue, Nov 26, 2013 at 10:41:36AM +, Colin Guthrie wrote:
>> 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble:
>>> Hey Lennart,
>>>
>>> Lennart Poettering [2013-11-26  5:12 +0100]:
 I implemented this now, using a different approach than Martin's
 original patch (i.e. I don't think it is a good idea to involve stat()
 here, instead let's just let logind pass all information to
 pam_systemd).
>>>
>>> Thanks!
>>
>> Indeed, thanks for this!
>>
>> If anyone backports this fix to v208 (i.e. pre sd-bus) please share it
>> here. I'll likely do it just to have the "upstream-blessed" fix, but
>> doubt I'll get around it it until later in the week.
> 
> I've backported it.

Can you link to it or attach it please?

> But during tests I've found that it does not help
> if the environment variable XDG_RUNTIME_DIR already exists before doing
> su.  It will not unset but exported.

That's expected.

su does not do any env cleaning, su - does, sudo does, pkexec does.

su's behaviour is to always not touch stuff and thus this is known and
expected and has always been a problem.

Longer term we need to solve this more holistically (hence why I've
tried to get information about "how things should be done" in the future
to start helping lay the ground work for thise bits), but this is still
the best fix we have just now.

For the specific case of su'ing to root, (which is the most common and
potentially problematic one), I will probably use Colin W's most recent
patch to have a static root runtime dir and for logind to set it. This
should fix XDG_RUNTIME_DIR when su'ing to root. I'm not so worried about
su'ing to other users (the damage that can be done is much more
limited), but longer term we do need to address that nicely too IMO
(which will likely need changes in su itself and a number of other places)

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Question regarding the NotifyAccess parameter

2013-11-26 Thread Zbigniew Jędrzejewski-Szmek
On Thu, Nov 21, 2013 at 08:56:25PM +0530, salil GK wrote:
> Hello
> 
>I have a service in shell script in which I am sending notification to
> systemd using `systemd-notify WATCHDOG=1` command. What happens is -
> systemd-notify will be a child process and in the systemd notification will
> not be honoured if NotifyAccess is set to main. Is there any work around
> for this.
Use NotifyAccess=all?

>One more issue I observed is - if I specify Restart=on-failure, if
> watchdog timer expire, it restart the service. But I can see that it create
> two processes rather than restarting the process. But if I do systemctl
> restart Myservice , it kills the previous instance of service and start a
> new service. Any pointers on why it happens so.
That would be a significant bug! Can you post a short example which shows
the bug?

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


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

2013-11-26 Thread Dr. Werner Fink
On Tue, Nov 26, 2013 at 10:41:36AM +, Colin Guthrie wrote:
> 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble:
> > Hey Lennart,
> > 
> > Lennart Poettering [2013-11-26  5:12 +0100]:
> >> I implemented this now, using a different approach than Martin's
> >> original patch (i.e. I don't think it is a good idea to involve stat()
> >> here, instead let's just let logind pass all information to
> >> pam_systemd).
> > 
> > Thanks!
> 
> Indeed, thanks for this!
> 
> If anyone backports this fix to v208 (i.e. pre sd-bus) please share it
> here. I'll likely do it just to have the "upstream-blessed" fix, but
> doubt I'll get around it it until later in the week.

I've backported it.  But during tests I've found that it does not help
if the environment variable XDG_RUNTIME_DIR already exists before doing
su.  It will not unset but exported.

Werner

-- 
  "Having a smoking section in a restaurant is like having
  a peeing section in a swimming pool." -- Edward Burr
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd 208:trouble with inactive user sessions at non-seat0 seats

2013-11-26 Thread Laércio de Sousa
David,

Looking at GDM debug and gdm-simple-slave.c source file, as well as
"loginctl show-seat" output. I guess that GDM only requests user session
activation for seats with CanMultiSession=yes, but currently systemd-logind
still sets CanMultiSession=no for my non-seat0 seats.

Does seat_can_multi_session() heuristics need some improvement since
generic multi-session was introduced?

I've applied your last patch. I confirm it solves my problem for now.

Thanks for all, and keep the good work!

CANTATE DOMINO CANTICUM NOVUM
QUIA MIRABILIA FECIT

Laércio


2013/11/26 David Herrmann 

> Hi
>
> On Tue, Nov 26, 2013 at 11:45 AM, Laércio de Sousa 
> wrote:
> > Hi David!
> >
> > I just tested your patch, and unfortunately it didn't work. However, I
> guess
> > what could be the question.
> >
> > If I understand correctly, your patch applies for the case we have no
> active
> > session at all in a non-seat0 seat. However, I do have an active session
> in
> > my non-seat0 seat --- the one GDM opens to show the greeter. The
> question is
> > that, when a user logs in at a non-seat0 seat, GDM (or whatever) should
> > activate automatically the new user session, sending the greeter session
> to
> > the background while it's in state "closing", as we have for seat0. In my
> > current setup, it doesn't occur: in a non-seat0 seat, the "closing"
> greeter
> > session remains active, while the new user session starts at background
> > (inactive).
> >
> > If I activate manually the user session (loginctl activate ),
> the
> > converse occurs: when the user logs out, its "closing" user session
> remains
> > active, while the new greeter session starts at background (inactive).
> >
> > Just for comparison: if I configure autologin for GDM (to avoid opening
> the
> > greeter session), or use multiseat-patched KDM (which doesn't open
> greeter
> > sessions), the user session starts at foreground, as expected, even
> without
> > this patch.
>
> That gdm behavior is actually weird. It should explicitly request the
> new session to become active. There is no reason for logind to
> *assume* the new session should be activated automatically.. hmm.
>
> The appended patch reverts the new behavior. Can you give it a try? I
> actually cannot get gdm to pick up my additional seats.. and looking
> into the monstrosity called gdm-source-base I have no clue what it
> does. If you can confirm that this resets the behavior, I will apply
> it so we don't break existing setups.
>
> I will figure out something else for new multi-session capable seats.
>
> Thanks
> David
>
>
> diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
> index b30c4ce..ca0e8d7 100644
> --- a/src/login/logind-seat.c
> +++ b/src/login/logind-seat.c
> @@ -413,8 +413,8 @@ int seat_attach_session(Seat *s, Session *session) {
>  seat_send_changed(s, "Sessions", NULL);
>
>  /* On seats with VTs, the VT logic defines which session is
> active. On
> - * seats without VTs, we automatically activate the first
> session. */
> -if (!seat_has_vts(s) && !s->active)
> + * seats without VTs, we automatically activate new sessions. */
> +if (!seat_has_vts(s))
>  seat_set_active(s, session);
>
>  return 0;
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] tree-wide usage of %m specifier instead of strerror(errno)

2013-11-26 Thread David Timothy Strauss
Thanks. Applied.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-26 Thread Colin Guthrie
'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble:
> Hey Lennart,
> 
> Lennart Poettering [2013-11-26  5:12 +0100]:
>> I implemented this now, using a different approach than Martin's
>> original patch (i.e. I don't think it is a good idea to involve stat()
>> here, instead let's just let logind pass all information to
>> pam_systemd).
> 
> Thanks!

Indeed, thanks for this!

If anyone backports this fix to v208 (i.e. pre sd-bus) please share it
here. I'll likely do it just to have the "upstream-blessed" fix, but
doubt I'll get around it it until later in the week.


Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-26 Thread Colin Guthrie
'Twas brillig, and Lennart Poettering at 26/11/13 04:17 did gyre and gimble:
> On Wed, 20.11.13 19:19, Colin Walters (walt...@verbum.org) wrote:
> 
>>> So yeah, there your mix
>>> and match is broken: 
>>
>> I'm proposing a simple goal: XDG_RUNTIME_DIR should always be that
>> matching the current uid.  I can't think of any case where you'd
>> want it otherwise.
> 
> That can't work. As the directory only exists when a real login session
> is around. su/sudo don't get their own login sessins, hence the dir
> doesn't necessarily exist and from the perspective of the code running
> in su/sudo the lifetime semantics of the dir wouldn't match any
> expections...


Colin W's later patch did implement these semantics for the root user's
XDG_RUNTIME_DIR. It kept it around and didn't tidy it up. Doesn't this
solve the problem for the root user nicely (which is the primary problem)?

[The rest of this mail is more "asking daft questions" than any kind of
complaint. I'm still struggling to see the problems of some proposed
solutions]

But regardless, why doesn't this code create the dirs if they don't
exist anyway? Sure, this doesn't deal with lifetime problem (i.e.
knowing when to delete the dirs again) which is highlighted when su'ing
to another non-root user, but I think there was some talk in this thread
of adding some kind of refcounting here to make this behave better.

Could logind not learn to track these "nested-mini-sessions"? They all
seem to go through the pam stack so what is the fundamental problem in
adding some kind of tracking to them? The statement that "As the
directory only exists when a real login session is around" appears to be
a self-imposed limitation inside logind. Surely we just create the
folder as needed - even on a su, track this mini-session. We don't need
to expose it as a real session - nor really track the processes
specifically (this isn't done now anyway), but just use it internally in
any "how many sessions are there for this user" checks when nuking the
runtime dir. This would solve the lifetime issue so it's possible to
tidy up the runtime dirs properly only when the last user really does
disappear.

I'm sure I'm missing something but all this just seems to be being made
more complicated than is necessary due to artificial problems that are
just a product of the current implementation! Is this a route forward or
is there a fundamental problem with that? I appreciate that not making
it a proper session and tracking the processes properly would maybe feel
nasty, but I'd still say it was a step in the right direction.

Perhaps it is a problem tracking when one of these nested sessions
actually logs out? Or perhaps the a problem would be doing something
like su[mini-session]+screen+detatch+logout[mini-session]? The
mini-session would be closed and the runtime dir cleaned up even tho'
processes in screen were still needing it. If this is a problem case
does screen need to be capable of registering it's own session (and I'd
say it needs to be a full session here such that it doesn't really die
when the containing session logs out). Would logind have to learn a new
mode for creating detachable sessions for things like screen or would
screen have to become setuid or something equally ugly here?


I'd really appreciate it if someone who groks all this stuff could write
up a "how it should be done" wiki page or something explaining what the
"best practices" would be to approach solutions to:

 1. starting a text shell as another user (either within a graphical env
or not (and whether to run GUI apps and play sound as the other user or
not).
 2. starting a new, detachable session e.g. screen as the current user
(possible done *after* 1. above).

These are things people do and I think there are legitimate reasons for
doing them - regardless, even if some of the cases only have
illegitimate reasons, we need to be able to fail gracefully and,
ideally, present a warning to the user. Making a stance and not
supporting certain (stupid) setups only works when you actually *tell*
the user that they can't do something and explain why and what the
alternative is.


Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] tree-wide usage of %m specifier instead of strerror(errno)

2013-11-26 Thread Daniel Buch
Also for log_error() except where a specific error is specified

e.g. errno ? strerror(errno) : "Some user specified message"
---
 TODO   |  2 --
 src/core/automount.c   |  2 +-
 src/core/main.c|  2 +-
 src/core/manager.c |  2 +-
 src/core/mount-setup.c |  2 +-
 src/core/service.c |  4 ++--
 src/initctl/initctl.c  | 12 +---
 src/journal/coredumpctl.c  |  2 +-
 src/journal/journald-console.c |  4 ++--
 src/journal/journald-kmsg.c|  2 +-
 src/udev/collect/collect.c |  6 +++---
 src/udev/scsi_id/scsi_id.c |  2 +-
 src/udev/scsi_id/scsi_serial.c |  8 +++-
 src/udev/udev-rules.c  |  2 +-
 14 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/TODO b/TODO
index d63e13e..653258a 100644
--- a/TODO
+++ b/TODO
@@ -824,8 +824,6 @@ Regularly:
 
 * Use PR_SET_PROCTITLE_AREA if it becomes available in the kernel
 
-* %m in printf() instead of strerror(errno);
-
 * pahole
 
 * set_put(), hashmap_put() return values check. i.e. == 0 doesn't free()!
diff --git a/src/core/automount.c b/src/core/automount.c
index 49a64b1..66e3d78 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -304,7 +304,7 @@ static int open_dev_autofs(Manager *m) {
 
 m->dev_autofs_fd = open("/dev/autofs", O_CLOEXEC|O_RDONLY);
 if (m->dev_autofs_fd < 0) {
-log_error("Failed to open /dev/autofs: %s", strerror(errno));
+log_error("Failed to open /dev/autofs: %m");
 return -errno;
 }
 
diff --git a/src/core/main.c b/src/core/main.c
index dbc98db..69d3a43 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -131,7 +131,7 @@ _noreturn_ static void crash(int sig) {
 
 pid = fork();
 if (pid < 0)
-log_error("Caught <%s>, cannot fork for core dump: 
%s", signal_to_string(sig), strerror(errno));
+log_error("Caught <%s>, cannot fork for core dump: 
%m", signal_to_string(sig));
 
 else if (pid == 0) {
 struct rlimit rl = {};
diff --git a/src/core/manager.c b/src/core/manager.c
index aa4baaa..65cb73c 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -307,7 +307,7 @@ static int enable_special_signals(Manager *m) {
 } else {
 /* Enable that we get SIGWINCH on kbrequest */
 if (ioctl(fd, KDSIGACCEPT, SIGWINCH) < 0)
-log_warning("Failed to enable kbrequest handling: %s", 
strerror(errno));
+log_warning("Failed to enable kbrequest handling: %m");
 }
 
 return 0;
diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index 73c2698..c601c97 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -188,7 +188,7 @@ static int mount_one(const MountPoint *p, bool relabel) {
   p->type,
   p->flags,
   p->options) < 0) {
-log_full((p->mode & MNT_FATAL) ? LOG_ERR : LOG_DEBUG, "Failed 
to mount %s: %s", p->where, strerror(errno));
+log_full((p->mode & MNT_FATAL) ? LOG_ERR : LOG_DEBUG, "Failed 
to mount %s: %m", p->where);
 return (p->mode & MNT_FATAL) ? -errno : 0;
 }
 
diff --git a/src/core/service.c b/src/core/service.c
index 28b1465..7c5d5d8 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -3507,7 +3507,7 @@ static int service_enumerate(Manager *m) {
 d = opendir(path);
 if (!d) {
 if (errno != ENOENT)
-log_warning("opendir(%s) failed: %s", 
path, strerror(errno));
+log_warning("opendir(%s) failed: %m", 
path);
 
 continue;
 }
@@ -3540,7 +3540,7 @@ static int service_enumerate(Manager *m) {
 if (access(fpath, X_OK) < 0) {
 
 if (errno != ENOENT)
-log_warning("access() failed 
on %s: %s", fpath, strerror(errno));
+log_warning("access() failed 
on %s: %m", fpath);
 
 continue;
 }
diff --git a/src/initctl/initctl.c b/src/initctl/initctl.c
index d6c..284319f 100644
--- a/src/initctl/initctl.c
+++ b/src/initctl/initctl.c
@@ -217,7 +217,7 @@ static int fifo_process(Fifo *f) {
 if (errno == EAGAIN)
 return 0;
 
-log_warning("Failed to read from fifo: %s", strerror(errno));
+log_warning("Failed to read from fifo: %m");
 return -1;
 }
 
@@ -278,7 +278,7 @@ static int server_init(Server *s, unsigned n_sockets) {
 s->epoll_fd = epoll_create1(EPOLL_CLOEXEC);