Re: [systemd-devel] [BUG] too many rfkill services

2014-11-23 Thread Andrei Borzenkov
В 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. 
___
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-11-23 Thread Tomasz Torcz
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?

-- 
Tomasz Torcz   RIP is irrevelant. Spoofing is futile.
xmpp: zdzich...@chrome.pl Your routes will be aggreggated. -- Alex Yuriev

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


Re: [systemd-devel] systemctl list-dependencies: consider BindsTo as well?

2014-11-23 Thread Peter Mattern

Thanks for fixing this.

May I suggest to adjust man systemctl accordingly, too?
Necessary parts would be the one about option --reverse and command 
list-dependencies.
As for the latter I'd suggest another change while you're at it. Phrase 
required and wanted units is rather ambiguous as it might make think 
of WantedBy and RequiredBy which don't apply here, if I didn't get it 
wrong. I for one think it would be better to simply state the parameters 
themselves, much as it's done with --reverse already, e. g. something 
like Show units stated by options Requires, Wants or BindsTo in unit NAME.

___
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-11-23 Thread Mantas Mikulėnas
On Sun, Nov 23, 2014 at 1: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?


lldpd has the same feature... In fact, I'm curious what advantages will
networkd's implementation have over the existing lldpd and ladvd?

-- 
Mantas Mikulėnas graw...@gmail.com
___
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-11-23 Thread Tom Gundersen
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?

We definitely want this information easily accessible through
networkctl, so maybe we won't need to touch ifAlias (though I'm
absolutely not opposed to the idea as long as we don't gratuitously
break other users).

Cheers,

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


Re: [systemd-devel] [PATCH v2] smack: introduce new SmackLabelAccess option

2014-11-23 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Nov 21, 2014 at 03:16:01PM +0900, WaLyong Cho wrote:
 In case of systemd has _ label and run as root, if a service file
 has User= option and the command line file has a special SMACK label
 then systemd will fail to access to given file. SMACK label is ignored
 for root uid processes. But if a service has a User= then systemd
 will call setresuid() in the child process. After then it no more root
 uid. So it should have some of accessable label for the command. To
 set the before the uid is changed, introduce new SmackLabelAccess=.

^ This provides the motivation. Please add a paragraph which
describes the functionality.

I'm still bothered by the name SmackLabelAccess.

To recap the discussion, SMACK64EXEC is set on a file and it is an
analogue of setuid bit, except that it changes the 'current' smack
label instead of the UID.  The analogy goes pretty far, since the
value set with User= may be overriden by the setuid bit on the file,
and SmackLabelAccess= may be be overriden by SMACK64EXEC on the file.

We use User= to set the user, so the name SmackLabel= would be the
most appropriate. But like Lennart said, SmackLabel= is already used
to label the socket file, so we need something different.  By process
of elimination you arrived as SmackLabelAccess=. I don't like the name
because Access is misleading, because it suggests that the label is
only used when starting the process. But the normal case is that
SMACK64EXEC is *not* set, so the process will run with this label
until the end.

Please just call it SmackProcessLabel.

 ---
  man/systemd.exec.xml  | 15 +++
  src/core/dbus-execute.c   | 19 +
  src/core/execute.c| 11 
  src/core/execute.h|  3 +++
  src/core/load-fragment-gperf.gperf.m4 |  7 +++--
  src/core/load-fragment.c  | 50 
 +++
  src/core/load-fragment.h  |  1 +
  src/shared/exit-status.h  |  1 +
  src/shared/smack-util.c   | 20 ++
  src/shared/smack-util.h   |  1 +
  10 files changed, 126 insertions(+), 2 deletions(-)
 
 diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
 index e9af4ab..5381946 100644
 --- a/man/systemd.exec.xml
 +++ b/man/systemd.exec.xml
 @@ -1138,6 +1138,21 @@
  /varlistentry
  
  varlistentry
 +
 termvarnameSmackLabelAccess=/varname/term
 +
 +listitemparaSet the SMACK security
 +label to access given executable file in
 +varnameExecStart=/varname.
If my analysis above is correct this should be reworded - it's not
just for access.

  This option only has effect with
 +varnameUser=/varname.
Is this entirely true? If User= is not given, and SMACK64EXEC is not set,
then this will determine the label of the process iiuc.

 Because, SMACK access checking is ignored
 +for root uid processes. If 
 varnameSmackLabelAccess=/varname is
 +specified with varnameUser=/varname, 
 forked child systemd process
 +set its /proc/$PID/attr/current to specified 
 label in
 +varnameSmackLabelAccess=/varname. This 
 directive is ignored if
 +SMACK is disabled. If prefixed by 
 literal-/literal, all errors
 +will be ignored./para/listitem

Please describe the functionality in higher level language,
what the option does. Don't go into the details of SMACK.

Please mention that empty rhs can be used to unset.

 +/varlistentry
 +
 +varlistentry
  
 termvarnameIgnoreSIGPIPE=/varname/term
  
  listitemparaTakes a boolean
 diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
 index 9276da4..0764a42 100644
 --- a/src/core/dbus-execute.c
 +++ b/src/core/dbus-execute.c
 @@ -508,6 +508,24 @@ static int property_get_apparmor_profile(
  return sd_bus_message_append(reply, (bs), 
 c-apparmor_profile_ignore, c-apparmor_profile);
  }
  
 +static int property_get_smack_label_access(
 +sd_bus *bus,
 +const char *path,
 +const char *interface,
 +const char *property,
 +sd_bus_message *reply,
 +void *userdata,
 +sd_bus_error *error) {
 +
 +ExecContext *c = userdata;
 +
 +assert(bus);
 +assert(reply);
 +assert(c);
 +
 +return sd_bus_message_append(reply, (bs), 
 c-smack_label_access_ignore, c-smack_label_access);
 +}
 +
  static int property_get_personality(
  sd_bus *bus,
 

Re: [systemd-devel] [PATCH v2 0/2] Empty environment variables in unit files work

2014-11-23 Thread Zbigniew Jędrzejewski-Szmek
On Thu, Nov 20, 2014 at 09:18:22PM +0100, Iago López Galeiras wrote:
 Clarified commit message (thanks Koen Kooi)
 
 Iago López Galeiras (2):
   test: support empty environment variables in unit files
   update TODO
 
  TODO  |  2 --
  src/test/test-unit-file.c | 22 ++
  2 files changed, 22 insertions(+), 2 deletions(-)
Applied. I squashed the two patches together though.

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


Re: [systemd-devel] systemd-run checks path on host before running on container

2014-11-23 Thread Peter Hutterer
On Sun, Nov 23, 2014 at 02:40:29AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
 On Sun, Nov 23, 2014 at 12:14:32AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
  On Sat, Nov 22, 2014 at 02:47:49PM +0100, David Herrmann wrote:
   Hi
   
   On Fri, Nov 21, 2014 at 5:00 AM, Peter Hutterer
   peter.hutte...@who-t.net wrote:
I was playing around with systemd-nspawn and systemd-run. The latter 
doesn't
seem to let me run a command that solely exists on the container.
simple way of reproducing: drop a file foo into the container, then on 
the
host run
   
systemd-run -M mycontainer /path/to/foo
   
I expected this to run foo on the container. It does, but checks for the
command to exist locally first and fails. A simple touch /path/to/foo; 
chmod
+x $_ is sufficient to bypass that check, but that feels somewhat odd.
  I pushed a partial fix which makes find_binary() ignore errors for
  non-local absolute paths. Something which checks the path remotely
  might be nicer, but I'm not sure if it's worth the hassle.
 
 Looking at this again, I think we should resolve the paths remotely.
 We could redefine the dbus call to do $PATH resolution on paths without any
 slashes, or we could a new call. I like redefining the existing one more.
 I don't think anyone will care.
 
 See also https://bugs.freedesktop.org/show_bug.cgi?id=86555, which is a 
 similar
 problem with systemd-nspawn.

from a user's perspective having systemd-run fail if the remote path doesn't
exist would certainly be nice. Except it'd be a change in behaviour and may
break scripts, but that's something you'd have to decide - I'm not affected
here :)

thanks for the quick fix

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


Re: [systemd-devel] [PATCH] dbus: return non-zero return value in the case that prefix won't match

2014-11-23 Thread Zbigniew Jędrzejewski-Szmek
On Thu, Nov 20, 2014 at 04:06:18PM +0100, Lukas Nykryn wrote:
 strv_extend returns 0 in the case of success which means that
 else if (bus_track_deserialize_item(m-deserialized_subscribed, l) == 0)
 log_warning(Unknown serialization item '%s', l);
 will be printed when value is added correctly.
Applied.

Zbyszek
___
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-11-23 Thread Zbigniew Jędrzejewski-Szmek
On Thu, Nov 20, 2014 at 09:13:02AM +0200, Mantas Mikulėnas wrote:
 ~ I'm getting this on every reload:
 
 systemd[1]: Unknown serialization item 'subscribed=:1.1'
Lukas' patch should fix that.

 Doesn't seem to break anything though.
 
 ~ 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).
Yeah, that's annoying. We could add a wrapper around cap_from_name...

 ~ If there are two .swap units for the same partition (one made by
 fstab-generator, another by gpt-generator), systemd tries to swapon it
 twice, resulting in swapon failed: Device or resource busy.
IIUC, the problem is that swapons are called concurently, and one fails.
If they were started sequentially, systemd would notice that they both
refer to the same device and make one of the follow the other. What
I'm guessing happens is that initially the units use different device
names, so they are not merged. Could you post both units (systemctl cat
would probably be best), so we can see what is going wrong?

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


[systemd-devel] [PATCH v2 2/5] mount: check options as well as fstype for network mounts

2014-11-23 Thread Chris Leech
When creating a new mount unit after an event on /proc/self/mountinfo,
check the mount options as well as the fstype to determine if this is a
remote mount that requires network access.

Signed-off-by: Chris Leech cle...@redhat.com
---
 src/core/mount.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/core/mount.c b/src/core/mount.c
index 74a1da8..81e9fde 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -63,16 +63,20 @@ static const UnitActiveState 
state_translation_table[_MOUNT_STATE_MAX] = {
 static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void 
*userdata);
 static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t 
revents, void *userdata);
 
+static bool mount_needs_network(const char *options, const char *fstype) {
+if (mount_test_option(options, _netdev))
+return true;
+
+if (fstype  fstype_is_network(fstype))
+return true;
+
+return false;
+}
+
 static bool mount_is_network(MountParameters *p) {
 assert(p);
 
-if (mount_test_option(p-options, _netdev))
-return true;
-
-if (p-fstype  fstype_is_network(p-fstype))
-return true;
-
-return false;
+return mount_needs_network(p-options, p-fstype);
 }
 
 static bool mount_is_bind(MountParameters *p) {
@@ -1403,8 +1407,7 @@ static int mount_add_one(
 if (m-running_as == SYSTEMD_SYSTEM) {
 const char* target;
 
-target = fstype_is_network(fstype) ? 
SPECIAL_REMOTE_FS_TARGET : SPECIAL_LOCAL_FS_TARGET;
-
+target = mount_needs_network(options, fstype) ?  
SPECIAL_REMOTE_FS_TARGET : SPECIAL_LOCAL_FS_TARGET;
 r = unit_add_dependency_by_name(u, UNIT_BEFORE, 
target, NULL, true);
 if (r  0)
 goto fail;
-- 
1.9.3

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


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

2014-11-23 Thread Chris Leech
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
---
 src/core/mount.c | 168 +++
 1 file changed, 158 insertions(+), 10 deletions(-)

diff --git a/src/core/mount.c b/src/core/mount.c
index 513dcec..52a4ba7 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -26,6 +26,7 @@
 #include signal.h
 #include libmount.h
 #include sys/inotify.h
+#include libudev.h
 
 #include manager.h
 #include unit.h
@@ -44,6 +45,7 @@
 #include bus-errors.h
 #include exit-status.h
 #include def.h
+#include udev-util.h
 
 static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = {
 [MOUNT_DEAD] = UNIT_INACTIVE,
@@ -64,20 +66,166 @@ static const UnitActiveState 
state_translation_table[_MOUNT_STATE_MAX] = {
 static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void 
*userdata);
 static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t 
revents, void *userdata);
 
-static bool mount_needs_network(const char *options, const char *fstype) {
+static bool scsi_host_is(struct udev_device *host, const char *type) {
+_cleanup_free_ char *path;
+struct stat st;
+
+assert(host);
+assert(type);
+
+/* a type_host device created by the scsi transport class
+ * should exists for all the transport types looked for */
+
+if (asprintf(path, %s/%s_host, udev_device_get_syspath(host), type) 
 0) {
+log_oom();
+return false;
+}
+return stat(path, st) == 0  S_ISDIR(st.st_mode);
+}
+
+static bool scsi_host_is_network(struct udev_device *block) {
+struct udev_device *host = NULL;
+struct udev *udev;
+
+assert(block);
+udev = udev_device_get_udev(block);
+
+host = udev_device_get_parent_with_subsystem_devtype(block, scsi, 
scsi_host);
+if (!host)
+return false;
+
+/* iSCSI */
+if (scsi_host_is(host, iscsi))
+return true;
+
+/* FCoE appears as an FC host with a parent FCoE Ctlr device
+ * This will at least detect the software fcoe module and bnx2fc on 
kernels = 3.5 */
+if (scsi_host_is(host, fc)) {
+_cleanup_free_ char *fc_host_path = NULL;
+_cleanup_udev_device_unref_ struct udev_device *fc_host = NULL;
+struct udev_device *fcoe_ctlr = NULL;
+
+if (asprintf(fc_host_path, %s/fc_host/%s, 
udev_device_get_syspath(host), udev_device_get_sysname(host))  0) {
+log_oom();
+return false;
+};
+fc_host = udev_device_new_from_syspath(udev, fc_host_path);
+fcoe_ctlr = 
udev_device_get_parent_with_subsystem_devtype(fc_host, fcoe, fcoe_ctlr);
+if (fcoe_ctlr)
+return true;
+}
+
+return false;
+}
+
+static int dot_filter(const struct dirent *d) {
+if (streq(., d-d_name) || streq(.., d-d_name))
+return 0;
+return 1;
+}
+
+static inline bool is_partition(struct udev_device *block) {
+const char *devtype;
+
+devtype = udev_device_get_devtype(block);
+if (streq(devtype, partition)) {
+return true;
+}
+return false;
+}
+
+static bool block_needs_network(struct udev_device *block) {
+struct udev_device *parent = NULL;
+_cleanup_free_ char *slaves_path = NULL;
+_cleanup_free_ struct dirent **slaves = NULL;
+struct udev *udev;
+int n, i;
+bool rc = false;
+
+assert(block);
+udev = udev_device_get_udev(block);
+
+if (scsi_host_is_network(block))
+return true;
+
+/* if this is a partition, check the parent device */
+
+if (is_partition(block)) {
+parent = udev_device_get_parent(block);
+if (parent  block_needs_network(parent))
+return true;
+}
+
+/* if this block device has slaves check them as well
+ * this handles DM maps including LVM and multipath */
+
+if (asprintf(slaves_path, %s/slaves, 
udev_device_get_syspath(block))  0) {
+log_oom();
+return false;
+}
+
+n = scandir(slaves_path, slaves, dot_filter, alphasort);
+for (i = 0; i  n; i++) {
+_cleanup_udev_device_unref_ struct udev_device *slave = NULL;
+_cleanup_free_ char *newpath = NULL;
+_cleanup_free_ char *rp = NULL;
+
+if (asprintf(newpath, %s/%s, slaves_path, 
slaves[i]-d_name)  0) {
+log_oom();
+goto free_the_slaves;
+}
+rp = realpath(newpath, NULL);
+if (!rp) {
+   

[systemd-devel] [PATCH v2 0/5] mount unit handling improvments with libmount

2014-11-23 Thread Chris Leech
This patch set is an attempt to use libmount for mount unit handling, in order
to address the issues I raised with the _netdev option and remote-fs ordering
not working as expected.

In addition, given the feedback on my previous posting I went ahead and
implemented auto-detection of iSCSI and the FCoE drivers that need a
functioning network interface and userspace support.  It's somewhat based on
the transport detection from lsblk, but using libudev and with a different FCoE
check.

Changes in v2:
  Removed all existing mountinfo parsing code that was previously left as a
  fall-back for building without libmount, which is now a hard build
  requirement.

  Added iSCSI and FCoE auto-detection.

Chris Leech (5):
  mount: use libmount to enumerate /proc/self/mountinfo
  mount: check options as well as fstype for network mounts
  mount: monitor for utab changes with inotify
  mount: add remote-fs dependencies if needed after change
  mount: auto-detect iSCSI and FCoE as requiring network

 .travis.yml|   2 +-
 Makefile.am|   4 +-
 README |   1 +
 configure.ac   |  10 ++
 src/core/build.h   |   7 ++
 src/core/manager.c |   2 +-
 src/core/manager.h |   2 +
 src/core/mount.c   | 309 -
 8 files changed, 282 insertions(+), 55 deletions(-)

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


[systemd-devel] [PATCH v2 4/5] mount: add remote-fs dependencies if needed after change

2014-11-23 Thread Chris Leech
This is an attempt to add it the remote-fs dependencies to a mount unit
if the options change, like when the utab options are picked up after
mountinfo has already been processed.  It just adds the remote-fs
dependencies, leaving the local-fs ones in place.

With this change I always get mount units with proper remote-fs
dependencies when mounted with the _netdev option.

Signed-off-by: Chris Leech cle...@redhat.com
---
 src/core/mount.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/core/mount.c b/src/core/mount.c
index ef45115..513dcec 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1433,6 +1433,19 @@ static int mount_add_one(
 }
 }
 
+if (m-running_as == SYSTEMD_SYSTEM) {
+const char* target;
+
+target = mount_needs_network(options, fstype) ?  
SPECIAL_REMOTE_FS_TARGET : NULL;
+/* _netdev option may have shown up late, or on a
+ * remount. Add remote-fs dependencies, even though
+ * local-fs ones may already be there */
+if (target) {
+unit_add_dependency_by_name(u, UNIT_BEFORE, 
target, NULL, true);
+load_extras = true;
+}
+}
+
 if (u-load_state == UNIT_NOT_FOUND) {
 u-load_state = UNIT_LOADED;
 u-load_error = 0;
-- 
1.9.3

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


[systemd-devel] [PATCH v2 3/5] mount: monitor for utab changes with inotify

2014-11-23 Thread Chris Leech
Parsing the mount table with libmount races against the mount command,
which will handle the actual mounting before updating utab.  This means
the poll event on /proc/self/mountinfo can kick of a reparse in systemd
before the utab information is available.

This change adds in an additional event source using inotify to watch
for changes to utab.  It only watches for IN_MOVED_TO events, matching
libmount behavior of always overwriting this file using rename(2).

This does add a second pass through the mount table parsing when utab is
updated.

Signed-off-by: Chris Leech cle...@redhat.com
---
 src/core/manager.c |  2 +-
 src/core/manager.h |  2 ++
 src/core/mount.c   | 50 --
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 2bc1058..e71a96a 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -548,7 +548,7 @@ int manager_new(SystemdRunningAs running_as, bool test_run, 
Manager **_m) {
 
 m-idle_pipe[0] = m-idle_pipe[1] = m-idle_pipe[2] = m-idle_pipe[3] 
= -1;
 
-m-pin_cgroupfs_fd = m-notify_fd = m-signal_fd = m-time_change_fd = 
m-dev_autofs_fd = m-private_listen_fd = m-kdbus_fd = -1;
+m-pin_cgroupfs_fd = m-notify_fd = m-signal_fd = m-time_change_fd = 
m-dev_autofs_fd = m-private_listen_fd = m-kdbus_fd = m-utab_inotify_fd = -1;
 m-current_job_id = 1; /* start as id #1, so that we can leave #0 
around as null-like value */
 
 m-ask_password_inotify_fd = -1;
diff --git a/src/core/manager.h b/src/core/manager.h
index ab72548..bdeea14 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -182,6 +182,8 @@ struct Manager {
 /* Data specific to the mount subsystem */
 FILE *proc_self_mountinfo;
 sd_event_source *mount_event_source;
+int utab_inotify_fd;
+sd_event_source *mount_utab_event_source;
 
 /* Data specific to the swap filesystem */
 FILE *proc_swaps;
diff --git a/src/core/mount.c b/src/core/mount.c
index 81e9fde..ef45115 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -25,6 +25,7 @@
 #include sys/epoll.h
 #include signal.h
 #include libmount.h
+#include sys/inotify.h
 
 #include manager.h
 #include unit.h
@@ -1548,11 +1549,13 @@ static void mount_shutdown(Manager *m) {
 assert(m);
 
 m-mount_event_source = sd_event_source_unref(m-mount_event_source);
+m-mount_utab_event_source = 
sd_event_source_unref(m-mount_utab_event_source);
 
 if (m-proc_self_mountinfo) {
 fclose(m-proc_self_mountinfo);
 m-proc_self_mountinfo = NULL;
 }
+m-utab_inotify_fd = safe_close(m-utab_inotify_fd);
 }
 
 static int mount_get_timeout(Unit *u, uint64_t *timeout) {
@@ -1592,12 +1595,32 @@ static int mount_enumerate(Manager *m) {
 goto fail;
 }
 
+if (m-utab_inotify_fd  0) {
+m-utab_inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC);
+if (m-utab_inotify_fd  0)
+goto fail_with_errno;
+
+r = inotify_add_watch(m-utab_inotify_fd, /run/mount, 
IN_MOVED_TO);
+if (r  0)
+goto fail_with_errno;
+
+r = sd_event_add_io(m-event, m-mount_utab_event_source, 
m-utab_inotify_fd, EPOLLIN, mount_dispatch_io, m);
+if (r  0)
+goto fail;
+
+r = sd_event_source_set_priority(m-mount_utab_event_source, 
-10);
+if (r  0)
+goto fail;
+}
+
 r = mount_load_proc_self_mountinfo(m, false);
 if (r  0)
 goto fail;
 
 return 0;
 
+fail_with_errno:
+r = -errno;
 fail:
 mount_shutdown(m);
 return r;
@@ -1609,11 +1632,34 @@ static int mount_dispatch_io(sd_event_source *source, 
int fd, uint32_t revents,
 int r;
 
 assert(m);
-assert(revents  EPOLLPRI);
+assert(revents  (EPOLLPRI | EPOLLIN));
 
 /* The manager calls this for every fd event happening on the
  * /proc/self/mountinfo file, which informs us about mounting
- * table changes */
+ * table changes
+ * This may also be called for /run/mount events */
+
+if (fd == m-utab_inotify_fd) {
+char inotify_buffer[sizeof(struct inotify_event) + NAME_MAX + 
1];
+struct inotify_event *event;
+char *p;
+int rescan = 0;
+
+while ((r = read(fd, inotify_buffer, sizeof(inotify_buffer))) 
 0) {
+for (p = inotify_buffer; p  inotify_buffer + r; ) {
+event = (struct inotify_event *) p;
+/* only care about changes to utab, but we have
+ * to monitor the directory to reliably get
+ * notifications 

[systemd-devel] [PATCH v2 1/5] mount: use libmount to enumerate /proc/self/mountinfo

2014-11-23 Thread Chris Leech
This lets libmount add in user options from /run/mount/utab, like
_netdev which is needed to get proper ordering against remote-fs.target

Signed-off-by: Chris Leech cle...@redhat.com
---
 .travis.yml  |  2 +-
 Makefile.am  |  4 +++-
 README   |  1 +
 configure.ac | 10 +
 src/core/build.h |  7 ++
 src/core/mount.c | 65 ++--
 6 files changed, 52 insertions(+), 37 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 7e5251c..4ea2bc2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,7 +3,7 @@ compiler:
   - gcc
 before_install:
  - sudo apt-get update -qq
- - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev
+ - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libmount-dev libpam-dev libcryptsetup-dev libaudit-dev 
libacl1-dev libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev 
libqrencode-dev libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev
 script: ./autogen.sh  ./configure --enable-gtk-doc --enable-gtk-doc-pdf  
make V=1  sudo ./systemd-machine-id-setup  make check  make distcheck
 after_failure: cat test-suite.log
 notifications:
diff --git a/Makefile.am b/Makefile.am
index 3f9f3fa..042fb0e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1149,6 +1149,7 @@ libsystemd_core_la_CFLAGS = \
$(KMOD_CFLAGS) \
$(APPARMOR_CFLAGS) \
$(SECCOMP_CFLAGS) \
+   $(MOUNT_CFLAGS) \
-pthread
 
 libsystemd_core_la_LIBADD = \
@@ -1161,7 +1162,8 @@ libsystemd_core_la_LIBADD = \
$(AUDIT_LIBS) \
$(KMOD_LIBS) \
$(APPARMOR_LIBS) \
-   $(SECCOMP_LIBS)
+   $(SECCOMP_LIBS) \
+   $(MOUNT_LIBS)
 
 if HAVE_SECCOMP
 libsystemd_core_la_LIBADD += \
diff --git a/README b/README
index aefb349..84c6ed4 100644
--- a/README
+++ b/README
@@ -106,6 +106,7 @@ REQUIREMENTS:
 
 glibc = 2.14
 libcap
+libmount = 2.20 (from util-linux)
 libseccomp = 1.0.0 (optional)
 libblkid = 2.20 (from util-linux) (optional)
 libkmod = 15 (optional)
diff --git a/configure.ac b/configure.ac
index 2c8be53..ecd53a4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -427,6 +427,15 @@ fi
 AM_CONDITIONAL(HAVE_BLKID, [test $have_blkid = yes])
 
 # 
--
+have_libmount=no
+PKG_CHECK_MODULES(MOUNT, [ mount = 2.20 ],
+[AC_DEFINE(HAVE_LIBMOUNT, 1, [Define if libmount is available]) 
have_libmount=yes], have_libmount=no)
+if test x$have_libmount = xno; then
+AC_MSG_ERROR([*** libmount support required but libraries not found])
+fi
+AM_CONDITIONAL(HAVE_LIBMOUNT, [test $have_libmount = yes])
+
+# 
--
 have_seccomp=no
 AC_ARG_ENABLE(seccomp, AS_HELP_STRING([--disable-seccomp], [Disable optional 
SECCOMP support]))
 if test x$enable_seccomp != xno; then
@@ -1375,6 +1384,7 @@ AC_MSG_RESULT([
 efi: ${have_efi}
 kmod:${have_kmod}
 blkid:   ${have_blkid}
+libmount:${have_libmount}
 dbus:${have_dbus}
 nss-myhostname:  ${have_myhostname}
 gudev:   ${enable_gudev}
diff --git a/src/core/build.h b/src/core/build.h
index 24873ab..4f639ed 100644
--- a/src/core/build.h
+++ b/src/core/build.h
@@ -117,6 +117,12 @@
 #define _BLKID_FEATURE_ -BLKID
 #endif
 
+#ifdef HAVE_LIBMOUNT
+#define _LIBMOUNT_FEATURE_ +LIBMOUNT
+#else
+#define _LIBMOUNT_FEATURE_ -LIBMOUNT
+#endif
+
 #ifdef HAVE_ELFUTILS
 #define _ELFUTILS_FEATURE_ +ELFUTILS
 #else
@@ -152,6 +158,7 @@
 _LZ4_FEATURE_ \
 _SECCOMP_FEATURE_ \
 _BLKID_FEATURE_   \
+_LIBMOUNT_FEATURE_\
 _ELFUTILS_FEATURE_\
 _KMOD_FEATURE_\
 _IDN_FEATURE_
diff --git a/src/core/mount.c b/src/core/mount.c
index 8b787f6..74a1da8 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -24,6 +24,7 @@
 #include mntent.h
 #include sys/epoll.h
 #include signal.h
+#include libmount.h
 
 #include manager.h
 #include unit.h
@@ -1492,55 +1493,47 @@ fail:
 return r;
 }
 
+static inline void mnt_free_table_p(struct libmnt_table **tb) {
+mnt_free_table(*tb);
+}
+
+static inline void mnt_free_iter_p(struct libmnt_iter **itr) {
+mnt_free_iter(*itr);
+}
+
 static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {

Re: [systemd-devel] serialization bug, swap bug, etc.

2014-11-23 Thread Mantas Mikulėnas
On Mon, Nov 24, 2014 at 2:59 AM, Zbigniew Jędrzejewski-Szmek 
zbys...@in.waw.pl wrote:
 On Thu, Nov 20, 2014 at 09:13:02AM +0200, Mantas Mikulėnas wrote:
  ~ If there are two .swap units for the same partition (one made by
  fstab-generator, another by gpt-generator), systemd tries to swapon it
  twice, resulting in swapon failed: Device or resource busy.
 IIUC, the problem is that swapons are called concurently, and one fails.
 If they were started sequentially, systemd would notice that they both
 refer to the same device and make one of the follow the other. What
 I'm guessing happens is that initially the units use different device
 names, so they are not merged. Could you post both units (systemctl cat
 would probably be best), so we can see what is going wrong?

Right, after all, since .swap unit names directly depend on device names,
then it's pretty much guaranteed that the latter will differ...

What I have is:

---
# /run/systemd/generator/dev-disk-by\x2dpartlabel-swap.swap
# Automatically generated by systemd-fstab-generator

[Unit]
SourcePath=/etc/fstab
Documentation=man:fstab(5) man:systemd-fstab-generator(8)

[Swap]
What=/dev/disk/by-partlabel/swap
Options=rw
---

---
# /run/systemd/generator.late/dev-sda7.swap
# Automatically generated by systemd-gpt-auto-generator

[Unit]
Description=Swap Partition
Documentation=man:systemd-gpt-auto-generator(8)

[Swap]
What=/dev/sda7
---

-- 
Mantas Mikulėnas graw...@gmail.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2] smack: introduce new SmackLabelAccess option

2014-11-23 Thread WaLyong Cho
On 11/24/2014 02:36 AM, Zbigniew Jędrzejewski-Szmek wrote:
 On Fri, Nov 21, 2014 at 03:16:01PM +0900, WaLyong Cho wrote:
 In case of systemd has _ label and run as root, if a service file
 has User= option and the command line file has a special SMACK label
 then systemd will fail to access to given file. SMACK label is ignored
 for root uid processes. But if a service has a User= then systemd
 will call setresuid() in the child process. After then it no more root
 uid. So it should have some of accessable label for the command. To
 set the before the uid is changed, introduce new SmackLabelAccess=.
 
 ^ This provides the motivation. Please add a paragraph which
 describes the functionality.
 
 I'm still bothered by the name SmackLabelAccess.
 
 To recap the discussion, SMACK64EXEC is set on a file and it is an
 analogue of setuid bit, except that it changes the 'current' smack
 label instead of the UID.  The analogy goes pretty far, since the
 value set with User= may be overriden by the setuid bit on the file,
 and SmackLabelAccess= may be be overriden by SMACK64EXEC on the file.
 
 We use User= to set the user, so the name SmackLabel= would be the
 most appropriate. But like Lennart said, SmackLabel= is already used
 to label the socket file, so we need something different.  By process
 of elimination you arrived as SmackLabelAccess=. I don't like the name
 because Access is misleading, because it suggests that the label is
 only used when starting the process. But the normal case is that
 SMACK64EXEC is *not* set, so the process will run with this label
 until the end.
 
 Please just call it SmackProcessLabel.
 
I'm agree. I will change.

 ---
  man/systemd.exec.xml  | 15 +++
  src/core/dbus-execute.c   | 19 +
  src/core/execute.c| 11 
  src/core/execute.h|  3 +++
  src/core/load-fragment-gperf.gperf.m4 |  7 +++--
  src/core/load-fragment.c  | 50 
 +++
  src/core/load-fragment.h  |  1 +
  src/shared/exit-status.h  |  1 +
  src/shared/smack-util.c   | 20 ++
  src/shared/smack-util.h   |  1 +
  10 files changed, 126 insertions(+), 2 deletions(-)

 diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
 index e9af4ab..5381946 100644
 --- a/man/systemd.exec.xml
 +++ b/man/systemd.exec.xml
 @@ -1138,6 +1138,21 @@
  /varlistentry
  
  varlistentry
 +
 termvarnameSmackLabelAccess=/varname/term
 +
 +listitemparaSet the SMACK security
 +label to access given executable file in
 +varnameExecStart=/varname.
 If my analysis above is correct this should be reworded - it's not
 just for access.
 
Yes, if the file has SMACK64EXEC then the label only be used for access
checking. But if that file has no SMACK64EXEC then the label is also set
as the subject of executed process. The label is used also for checking
so I'd like to just add when if the file has no SMACK64EXEC then the
executed process will have specified label.

  This option only has effect with
 +varnameUser=/varname.
 Is this entirely true? If User= is not given, and SMACK64EXEC is not set,
 then this will determine the label of the process iiuc.
 
You are totally right. I just focused on accessing to execute. As you
said, if the file has no SMACK64EXEC and SmackProcessLabel= is given
then the executed process will has given label.

I will change this.

 Because, SMACK access checking is ignored
 +for root uid processes. If 
 varnameSmackLabelAccess=/varname is
 +specified with varnameUser=/varname, 
 forked child systemd process
 +set its /proc/$PID/attr/current to 
 specified label in
 +varnameSmackLabelAccess=/varname. This 
 directive is ignored if
 +SMACK is disabled. If prefixed by 
 literal-/literal, all errors
 +will be ignored./para/listitem
 
 Please describe the functionality in higher level language,
 what the option does. Don't go into the details of SMACK.
 
OK, I will.

 Please mention that empty rhs can be used to unset.
 
Oh, I didn't imagine this. But I can't find some other proper operation.
So I think if rhs was empty then just do like the config was not given.


Really thanks to read.

WaLyong


 +/varlistentry
 +
 +varlistentry
  
 termvarnameIgnoreSIGPIPE=/varname/term
  
  listitemparaTakes a boolean
 diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
 index 9276da4..0764a42 100644
 ---