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 cle...@redhat.com
  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  k...@redhat.com
 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 lenn...@poettering.net 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 martin.p...@ubuntu.com
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
firstif (!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 cle...@redhat.com
   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
arnaud.gabo...@gmail.com 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
rosslagerw...@gmail.com 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 lenn...@poettering.net
 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 dh.herrm...@gmail.com 
   wrote:
On Wed, Nov 5, 2014 at 4:11 PM, Ray Strode halfl...@gmail.com 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  k...@redhat.com
 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
koc...@gmail.com 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 l...@amacapital.net 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 l...@amacapital.net
 ---

 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_namespace *ns;
 +   u64 highnr;
 struct hlist_node pid_chain;
  };

 @@ -170,6 +171,7 @@ static inline pid_t pid_nr(struct pid *pid)
  }

  pid_t 

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 koc...@gmail.com wrote:
 On Mon, Dec 1, 2014 at 7:21 PM, Andy Lutomirski l...@amacapital.net wrote:
 On Sun, Nov 30, 2014 at 11:03 PM, Konstantin Khlebnikov
 koc...@gmail.com 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 l...@amacapital.net 
 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 l...@amacapital.net
 ---

 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 

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(-1, initial_mntns_fd, -1, -1);
 +if (r  0) {
 +log_warning(Failed to switch back to initial mount 
 namespace. We'll keep transient %s file until next 

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 to...@pipebreaker.pl 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: BROADCAST,MULTICAST,UP,LOWER_UP 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 TorczFuneral 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
lenn...@poettering.net 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 to...@pipebreaker.pl 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 to...@pipebreaker.pl 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: BROADCAST,MULTICAST,UP,LOWER_UP 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
rosslagerw...@gmail.com 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 lenn...@poettering.net
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 unit 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/unit, then systemctl status
 unit 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 unit. 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 gm...@colin.guthr.ie:
  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 new_service (as the discussion was
 to remove our debian enable service 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 new_service 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 lenn...@poettering.net пишет:
 
  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 dh.herrm...@gmail.com
 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 lenn...@poettering.net 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 xinx@intel.com пишет:
 
  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