Re: [systemd-devel] systemd device appeared twice with different sysfs path
On Wed, May 13, 2015 at 06:15:24PM +0800, 李炎戌 wrote: Hello, everyone!I am a newer to here.Recently, I have a problem. Whenever I boot my gentoo linux, it output the infomation like following: [ 11.428530] systemd[1]: Device dev-disk-by\x2dpartlabel-Basic\x5cx20data\x5cx20partition.device appeared twice with dif ferent sysfs paths /sys/devices/pci:00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda3 and /sys/devices/pci :00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda4 Already ”fixedń in git, see http://cgit.freedesktop.org/systemd/systemd/commit/?id=5259bcf6a638d8d489db1ddefd55327aa15f3e51 This message is harmless and can be ignored. -- Tomasz Torcz 72-| 80-| xmpp: zdzich...@chrome.pl 72-| 80-| ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=
On 13 May 2015 at 10:15, Karel Zak k...@redhat.com wrote: On Wed, May 13, 2015 at 06:35:58AM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 20:37:15 +0200 Karel Zak k...@redhat.com пишет: On Tue, May 12, 2015 at 07:29:33PM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 18:04:50 +0200 Karel Zak k...@redhat.com пишет: Currently we have no way how to specify dependencies between fstab entries (or another units) in the /etc/fstab. It means that users are forced to bypass fstab and write .mount units manually. Actually we have. mkdir -p /etc/systemd/system/path-to-mount-point.mount.d cat /etc/systemd/system/path-to-mount-point.mount.d/deps.conf EOF [Unit] After=xxx Before=xxx Wants=xxx Requires=xxx EOF You miss the point -- keep all in fstab. I admit I do. Why? We want to keep in fstab bits and pieces that are common with other utilities. Which other tool needs to know systemd dependencies? Did you read the reference in the patch? It's not about systemd dependences, but about dependences between mount points. The fstab has been originally (by mount -a) serialized during boot. Now it's parallelized and in some cases it's bad thing and without extra configuration systemd is not able to understand the dependencies in fstab. It's admins' nightmare to require additional file somewhere in /etc/systemd to fix systemd fstab interpretation. BTW, we already have x-systemd stuff in fstab... +1 from me, this is needed. Back in the upstart / mountall case, dependencies were established between mount points, and failing that explicit ordering was established - as in each subsequent fstab stanza depended on the previous one. That was enough in general case, but it was at times un-expected that reordering entries resulted in wrong boot order. If ftab generator creates linear dependency chain, would that be enough without adding extra mount options or do you want generic/explicit dependencies as per your patch? -- Regards, Dimitri. Pura Vida! https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] systemd device appeared twice with different sysfs path
Hello, everyone!I am a newer to here.Recently, I have a problem. Whenever I boot my gentoo linux, it output the infomation like following: [ 11.428530] systemd[1]: Device dev-disk-by\x2dpartlabel-Basic\x5cx20data\x5cx20partition.device appeared twice with dif ferent sysfs paths /sys/devices/pci:00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda3 and /sys/devices/pci :00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda4 [ 13.027629] systemd[1]: Device dev-disk-by\x2dpartlabel-Basic\x5cx20data\x5cx20partition.device appeared twice with dif ferent sysfs paths /sys/devices/pci:00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda3 and /sys/devices/pci :00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda6 [ 13.085674] systemd[1]: Device dev-disk-by\x2dpartlabel-Basic\x5cx20data\x5cx20partition.device appeared twice with dif ferent sysfs paths /sys/devices/pci:00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda3 and /sys/devices/pci :00/:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda/sda5 And the following is my /etc/fstab. /dev/sda7/ext4noatime0 1 /dev/sda1/boot/efivfatdefaults0 0 /dev/sda8/homeext4defaults0 0 /dev/sda4/mnt/softwarentfsdefaults0 0 /dev/sda5/mnt/datantfsdefaults0 0 /dev/sda6/mnt/backupntfsdefaults0 0 /dev/sda3/mnt/systemntfsdefaults0 0 The four NTFS partitions are my Windows disk partition. The followint is my 'lsblk' output. NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT sda 8:00 931.5G 0 disk ├─sda1 8:10 200M 0 part /boot/efi ├─sda2 8:20 128M 0 part ├─sda3 8:30 58.5G 0 part /mnt/system ├─sda4 8:40 195.3G 0 part /mnt/software ├─sda5 8:50 244.1G 0 part /mnt/data ├─sda6 8:60 244.1G 0 part ├─sda7 8:70 29.3G 0 part / └─sda8 8:80 159.9G 0 part /home sr0 11:01 1024M 0 rom The sda2 partition is a MSR(Microsoft System Recover) partition. sda1 for EFI partition. sda2 for MSR partition. sda3-sda6 fro windows partition. sda7 for gentoo linux root partition sda8 for gentoo home partition. Thank you! Yours sincerely ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Reduce unit-loading time
Hi, -Original Message- From: systemd-devel [mailto:systemd-devel- boun...@lists.freedesktop.org] On Behalf Of cee1 Sent: Wednesday, May 13, 2015 11:52 AM To: systemd Mailing List Subject: [systemd-devel] Reduce unit-loading time Hi all, We're trying systemd to boot up an ARM board, and find systemd uses more than one second to load units. This sounds far a bit too long to me. Our systemd comes up in an arm based system in about 200-300ms from executing init up to the first unit being executed. Comparing with the init of Android on the same board, it manages to boot the system very fast. We guess following factors are involved: 1. systemd has a much bigger footprint than the init of Android: the latter is static linked, and is about 1xxKB (systemd is about 1.4MB, and is linked with libc/libcap/libpthread, etc) 2. systemd spends quiet a while to read/parse unit files. This depends on the numbers of units involved in the startup (finally connected in the dependency tree that ends in the default.target root). In our case, a comparable large unit set takes by about 40-60ms, not so long, I'd say. Any idea to reduce the unit-loading time? e.g. one-single file contains all units descriptions, or a compiled version(containing resolved dependencies, or even the boot up sequence) Could you provide me some additional information about your system setup? - Version of systemd - Are you starting something in parallel to systemd which might take significant IO? - Version of the kernel - The kernel ticker frequency - The enabled cgroups controllers The last three points might sound a bit far away, but I've an idea in mind ... Best regards Marko Hoyer Software Group II (ADITG/SW2) Tel. +49 5121 49 6948 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=
On Wed, May 13, 2015 at 06:35:58AM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 20:37:15 +0200 Karel Zak k...@redhat.com пишет: On Tue, May 12, 2015 at 07:29:33PM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 18:04:50 +0200 Karel Zak k...@redhat.com пишет: Currently we have no way how to specify dependencies between fstab entries (or another units) in the /etc/fstab. It means that users are forced to bypass fstab and write .mount units manually. Actually we have. mkdir -p /etc/systemd/system/path-to-mount-point.mount.d cat /etc/systemd/system/path-to-mount-point.mount.d/deps.conf EOF [Unit] After=xxx Before=xxx Wants=xxx Requires=xxx EOF You miss the point -- keep all in fstab. I admit I do. Why? We want to keep in fstab bits and pieces that are common with other utilities. Which other tool needs to know systemd dependencies? Did you read the reference in the patch? It's not about systemd dependences, but about dependences between mount points. The fstab has been originally (by mount -a) serialized during boot. Now it's parallelized and in some cases it's bad thing and without extra configuration systemd is not able to understand the dependencies in fstab. It's admins' nightmare to require additional file somewhere in /etc/systemd to fix systemd fstab interpretation. BTW, we already have x-systemd stuff in fstab... 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
[systemd-devel] [PATCH 0/2] Allow setting the cgroup hierarchy manually before calling nspawn
These two commits allow nspawn to play nicely with a cgroup hierarchy manually set beforehand. Iago López Galeiras (2): nspawn: only mount the cgroup root if it's not already mounted nspawn: skip symlink to a combined cgroup hierarchy if it already exists src/nspawn/nspawn.c | 34 ++ 1 file changed, 30 insertions(+), 4 deletions(-) -- 2.4.0 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] nspawn: only mount the cgroup root if it's not already mounted
This allows the user to set the cgroups manually before calling nspawn.
---
src/nspawn/nspawn.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 2f7dd53..c67cab2 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1053,6 +1053,21 @@ static int mount_cgroup_hierarchy(const char *dest,
const char *controller, cons
return 1;
}
+static int mount_cgroup_tmpfs(const char *cgroup_root) {
+int r;
+
+r = path_is_mount_point(cgroup_root, false);
+if (r 0 r != -ENOENT)
+return log_error_errno(r, Failed to determine if %s is
mounted already: %m, cgroup_root);
+if (r 0)
+return 0;
+
+if (mount(tmpfs, cgroup_root, tmpfs,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, mode=755) 0)
+return log_error_errno(errno, Failed to mount tmpfs to
/sys/fs/cgroup: %m);
+
+return 1;
+}
+
static int mount_cgroup(const char *dest) {
_cleanup_set_free_free_ Set *controllers = NULL;
_cleanup_free_ char *own_cgroup_path = NULL;
@@ -1072,8 +1087,9 @@ static int mount_cgroup(const char *dest) {
return log_error_errno(r, Failed to determine our own cgroup
path: %m);
cgroup_root = strjoina(dest, /sys/fs/cgroup);
-if (mount(tmpfs, cgroup_root, tmpfs,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, mode=755) 0)
-return log_error_errno(errno, Failed to mount tmpfs to
/sys/fs/cgroup: %m);
+r = mount_cgroup_tmpfs(cgroup_root);
+if (r 0)
+return r;
for (;;) {
_cleanup_free_ char *controller = NULL, *origin = NULL,
*combined = NULL;
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] nspawn: skip symlink to a combined cgroup hierarchy if it already exists
If a symlink to a combined cgroup hierarchy already exists and points to
the right path, skip it. This avoids an error when the cgroups are set
manually before calling nspawn.
---
src/nspawn/nspawn.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index c67cab2..cc0b84f 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1113,7 +1113,7 @@ static int mount_cgroup(const char *dest) {
} else if (r 0)
return log_error_errno(r, Failed to read link %s:
%m, origin);
else {
-_cleanup_free_ char *target = NULL;
+_cleanup_free_ char *target = NULL, *p = NULL;
target = strjoin(dest, /sys/fs/cgroup/, controller,
NULL);
if (!target)
@@ -1130,8 +1130,18 @@ static int mount_cgroup(const char *dest) {
if (r 0)
return r;
-if (symlink(combined, target) 0)
+if (symlink(combined, target) 0) {
+if (errno == EEXIST) {
+r = readlink_malloc(target, p);
+if (r 0)
+return log_error_errno(r, Failed to read
link %s: %m, target);
+else if (!streq(p, combined)) {
+log_error(Invalid existing symlink for
combined hierarchy);
+return -EINVAL;
+}
+} else
return log_error_errno(errno, Failed to
create symlink for combined hierarchy: %m);
+}
}
}
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] nspawn: skip symlink to a combined cgroup hierarchy if it already exists
Patchset imported to github. Pull request: https://github.com/systemd-devs/systemd/compare/master...systemd-mailing-devs:1431508540-3855-3-git-send-email-iago%40endocode.com -- Generated by https://github.com/haraldh/mail2git ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Reduce unit-loading time
Hi all, We're trying systemd to boot up an ARM board, and find systemd uses more than one second to load units. Comparing with the init of Android on the same board, it manages to boot the system very fast. We guess following factors are involved: 1. systemd has a much bigger footprint than the init of Android: the latter is static linked, and is about 1xxKB (systemd is about 1.4MB, and is linked with libc/libcap/libpthread, etc) 2. systemd spends quiet a while to read/parse unit files. Any idea to reduce the unit-loading time? e.g. one-single file contains all units descriptions, or a compiled version(containing resolved dependencies, or even the boot up sequence) -- Regards, - cee1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-bus: fix memory leak in test-bus-chat
On 05/10/2015 03:14 AM, Cristian Rodríguez wrote:
Building with address sanitizer enabled on GCC 5.1.x a memory leak
is reported because we never close the bus, fix it by using
cleanup variable attribute.
---
src/libsystemd/sd-bus/test-bus-chat.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/libsystemd/sd-bus/test-bus-chat.c
b/src/libsystemd/sd-bus/test-bus-chat.c
index 99261fa..1e50dfc 100644
--- a/src/libsystemd/sd-bus/test-bus-chat.c
+++ b/src/libsystemd/sd-bus/test-bus-chat.c
@@ -262,7 +262,7 @@ fail:
static void* client1(void*p) {
_cleanup_bus_message_unref_ sd_bus_message *reply = NULL;
-sd_bus *bus = NULL;
+_cleanup_bus_close_unref_ sd_bus *bus = NULL;
sd_bus_error error = SD_BUS_ERROR_NULL;
const char *hello;
int r;
@@ -345,8 +345,6 @@ finish:
else
sd_bus_send(bus, q, NULL);
-sd_bus_flush(bus);
We should still keep this flush, right?
-sd_bus_unref(bus);
}
sd_bus_error_free(error);
Thanks,
Daniel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] nspawn: only mount the cgroup root if it's not already mounted
On Wed, 13.05.15 11:15, Iago López Galeiras (i...@endocode.com) wrote:
This allows the user to set the cgroups manually before calling
nspawn.
I think it would be better to simply move mounting of /sys/fs/cgroup
into the array at the top of mount_all(), which already does this
path_is_mount_point() checking...
---
src/nspawn/nspawn.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 2f7dd53..c67cab2 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1053,6 +1053,21 @@ static int mount_cgroup_hierarchy(const char *dest,
const char *controller, cons
return 1;
}
+static int mount_cgroup_tmpfs(const char *cgroup_root) {
+int r;
+
+r = path_is_mount_point(cgroup_root, false);
+if (r 0 r != -ENOENT)
+return log_error_errno(r, Failed to determine if %s is
mounted already: %m, cgroup_root);
+if (r 0)
+return 0;
+
+if (mount(tmpfs, cgroup_root, tmpfs,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, mode=755) 0)
+return log_error_errno(errno, Failed to mount tmpfs to
/sys/fs/cgroup: %m);
+
+return 1;
+}
+
static int mount_cgroup(const char *dest) {
_cleanup_set_free_free_ Set *controllers = NULL;
_cleanup_free_ char *own_cgroup_path = NULL;
@@ -1072,8 +1087,9 @@ static int mount_cgroup(const char *dest) {
return log_error_errno(r, Failed to determine our own
cgroup path: %m);
cgroup_root = strjoina(dest, /sys/fs/cgroup);
-if (mount(tmpfs, cgroup_root, tmpfs,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, mode=755) 0)
-return log_error_errno(errno, Failed to mount tmpfs to
/sys/fs/cgroup: %m);
+r = mount_cgroup_tmpfs(cgroup_root);
+if (r 0)
+return r;
for (;;) {
_cleanup_free_ char *controller = NULL, *origin = NULL,
*combined = NULL;
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] journalctl --directory -f rotation issue
Hi, I'm having a problem with the combination of the --directory (or --file) and the --follow option of journalctl: If followed journal files are rotated, journalctl stops showing new content of those files. Non rotated or new created files are handled properly. After a restart of journalctl the new entries can be seen as expected. For an example view the attachments of https://bugs.freedesktop.org/show_bug.cgi?id=90193 I suspect this bug to happen on a deeper level: I use a tool which calls sd_journal_open_directory(). After a rotation on a file it doesn't handle the new content of that file. Other files (even new created one) are handled properly though. Maybe an important note: The follow option works perfectly fine with the default (system) journal. Rotations are handled seamlessly. Are my problems expected behaviour (or another error on my side)? Best Regards Sebastian ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=
On Tue, 12.05.15 19:29, Andrei Borzenkov (arvidj...@gmail.com) wrote: All examples can be configured using drop-ins. Do we really need yet another syntax now? Well, even though units are all-powerful, I still believe that fstab is probably where most people will configure their stuff, simply because it's so much simpler to use. Now, of course, we shouldn't try to reimplement all possible settings that .mount unit files provide as fstab options. But the most obvious ones probably *do* make sense to add, and x-systemd.requires= is probably one of them. It's not only useful for stuff like overlayfs, but also pretty essential for things like ext3, ext4, xfs which support external journal logging devices, for which the mounts need to express dependencies: https://bugzilla.redhat.com/show_bug.cgi?id=1164334 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] unit: Set KillMode=mixed for services which use sulogin
On Tue, 12.05.15 14:36, Michael Biebl (mbi...@gmail.com) wrote: 2015-05-12 0:03 GMT+02:00 Lennart Poettering lenn...@poettering.net: On Tue, 12.05.15 00:01, Lennart Poettering (lenn...@poettering.net) wrote: On Mon, 11.05.15 23:50, Michael Biebl (mbi...@gmail.com) wrote: 2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com: See http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html Lennart, are you ok if I commit this? Yes, please! But actually, I agree with Zbigniew, can you please replace all occurences of Killmode=process with KillMode=mixed, with the exception of debug-shell.service. Also, if you'd add a comment there explaining that that file sticks to KillMode=process to not affect backgrounded debugging process, things would even be better! Hm, I just tested KillMode=mixed with getty@.service on installations which didn't have libpam-systemd enabled. Ah, humm. So, we don't really care about installations without pam_systemd upstream regarding this. However, logind is actually responsible for killing user process on logout if that's configured via logind.conf. sulogin generally does not set up a PAM session, and we indeed should allow processes like screen staying around in such a context. Hence KillMode=process is actually the right choice for all these services, indeed. Hence I figure the status quo for all of this is pretty OK already... 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] dev-root.device is not active, results in an umount spree
On Wed, 13.05.15 12:48, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote: I am booting without initramfs, using a plan9 filesystem as rootfs in a kvm. Thus my /proc/cmdline has: root=/dev/root rootflags=rw,trans=virtio,version=9p2000.L rootfstype=9p # mount /dev/root on / type 9p (rw,relatime,sync,dirsync,rw,trans=virtio,version=9p2000.L) Yet, dev-root.device is dead: # systemctl status dev-root.device ● dev-root.device Loaded: loaded Active: inactive (dead) Yeah, this /dev/root thing is really weird in the kernel. It's not an actual device, it's just a weird string. We probably should filter it out entirely, and never ever generate a unit dependency for it. Please check if this fixes things for you: http://cgit.freedesktop.org/systemd/systemd/commit/?id=7ba2711d3fd283c389db2a1e7b9598ba9f0dac0c That said, I figure you should backport 628c89cc68ab96fce2de7ebba5933725d147aecc and friends to your tree, which should also make this problem go away for you. I am planning to locally patch mount_add_device_links to skip if what is /dev/root, to avoid a call to unit_add_node_link... But I'm not sure if this is the right thing to do or not. It is, it's the change I made now, too. 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] nspawn: skip symlink to a combined cgroup hierarchy if it already exists
On Wed, 13.05.15 11:15, Iago López Galeiras (i...@endocode.com) wrote:
-if (symlink(combined, target) 0)
+if (symlink(combined, target) 0) {
+if (errno == EEXIST) {
+r = readlink_malloc(target, p);
+if (r 0)
+return log_error_errno(r, Failed to
read link %s: %m, target);
+else if (!streq(p, combined)) {
+log_error(Invalid existing symlink for
combined hierarchy);
+return -EINVAL;
+}
+} else
return log_error_errno(errno, Failed to
create symlink for combined hierarchy: %m);
Indentation is weird, needs to be 8 space...
I think I'd prefer if the code for supressing and error when the
symlink is already OK would actually live in some new call in util.c
though. Maybe call it symlink_idempotent() 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] Default to /usr/bin/u?mount, configurable, rather than hard-coded /bin/u?mount.
On Mon, 11.05.15 16:58, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote:
Hmm, all the other AM_PATH_PROG variables do not carry a _BIN suffix,
should these two? I don't think so.
(In general, I am not too big a fan of abbreviating things
unnecessarily, unless this is commonly done elsewhere...)
Otherwise looks fine,
Lennart
---
Makefile.am | 2 ++
configure.ac| 3 +++
src/core/mount.c| 6 +++---
src/remount-fs/remount-fs.c | 10 +-
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index e4d00a8..0ff11cc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -208,6 +208,8 @@ AM_CPPFLAGS = \
-DPOLKIT_AGENT_BINARY_PATH=\$(bindir)/pkttyagent\ \
-DQUOTACHECK=\$(QUOTACHECK)\ \
-DKEXEC=\$(KEXEC)\ \
+ -DMOUNT_BIN=\$(MOUNT_BIN)\ \
+ -DUMOUNT_BIN=\$(UMOUNT_BIN)\ \
-DLIBDIR=\$(libdir)\ \
-DROOTLIBDIR=\$(rootlibdir)\ \
-DROOTLIBEXECDIR=\$(rootlibexecdir)\ \
diff --git a/configure.ac b/configure.ac
index 600e203..61dffc6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -100,6 +100,9 @@ AC_PATH_PROG([KEXEC], [kexec], [/usr/sbin/kexec],
[$PATH:/usr/sbin:/sbin])
AC_PATH_PROG([SULOGIN], [sulogin], [/usr/sbin/sulogin],
[$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([MOUNT_BIN], [mount], [/usr/bin/mount], [$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([UMOUNT_BIN], [umount], [/usr/bin/umount],
[$PATH:/usr/sbin:/sbin])
+
AS_IF([! ln --relative --help /dev/null 21], [AC_MSG_ERROR([*** ln
doesn't support --relative ***])])
M4_DEFINES=
diff --git a/src/core/mount.c b/src/core/mount.c
index 65a66b4..a370c74 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -871,7 +871,7 @@ static void mount_enter_unmounting(Mount *m) {
m-control_command_id = MOUNT_EXEC_UNMOUNT;
m-control_command = m-exec_command + MOUNT_EXEC_UNMOUNT;
-r = exec_command_set(m-control_command, /bin/umount, m-where,
NULL);
+r = exec_command_set(m-control_command, UMOUNT_BIN, m-where, NULL);
if (r = 0 UNIT(m)-manager-running_as == SYSTEMD_SYSTEM)
r = exec_command_append(m-control_command, -n, NULL);
if (r 0)
@@ -924,7 +924,7 @@ static void mount_enter_mounting(Mount *m) {
if (r 0)
goto fail;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_BIN,
m-parameters_fragment.what, m-where,
NULL);
if (r = 0 UNIT(m)-manager-running_as == SYSTEMD_SYSTEM)
r = exec_command_append(m-control_command, -n,
NULL);
@@ -973,7 +973,7 @@ static void mount_enter_remounting(Mount *m) {
else
o = remount;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_BIN,
m-parameters_fragment.what, m-where,
-o, o, NULL);
if (r = 0 UNIT(m)-manager-running_as == SYSTEMD_SYSTEM)
diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c
index 70dacfa..ff7de2c 100644
--- a/src/remount-fs/remount-fs.c
+++ b/src/remount-fs/remount-fs.c
@@ -94,15 +94,15 @@ int main(int argc, char *argv[]) {
const char *arguments[5];
/* Child */
-arguments[0] = /bin/mount;
+arguments[0] = MOUNT_BIN;
arguments[1] = me-mnt_dir;
arguments[2] = -o;
arguments[3] = remount;
arguments[4] = NULL;
-execv(/bin/mount, (char **) arguments);
+execv(MOUNT_BIN, (char **) arguments);
-log_error_errno(errno, Failed to execute
/bin/mount: %m);
+log_error_errno(errno, Failed to execute
MOUNT_BIN : %m);
_exit(EXIT_FAILURE);
}
@@ -142,9 +142,9 @@ int main(int argc, char *argv[]) {
if (s) {
if (!is_clean_exit(si.si_code, si.si_status, NULL)) {
if (si.si_code == CLD_EXITED)
-log_error(/bin/mount for %s exited
with exit status %i., s, si.si_status);
+log_error(MOUNT_BIN for %s exited
with exit status %i., s, si.si_status);
else
-log_error(/bin/mount for %s
terminated by signal %s., s, signal_to_string(si.si_status));
+log_error(MOUNT_BIN for %s
terminated by signal %s., s,
Re: [systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=
On 13 May 2015 at 13:33, Lennart Poettering lenn...@poettering.net wrote: On Tue, 12.05.15 19:29, Andrei Borzenkov (arvidj...@gmail.com) wrote: All examples can be configured using drop-ins. Do we really need yet another syntax now? Well, even though units are all-powerful, I still believe that fstab is probably where most people will configure their stuff, simply because it's so much simpler to use. ... and the only place where you can define different mount options and expect them to take effect. This is because remount-fs only remounts things defined in fstab, and doesn't remount things that are defined via .mount units. Now, of course, we shouldn't try to reimplement all possible settings that .mount unit files provide as fstab options. But the most obvious ones probably *do* make sense to add, and x-systemd.requires= is probably one of them. It's not only useful for stuff like overlayfs, but also pretty essential for things like ext3, ext4, xfs which support external journal logging devices, for which the mounts need to express dependencies: https://bugzilla.redhat.com/show_bug.cgi?id=1164334 Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Regards, Dimitri. Pura Vida! https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin
On Wed, 13.05.15 15:32, Michael Biebl (mbi...@gmail.com) wrote: 2015-05-13 15:19 GMT+02:00 Lennart Poettering lenn...@poettering.net: sulogin generally does not set up a PAM session, and we indeed should allow processes like screen staying around in such a context. Hence KillMode=process is actually the right choice for all these services, indeed. Do you really think it makes sense to start screen from emergency/rescue mode? No I don't. But I think we shouldn't try to enforce any policy on process lifetime in debug/emergency/rescue mode... They are supposed to be low-level recovery features, that give you raw, naked, rough access to the system guts, really. And hence we probably shouldn't kill what they leave around... I mean, if admins do something like this: ( while : ; do ps xawuf /tmp/ps-log ; sleep 10 ; done ) disown in the debug shell, to debug something, then we should not break that at logout, really. Imho those are the cases where you don't actually want stuff to stay around after you log out. Hence I figure the status quo for all of this is pretty OK already... Well, I was intending to commit my original patch, which only uses KillMode=mixed for services which use sulogin, i.e. emergency.service, rescue.service and console-shell.service. See the original bug that triggered this patch [1]. We don't really want a stray bash process to stay around which potentially fights with sulogin over the input. Well, I am pretty sure that in this case, it should be sulogin that propagates the shutdown request to the shell it spawned, but we should not do it otherwise. Note that by default we don't even clean up processes of unprivileged users on logout. You have to turn this on via KillUserProcesses= explicitly. And if we don't do this for unprivileged users, we certainly shouldn't do it for debug shells either [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784238 That bug reports is long... From what I got this really looks like something to fix in Debian's sulogin implementation really. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 2/2] nspawn: skip symlink to a combined cgroup hierarchy if it already exists
If a symlink to a combined cgroup hierarchy already exists and points to
the right path, skip it. This avoids an error when the cgroups are set
manually before calling nspawn.
---
src/nspawn/nspawn.c | 10 --
src/shared/util.c | 23 +++
src/shared/util.h | 1 +
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index f292c63..e9e703f 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1365,8 +1365,14 @@ static int mount_cgroup(const char *dest) {
if (r 0)
return r;
-if (symlink(combined, target) 0)
-return log_error_errno(errno, Failed to
create symlink for combined hierarchy: %m);
+r = symlink_idempotent(combined, target);
+if (r 0) {
+if (r == -EINVAL) {
+log_error(Invalid existing symlink
for combined hierarchy);
+return r;
+} else
+return log_error_errno(r, Failed to
create symlink for combined hierarchy: %m);
+}
}
}
diff --git a/src/shared/util.c b/src/shared/util.c
index 466dce4..22e00a6 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -2765,6 +2765,29 @@ int symlink_atomic(const char *from, const char *to) {
return 0;
}
+int symlink_idempotent(const char *from, const char *to) {
+_cleanup_free_ char *p = NULL;
+int r;
+
+assert(from);
+assert(to);
+
+if (symlink(from, to) 0) {
+if (errno == EEXIST) {
+r = readlink_malloc(to, p);
+if (r 0)
+return r;
+
+if (!streq(p, from)) {
+return -EINVAL;
+}
+} else
+return -errno;
+}
+
+return 0;
+}
+
int mknod_atomic(const char *path, mode_t mode, dev_t dev) {
_cleanup_free_ char *t = NULL;
int r;
diff --git a/src/shared/util.h b/src/shared/util.h
index 4a67d5c..8565fd6 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -404,6 +404,7 @@ bool machine_name_is_valid(const char *s) _pure_;
char* strshorten(char *s, size_t l);
+int symlink_idempotent(const char *from, const char *to);
int symlink_atomic(const char *from, const char *to);
int mknod_atomic(const char *path, mode_t mode, dev_t dev);
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=
On Wed, 13.05.15 14:28, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote: On 13 May 2015 at 13:33, Lennart Poettering lenn...@poettering.net wrote: On Tue, 12.05.15 19:29, Andrei Borzenkov (arvidj...@gmail.com) wrote: All examples can be configured using drop-ins. Do we really need yet another syntax now? Well, even though units are all-powerful, I still believe that fstab is probably where most people will configure their stuff, simply because it's so much simpler to use. ... and the only place where you can define different mount options and expect them to take effect. This is because remount-fs only remounts things defined in fstab, and doesn't remount things that are defined via .mount units. Well remount-fs is only really necessary for API mounts like /sys, /proc or /dev, and little else. Otherwise it has no reason to exist really. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 1/2] nspawn: only mount the cgroup root if it's not already mounted
This allows the user to set the cgroups manually before calling nspawn.
---
src/nspawn/nspawn.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 8c91726..f292c63 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1031,15 +1031,16 @@ static int mount_all(const char *dest) {
} MountPoint;
static const MountPoint mount_table[] = {
-{ proc, /proc, proc, NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
-{ /proc/sys, /proc/sys, NULL,NULL,MS_BIND,
true }, /* Bind mount first */
-{ NULL,/proc/sys, NULL,NULL,
MS_BIND|MS_RDONLY|MS_REMOUNT, true }, /* Then, make it r/o */
-{ sysfs, /sys, sysfs, NULL,
MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
-{ tmpfs, /dev, tmpfs, mode=755,
MS_NOSUID|MS_STRICTATIME, true },
-{ devpts,/dev/pts,
devpts,newinstance,ptmxmode=0666,mode=620,gid= STRINGIFY(TTY_GID),
MS_NOSUID|MS_NOEXEC, true },
-{ tmpfs, /dev/shm, tmpfs, mode=1777,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
-{ tmpfs, /run, tmpfs, mode=755,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
-{ tmpfs, /tmp, tmpfs, mode=1777,
MS_STRICTATIME, true },
+{ proc, /proc, proc, NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
+{ /proc/sys, /proc/sys, NULL,NULL,
MS_BIND,true }, /* Bind mount first */
+{ NULL,/proc/sys, NULL,NULL,
MS_BIND|MS_RDONLY|MS_REMOUNT, true }, /* Then, make it r/o */
+{ sysfs, /sys, sysfs, NULL,
MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
+{ tmpfs, /sys/fs/cgroup, tmpfs, mode=755,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, true },
+{ tmpfs, /dev, tmpfs, mode=755,
MS_NOSUID|MS_STRICTATIME, true },
+{ devpts,/dev/pts,
devpts,newinstance,ptmxmode=0666,mode=620,gid= STRINGIFY(TTY_GID),
MS_NOSUID|MS_NOEXEC, true },
+{ tmpfs, /dev/shm, tmpfs, mode=1777,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
+{ tmpfs, /run, tmpfs, mode=755,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
+{ tmpfs, /tmp, tmpfs, mode=1777,
MS_STRICTATIME, true },
#ifdef HAVE_SELINUX
{ /sys/fs/selinux, /sys/fs/selinux, NULL, NULL, MS_BIND,
false }, /* Bind mount first */
{ NULL, /sys/fs/selinux, NULL, NULL,
MS_BIND|MS_RDONLY|MS_REMOUNT, false }, /* Then, make it r/o */
@@ -1324,9 +1325,6 @@ static int mount_cgroup(const char *dest) {
if (r 0)
return log_error_errno(r, Failed to determine our own cgroup
path: %m);
-cgroup_root = strjoina(dest, /sys/fs/cgroup);
-if (mount(tmpfs, cgroup_root, tmpfs,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, mode=755) 0)
-return log_error_errno(errno, Failed to mount tmpfs to
/sys/fs/cgroup: %m);
for (;;) {
_cleanup_free_ char *controller = NULL, *origin = NULL,
*combined = NULL;
@@ -1386,6 +1384,7 @@ static int mount_cgroup(const char *dest) {
if (mount(NULL, systemd_root, NULL,
MS_BIND|MS_REMOUNT|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, NULL) 0)
return log_error_errno(errno, Failed to mount cgroup root
read-only: %m);
+cgroup_root = strjoina(dest, /sys/fs/cgroup);
if (mount(NULL, cgroup_root, NULL,
MS_REMOUNT|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME|MS_RDONLY, mode=755)
0)
return log_error_errno(errno, Failed to remount %s read-only:
%m, cgroup_root);
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 0/2] Allow setting the cgroup hierarchy manually before calling nspawn
These two commits allow nspawn to play nicely with a cgroup hierarchy manually set beforehand. Iago López Galeiras (2): nspawn: only mount the cgroup root if it's not already mounted nspawn: skip symlink to a combined cgroup hierarchy if it already exists src/nspawn/nspawn.c | 33 +++-- src/shared/util.c | 23 +++ src/shared/util.h | 1 + 3 files changed, 43 insertions(+), 14 deletions(-) -- 2.4.0 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemctl: introduce -e and -d for start and stop
From: Jan Synacek jsyna...@redhat.com
---
man/systemctl.xml | 26 ++
src/systemctl/systemctl.c | 40 ++--
2 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 4dbdfe1..51c19e1 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -432,6 +432,26 @@
/varlistentry
varlistentry
+termoption-e/option/term
+termoption--enable/option/term
+
+listitem
+ paraWhen used with commandstart/command, additionally
+ enable unit after it has been successfully started./para
+/listitem
+ /varlistentry
+
+ varlistentry
+termoption-d/option/term
+termoption--disable/option/term
+
+listitem
+ paraWhen used with commandstop/command, additionally
+ disable unit after it has been successfully stopped./para
+/listitem
+ /varlistentry
+
+ varlistentry
termoption-f/option/term
termoption--force/option/term
@@ -633,6 +653,9 @@ kobject-uevent 1 systemd-udevd-kernel.socket
systemd-udevd.service
instance name until the instance has been started. Therefore,
using glob patterns with commandstart/command
has limited usefulness./para
+
+paraWhen used with option-e/option, additional
commandenable/command
+operation is performed./para
/listitem
/varlistentry
varlistentry
@@ -641,6 +664,9 @@ kobject-uevent 1 systemd-udevd-kernel.socket
systemd-udevd.service
listitem
paraStop (deactivate) one or more units specified on the
command line./para
+
+paraWhen used with option-d/option, additional
commanddisable/command
+operation is performed./para
/listitem
/varlistentry
varlistentry
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 1f18f9c..5655c57 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -136,12 +136,15 @@ static unsigned arg_lines = 10;
static OutputMode arg_output = OUTPUT_SHORT;
static bool arg_plain = false;
static bool arg_firmware_setup = false;
+static bool arg_startenable = false;
+static bool arg_stopdisable = false;
static bool original_stdout_is_tty;
static int daemon_reload(sd_bus *bus, char **args);
static int halt_now(enum action a);
static int check_one_unit(sd_bus *bus, const char *name, const char
*good_states, bool quiet);
+static int enable_unit(sd_bus *bus, char **args);
static char** strv_skip_first(char **strv) {
if (strv_length(strv) 0)
@@ -2677,7 +2680,7 @@ static int start_unit(sd_bus *bus, char **args) {
const char *method, *mode, *one_name, *suffix = NULL;
_cleanup_strv_free_ char **names = NULL;
char **name;
-int r = 0;
+int r = 0, start_rc = 0;
assert(bus);
@@ -2722,11 +2725,26 @@ static int start_unit(sd_bus *bus, char **args) {
STRV_FOREACH(name, names) {
_cleanup_bus_error_free_ sd_bus_error error =
SD_BUS_ERROR_NULL;
-int q;
-q = start_unit_one(bus, method, *name, mode, error, w);
-if (r = 0 q 0)
-r = translate_bus_error_to_exit_status(q, error);
+start_rc = start_unit_one(bus, method, *name, mode, error, w);
+if (r = 0 start_rc 0)
+r = translate_bus_error_to_exit_status(start_rc,
error);
+}
+
+if (((streq(args[0], start) arg_startenable)
+ || (streq(args[0], stop) arg_stopdisable))
+ start_rc = 0) {
+_cleanup_strv_free_ char **newargs;
+
+newargs = strv_copy(args);
+if (!newargs)
+return -ENOMEM;
+free(newargs[0]);
+newargs[0] = streq(args[0], start) ? strdup(enable) :
strdup(disable);
+if (!newargs[0])
+return -ENOMEM;
+
+r = enable_unit(bus, newargs);
}
if (!arg_no_block) {
@@ -6257,6 +6275,8 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
{ recursive, no_argument, NULL, 'r'
},
{ preset-mode, required_argument, NULL,
ARG_PRESET_MODE },
{ firmware-setup, no_argument, NULL,
ARG_FIRMWARE_SETUP },
+{ enable, no_argument, NULL, 'e'
},
+{ disable, no_argument, NULL, 'd'
},
{}
};
@@ -6265,7 +6285,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
assert(argc = 0);
assert(argv);
-while ((c = getopt_long(argc, argv,
Re: [systemd-devel] [PATCH] Default to /usr/bin/u?mount, configurable, rather than hard-coded /bin/u?mount.
On Wed, 13.05.15 14:24, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote: On 13 May 2015 at 14:00, Lennart Poettering lenn...@poettering.net wrote: On Mon, 11.05.15 16:58, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote: Hmm, all the other AM_PATH_PROG variables do not carry a _BIN suffix, should these two? I don't think so. (In general, I am not too big a fan of abbreviating things unnecessarily, unless this is commonly done elsewhere...) There is already MOUNT define used elsewhere in the code... DEFINE_CAST(MOUNT, Mount); Hence the _BIN suffix. I see... But _BIN sounds weird... Please use _PATH then, which is already used for some of the paths configured in Makefile.am 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] systemctl: introduce -e and -d for start and stop
On Wed, 13.05.15 16:39, Mantas Mikulėnas (graw...@gmail.com) wrote: On Wed, May 13, 2015 at 4:36 PM, Lennart Poettering lenn...@poettering.net wrote: On Wed, 13.05.15 15:21, jsyna...@redhat.com (jsyna...@redhat.com) wrote: From: Jan Synacek jsyna...@redhat.com Hmm, do we really need two options for this? I mean, since -e would only ever be combined with start, and -d only with stop it could as well be a single option that works for both? Would be less of a problem if subcommands could have their own options separate from global ones... Why? I think either way it's a good idea to be conservative with adding options to systemctl. Also, even if there were per-verb options, then I'd still believe that it would be a bad idea to overload the same option for different verbs, to avoid confusion... 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] Default to /usr/bin/u?mount, configurable, rather than hard-coded /bin/u?mount.
On Wed, 13.05.15 14:43, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote:
Thanks! Applied!
---
Makefile.am | 2 ++
configure.ac| 3 +++
src/core/execute.h | 2 +-
src/core/mount.c| 12 ++--
src/core/mount.h| 4 ++--
src/remount-fs/remount-fs.c | 10 +-
6 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index e4d00a8..e8a329f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -208,6 +208,8 @@ AM_CPPFLAGS = \
-DPOLKIT_AGENT_BINARY_PATH=\$(bindir)/pkttyagent\ \
-DQUOTACHECK=\$(QUOTACHECK)\ \
-DKEXEC=\$(KEXEC)\ \
+ -DMOUNT_PATH=\$(MOUNT_PATH)\ \
+ -DUMOUNT_PATH=\$(UMOUNT_PATH)\ \
-DLIBDIR=\$(libdir)\ \
-DROOTLIBDIR=\$(rootlibdir)\ \
-DROOTLIBEXECDIR=\$(rootlibexecdir)\ \
diff --git a/configure.ac b/configure.ac
index 600e203..70e594d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -100,6 +100,9 @@ AC_PATH_PROG([KEXEC], [kexec], [/usr/sbin/kexec],
[$PATH:/usr/sbin:/sbin])
AC_PATH_PROG([SULOGIN], [sulogin], [/usr/sbin/sulogin],
[$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([MOUNT_PATH], [mount], [/usr/bin/mount],
[$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([UMOUNT_PATH], [umount], [/usr/bin/umount],
[$PATH:/usr/sbin:/sbin])
+
AS_IF([! ln --relative --help /dev/null 21], [AC_MSG_ERROR([*** ln
doesn't support --relative ***])])
M4_DEFINES=
diff --git a/src/core/execute.h b/src/core/execute.h
index a0908e0..f5d5c1d 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -165,7 +165,7 @@ struct ExecContext {
/* This is not exposed to the user but available
* internally. We need it to make sure that whenever we spawn
- * /bin/mount it is run in the same process group as us so
+ * /usr/bin/mount it is run in the same process group as us so
* that the autofs logic detects that it belongs to us and we
* don't enter a trigger loop. */
bool same_pgrp;
diff --git a/src/core/mount.c b/src/core/mount.c
index 8853311..8ef3d17 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -135,8 +135,8 @@ static void mount_init(Unit *u) {
m-exec_context.std_error = u-manager-default_std_error;
}
-/* We need to make sure that /bin/mount is always called in
- * the same process group as us, so that the autofs kernel
+/* We need to make sure that /usr/bin/mount is always called
+ * in the same process group as us, so that the autofs kernel
* side doesn't send us another mount request while we are
* already trying to comply its last one. */
m-exec_context.same_pgrp = true;
@@ -833,7 +833,7 @@ static void mount_enter_unmounting(Mount *m) {
m-control_command_id = MOUNT_EXEC_UNMOUNT;
m-control_command = m-exec_command + MOUNT_EXEC_UNMOUNT;
-r = exec_command_set(m-control_command, /bin/umount, m-where,
NULL);
+r = exec_command_set(m-control_command, UMOUNT_PATH, m-where,
NULL);
if (r = 0 UNIT(m)-manager-running_as == MANAGER_SYSTEM)
r = exec_command_append(m-control_command, -n, NULL);
if (r 0)
@@ -884,7 +884,7 @@ static void mount_enter_mounting(Mount *m) {
if (r 0)
goto fail;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_PATH,
m-parameters_fragment.what, m-where,
NULL);
if (r = 0 UNIT(m)-manager-running_as == MANAGER_SYSTEM)
r = exec_command_append(m-control_command, -n,
NULL);
@@ -931,7 +931,7 @@ static void mount_enter_remounting(Mount *m) {
else
o = remount;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_PATH,
m-parameters_fragment.what, m-where,
-o, o, NULL);
if (r = 0 UNIT(m)-manager-running_as == MANAGER_SYSTEM)
@@ -1582,7 +1582,7 @@ static int mount_enumerate(Manager *m) {
/* Dispatch this before we dispatch SIGCHLD, so that
* we always get the events from /proc/self/mountinfo
- * before the SIGCHLD of /bin/mount. */
+ * before the SIGCHLD of /usr/bin/mount. */
r = sd_event_source_set_priority(m-mount_event_source, -10);
if (r 0)
goto fail;
diff --git a/src/core/mount.h b/src/core/mount.h
index a01e6da..280ea0d 100644
--- a/src/core/mount.h
+++ b/src/core/mount.h
@@ -28,8 +28,8 @@ typedef struct Mount Mount;
typedef enum MountState {
MOUNT_DEAD,
-
Re: [systemd-devel] [PATCH v2 1/2] nspawn: only mount the cgroup root if it's not already mounted
On Wed, 13.05.15 15:45, Iago López Galeiras (i...@endocode.com) wrote:
This allows the user to set the cgroups manually before calling
nspawn.
Applied! Thanks!
---
src/nspawn/nspawn.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 8c91726..f292c63 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1031,15 +1031,16 @@ static int mount_all(const char *dest) {
} MountPoint;
static const MountPoint mount_table[] = {
-{ proc, /proc, proc, NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
-{ /proc/sys, /proc/sys, NULL,NULL,MS_BIND,
true }, /* Bind mount first */
-{ NULL,/proc/sys, NULL,NULL,
MS_BIND|MS_RDONLY|MS_REMOUNT, true }, /* Then, make it r/o */
-{ sysfs, /sys, sysfs, NULL,
MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
-{ tmpfs, /dev, tmpfs, mode=755,
MS_NOSUID|MS_STRICTATIME, true },
-{ devpts,/dev/pts,
devpts,newinstance,ptmxmode=0666,mode=620,gid= STRINGIFY(TTY_GID),
MS_NOSUID|MS_NOEXEC, true },
-{ tmpfs, /dev/shm, tmpfs, mode=1777,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
-{ tmpfs, /run, tmpfs, mode=755,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
-{ tmpfs, /tmp, tmpfs, mode=1777,
MS_STRICTATIME, true },
+{ proc, /proc, proc, NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
+{ /proc/sys, /proc/sys, NULL,NULL,
MS_BIND,true }, /* Bind mount first */
+{ NULL,/proc/sys, NULL,NULL,
MS_BIND|MS_RDONLY|MS_REMOUNT, true }, /* Then, make it r/o */
+{ sysfs, /sys, sysfs, NULL,
MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, true },
+{ tmpfs, /sys/fs/cgroup, tmpfs, mode=755,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, true },
+{ tmpfs, /dev, tmpfs, mode=755,
MS_NOSUID|MS_STRICTATIME, true },
+{ devpts,/dev/pts,
devpts,newinstance,ptmxmode=0666,mode=620,gid= STRINGIFY(TTY_GID),
MS_NOSUID|MS_NOEXEC, true },
+{ tmpfs, /dev/shm, tmpfs, mode=1777,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
+{ tmpfs, /run, tmpfs, mode=755,
MS_NOSUID|MS_NODEV|MS_STRICTATIME, true },
+{ tmpfs, /tmp, tmpfs, mode=1777,
MS_STRICTATIME, true },
#ifdef HAVE_SELINUX
{ /sys/fs/selinux, /sys/fs/selinux, NULL, NULL, MS_BIND,
false }, /* Bind mount first */
{ NULL, /sys/fs/selinux, NULL, NULL,
MS_BIND|MS_RDONLY|MS_REMOUNT, false }, /* Then, make it r/o */
@@ -1324,9 +1325,6 @@ static int mount_cgroup(const char *dest) {
if (r 0)
return log_error_errno(r, Failed to determine our own
cgroup path: %m);
-cgroup_root = strjoina(dest, /sys/fs/cgroup);
-if (mount(tmpfs, cgroup_root, tmpfs,
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, mode=755) 0)
-return log_error_errno(errno, Failed to mount tmpfs to
/sys/fs/cgroup: %m);
for (;;) {
_cleanup_free_ char *controller = NULL, *origin = NULL,
*combined = NULL;
@@ -1386,6 +1384,7 @@ static int mount_cgroup(const char *dest) {
if (mount(NULL, systemd_root, NULL,
MS_BIND|MS_REMOUNT|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, NULL) 0)
return log_error_errno(errno, Failed to mount cgroup root
read-only: %m);
+cgroup_root = strjoina(dest, /sys/fs/cgroup);
if (mount(NULL, cgroup_root, NULL,
MS_REMOUNT|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME|MS_RDONLY, mode=755)
0)
return log_error_errno(errno, Failed to remount %s
read-only: %m, cgroup_root);
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
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] dev-root.device is not active, results in an umount spree
Hey Dimitri, Dimitri John Ledkov [2015-05-13 12:48 +0100]: Yet, dev-root.device is dead: # systemctl status dev-root.device ● dev-root.device Loaded: loaded Active: inactive (dead) This is very bad. As a harmless action like following: # mount --bind /opt /opt Results in opt.mount unit to be generated which BindsTo dev-root.device, which is inactive, thus systemd tries to stop that unit straight away, and umount fails and is retried infinitely... effectively DoSing init. For the record, I got a similar bug report a while ago: https://bugs.launchpad.net/systemd/+bug/102 This is reproducible in a container with udev running, see the small reproducer in the bug trail. This is on my plate to investigate/fix, I just got interrupted by a couple of security issues, so not this week. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] dev-root.device is not active, results in an umount spree
Heya, On 13 May 2015 at 12:53, Martin Pitt martin.p...@ubuntu.com wrote: Hey Dimitri, Dimitri John Ledkov [2015-05-13 12:48 +0100]: Yet, dev-root.device is dead: # systemctl status dev-root.device ● dev-root.device Loaded: loaded Active: inactive (dead) This is very bad. As a harmless action like following: # mount --bind /opt /opt Results in opt.mount unit to be generated which BindsTo dev-root.device, which is inactive, thus systemd tries to stop that unit straight away, and umount fails and is retried infinitely... effectively DoSing init. For the record, I got a similar bug report a while ago: https://bugs.launchpad.net/systemd/+bug/102 This is reproducible in a container with udev running, see the small reproducer in the bug trail. This is on my plate to investigate/fix, I just got interrupted by a couple of security issues, so not this week. Yes, quite. So this affects / affected rocket, dracut, ostree, rkt and any other similar containers / VMs. Colin, has this been addressed and fixed at all? Or was this workaround with don't do this. My current plug is to exclude umount har har. -- Regards, Dimitri. Pura Vida! https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=
On Tue, 12.05.15 18:04, Karel Zak (k...@redhat.com) wrote:
Currently we have no way how to specify dependencies between fstab
entries (or another units) in the /etc/fstab. It means that users are
forced to bypass fstab and write .mount units manually.
Years ago Lennart suggested to add
x-systemd.after=PATH
- to specify dependence an another mount (PATH is translated to PATH.mount)
x-systemd.after=UNIT
- to specify dependence on arbitrary UNIT
The x-systemd.after= is implemented by After= and Requires=.
Hmm, I am pretty sure x-systemd.after= should only create After=
dependencies, not Requires=. I'd be open though to add
x-systemd.requires= which adds both After= and Requires=.
(So far we never augmented ordering deps by requirement deps, but
sometimes we do augment requirement deps with ordering deps, for
example in many cases for DefaultDependencies=yes logic. Hence we
should follow this here, too.)
Hence, please rename this option to x-systemd.requires=. Otherwise I
like the concept.
x-systemd.requires-mounts-for=PATH ...
- to specify dependence on another paths, implemented by
RequiresMountsFor=.
Sounds good.
varlistentry
+termoptionx-systemd.after=/option/term
+
+listitemparaConfigures varnameAfter=/varname and
varnameRequires=/varname dependence
s/dependence/dependency
+
+listitemparaConfigures varnameRequiresMountsFor=/varname
dependence between the mount and
+another mount. The argument is a space-separated list of absolute
paths. Note
+that filename/etc/fstab/filename format requires to escape space
as \040.
+See varnameRequiresMountsFor=/varname in
+
citerefentryrefentrytitlesystemd.unit/refentrytitlemanvolnum5/manvolnum/citerefentry
+for details./para/listitem
Hmm, I think it would be better to allow specifiying the option
multiple times rather than using spaces to seperate the values...
+r = fstab_filter_options(opts, x-systemd.after\0, NULL, arg,
NULL);
+if (r 0)
+return log_warning_errno(r, Failed to parse options: %m);
+if (r == 0)
+return 0;
+
+if (*arg == '/') {
+r = unit_name_from_path(arg, .mount, unit);
I figure this should simply use unit_name_mangle_with_suffix(), which
will do the right thing for device nodes, normal paths and unit
names. No need to manually mangle this...
+if (!automount opts) {
+ write_requires_after(f, opts);
+ write_requires_mounts_for(f, opts);
+}
+
This should probably propagate errors.
if (passno != 0) {
r = generator_write_fsck_deps(f, arg_dest, what, where,
fstype);
if (r 0)
@@ -315,6 +359,11 @@ static int add_mount(
Before=%s\n,
post);
+if (opts) {
+write_requires_after(f, opts);
+write_requires_mounts_for(f, opts);
+}
Same 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] systemctl: introduce -e and -d for start and stop
On Wed, 13.05.15 15:21, jsyna...@redhat.com (jsyna...@redhat.com) wrote: From: Jan Synacek jsyna...@redhat.com Hmm, do we really need two options for this? I mean, since -e would only ever be combined with start, and -d only with stop it could as well be a single option that works for both? Also, I am tempted to say that this should be reversed: instead of making this options that alter start/stop behaviour, it should be options that alter enable/disable behaviour, and actually take into account the precise units that were enabled, including everything referenced in the [Install] section of the unit files. hence, I think I would prefer something like this instead: systemctl enable --now foobar.service systemctl disable --now foobar.service Where --now simply means that the service shall be started after enabling, and stopped after disabling. Does this make sense? 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] unit: Set KillMode=mixed for services which use sulogin
2015-05-13 15:19 GMT+02:00 Lennart Poettering lenn...@poettering.net: sulogin generally does not set up a PAM session, and we indeed should allow processes like screen staying around in such a context. Hence KillMode=process is actually the right choice for all these services, indeed. Do you really think it makes sense to start screen from emergency/rescue mode? Imho those are the cases where you don't actually want stuff to stay around after you log out. Hence I figure the status quo for all of this is pretty OK already... Well, I was intending to commit my original patch, which only uses KillMode=mixed for services which use sulogin, i.e. emergency.service, rescue.service and console-shell.service. See the original bug that triggered this patch [1]. We don't really want a stray bash process to stay around which potentially fights with sulogin over the input. Michael [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784238 -- Why is it that all of the instruments seeking intelligent life in the universe are pointed away from Earth? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: introduce -e and -d for start and stop
On Wed, May 13, 2015 at 4:36 PM, Lennart Poettering lenn...@poettering.net wrote: On Wed, 13.05.15 15:21, jsyna...@redhat.com (jsyna...@redhat.com) wrote: From: Jan Synacek jsyna...@redhat.com Hmm, do we really need two options for this? I mean, since -e would only ever be combined with start, and -d only with stop it could as well be a single option that works for both? Would be less of a problem if subcommands could have their own options separate from global ones... -- Mantas Mikulėnas graw...@gmail.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] dev-root.device is not active, results in an umount spree
I am booting without initramfs, using a plan9 filesystem as rootfs in a kvm. Thus my /proc/cmdline has: root=/dev/root rootflags=rw,trans=virtio,version=9p2000.L rootfstype=9p # mount /dev/root on / type 9p (rw,relatime,sync,dirsync,rw,trans=virtio,version=9p2000.L) Yet, dev-root.device is dead: # systemctl status dev-root.device ● dev-root.device Loaded: loaded Active: inactive (dead) This is very bad. As a harmless action like following: # mount --bind /opt /opt Results in opt.mount unit to be generated which BindsTo dev-root.device, which is inactive, thus systemd tries to stop that unit straight away, and umount fails and is retried infinitely... effectively DoSing init. What am I missing and/or how can I force make dev-root.device to be considered active? I am planning to locally patch mount_add_device_links to skip if what is /dev/root, to avoid a call to unit_add_node_link... But I'm not sure if this is the right thing to do or not. -- Regards, Dimitri. Pura Vida! https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] dev-root.device is not active, results in an umount spree
On 13 May 2015 at 13:43, Lennart Poettering lenn...@poettering.net wrote: On Wed, 13.05.15 12:48, Dimitri John Ledkov (dimitri.j.led...@intel.com) wrote: I am booting without initramfs, using a plan9 filesystem as rootfs in a kvm. Thus my /proc/cmdline has: root=/dev/root rootflags=rw,trans=virtio,version=9p2000.L rootfstype=9p # mount /dev/root on / type 9p (rw,relatime,sync,dirsync,rw,trans=virtio,version=9p2000.L) Yet, dev-root.device is dead: # systemctl status dev-root.device ● dev-root.device Loaded: loaded Active: inactive (dead) Yeah, this /dev/root thing is really weird in the kernel. It's not an actual device, it's just a weird string. We probably should filter it out entirely, and never ever generate a unit dependency for it. Please check if this fixes things for you: http://cgit.freedesktop.org/systemd/systemd/commit/?id=7ba2711d3fd283c389db2a1e7b9598ba9f0dac0c Yeap. All good now, can this go into v219-stable tree as well please? That said, I figure you should backport 628c89cc68ab96fce2de7ebba5933725d147aecc and friends to your tree, which should also make this problem go away for you. I have these via stable tree. I am planning to locally patch mount_add_device_links to skip if what is /dev/root, to avoid a call to unit_add_node_link... But I'm not sure if this is the right thing to do or not. It is, it's the change I made now, too. Ok, cool. -- Regards, Dimitri. Pura Vida! https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Default to /usr/bin/u?mount, configurable, rather than hard-coded /bin/u?mount.
On 13 May 2015 at 14:00, Lennart Poettering lenn...@poettering.net wrote:
On Mon, 11.05.15 16:58, Dimitri John Ledkov (dimitri.j.led...@intel.com)
wrote:
Hmm, all the other AM_PATH_PROG variables do not carry a _BIN suffix,
should these two? I don't think so.
(In general, I am not too big a fan of abbreviating things
unnecessarily, unless this is commonly done elsewhere...)
There is already MOUNT define used elsewhere in the code...
DEFINE_CAST(MOUNT, Mount);
Hence the _BIN suffix.
Otherwise looks fine,
Lennart
---
Makefile.am | 2 ++
configure.ac| 3 +++
src/core/mount.c| 6 +++---
src/remount-fs/remount-fs.c | 10 +-
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index e4d00a8..0ff11cc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -208,6 +208,8 @@ AM_CPPFLAGS = \
-DPOLKIT_AGENT_BINARY_PATH=\$(bindir)/pkttyagent\ \
-DQUOTACHECK=\$(QUOTACHECK)\ \
-DKEXEC=\$(KEXEC)\ \
+ -DMOUNT_BIN=\$(MOUNT_BIN)\ \
+ -DUMOUNT_BIN=\$(UMOUNT_BIN)\ \
-DLIBDIR=\$(libdir)\ \
-DROOTLIBDIR=\$(rootlibdir)\ \
-DROOTLIBEXECDIR=\$(rootlibexecdir)\ \
diff --git a/configure.ac b/configure.ac
index 600e203..61dffc6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -100,6 +100,9 @@ AC_PATH_PROG([KEXEC], [kexec], [/usr/sbin/kexec],
[$PATH:/usr/sbin:/sbin])
AC_PATH_PROG([SULOGIN], [sulogin], [/usr/sbin/sulogin],
[$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([MOUNT_BIN], [mount], [/usr/bin/mount],
[$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([UMOUNT_BIN], [umount], [/usr/bin/umount],
[$PATH:/usr/sbin:/sbin])
+
AS_IF([! ln --relative --help /dev/null 21], [AC_MSG_ERROR([*** ln
doesn't support --relative ***])])
M4_DEFINES=
diff --git a/src/core/mount.c b/src/core/mount.c
index 65a66b4..a370c74 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -871,7 +871,7 @@ static void mount_enter_unmounting(Mount *m) {
m-control_command_id = MOUNT_EXEC_UNMOUNT;
m-control_command = m-exec_command + MOUNT_EXEC_UNMOUNT;
-r = exec_command_set(m-control_command, /bin/umount, m-where,
NULL);
+r = exec_command_set(m-control_command, UMOUNT_BIN, m-where,
NULL);
if (r = 0 UNIT(m)-manager-running_as == SYSTEMD_SYSTEM)
r = exec_command_append(m-control_command, -n, NULL);
if (r 0)
@@ -924,7 +924,7 @@ static void mount_enter_mounting(Mount *m) {
if (r 0)
goto fail;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_BIN,
m-parameters_fragment.what, m-where,
NULL);
if (r = 0 UNIT(m)-manager-running_as ==
SYSTEMD_SYSTEM)
r = exec_command_append(m-control_command, -n,
NULL);
@@ -973,7 +973,7 @@ static void mount_enter_remounting(Mount *m) {
else
o = remount;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_BIN,
m-parameters_fragment.what, m-where,
-o, o, NULL);
if (r = 0 UNIT(m)-manager-running_as ==
SYSTEMD_SYSTEM)
diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c
index 70dacfa..ff7de2c 100644
--- a/src/remount-fs/remount-fs.c
+++ b/src/remount-fs/remount-fs.c
@@ -94,15 +94,15 @@ int main(int argc, char *argv[]) {
const char *arguments[5];
/* Child */
-arguments[0] = /bin/mount;
+arguments[0] = MOUNT_BIN;
arguments[1] = me-mnt_dir;
arguments[2] = -o;
arguments[3] = remount;
arguments[4] = NULL;
-execv(/bin/mount, (char **) arguments);
+execv(MOUNT_BIN, (char **) arguments);
-log_error_errno(errno, Failed to execute
/bin/mount: %m);
+log_error_errno(errno, Failed to execute
MOUNT_BIN : %m);
_exit(EXIT_FAILURE);
}
@@ -142,9 +142,9 @@ int main(int argc, char *argv[]) {
if (s) {
if (!is_clean_exit(si.si_code, si.si_status, NULL))
{
if (si.si_code == CLD_EXITED)
-log_error(/bin/mount for %s exited
with exit status %i., s, si.si_status);
+log_error(MOUNT_BIN for %s exited
with exit status %i., s, si.si_status);
else
-log_error(/bin/mount
[systemd-devel] [PATCH v2] Default to /usr/bin/u?mount, configurable, rather than hard-coded /bin/u?mount.
---
Makefile.am | 2 ++
configure.ac| 3 +++
src/core/execute.h | 2 +-
src/core/mount.c| 12 ++--
src/core/mount.h| 4 ++--
src/remount-fs/remount-fs.c | 10 +-
6 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index e4d00a8..e8a329f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -208,6 +208,8 @@ AM_CPPFLAGS = \
-DPOLKIT_AGENT_BINARY_PATH=\$(bindir)/pkttyagent\ \
-DQUOTACHECK=\$(QUOTACHECK)\ \
-DKEXEC=\$(KEXEC)\ \
+ -DMOUNT_PATH=\$(MOUNT_PATH)\ \
+ -DUMOUNT_PATH=\$(UMOUNT_PATH)\ \
-DLIBDIR=\$(libdir)\ \
-DROOTLIBDIR=\$(rootlibdir)\ \
-DROOTLIBEXECDIR=\$(rootlibexecdir)\ \
diff --git a/configure.ac b/configure.ac
index 600e203..70e594d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -100,6 +100,9 @@ AC_PATH_PROG([KEXEC], [kexec], [/usr/sbin/kexec],
[$PATH:/usr/sbin:/sbin])
AC_PATH_PROG([SULOGIN], [sulogin], [/usr/sbin/sulogin],
[$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([MOUNT_PATH], [mount], [/usr/bin/mount], [$PATH:/usr/sbin:/sbin])
+AC_PATH_PROG([UMOUNT_PATH], [umount], [/usr/bin/umount],
[$PATH:/usr/sbin:/sbin])
+
AS_IF([! ln --relative --help /dev/null 21], [AC_MSG_ERROR([*** ln doesn't
support --relative ***])])
M4_DEFINES=
diff --git a/src/core/execute.h b/src/core/execute.h
index a0908e0..f5d5c1d 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -165,7 +165,7 @@ struct ExecContext {
/* This is not exposed to the user but available
* internally. We need it to make sure that whenever we spawn
- * /bin/mount it is run in the same process group as us so
+ * /usr/bin/mount it is run in the same process group as us so
* that the autofs logic detects that it belongs to us and we
* don't enter a trigger loop. */
bool same_pgrp;
diff --git a/src/core/mount.c b/src/core/mount.c
index 8853311..8ef3d17 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -135,8 +135,8 @@ static void mount_init(Unit *u) {
m-exec_context.std_error = u-manager-default_std_error;
}
-/* We need to make sure that /bin/mount is always called in
- * the same process group as us, so that the autofs kernel
+/* We need to make sure that /usr/bin/mount is always called
+ * in the same process group as us, so that the autofs kernel
* side doesn't send us another mount request while we are
* already trying to comply its last one. */
m-exec_context.same_pgrp = true;
@@ -833,7 +833,7 @@ static void mount_enter_unmounting(Mount *m) {
m-control_command_id = MOUNT_EXEC_UNMOUNT;
m-control_command = m-exec_command + MOUNT_EXEC_UNMOUNT;
-r = exec_command_set(m-control_command, /bin/umount, m-where,
NULL);
+r = exec_command_set(m-control_command, UMOUNT_PATH, m-where, NULL);
if (r = 0 UNIT(m)-manager-running_as == MANAGER_SYSTEM)
r = exec_command_append(m-control_command, -n, NULL);
if (r 0)
@@ -884,7 +884,7 @@ static void mount_enter_mounting(Mount *m) {
if (r 0)
goto fail;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_PATH,
m-parameters_fragment.what, m-where,
NULL);
if (r = 0 UNIT(m)-manager-running_as == MANAGER_SYSTEM)
r = exec_command_append(m-control_command, -n,
NULL);
@@ -931,7 +931,7 @@ static void mount_enter_remounting(Mount *m) {
else
o = remount;
-r = exec_command_set(m-control_command, /bin/mount,
+r = exec_command_set(m-control_command, MOUNT_PATH,
m-parameters_fragment.what, m-where,
-o, o, NULL);
if (r = 0 UNIT(m)-manager-running_as == MANAGER_SYSTEM)
@@ -1582,7 +1582,7 @@ static int mount_enumerate(Manager *m) {
/* Dispatch this before we dispatch SIGCHLD, so that
* we always get the events from /proc/self/mountinfo
- * before the SIGCHLD of /bin/mount. */
+ * before the SIGCHLD of /usr/bin/mount. */
r = sd_event_source_set_priority(m-mount_event_source, -10);
if (r 0)
goto fail;
diff --git a/src/core/mount.h b/src/core/mount.h
index a01e6da..280ea0d 100644
--- a/src/core/mount.h
+++ b/src/core/mount.h
@@ -28,8 +28,8 @@ typedef struct Mount Mount;
typedef enum MountState {
MOUNT_DEAD,
-MOUNT_MOUNTING, /* /bin/mount is running, but the mount
is not done yet. */
-MOUNT_MOUNTING_DONE, /* /bin/mount is running, and the mount
is done. */
+
Re: [systemd-devel] [PATCH] systemctl: introduce -e and -d for start and stop
Patchset imported to github. Pull request: https://github.com/systemd-devs/systemd/compare/master...systemd-mailing-devs:1431523273-24283-1-git-send-email-jsynacek%40redhat.com -- Generated by https://github.com/haraldh/mail2git ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Crash with extra space after Exec prefix
Hello all, I got a report [1] that you can trivially crash systemd (pid1) at boot by creating a unit with an Exec= line with a modifier and a space: $ cat /tmp/foo.service [Service] ExecStart=- /bin/echo hello $ systemd-analyze verify /tmp/foo.service Assertion 'skip l' failed at ../src/core/load-fragment.c:607, function config_parse_exec(). Aborting. Aborted (core dumped) systemd pid 1 will crash the same way at boot, but with systemd-analyze it's less harmful to test :-) So, obviously we need to fix the crash; but I was wondering what the desired behaviour should be? In the sense of be liberal what you accept I think the extra space(s) should just be ignored; or should that count as an error and the unit get rejected? Thanks, Martin [1] https://launchpad.net/bugs/1454173 -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin
2015-05-13 15:45 GMT+02:00 Lennart Poettering lenn...@poettering.net: Well, I am pretty sure that in this case, it should be sulogin that propagates the shutdown request to the shell it spawned, but we should not do it otherwise. Note that by default we don't even clean up processes of unprivileged users on logout. You have to turn this on via KillUserProcesses= explicitly. And if we don't do this for unprivileged users, we certainly shouldn't do it for debug shells either Well, it's not the debug shell, it's emergency/rescue mode. You typically end up there, because you have a fatal (config) error. You then fix the problem and exit the shell to boot into the regular mode. I don't see the point of letting stuff from emergency/rescue mode continue to run after you've stopped it. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784238 That bug reports is long... From what I got this really looks like something to fix in Debian's sulogin implementation really. You are correct. This turned out to be a bug in Debian's sulogin implementation (which is currently provided by sysvinit-utils). I filed bugs to have util-linux provide sulogin [1], since I wasn't able to reproduce the problem with that implementation. Still, while handling this bug report, I couldn't think of a good reason to not use KillMode=mixed for emergency/rescue mode, that's why I proposed this patch. I see that you don't like it, so I'll probably just ship it downstream for now. And drop it eventually, once we have u-l's sulogin as default. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784566#77 -- Why is it that all of the instruments seeking intelligent life in the universe are pointed away from Earth? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin
On Wed, 13.05.15 16:18, Michael Biebl (mbi...@gmail.com) wrote: 2015-05-13 15:45 GMT+02:00 Lennart Poettering lenn...@poettering.net: Well, I am pretty sure that in this case, it should be sulogin that propagates the shutdown request to the shell it spawned, but we should not do it otherwise. Note that by default we don't even clean up processes of unprivileged users on logout. You have to turn this on via KillUserProcesses= explicitly. And if we don't do this for unprivileged users, we certainly shouldn't do it for debug shells either Well, it's not the debug shell, it's emergency/rescue mode. You typically end up there, because you have a fatal (config) error. Well, if you in either mode, you ran into problems, and you have to debug them now. I am pretty sure we should make more people unhappy if we killed everything they forked off from these recovery shells, then we'd make happy by cleaning up after them... KillMode=mixed appears OK as a temporary fix to work-around a broken sulogin version, but I dont think it makes a good default otherwise. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: fix memory leak in manager_run_generators()
On Mon, 11.05.15 23:30, Cristian Rodríguez (crrodrig...@opensuse.org) wrote:
If systemd is built with GCC address sanitizer or leak sanitizer
the following memory leak ocurs:
Thanks! Applied!
May 12 02:02:46 linux.site systemd[326]:
=
May 12 02:02:46 linux.site systemd[326]: ==326==ERROR: LeakSanitizer:
detected memory leaks
May 12 02:02:46 linux.site systemd[326]: Direct leak of 101 byte(s) in 3
object(s) allocated from:
May 12 02:02:46 linux.site systemd[326]: #0 0x7fd1f504993f in strdup
(/usr/lib64/libasan.so.2+0x6293f)
May 12 02:02:46 linux.site systemd[326]: #1 0x55d6ffac5336 in strv_new_ap
src/shared/strv.c:163
May 12 02:02:46 linux.site systemd[326]: #2 0x55d6ffac56a9 in strv_new
src/shared/strv.c:185
May 12 02:02:46 linux.site systemd[326]: #3 0x55d6ffa80272 in generator_paths
src/shared/path-lookup.c:223
May 12 02:02:46 linux.site systemd[326]: #4 0x55d6ff9bdb0f in
manager_run_generators src/core/manager.c:2828
May 12 02:02:46 linux.site systemd[326]: #5 0x55d6ff9b1a10 in manager_startup
src/core/manager.c:1121
May 12 02:02:46 linux.site systemd[326]: #6 0x55d6ff9a78e3 in main
src/core/main.c:1667
May 12 02:02:46 linux.site systemd[326]: #7 0x7fd1f394e8c4 in
__libc_start_main (/lib64/libc.so.6+0x208c4)
May 12 02:02:46 linux.site systemd[326]: Direct leak of 29 byte(s) in 1
object(s) allocated from:
May 12 02:02:46 linux.site systemd[326]: #0 0x7fd1f504993f in strdup
(/usr/lib64/libasan.so.2+0x6293f)
May 12 02:02:46 linux.site systemd[326]: #1 0x55d6ffac5288 in strv_new_ap
src/shared/strv.c:152
May 12 02:02:46 linux.site systemd[326]: #2 0x55d6ffac56a9 in strv_new
src/shared/strv.c:185
May 12 02:02:46 linux.site systemd[326]: #3 0x55d6ffa80272 in generator_paths
src/shared/path-lookup.c:223
May 12 02:02:46 linux.site systemd[326]: #4 0x55d6ff9bdb0f in
manager_run_generators src/core/manager.c:2828
May 12 02:02:46 linux.site systemd[326]: #5 0x55d6ff9b1a10 in manager_startup
src/core/manager.c:1121
May 12 02:02:46 linux.site systemd[326]: #6 0x55d6ff9a78e3 in main
src/core/main.c:1667
May 12 02:02:46 linux.site systemd[326]: #7 0x7fd1f394e8c4 in
__libc_start_main (/lib64/libc.so.6+0x208c4)
May 12 02:02:46 linux.site systemd[326]: SUMMARY: AddressSanitizer: 130
byte(s) leaked in 4 allocation(s).
There is a leak due to the the use of cleanup_free instead _cleanup_strv_free_
---
src/core/manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/manager.c b/src/core/manager.c
index 28b9427..8254090 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2815,7 +2815,7 @@ static void trim_generator_dir(Manager *m, char
**generator) {
}
static int manager_run_generators(Manager *m) {
-_cleanup_free_ char **paths = NULL;
+_cleanup_strv_free_ char **paths = NULL;
const char *argv[5];
char **path;
int r;
--
2.3.7
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
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 2/2] nspawn: skip symlink to a combined cgroup hierarchy if it already exists
On Wed, 13.05.15 15:45, Iago López Galeiras (i...@endocode.com) wrote:
If a symlink to a combined cgroup hierarchy already exists and points to
the right path, skip it. This avoids an error when the cgroups are set
manually before calling nspawn.
Thanks! Applied with some minor modifications.
---
src/nspawn/nspawn.c | 10 --
src/shared/util.c | 23 +++
src/shared/util.h | 1 +
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index f292c63..e9e703f 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1365,8 +1365,14 @@ static int mount_cgroup(const char *dest) {
if (r 0)
return r;
-if (symlink(combined, target) 0)
-return log_error_errno(errno, Failed to
create symlink for combined hierarchy: %m);
+r = symlink_idempotent(combined, target);
+if (r 0) {
+if (r == -EINVAL) {
+log_error(Invalid existing symlink
for combined hierarchy);
+return r;
+} else
+return log_error_errno(r, Failed to
create symlink for combined hierarchy: %m);
+}
}
}
diff --git a/src/shared/util.c b/src/shared/util.c
index 466dce4..22e00a6 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -2765,6 +2765,29 @@ int symlink_atomic(const char *from, const char *to) {
return 0;
}
+int symlink_idempotent(const char *from, const char *to) {
+_cleanup_free_ char *p = NULL;
+int r;
+
+assert(from);
+assert(to);
+
+if (symlink(from, to) 0) {
+if (errno == EEXIST) {
+r = readlink_malloc(to, p);
+if (r 0)
+return r;
+
+if (!streq(p, from)) {
+return -EINVAL;
+}
+} else
+return -errno;
+}
+
+return 0;
+}
+
int mknod_atomic(const char *path, mode_t mode, dev_t dev) {
_cleanup_free_ char *t = NULL;
int r;
diff --git a/src/shared/util.h b/src/shared/util.h
index 4a67d5c..8565fd6 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -404,6 +404,7 @@ bool machine_name_is_valid(const char *s) _pure_;
char* strshorten(char *s, size_t l);
+int symlink_idempotent(const char *from, const char *to);
int symlink_atomic(const char *from, const char *to);
int mknod_atomic(const char *path, mode_t mode, dev_t dev);
--
2.4.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] systemd-socket-proxyd usage: remote's directly ping-/telnet-able, but via proxy Network is unreachable?
I'm attempting to use systemd's socket-proxyd to forward a static IP on a VPS, over a VPN to a mailserver at a remote office location, listening at a NAT'd, internal IP. The mailserver listens @ IP = 10.2.2.12. The staticIP at the VPS is IP = 111.222.333.444 The VPS's staticIP is pingable from the VPS ping -c 1 111.222.333.444 PING 111.222.333.444 (111.222.333.444) 56(84) bytes of data. 64 bytes from 111.222.333.444: icmp_seq=1 ttl=64 time=0.060 ms --- 111.222.333.444 ping statistics --- 1 packets transmitted, 1 received, 0% packet loss, time 0ms rtt min/avg/max/mdev = 0.060/0.060/0.060/0.000 ms The office's mailserver is pingable over the VPN link ping -c 1 10.2.2.12 PING 10.2.2.12 (10.2.2.12) 56(84) bytes of data. 64 bytes from 10.2.2.12: icmp_seq=1 ttl=63 time=46.8 ms --- 10.2.2.12 ping statistics --- 1 packets transmitted, 1 received, 0% packet loss, time 0ms rtt min/avg/max/mdev = 46.817/46.817/46.817/0.000 ms I can connect to the SMTP server from the VPS as well telnet 10.2.2.12 25 Trying 10.2.2.12... Connected to 10.2.2.12. Escape character is '^]'. 220 mx.mydomain.com ESMTP . No UCE permitted. ^] telnet quit Connection closed. Cribbing from the nginx examples at http://www.freedesktop.org/software/systemd/man/systemd-socket-proxyd.html I've created a socket unit to listen on the staticIP cat /etc/systemd/system/proxy-to-mailserver.socket [Socket] ListenStream=111.222.333.444:25 [Install] WantedBy=sockets.target and a service unit to forward the traffic to the mailserver listener cat /etc/systemd/system/proxy-to-mailserver.service [Unit] Requires=openvpn.service After=openvpn.service [Service] ExecStart=/usr/lib/systemd/systemd-socket-proxyd 10.2.2.12:25 PrivateTmp=yes PrivateNetwork=yes Enable/start of the socket works systemctl enable proxy-to-mailserver.socket systemctl start proxy-to-mailserver.socket systemctl status proxy-to-mailserver.socket proxy-to-mailserver.socket Loaded: loaded (/etc/systemd/system/proxy-to-mailserver.socket; enabled) Active: active (listening) since Wed 2015-05-13 21:22:41 PDT; 2min 37s ago Listen: 111.222.333.444:25 (Stream) IIUC, at this point I should be able to connect to the mailserver @ the forwarded staticIP. But, at the VPS, the connection is immediately dropped telnet 111.222.333.444 25 Trying 111.222.333.444... Connected to 111.222.333.444. Escape character is '^]'. Connection closed by foreign host. and @ `journalctl -f`, May 13 21:36:57 edge.mydomain.com systemd-socket-proxyd[5291]: Failed to connect to remote host: Network is unreachable I'm not clear why I'm seeing Network is unreachable when the remote host is clearly pingable and accessible via telnet. I suspect 'PrivateNetwork' may have a hand in it, but I'm fuzzy on usage. What's missing or incorrect about that ^^ scenario/usage? Thanks. pgnd ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-socket-proxyd usage: remote's directly ping-/telnet-able, but via proxy Network is unreachable?
Simple fix. As usual, 5 minutes after posting ... With a helpful prod @ #irc to look at what PrivateNetwork does @ http://www.freedesktop.org/software/systemd/man/systemd.exec.html If true, sets up a new network namespace for the executed processes and configures only the loopback network device lo inside it. Then changing, - PrivateNetwork=yes + PrivateNetwork=no does the trick. Remote SMTP now accessible over the establish proxy link. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: fix event source annotations
On Wed, 29.04.15 21:29, Mantas Mikulėnas (graw...@gmail.com) wrote:
These looked like a mass-replace gone slightly wrong – two statements
with no { }'s, and no error checking.
---
src/core/busname.c | 4 +++-
src/core/manager.c | 5 -
src/core/socket.c | 3 ++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/core/busname.c b/src/core/busname.c
index 48cc045..94db122 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -291,13 +291,15 @@ static int busname_watch_fd(BusName *n) {
r = sd_event_source_set_enabled(n-starter_event_source,
SD_EVENT_ON);
else
r = sd_event_add_io(UNIT(n)-manager-event,
n-starter_event_source, n-starter_fd, EPOLLIN, busname_dispatch_io, n);
-(void)
sd_event_source_set_description(n-starter_event_source, busname-starter);
+
if (r 0) {
log_unit_warning_errno(UNIT(n)-id, r, Failed to watch
starter fd: %m);
busname_unwatch_fd(n);
return r;
}
+(void) sd_event_source_set_description(n-starter_event_source,
busname-starter);
+
Hmm, it sounds a bit unnecessary to set the same description over and
over again. We should really only set it when we allocate a new event
source, and not every single time we toggle is enablement state.
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1272,11 +1272,12 @@ static int socket_watch_fds(Socket *s) {
else
r = sd_event_add_io(UNIT(s)-manager-event,
p-event_source, p-fd, EPOLLIN, socket_dispatch_io, p);
-(void)
sd_event_source_set_description(p-event_source, socket-port-io);
if (r 0) {
log_unit_warning_errno(UNIT(s)-id, r, Failed to
watch listening fds: %m);
goto fail;
}
+
+(void) sd_event_source_set_description(p-event_source,
socket-port-io);
Same here...
I fixed that now.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Crash with extra space after Exec prefix
On Wed, 13.05.15 17:01, Martin Pitt (martin.p...@ubuntu.com) wrote: Hello all, I got a report [1] that you can trivially crash systemd (pid1) at boot by creating a unit with an Exec= line with a modifier and a space: $ cat /tmp/foo.service [Service] ExecStart=- /bin/echo hello $ systemd-analyze verify /tmp/foo.service Assertion 'skip l' failed at ../src/core/load-fragment.c:607, function config_parse_exec(). Aborting. Aborted (core dumped) Urks, that function isn't particular pretty anyway... I figure we should rewrite this eventually to use unquote_first_word() instead of FOREACH_WORD_QUOTED, which is pretty ugly... systemd pid 1 will crash the same way at boot, but with systemd-analyze it's less harmful to test :-) So, obviously we need to fix the crash; but I was wondering what the desired behaviour should be? In the sense of be liberal what you accept I think the extra space(s) should just be ignored; or should that count as an error and the unit get rejected? Neither. It should be considered an error, logged about, but the line should be ignored and we should continue. This is how we usually do it so far, to ensure unit files stay relatively portable between version, but on the other hand we aren't too liberal with accepting any data. I think the be liberal what you accept thing really applies if you have a true interchange format, that is read and written by multiple different implementations. But this is not of this kind, since there's exactly one implementation reading it that defines its semantics. 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] [PATCH v3] core: Private*/Protect* options with RootDirectory
On Tue, 12.05.15 15:36, Alban Crequy (alban.cre...@gmail.com) wrote:
diff --git a/src/core/execute.c b/src/core/execute.c
index 1a297ba..d4ccac6 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1277,6 +1277,7 @@ static int exec_child(
uid_t uid = UID_INVALID;
gid_t gid = GID_INVALID;
int i, r;
+bool new_mnt_namespace;
I don't like unnecessary abbreviations I must say. Just call this
new_mount_namespace or needs_mount_namespace or so... There's really
no need in sparing two characters, it certainly doesn't help
readability...
-if (!strv_isempty(context-read_write_dirs) ||
+new_mnt_namespace = !strv_isempty(context-read_write_dirs) ||
!strv_isempty(context-read_only_dirs) ||
!strv_isempty(context-inaccessible_dirs) ||
context-mount_flags != 0 ||
@@ -1563,8 +1564,9 @@ static int exec_child(
params-bus_endpoint_path ||
context-private_devices ||
context-protect_system != PROTECT_SYSTEM_NO ||
-context-protect_home != PROTECT_HOME_NO) {
+context-protect_home != PROTECT_HOME_NO;
I think we should take this one step further even, and move this whole
humgous if check into its own function.
Done so now in git.
+static int mount_move_root(const char *path) {
+if (chdir(path) 0) {
+return -errno;
+}
+
+if (mount(path, /, NULL, MS_MOVE, NULL) 0) {
+return -errno;
+}
+
+if (chroot(.) 0) {
+return -errno;
+}
+
+if (chdir(/) 0) {
+return -errno;
+}
+
+return 0;
+}
Please see CODING_STYLE, we do not add braces around single-line if
blocks. This is not PHP...
+
static int mount_dev(BindMount *m) {
static const char devnodes[] =
/dev/null\0
@@ -225,7 +246,13 @@ static int mount_dev(BindMount *m) {
dev_setup(temporary_mount);
-if (mount(dev, /dev/, NULL, MS_MOVE, NULL) 0) {
+/* Create the /dev directory if missing. It is more likely to be
+ * missing when the service is started with RootDirectory. This is
+ * consistent with mount units creating the mount points when
missing.
+ */
+mkdir_p_label (m-path, 0755);
Please see CODING_STYLE, no space between function name and opening (
please...
Also, we nowadays try to explicitly mark calls where we knowingly
ignore the exit code by casting its result to (void). This is helpful
to indicate to static checkers like coverity, whether a return value
may be safely ignored, or not.
+
+if (mount(dev, m-path, NULL, MS_MOVE, NULL) 0) {
r = -errno;
goto fail;
}
@@ -404,6 +431,7 @@ static int make_read_only(BindMount *m) {
}
int setup_namespace(
+const char* chroot,
Hmm, I'd avoid naming a variable like a function needlessly...
char** read_write_dirs,
char** read_only_dirs,
char** inaccessible_dirs,
@@ -449,37 +477,48 @@ int setup_namespace(
return r;
if (tmp_dir) {
-m-path = /tmp;
+m-path = strjoina(chroot?:, /tmp);
Hmm, this is very common expression that is used all over the place
now, I figure it's time to introduce a better expression for this,
that unifies this logic, and properly avoids creating duplicate
slashes in the resulting path.
I have thus added two calls now to path-util.c: prefix_root() and
prefix_roota(). The former allocates from the heap, the latter from
the stack using alloca(). In this case use prefix_roota() please.
m-mode = PRIVATE_TMP;
m++;
}
if (var_tmp_dir) {
-m-path = /var/tmp;
+m-path = strjoina(chroot?:, /var/tmp);
Same here...
m-mode = PRIVATE_VAR_TMP;
m++;
}
if (private_dev) {
-m-path = /dev;
+m-path = strjoina(chroot?:, /dev);
Same here...
m-mode = PRIVATE_DEV;
m++;
}
if (bus_endpoint_path) {
-m-path = bus_endpoint_path;
+m-path = strjoina(chroot?:,
bus_endpoint_path);
same here...
m-mode = PRIVATE_BUS_ENDPOINT;
m++;
}
if (protect_home != PROTECT_HOME_NO) {
-r = append_mounts(m, STRV_MAKE(-/home,
-/run/user, -/root), protect_home == PROTECT_HOME_READ_ONLY ? READONLY :
INACCESSIBLE);
+r = append_mounts(m,
Re: [systemd-devel] [PATCH] nspawn: cloexec extraneous fds
On Mon, 11.05.15 16:41, Alban Crequy (alban.cre...@gmail.com) wrote:
src/nspawn/nspawn.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 71a6239..2e45c3b 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -3739,6 +3739,9 @@ int main(int argc, char *argv[]) {
bool root_device_rw = true, home_device_rw = true, srv_device_rw =
true;
_cleanup_close_ int master = -1, image_fd = -1;
_cleanup_fdset_free_ FDSet *fds = NULL;
+_cleanup_fdset_free_ FDSet *misc_fds = NULL;
+int fd;
+Iterator i;
int r, n_fd_passed, loop_nr = -1;
char veth_name[IFNAMSIZ];
bool secondary = false, remove_subvol = false;
@@ -3775,7 +3778,11 @@ int main(int argc, char *argv[]) {
goto finish;
}
}
-fdset_close_others(fds);
+fdset_new_fill(misc_fds);
+FDSET_FOREACH(fd, fds, i) {
+fdset_remove(misc_fds, fd);
+}
+fdset_cloexec(misc_fds, true);
log_open();
Do we really need an extra FDSet object for this? Why not just remove
the fdset_close_others() from the nspawn parent and adding it to the
child process instead, without depending on O_CLOEXEC? Appears much
simpler to as it avoids keeping two fdsets around...
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] nspawn: check the pid in SIGCHLD handler before terminating the container
On Sun, 10.05.15 19:29, Alban Crequy (alban.cre...@gmail.com) wrote: From: Alban Crequy al...@endocode.com When a process starts systemd-nspawn with exec*() without fork(), systemd-nspawn can be the parent process of children processes unknown to systemd-nspawn. It can then receive the signal SIGCHLD for both the container leader process and the previously started processes. So it should distinguish them. Is this still relevant? systemd-nspawn uses waitid(P_PID) and specifies the container's main PID, thus it should explicitly only wait for that and not get confused by other PIDs. Hence I am not really getting what the patch is about... (I mean, it will not reap those other processes, but it will not get confused by them either...) I am pretty sure we should never bother with SIGCHLD for this. It's the wrong kind of notification. If this still is an issue, and this is about reaping unknown processes, then I'd be open to extending wait_for_terminate() to also reap all unknown processes while we wait for the one we really care about... 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] Crash with extra space after Exec prefix
Lennart Poettering [2015-05-13 17:55 +0200]: On Wed, 13.05.15 17:01, Martin Pitt (martin.p...@ubuntu.com) wrote: So, obviously we need to fix the crash; but I was wondering what the desired behaviour should be? In the sense of be liberal what you accept I think the extra space(s) should just be ignored; or should that count as an error and the unit get rejected? Neither. It should be considered an error, logged about, but the line should be ignored and we should continue. This is how we usually do it so far, to ensure unit files stay relatively portable between version, but on the other hand we aren't too liberal with accepting any data. You mean ignoring this single line, but still starting the unit (with any other Exec*=)? That feels quite odd to me, TBH -- it feels more robust if a unit is either completely valid, or completely inert? Anyway, I'm glad I asked. :-) Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-bus: fix memory leak in test-bus-chat
On Wed, May 13, 2015 at 8:01 AM, Daniel Mack dan...@zonque.org wrote:
We should still keep this flush, right?
-sd_bus_unref(bus);
}
The cleanup function already does :
static inline void sd_bus_close_unrefp(sd_bus **bus) {
if (*bus) {
sd_bus_flush(*bus);
sd_bus_close(*bus);
sd_bus_unref(*bus);
}
}
Or I am missing something ?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] networkd: don't touch global forwarding setting
On Sun, 10.05.15 20:52, Benedikt Morbach (benedikt.morb...@googlemail.com) wrote: This reverts commit 43c6d5abacaebf813845934ec8d5e5ee3c431854 (and a small part of 4046d8361c55c80ab8577aea52523b9e6eab0d0c) It turns out we don't actually need to set the global ip_forward setting. The only relevant setting is the one on each interface. Hmm, I tried to understand the kernel side for the setting to verify that but that code isn't particularly readable. How do ip_forward and conf/*/forwarding actually relate in detail? I mean, from playing around with it, usually echoing 1 into the global option also sets the local ones, and echoing 0 into the local option also unset the local ones, except not always: if some but not all local interfaces have it turned on, then setting 1 in the global setting doesn't do anything. Setting 0 in the global setting OTOH turns all off... So what are the precise semantics here? Also, do the local options ever propagate to the global one? What's the precise relation between conf/all/forwarding and ip_forward? It appears to do the very same thing? What's going on here? The actual IP forwarding code, does it ever check the global setting? Or only the local settings? I kinda would like to understand the actual behaviour before we fix this, because if we don't udnerstand the real behaviour it's really hard to fix this properly for good... Any ideas? 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] Crash with extra space after Exec prefix
On Wed, 13.05.15 19:11, Martin Pitt (martin.p...@ubuntu.com) wrote: Lennart Poettering [2015-05-13 17:55 +0200]: On Wed, 13.05.15 17:01, Martin Pitt (martin.p...@ubuntu.com) wrote: So, obviously we need to fix the crash; but I was wondering what the desired behaviour should be? In the sense of be liberal what you accept I think the extra space(s) should just be ignored; or should that count as an error and the unit get rejected? Neither. It should be considered an error, logged about, but the line should be ignored and we should continue. This is how we usually do it so far, to ensure unit files stay relatively portable between version, but on the other hand we aren't too liberal with accepting any data. You mean ignoring this single line, but still starting the unit (with any other Exec*=)? That feels quite odd to me, TBH -- it feels more robust if a unit is either completely valid, or completely inert? Well, for things like PrivateTmp= it's really the behaviour you want: if we cannot parse it, then we should log about it, but otherwise ignore it, after all the feature doesn't really have any effect on execution, it just adds a sandbox. Now, since PrivateTmp= is still relatively new, it's a good thing if old versions accept newer unit files where this setting is set or set to an unknown value, without completely failing, so that people can ship unit files with this, without completely breaking on old versions. This is similar for most other options. i.e. If we cannot parse Nice= or CPUPriority it doesn't really matter either... It's good if it works, and it should work, but it really shouldn't result in total failure if we cannot parse it properly. Now, you could argue that ExecStart= is not of this kind, and that not having it set properly should cause failure. But that's the case for ExecStart= only, for thing things like ExecStartPre= and friends it might already be quite different, they tend to be much less essential. Also, and most importantly, note that after parsing the unit file, systemd will actually do a final verification check at the time it marks the unit as fully loaded. Then, it will verify that an ExecStart= has actually been set, and if it is missing it will indicate this in the LoadState field, for clients to query. Or in other words: yes, some bits are essential to be set, but those should be checked for after parsing is complete, not already while parsing. 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 1/1] Fix typo
On Tue, 12.05.15 14:45, Peter Lemenkov (lemen...@gmail.com) wrote: Looks like sizeof(struct Header) is 240 not 224 Ah indeed. I figure I forgot to update that in 189 when I added the two most new fields. Signed-off-by: Peter Lemenkov lemen...@gmail.com We don't use S-o-b in systemd. Stripped this from the commit, and pushed. 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] Crash with extra space after Exec prefix
Am 13.05.2015 um 19:11 schrieb Martin Pitt: Lennart Poettering [2015-05-13 17:55 +0200]: On Wed, 13.05.15 17:01, Martin Pitt (martin.p...@ubuntu.com) wrote: So, obviously we need to fix the crash; but I was wondering what the desired behaviour should be? In the sense of be liberal what you accept I think the extra space(s) should just be ignored; or should that count as an error and the unit get rejected? Neither. It should be considered an error, logged about, but the line should be ignored and we should continue. This is how we usually do it so far, to ensure unit files stay relatively portable between version, but on the other hand we aren't too liberal with accepting any data. You mean ignoring this single line, but still starting the unit (with any other Exec*=)? That feels quite odd to me, TBH -- it feels more robust if a unit is either completely valid, or completely inert? no it is not - where do you draw the line if a unit contains options for systemd-216 and completly valid on F21 but the same src.rpm is used for F20 would you like to fail the service or just have the log noise of the unknown option? signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Dependency on timers.target
On Thu, 07.05.15 11:26, Bas van Dijk (v.dijk@gmail.com) wrote: Hello, I have a systemd timer (sensors-logger.timer) which requires another service (carbonCache.service) to be active: [Unit] Requires=carbonCache.service After=carbonCache.service Description=Log hardware sensors to graphite. [Timer] OnCalendar=*-*-* *:*:00 I noticed the following systemd warnings in my journal: Found ordering cycle on basic.target/start Found dependency on timers.target/start Found dependency on sensors-logger.timer/start Found dependency on carbonCache.service/start Found dependency on basic.target/start Breaking ordering cycle by deleting job timers.target/start Job timers.target/start deleted to break ordering cycle starting with basic.target/start Found ordering cycle on basic.target/start Found dependency on timers.target/start Found dependency on sensors-logger.timer/start Found dependency on carbonCache.service/start Found dependency on basic.target/start Unable to break cycle Requested transaction contains an unfixable cyclic ordering dependency: Transaction order is cyclic. See system logs for details. Apparantly timers.target has a dependency on sensors-logger.timer. However I didn't specify this. Is this done automatically by systemd? Yes, timers are by default ordered before timers.target. If so, is it possible to disable this and is it wise to do so? Set DefaultDependencies=no in the timer unit to turn this off. It should be OK to turn this off. 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] Shutting down service using systemd-nspawn
On Thu, 07.05.15 06:38, Peter Paule (systemd-de...@fedux.org) wrote: I implemented this now: http://cgit.freedesktop.org/systemd/systemd/commit/?id=c6c8f6e218995852350e5e35c080dec788c42c3f Thanks a lot. Sorry, have seen your mail to late - I'm trying out a new mua (sup) and I'm not that familiar with it yet. Do you think it makes sense to add something like `--pass-signals SIGHUP,SIGINT,SIGWINCH` as well? Making `systemd-nspawn` to pass those signals to PID 1 if it receives those signals. Use case: Either make PID 1 to react on those signals or test how it will react if you're using `systemd-nspawn` to test some software you develop. Not sure I follow: why do this indirectly by killing nspawn? Why not send this to the container init directly? Note that if you know the nspawn PID, you can derive the external PID of the init process easily by reading /proc/$PID/task/$PID/children. It will only container one PID, and that's init. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
