Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network

2014-12-01 Thread Karel Zak
On Fri, Nov 28, 2014 at 09:27:43PM +0100, Lennart Poettering wrote:
> On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> 
> > On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote:
> > > This adds auto detection for iSCSI and some FCoE drivers and treats
> > > mounts to file-systems on those devices as remote-fs.
> > > 
> > > Signed-off-by: Chris Leech 
> > No need for this.
> > 
> > I now pushed patches 1-4 with some small changes here and there.
> > Since libmount is not optional, I removed if from the version string.
> > 
> > This patch I didn't push: this seems like something that would be better
> > done through udev rules, by setting some tags. Then systemd could simply
> > check for the presence of a tag on the device without having any
> > special knowledge about iscsi and firends.
> 
> Honestly, I am really not sure I like this patch.
> 
> One one hand we now have a hard dep on libmount. Which I figure is
> mostly OK. However, what I find really weird about this is that even
> though libmount is supposed to abstract access to the "utab" away, it
> doesn't sufficiently: we still hardcode the utab path now so that we
> can watch it. I mean, either we use an abstracted interface or we
> don't. THis really smells to me as either libmount should provide some
> form of inotify iface to utab, or we should parse utab directly. If
> libmount is the only and official API for utab, then we should
> that. If the utab file however is API too, then we can well go ahead
> and parse it directly.

I'd like to keep the utab file private. The whole libmount API is
abstraction and completely hides the difference between original
/etc/mtab and new /run/mount/utab.

The original Chris' purpose for the patches has been _netdev, it
means to improve detection of dependence between filesystem and
network. I still believe that the right way is to check for network
block devices than rely on crazy _netdev userspace mount option.

Wouldn't be enough to use Chris' iSCSI and FCoE auto detection?

Or we can add udev rule to have NET_DEVBLK tag in udevdb, or maybe we
can ask kernel developers to add /sys/block/sdb/network (as we already
have "removable" in /sys).

Chris, why do you want both ways (utab and the auto detection)? 

IMHO it would be really nice to completely kill _netdev in systemd
universe ;-) It's legacy from shell init scripts.


Anyway, if you still want to read userspace mount options than I can
add something like:

 mnt_inotify_mtab_add_watch()
 mnt_inotify_mtab_rm_watch()
 mnt_inotify_mtab_changed()

to hide all the paths and private mtab/utab stuff.

> THe new code looks also buggy to me. For example, am I missing
> something or is nobody creating /run/mount? I mean, we need to make
> sure that it exists before we can install an inotify watch on it.

yep, it should be hidden within libmount

> Also, it leaves the cescape() calls in for what we read from libmount,
> which makes a lot of alarm bells ring in my head: doesn't libmount do
> that on its own?

sure, libmount encodes/decodes all when read/write the files.

Karel

-- 
 Karel Zak  
 http://karelzak.blogspot.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label

2014-12-01 Thread Przemyslaw Kedzierski

Hello
Could you take a look at my patch?
Regards

Przemyslaw Kedzierski

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


Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors

2014-12-01 Thread Jan Synacek
Lennart Poettering  writes:
> On Tue, 25.11.14 10:01, David Herrmann (dh.herrm...@gmail.com) wrote:
>
>> I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
>> libxkbcommon is still not 100% compatible to libxkb (and doesn't
>> intend to be that, I guess). As we write X11 configs here, I just
>> continue with a warning.
>> 
>> If you call bus_error_setfv(), then sd-bus will return a method-error
>> to the caller. However, you also send a method-return in
>> method_set_x11_keyboard(). You thus end up with 2 calls.
>> 
>> I'm not sure how to solve this. Furthermore, libxkbcommon can print a
>> lot of informational warnings, and we shouldn't use just the last one.
>> One idea would be to copy the same logic into localectl. You could
>> also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get
>> more information there.
>> Yes, this would mean compiling the keymap twice, but it's for the sake
>> of debugging output so I think it's fine.
>
> What precisely are the error messages libxkbcommon prints? Are they
> really material to pass to the client? I mean, are they really about
> some invalid data the client passed or just about something broken in
> the way the system is set up? If it's the latter than I figure we
> should return just a simple error to the client, and pass the full
> logs to the logging stream...
>
> Lennart

Thinking about it again, I don't think that it's a good idea to pass
those errors to the client...

A few examples:

# localectl set-x11-keymap QQ

Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Couldn't find file "symbols/QQ" in include paths
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
1 include paths searched:
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
/usr/share/X11/xkb
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Abandoning symbols file "(unnamed)"
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Failed to compile xkb_symbols
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Failed to compile keymap
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: Cannot 
compile XKB keymap for new x11 keyboard layout ('' / 'QQ' / '' / ''): Invalid 
argument

# localectl set-x11-keymap us pc105 QQ

Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Couldn't process include statement for 'us(QQ)'
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Abandoning symbols file "(unnamed)"
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Failed to compile xkb_symbols
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Failed to compile keymap
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: Cannot 
compile XKB keymap for new x11 keyboard layout ('pc105' / 'us' / 'QQ' / ''): 
Invalid argument

Those errors are just stupid. Maybe they would make sense if there was
also libxkbcommon code in front of me... But from the user's
perspective, they say nothing useful. I added the prefix so the errors
at least have some context.

My original patch would say something like "No such layout 'QQ'" in the
first case and "No variant 'QQ' for layout 'us'" in the second.

So, is it worth logging this information? Any other ideas?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [WIP PATCH] Do not realize and migrate cgroups multiple times

2014-12-01 Thread Martin Pitt
Hello all,

In my efforts to make user LXC containers work I noticed that under a
"real" desktop (not just nspawn with VT login or ssh logins) my
carefully set up cgroups in the non-systemd controllers get reverted.
I. e. I put the session leader (and all other pids) of logind sessions
(/user.slice/user-1000.slice/session-XX.scope) into all cgroup
controllers, but a second after they are all back in the / cgroups (or
sometimes in /user.slice). After some days of debugging (during which
I learned a lot about the innards of systemd :-) I finally found out
why:

During unit cgroup initialization (unit_realize_cgroup), sibling
cgroups are queued instead of initialized immediately.
unit_create_cgroups() makes an attempt to avoid initializing and
migrating a cgroup more than once:

   path = unit_default_cgroup_path(u);
   [...]
   r = hashmap_put(u->manager->cgroup_unit, path, u);
if (r < 0) {
log_error(r == -EEXIST ? "cgroup %s exists already: %s" : 
"hashmap_put failed for %s: %s", path, strerror(-r));
return r;
}

But this doesn't work: "path" always gets allocated freshly in that
function, so the pointer is never in the hashmap and the -EEXISTS
never actually hits. This causes -.slice to be initialized and
recursively migrated a gazillion times, which explains the random
punting of sub-cgroup PIDs back to /.

I fixed this with an explicit hashmap_get() call, which compares
values instead of pointers.

I also added some debugging to that function:

  log_debug("unit_create_cgroups %s: path=%s realized %i hashmap %p", u->id, 
path, u->cgroup_realized, hashmap_get(u->manager->cgroup_unit, path));

(right before the hashmap_put() above), which illustrates the
problem:

systemd[1]: Starting Root Slice.
systemd[1]: unit_create_cgroups -.slice: path= realized 0 hashmap (nil)
systemd[1]: Created slice Root Slice.
[...]
pid1 systemd[1]: unit_create_cgroups session-c1.scope: 
path=/user.slice/user-1000.slice/session-c1.scope realized 0 hashmap (nil)
systemd[1]: Started Session c1 of user martin.
[... later on when most things have been started ...]
systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850
systemd[1]: unit_create_cgroups -.slice: cgroup  exists already
systemd[1]: Failed to realize cgroups for queued unit user.slice: File exists
systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850
systemd[1]: unit_create_cgroups -.slice: cgroup  exists already
systemd[1]: Failed to realize cgroups for queued unit grub-common.slice: File 
exists
systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850
systemd[1]: unit_create_cgroups -.slice: cgroup  exists already
systemd[1]: Failed to realize cgroups for queued unit networking.slice: File 
exists

... and so on, basically once for each .service.

Initializing -.slice so many times is certainly an unintended effect
of the peer cgroup creation. Thus the patch fixes the multiple
initialization/creation, but a proper fix should also quiesce these
error messages. The condition could be checked explicitly, i. e. we
skip the "Failed to realize..." error for EEXISTS, or we generally
tone this down to log_debug. I'm open to suggestions. And of course
the log_debug should be removed; it's nice to illustrate the problem,
but doesn't really belong into production code.

Thanks,

Martin

-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From fd2f8a444c9c644a39dd3b619934e8768e03c2a3 Mon Sep 17 00:00:00 2001
From: Martin Pitt 
Date: Mon, 1 Dec 2014 10:50:06 +0100
Subject: [PATCH] Do not realize and migrate cgroups multiple times

unit_create_cgroups() tries to check if a cgroup already exists. But has the
destination path is always allocated dynamically as a new string, that pointer
will never already be in the hashmap, thus hashmap_put() will never actually
fail with EEXISTS. Thus check for the existance of the cgroup path explicitly.

Before this, "-.slice" got initialized and its child PIDs migrated many times
through queuing the realization of sibling units; thiss caused any cgroup
controller layout from sub-cgroups to be reverted and their pids moved back to
the root cgroup in all controllers (except systemd).
---
 src/core/cgroup.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 8820bee..3d36080 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -614,6 +614,13 @@ static int unit_create_cgroups(Unit *u, CGroupControllerMask mask) {
 if (!path)
 return log_oom();
 
+log_debug("unit_create_cgroups %s: path=%s realized %i hashmap %p", u->id, path, u->cgroup_realized, hashmap_get(u->manager->cgroup_unit, path));
+
+if (hashmap_get(u->manager->cgroup_unit, path)) {
+log_error("unit_create_cgroups %s: cgroup %s exists already", u->id, u->cgroup_path);
+

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

2014-12-01 Thread Quentin Lefebvre

Hi,

On 01/12/2014 01:12, Lennart Poettering wrote :

On Mon, 24.11.14 19:25, Quentin Lefebvre (qlefebvre_...@yahoo.com) wrote:


Le 24/11/2014 19:17, Zbigniew Jędrzejewski-Szmek a écrit :

On Mon, Nov 24, 2014 at 07:03:27PM +0100, Quentin Lefebvre wrote:

Le 24/11/2014 19:01, Zbigniew Jędrzejewski-Szmek a écrit :

On Mon, Nov 24, 2014 at 06:44:25PM +0100, Quentin Lefebvre wrote:

Hi,

I tested your patch and actually it doesn't solve the bug.
For example, if "hash=sha512" is provided in /etc/crypttab, the
first >   if (!streq(arg_hash, "plain"))
is true, and the

+} else if (!key_file)

is not reached.

This is be design. My patch is quite different from your patch,
which I tried to make clear in the description.

If you specify hash=sha512, then you get hash=sha512.


Yes, and this is the problem.
cryptsetup ignores the hash, so that we should obtain hash=NULL for
it to work.

Systemd is not going to work around a bug in a different package.
Specifying a hash in the configuration if you don't want a hash
is an error, please just fix it there.


I understand your point.
Still you have a cryptsetup tool in systemd, so I would expect it behaves as
the "true" cryptsetup program.

The problem here is compatibility, you do something with cryptsetup and then
your system fails to boot because of a different behaviour of
systemd.


Note that compatibiltiy is really a problem with crypttab anyway, as
there were multiple implementations of the code that reads it around:
at least the one from DEbian and the one from Fedora, both of which
had very different feature sets and even different syntaxes.

With systemd's crypttab support we try to provide a decent amount of
compatibility with both, to the level that it makes sense. Since we
cannot reach 100% compat anyway, this explicitly means no bug-for-bug
compatibility however. Hence I really think we should do the correct
thing, rather than the traditional thing here, given that this is not
the most common usecase anyway,

Hope that makes sense,


OK. I also read Zbigniew's answer on the bug report.
I understand your point, which makes sense.

I guess we'll have to document these different behaviors in Debian, so 
that users don't get too confused...


But anyway, plain dm-crypt devices, even if they're not the most common 
usecase, should be handled in the long-term, as it is still a useful, 
and quite different setup compared to LUKS devices.


Thanks for your replies and great work!

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


Re: [systemd-devel] [PATCH] x86: defconfig: Enable CONFIG_FHANDLE

2014-12-01 Thread Lennart Poettering
On Mon, 01.12.14 14:54, Dave Chinner (da...@fromorbit.com) wrote:

> On Mon, Dec 01, 2014 at 02:03:43AM +0100, Lennart Poettering wrote:
> > On Mon, 01.12.14 01:41, Richard Weinberger (rich...@nod.at) wrote:
> > 
> > > CC'ing systemd folks.
> > > 
> > > Lennart, can you please explain why you need CONFIG_FHANDLE for systemd?
> > > Maybe I'm reading the source horrible wrong.
> > 
> > For two usecases:
> > 
> > a) Being able to detect if something is a mount point. The traditional
> >way to do this is by stat()ing the dir in question and its parent
> >and comparing st_dev. That logic is not able to detect bind mounts
> >however, if destination and the place the mount is at are actually
> >on the same file system... Thus we check the mount id too, if we
> >can get our hands on it.
> 
> So what you really want in the mount id in st_buf.st_dev, not the
> underlying device number. i.e. fstatat(dirfd, path, buf,
> AT_MOUNTID)?

Well, I am not a fan of overloading things, and there might be reasons
why one would want to know both the mount id and the device id at the
same time with one atomic call, but ultimately I don't really care,
and fstatat(AT_MOUNT_ID) would certainly be at least as useful as
name_to_handle_at() is. 

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


Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network

2014-12-01 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Dec 01, 2014 at 11:24:58AM +0100, Karel Zak wrote:
> On Fri, Nov 28, 2014 at 09:27:43PM +0100, Lennart Poettering wrote:
> > On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) 
> > wrote:
> > 
> > > On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote:
> > > > This adds auto detection for iSCSI and some FCoE drivers and treats
> > > > mounts to file-systems on those devices as remote-fs.
> > > > 
> > > > Signed-off-by: Chris Leech 
> > > No need for this.
> > > 
> > > I now pushed patches 1-4 with some small changes here and there.
> > > Since libmount is not optional, I removed if from the version string.
> > > 
> > > This patch I didn't push: this seems like something that would be better
> > > done through udev rules, by setting some tags. Then systemd could simply
> > > check for the presence of a tag on the device without having any
> > > special knowledge about iscsi and firends.
> > 
> > Honestly, I am really not sure I like this patch.
> > 
> > One one hand we now have a hard dep on libmount. Which I figure is
> > mostly OK. However, what I find really weird about this is that even
> > though libmount is supposed to abstract access to the "utab" away, it
> > doesn't sufficiently: we still hardcode the utab path now so that we
> > can watch it. I mean, either we use an abstracted interface or we
> > don't. THis really smells to me as either libmount should provide some
> > form of inotify iface to utab, or we should parse utab directly. If
> > libmount is the only and official API for utab, then we should
> > that. If the utab file however is API too, then we can well go ahead
> > and parse it directly.
> 
> I'd like to keep the utab file private. The whole libmount API is
> abstraction and completely hides the difference between original
> /etc/mtab and new /run/mount/utab.
> 
> The original Chris' purpose for the patches has been _netdev, it
> means to improve detection of dependence between filesystem and
> network. I still believe that the right way is to check for network
> block devices than rely on crazy _netdev userspace mount option.
> 
> Wouldn't be enough to use Chris' iSCSI and FCoE auto detection?
Please see previous discussion... Detecting network might not be trivial
if the devices are layered and there's a network-requiring device somewhere
lower in the stack.

Using _netdev is supposed to be an easy mechanism which admins can use
in case whatever other schemes we have in place don't work.

> Or we can add udev rule to have NET_DEVBLK tag in udevdb, or maybe we
> can ask kernel developers to add /sys/block/sdb/network (as we already
> have "removable" in /sys).
> 
> Chris, why do you want both ways (utab and the auto detection)? 
> 
> IMHO it would be really nice to completely kill _netdev in systemd
> universe ;-) It's legacy from shell init scripts.
> 
> 
> Anyway, if you still want to read userspace mount options than I can
> add something like:
> 
>  mnt_inotify_mtab_add_watch()
>  mnt_inotify_mtab_rm_watch()
>  mnt_inotify_mtab_changed()
> 
> to hide all the paths and private mtab/utab stuff.
Maybe something more like what journal client code does here:

 mnt_mtab_get_fd() -> epoll fd
 mnt_mtab_get_events() -> EPOLLIN|EPOLLPRI
 mnt_mtab_process() -> information what changed

and then mnt_mtab_wait() can be constructed on top of this,
but systemd wouldn't be using it.

> > THe new code looks also buggy to me. For example, am I missing
> > something or is nobody creating /run/mount? I mean, we need to make
> > sure that it exists before we can install an inotify watch on it.
> 
> yep, it should be hidden within libmount
> 
> > Also, it leaves the cescape() calls in for what we read from libmount,
> > which makes a lot of alarm bells ring in my head: doesn't libmount do
> > that on its own?
> 
> sure, libmount encodes/decodes all when read/write the files.

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


Re: [systemd-devel] mate desktop user service file

2014-12-01 Thread David Herrmann
Hi

On Sat, Nov 29, 2014 at 10:48 AM, arnaud gaboury
 wrote:
> I only use some settings from mate desktop (clipboard, appearance...)
> thus looking for a service file to start mate-settings-daemon.
>
>
> /home/gabx/.config/systemd/user/mate-settings-daemon.service
> -
> [Unit]
> Description=Mate settings daemon
>
> [Service]
> Type=daemon
> ExecStart=/usr/lib/mate-settings-daemon/mate-settings-daemon
>
> [Install]
> WantedBy=wm.target
> 
>
> The above service leave me with an error:
>
> --
> gabx@hortensia ➤➤ ~ % systemctl --user status mate-settings-daemon
> ● mate-settings-daemon.service - Mate settings daemon
>Loaded: loaded
> (/home/gabx/.config/systemd/user/mate-settings-daemon.service;
> disabled)
>Active: failed (Result: exit-code) since Sat 2014-11-29 10:29:09
> CET; 14min ago
>   Process: 26343
> ExecStart=/usr/lib/mate-settings-daemon/mate-settings-daemon
> (code=exited, status=1/FAILURE)
>  Main PID: 26343 (code=exited, status=1/FAILURE)
>
> Nov 29 10:29:09 hortensia systemd[914]: Started Mate settings daemon.
> Nov 29 10:29:09 hortensia mate-settings-daemon[26343]: No protocol specified
> Nov 29 10:29:09 hortensia mate-settings-daemon[26343]: **
> (mate-settings-daemon:26343): WARNING **: Unable to initialize GTK+

mate-settings-daemon might expect to be run from within an X-session.
These errors look like DISPLAY= isn't set, which is reasonable because
systemd starts programs from a clean environment.

Anyway, you really should talk to the mate developers to ask them
what's needed to run it from systemd. Just putting stuff into services
usually does not work. Many of the X11 legacy expects to be run from
within the X11 environment, not from a clean service.

Thanks
David

> --
>
> I have tried unsuccessfully to change parts of the service file.
> Running manually the command < $
> /usr/lib/mate-settings-daemon/mate-settings-daemon > works.
>
> N.B: display is in my environment.
> ---
> gabx@hortensia ➤➤ ~ % systemctl --user show-environment
> DBUS_SESSION_BUS_ADDRESS=/run/user/1000/dbus/user_bus_socket
> DISPLAY=:0
> --
>
>
> I have no idea what I do wrong. Thank you for advices.
>
>
> --
>
> google.com/+arnaudgabourygabx
> ___
> 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] TimeoutStopSec is ignored (regression)

2014-12-01 Thread David Herrmann
Hi

On Sat, Nov 29, 2014 at 12:35 PM, Ross Lagerwall
 wrote:
> Hi,
>
> On recent versions of systemd, unit_kill_context doesn't set
> wait_for_exit to true which means that service_enter_signal sends
> SIGTERM, immediately moves into stop-sigkill and sends SIGKILL, ignoring
> TimeoutStopSec and often killing processes without giving them a chance
> to cleanup.
>
> Reverting the following change, fixes the problem:
>
> commit 1baccdda2e954214e0c5463d6ed8f06009b33c41
> Author: Lennart Poettering 
> Date:   Wed Feb 5 02:22:11 2014 +0100
>
> core: don't wait for non-control/non-main processes when killing 
> processes on the host either
>
> Since the current kernel cgroup notification logic is easily confused by
> existing subgroups, let's do the same thing as in containers before. and
> just not wait for non-control and non-main processes.
>
> This should be corrected as soon as we have sane cgroup notifications
> from the kernel.

The commit-message and the comment it adds should answer your
question: The kernel cgroup API does not allow us to wait for
non-control processes. That is, we still honor TimeoutStopSec and
friends if we have to wait for the main-process and/or control process
(in those cases, wait_for_exit is still set to true). However, if
there are other processes remaining in the cgroup, we now ignore it.
See the commit you mentioned for an explanation.

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


Re: [systemd-devel] [PATCH] rules: rerun vconsole-setup when switching from vgacon to fbcon

2014-12-01 Thread Ivan Shapovalov
On Friday 07 November 2014 at 16:45:02, Lennart Poettering wrote:   
> On Fri, 07.11.14 17:45, Ivan Shapovalov (intelfx...@gmail.com) wrote:
> 
> > On Thursday 06 November 2014 at 11:02:44, David Herrmann wrote: 
> > > Hi Ray
> > > 
> > > On Thu, Nov 6, 2014 at 10:40 AM, David Herrmann  
> > > wrote:
> > > > On Wed, Nov 5, 2014 at 4:11 PM, Ray Strode  wrote:
> > > >>> So if you have no idea how to make that rule be generated only if
> > > >>> ENABLE_VCONSOLE is set by configure, then we probably should take my
> > > >>> patch below.
> > > >> Your patch seems far better than the options above, but I think it
> > > >> needs a dracut patch to make sure the new rules file gets in the
> > > >> initrd too, or it won't work.
> > > >
> > > > I will push the patch to systemd and let Harald know. I'm not really
> > > > familiar with dracut..
> > > 
> > > Pushed.
> > 
> > Isn't it ugly to re-runn systemd-vconsole-setup straight from an udev rule?
> > I mean, udev has a tendency to kill long-running RUN processes, and I've 
> > seen
> > systemd-vconsole-setup.service to take >5 seconds on some systems...
> > Also, these additional systemd-vconsole-setup instances aren't supervised by
> > systemd...
> 
> Is systemd-vconsole-setup really long-running? Why would it need that
> long?
> 
> To me it appears to be quite OK to be run inside a udev rule.

Well, maybe it is OK to be run from an udev rule, but it is still an
inconsistency between running that binary on boot (via a unit) and
running the same binary when a framebuffer console is added (directly)...

-- 
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] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network

2014-12-01 Thread Karel Zak
On Mon, Dec 01, 2014 at 02:28:33PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> > Wouldn't be enough to use Chris' iSCSI and FCoE auto detection?
> Please see previous discussion... Detecting network might not be trivial
> if the devices are layered and there's a network-requiring device somewhere
> lower in the stack.

 Good point. (It would be possible to analyze whole stack by slave/holders
 relations, but I agree it's too complex and probably too fragile.)

> > Anyway, if you still want to read userspace mount options than I can
> > add something like:
> > 
> >  mnt_inotify_mtab_add_watch()
> >  mnt_inotify_mtab_rm_watch()
> >  mnt_inotify_mtab_changed()
> > 
> > to hide all the paths and private mtab/utab stuff.
> Maybe something more like what journal client code does here:
> 
>  mnt_mtab_get_fd() -> epoll fd
>  mnt_mtab_get_events() -> EPOLLIN|EPOLLPRI
>  mnt_mtab_process() -> information what changed

 Hmm.. utab is optional and very often does not exist, and library
 uses rename(2) to do atomic update of the file. This is reason why
 Chris have used inotify for /run/mount directory (and Lennart asked
 for inotify API). I have doubts you can use epoll to monitor 
 directories, epoll is about I/O monitoring.

Karel

-- 
 Karel Zak  
 http://karelzak.blogspot.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC PATCH] proc, pidns: Add highpid

2014-12-01 Thread Andy Lutomirski
On Sun, Nov 30, 2014 at 11:03 PM, Konstantin Khlebnikov
 wrote:
> Hmm. What about per-task/thread UUID? exported via separate file: 
> /proc/PID/uuid
> It could be created at the first access, thus this wouldn't shlowdown clone().
> Also it could be droped at execve(), so it'll describe execution
> context more precisely than pid.
>

I'd be all for this if it weren't for two issues:

1. This thing needs to work as soon as init is started, and we can't
reliably generate random numbers that early.

2. Migration / CRIU is rather fundamentally at odds with making
anything universally unique, as opposed to just per-user-ns.

So, given that I don't think we can actually provide a universally
unique pid-like thing, I'd rather not even try.

That being said, I want to rework this a little bit.  I think that
highpid should be restricted to the range 2^32 through 2^64 - 4096.
The former prevents anyone from confusing highpid with regular pid,
and the latter means that we don't need to worry about confusion
between errors and valid highpids (e.g. -1 will never be a highpid).

Implementing that will be only mildly annoying.

--Andy

> On Sat, Nov 29, 2014 at 2:05 AM, Andy Lutomirski  wrote:
>> Pid reuse is common, which means that it's difficult or impossible
>> to read information about a pid from /proc without races.
>>
>> This introduces a second number associated with each (task, pidns)
>> pair called highpid.  Highpid is a 64-bit number, and, barring
>> extremely unlikely circumstances or outright error, a (highpid, pid)
>> will never be reused.
>>
>> With just this change, a program can open /proc/PID/status, read the
>> "Highpid" field, and confirm that it has the expected value.  If the
>> pid has been reused, then highpid will be different.
>>
>> The initial implementation is straightforward: highpid is simply a
>> 64-bit counter. If a high-end system can fork every 3 ns (which
>> would be amazing, given that just allocating a pid requires at
>> atomic operation), it would take well over 1000 years for highpid to
>> wrap.
>>
>> For CRIU's benefit, the next highpid can be set by a privileged
>> user.
>>
>> NB: The sysctl stuff only works on 64-bit systems.  If the approach
>> looks good, I'll fix that somehow.
>>
>> Signed-off-by: Andy Lutomirski 
>> ---
>>
>> If this goes in, there's plenty of room to add new interfaces to
>> make this more useful.  For example, we could add a fancier tgkill
>> that adds and validates hightgid and highpid, and we might want to
>> add a syscall to read one's own hightgid and highpid.  These would
>> be quite useful for pidfiles.
>>
>> David, would this be useful for kdbus?
>>
>> CRIU people: will this be unduly difficult to support in CRIU?
>>
>> If you all think this is a good idea, I'll fix the sysctl stuff and
>> document it.  It might also be worth adding "Hightgid" to status.
>>
>>  fs/proc/array.c   |  5 -
>>  include/linux/pid.h   |  2 ++
>>  include/linux/pid_namespace.h |  1 +
>>  kernel/pid.c  | 19 +++
>>  kernel/pid_namespace.c| 22 ++
>>  5 files changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/proc/array.c b/fs/proc/array.c
>> index cd3653e4f35c..f1e0e69d19f9 100644
>> --- a/fs/proc/array.c
>> +++ b/fs/proc/array.c
>> @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct 
>> pid_namespace *ns,
>> int g;
>> struct fdtable *fdt = NULL;
>> const struct cred *cred;
>> +   const struct upid *upid;
>> pid_t ppid, tpid;
>>
>> rcu_read_lock();
>> @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, 
>> struct pid_namespace *ns,
>> if (tracer)
>> tpid = task_pid_nr_ns(tracer, ns);
>> }
>> +   upid = pid_upid_ns(pid, ns);
>> cred = get_task_cred(p);
>> seq_printf(m,
>> "State:\t%s\n"
>> "Tgid:\t%d\n"
>> "Ngid:\t%d\n"
>> "Pid:\t%d\n"
>> +   "Highpid:\t%llu\n"
>> "PPid:\t%d\n"
>> "TracerPid:\t%d\n"
>> "Uid:\t%d\t%d\t%d\t%d\n"
>> @@ -183,7 +186,7 @@ static inline void task_state(struct seq_file *m, struct 
>> pid_namespace *ns,
>> get_task_state(p),
>> task_tgid_nr_ns(p, ns),
>> task_numa_group_id(p),
>> -   pid_nr_ns(pid, ns),
>> +   upid ? upid->nr : 0, upid ? upid->highnr : 0,
>> ppid, tpid,
>> from_kuid_munged(user_ns, cred->uid),
>> from_kuid_munged(user_ns, cred->euid),
>> diff --git a/include/linux/pid.h b/include/linux/pid.h
>> index 23705a53abba..ece70b64d04c 100644
>> --- a/include/linux/pid.h
>> +++ b/include/linux/pid.h
>> @@ -51,6 +51,7 @@ struct upid {
>> /* Try to keep pid_chain in the same cacheline as nr for find_vpid */
>> int nr;
>> struct pid_name

Re: [systemd-devel] [RFC PATCH] proc, pidns: Add highpid

2014-12-01 Thread Andy Lutomirski
On Mon, Dec 1, 2014 at 8:39 AM, Konstantin Khlebnikov  wrote:
> On Mon, Dec 1, 2014 at 7:21 PM, Andy Lutomirski  wrote:
>> On Sun, Nov 30, 2014 at 11:03 PM, Konstantin Khlebnikov
>>  wrote:
>>> Hmm. What about per-task/thread UUID? exported via separate file: 
>>> /proc/PID/uuid
>>> It could be created at the first access, thus this wouldn't shlowdown 
>>> clone().
>>> Also it could be droped at execve(), so it'll describe execution
>>> context more precisely than pid.
>>>
>>
>> I'd be all for this if it weren't for two issues:
>>
>> 1. This thing needs to work as soon as init is started, and we can't
>> reliably generate random numbers that early.
>>
>> 2. Migration / CRIU is rather fundamentally at odds with making
>> anything universally unique, as opposed to just per-user-ns.
>>
>> So, given that I don't think we can actually provide a universally
>> unique pid-like thing, I'd rather not even try.
>
> Well... another idea: what about pid generation counter? Of course it
> should be per-pid-ns.
> As I see struct upid has nice 32-bit hole next to 'nr' field. (on
> 64-bit systems)
>

I thought about that, but it has two issues:

1. Implementing it will be pain.  The pid allocation algorithm is
already complicated, and it wasn't obvious to me how to accurately
detect wraparound without racing against other wrapping users.

2. I don't think it will be reliable.  Suppose that there are pid_max
- 1 processes.  One of them can repeatedly clone and exit,
incrementing the generation counter each time.  After 2^32 iterations,
which won't take all that long, the pid generation will wrap.

--Andy

>>
>> That being said, I want to rework this a little bit.  I think that
>> highpid should be restricted to the range 2^32 through 2^64 - 4096.
>> The former prevents anyone from confusing highpid with regular pid,
>> and the latter means that we don't need to worry about confusion
>> between errors and valid highpids (e.g. -1 will never be a highpid).
>>
>> Implementing that will be only mildly annoying.
>>
>> --Andy
>>
>>> On Sat, Nov 29, 2014 at 2:05 AM, Andy Lutomirski  
>>> wrote:
 Pid reuse is common, which means that it's difficult or impossible
 to read information about a pid from /proc without races.

 This introduces a second number associated with each (task, pidns)
 pair called highpid.  Highpid is a 64-bit number, and, barring
 extremely unlikely circumstances or outright error, a (highpid, pid)
 will never be reused.

 With just this change, a program can open /proc/PID/status, read the
 "Highpid" field, and confirm that it has the expected value.  If the
 pid has been reused, then highpid will be different.

 The initial implementation is straightforward: highpid is simply a
 64-bit counter. If a high-end system can fork every 3 ns (which
 would be amazing, given that just allocating a pid requires at
 atomic operation), it would take well over 1000 years for highpid to
 wrap.

 For CRIU's benefit, the next highpid can be set by a privileged
 user.

 NB: The sysctl stuff only works on 64-bit systems.  If the approach
 looks good, I'll fix that somehow.

 Signed-off-by: Andy Lutomirski 
 ---

 If this goes in, there's plenty of room to add new interfaces to
 make this more useful.  For example, we could add a fancier tgkill
 that adds and validates hightgid and highpid, and we might want to
 add a syscall to read one's own hightgid and highpid.  These would
 be quite useful for pidfiles.

 David, would this be useful for kdbus?

 CRIU people: will this be unduly difficult to support in CRIU?

 If you all think this is a good idea, I'll fix the sysctl stuff and
 document it.  It might also be worth adding "Hightgid" to status.

  fs/proc/array.c   |  5 -
  include/linux/pid.h   |  2 ++
  include/linux/pid_namespace.h |  1 +
  kernel/pid.c  | 19 +++
  kernel/pid_namespace.c| 22 ++
  5 files changed, 44 insertions(+), 5 deletions(-)

 diff --git a/fs/proc/array.c b/fs/proc/array.c
 index cd3653e4f35c..f1e0e69d19f9 100644
 --- a/fs/proc/array.c
 +++ b/fs/proc/array.c
 @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, 
 struct pid_namespace *ns,
 int g;
 struct fdtable *fdt = NULL;
 const struct cred *cred;
 +   const struct upid *upid;
 pid_t ppid, tpid;

 rcu_read_lock();
 @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, 
 struct pid_namespace *ns,
 if (tracer)
 tpid = task_pid_nr_ns(tracer, ns);
 }
 +   upid = pid_upid_ns(pid, ns);
 cred = get_task_cred(p);
 seq_printf(m,
 "State:\t%s\n"

Re: [systemd-devel] [PATCH 2/5] Add a machine_id_commit call to commit on disk a, transient machine-id

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 12:35, Didier Roche (didro...@ubuntu.com) wrote:

>  
> +static int is_on_temporary_fs(int fd) {
> +struct statfs s;
> +
> +if (fstatfs(fd, &s) < 0)
> +return -errno;
> +
> +return F_TYPE_EQUAL(s.f_type, TMPFS_MAGIC) ||
> +   F_TYPE_EQUAL(s.f_type, RAMFS_MAGIC);
> +}

This should really move to util.[ch] I figure, and reuse
is_temporary_fs() that we already have there.

> +
> +int machine_id_commit(const char *root) {
> +const char *etc_machine_id;
> +_cleanup_close_ int fd = -1;
> +_cleanup_close_ int initial_mntns_fd = -1;
> +struct stat st;
> +char id[34]; /* 32 + \n + \0 */
> +int r;
> +
> +if (isempty(root))
> +etc_machine_id = "/etc/machine-id";
> +else {
> +etc_machine_id = strappenda(root, "/etc/machine-id");
> +path_kill_slashes((char*) etc_machine_id);
> +}
> +
> +r = path_is_mount_point(etc_machine_id, false);
> +if (r <= 0) {
> +log_error("We expected that %s was an independent mount.", 
> etc_machine_id);
> +return r < 0 ? r : -ENOENT;
> +}

I think this should really work in idempotent style, i.e. so that you
exit cleanly if the thing is already a proper file.

> +
> +/* read existing machine-id */
> +fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY);
> +if (fd < 0) {
> +log_error("Cannot open %s: %m", etc_machine_id);
> +return -errno;
> +}
> +if (fstat(fd, &st) < 0) {
> +log_error("fstat() failed: %m");
> +return -errno;
> +}
> +if (!S_ISREG(st.st_mode)) {
> +log_error("We expected %s to be a regular file.", 
> etc_machine_id);
> +return -EISDIR;
> +}
> +r = get_valid_machine_id(fd, id);
> +if (r < 0) {
> +log_error("We didn't find a valid machine-id.");
> +return r;
> +}
> +
> +r = is_on_temporary_fs(fd);
> +if (r <= 0) {
> +log_error("We expected %s to be a temporary file system.", 
> etc_machine_id);
> +return r;
> +}
> +
> +fd = safe_close(fd);
> +
> +/* store current mount namespace */
> +r = namespace_open(0, NULL, &initial_mntns_fd, NULL, NULL);
> +if (r < 0) {
> +log_error("Can't fetch current mount namespace: %s", 
> strerror(r));
> +return r;
> +}
> +
> +/* switch to a new mount namespace, isolate ourself and unmount 
> etc_machine_id in our new namespace */
> +if (unshare(CLONE_NEWNS) == -1) {
> + log_error("Not enough privilege to switch to another 
> namespace.");
> + return EPERM;
> +}
> +
> +if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) < 0) {
> + log_error("Couldn't make-rslave / mountpoint in our private 
> namespace.");
> + return EPERM;
> +}
> +
> +r = umount(etc_machine_id);
> +if (r < 0) {
> +log_error("Failed to unmount transient %s file in our 
> private namespace: %m", etc_machine_id);
> +return -errno;
> +}
> +
> +/* update a persistent version of etc_machine_id */
> +fd = open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444);
> +if (fd < 0) {
> +log_error("Cannot open for writing %s. This is mandatory to 
> get a persistent machine-id: %m",
> +  etc_machine_id);
> +return -errno;
> +}
> +if (fstat(fd, &st) < 0) {
> +log_error("fstat() failed: %m");
> +return -errno;
> +}
> +if (!S_ISREG(st.st_mode)) {
> +log_error("We expected %s to be a regular file.", 
> etc_machine_id);
> +return -EISDIR;
> +}
> +
> +r = write_machine_id(fd, id);
> +if (r < 0) {
> +log_error("Cannot write %s: %s", etc_machine_id, 
> strerror(r));
> +return r;
> +}

Since you prepared the original patch, we improved the loggic logic of
systemd. It would be great if you would update the patch to make use
of it. In particular, this means avoiding strerror() for cases like
the above (because it is not thread-safe to use). Instead use the new
log_error_errno() that takes the error value as first parameter, and
then makes sure that %m actually evaluates to the error string for
that error. Also it will then return the error, so that you can
simplify the four lines above into:

if (r < 0) 
return log_error_errno(r, "Cannot write %s: %m",
etc_machine_id);

> +fd = safe_close(fd);
> +
> +/* return to initial namespace and proceed a lazy tmpfs unmount */
> +r = namespace_enter(-

Re: [systemd-devel] [PATCH 1/5] Factorize some machine-id-setup functions to be reused

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 12:35, Didier Roche (didro...@ubuntu.com) wrote:

>  
> +static int get_valid_machine_id(int fd, char id[34]) {
> +assert(fd >= 0);
> +assert(id);
> +
> +if (loop_read(fd, id, 33, false) == 33 && id[32] == '\n') {
> +id[32] = 0;
> +
> +if (id128_is_valid(id)) {
> +id[32] = '\n';
> +id[33] = 0;
> +return 0;
> +}
> +}
> +
> +return -EINVAL;
> +}

As a matter of coding style we try hard to avoid functions that clobber
their parameters if they fail. Please follow the same scheme 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] [PATCH 3/5] Introduce machine-id-commit binary

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 12:35, Didier Roche (didier.ro...@canonical.com) wrote:

This one looks flawless to me!

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 5/5] machine-id-commit: add man pages

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 12:36, Didier Roche (didro...@ubuntu.com) wrote:

The last two look great too!

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] rules: rerun vconsole-setup when switching from vgacon to fbcon

2014-12-01 Thread Lennart Poettering
On Mon, 01.12.14 17:25, Ivan Shapovalov (intelfx...@gmail.com) wrote:

> > To me it appears to be quite OK to be run inside a udev rule.
> 
> Well, maybe it is OK to be run from an udev rule, but it is still an
> inconsistency between running that binary on boot (via a unit) and
> running the same binary when a framebuffer console is added (directly)...

Yeah, we do that a couple of times however. For example the alsa
volume restor tools are both run from an udev rule and as unit. The
latter is important since /var (where they store their data) is not
available when udev starts probing things...

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] Unbounded journal~ file accumulation

2014-12-01 Thread Moyer, Keith
On Wed, Nov 27, 2014 at 09:21:00AM -0500, Zbigniew Jedrzejewski-Szmek wrote:
> There were some fixes in this area. 3bfd4e0c6341b0ef946d2198f089743fa99e0a97
> might fix the unbounded size.

Indeed, it does!  Applying this change on top of the Debian jessie version of 
v215 cleaned it right up on each bootup.  Thanks for locating that commit.

> You can make the files smaller, ...

See below.

On Sat, Nov 29, 2014 at 05:44:14AM -0500, Lennart Poettering wrote:
> There's now "journalctl --vacuum-size=23M" that removes journal files
> until the disk usage is under the specified limit. It removes both 
> the clean and unclean journal files.

I'll keep that in mind for when we get a version of systemd that includes that 
option.  But, with the 3bfd4e patch, I think we're set.

> Well, very small journal files are not efficient. Neither in size 
> (because we compress identical fields into one instance if they keep 
> appearing), nor in access time (since access is optimized for few 
> small rather than many small files, and complexity grows O(n) with 
> number of files).

Thank you both for humoring me by discussing this.  I'll plan on leaving the 
size alone unless further problems arise.  The forced rotation (my workaround 
for the lack of vacuum on startup) was compounding the issue (at least two 4M 
files per boot instead of just one); without needing to do that, it's not 
nearly as bad.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Compatibility between D-Bus and kdbus

2014-12-01 Thread Djalal Harouni
On Wed, Nov 26, 2014 at 01:25:18AM +0100, Lennart Poettering wrote:
> On Tue, 25.11.14 12:01, Thiago Macieira (thi...@kde.org) wrote:
[...]

> > > > === KDBUS_ATTACH_NAMES ===
> > > > 
> > > > Documentation for metadata says that userspace must cope with some
> > > > metadata
> > > > not being delivered. Can we at least require that KDBUS_ATTACH_NAMES be
> > > > delivered if requested? If the cookie in the match rule isn't provided 
> > > > in
> > > > the message reception, having the source's names would help solve the
> > > > problem of the signal delivery.
> > > > 
> > > > The timestamp should also be mandatory.
> > > 
> > > Yes, they are mandatory. process credentials might be suppressed
> > > hwover, for example if they cannot be translated due to namespaces.
> > 
> > Thanks. Could you clarify in the docs?
> 
> Daniel, David? Could you add a note about this?
Ok pushed a note about namespace issues, thanks!

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


Re: [systemd-devel] [PATCH 1/2] networkd: Introduce Link Layer Discovery Protocol (LLDP)

2014-12-01 Thread Lennart Poettering
On Sun, 23.11.14 10:15, Susant Sahani (sus...@redhat.com) wrote:

> This patch introduces LLDP support to networkd. it implements the
> receiver side of the protocol.
> 
> The Link Layer Discovery Protocol (LLDP) is an industry-standard,
> vendor-neutral method to allow networked devices to advertise
> capabilities, identity, and other information onto a LAN. The Layer 2
> protocol, detailed in IEEE 802.1AB-2005.LLDP allows network devices
> that operate at the lower layers of a protocol stack (such as
> Layer 2 bridges and switches) to learn some of the capabilities
> and characteristics of LAN devices available to higher
> layer protocols.

Looks great! Would love to see this systemd! TOm, what's the plan
here, will you work with Susant to make sure this gets added? 

BTW, would be great to also expose this in networkctl
right-away. i.e. change networkd to write the learned info into the
state file, and then show this in "networkctl status".

Lennart

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


Re: [systemd-devel] [PATCH 2/2] networkd: integrate LLDP

2014-12-01 Thread Tomasz Torcz
On Sun, Nov 23, 2014 at 02:17:18PM +0100, Tom Gundersen wrote:
> On Sun, Nov 23, 2014 at 12:01 PM, Tomasz Torcz  wrote:
> > On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote:
> >> This patch integrates LLDP with networkd.
> >
> >   In Fedora, we already have LLDP receiver/broadcaster – ladvd.
> > It has this neat feature of abusing ifAlias by putting endpoint
> > name there.  This gives port information right at "ip l" output:
> >
> > 3: enp5s0:  mtu 1500 qdisc fq_codel state 
> > UP mode DEFAULT group default qlen 1000
> > link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff
> > alias connected to Core2-3b-24p (Fa0/7)
> >
> >   Would it be possible to implement this in networkd?
> 
> Definitely possible. However, any suggestions on how to avoid stepping
> on the toes of other abusers of ifAlias, or is ladvd simply ignoring
> this?

  I've enabled it in Fedora's ladvd almost three years ago, and
received no bugreports about breaking any other software.

  ladvd does not take any special precautions. I think it would
be safe to refrained from changing ifAlias if:
  - ifAlias is already set, AND
  - it doesn't start with "connected to "

-- 
Tomasz Torcz"Funeral in the morning, IDE hacking
xmpp: zdzich...@chrome.plin the afternoon and evening." - Alan Cox

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


Re: [systemd-devel] [PATCH 1/2] networkd: Introduce Link Layer Discovery Protocol (LLDP)

2014-12-01 Thread Tom Gundersen
On Mon, Dec 1, 2014 at 10:42 PM, Lennart Poettering
 wrote:
> On Sun, 23.11.14 10:15, Susant Sahani (sus...@redhat.com) wrote:
>
>> This patch introduces LLDP support to networkd. it implements the
>> receiver side of the protocol.
>>
>> The Link Layer Discovery Protocol (LLDP) is an industry-standard,
>> vendor-neutral method to allow networked devices to advertise
>> capabilities, identity, and other information onto a LAN. The Layer 2
>> protocol, detailed in IEEE 802.1AB-2005.LLDP allows network devices
>> that operate at the lower layers of a protocol stack (such as
>> Layer 2 bridges and switches) to learn some of the capabilities
>> and characteristics of LAN devices available to higher
>> layer protocols.
>
> Looks great! Would love to see this systemd! TOm, what's the plan
> here, will you work with Susant to make sure this gets added?

Yup, it is all good as far as I'm concerned, just thought I'd leave it
on the ML for some time to hopefully get some reviews before I go
ahead and merge it.

> BTW, would be great to also expose this in networkctl
> right-away. i.e. change networkd to write the learned info into the
> state file, and then show this in "networkctl status".

Yeah, we are just discussing this now. Susant is looking into it.

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


Re: [systemd-devel] [PATCH 2/2] networkd: integrate LLDP

2014-12-01 Thread Tom Gundersen
On Mon, Dec 1, 2014 at 11:01 PM, Tomasz Torcz  wrote:
> On Sun, Nov 23, 2014 at 02:17:18PM +0100, Tom Gundersen wrote:
>> On Sun, Nov 23, 2014 at 12:01 PM, Tomasz Torcz  wrote:
>> > On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote:
>> >> This patch integrates LLDP with networkd.
>> >
>> >   In Fedora, we already have LLDP receiver/broadcaster – ladvd.
>> > It has this neat feature of abusing ifAlias by putting endpoint
>> > name there.  This gives port information right at "ip l" output:
>> >
>> > 3: enp5s0:  mtu 1500 qdisc fq_codel state 
>> > UP mode DEFAULT group default qlen 1000
>> > link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff
>> > alias connected to Core2-3b-24p (Fa0/7)
>> >
>> >   Would it be possible to implement this in networkd?
>>
>> Definitely possible. However, any suggestions on how to avoid stepping
>> on the toes of other abusers of ifAlias, or is ladvd simply ignoring
>> this?
>
>   I've enabled it in Fedora's ladvd almost three years ago, and
> received no bugreports about breaking any other software.
>
>   ladvd does not take any special precautions. I think it would
> be safe to refrained from changing ifAlias if:
>   - ifAlias is already set, AND
>   - it doesn't start with "connected to "

As discussed in the other thread, we'll expose this info through
networkctl anyway, so it may not be necessary to use ifAlias (or are
there other benefits to this, than just to get it in ip(8) output?

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


[systemd-devel] reacting to unit failures (OnFailure)

2014-12-01 Thread Nekrasov, Alexander
Hello,

While converting from Upstart to SystemD, came upon this issue. Is this case 
not covered or am I missing something?

In Upstart, I can start a job when another job fails, and there's a $JOB 
variable that tells me what was the job that failed.

start on (stopped RESULT=failed PROCESS=post-stop)

script
echo "this just failed: $JOB"
end script

In SystemD I can register a unit to be started when another unit fails. But 
there seems to be no way of knowing what unit failed. Is that correct?

Thanks,
Alexander


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


Re: [systemd-devel] reacting to unit failures (OnFailure)

2014-12-01 Thread Lennart Poettering
On Mon, 01.12.14 17:10, Nekrasov, Alexander (alexander.nekra...@emc.com) wrote:

> Hello,
> 
> While converting from Upstart to SystemD, came upon this issue. Is this case 
> not covered or am I missing something?
> 
> In Upstart, I can start a job when another job fails, and there's a $JOB 
> variable that tells me what was the job that failed.
> 
> start on (stopped RESULT=failed PROCESS=post-stop)
> 
> script
> echo "this just failed: $JOB"
> end script
> 
> In SystemD I can register a unit to be started when another unit
> fails. But there seems to be no way of knowing what unit failed. Is
> that correct?

The idea is that you use "OnFailure=myfailureunit@%n.service". %n is
automatically replaced by the name of the unit you place this
in. Then, in the failure unit you can identify the instance again with
%i or %I.

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] bus-proxy: cloning smack label

2014-12-01 Thread Lennart Poettering
On Mon, 01.12.14 11:47, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote:

> Hello
> Could you take a look at my patch?

Sorry for the delay, I have a huge backlog of unreviewed patches, and
am now working through them!

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] bus-proxy: cloning smack label

2014-12-01 Thread Lennart Poettering
On Thu, 13.11.14 18:11, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote:

Looks pretty good, but I coudln't apply it. There's something wrong
with the patch the deletion/renaming of the service files doesn't
work. Did you create this patch with git-format-patch? 

>  if (is_unix) {
>  (void) getpeercred(in_fd, &ucred);
>  (void) getpeersec(in_fd, &peersec);
> +
> +#ifdef HAVE_SMACK
> +if (mac_smack_use()) {
> +if (peersec) {
> +
> +r = mac_smack_apply_pid(getpid(), peersec);
> +if (r < 0)
> +log_warning("Failed to set SMACK 
> label %s : %s", peersec, strerror(-r));
> +} else
> +log_warning("Invalid SMACK label");
> +
> +r = drop_capability(CAP_MAC_ADMIN);
> +if (r < 0)
> +log_warning("Failed to drop CAP_MAC_ADMIN: 
> %s", strerror(-r));
> +}
> +#endif
>  }

Hmm, could you make this bit a function of its own please?

> +m4_ifdef(`HAVE_SMACK',
> +Capabilities=cap_mac_admin=i
> +SecureBits=keep-caps
> +)

Hmm, it might be a good idea to also add some code to Makefile.am to
add the capability to the file after installation in case of
HAVE_SMACK. We used to do set a file cap like this on
systemd-detect-virt until a while back. 

See commit fdd25311706bd32580ec4d43211cdf4665d2f9de for details about
the setcap lines we removed back then. It should be easy to just readd
those lines and adapt them to apply to systemd-bus-proxyd instead!

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] serialization bug, swap bug, etc.

2014-12-01 Thread Lennart Poettering
On Thu, 20.11.14 09:13, Mantas Mikulėnas (graw...@gmail.com) wrote:

> ~ I'm also getting this on every reload:
> 
> systemd[1]: [/usr/lib/systemd/system/systemd-journald.service:24] Failed to
> parse capability in bounding set, ignoring: CAP_AUDIT_READ
> 
> I suppose I can ignore the message. I see that cap_audit_read was added to
> kernel 3.16, but unfortunately it doesn't exist in the current libcap
> release (libcap 2.24).

We currently need one actual operation from libcap, plus the
capability string tables. THat's all. Which really makes me wonder
whether we shouldn't simply do the table and the one syscall in
systemd and get rid of the dep, it's not really that complex, and we
have some caps code anyway in place...

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] TimeoutStopSec is ignored (regression)

2014-12-01 Thread Michael Chapman

On Mon, 1 Dec 2014, David Herrmann wrote:

Hi

On Sat, Nov 29, 2014 at 12:35 PM, Ross Lagerwall
 wrote:

Hi,

On recent versions of systemd, unit_kill_context doesn't set
wait_for_exit to true which means that service_enter_signal sends
SIGTERM, immediately moves into stop-sigkill and sends SIGKILL, ignoring
TimeoutStopSec and often killing processes without giving them a chance
to cleanup.

Reverting the following change, fixes the problem:

commit 1baccdda2e954214e0c5463d6ed8f06009b33c41
Author: Lennart Poettering 
Date:   Wed Feb 5 02:22:11 2014 +0100

core: don't wait for non-control/non-main processes when killing processes 
on the host either

Since the current kernel cgroup notification logic is easily confused by
existing subgroups, let's do the same thing as in containers before. and
just not wait for non-control and non-main processes.

This should be corrected as soon as we have sane cgroup notifications
from the kernel.


The commit-message and the comment it adds should answer your
question: The kernel cgroup API does not allow us to wait for
non-control processes. That is, we still honor TimeoutStopSec and
friends if we have to wait for the main-process and/or control process
(in those cases, wait_for_exit is still set to true). However, if
there are other processes remaining in the cgroup, we now ignore it.
See the commit you mentioned for an explanation.


What specifically would happen if wait_for_exit were kept true for other 
processes in the cgroup?


As far as I can see they would continue to be watched for SIGCHLD (since 
unit_watch_all_pids should have been previously called on the unit). PID 1 
may or may not get SIGCHLD for them, depending on whether they got 
reparented before they exited. Each time systemd gets a SIGCHLD, it can 
use unit_tidy_watch_pids to check the unit's entire PID list to see which 
ones are still present.


So at best we see the PIDs go away one by one in the cgroup, and we know 
when it's empty ourselves. At worst we don't see the last PID's SIGCHLD, 
so we have to wait the entire TimeoutStopSec interval before discovering 
that the cgroup is empty.


So I must be missing something important here, since everyone is stating 
emphatically that this is unsolveable until cgroup empty notifications are 
fixed. The only issue I can think of is that PIDs may be reused before the 
TimeoutStopSec interval completes.


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


Re: [systemd-devel] [PATCH] systemd: Fix wrong timestamps in rtc-in-local time mode.

2014-12-01 Thread Lennart Poettering
On Wed, 19.11.14 18:20, Chunhui He (hchun...@mail.ustc.edu.cn) wrote:

> systemd generates some timestamps before the very first call of 
> settimeofday().
> When we are in rtc-in-local time mode, these timestamps are wrong.
> 
> Affected timestamps are:
> Kernel, InitRD, Userspace, SecurityStart, SecurityFinish

I am not really convinced that we really should try to make this
work. rtc-in-local-time has so many issues, it really doesn't stop
here. If people make use of this, then this is what they get really,
and I am not sure we really should work around it. I mean, systemd
really isn't the only component which might query the clock this
early, in the initrd there might be a ton of other components too, and
it's not realistic to add similar kludges to them all.

In general: rtc-in-local-time is a compatibility hack, and we only
want to support it to the minimal level necessary for compatibility,
but not more. The proper fix for this problem I guess is to use
rtc-in-utc instead!

Sorry if that's disappointing,

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] networkd: disable tmpfiles and sysusers bits associated with networkd

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 09:30, Łukasz Stelmach (l.stelm...@samsung.com) wrote:

> It was <2014-11-21 pią 21:36>, when Lennart Poettering wrote:
> > On Fri, 21.11.14 17:07, Łukasz Stelmach (l.stelm...@samsung.com) wrote:
> >
> >> On a system configured without networkd and sysusers there still needs
> >> to be the unnecessary systemd-network user, otherwise systemd-tmpfiles
> >> fails to start.
> >> 
> >> Move information associated with networkd in tmpfiles.d and sysusers.d
> >> to separate files. Do not install it if netwrorkd is not enabled.
> >
> > In principle looks OK, but I'd prefer if we would write this out with
> > m4 (see etc.conf.m4) and keep it in the current files, rather than
> > split this up in numerous files.
> >
> > Especially in the case of /run/systemd/netif this actually matters: if
> > we split that out into its own tmpfiles snippet, then packagers would
> > most likely put that in its own RPM/DEB if they split out those
> > daemons. But this is not advisable in this case, as sd-network (which
> > will eventually be a public API of libsystems) needs the directory to
> > be around to install an inotify watch. If the directory doesn't exist,
> > and the API is used it will fail entirely, which is suboptimal, given
> > that networkd might be installed later on, and things should then just
> > start to work.
> 
> Will it be necessary for this directory to be owned by systemd-network
> even without networkd?

Yes. If networkd is compile-time enable the dir should exist and be
properly owned, even if it networkd is split off into a separate
binary package and currently not installed.

Your patch in the version Zbigniew commited looks correct in this
regard!

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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Tue, 18.11.14 12:11, Didier Roche (didro...@ubuntu.com) wrote:

> Fedora doesn't enable and start all units on package installation: there are
> some preset files, based on flavors, which is basically the policy stating
> which units to enable/disable by default. Some other units are always
> enabled (unless masked), by using symlinks directly shipped with the package
> like in /usr/lib/systemd/system/multi-users.target.wants/ for intance.
> Contrary to that, Debian/Ubuntu has the policy to enable/start services and
> facilities on package installation during postinst. This is done
> (indirectly) via "systemctl enable", which creates symlinks in
> /etc/systemd/system/*.wants/.

Distros really should use "systemctl preset" for this. It's what it is
for. 

If no preset policy is installed "systemctl preset" is actually
equivalent to "systemctl enable", but it allows admins to install
their own policy which then takes effect.

Really, distributions should *not* use "systemctl enable" in
postinst. It breaks presets for zero gain.
knowledgable admins.

If the admin should be able to disable/enable a service during normal
operation as part of his normal workflow then it should *not* be
enabled via symlinks in /usr, but instead carry [Install] and be
enabled by the postinst script via "systemctl preset".

> - We are mixing sys admin information and distro default choices in the same
> directories, and can't tell apart what is what.

You can actually. There's "systemctl preset-all" for example which
brings the system back into the exact choices of the upstream distro.

> We were thus thinking about having all default distro choices shipping
> target symlinks in /usr/lib, and having the administrator overrides this in
> /etc via systemctl. This could look similar to masking a unit, i. e. a
> symlink "/etc/.../wants.d/foo -> /dev/null" overrides
> "/usr/lib/.../wants.d/foo -> ../foo.service", and would be an explicit
> representation of "the admin does not want foo.service to autostart" in
> /etc.

Overriding symlinks with symlinks to different targets is difficult,
as systemd only really cares about the symlink names, and not so much
about the targets.

> However, we did notice the following: taking an unit, with an [Install]
> section and a symlink to enable in via that target in /usr/lib:
> - systemctl status  will report "disabled", where it's actually
> enabled and starting for that unit. This is a bug which should be fixed in
> any case, as "disabled" is outright wrong. On IRC it was suggested that
> those are "static" units, but then it should say at least that.

As mentioned above: units that carry [Install] and are also enabled
via /usr/lib/ are broken really. Either you use [Install] and thus ask
systemd to manage the symlinks in /etc for you via commands like
systemctl enable/disable/preset, or you say that systemctl shall not
manage the symlinks and instead create a static on in /usr/lib. But
having a unit that is both managed and unmanaged doesn't really make sense.

> - If we ln -s /dev/null /etc/<…>/<…>.wants/, then systemctl status
>  will display the unit as being enabled, and it will activated once
> the target is reached.  This is also counterintuitive, as usually this means
> to mask/completely disable the unit.

Yeah, symlinks in .wants/ are just about creating dependencies, not
about overriding units really...

> Part of the discussion on #systemd pointed out that the admin should then
> use systemctl mask . However, that means:

Hmm, "systemctl mask" is really nothing we should advertise to users
or admins. It's a last resort tool. Should not appear in normal workflows.

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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Tue, 18.11.14 13:01, Martin Pitt (martin.p...@ubuntu.com) wrote:

> We can certainly ship a preset of "enable *" to reflect the policy
> that in general services do get enabled by default. But this still
> leaves some issues:

No need to ship "enable *", btw. It's the implied default if no preset
file is found or no matching line is found in them. Or in other words:
the fedora default of "disable *" needs an explicit preset file, but
the Debian default of "enable *" is actually the upstream default
without preset file, too.

>  * This doesn't solve the problem of having these rather uninteresting
>and cluttering symlinks in /etc at all; the wants.d symlinks would
>still be as they are now, just the place that decides when to
>enable them changes.

If they are so uninteresting that there's no real benefit in allowing
them to be modified, then I'd really recommend to simply ship the
symlinks in /usr/lib, and not include an [Install] section. A lot of
systemd's own units are like that. For example, since disabling udev
or journald will only have the effect of making your system unbootable
we don't ship [Install] sections for them, and instead hook them in
statically.

> I. e. my question is not so much about being able to restore the
> default wants.d symlinks in /etc after a factory reset -- there are
> multiple ways how this can be done: with presets or iterating over the
> installed packages and re-enabling them (Debian also does some
> house-keeping which unit files got enabled by postinstall scripts,
> which can simply be replayed).

Note that "systemctl preset-all" erally just iterates throught unit
files that are installed and individually does the equivalent of
"systemctl preset". There's little magic in there...

> I'm interested in the reason for that. This basically cements the
> status quo that one *has* to have a gazillion links in /etc in order
> for your system to work, even if they are not at all specific to the
> particular system or represent a deviation from the default install.

It's not a gazillion really. It's 15 or so on my laptop 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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Tue, 18.11.14 14:40, Colin Guthrie (gm...@colin.guthr.ie) wrote:

> Well the "upstream blessed" RPM way is to call "%systemd_post" macro in
> your %post script, but (personally) I don't like this as it makes the
> implementation very much embedded into the RPMs so changing the upstream
> macro needs a full package rebuild.
> 
> In Mageia we do something similar but we shell out to a script
> instead.

The idea was to make "systemctl preset" generic enough so that it is
all we need to change should we want to change the effect of the RPM
macros one day, if you follow what I mean...

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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Tue, 18.11.14 14:10, Tom Gundersen (t...@jklm.no) wrote:

> > - We are mixing sys admin information and distro default choices in the same
> > directories, and can't tell apart what is what.
> 
> That is true. Could we perhaps improve on systemctl by printing
> "enabled (preset)"/"disable (preset)" for units that are in the
> default state? I know this does not change the fact that you have
> distro-default (via presets) links in /etc, but it should give the
> end-user a nicer experienc I guess?

Sounds like a good idea. Added to TODO list.

> My take on this is: make sure presets are applied on "firstboot"
> (which I think they are), so empty /etc works just fine, and then
> improve on systemctl to better show the distinction between 'enabled
> by default' or 'enabled by choice' (and same for 'disabled').

Yes systemd applies the presets on firstboot automatically, before
calculating the initial transactions.

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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Tue, 18.11.14 14:37, Martin Pitt (martin.p...@ubuntu.com) wrote:
> > We now have:
> > 
> > enabeld - [Install] section and symlink in /etc/**/*.wants.d/
> > disabled - [Install] section and no symlink in /etc/**/*.wants.d/
> > static - no [Install] section and symlink in /usr/lib/**/*.wants.d/
> > masked - symlink from the unit file-name to /dev/null in /etc
> > 
> > you want in addition:
> > 
> > preset (or something like that) - [Install] section and symlink in
> > /usr/lib/**/*.wants.d/

I am not totally opposed to allowing /dev/null symlinks in .wants.d/
subdirs, and considering them a way to override dependencies. Such a
"mask a dependency" concept would be OK.

> I'd call this just "enabled". It follows the same "/etc/ trumps /usr"
> schema that we have for udev rules, units, etc., i. e. an "enabled"
> symlink should be allowed to live in /etc, /usr, and maybe even /run
> (haven't though about whether this really makes sense, but perhaps for
> full consistency it should be allowed).

OK, let me get this right. You want an algorithm like this when
somebody invokes "systemctl enable":

a) if the unit file contains [Install], execute that, done
b) if the unit file does not contain [Install], then delete any
   /dev/null symlinks in /etc/systemd/*.{wants,requires}.d/* of the
   same name.

Then, "systemctl disable" should do this in your opinion:

a) if the unit file contains [Install] remove all symlinks in
   /etc/systemd/*.{wants,requires}.d/* to it.
b) if it doesn't, then *add* new symlinks to /dev/null in all
   .{wants,requires}.d/ directories where symlinks exist for it in
   /usr?

Did I get this right?

I am not convinced this is really a good idea though, as with this
scheme package changes might reenable a unit that is otherwise
disabled. Also, I kind like the fact that there's currently a clean
error message generated when people try to disable a unit that is part
of the OS itself. The scheme you propose would degrade that case to a
NOP however, right?

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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Tue, 18.11.14 16:09, Michael Biebl (mbi...@gmail.com) wrote:

> 2014-11-18 15:59 GMT+01:00 Colin Guthrie :
> > Didier Roche wrote on 18/11/14 13:58:
> >> This would be maybe a nice way for the admin to know what's coming from
> >> a distribution default or not. However, let's say I want to ensure that
> >> ssh will always be available on my server, I would (even if it's in my
> >> server preset) then systemctl enable openssh, no matter whatever future
> >> preset updates does (like disable it in the next batch upgrade).
> >
> > For the avoidance of doubt, I believe that running systemctl preset
> > should only ever happen on *first* install, never on upgrade or such like.
> >
> 
> And what are you going to do, if the unit file changes?
> Say v1  had
> 
> [Install]
> WantedBy=multi-user.target
> 
> and version B has
> [Install]
> WantedBy=foo.target

Package installs should probably not try to do something about this
case, just leave things as is.

I mean, let's not forget that admins should be able to define their
own targets and then enabled units in them and disable them
elsewhere. Package upgrades should not manipulate that. The first
installation of a package should enable a unit if that's what the
preset policy says, but from that point on the configuration is admin
configuration and not be changed anymore automatically.

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] /usr vs /etc for default distro units enablement

2014-12-01 Thread Lennart Poettering
On Fri, 28.11.14 11:15, Didier Roche (didro...@ubuntu.com) wrote:

> The distribution comes preinstalled with one dm, enable * -> enable it, have
> the Alias=display-manager.service picking the right one.
> However, let's say the user installed then another dm, what happens? Both
> will be enabled if we systemctl preset  (as the discussion was
> to remove our debian enable  that was conditioned on the
> postinst).

"systemctl preset" will fail if there are already conflicting
symlinks. Hence the first installed DM wins, the second loses.

> I don't think we should have systemctl preset  running under
> any condition as a wipe of /etc and then "systemctl preset-all" would give a
> potential different result (I'm not even sure how this will work with those
> alias, the first matching the alias wins and get the symlinks?)

Dont follow? "wipe"?

> We can of course have an ubuntu-desktop.preset which disables all dms by
> lightdm, and an ubuntu-gnome.preset which disables all dms but gdm (and
> having those settings conflicting with each other), but it's seems that for
> every aliases, we need to maintain such a list (as we enable * by
> default)?

Not following here. Different flavours of Ubuntu should probably just
ship different preset files. (Or well, the main ubuntu should ship one
that enables lightdm, and then the gnome flavour ship another preset
file, with a lower name, tht overries the lightdm line, and enables
gdm instead).

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] localed: forward xkbcommon errors

2014-12-01 Thread Lennart Poettering
On Mon, 01.12.14 12:04, Jan Synacek (jsyna...@redhat.com) wrote:

> Thinking about it again, I don't think that it's a good idea to pass
> those errors to the client...
> 
> A few examples:
> 
> # localectl set-x11-keymap QQ
> 
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: 
> libxkbcommon: Couldn't find file "symbols/QQ" in include paths
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: 
> libxkbcommon: 1 include paths searched:
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: 
> libxkbcommon: /usr/share/X11/xkb
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: 
> libxkbcommon: Abandoning symbols file "(unnamed)"
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: 
> libxkbcommon: Failed to compile xkb_symbols
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: 
> libxkbcommon: Failed to compile keymap
> Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: Cannot 
> compile XKB keymap for new x11 keyboard layout ('' / 'QQ' / '' / ''): Invalid 
> argument
> 
> # localectl set-x11-keymap us pc105 QQ
> 
> Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: 
> libxkbcommon: Couldn't process include statement for 'us(QQ)'
> Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: 
> libxkbcommon: Abandoning symbols file "(unnamed)"
> Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: 
> libxkbcommon: Failed to compile xkb_symbols
> Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: 
> libxkbcommon: Failed to compile keymap
> Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: Cannot 
> compile XKB keymap for new x11 keyboard layout ('pc105' / 'us' / 'QQ' / ''): 
> Invalid argument
> 
> Those errors are just stupid. Maybe they would make sense if there was
> also libxkbcommon code in front of me... But from the user's
> perspective, they say nothing useful. I added the prefix so the errors
> at least have some context.
> 
> My original patch would say something like "No such layout 'QQ'" in the
> first case and "No variant 'QQ' for layout 'us'" in the second.
> 
> So, is it worth logging this information? Any other ideas?

Maybe log it, but downgrade it to LOG_DEBUG?

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] [BUG] too many rfkill services

2014-12-01 Thread Lennart Poettering
On Sun, 23.11.14 12:34, Andrei Borzenkov (arvidj...@gmail.com) wrote:

> В Fri, 21 Nov 2014 01:26:36 +0100
> Lennart Poettering  пишет:
> 
> > On Thu, 20.11.14 19:56, Lukasz Stelmach (stl...@poczta.fm) wrote:
> > 
> > > I talked to the kernel guys at my office and they told me that it is
> > > quite usual (at least for USB devices, and my wlan and bt are USB)
> > > that devices are stopped and unregistered in the kernel before
> > > a system is suspended end reported as completely new ones
> > > with increased numbers after machine resumes.
> > 
> > So, I have now added some code that adds BindsTo= for the device unit
> > to the service. This won't fix much though, as the service is likely
> > to fail in ExecStop= because it cannot find the device anymore.
> > 
> 
> Yes, you are right. It accumulates the same services but now failed
> instead of active.
> 
> Hmm ... should not systemd inform service that device it is BoundTo is
> gone? I mean, while service may need to do some cleanup in this case,
> it is obvious that it cannot access device which no more exists. This
> would allow graceful exit, instead of returning error. 

Well, it's racy really. Of course systemd sends SIGTERM before
shutting down a service, but this is async...

Lennart

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


Re: [systemd-devel] [PATCH v2] localed: validate set-x11-keymap input

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 15:21, David Herrmann (dh.herrm...@gmail.com) wrote:

> I now pushed something to -git, see:
> 
> commit d4f5a1f47dbd04f26f2ddf951c97c4cb0ebbbe62
> Author: David Herrmann 
> Date:   Mon Nov 24 15:12:42 2014 +0100
> 
> localed: validate xkb keymaps
> 
> I think duplicating the xkb-parsers is something we should avoid. Ran
> Benita was even working on a v2 format to drop all the legacy bits
> no-one uses in the old, crufty xkb format. I dislike having a fixed
> parser in systemd. Sure, our --list-layouts option needs to do this,
> but maybe we can drop that some day, too.
> 
> I pushed a fix to test-compile keymaps on set-x11-keymap calls. So
> far, I only print a warning if it fails. Please feel free to post
> patches to extend this. For instance, I think we might want to do that
> in localectl (maybe even forward the xkb-logs then) and fail hard if
> the compilation fails (even though I dislike --force options..).

Wouldn't it make sense to refuse keymaps we cannot compile from the
localed side? I kinda prefer doing the validity checks on the server
side. Doing enumeration (which is primarily relevant fo cmdline
completion) on the client side is OK, but validity checking should
really be done on the server side.

I think it would be great if we'd generate a simply nice error message
if the compile test failed (just some error "Keyboard map could not be
compiled, refusing." or so).

Lennart

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


Re: [systemd-devel] [PATCH v2] localed: validate set-x11-keymap input

2014-12-01 Thread Lennart Poettering
On Mon, 24.11.14 13:31, Jan Synacek (jsyna...@redhat.com) wrote:

> Lennart Poettering  writes:
> > On Fri, 14.11.14 12:42, Jan Synacek (jsyna...@redhat.com) wrote:
> >> +if (look_for == LAYOUTS) {
> >> +Set *s;
> >> +char *k;
> >> +Iterator i;
> >> +/* XXX: Is there a better way to sort Hashmap keys? */
> >> +_cleanup_strv_free_ char **tmp = NULL;
> >> +
> >> +HASHMAP_FOREACH_KEY(s, k, keymap->x11_layouts, i)
> >> +if (strv_extend(&tmp, k) < 0)
> >> +(void) log_oom();
> >
> > There's hashmap_get_strv() for cases like this.
> 
> I need keys, not values. I didn't find any hashmap_get_strv() equivalent
> for keys.

Maybe add it then? I think this might become useful even beyond the
kbd mapping part one day.

> > Also, please neer invoke strv_extend() when appending to unbounded
> > arrays. It's slow! 
> 
> What is a preffered way to extend a strv? In this case, I know the final
> size, so it the array could manually be copied. I don't think that
> that's very nice, though.

Best case: you know the precise size of the array in the end. In that
case, simply use realloc()/realloc_multiply() to allocate the right
sized array, fill it in, done.

If you don#t know the size, but you guess that it might be quite a few
entries in there in the end and you might be adding a good chunk of
them, please use greedy_realloc(). It grows the array exponentially,
which is very beneficial performance-wise.

If you don't know the size, but you are pretty sure that it will never
gro to more than 10 entries or so, and that you are just going to
add a couple of them, then strv_extend()/strv_push()/strv_consume()
are OK.

> > Humm, when clearing up hashmaps please just write a loop that steals
> > the first entry, free it and then repeat. Iterating through a hashmap
> > while deleting its entries is unnecessary...
> 
> I need to free its entries *and* keys. I didn't find any function that I
> could use for that, apart from hashmap_free_free(), which crashes in
> my case.

while ((k = hashmap_first_key())) {
  v = hashmap_steal_first();
  free(v);
  free(k);
}

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] make systemd service takes cpu exclusively

2014-12-01 Thread Lennart Poettering
On Wed, 19.11.14 20:05, Andrei Borzenkov (arvidj...@gmail.com) wrote:

> В Tue, 18 Nov 2014 06:25:43 +
> "Cao, XinX"  пишет:
> 
> > Hi, Umut  & David,
> > 
> > My project needs the Graphical desktop to display on monitor as fast as 
> > possible, but I found lots of unrelated services( such as sound, network, 
> > ... ) are competing CPU even they are explicitly After graphical service. 
> > And this competion delays the startup of graphical desktop process. So, my 
> > opinion is to make graphical related programs runs first and  the other 
> > unrelated services only starts after graphical program finished startup. 
> 
> Let me guess. Your Graphical desktop is defined of simple or forking,
> right? So from systemd point of view service is started as soon as
> process is forked and it proceeds with starting further service defined
> After this one.

One minor correction: "forking" actually is supposed to provide
propery synchronization too, as daemons that use double-forking to
daemonize should return in the parent only after the daemon is fully
initialized. THis is in fact what classic sysv init scripts rely on to
properly serialize startup of services that need to talk to each
other.

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] [k]dbus: api, match replace and test extending

2014-12-01 Thread Lennart Poettering
On Mon, 17.11.14 12:31, Rui Miguel Silva (rmf...@gmail.com) wrote:

Heya,

> > >  - technical debt, if in the future the filter mechanism is change by
> > >other than bloom.
> > > so bloom maybe just be replaced with only generic filter could make more
> > > sense?
> > 
> > What do you mean by "only generic filter"?
> > 
> Maybe I did not explain myself well, what I mean is:
> Imagine that ahead we find that instead of bloom filtering mechanism, for
> example, cuckoo filters are more eficient. The api have the filter
> structs called struct kdbus_bloom_filter, my suggestion was to just change
> that to struct kdbus_filter (and no attach to filter specific
> implementation). Since they are very generic (generation and a data field)
> and for the kdbus it is just a check between a mask and a filter.

I had a closer look at cuckoo filters now. The lookup logic is quite
different from bloom filters and involves iterating through "entries"
of a bucket. Now, I am not convinced that Cuckoo filters are really
something we want to do in kdbus, but should we determine one day that
they in fact are, then the kernel side matching of filter against mask
needs to look very different anyway, with different data
structures. And in that case we really should define new items for
that, that should not overload the existing kdbus_bloom_filter
structures but be seperate, new structs.

Hope this makes sense,

Lennart

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