[systemd-devel] [PATCH] Set default polling interval on removable devices as well
Hey all,
udev's 60-persistent-storage.rules enables in-kernel polling if
block is built as a module. But that doesn't work if it is built
into the kernel, as done by Debian and Ubuntu (for efficiency, as
pretty much everyone will need it). This causes CD drive eject buttons
to not work (see the linked bugs).
Thanks for considering,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From daa826169c1cef081da50c0127ad9311fe93de8a Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Sat, 26 Apr 2014 16:43:35 +0200
Subject: [PATCH] Set default polling interval on removable devices as well
The events_dfl_poll_msecs rule will not trigger if block is not a module, but
built in. This will avoid udisks etc. having to poll from userspace, and
provide proper ejection when the hardware eject button is pressed.
Bug-Debian: https://bugs.debian.org/713877
Bug-Ubuntu: https://launchpad.net/bugs/890592
---
rules/60-persistent-storage.rules | 1 +
1 file changed, 1 insertion(+)
diff --git a/rules/60-persistent-storage.rules b/rules/60-persistent-storage.rules
index 475b151..40b4eb9 100644
--- a/rules/60-persistent-storage.rules
+++ b/rules/60-persistent-storage.rules
@@ -10,6 +10,7 @@ ACTION==remove, GOTO=persistent_storage_end
# enable in-kernel media-presence polling
ACTION==add, SUBSYSTEM==module, KERNEL==block, ATTR{parameters/events_dfl_poll_msecs}==0, ATTR{parameters/events_dfl_poll_msecs}=2000
+ACTION==add, ATTR{removable}==1, ATTR{events_poll_msecs}==-1, ATTR{events_poll_msecs}=2000
SUBSYSTEM!=block, GOTO=persistent_storage_end
--
1.9.1
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] kvm modules don't load any more with kernel = 3.11
Hello all,
a while ago we got a report (https://launchpad.net/bugs/1207705) that
with kernel = 3.11 the kvm modules (in particular, kvm_intel) don't
get autoloaded any more. Andy (CC'ed) fixed that back then with the
attached patch to 80-drivers.rules. Yesterday on the Debian systemd
sprint this was confirmed by someone else who runs pure systemd on
Debian with the Debian kernel, so it doesn't seem to be an unique
quirk of either the Ubuntu kernel or the previous Ubuntu systemd
modifications.
I just confirmed that on linux 3.13, kmod 16, and systemd 204 (I know,
old, but the corresponding udev rules didn't change since then). Is
that something which ought to be in 80-drivers.rules, or is that a
kernel regression? Andy's description suggests the former, and he can
hopefully chime in.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From: Andy Whitcroft a...@canonical.com
Date: Sat, 26 Apr 2014 23:12:58 +0200
Subject: Always probe cpu support drivers
The kernel from v3.11 now reports (correctly) that there is a
CPU driver connected to the CPUs in the kernel. This causes udev
to ignore the device and prevents any CPU helper modules such as KVM or
AES optimisations from being loaded. These should be loaded regardless
of whether there is a CPU driver.
Reported-by: Chris J Arges chris.j.ar...@canonical.com
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1207705
Forwarded: no
---
rules/80-drivers.rules | 1 +
1 file changed, 1 insertion(+)
diff --git a/rules/80-drivers.rules b/rules/80-drivers.rules
index 01760ef..c65ea15 100644
--- a/rules/80-drivers.rules
+++ b/rules/80-drivers.rules
@@ -3,6 +3,7 @@
ACTION==remove, GOTO=drivers_end
DRIVER!=?*, ENV{MODALIAS}==?*, RUN{builtin}=kmod load $env{MODALIAS}
+SUBSYSTEM==cpu, ENV{MODALIAS}==?*, RUN{builtin}=kmod load $env{MODALIAS}
SUBSYSTEM==tifm, ENV{TIFM_CARD_TYPE}==SD, RUN{builtin}=kmod load tifm_sd
SUBSYSTEM==tifm, ENV{TIFM_CARD_TYPE}==MS, RUN{builtin}=kmod load tifm_ms
SUBSYSTEM==memstick, RUN{builtin}=kmod load ms_block mspro_block
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 02/10] rules: updates to default device permissions in 50-udev-default.rules
Hello Jon, Jon Severinsson [2014-07-16 12:09 +0200]: Taken from the previous Debian specific rules, this is the remaining difference over the upstream 50-udev-default.rules. I deliberately didn't forward that upstream, as most of these are ancient hacks which are mostly required for not breaking backwards compatibility on upgrades. Definitively not upstream material. 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 03/10] rules: load sg module from 80-drivers.rules
Kay Sievers [2014-07-16 12:53 +0200]:
On Wed, Jul 16, 2014 at 12:09 PM, Jon Severinsson j...@severinsson.net
wrote:
From: Martin Pitt martin.p...@ubuntu.com
Taken from the Debian specific rules, this is the remaining difference over
the
upstream 80-drivers.rules.
Bug-Debian: http://bugs.debian.org/657948
---
rules/80-drivers.rules | 1 +
1 file changed, 1 insertion(+)
diff --git a/rules/80-drivers.rules b/rules/80-drivers.rules
index 8551f47..f764075 100644
--- a/rules/80-drivers.rules
+++ b/rules/80-drivers.rules
@@ -9,5 +9,6 @@ SUBSYSTEM==memstick, RUN{builtin}+=kmod load ms_block
mspro_block
SUBSYSTEM==i2o, RUN{builtin}+=kmod load i2o_block
SUBSYSTEM==module, KERNEL==parport_pc, RUN{builtin}+=kmod load ppdev
KERNEL==mtd*ro, ENV{MTD_FTL}==smartmedia, RUN{builtin}+=kmod load
sm_ftl
+SUBSYSTEM==scsi, ENV{DEVTYPE}==scsi_device, TEST!=[module/sg],
RUN{builtin}+=kmod load sg
We do not want to force-load the sg driver. Why would that be needed?
Most things don't need it. This was re-added because of
https://bugs.debian.org/657948 as a user complained that without it
some tape devices don't work any more with mtx.
Again, I deliberately didn't forward that upstream as this really
should be fixed in mtx. It's just a rule which the Debian package has
had for many years.
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 05/10] rules: skip 99-systemd.rules when not running systemd as init
Jon Severinsson [2014-07-16 12:09 +0200]: ACTION==remove, GOTO=systemd_end +TEST!=/run/systemd/system, GOTO=systemd_end I'm fairly sure that this is obsolete. Can you please test without this? 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] keymap: Add microphone mute keymap for Dell Latitude
Hello Wang, Hui Wang [2014-07-30 16:09 +0800]: On the Dell Latitude, the mic mute key event is generated by wmi driver, the keycode assigned to this hotkey from kernel is KEY_MICMUTE (248), this keycode is too big for xorg to handle, in the xorg, the XF86AudioMicMute is assigned to F20. Thanks! Applied. 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] update: udev keymaps are moving to hwdb
Kay Sievers [2013-07-16 16:25 +0200]: Committed it now. All the old udev keymap files are gone and only the hwdb file: http://cgit.freedesktop.org/systemd/systemd/tree/hwdb/60-keyboard.hwdb is carrying all the data. \o/ Martin is looking into the missing USB Logitech maps at the moment. I went through the original bug reports and usb.ids to salvage the real product IDs, committed to http://cgit.freedesktop.org/systemd/systemd/commit/?id=1b6bce89 That should cover most bits, other models need new bug reports now (Logitech USB keyboards are a catastrophe..) Thanks, 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] udev battery level changed events
Hey Bastien, Bastien Nocera [2013-10-23 14:10 +0200]: Is there a way to reproduce those events on my system (using udevadm trigger I guess)? With an actual installation, you can synthesize change events with udevadm trigger, indeed. Or using some more fundamental tools, with something like echo change | sudo tee /sys/class/power_supply/BAT*/uevent upower's test suite uses umockdev which can send uevents for the mocked devices with self.testbed.uevent(device_path, change), like the test_bluetooth_mouse_reconnect() test case already does. 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] udev battery level changed events
Bastien Nocera [2013-10-23 14:10 +0200]: I want to try and reduce wake-ups in UPower (some more), and wanted to disable polling of batteries when we have events sent for battery capacity changes. Unfortunately, I don't have hardware that does this properly (my laptop only sends out events when the state transitions, not when the battery level changes). My phone seems to do that. On charging: | KERNEL[92.074140] change /devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery (power_supply) | ACTION=change | DEVPATH=/devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery | POWER_SUPPLY_CAPACITY=38 | POWER_SUPPLY_CHARGE_EMPTY=147070 | POWER_SUPPLY_CHARGE_FULL=2101000 | POWER_SUPPLY_CHARGE_NOW=2102473 | POWER_SUPPLY_CHARGE_TYPE=Fast | POWER_SUPPLY_CURRENT_NOW=-41 | POWER_SUPPLY_HEALTH=Good | POWER_SUPPLY_NAME=battery | POWER_SUPPLY_PRESENT=1 | POWER_SUPPLY_STATUS=Charging | POWER_SUPPLY_TECHNOLOGY=Li-ion | POWER_SUPPLY_TEMP=245 | POWER_SUPPLY_VOLTAGE_MAX_DESIGN=436 | POWER_SUPPLY_VOLTAGE_MIN_DESIGN=320 | POWER_SUPPLY_VOLTAGE_NOW=3872145 | SEQNUM=4086 | SUBSYSTEM=power_supply | | UDEV [92.107882] change /devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery (power_supply) | ACTION=change | DEVPATH=/devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery | POWER_SUPPLY_CAPACITY=38 | POWER_SUPPLY_CHARGE_EMPTY=147070 | POWER_SUPPLY_CHARGE_FULL=2101000 | POWER_SUPPLY_CHARGE_NOW=2102473 | POWER_SUPPLY_CHARGE_TYPE=Fast | POWER_SUPPLY_CURRENT_NOW=-41 | POWER_SUPPLY_HEALTH=Good | POWER_SUPPLY_NAME=battery | POWER_SUPPLY_PRESENT=1 | POWER_SUPPLY_STATUS=Charging | POWER_SUPPLY_TECHNOLOGY=Li-ion | POWER_SUPPLY_TEMP=245 | POWER_SUPPLY_VOLTAGE_MAX_DESIGN=436 | POWER_SUPPLY_VOLTAGE_MIN_DESIGN=320 | POWER_SUPPLY_VOLTAGE_NOW=3872145 | SEQNUM=4086 | SUBSYSTEM=power_supply | USEC_INITIALIZED=76247 On discharging: | KERNEL[204.102065] change /devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery (power_supply) | ACTION=change | DEVPATH=/devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery | POWER_SUPPLY_CAPACITY=38 | POWER_SUPPLY_CHARGE_EMPTY=83920 | POWER_SUPPLY_CHARGE_FULL=2098000 | POWER_SUPPLY_CHARGE_NOW=2105634 | POWER_SUPPLY_CHARGE_TYPE=N/A | POWER_SUPPLY_CURRENT_NOW=226600 | POWER_SUPPLY_HEALTH=Good | POWER_SUPPLY_NAME=battery | POWER_SUPPLY_PRESENT=1 | POWER_SUPPLY_STATUS=Discharging | POWER_SUPPLY_TECHNOLOGY=Li-ion | POWER_SUPPLY_TEMP=228 | POWER_SUPPLY_VOLTAGE_MAX_DESIGN=436 | POWER_SUPPLY_VOLTAGE_MIN_DESIGN=320 | POWER_SUPPLY_VOLTAGE_NOW=3750297 | SEQNUM=4129 | SUBSYSTEM=power_supply | | UDEV [204.150806] change /devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery (power_supply) | ACTION=change | DEVPATH=/devices/platform/msm_ssbi.0/pm8921-core/pm8921-charger/power_supply/battery | POWER_SUPPLY_CAPACITY=38 | POWER_SUPPLY_CHARGE_EMPTY=83920 | POWER_SUPPLY_CHARGE_FULL=2098000 | POWER_SUPPLY_CHARGE_NOW=2105634 | POWER_SUPPLY_CHARGE_TYPE=N/A | POWER_SUPPLY_CURRENT_NOW=226600 | POWER_SUPPLY_HEALTH=Good | POWER_SUPPLY_NAME=battery | POWER_SUPPLY_PRESENT=1 | POWER_SUPPLY_STATUS=Discharging | POWER_SUPPLY_TECHNOLOGY=Li-ion | POWER_SUPPLY_TEMP=228 | POWER_SUPPLY_VOLTAGE_MAX_DESIGN=436 | POWER_SUPPLY_VOLTAGE_MIN_DESIGN=320 | POWER_SUPPLY_VOLTAGE_NOW=3750297 | SEQNUM=4129 | SUBSYSTEM=power_supply | USEC_INITIALIZED=4104080 I get about one uevent in 10 seconds during discharging, and about one per second during charging. So using uevents for the latter won't help much for reducing wakeups, but I take it it doesn't matter that much while an AC is attached. 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] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
Martin Pitt [2013-11-14 17:53 +0100]: So option 1 is to update the patch to not rely on uid, but instead always get it from PAM. I went through all instances of using the uid, username, or pw, and I cannot find any place in the PAM module where we would actually want the originating user name, so I retract this. Option 2 is to never read it from loginuid, as that's indeed not what one should be concerned about in a PAM module. Attached patch is doing option 2. ... and hence I'm convinced that this is the right thing to do. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2] Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su
Martin Pitt [2013-11-14 7:45 +0100]:
+} else {
+pam_syslog(handle, LOG_DEBUG, Runtime dir %s is not owned
by the target uid %u, ignoring.,
+ runtime_path, uid);
Sorry, LOG_DEBUG appears by default, this needs to be guarded with
checking debug. Fixed in updated patch.
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 9a33dbf7cc906539df8b6e152374d15dbf8c7a99 Mon Sep 17 00:00:00 2001
From: Martin Pitt martinp...@gnome.org
Date: Wed, 13 Nov 2013 13:02:28 +0100
Subject: [PATCH 1/2] pam: Check $XDG_RUNTIME_DIR owner
http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html requires
that $XDG_RUNTIME_DIR MUST be owned by the user, and he MUST be the only one
having read and write access to it..
Don't set an existing $XDG_RUNTIME_DIR in the PAM module if it isn't owned by
the session user. Otherwise su sessions get a runtime dir from a different user
which leads to either permission errors or scribbling over the other user's
files.
https://bugzilla.redhat.com/show_bug.cgi?id=753882
https://launchpad.net/bugs/1197395
---
src/login/pam-module.c | 23 +++
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/login/pam-module.c b/src/login/pam-module.c
index 1975d80..4fd2212 100644
--- a/src/login/pam-module.c
+++ b/src/login/pam-module.c
@@ -194,6 +194,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
uint32_t uid, pid, vtnr = 0;
bool debug = false, remote;
struct passwd *pw;
+struct stat st;
assert(handle);
@@ -385,10 +386,24 @@ _public_ PAM_EXTERN int pam_sm_open_session(
return r;
}
-r = pam_misc_setenv(handle, XDG_RUNTIME_DIR, runtime_path, 0);
-if (r != PAM_SUCCESS) {
-pam_syslog(handle, LOG_ERR, Failed to set runtime dir.);
-return r;
+/* only set $XDG_RUNTIME_DIR if it is owned by the target user, as per
+ * XDG basedir-spec; this avoids su sessions to scribble over a runtime
+ * dir of a different user */
+r = lstat(runtime_path, st);
+if (r != 0) {
+pam_syslog(handle, LOG_ERR, Failed to stat runtime dir: %s, strerror(errno));
+return PAM_SYSTEM_ERR;
+}
+
+if (st.st_uid == uid) {
+r = pam_misc_setenv(handle, XDG_RUNTIME_DIR, runtime_path, 0);
+if (r != PAM_SUCCESS) {
+pam_syslog(handle, LOG_ERR, Failed to set runtime dir.);
+return r;
+}
+} else if (debug) {
+pam_syslog(handle, LOG_DEBUG, Runtime dir %s is not owned by the target uid %u, ignoring.,
+ runtime_path, uid);
}
if (!isempty(seat)) {
--
1.8.4.3
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
Hello Colin, Colin Walters [2013-11-18 19:35 -0500]: This is on my radar; the patch wasn't working for me but I haven't had time to add debug prints and figure out whether it's my (gnome-continuous) side or something else. Give me a day or two. Did you just try the original patch that I sent to RHBZ? As I found out with the other Colin, this actually needs two patches. One is the check runtime dir ownership one which I sent to RHBZ (in slightly different form), the final one is http://lists.freedesktop.org/archives/systemd-devel/2013-November/014392.html and second, dropping the usage of loginuid, which is http://lists.freedesktop.org/archives/systemd-devel/2013-November/014390.html Colin Guthrie confirmed that this fixed things for him. Can you check with these two, or confirm that even with these two you still get the wrong runtime dir in pkexec? Thanks, 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 to remap some hotkeys in Toshiba Satellite U940
Hello Jose, Jose Ignacio Naranjo [2013-11-18 19:30 +0100]: I don't know if this is the right place to send this small patch. It is. It is just just some remapping in hwdb for a few hotkeys. If you need any rework in the patch or whatever, please tell me. Looks good, thank you! Applied with some changelog standardization: http://cgit.freedesktop.org/systemd/systemd/commit/?id=bc9cdba udevadm test-builtin says this: keyboard: mapping scan code 343 (0x157) to key code 113 (0x71) Error calling EVIOCSKEYCODE: Invalid argument keyboard: mapping scan code 344 (0x158) to key code 238 (0xee) Error calling EVIOCSKEYCODE: Invalid argument Hm, I don't see these scan codes in the keymaps. Is that from some local hwdb file of your's? Thanks! 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] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
Hello, Colin Walters [2013-11-19 10:42 -0500]: Both of our patch series currently are basically going to have the effect that with pkexec, XDG_RUNTIME_DIR is unset. But this is undesirable because it forces the rest of userspace to go back to the old dark ages when XDG_RUNTIME_DIR didn't exist and there was no reliable mechanism for two processes of equal uid but different sessions find each other and communicate. (Think ssh login + gdm login). Full ack. My patch though starts to pave the way for having XDG_RUNTIME_DIR consistently point to that of the user's uid - I am increasingly thinking the option #3 I mention in the patch where we refcount the runtime dir in addition to users and sessions would work. For the record, I much prefer something like this to my original patch which simply unsets it. I just shied away from that as Lennart repeatedly said on the RHBZ bug that he doesn't want su behave that way. I disagree, but his word counts more than mine in this situation, so I at least want to stop sessions using the wrong runtime dir. If logind would actually give you the session data for the uid you call it for, instead of only looking at the seat/logind session data, that would indeed be more useful/correct in my opinion. Doing ~user$ su - otheruser or ssh otheruser@localhost should effectively behave the same, but right now logind gives you the session info for ~user in the first, and for ~otheruser in the second case. Thanks! Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
Hey Lennart, Lennart Poettering [2013-11-21 0:16 +0100]: On Thu, 14.11.13 07:45, Martin Pitt (martin.p...@ubuntu.com) wrote: Hello all, pam_systemd currently causes some havoc when you run programs or shells with su: it passes on the $XDG_RUNTIME_DIR from the original user session, so that programs like pulseaudio or dconf end up scribbling into the original user's runtime dir. This has been discussed at length at [1][2] and is leading people to consider workarounds like [3]. It seems Lennart is against giving the new user a new logind session and runtime dir; I think it would be right to give it a fresh (or an already existing one for the target user) runtime dir, but in either case passing it the original user's runtime dir is actively wrong and harmful. Well, that's quite arbitrary. What about dbus, X11, and so on, do you plan to turn that off for the new session too? At the moment, there is no dbus to turn off -- su - otheruser does not take the environment from the original session (if it does, it's misconfigured); and even if it does, it wouldn't work as otheruser cannot access the original user's session or user bus anyway. As to what happens to $DISPLAY, I have no strong opinion about it. But that's not the concern of libpam-systemd anyway. su is a hack, it is not clear what credentials it changes and which ones it doesn't. I think people keep confusing su and su - here. The manpage says -, -l, --login Provide an environment similar to what the user would expect had the user logged in directly. which shows the intention pretty well: It should be by and large similar to what I get when I log into a VT. It doesn't do that now as it doesn't get a seat and a runtime dir, but concerning credentials it should fully switch to otheruser's uid, groups, etc. It runs a full PAM stack after all, which should prepare the environment accordingly. This is similar to pkexec. su user is an entirely different matter, as you say. It should do little more than calling setgid()/setuid(), so there is no hope that this could ever do anything sensible for a program which depends on the environment. I don't remember ever having used it. Please let's keep it out of the discussion, since it isn't related to pam-systemd and as you say its semantics are pretty much broken anyway. Quit frankly, I am pretty sure the best approach is to simply prohibit running graphical applications from su sessions, it's never going to work. Fine for me. But trying to run them should properly fail then, not clobber the other user's home directory and then cause failures for the original user. Letting other user access some (but not all) of a private user's bits and pieces is never going to work if those bits and pieces are nowadays a mix of dconf, X11, PA, dbus, security creds, keyrings, yadda yada... Exactly my point! Hence, we must *not* pass another user's runtime dir in logind/the PAM module. So, what's the intention here? That XDG_RUNTIME_DIR is entirely unset after su? That sounds kinda acceptable to me. Yes, for su - and pkexec. It might not work for su as that is likely configured to not run PAM, see above. That's what my patch is doing and what would prevent the damaging of runtime dirs. It's not what I consider ideal (I prefer Colin's approach of giving him the *correct* user's runtime dir), but if we can't have that, let's at least not pass the wrong one. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
Hey Lennart, Lennart Poettering [2013-11-26 5:12 +0100]: I implemented this now, using a different approach than Martin's original patch (i.e. I don't think it is a good idea to involve stat() here, instead let's just let logind pass all information to pam_systemd). Thanks! Lennart Poettering [2013-11-26 5:17 +0100]: That can't work. As the directory only exists when a real login session is around. su/sudo don't get their own login sessins, hence the dir doesn't necessarily exist and from the perspective of the code running in su/sudo the lifetime semantics of the dir wouldn't match any expections... Right, as long as they don't actually get one. I (and I think Colin) argued that su -/pkexec should (just like ssh localhost), as they run a full PAM stack which is like logging in. But let's agree to disagree at this point. I'm happy that the not your own runtime dir issue is fixed now at least. Thanks, 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] Question about a udev rule
Robert Milasan [2013-12-08 19:14 +0100]:
IMPORT{program}=/usr/bin/udevadm info --export --export-prefix=ROOT_
--device-id-of-file=/ ENV{MAJOR}!=0, ENV{MAJOR}==$env{ROOT_MAJOR},
^
For starters you seem to be missing a comma there.
--
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] The whole su/pkexec session debate
Lennart Poettering [2013-12-11 3:11 +0100]: I used to believe that screen should set up a new session, but I don't think so anymore. Nowadays think they should do exactly what they currently do: fork and stay around. This will cause the session to stay in closing state when the user logs out, but that's exactly what it would be good for: i.e. sessions which have officially finished but of which some parts remain. FWIW, I fully agree. I don't see anything what would make screen special. It does not even get close to the PAM stack, being concerned about users, etc. 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] The whole su/pkexec session debate
Lennart Poettering [2013-12-11 3:17 +0100]: I am pretty sure that screen should not get the right to escape here. [..] The kill-on-logout thing really is something that explicitly should kill screen too, otherwise it would not be so useful. Not only not so useful, it would be rendered entirely useless as you can run any program in a screen shell. If you poke holes into that, the next person will come around and say but keep nohup, too, and so on. For desktops you usually want kill-on-logout configured off (at least if you expect your users to run things like screen, nohup, or plus disown), and if you turn it on in environments like coffee shops, school labs, or other public computers it should really kill everything, period. 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] udev rules environment variable
Robert Milasan [2013-12-17 12:44 +0100]:
I have this rule as a test, but doesn't do squat (meaning it doesnt
work) :)
ACTION==add, SUBSYSTEM==net, KERNEL==?*, ENV{test_device}=1
ACTION==remove, SUBSYSTEM==net, KERNEL==?*, ENV{test_device}==1,
RUN+=/bin/sh -c 'echo test_device /tmp/test_device.log'
Drop the KERNEL== bits. Network devices don't have a /dev/... device
node in Linux, so KERNEL will never be set for those.
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
[systemd-devel] [PATCH] rules: Fix persistent input symlinks for interface 00
Hello,
https://launchpad.net/bugs/1057824 reports a race condition in
creating the persistent symlinks for input devices with more than one
interface. Patch attached with a detailled explanation; this fixes a
thinko of mine when I wrote the original rule.
Can I commit this?
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From fb5d873cfd4606383964c707da0ec21ebaa1c04b Mon Sep 17 00:00:00 2001
From: Martin Pitt martinp...@gnome.org
Date: Thu, 11 Oct 2012 08:21:17 +0200
Subject: [PATCH] rules: Fix persistent input symlinks for interface 00
Commits 5e9eb156c and 32567f8 introduced persistent symlinks for input devices
with more than one interface. However, this does not ensure stability for the
default interface, i. e. with interface number 00 or a nonexisting one. If a
device with a higher interface number appears first, it'll claim the symlink
name without an interface number, and the interface 00 device won't get any.
Fix this by creating the default symlink only for interface 00 or a nonexisting
one, so that we properly partition the two cases over the two rules.
https://launchpad.net/bugs/1057824
---
rules/60-persistent-input.rules |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rules/60-persistent-input.rules b/rules/60-persistent-input.rules
index fb798dd..0e33e68 100644
--- a/rules/60-persistent-input.rules
+++ b/rules/60-persistent-input.rules
@@ -19,9 +19,9 @@ ATTRS{name}==*dvb*|*DVB*|* IR *, ENV{.INPUT_CLASS}=ir
ENV{.INPUT_CLASS}==?*, ENV{ID_SERIAL}==, ENV{ID_SERIAL}=noserial
# by-id links
-KERNEL==mouse*|js*, ENV{ID_BUS}==?*, ENV{.INPUT_CLASS}==?*, SYMLINK+=input/by-id/$env{ID_BUS}-$env{ID_SERIAL}-$env{.INPUT_CLASS}
+KERNEL==mouse*|js*, ENV{ID_BUS}==?*, ENV{.INPUT_CLASS}==?*, ATTRS{bInterfaceNumber}==|00, SYMLINK+=input/by-id/$env{ID_BUS}-$env{ID_SERIAL}-$env{.INPUT_CLASS}
KERNEL==mouse*|js*, ENV{ID_BUS}==?*, ENV{.INPUT_CLASS}==?*, ATTRS{bInterfaceNumber}==?*, ATTRS{bInterfaceNumber}!=00, SYMLINK+=input/by-id/$env{ID_BUS}-$env{ID_SERIAL}-if$attr{bInterfaceNumber}-$env{.INPUT_CLASS}
-KERNEL==event*, ENV{ID_BUS}==?*, ENV{.INPUT_CLASS}==?*, SYMLINK+=input/by-id/$env{ID_BUS}-$env{ID_SERIAL}-event-$env{.INPUT_CLASS}
+KERNEL==event*, ENV{ID_BUS}==?*, ENV{.INPUT_CLASS}==?*, ATTRS{bInterfaceNumber}==|00, SYMLINK+=input/by-id/$env{ID_BUS}-$env{ID_SERIAL}-event-$env{.INPUT_CLASS}
KERNEL==event*, ENV{ID_BUS}==?*, ENV{.INPUT_CLASS}==?*, ATTRS{bInterfaceNumber}==?*, ATTRS{bInterfaceNumber}!=00, SYMLINK+=input/by-id/$env{ID_BUS}-$env{ID_SERIAL}-if$attr{bInterfaceNumber}-event-$env{.INPUT_CLASS}
# allow empty class for USB devices, by appending the interface number
SUBSYSTEMS==usb, ENV{ID_BUS}==?*, KERNEL==event*, ENV{.INPUT_CLASS}==, ATTRS{bInterfaceNumber}==?*, \
--
1.7.10.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] src/login/uaccess.c: Dead code?
Hello,
I just noticed that src/login/uaccess.c seems to be dead code, not
being referenced anywhere in the build system. It looks like it was
forgotten to clean up when moving that functionality to
src/udev/udev-builtin-uaccess.c?
OK to push?
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 072c60c8313ee18bb8cb8ef58063c62b63bc4e09 Mon Sep 17 00:00:00 2001
From: Martin Pitt martinp...@gnome.org
Date: Tue, 26 Mar 2013 11:35:17 +0100
Subject: [PATCH 1/2] Drop src/login/uaccess.c, dead code
This moved to src/udev/udev-builtin-uaccess.c a while ago.
---
src/login/uaccess.c | 91 -
1 file changed, 91 deletions(-)
delete mode 100644 src/login/uaccess.c
diff --git a/src/login/uaccess.c b/src/login/uaccess.c
deleted file mode 100644
index 2c530c8..000
--- a/src/login/uaccess.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
-
-/***
- This file is part of systemd.
-
- Copyright 2011 Lennart Poettering
-
- systemd is free software; you can redistribute it and/or modify it
- under the terms of the GNU Lesser General Public License as published by
- the Free Software Foundation; either version 2.1 of the License, or
- (at your option) any later version.
-
- systemd is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public License
- along with systemd; If not, see http://www.gnu.org/licenses/.
-***/
-
-#include errno.h
-#include string.h
-
-#include systemd/sd-daemon.h
-#include systemd/sd-login.h
-
-#include logind-acl.h
-#include util.h
-#include log.h
-
-int main(int argc, char *argv[]) {
-int r;
-const char *path = NULL, *seat;
-bool changed_acl = false;
-uid_t uid;
-
-log_set_target(LOG_TARGET_AUTO);
-log_parse_environment();
-log_open();
-
-umask(0022);
-
-if (argc 2 || argc 3) {
-log_error(This program expects one or two arguments.);
-r = -EINVAL;
-goto finish;
-}
-
-/* Make sure we don't muck around with ACLs the system is not
- * running systemd. */
-if (!sd_booted())
-return 0;
-
-path = argv[1];
-seat = argc 3 || isempty(argv[2]) ? seat0 : argv[2];
-
-r = sd_seat_get_active(seat, NULL, uid);
-if (r == -ENOENT) {
-/* No active session on this seat */
-r = 0;
-goto finish;
-} else if (r 0) {
-log_error(Failed to determine active user on seat %s., seat);
-goto finish;
-}
-
-r = devnode_acl(path, true, false, 0, true, uid);
-if (r 0) {
-log_error(Failed to apply ACL on %s: %s, path, strerror(-r));
-goto finish;
-}
-
-changed_acl = true;
-r = 0;
-
-finish:
-if (path !changed_acl) {
-int k;
-/* Better be safe that sorry and reset ACL */
-
-k = devnode_acl(path, true, false, 0, false, 0);
-if (k 0) {
-log_error(Failed to apply ACL on %s: %s, path, strerror(-k));
-if (r = 0)
-r = k;
-}
-}
-
-return r 0 ? EXIT_FAILURE : EXIT_SUCCESS;
-}
--
1.8.1.2
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] udev hwdb: Store binary database in libdir, not in /etc
Hello all,
right now, udev stores its binary hwdb cache in /etc/udev/, which is
ugly IMHO. This is neither user-editable nor configuration of any
kind. It's just a cache file, and does not need to appear in backup,
VCSes of /etc and the like.
Can we move it to the libdir instead? Attached patch does that.
Thanks for considering!
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 0d5c4f5d492d13c99ac1a8619ccb0dc3fe438d16 Mon Sep 17 00:00:00 2001
From: Martin Pitt martinp...@gnome.org
Date: Fri, 14 Jun 2013 12:21:38 +0200
Subject: [PATCH] udev hwdb: Store binary database in libdir, not in /etc
Storing huge binary files in /etc/ is ugly, as this is neither user-editable
nor configuration of any kind. This is just a cache file, and does not need
backing up.
Move it to udevlibexecdir instead.
---
Makefile.am| 2 +-
man/udevadm.xml| 6 +++---
src/libudev/libudev-hwdb.c | 10 +-
src/udev/udevadm-hwdb.c| 2 +-
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index a74c19d..a4db4a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2179,7 +2179,7 @@ INSTALL_DATA_HOOKS += \
hwdb-update-hook
hwdb-remove-hook:
- -test -n $(DESTDIR) || rm -f /etc/udev/hwdb.bin
+ -test -n $(DESTDIR) || rm -f $(udevlibexecdir)/hwdb.bin
# --
TESTS += \
diff --git a/man/udevadm.xml b/man/udevadm.xml
index d0b257d..75a73c2 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -424,13 +424,13 @@
/refsect2
refsect2titleudevadm hwdb optionaloptions/optional/title
- paraMaintain the hardware database index in filename/etc/udev/hwdb.bin/filename./para
+ paraMaintain the hardware database index in filename/lib/udev/hwdb.bin/filename./para
variablelist
varlistentry
termoption--update/option/term
listitem
-paraCompile the hardware database information located in /usr/lib/udev/hwdb.d/,
-/etc/udev/hwdb.d/ and store it in filename/etc/udev/hwdb.bin/filename. This should be done after
+paraCompile the hardware database information located in /lib/udev/hwdb.d/,
+/etc/udev/hwdb.d/ and store it in filename/lib/udev/hwdb.bin/filename. This should be done after
any update to the source files; it will not be called automatically. The running
udev daemon will detect a new database on its own and does not need to be
notified about it./para
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index a56ad75..b8de7ed 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -271,30 +271,30 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
hwdb-refcount = 1;
udev_list_init(udev, hwdb-properties_list, true);
-hwdb-f = fopen(/etc/udev/hwdb.bin, re);
+hwdb-f = fopen(UDEVLIBEXECDIR /hwdb.bin, re);
if (!hwdb-f) {
-log_debug(error reading /etc/udev/hwdb.bin: %m);
+log_debug(error reading UDEVLIBEXECDIR /hwdb.bin: %m);
udev_hwdb_unref(hwdb);
return NULL;
}
if (fstat(fileno(hwdb-f), hwdb-st) 0 ||
(size_t)hwdb-st.st_size offsetof(struct trie_header_f, strings_len) + 8) {
-log_debug(error reading /etc/udev/hwdb.bin: %m);
+log_debug(error reading UDEVLIBEXECDIR /hwdb.bin: %m);
udev_hwdb_unref(hwdb);
return NULL;
}
hwdb-map = mmap(0, hwdb-st.st_size, PROT_READ, MAP_SHARED, fileno(hwdb-f), 0);
if (hwdb-map == MAP_FAILED) {
-log_debug(error mapping /etc/udev/hwdb.bin: %m);
+log_debug(error mapping UDEVLIBEXECDIR /hwdb.bin: %m);
udev_hwdb_unref(hwdb);
return NULL;
}
if (memcmp(hwdb-map, sig, sizeof(hwdb-head-signature)) != 0 ||
(size_t)hwdb-st.st_size != le64toh(hwdb-head-file_size)) {
-log_debug(error recognizing the format of /etc/udev/hwdb.bin);
+log_debug(error recognizing the format of UDEVLIBEXECDIR /hwdb.bin);
udev_hwdb_unref(hwdb);
return NULL;
}
diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c
index 3e849aa..65f08f0 100644
--- a/src/udev/udevadm-hwdb.c
+++ b/src/udev/udevadm-hwdb.c
@@ -569,7 +569,7 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) {
log_debug(strings dedup'ed: %8zu bytes (%8zu)\n,
trie-strings-dedup_len, trie-strings-dedup_count);
-if (asprintf(hwdb_bin, %s/etc/udev/hwdb.bin, root) 0) {
+if (asprintf(hwdb_bin, %s/ UDEVLIBEXECDIR /hwdb.bin, root) 0) {
rc
Re: [systemd-devel] [PATCH] udev hwdb: Store binary database in libdir, not in /etc
Hello Tom,
Tom Gundersen [2013-06-14 13:08 +0200]:
That said, I don't think libdir is appropriate as this data is not
under the control of the package manager (as it is generated at
install-time rather than at build-time, it would for instance not be
suitable for sharing between hosts). I guess localstatedir would be
another alternative, but the problem there is that it is not
(necessarily) available during early boot when this db is needed.
I tought about using /var/cache/ first as well, but /var might be on a
different partition. Hence I was using /lib/udev (i. e. udevlibdir) as
this guaranteed to be on the same partition as all the other udev
helpers.
Why is it a problem if a file in /lib is not under control of the
package manager? There are other files in /lib which are not, e. g.
/lib/modules/kernel/modules.dep{,.bin}. Also, if you share
/lib/udev/hwdb.d/, you can also share /lb/udev/hwdb.bin? How would
sharing be any different whether the cache file is in /etc or /lib?
Thanks,
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] udev hwdb: Store binary database in libdir, not in /etc
Hello Tom, replying on-list again, if you don't mind. Tom Gundersen [2013-06-16 22:52 +0200]: On Sun, Jun 16, 2013 at 10:32 PM, Martin Pitt martin.p...@ubuntu.com wrote: We actually generate the hwdb during package build and ship hwdb.bin statically. The problem with that is that local configuration in /etc/udev/hwdb.d/ will be ignored, so it seems to me that the correct thing to do is to generate hwdb.bin in post-install. This bit striked me as overengineered, so for now our udev package doesn't even ship /etc/udev/. pci.ids or usb.ids aren't user-configurable, and there has never been a case that we heard of where the admin would have the need to change the name of a piece of hardware. Michael proposed a compromise where the hwdb.bin would be in /lib/udev, and as soon as there are files in /etc/udev/hwdb.d/ it would move the cache to /etc/udev/. I can't say I like that very much as it would introduce the same problem with just less predictability, but I want to mention it for the records. Anyway, Lennart's and your responses have demonstrated that putting the cache into /etc wasn't by accident but deliberate; that's fine, so let's keep it as it is, and we keep the patch downstream for now. As it is it is trivial to revert or change as it only changes statically shipped files in the package. Thanks for the discussion! 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] udev hwdb: Store binary database in libdir, not in /etc
Lennart Poettering [2013-06-17 2:52 +0200]: The file is supposed to be be built on the installed system so that other packages or the admin can drop in additional hwdb files. And yes, it is not a package manager controlled file, which is precisely why I am saying it belongs to /etc, not /usr. (See my other response to Tom about details) No, by placing it in /usr (or /lib, for old distributions which haven't done the /usr merge yet) you break the rule that the files the systemd package installs in /usr should be the same on all installations of the same package version. It doesn't at the moment, as the file is in the package it is the same on all installations (of the same architecture). 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] udev hwdb: Store binary database in libdir, not in /etc
Lennart Poettering [2013-06-17 18:27 +0200]: We try to push some of the more exotic device metadata into the respective packages and the developers of those packages should be able to rely on being able to drop in additional hwdb and have things work everywhere. If Ubuntu then goes and doesn't allow drop-ins and moves generation of these files to compile-time then you effectively make that impossible. And that's really uncool of you. It wasn't really about not allowing things, but rather about making it easy to change or revert the current patch while this is being discussed. I wanted to fix the hwdb integration without adding a config file which then needs special treatment again for removal, as in dpkg config files are basically holy. If/once there are actually other packages wanting to add stuff there, we'll re-add the dynamic generation of course. But for now there is no reason to do so, it just wastes cycles, dramatically complicates the handling of the cache files, and adds an extra 5 MB to the install footprint. 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] udev hwdb: Store binary database in libdir, not in /etc
Lennart Poettering [2013-06-17 18:28 +0200]: On Mon, 17.06.13 06:41, Martin Pitt (martin.p...@ubuntu.com) wrote: No, by placing it in /usr (or /lib, for old distributions which haven't done the /usr merge yet) you break the rule that the files the systemd package installs in /usr should be the same on all installations of the same package version. It doesn't at the moment, as the file is in the package it is the same on all installations (of the same architecture). I cannot parse this. Let me try to explain this a different way: in RPM-speak the files in /etc should ideally either be %ghost'ed or %config'ed, and the ones in /usr should be neither. That's the situation with a dynamically created file as you have in current Fedora. But the current Ubuntu package just ships a static /lib/udev/hwdb.bin, thus it is perfectly shareable between installations and thus Tom's concern about not being a package-owned file doesn't apply. That's of course unrelated to the question whether we should move to dynamic creation of that file, at which point it wouldn't be suited very well in /lib (but still much better than in /etc). 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] Please review the patch about support the samsung series 2 Fn+keys.
Hello Dongjun Jang,
장동준 [2013-06-28 7:09 +]:
Please review the attached patch for the samsung series 3 Fn+F* keys(keymap
and forced release events).
This patch is for 300E5EV/300E4EV/270E5EV/270E4EV which is samsung series3
models.
This looks good by and large, but I wonder about the product matching:
+ENV{DMI_VENDOR}==[sS][aA][mM][sS][uU][nN][gG]*,
ATTR{[dmi/id]product_name}==300E5EV/300E4EV/270E5EV/270E4EV,
RUN+=keyboard-force-release.sh $devpath samsung-series-3
This is called series-3, but only applies to very specific models.
Will other, similar, models use the same keymap? I. e. would it be
appropriate to generalize this to something like
+ENV{DMI_VENDOR}==[sS][aA][mM][sS][uU][nN][gG]*,
ATTR{[dmi/id]product_name}==300*|270E*, RUN+=keyboard-force-release.sh
$devpath samsung-series-3
(Please note that alternatives are specified with |, not with / as in
your patch). This rule would apply to any model that starts with 300
or 270E. Perhaps it can be generalized even further.
Thanks,
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] Please review the patch about support the samsung series 2 Fn+keys.
Hello again,
장동준 [2013-06-28 8:15 +]:
Please review the attached patch for the samsung series 3 Fn+F* keys(keymap
and forced release events).
This patch is for samsung series3 models.
Ah, I sent my reply to your first patch at the same time when you sent this
second patch.
+ENV{DMI_VENDOR}==[sS][aA][mM][sS][uU][nN][gG]*,
ATTR{[dmi/id]product_name}==*300E5*|*300E4*|*300E7*|*270E5*|*270E4*,
RUN+=keyboard-force-release.sh $devpath samsung-series-3
So you already fixed the | and generalized these a bit. I still
wonder if it would be appropriate to use 300*|270*?
Thanks,
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] Please review the patch about support the samsung series 2 Fn+keys.
Hello Dongjun, 장동준 [2013-06-28 8:44 +]: As your opinion, I discussed about this issue with our firmware engineer. He said he can not guarantee about this. Because Product ID 300*, 270* will be expanded, And firmware have possibility to change. I think it have some risk... Honestly, it's out of my control. I'm sorry about that. No worries, thanks for checking. So let's take the specific matches for now, we can always enlarge them later on. I adjusted your patch to current trunk and pushed: http://cgit.freedesktop.org/systemd/systemd/commit/?id=cda4380d9ffb Thank you! Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] I wonder… why systemd provokes this amount of polarity and resistance
Hello Rob, this is higly Debian specific (doesn't even apply to Ubuntu) and thus a bit off-topic, but as the question already is on the upstream ML.. sorry! Rob Owens [2014-10-06 14:56 -0400]: brasero - gvfs - gvfs-daemons - udisks2 - libpam-systemd - systemd-sysv You can break it up after libpam-systemd, as this has dependency alternatives to systemd-shim. With that you can use sysvinit or upstart. But currently systemd-sysv is the preferred alternative, so if you don't specify anything else it will pick systemd-sysv. You can do e. g. apt-get install brasero systemd-shim to select another (or install -shim first). I gather that this has something to do with logind and/or cgroups. That's correct; in fact most desktop-y software talks to logind only, but logind is a crucial component on a modern desktop. Systemd-shim is a duplication of effort. It's a looong story/history, but systemd-shim itself is actually fairly small. It's mostly glue to provide systemd's D-BUS API and implement it in terms of other components like cgmanager and pm-utils. And its development was quite inevitable at least from Debian's/Ubuntu's perspective as it is just practically impossible to do a SysV/upstart → systemd migration on a flag day. Not only that, but it must time its releases with the releases of systemd-sysv. Mostly not. That needs to happen for D-BUS API changes like they happened around version 209, but that happens fairly seldomly. 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] Fwd: [PATCH v3] tests: added tests for unit_file_get_{state, list}
Hello Ken, David,
sorry for breaking the thread.
Thanks for adding these tests! This looks good to me, I just have some
nitpicks. NB that I'm not really familiar with the systemd code, so my
comments are certainly not complete.
David Timothy Strauss [2014-10-17 13:42 +0200]:
diff --git a/Makefile.am b/Makefile.am
index e52db17..7d4f2f5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1358,7 +1358,8 @@ tests += \
test-ratelimit \
test-condition-util \
test-uid-range \
- test-bus-policy
+ test-bus-policy \
+ test-enabled
Nitpick: Can we end lists with a $(NULL) line so that patches don't
always have to change the previous last line?
EXTRA_DIST += \
test/a.service \
@@ -1394,8 +1395,36 @@ EXTRA_DIST += \
test/bus-policy/hello.conf \
test/bus-policy/methods.conf \
test/bus-policy/ownerships.conf \
- test/bus-policy/signals.conf
-
+ test/bus-policy/signals.conf \
+ test/test-enabled-root/usr/lib/systemd/system/masked.service \
[...] Personally I find it a lot easier to have a test with this
style:
unit = create_test_unit(contents);
assert(something_about(unit));
as you keep the data what you test and what you expect together in one
spot. Also it keeps clutter out of the tree and it's easier to
factorize/mass-change test units.
But this is again just nitpicking, and actually also a question to the
systemd developers which style they prefer.
+static void test_enabled(int argc, char* argv[]) {
[...]
+h = hashmap_new(string_hash_ops);
+r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h);
+assert_se(r = 0);
I'm a bit confused here -- is this supposed to have a side effect and
the condition was rewritten later?
+HASHMAP_FOREACH(p, h, i) {
+UnitFileState s;
+
+s = unit_file_get_state(UNIT_FILE_SYSTEM, root_dir,
+basename(p-path));
+
+/* unit_file_get_list and unit_file_get_state are
+ * a little different in some cases. Handle these
+ * cases here ...
+ */
+switch ((int)s) {
+case UNIT_FILE_ENABLED_RUNTIME:
Meh type safety (enum → int → back to enum), but this looks
safe/correct anyway. Handling both errno and enum value in the same
switch isn't otherwise possible, it'd need to be moved into an if/else
ladder.
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
[systemd-devel] [PATCH] udev hwdb: Support shipping pre-compiled database in system images
Hello all,
as discussed on the plumbers conference, I would like to optionally
move the compiled hwdb.bin out of /etc/. For r/o system images it is
much nicer if they can just ship a pre-generated hwdb in /usr without
having to build it at runtime/factory reset time, and avoid having to
carry it in /etc.
The behaviour of this should be fully backwards compatible.
Thanks for considering,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 38b8a915fd71204ed9e8b0568ad16516dfa9208b Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 17 Oct 2014 15:01:01 +0200
Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system
images
In some cases it is preferable to ship system images with a pre-generated
binary hwdb database, to avoid having to build it at runtime, avoid shipping
the source hwdb files, or avoid storing large binary files in /etc.
So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in
UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/
and re-generating the database which trumps the one in /usr/lib.
Also add a new --vendor flag to udevadm hwdb --update which puts the database
into UDEVLIBEXECDIR.
---
man/udev.xml | 4 +++-
man/udevadm.xml| 9 +
src/libudev/libudev-hwdb.c | 20 ++--
src/udev/udevadm-hwdb.c| 10 --
4 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/man/udev.xml b/man/udev.xml
index d77cbb0..71c1220 100644
--- a/man/udev.xml
+++ b/man/udev.xml
@@ -766,7 +766,9 @@
paraThe content of all hwdb files is read by
citerefentryrefentrytitleudevadm/refentrytitlemanvolnum8/manvolnum/citerefentry
- and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename.
+ and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename,
+ or alternatively filename/lib/udev/hwdb.bin/filename if you want ship the compiled
+ database in an immutable image.
During runtime only the binary database is used./para
/refsect1
diff --git a/man/udevadm.xml b/man/udevadm.xml
index 749144d..571ef7d 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
/listitem
/varlistentry
varlistentry
+ termoption--vendor/option/term
+ listitem
+paraPut the compiled database into filename/usr/lib/udev/hwdb.bin/filename instead.
+Use this if you want to ship a pre-compiled database in immutable system images, or
+don't use filename/etc/udev/hwdb.d/filename and want to avoid large binary files in
+filename/etc/filename./para
+ /listitem
+/varlistentry
+varlistentry
termoption-t/option/term
termoption--test=replaceablestring/replaceable/option/term
listitem
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index 8fb7240..91c05e8 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,13 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
return 0;
}
+static const char *get_hwdb_bin_path(void) {
+if (access(/etc/udev/hwdb.bin, F_OK) = 0)
+return /etc/udev/hwdb.bin;
+else
+return UDEVLIBEXECDIR /hwdb.bin;
+}
+
/**
* udev_hwdb_new:
* @udev: udev library context
@@ -267,6 +274,7 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
_public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
struct udev_hwdb *hwdb;
const char sig[] = HWDB_SIG;
+const char *hwdb_bin_path = get_hwdb_bin_path();
hwdb = new0(struct udev_hwdb, 1);
if (!hwdb)
@@ -275,30 +283,30 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
hwdb-refcount = 1;
udev_list_init(udev, hwdb-properties_list, true);
-hwdb-f = fopen(/etc/udev/hwdb.bin, re);
+hwdb-f = fopen(hwdb_bin_path, re);
if (!hwdb-f) {
-udev_dbg(udev, error reading /etc/udev/hwdb.bin: %m);
+udev_dbg(udev, error reading %s: %m, hwdb_bin_path);
udev_hwdb_unref(hwdb);
return NULL;
}
if (fstat(fileno(hwdb-f), hwdb-st) 0 ||
(size_t)hwdb-st.st_size offsetof(struct trie_header_f, strings_len) + 8) {
-udev_dbg(udev, error reading /etc/udev/hwdb.bin: %m);
+udev_dbg(udev, error reading %s: %m, hwdb_bin_path);
udev_hwdb_unref(hwdb);
return NULL;
}
hwdb-map = mmap(0, hwdb-st.st_size, PROT_READ, MAP_SHARED, fileno(hwdb-f), 0);
if (hwdb-map == MAP_FAILED) {
-udev_dbg(udev, error mapping /etc/udev/hwdb.bin: %m);
+udev_dbg(udev
[systemd-devel] [PATCH v2] udev hwdb: Support shipping pre-compiled database in system images
Hello again,
the previous patch had a typo in the manpage (it said /lib/udev
instead of /usr/lib/udev at one place), and also forgot to adjust
systemd-udev-hwdb-update.service.in. Both done now.
However, the latter currently has a gotcha:
+ConditionPathExists=!@udevlibexecdir@/hwdb.bin
This works correctly if you use this with the factory reset
semantics, i. e. start with an empty /etc. But it would not work if
you update /usr and have an already existing /etc/udev/hwdb.d/*. So
ideally the condition would be
ConditionPathExists=!@udevlibexecdir@/hwdb.bin OR
ConditionDirectoryNotEmpty=/etc/udev/hwdb.d/
but this isn't possible AFAIK. The alternative would be to change the
Exec= to call hdwb --update --vendor iff /etc/udev/hwdb.d/ is empty.
An alternative that was discussed at Plumbers was to make hwdb
--update automatically use @udevlibexecdir@ if there is no
/etc/udev/hwdb.d/, which would avoid this problem, avoid introducing a
new option, and still generally DTRT. I'd prefer this, so if you think
that's easier/better, I'm happy to update the patch accordingly.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 5c093f2552d7ebbafe079eafe9b0a9bada7615ad Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 17 Oct 2014 15:01:01 +0200
Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system
images
In some cases it is preferable to ship system images with a pre-generated
binary hwdb database, to avoid having to build it at runtime, avoid shipping
the source hwdb files, or avoid storing large binary files in /etc.
So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in
UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/
and re-generating the database which trumps the one in /usr/lib.
Add a new --vendor flag to udevadm hwdb --update which puts the database
into UDEVLIBEXECDIR.
Adjust systemd-udev-hwdb-update.service to not generate the file in /etc if we
already have it in /usr.
---
man/udev.xml | 4 +++-
man/udevadm.xml | 9 +
src/libudev/libudev-hwdb.c| 20 ++--
src/udev/udevadm-hwdb.c | 10 --
units/systemd-udev-hwdb-update.service.in | 1 +
5 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/man/udev.xml b/man/udev.xml
index d77cbb0..87c16c7 100644
--- a/man/udev.xml
+++ b/man/udev.xml
@@ -766,7 +766,9 @@
paraThe content of all hwdb files is read by
citerefentryrefentrytitleudevadm/refentrytitlemanvolnum8/manvolnum/citerefentry
- and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename.
+ and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename,
+ or alternatively filename/usr/lib/udev/hwdb.bin/filename if you want ship the compiled
+ database in an immutable image.
During runtime only the binary database is used./para
/refsect1
diff --git a/man/udevadm.xml b/man/udevadm.xml
index 749144d..571ef7d 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
/listitem
/varlistentry
varlistentry
+ termoption--vendor/option/term
+ listitem
+paraPut the compiled database into filename/usr/lib/udev/hwdb.bin/filename instead.
+Use this if you want to ship a pre-compiled database in immutable system images, or
+don't use filename/etc/udev/hwdb.d/filename and want to avoid large binary files in
+filename/etc/filename./para
+ /listitem
+/varlistentry
+varlistentry
termoption-t/option/term
termoption--test=replaceablestring/replaceable/option/term
listitem
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index 8fb7240..91c05e8 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,13 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
return 0;
}
+static const char *get_hwdb_bin_path(void) {
+if (access(/etc/udev/hwdb.bin, F_OK) = 0)
+return /etc/udev/hwdb.bin;
+else
+return UDEVLIBEXECDIR /hwdb.bin;
+}
+
/**
* udev_hwdb_new:
* @udev: udev library context
@@ -267,6 +274,7 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
_public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
struct udev_hwdb *hwdb;
const char sig[] = HWDB_SIG;
+const char *hwdb_bin_path = get_hwdb_bin_path();
hwdb = new0(struct udev_hwdb, 1);
if (!hwdb)
@@ -275,30 +283,30 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
hwdb-refcount = 1;
udev_list_init(udev, hwdb-properties_list, true);
-hwdb-f
Re: [systemd-devel] [PATCH v2] udev hwdb: Support shipping pre-compiled database in system images
Martin Pitt [2014-10-17 15:44 +0200]: So ideally the condition would be ConditionPathExists=!@udevlibexecdir@/hwdb.bin OR ConditionDirectoryNotEmpty=/etc/udev/hwdb.d/ but this isn't possible AFAIK. Hah, I stand corrected. So this could be ConditionPathExists=|!@udevlibexecdir@/hwdb.bin ConditionPathExists=|/etc/udev/hwdb.bin ConditionDirectoryNotEmpty=|/etc/udev/hwdb.d/ for catching all the edge cases, if we want to go ahead with the explicit --vendor option instead of putting this logic into hwdb --update itself. (I'd otherwise just replicate the logic again in the Debian/Ubuntu package trigger). Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Apply ProtectSystem to non-merged /usr directories
Hello all,
in Debian/Ubuntu we don't use the merged /usr tree for now. systemd
generally supports that (HAVE_SPLIT_USR), but doesn't consider that
for ProtectSystem=.
Ansgar (CC'ed) wrote a Debian specific patch for that some months ago.
I generalized it for upstream now.
Thanks for considering,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 84133fed054b02702955d9371a553c213a45ee9e Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Sun, 19 Oct 2014 11:56:45 -0400
Subject: [PATCH] Apply ProtectSystem to non-merged /usr directories
For systems that don't use a merged /usr, also protect /bin, /sbin, /lib, and
/lib64.
Separately handle /etc (for ProtectSystem=full) to make the code a bit easier
to read.
Replace hard-coded length of systemd dirs list with strv_length() to avoid
pitfalls.
---
src/core/namespace.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/core/namespace.c b/src/core/namespace.c
index ab03aeb..ac48f4d 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -420,6 +420,12 @@ static int make_read_only(BindMount *m) {
return r;
}
+#ifdef HAVE_SPLIT_USR
+#define SYSTEM_DIRS STRV_MAKE(/usr, /bin, /sbin, /lib, -/lib64, -/boot)
+#else
+#define SYSTEM_DIRS STRV_MAKE(/usr, -/boot)
+#endif
+
int setup_namespace(
char** read_write_dirs,
char** read_only_dirs,
@@ -448,7 +454,7 @@ int setup_namespace(
strv_length(inaccessible_dirs) +
private_dev +
(protect_home != PROTECT_HOME_NO ? 3 : 0) +
-(protect_system != PROTECT_SYSTEM_NO ? 2 : 0) +
+(protect_system != PROTECT_SYSTEM_NO ? strv_length(SYSTEM_DIRS) : 0) +
(protect_system == PROTECT_SYSTEM_FULL ? 1 : 0);
if (n 0) {
@@ -496,9 +502,14 @@ int setup_namespace(
}
if (protect_system != PROTECT_SYSTEM_NO) {
-r = append_mounts(m, protect_system == PROTECT_SYSTEM_FULL ? STRV_MAKE(/usr, -/boot, /etc) : STRV_MAKE(/usr, -/boot), READONLY);
+r = append_mounts(m, SYSTEM_DIRS, READONLY);
if (r 0)
return r;
+if (protect_system == PROTECT_SYSTEM_FULL) {
+r = append_mounts(m, STRV_MAKE(/etc), READONLY);
+if (r 0)
+return r;
+}
}
assert(mounts + n == m);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] udev hwdb: Support shipping pre-compiled database in system images
Hello Ivan, Ivan Shapovalov [2014-10-19 5:42 +0400]: but this isn't possible AFAIK. The alternative would be to change the Exec= to call hdwb --update --vendor iff /etc/udev/hwdb.d/ is empty. I'm just an innocent bystander, but isn't it possible with these two lines? ConditionPathExists=|!@udevlibexecdir@/hwdb.bin ConditionDirectoryNotEmpty=|/etc/udev/hwdb.d/ This will succeed if EITHER @udevlibexecdir@/hwdb.bin does not exist OR /etc/udev/hwdb.d/ is not empty. Correct. That's indeed what got pointed out to me on IRC on Friday, and I sent another followup to the list. For some unknown reason that didn't actually hit the list though, so I bounced my reply again this morning. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v3] udev hwdb: Support shipping pre-compiled database in system images
Hello all,
thanks Lennart for the detailled review! Took me a while to respond as
I'm on a company sprint this week. I think I addressed all your
points, plus fixing the unit condition. open_hwdb_bin() isn't the
prettiest thing in the world after the change of immediately
fopen()ing instead of just returning a path, but I don't have a good
idea to beautify it.
I tested ./udevadm hwdb --update with and without --vendor, and
./udevadm test-builtin hwdb in both cases.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 10f1faf25c2846964517c8d8d474e2b3f0b98bfc Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 17 Oct 2014 15:01:01 +0200
Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system
images
In some cases it is preferable to ship system images with a pre-generated
binary hwdb database, to avoid having to build it at runtime, avoid shipping
the source hwdb files, or avoid storing large binary files in /etc.
So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in
UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/
and re-generating the database which trumps the one in /usr/lib.
Add a new --vendor flag to udevadm hwdb --update which puts the database
into UDEVLIBEXECDIR.
Adjust systemd-udev-hwdb-update.service to not generate the file in /etc if we
already have it in /usr.
---
man/udev.xml | 4 +++-
man/udevadm.xml | 9 +
src/libudev/libudev-hwdb.c| 33 +--
src/udev/udevadm-hwdb.c | 13 +++-
units/systemd-udev-hwdb-update.service.in | 3 +++
5 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/man/udev.xml b/man/udev.xml
index d77cbb0..87c16c7 100644
--- a/man/udev.xml
+++ b/man/udev.xml
@@ -766,7 +766,9 @@
paraThe content of all hwdb files is read by
citerefentryrefentrytitleudevadm/refentrytitlemanvolnum8/manvolnum/citerefentry
- and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename.
+ and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename,
+ or alternatively filename/usr/lib/udev/hwdb.bin/filename if you want ship the compiled
+ database in an immutable image.
During runtime only the binary database is used./para
/refsect1
diff --git a/man/udevadm.xml b/man/udevadm.xml
index 749144d..571ef7d 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
/listitem
/varlistentry
varlistentry
+ termoption--vendor/option/term
+ listitem
+paraPut the compiled database into filename/usr/lib/udev/hwdb.bin/filename instead.
+Use this if you want to ship a pre-compiled database in immutable system images, or
+don't use filename/etc/udev/hwdb.d/filename and want to avoid large binary files in
+filename/etc/filename./para
+ /listitem
+/varlistentry
+varlistentry
termoption-t/option/term
termoption--test=replaceablestring/replaceable/option/term
listitem
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index 8fb7240..bfb8a4d 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,26 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
return 0;
}
+static FILE *open_hwdb_bin(const char **path) {
+static const char* candidates[] = {
+/etc/udev/hwdb.bin,
+UDEVLIBEXECDIR /hwdb.bin,
+NULL,
+};
+FILE* f;
+const char** c;
+
+for (c = candidates; *c; ++c) {
+*path = *c;
+f = fopen(*c, re);
+if (f)
+return f;
+else if (errno != ENOENT)
+break;
+}
+return NULL;
+}
+
/**
* udev_hwdb_new:
* @udev: udev library context
@@ -267,6 +287,7 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
_public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
struct udev_hwdb *hwdb;
const char sig[] = HWDB_SIG;
+const char *hwdb_bin_path;
hwdb = new0(struct udev_hwdb, 1);
if (!hwdb)
@@ -275,30 +296,30 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
hwdb-refcount = 1;
udev_list_init(udev, hwdb-properties_list, true);
-hwdb-f = fopen(/etc/udev/hwdb.bin, re);
+hwdb-f = open_hwdb_bin(hwdb_bin_path);
if (!hwdb-f) {
-udev_dbg(udev, error reading /etc/udev/hwdb.bin: %m);
+udev_dbg(udev, error reading %s: %m, hwdb_bin_path);
udev_hwdb_unref(hwdb);
return NULL
[systemd-devel] [PATCH v4] udev hwdb: Support shipping pre-compiled database in system images
Hey Zbyszek,
Zbigniew Jędrzejewski-Szmek [2014-10-24 19:45 +0200]:
This enumeration is also used below... The definition should be shared.
You might want to also consider using NULSTR_FOREACH for iteration.
Ah, thanks for pointing that out.
This function should have the prototype of 'int open_hwdb_bin(const char
**path, FILE *file)'
and return a proper value on error. Right now the return value is passed
through errno, which works but is inelegant.
OK. Personally I prefer ret 0 and errno as that's the standard
POSIX way and also allows using %m etc., but if strerror(-retval) is
the systemd style, fair enough.
The updated patch fixes both now.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 1624d949c60a911f7bf578d0141c8cd0d98c54e9 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 17 Oct 2014 15:01:01 +0200
Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system
images
In some cases it is preferable to ship system images with a pre-generated
binary hwdb database, to avoid having to build it at runtime, avoid shipping
the source hwdb files, or avoid storing large binary files in /etc.
So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in
UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/
and re-generating the database which trumps the one in /usr/lib.
Add a new --vendor flag to udevadm hwdb --update which puts the database
into UDEVLIBEXECDIR.
Adjust systemd-udev-hwdb-update.service to not generate the file in /etc if we
already have it in /usr.
---
man/udev.xml | 4 ++-
man/udevadm.xml | 9 ++
src/libudev/libudev-hwdb.c| 46 ++-
src/udev/udevadm-hwdb.c | 13 -
units/systemd-udev-hwdb-update.service.in | 3 ++
5 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/man/udev.xml b/man/udev.xml
index d77cbb0..87c16c7 100644
--- a/man/udev.xml
+++ b/man/udev.xml
@@ -766,7 +766,9 @@
paraThe content of all hwdb files is read by
citerefentryrefentrytitleudevadm/refentrytitlemanvolnum8/manvolnum/citerefentry
- and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename.
+ and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename,
+ or alternatively filename/usr/lib/udev/hwdb.bin/filename if you want ship the compiled
+ database in an immutable image.
During runtime only the binary database is used./para
/refsect1
diff --git a/man/udevadm.xml b/man/udevadm.xml
index 749144d..571ef7d 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
/listitem
/varlistentry
varlistentry
+ termoption--vendor/option/term
+ listitem
+paraPut the compiled database into filename/usr/lib/udev/hwdb.bin/filename instead.
+Use this if you want to ship a pre-compiled database in immutable system images, or
+don't use filename/etc/udev/hwdb.d/filename and want to avoid large binary files in
+filename/etc/filename./para
+ /listitem
+/varlistentry
+varlistentry
termoption-t/option/term
termoption--test=replaceablestring/replaceable/option/term
listitem
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index 8fb7240..ed8cdff 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,25 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
return 0;
}
+static const char hwdb_bin_paths[] =
+/etc/udev/hwdb.bin\0
+UDEVLIBEXECDIR /hwdb.bin\0;
+
+
+static int open_hwdb_bin(const char **path, FILE** f) {
+const char* p;
+
+NULSTR_FOREACH(p, hwdb_bin_paths) {
+*path = p;
+*f = fopen(p, re);
+if (*f)
+return 0;
+else if (errno != ENOENT)
+return -errno;
+}
+return -errno;
+}
+
/**
* udev_hwdb_new:
* @udev: udev library context
@@ -266,6 +285,8 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
**/
_public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
struct udev_hwdb *hwdb;
+int r;
+const char *hwdb_bin_path;
const char sig[] = HWDB_SIG;
hwdb = new0(struct udev_hwdb, 1);
@@ -275,30 +296,30 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
hwdb-refcount = 1;
udev_list_init(udev, hwdb-properties_list, true);
-hwdb-f = fopen(/etc/udev/hwdb.bin, re);
-if (!hwdb-f) {
-udev_dbg(udev, error reading /etc/udev/hwdb.bin: %m);
+r = open_hwdb_bin(hwdb_bin_path, hwdb-f
Re: [systemd-devel] [PATCH] udev hwdb: Support shipping pre-compiled database in system images
Hey Kay,
Kay Sievers [2014-10-25 23:18 +0200]:
Sounds all fine, I would prefer though to use --usr-only or
--system-only
Indeed, I had some difficulty picking a name, and you already refused
--no-etc. --usr-only wouldn't apply to !SPLIT_USR, --system-only WFM.
I'll commit that, with fixing Zbyszek's suggestion of removing some
unneeded { }.
Thanks,
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
[systemd-devel] string constants vs. #defines [was: Re: [PATCH v4] udev hwdb: Support shipping pre-compiled database in system images]
Hello all, Zbigniew Jędrzejewski-Szmek [2014-10-25 21:51 +0200]: Looks fine and push-worthy. Two minor nitpicks below, feel free to fix up before pushing or ignore as you see fit. Michael raised the question about the option name. I'll wait another day or two for responses (no time today/flight back home until tomorrow). +static const char hwdb_bin_paths[] = +/etc/udev/hwdb.bin\0 +UDEVLIBEXECDIR /hwdb.bin\0; Actually we don't need to define a variable for this, a #define would be enough. Are #defines actually preferred in systemd's code style/conventions? Personally I prefer proper constants, as they are more type safe and less magic, and presumably string constants will end up in the same place in the ELF anyway? Using #defines potentially duplicates the strings too, although I suspect gcc to be clever enough to de-duplicate identical string constants in the source. Is there any downside to using static constants? Parens after NULSTR_FOREACH are not necessary. Ack, will change that. Thanks, 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] udev hwdb: Support shipping pre-compiled database in system images
Lennart Poettering [2014-10-27 16:37 +0100]: After all, moving those files away from /etc doesn't really make sense on split-/usr systems anyway, as it wouldn't help monopolizing vendor data in /usr really... Or at least I think the --usr stuff is really about monopolizing vendor data in /usr, and if you have a split-/usr system then that goal is moot anyway... It still does make sense. On those systems, the OS == /bin, /sbin, /lib*, /usr, /boot. Except for that it's pretty much like a single /usr tree. I. e. with this one can still build system images with these dirs being r/o, and it also avoids an architecture-specific and big binary file in an otherwise config file only /etc. (Yes, /etc/ld.so.cache is somewhat of an exception, but it's arch independent, text, and small). Thanks, 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
[systemd-devel] [PATCH v5] udev hwdb: Support shipping pre-compiled database in system images
Hello Lennart,
Lennart Poettering [2014-10-27 16:09 +0100]:
+static const char hwdb_bin_paths[] =
+/etc/udev/hwdb.bin\0
+UDEVLIBEXECDIR /hwdb.bin\0;
+
+
+static int open_hwdb_bin(const char **path, FILE** f) {
+const char* p;
+
+NULSTR_FOREACH(p, hwdb_bin_paths) {
+*path = p;
Please do not write functions that clobber passed-in arguments on
failure. Please store any result in a temporary variable first, and
clobber the passed-in variables only on success.
+*f = fopen(p, re);
same here.
Done for the FILE*, but I kept the behaviour for the path as the
caller uses the path in the error message on failure. I added a
comment to clarify this.
-if (stat(/etc/udev/hwdb.bin, st) 0)
+
+/* if hwdb.bin doesn't exist anywhere, we need to update */
+NULSTR_FOREACH(p, hwdb_bin_paths) {
+if (stat(p, st) = 0) {
+found = true;
+break;
+}
+}
+if (!found)
return true;
I'd prefer if you could use access(..., F_OK) here, for checking for
file existance, as stat() does substantially more than just checking
existance.
I'm aware of access(), but the code below actually uses the stat value
for timestamp comparison, so we really need stat() here.
I also changed the hwdb option to --usr now. It's not very
descriptive and wrong for /lib, but then again this is only really
being used in package maintainer scripts or custom system image
builds; users will hardly ever see this.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From e0feeb5eb9de066e6908d34dff47380e3e860d09 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 17 Oct 2014 15:01:01 +0200
Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system
images
In some cases it is preferable to ship system images with a pre-generated
binary hwdb database, to avoid having to build it at runtime, avoid shipping
the source hwdb files, or avoid storing large binary files in /etc.
So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in
UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/
and re-generating the database which trumps the one in /usr/lib.
Add a new --usr flag to udevadm hwdb --update which puts the database
into UDEVLIBEXECDIR.
Adjust systemd-udev-hwdb-update.service to not generate the file in /etc if we
already have it in /usr.
---
man/udev.xml | 4 ++-
man/udevadm.xml | 9 ++
src/libudev/libudev-hwdb.c| 50 ++-
src/udev/udevadm-hwdb.c | 13 +++-
units/systemd-udev-hwdb-update.service.in | 3 ++
5 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/man/udev.xml b/man/udev.xml
index d77cbb0..87c16c7 100644
--- a/man/udev.xml
+++ b/man/udev.xml
@@ -766,7 +766,9 @@
paraThe content of all hwdb files is read by
citerefentryrefentrytitleudevadm/refentrytitlemanvolnum8/manvolnum/citerefentry
- and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename.
+ and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename,
+ or alternatively filename/usr/lib/udev/hwdb.bin/filename if you want ship the compiled
+ database in an immutable image.
During runtime only the binary database is used./para
/refsect1
diff --git a/man/udevadm.xml b/man/udevadm.xml
index 749144d..b85d9a9 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
/listitem
/varlistentry
varlistentry
+ termoption--usr/option/term
+ listitem
+paraPut the compiled database into filename/usr/lib/udev/hwdb.bin/filename instead.
+Use this if you want to ship a pre-compiled database in immutable system images, or
+don't use filename/etc/udev/hwdb.d/filename and want to avoid large binary files in
+filename/etc/filename./para
+ /listitem
+/varlistentry
+varlistentry
termoption-t/option/term
termoption--test=replaceablestring/replaceable/option/term
listitem
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index 8fb7240..1089645 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,29 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
return 0;
}
+static const char hwdb_bin_paths[] =
+/etc/udev/hwdb.bin\0
+UDEVLIBEXECDIR /hwdb.bin\0;
+
+
+/* path is also written to on failures, so you can use it in error messages */
+static int open_hwdb_bin(const char **path, FILE** file) {
+const char* p
[systemd-devel] [PATCH v6] udev hwdb: Support shipping pre-compiled database in system images
Hello Lennart,
Lennart Poettering [2014-10-28 11:31 +0100]:
I'd prefer if you'd move the log message for the error into
open_hwdb_bin() then, so that it is not the caller, but the callee
which prints the error message in this case.
That would then mean to move most of udev_hwdb_new() into
open_hwdb_bin(). As the latter is only used once, I instead moved that
logic into udev_hwdb_new(), which also simplifies it somewhat. I also
added a new and clearer error message if hwdb.bin doesn't exist
anywhere.
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From cec6833eb90b6211134ab151b00700f8032914d0 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 17 Oct 2014 15:01:01 +0200
Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system
images
In some cases it is preferable to ship system images with a pre-generated
binary hwdb database, to avoid having to build it at runtime, avoid shipping
the source hwdb files, or avoid storing large binary files in /etc.
So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in
UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/
and re-generating the database which trumps the one in /usr/lib.
Add a new --usr flag to udevadm hwdb --update which puts the database
into UDEVLIBEXECDIR.
Adjust systemd-udev-hwdb-update.service to not generate the file in /etc if we
already have it in /usr.
---
man/udev.xml | 4 ++-
man/udevadm.xml | 9 +++
src/libudev/libudev-hwdb.c| 42 ++-
src/udev/udevadm-hwdb.c | 13 +-
units/systemd-udev-hwdb-update.service.in | 3 +++
5 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/man/udev.xml b/man/udev.xml
index d77cbb0..87c16c7 100644
--- a/man/udev.xml
+++ b/man/udev.xml
@@ -766,7 +766,9 @@
paraThe content of all hwdb files is read by
citerefentryrefentrytitleudevadm/refentrytitlemanvolnum8/manvolnum/citerefentry
- and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename.
+ and compiled to a binary database located at filename/etc/udev/hwdb.bin/filename,
+ or alternatively filename/usr/lib/udev/hwdb.bin/filename if you want ship the compiled
+ database in an immutable image.
During runtime only the binary database is used./para
/refsect1
diff --git a/man/udevadm.xml b/man/udevadm.xml
index 749144d..b85d9a9 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
/listitem
/varlistentry
varlistentry
+ termoption--usr/option/term
+ listitem
+paraPut the compiled database into filename/usr/lib/udev/hwdb.bin/filename instead.
+Use this if you want to ship a pre-compiled database in immutable system images, or
+don't use filename/etc/udev/hwdb.d/filename and want to avoid large binary files in
+filename/etc/filename./para
+ /listitem
+/varlistentry
+varlistentry
termoption-t/option/term
termoption--test=replaceablestring/replaceable/option/term
listitem
diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c
index 8fb7240..a1cfc0b 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,11 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
return 0;
}
+static const char hwdb_bin_paths[] =
+/etc/udev/hwdb.bin\0
+UDEVLIBEXECDIR /hwdb.bin\0;
+
+
/**
* udev_hwdb_new:
* @udev: udev library context
@@ -266,6 +271,7 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
**/
_public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
struct udev_hwdb *hwdb;
+const char *hwdb_bin_path;
const char sig[] = HWDB_SIG;
hwdb = new0(struct udev_hwdb, 1);
@@ -275,30 +281,43 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) {
hwdb-refcount = 1;
udev_list_init(udev, hwdb-properties_list, true);
-hwdb-f = fopen(/etc/udev/hwdb.bin, re);
+/* find hwdb.bin in hwdb_bin_paths */
+NULSTR_FOREACH(hwdb_bin_path, hwdb_bin_paths) {
+hwdb-f = fopen(hwdb_bin_path, re);
+if (hwdb-f)
+break;
+else if (errno == ENOENT)
+continue;
+else {
+udev_dbg(udev, error reading %s: %m, hwdb_bin_path);
+udev_hwdb_unref(hwdb);
+return NULL;
+}
+}
+
if (!hwdb-f) {
-udev_dbg(udev, error reading /etc/udev/hwdb.bin: %m);
+udev_err(udev, hwdb.bin does not exist, please run udevadm hwdb --update
Re: [systemd-devel] [PATCH] hwdb: Ignore brightness keys on Dell Inspiron 1520 to avoid double events
Hello Hans, Hans de Goede [2014-10-30 10:15 +0100]: On the Dell Inspiron 1520 both the atkbd and acpi-video input devices report an event for pressing the brightness up / down key-combos, resulting in user space seeing double events and increasing / decreasing the brightness 2 steps for each keypress. Thanks! Applied with some commit log cleanup (we don't usually do Signed-Off-By etc. in systemd): http://cgit.freedesktop.org/systemd/systemd/commit/?id=aba248e 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
[systemd-devel] Put user@.service cgroups into all controllers (user LXC)
Hello all, LXC upstream (in CC:) supports unprivileged containers, i. e. you can create a rootfs in your $HOME and then run lxc-start on it with some initial preparation [1]. While of course they have some limits, they are very useful for a lot of applications and are by nature quite safe towards other users/containers/services on the same machine. However, that requires putting at least the per-user session cgroup (from logind) into *all* available cgroup controllers, not just the systemd one, so that the per-user container actually has privileges to create sub-cgroups under the session-cN.scope parent. Thus this currently only works with cgmanager (which creates all cgroups that way) or with systemd = 204, which had the Controllers option in logind.conf: Controllers=blkio cpu cpuacct cpuset devices freezer hugetlb memory perf_event net_cls net_prio This certainly wasn't pretty, but it did the job. This option went away from later versions with moving to calling pid1's StartTransientUnit() [2]. I'd like to get this functionality back, to eliminate another blocker for switching Ubuntu to systemd by default, and would like to pick your brain what you'd recommend as a solution. Note: this isn't Ubuntu specific at all, just a generic question whether systemd wants to support LXC's per-user containers, and whether potentially changing the default behaviour would collide with anything else systemd wants (or doesn't want to) do. AFAIUI, the consequence of just always adding the session-cN.scope into all controllers is mostly a very small performance penalty due to the additional cgroup translations. If there are reasons to not do this by default, the other options would be to (re-)introduce some config option (which would certainly look different now, as logind cgroups are now not particularly special compared to other service cgroups), or carrying a downstream patch (least preferred of course, but if necessary we'll have to do that -- we don't want to regress LXC). Hints are appreciated. Thanks! Martin [1] https://www.stgraber.org/2014/01/17/lxc-1-0-unprivileged-containers/ [2] http://cgit.freedesktop.org/systemd/systemd/commit/?id=fb6becb4436ae -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Put user@.service cgroups into all controllers (user LXC)
Jóhann B. Guðmundsson [2014-11-03 16:20 +]: Assuming you have read [1] Is not the solution to this problem to simply drop systemd-shim and cgmanager and just use systemd? For the most part, this can't be retroactively done for stable releases (for the container payloads). Also, I think running systemd as pid 1 in user containers is not currently working, but Stéphane has more details about that. 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 v6] udev hwdb: Support shipping pre-compiled database in system images
Hey Colin, Colin Guthrie [2014-11-05 15:30 +]: While it's a nice error message, I wonder if it should be reverted back to being dbg again for the initrd use case or perhaps some other mechanism could be used to suppress the error in that case? Oh indeed, I didn't consider that this would happen in an initramfs. So +1 from my side for dialing it back to _dbg(). Thanks! 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] Udev rules hardware database
Patrick Häcker [2014-11-05 16:55 +0100]:
I you want to have permanent power saving activated for your devices, the
recommended way is to use udev (e.g.
https://wiki.archlinux.org/index.php/Power_saving#USB_autosuspend). Some
[...]
- Is there already something like this?
By coincidence I recently noticed something interesting in sysfs: My
USB devices seem to have an attribute supports_autosuspend. These
are all 1 except for my USB webcam [1] where it is 0. This sounds very
promising indeed! So apparently the kernel now already grew either a
heuristics or some black/whitelists?
At least the current state how Linux (3.16.0) and udev (215) configure
autosuspend seems a bit weird:
- Some of my USB devices have power/autosuspend == 2: internal
laptop webcam, USB webcam ([1] again), USB Keyboard, USB mouse.
All others have 0. There is no udev rule or other thing on my
system to set those, so unless it's udev or systemd itself I guess
it's from the kernel.
- Most of my USB devices have power/level==auto (i. e. suspend
enabled), only some of them are on (i. e. suspend disabled).
Curiously those which are on are three of above devices where
autosuspend==2: USB webcam, USB Keyboard, USB mouse.
- The only udev rule which I'm aware of that does autosuspend is
42-usb-hid-pm.rules:
ACTION==add, SUBSYSTEM==usb, ATTR{bInterfaceClass}==03,
ATTRS{removable}==fixed, TEST==../power/control,
ATTR{../power/control}=auto
So it does not seem to be the case that we don't currently enable
autosuspend at all, but it's currently highly inconsistant and
confusing.
Martin
[1]
E: ID_MODEL=08af
E: ID_MODEL_FROM_DATABASE=QuickCam Easy/Cool
E: ID_VENDOR=046d
E: ID_VENDOR_FROM_DATABASE=Logitech, Inc.
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] udevadm: Use correct debug logging function
Hello all,
while debugging a curious build failure of umockdev on our ARM
machines [1] I noticed a weird behaviour of udevadm wrt. debug logging
[2].
This patch fixes this (details are in the commit log). Kay, OK to
push?
Thanks,
Martin
[1] https://bugs.debian.org/767909
[2] https://bugs.debian.org/769228
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From f860a812b16e0e2f5e5bf99d59197d25f99f Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 12 Nov 2014 10:55:28 +0100
Subject: [PATCH] udevadm: Use correct debug logging function
For debugging which command gets called in udevadm we want the udev debug
logging, not the systemd one. Otherwise we get rather unexpected udevadm
debugging output (like calling: info) when booting with debug in the kernel
command line, and conversely UDEV_LOG=debug doesn't enable the udevadm
debugging.
https://bugs.debian.org/769228
---
src/udev/udevadm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/udev/udevadm.c b/src/udev/udevadm.c
index 7026c50..9060abd 100644
--- a/src/udev/udevadm.c
+++ b/src/udev/udevadm.c
@@ -76,7 +76,7 @@ static int adm_help(struct udev *udev, int argc, char *argv[]) {
static int run_command(struct udev *udev, const struct udevadm_cmd *cmd, int argc, char *argv[]) {
if (cmd-debug)
log_set_max_level(LOG_DEBUG);
-log_debug(calling: %s, cmd-name);
+udev_dbg(udev, calling: %s, cmd-name);
return cmd-cmd(udev, argc, argv);
}
--
2.1.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] keymap: Fix special keys on ThinkPad X60/X61 Tablet
Hey Bastien, Bastien Nocera [2014-11-12 23:31 +0100]: KEY_DIRECTION is mapped to XF86RotateWindows, to rotate the display: http://cgit.freedesktop.org/xkeyboard-config/commit/symbols/inet?id=ec875f6f9b7c4028e11d32b071989c682e6502bd Ack, good to know. F21 is indeed wrong, that's Touchpad Toggle these days. And F13 is mapped to XF86Tools, which is closest to the original toolbox usage: - KEYBOARD_KEY_68=screenlock # screenlock + KEYBOARD_KEY_68=f13# toolbox I haven't seen such a laptop myself yet, but the best photo I could find on the interweb is http://images.harlander.com/upload/files/artikelbilder/computer/notebooks/lenovo/x60/mit_dock/1000x1000.jpg There are three round black push buttons at the bottom screen edge; the first one is obviously the rotation from above, the third is Escape, the second one is indecipherable for me. It neither looks like a screen lock nor a tool box to me, though; is that the button which generates code 68 which this part of the patch addresses? Thanks, 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 2/2] keymap: Fix special keys on ThinkPad X60/X61 Tablet
Hello again, Martin Pitt [2014-11-13 9:00 +0100]: I haven't seen such a laptop myself yet, but the best photo I could find on the interweb is http://images.harlander.com/upload/files/artikelbilder/computer/notebooks/lenovo/x60/mit_dock/1000x1000.jpg There are three round black push buttons at the bottom screen edge; the first one is obviously the rotation from above, the third is Escape, the second one is indecipherable for me. Ah, nevermind. When searching for an X41 for your other patch, I found http://www.ecoshop.biz/pics/3005/gallery/ThinkPad_X41_Tablet_2.jpg which is quite sharp and clear to recognize. This indeed is a toolbox :-) I pushed both patches. Thank you! 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] /usr vs /etc for default distro units enablement
myunit.service (directly or indirectly) in packaging postinst, but rely on systemctl preset myunit.service instead to capture the distribution (or spin) rules. That part is fair enough, and we can see whether that should be done in Debian, but in practice it will make absolutely no difference wrt. file system layout, cleaning up /etc/, and making admin choices more obvious. This deals means: 1. distro installs the default setup for users, but admins can override later 2. factory reset works fine (assuming systemctl preset-all --preset-mode=enable is run after reset) 3. systemctl status will work (because only units without [Install] sections will have enabling links in /usr/lib) I fully agree on these properties; your recommendations from above (which is essentially the status quo) certainly works for that, but I really think we can do better here. With our proposal these properties would also be satisfied, except with no churn in package install (systemctl enable or preset), initialization of a system with empty /etc/, and cleanliness of /etc. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
meant with this. But I think this was just a followup on IRC where the current implementation was discussed, i. e. as an alternative way around systemctl disable for an existing wants.d symlink in /usr doesn't work. - There are 2 commands to disable an unit: mask for some, disable for others, this can bring confusion and admins won't know the semantic difference between the two (and indeed this is rather technical and unintuitive) Well, the meaning we have been using so far is that statically enabled units are things that does not really make sense to disable (which is different from it being enabled by default), and for all other units the way to enable/disable them (be it by default or manually) is by installing symlinks in /etc. Right, that's how I understood it as well. I. e. an admin would rarely go and mask an unit, unless she really knows what she's doing. OTOH it's perfectly reasonable to disable an unit like apache. - The status reported with systemctl is still disabled when it's not. We can probably improve on that. I guess no one really explored the case of static units with [Install] sections. I think they should not be considered static; they are merely enabled by default in the package, as the wants symlink happens to be in /usr instead of /etc, but other than that they should be exactly the same. My take on this is: make sure presets are applied on firstboot (which I think they are), so empty /etc works just fine, and then improve on systemctl to better show the distinction between 'enabled by default' or 'enabled by choice' (and same for 'disabled'). I guess if the full idea of not requiring the symlinks in /etc in the first place is rejected, then that would be the next best thing. Thanks! 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] /usr vs /etc for default distro units enablement
Hey Colin, thanks for the discussion! Trimming heavily; as you said we should let some other upstreams chime in too, I just have some followup questions. Colin Guthrie [2014-11-18 13:01 +]: * I suppose even wich such a policy the post-installation script still needs to call some systemd-update-policy-mumble-mumble magic to actually apply the new policy? Well, the *.policy files are simply read when calling systemctl preset Right. I meant, even with using presets, a newly installed package still needs to call systemd preset foo.service for all the units that it ships, so that the /etc symlinks are generated. I. e. we merely replace enable (what the current Debian packages do) with preset in the postinst. We need to do that as systemd doesn't automagically spot newly installed units (via inotify or what not) and enable them. Or did I misunderstand this? The idea is that there are very few policy files shipped in a distro Indeed. A generic distro should have exactly one, with disable * (Fedora policy) or enable * (Debian policy). Anything more special would be customization for specialized images/spins/etc. * With that, how would a package then say that it does *not* want a particular unit to get enabled? The idea is that you don't really decide that at a package level, but at a distro level. We do (and that policy drives the auto-generated postinst), but there are always special cases where a package might want to ship a unit which doesn't get enabled by default. I was wondering how that could be accomodated. So for that, the package itself could ship its own preset file, containing a disable myself.service? That would make sense (if I understood it right). Either way, this is certainly the rare exception. * This doesn't solve the problem of having these rather uninteresting and cluttering symlinks in /etc at all; the wants.d symlinks would still be as they are now, just the place that decides when to enable them changes. Indeed, but ultimately if you want to make something configurable in some capacity, you have to put that configuration somewhere and /etc is the defacto place to put it. Fully agreed. Any admin change should go into /etc. My point is, distro/package defaults should *not*. But I'd say that if you, as a vendor, are shipping an enabling symlink in /usr/lib in the first place, you're making a concious decision that this is something you don't generally want an admin to override easily. The admin only really has the choice of masking it. Yeah, that's not what I had in mind. I want to keep the interface and notion of enable/disable for admins, just implement the representation of these choices differently in /etc/ (i. e. just represent the admin changes, not distro defaults plus admin changes). I think moving enabling symlinks into the packages (and thus /usr) has several drawbacks: 1. It pushes decision making on such policy to be distributed through thousands of packages rather than being centralised and thus makes it very difficult to do respins and change such policy centrally. Right, this can't be done with Debian ATM as postinsts use systemctl enable. Moving to preset would fix that part. So, thanks again for pointing that out, this is something that we should do, independently of the (totally orthogonal) discussion of how to treat wants symlinks in /etc/ vs. /usr. 2. It makes the packaging task more complex - these links have to be created during packaging (although I'm sure this could be mostly automated). Yeah, it is. So I guess my reply to this is, this is OK, and I think this goal is not one to aim for. It's state information and it should be represented in /etc and I don't think trying to reduce this is a good idea (personally!) :) Okay. I was wondering if that was merely an oversight, or if there are people who actually want it the way it is currently. Here is the answer :-) Perhaps teaching systemd-delta or systemctl status to show you the preset state would solve this problem? e.g. if it showed something like: [colin@jimmy log (master)]$ systemctl status sshd.service ● sshd.service - OpenSSH server daemon Loaded: loaded (/usr/lib/systemd/system/sshd.service; enabled [preset: disabled]) Perhaps that would help? That would certainly help already, as then admins would have *a* way to check what was changed in the system. Well, I really think that pushing policy down to the package level is a real backward step. I mean, it's one of the main principles that the systemctl preset feature was developed to *avoid* in the first place! Indeed! I think the current packaging scripts in Debian still date from the time when presets didn't exist, so systemctl enable is all we had. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd
Re: [systemd-devel] /usr vs /etc for default distro units enablement
Hey Colin, Colin Guthrie [2014-11-18 14:40 +]: In Mageia we do something similar but we shell out to a script instead. This allows us to replace the implementation without rebuilding all packages. Debian does the same, there's a deb-systemd-helper wrapper called in the postinst scripts which then does that. Longer term, I want to move this to filetriggers. We have been using filetriggers for a while via an RPM patch and it looks like some kind of similar functionality will be (at long last) making it to upstream RPM in the nearish future. I believe .deb supports something like this? Yes, it has for many years. We've used it for ldconfig, rebuilding the initrd, updating gsettings schemas and the like, but not so far for units (and not sure if we actually should, given that not all units *should* be enabled by default -- hence we want to keep this knowledge in the package only). But ultimately this is just a recommendation and there is nothing stopping you being a rebel :D Heh; well, we deliberately wanted to start discussing it here first, as none of this is really very distro specific. After all, one of systemd's aims is to make all the various distros somewhat more alike. I. e. if that minimal /etc/ proposal is refused, we just go on and live with the clutter. 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
[systemd-devel] Improving systemd-nspawn@.service (container dir/nonpersistant journal)
Hello all, we just got a bug report [1] about the systemd-nspawn@.service not working very well by default: First, /var/lib/containers/ does not exist by default. To guard against information leaks or hard link attacks by users, this directory should be 0700 by default. LXC does the same (/var/lib/lxc is 0700 for these reasons). What do you think about adding d /var/lib/containers 0700 - - - to tmpfiles.d/var.conf? I can also add this to the Debian tmpfiles.d file, but it's not really Debian specific. Second, systemd-nspawn@.service uses --link-journal=guest. If you don't have a persistant journal, and /var/log/journal/ does not exist, then containers fail to start in a rather unfriendly way: Spawning container c on /tmp/c. Press ^] three times within 1s to kill container. Container c failed with error code 1. I. e. they don't tell you what's wrong. (SYSTEMD_LOG_LEVEL=debug doesn't help at all). But --link-journal=auto isn't right either as this then won't create the /var/log/journal/machineid symlink if you do have a persistant journal. I don't quite like creating /var/log/journal by default in the package, as that would create persistant journals on the host (for the guests) even though the admin disabled/didn't enable persistant journalling. - Option 1: Change the unit to use guest if /var/log/journal exists, and not use --link-journal at all if it doesn't. (This can't be directly expressed on the nspawn CLI, thus would need some Exec=/bin/sh -c 'if [ -d ... ]' shell commands) - Option 2: Make --link-journal=guest nonfatal and just print out a warning if /var/log/journal/ does not exist. - Any others? I'm happy to work on either solution. Thanks, Martin [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=770275 -- 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] Improving systemd-nspawn@.service (container dir/nonpersistant journal)
Hey,
Lennart Poettering [2014-11-20 12:29 +0100]:
d /var/lib/containers 0700 - - -
to tmpfiles.d/var.conf? I can also add this to the Debian tmpfiles.d
file, but it's not really Debian specific.
Sounds resonable. But first, can you elaborate on the reason for 0700
rather than 0755?
Mostly so that users on the host can't call suid root binaries in the
container. If containers are restricted with selinux/apparmor or
started as user (both happens in Ubuntu for LXC, for example) this
would open a way to escalate root privs in a container into root privs
on the host. https://launchpad.net/bugs/1244635 has the
details/history of this.
I know that upstream systemd doesn't ship AppArmor/SELinux profiles,
and thus your stanza is that containers are inherently insecure. So if
you aren't convinced by the calling of suid root binaries, 0755 is
also ok for upstream, or we just skip this part entirely (it's really
just a small detail, after all).
Patch attached.
Second, systemd-nspawn@.service uses --link-journal=guest. If you
don't have a persistant journal, and /var/log/journal/ does not exist,
then containers fail to start in a rather unfriendly way:
Hmm, another option would be to introduce --link-journal=try-guest
which is identical to --link-journal=guest except that it becomes a
NOP if /var/log/journal doesn't exist and doesn't even generate an
error or warning. Then, we could change -j to mean
--link-journal=try-guest and make that the default to use in the unit
file. I think that would be the best option really.
Excellent, sounds good! Patch attached. I went with a new
link_journal_try flag instead of introducing
LINK_TRY_HOST/LINK_TRY_GUEST, as that would make a lot of if
statements more unwieldy, and it's easy to forget to always check for
both cases. But if you prefer that, I'm happy to change the patch
accordingly.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 62a73c5c87b10ba3a40d012822aa000b176667c3 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Thu, 20 Nov 2014 14:30:52 +0100
Subject: [PATCH 1/2] nspawn: Add try-{host,guest} journal link modes
--link-journal={host,guest} fail if the host does not have persistent
journalling enabled and /var/log/journal/ does not exist. Even worse, as there
is no stdout/err any more, there is no error message to point that out.
Introduce two new modes try-host and try-guest which don't fail in this
case, and instead just silently skip the guest journal setup.
Change -j to mean try-guest instead of guest, and fix the wrong --help
output for it (it said host before).
Change systemd-nsp...@.service.in to use try-guest so that this unit works
with both persistent and non-persistent journals on the host without failing.
https://bugs.debian.org/770275
---
man/systemd-nspawn.xml | 11 ---
src/nspawn/nspawn.c | 37 +
units/systemd-nsp...@.service.in | 2 +-
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml
index b3a2d32..75db65e 100644
--- a/man/systemd-nspawn.xml
+++ b/man/systemd-nspawn.xml
@@ -439,7 +439,9 @@
versa). Takes one of
literalno/literal,
literalhost/literal,
+literaltry-host/literal,
literalguest/literal,
+literaltry-guest/literal,
literalauto/literal. If
literalno/literal, the journal is
not linked. If literalhost/literal,
@@ -453,8 +455,11 @@
guest file system (beneath
filename/var/log/journal/replaceablemachine-id/replaceable/filename)
and the subdirectory is symlinked into the host
-at the same location. If
-literalauto/literal (the default),
+at the same location. literaltry-host/literal
+and literaltry-guest/literal do the same
+but do not fail if the host does not have
+persistant journalling enabled.
+If literalauto/literal (the default),
and the right subdirectory of
filename/var/log/journal/filename
exists, it will be bind mounted
@@ -473,7 +478,7 @@
termoption-j/option/term
listitemparaEquivalent to
-option--link-journal=guest/option./para/listitem
Re: [systemd-devel] [PATCH v2] Improving systemd-nspawn@.service (container dir/nonpersistant journal)
Hello Lennart,
Lennart Poettering [2014-11-21 0:36 +0100]:
So far we prefixed all variables parsed from command line arguments
with arg_, please follow the same scheme for this.
Ah, sure. Done.
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From b743c4546148f61c855ea773e71a660c5dc9aa58 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Thu, 20 Nov 2014 14:30:52 +0100
Subject: [PATCH] nspawn: Add try-{host,guest} journal link modes
--link-journal={host,guest} fail if the host does not have persistent
journalling enabled and /var/log/journal/ does not exist. Even worse, as there
is no stdout/err any more, there is no error message to point that out.
Introduce two new modes try-host and try-guest which don't fail in this
case, and instead just silently skip the guest journal setup.
Change -j to mean try-guest instead of guest, and fix the wrong --help
output for it (it said host before).
Change systemd-nsp...@.service.in to use try-guest so that this unit works
with both persistent and non-persistent journals on the host without failing.
https://bugs.debian.org/770275
---
man/systemd-nspawn.xml | 11 ---
src/nspawn/nspawn.c | 37 +
units/systemd-nsp...@.service.in | 2 +-
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml
index b3a2d32..75db65e 100644
--- a/man/systemd-nspawn.xml
+++ b/man/systemd-nspawn.xml
@@ -439,7 +439,9 @@
versa). Takes one of
literalno/literal,
literalhost/literal,
+literaltry-host/literal,
literalguest/literal,
+literaltry-guest/literal,
literalauto/literal. If
literalno/literal, the journal is
not linked. If literalhost/literal,
@@ -453,8 +455,11 @@
guest file system (beneath
filename/var/log/journal/replaceablemachine-id/replaceable/filename)
and the subdirectory is symlinked into the host
-at the same location. If
-literalauto/literal (the default),
+at the same location. literaltry-host/literal
+and literaltry-guest/literal do the same
+but do not fail if the host does not have
+persistant journalling enabled.
+If literalauto/literal (the default),
and the right subdirectory of
filename/var/log/journal/filename
exists, it will be bind mounted
@@ -473,7 +478,7 @@
termoption-j/option/term
listitemparaEquivalent to
-option--link-journal=guest/option./para/listitem
+option--link-journal=try-guest/option./para/listitem
/varlistentry
varlistentry
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index c2311b3..b4dcf39 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -124,6 +124,7 @@ static bool arg_private_network = false;
static bool arg_read_only = false;
static bool arg_boot = false;
static LinkJournal arg_link_journal = LINK_AUTO;
+static bool arg_link_journal_try = false;
static uint64_t arg_retain =
(1ULL CAP_CHOWN) |
(1ULL CAP_DAC_OVERRIDE) |
@@ -202,8 +203,9 @@ static void help(void) {
--capability=CAP In addition to the default, retain specified\n
capability\n
--drop-capability=CAP Drop the specified capability from the default set\n
---link-journal=MODELink up guest journal, one of no, auto, guest, host\n
- -jEquivalent to --link-journal=host\n
+--link-journal=MODELink up guest journal, one of no, auto, guest, host,\n
+ try-guest, try-host\n
+ -jEquivalent to --link-journal=try-guest\n
--read-onlyMount the root directory read-only\n
--bind=PATH[:PATH] Bind mount a file or directory from the host into\n
the container\n
@@ -428,6 +430,7 @@ static int parse_argv(int argc, char *argv[]) {
case 'j
Re: [systemd-devel] [PATCH] hwdb: fix a typo
Hey Peter, Peter Hutterer [2014-11-25 20:45 +1000]: -# /etc/udev/hwdb.d/70-keyboad.hwdb +# /etc/udev/hwdb.d/70-keyboard.hwdb Applied, thanks! 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] systemd-cgroups-agent not working in containers
Hello all, Cameron Norman [2014-11-27 12:26 -0800]: On Wed, Nov 26, 2014 at 1:29 PM, Richard Weinberger rich...@nod.at wrote: Hi! I run a Linux container setup with openSUSE 13.1/2 as guest distro. After some time containers slow down. An investigation showed that the containers slow down because a lot of stale user sessions slow down almost all systemd tools, mostly systemctl. loginctl reports many thousand sessions. All in state closing. This sounds similar to an issue that systemd-shim in Debian had. Martin Pitt (helps to maintain systemd in Debian) fixed that issue; he may have some ideas here. I CC'd him. The problem with systemd-shim under sysvinit or upstart was that shim didn't set a cgroup release agent like systemd itself does. Thus the cgroups were never cleaned up after all the session processes died. (See 1.4 on https://www.kernel.org/doc/Documentation/cgroups/cgroups.txt for details) I don't think that SUSE uses systemd-shim, I take it in that setup you are running systemd proper on both the host and the guest? Then I suggest checking the cgroups that correspond to the closing sessions in the container, i. e. /sys/fs/cgroup/systemd/.../session-XX.scope/tasks. If there are still processes in it, logind is merely waiting for them to exit (or set KillUserProcesses in logind.conf). If they are empty, check that /sys/fs/cgroup/systemd/.../session-XX.scope/notify_on_release is 1 and that /sys/fs/cgroup/systemd/release_agent is set? 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] Native Journal source vs syslog forwarding
Hey Gergely, Gergely Nagy [2014-11-26 13:07 +0100]: Forwarding is enabled by default on Debian, as I wrote in my original mail. I have no control over the default, and I have no desire to argue for changing it. I'm just packaging systemd 217, and will revert the disabled forwarding by default (i. e. retain the behaviour of 215). As a systemd package maintainer I'm also not in a position to unilaterally changing the default and breaking rsyslog and friends. So this requires some coordination indeed (and either way, none of this is a matter for the frozen jessie). So we either need the journal.conf.d/ feature and have journal-pulling sysloggers disable forwarding along the way, or we need to wait until all packaged sysloggers can read from the journal before we turn off forwarding by default. Thanks, 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
[systemd-devel] [PATCH] Fix hostnamectl exit code
Hello all,
while packaging 217 my integration tests for hostnamed yelled at me
that hostnamectl fails. Indeed it exits with 1 now even though it
succeeds.
Trivial patch attached. OK to push?
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 416e06379d58a3f5b22cf3bd3ad587cc59b10c3e Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 28 Nov 2014 15:38:05 +0100
Subject: [PATCH] hostnamectl: Exit with zero on success
In show_all_names(), bus_map_all_properties() returns 1 on success which is
then used as the return code of show_all_names() and eventually main(). As for
a shell command nonzero signals failure, and the documentation also says 0 on
success, fix show_all_names() to return 0 on success just like show_one_name()
and the other dispatchers.
---
src/hostname/hostnamectl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c
index e487369..acd107d 100644
--- a/src/hostname/hostnamectl.c
+++ b/src/hostname/hostnamectl.c
@@ -216,7 +216,7 @@ fail:
free(info.virtualization);
free(info.architecture);
-return r;
+return r 0 ? r : 0;
}
static int show_status(sd_bus *bus, char **args, unsigned n) {
--
2.1.3
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Fix hostnamectl exit code
Hey David,
David Herrmann [2014-11-28 15:49 +0100]:
Why not fix all those other occurrences with one fix by changing
hostnamectl_main() the last line from:
return r 0 ? EXIT_FAILURE : r;
to
return r 0 ? EXIT_FAILURE : 0;
Usually, =0 means success, 0 failure, in systemd. We should not
return !=0 from main() if the return value wasn't negative.
Yeah, that's even more robust and guards against similar errors in the
future. Updated patch attached.
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From c8c55d0aa621ad2998b15ff461b20b3fcb1361b0 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Fri, 28 Nov 2014 15:38:05 +0100
Subject: [PATCH] hostnamectl: Exit with zero on success
In show_all_names(), bus_map_all_properties() returns 1 on success which is
then used as the return code of show_all_names() and eventually main(). Exit
with zero in main() on all nonnegative results to guard against similar errors.
---
src/hostname/hostnamectl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c
index e487369..ff4e9c9 100644
--- a/src/hostname/hostnamectl.c
+++ b/src/hostname/hostnamectl.c
@@ -536,5 +536,5 @@ int main(int argc, char *argv[]) {
r = hostnamectl_main(bus, argc, argv);
finish:
-return r 0 ? EXIT_FAILURE : r;
+return r 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}
--
2.1.3
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] cgroup-util: Add more well-known controller names
Hey Lennart, Lennart Poettering [2014-12-01 0:44 +0100]: +hugetlb\0 +cpuset\0 +net_cls\0 +net_prio\0 +freezer\0 +perf_event\0; In general we should be really careful about manipulating hierarchies we don't know enough about, since some controllers actually have an effect on a process even with no actual cgroup property changed from the default (cpu is an obvious one, which breaks RT scheduling). I asked Stéphane (LXC upstream) about that some days/weeks ago, and he said that this issue was fixed upstream in the kernel. I have no further info about this, perhaps he can reply with some details? I figure before adding these controllers to the list systemd manages we need to open the discussion with Tejun about the future of the respective controllers... I think it would be better to apply a patch of thee downstream if at all, for now, if you need it now. Ack, that's ok. 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
[systemd-devel] [WIP PATCH] Do not realize and migrate cgroups multiple times
Hello all,
In my efforts to make user LXC containers work I noticed that under a
real desktop (not just nspawn with VT login or ssh logins) my
carefully set up cgroups in the non-systemd controllers get reverted.
I. e. I put the session leader (and all other pids) of logind sessions
(/user.slice/user-1000.slice/session-XX.scope) into all cgroup
controllers, but a second after they are all back in the / cgroups (or
sometimes in /user.slice). After some days of debugging (during which
I learned a lot about the innards of systemd :-) I finally found out
why:
During unit cgroup initialization (unit_realize_cgroup), sibling
cgroups are queued instead of initialized immediately.
unit_create_cgroups() makes an attempt to avoid initializing and
migrating a cgroup more than once:
path = unit_default_cgroup_path(u);
[...]
r = hashmap_put(u-manager-cgroup_unit, path, u);
if (r 0) {
log_error(r == -EEXIST ? cgroup %s exists already: %s :
hashmap_put failed for %s: %s, path, strerror(-r));
return r;
}
But this doesn't work: path always gets allocated freshly in that
function, so the pointer is never in the hashmap and the -EEXISTS
never actually hits. This causes -.slice to be initialized and
recursively migrated a gazillion times, which explains the random
punting of sub-cgroup PIDs back to /.
I fixed this with an explicit hashmap_get() call, which compares
values instead of pointers.
I also added some debugging to that function:
log_debug(unit_create_cgroups %s: path=%s realized %i hashmap %p, u-id,
path, u-cgroup_realized, hashmap_get(u-manager-cgroup_unit, path));
(right before the hashmap_put() above), which illustrates the
problem:
systemd[1]: Starting Root Slice.
systemd[1]: unit_create_cgroups -.slice: path= realized 0 hashmap (nil)
systemd[1]: Created slice Root Slice.
[...]
pid1 systemd[1]: unit_create_cgroups session-c1.scope:
path=/user.slice/user-1000.slice/session-c1.scope realized 0 hashmap (nil)
systemd[1]: Started Session c1 of user martin.
[... later on when most things have been started ...]
systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850
systemd[1]: unit_create_cgroups -.slice: cgroup exists already
systemd[1]: Failed to realize cgroups for queued unit user.slice: File exists
systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850
systemd[1]: unit_create_cgroups -.slice: cgroup exists already
systemd[1]: Failed to realize cgroups for queued unit grub-common.slice: File
exists
systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850
systemd[1]: unit_create_cgroups -.slice: cgroup exists already
systemd[1]: Failed to realize cgroups for queued unit networking.slice: File
exists
... and so on, basically once for each .service.
Initializing -.slice so many times is certainly an unintended effect
of the peer cgroup creation. Thus the patch fixes the multiple
initialization/creation, but a proper fix should also quiesce these
error messages. The condition could be checked explicitly, i. e. we
skip the Failed to realize... error for EEXISTS, or we generally
tone this down to log_debug. I'm open to suggestions. And of course
the log_debug should be removed; it's nice to illustrate the problem,
but doesn't really belong into production code.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From fd2f8a444c9c644a39dd3b619934e8768e03c2a3 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Mon, 1 Dec 2014 10:50:06 +0100
Subject: [PATCH] Do not realize and migrate cgroups multiple times
unit_create_cgroups() tries to check if a cgroup already exists. But has the
destination path is always allocated dynamically as a new string, that pointer
will never already be in the hashmap, thus hashmap_put() will never actually
fail with EEXISTS. Thus check for the existance of the cgroup path explicitly.
Before this, -.slice got initialized and its child PIDs migrated many times
through queuing the realization of sibling units; thiss caused any cgroup
controller layout from sub-cgroups to be reverted and their pids moved back to
the root cgroup in all controllers (except systemd).
---
src/core/cgroup.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 8820bee..3d36080 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -614,6 +614,13 @@ static int unit_create_cgroups(Unit *u, CGroupControllerMask mask) {
if (!path)
return log_oom();
+log_debug(unit_create_cgroups %s: path=%s realized %i hashmap %p, u-id, path, u-cgroup_realized, hashmap_get(u-manager-cgroup_unit, path));
+
+if (hashmap_get(u-manager-cgroup_unit, path)) {
+log_error(unit_create_cgroups %s: cgroup %s exists already, u-id, u-cgroup_path
Re: [systemd-devel] [WIP PATCH] Do not realize and migrate cgroups multiple times
Michal Sekletar [2014-12-02 12:32 +0100]: Also this looks like a possible fix to the problem I tried to describe in, http://lists.freedesktop.org/archives/systemd-devel/2014-November/025607.html Yes, most probably. While I found that bug in the context of LXC user containers, it's by no means restricted to that case. Any unit which sets up cgroups in other controllers will be affected by that. 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] /usr vs /etc for default distro units enablement
Hello all, sorry for the late response. Andrei Borzenkov [2014-12-05 10:58 +0300]: That's not how I actually understood it. enable/disable still applies only to units with [Install] section as it is now. Just that Correct. I don't see any need to change the behaviour of static units, and I don't want to change the visible effect of systemctl/disable, nor the current semantics of changing wants/ symlinks in /etc. unit foo.service is disabled if [...[ 2. There are no links from [Install] in /usr/lib or /etc *OR* there are links in /usr/lib which are masked in /etc. Indeed the part after the OR is the only change that I propose. I. e. - systemctl enable: If /usr/.../wants/foo.service exists, remove the /dev/null symlink in /etc/.../wants/foo.service if it exists (if not, it's already enabled). Otherwise, behave as now. - systemctl disable: If /usr/.../wants/foo.service exists, create a /dev/null symlink in /etc/.../wants/foo.service if it doesn't exist yet (if it does, it's already disabled). Otherwise, behave as now. This will allow to cleanly separate distribution default (/usr/lib) and admin decision (/etc). Also this will allow systemctl list-unit-files to supply information like enabled (default)/enabled (admin) depending on whether link in /usr/lib or /etc exists. Exactly. Thanks, 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] /usr vs /etc for default distro units enablement
Lennart Poettering [2014-12-05 14:52 +0100]: To be honest I find the entire stuff with ENABLED=true/false really questionnable, I think it would be agreat step ahead to get rid of it. (But then again, I cannot make Debian's decisions there...) Indeed it is. It has never really been necessary, as all init systems have their canonical way of disabling/enabling things: SysV with the /etc/rc?.d/ symlinks, systemd with the wants/ symlinks, upstart with empty .override files. And Debian even has a script update-rc.d enable|disable foo which does that for all init systems. So the ENABLED in /etc/default/foo has always been entirely redundant and confusing. Fortunately it isn't that widespread, but some packages need to be cleaned up there. Anyway, quite a far disgression into distro specific oddities now. :-) Only preinst can (getting the install or upgrade argument), not postinst (getting configure in both case). And we need to run the preset/enable in postinst (meaning: after unpacking). This sounds quite a limitation. Maybe you can keep a couple of touch files in /var/lib/ somewhere where you store whether you already applied systemctl preset before? It's not a limitation, the postinst *can* differ between initial install and upgrade by looking at $2 (the most recently configured version). If it's empty, it's a new install. 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] /usr vs /etc for default distro units enablement
Hello, Andrei Borzenkov [2014-12-07 14:39 +0300]: Indeed the part after the OR is the only change that I propose. I. e. - systemctl enable: If /usr/.../wants/foo.service exists, remove the /dev/null symlink in /etc/.../wants/foo.service if it exists (if not, it's already enabled). Otherwise, behave as now. - systemctl disable: If /usr/.../wants/foo.service exists, create a /dev/null symlink in /etc/.../wants/foo.service if it doesn't exist yet (if it does, it's already disabled). Otherwise, behave as now. I think systemctl enable|disable should always create respective links in /etc. It makes it obvious that this is admin decision. Fully agreed, but that's exactly what I proposed, no? Of course it always acts on /etc, just the kind of symlink it removes/adds is different depending on whether a unit is already enabled by /usr/.../wants/foo.service or not. 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
[systemd-devel] timedatectl regression in 218: crash with Etc/UTC
Hey Tom, all,
with systemd 218 our integration tests picked out a regression with
timedatectl: just calling it without any arguments crashes with
*** Error in `./timedatectl': free(): invalid next size (fast): 0xf8cce8d8 ***
I only get this crash on i386 (32 bit), not x86_64. It also only
happens with /etc/timezone having Etc/UTC, other time zones like
Europe/Berlin work. Presumably because Etc/UTC does not have any DST
rules.
Back trace, with details from the interesting frame:
#0 0xf7fd9ca0 in __kernel_vsyscall ()
#1 0xf7e2b607 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#2 0xf7e2cd83 in __GI_abort () at abort.c:89
#3 0xf7e6a9e3 in __libc_message (do_abort=do_abort@entry=1,
fmt=fmt@entry=0xf7f628fc *** Error in `%s': %s: 0x%s ***\n)
at ../sysdeps/posix/libc_fatal.c:175
#4 0xf7e70b0a in malloc_printerr (action=optimized out, str=0xf7f62a9c
free(): invalid next size (fast), ptr=0x565b98d8)
at malloc.c:4996
#5 0xf7e71765 in _int_free (av=0xf7fa7420 main_arena, p=optimized out,
have_lock=0) at malloc.c:3840
#6 0x5659b613 in freep (p=0xd6c8) at src/shared/util.h:659
#7 0x5659c185 in time_get_dst (date=1418644009, tzfile=0x5659e9ee
/etc/localtime, switch_cur=0xd81c, zone_cur=0xd810,
dst_cur=0xd809, switch_next=0xd824, delta_next=0xd820,
zone_next=0xd814, dst_next=0xd80a)
at src/shared/time-dst.c:104
at src/shared/time-dst.c:104
type_idxs = 0x565b98d8
num_types = 1
types = 0x565b98d8
zone_names = 0x565b98e0 UTC
st = {st_dev = 40, __pad1 = 348, __st_ino = 351284, st_mode = 33188,
st_nlink = 1, st_uid = 0, st_gid = 0, st_rdev = 0,
__pad2 = 4, st_size = 118, st_blksize = 4096, st_blocks = 8, st_atim
= {tv_sec = 1418641254, tv_nsec = 924653672},
st_mtim = {tv_sec = 1418641254, tv_nsec = 920653672}, st_ctim =
{tv_sec = 1418641254, tv_nsec = 920653672},
st_ino = 351284}
num_isstd = 1
num_isgmt = 1
tzhead = {tzh_magic = TZif, tzh_version = 2, tzh_reserved = '\000'
repeats 14 times,
tzh_ttisgmtcnt = \000\000\000\001, tzh_ttisstdcnt =
\000\000\000\001, tzh_leapcnt = \000\000\000,
tzh_timecnt = \000\000\000, tzh_typecnt = \000\000\000\001,
tzh_charcnt = \000\000\000\004}
chars = 4
i = 1
total_size = 12
types_idx = 0
trans_width = 4
tzspec_len = 0
num_leaps = 0
lo = 4294956851
hi = 1
num_transitions = 0
transitions = 0x565b98d8
f = 0x565b9970
#8 0x5655965b in print_status_info (i=0xd924) at
src/timedate/timedatectl.c:167
#9 0x56559ae9 in show_status (bus=0x565b8008, args=0xdaa8, n=0) at
src/timedate/timedatectl.c:239
#10 0x5655aa8b in timedatectl_main (bus=0x565b8008, argc=1, argv=0xdaa4) at
src/timedate/timedatectl.c:542
#11 0x5655aba7 in main (argc=1, argv=0xdaa4) at
src/timedate/timedatectl.c:563
I bisected this to commit 681f9718c shared: time-dst - ensure nulstr is null
terminated, reverting it fixes the crash.
+zone_names[chars] = '\0';
At first sight this smells like writing one past the array boundary,
if chars is the length of a string.
Was there some actual visible bug before this?
Thanks!
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
[systemd-devel] [PATCH] Re: timedatectl regression in 218: crash with Etc/UTC
Martin Pitt [2014-12-15 12:50 +0100]:
#7 0x5659c185 in time_get_dst (date=1418644009, tzfile=0x5659e9ee
/etc/localtime, switch_cur=0xd81c, zone_cur=0xd810,
dst_cur=0xd809, switch_next=0xd824, delta_next=0xd820,
zone_next=0xd814, dst_next=0xd80a)
at src/shared/time-dst.c:104
at src/shared/time-dst.c:104
type_idxs = 0x565b98d8
num_types = 1
types = 0x565b98d8
zone_names = 0x565b98e0 UTC
st = {st_dev = 40, __pad1 = 348, __st_ino = 351284, st_mode = 33188,
st_nlink = 1, st_uid = 0, st_gid = 0, st_rdev = 0,
__pad2 = 4, st_size = 118, st_blksize = 4096, st_blocks = 8,
st_atim = {tv_sec = 1418641254, tv_nsec = 924653672},
st_mtim = {tv_sec = 1418641254, tv_nsec = 920653672}, st_ctim =
{tv_sec = 1418641254, tv_nsec = 920653672},
st_ino = 351284}
num_isstd = 1
num_isgmt = 1
tzhead = {tzh_magic = TZif, tzh_version = 2, tzh_reserved =
'\000' repeats 14 times,
tzh_ttisgmtcnt = \000\000\000\001, tzh_ttisstdcnt =
\000\000\000\001, tzh_leapcnt = \000\000\000,
tzh_timecnt = \000\000\000, tzh_typecnt = \000\000\000\001,
tzh_charcnt = \000\000\000\004}
chars = 4
i = 1
total_size = 12
types_idx = 0
trans_width = 4
tzspec_len = 0
num_leaps = 0
lo = 4294956851
hi = 1
num_transitions = 0
transitions = 0x565b98d8
f = 0x565b9970
To clarify:
186 transitions = malloc0(total_size + tzspec_len);
transitions gets 12 bytes allocated (see above frame for values of
variables).
192types = (struct ttinfo *)((char *)transitions + types_idx);
As types_idx == 0, types == transitions, thus 12 bytes long.
193zone_names = (char *)types + num_types * sizeof(struct ttinfo);
num_types == 1, thus zone_names == types + 8, i. e. zone_names is 4
bytes. chars is 4 bytes, thus
247zone_names[chars] = '\0';
writes at zone_names[4] aka transitions[12] which is one byte past the
allocated buffer.
I think the most robust solution would be to just allocate an extra
byte so that we can always actually fit that null byte. Does that
sound ok?
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 597f3557b73ae0b4b449dc54b6b0e0a720864051 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Mon, 15 Dec 2014 13:06:48 +0100
Subject: [PATCH] shared: time-dst: Avoid buffer overflow
Commit 681f9718 introduced an additional null terminator for the zone names.
Increase the allocation of transitions to actually make room for this.
---
src/shared/time-dst.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/shared/time-dst.c b/src/shared/time-dst.c
index 926d22b..1ce6f72 100644
--- a/src/shared/time-dst.c
+++ b/src/shared/time-dst.c
@@ -183,7 +183,8 @@ read_again:
return -EINVAL;
}
-transitions = malloc0(total_size + tzspec_len);
+/* leave space for additional zone_names zero terminator */
+transitions = malloc0(total_size + tzspec_len + 1);
if (transitions == NULL)
return -EINVAL;
--
2.1.3
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] loopback setup in unprivileged containers
Hello all,
I'm forwarding a patch for the loopback setup from Stéphane. I already
pushed one part of it as
http://cgit.freedesktop.org/systemd/systemd/commit/?id=58a489c
which is trivial and obvious, but the other part isn't.
Thanks,
Martin
- Forwarded message from Stéphane Graber stgra...@ubuntu.com -
Date: Thu, 25 Dec 2014 22:17:52 +0100
From: Stéphane Graber stgra...@ubuntu.com
To: martin.p...@ubuntu.com
Subject: systemd patch for unprivileged containers
X-Spam-Status: No, score=-1.9 required=3.4 tests=BAYES_00 autolearn=no
version=3.3.2
Hey Martin,
Attached is a pretty simple patch/workaround to fix the massive CPU
usage of systemd in unprivileged containers.
LXC provides each containers with an already-UP loopback device. systemd
will attempt to bring it up regardless of its current state and doing so
gets it into a broken codepath somewhere deep in the netlink handling
code of systemd.
The fix is to always check whether the loopback is ready to use before
doing anything.
I then noticed that this code path in systemd was broken too due to it
checking against 1.0.0.127 rather than 127.0.0.1 so I added the missing
htonl and now it all works as expected and systemd fails on the next
problem about a minute faster than it used to :)
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
- End forwarded message -
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From af71697dcc084b81f4c585c4b0c9dd6ac51b77d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= stgra...@ubuntu.com
Date: Sat, 27 Dec 2014 19:22:58 +0100
Subject: [PATCH] core: Optimize loopback setup
Check if the loopback device is already set up (like with LXC) before doing any
setup ourselves.
---
src/core/loopback-setup.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/core/loopback-setup.c b/src/core/loopback-setup.c
index ab6335c..e46b70a 100644
--- a/src/core/loopback-setup.c
+++ b/src/core/loopback-setup.c
@@ -86,15 +86,15 @@ int loopback_setup(void) {
_cleanup_rtnl_unref_ sd_rtnl *rtnl = NULL;
int r;
+if (check_loopback())
+return 0;
+
r = sd_rtnl_open(rtnl, 0);
if (r 0)
return r;
r = start_loopback(rtnl);
-if (r == -EPERM) {
-if (check_loopback() 0)
-return log_warning_errno(EPERM, Failed to configure loopback device: %m);
-} else if (r 0)
+if (r 0)
return log_warning_errno(r, Failed to configure loopback device: %m);
--
2.1.3
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Fix install location of systemd.pc
Hello all, systemd.pc is currently installed into /usr/share/pkgconfig/, but this isn't correct: It contains libdir whose value is (possibly) architecture specific. E. g. if you configure with --libdir=/usr/lib/x86_64-linux-gnu (we do that in Debian for multi-arch support) systemd.pc contains libdir=/usr/lib/x86_64-linux-gnu which isn't architecture agnostic and thus not suitable for /usr/share/. Attached patch fixes this, and puts systemd.pc into the same pkgconfig dir as the libraries. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) From d8cfbf1c7696371ee5f73e42a3dd031d67044099 Mon Sep 17 00:00:00 2001 From: Martin Pitt martin.p...@ubuntu.com Date: Sun, 28 Dec 2014 12:14:25 +0100 Subject: [PATCH] build-sys: Fix install location of systemd.pc systemd.pc contains libdir which can be architecture specific. Thus it needs to be installed into libdir/pkgconfig/ instead of datadir/pkgconfig. As nothing else is using pkgconfigdata any more, remove it entirely. Note that udev.pc does not contain architecture specific values and thus can be kept in /usr/share/pkgconfig/. --- Makefile.am | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index e1e0843..631d67e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -74,7 +74,6 @@ dbussessionservicedir=@dbussessionservicedir@ dbussystemservicedir=@dbussystemservicedir@ pamlibdir=@pamlibdir@ pamconfdir=@pamconfdir@ -pkgconfigdatadir=$(datadir)/pkgconfig pkgconfiglibdir=$(libdir)/pkgconfig polkitpolicydir=$(datadir)/polkit-1/actions bashcompletiondir=@bashcompletiondir@ @@ -1302,7 +1301,7 @@ dist_dbussystemservice_DATA += \ polkitpolicy_in_in_files += \ src/core/org.freedesktop.systemd1.policy.in.in -pkgconfigdata_DATA = \ +pkgconfiglib_DATA += \ src/core/systemd.pc nodist_rpmmacros_DATA = \ @@ -6081,7 +6080,6 @@ EXTRA_DIST += \ CLEANFILES += \ $(nodist_systemunit_DATA) \ $(nodist_userunit_DATA) \ - $(pkgconfigdata_DATA) \ $(pkgconfiglib_DATA) \ $(nodist_polkitpolicy_DATA) -- 2.1.3 signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Quiesce audit message flood from 218
Hello all, systemd 218 now enables audit in the kernel unconditionally [1]. While these messages might be nice to have in the journal, they literally flood dmesg and thus /var/log/syslog and friends with messages like [39098.129349] audit: type=1105 audit(1419765421.403:4233): pid=25633 uid=0 auid=0 ses=20 msg='op=PAM:session_open acct=root exe=/usr/sbin/cron hostname=? addr=? terminal=cron res=success' $ dmesg |grep -c audit 786 and more importantly, eats a lot of real kernel/daemon messages due to rate limiting: I have many dozen messages like [37444.978307] audit_printk_skb: 222 callbacks suppressed and they demonstrably cause e. g. AppArmor violations to not get shown due to this. Is there a way to make the audit messages *only* go to the journal, but not to dmesg and sysloggers? If not, could we perhaps add a ./configure or config file option for this, to disable audit on systems where we don't need it? Thanks, Martin [1] http://cgit.freedesktop.org/systemd/systemd/commit/?id=4d9ced995 -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] fix minor typo in comment
Hey Sylvain, both applied, thank you! 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] Fix install location of systemd.pc
Mike Gilbert [2014-12-28 12:41 -0500]: From Lennart's commit message, it seems like this was done intentionally. The addition of libdir was certainly intentional, that's why I didn't propose to just remove libdir. But it looks like this was just missing to adjust the install location accordingly? 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] loopback setup in unprivileged containers
Hey Tom, Tom Gundersen [2014-12-29 2:22 +0100]: The bug should now be fixed in git. Please let me know if you still experience problems. Nice! I confirm that the systemd spins 100% CPU for about one minute when booting an user level container is indeed fixed now. I can't test it much further as I don't have the other bits. @Stephane: I'll pull these patches into our package. Many thanks, 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] man: Fix spelling
Hey Susant, Applied, thanks! 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 1/2] accelerometer: drop unused -x option
Hey Robert, applied, thanks! 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] Fix install location of systemd.pc
Simon Peeters [2014-12-29 15:01 +]: I have no preference between the 2, but moving the pc file to $libdir/pkgconfig just does not make sense. Why not? pkg-config looks in both /usr/share and /usr/lib, so it doesn't care. And you can't install systemd for multiple architectures in parallel anyway, so you'd only ever have one version installed. 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] Fix install location of systemd.pc
Hey Zbyszek, Zbigniew Jędrzejewski-Szmek [2015-01-01 15:14 +0100]: Yeah, you can find out the libdir in other ways, but having it in the .pc file is convenient. Martin, please push the patch. Ack, done. And happy new year everyone! Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Deadlocks with reloading jobs which are part of current transaction [was: [PATCH] Avoid reloading services when shutting down]
Lennart Poettering [2015-02-04 16:38 +0100]: Sure, I can only recommend again: in the the glue code that calls out to systemctl from service, you can add the code to use --no-block or --job-mode=ignore-dependencies , if you notice you are in shutdown mode... Yeah, I agree that given all the options that's the best heuristics for now. Modern code, that talks directly to systemctl, that uses hook scripts, should always use --no-block for these cases. Yeah, for sure. We just don't have that luxury easily, as first these scripts don't call systemctl but service (for the Debianists: or invoke-rc.d, but same difference), it will take some time to find all these occurrences, and people will hate us for having to add stuff like if (running under systemd) systemctl --no-block reload foo else service foo reload I am not proposing anything like that. Right, this was just a reply to modern code that talks directly to systemctl, and we can't do that while we support more than one init system. Sorry for the misunderstanding. - Don't enqueue a reload if the service to be reloaded isn't running. E. g. postfix.service inactive/dead in https://bugs.debian.org/635777 or smbd.service start/waiting in https://launchpad.net/bugs/1417010. This would completely avoid the deadlock in most situations already, and doesn't change the semantics for working use cases either, so this should even be applicable for upstream? For the record, this was already discussed here: http://lists.freedesktop.org/archives/systemd-devel/2014-July/021457.html continuation: http://lists.freedesktop.org/archives/systemd-devel/2014-August/022048.html I mean, if you want precise sysv semantics you can just use --job-mode=ignore-dependencies always, since sysv ignore all deps too when sysv scripts are involved. Always sounds rather unsafe to me, as we would totally ignore a unit's Requires/Wants= then. Doing it if !systemctl is-system-running sounds ok. I'd strongly encourage you to work around this on client side in the sysv glue, not breaking the guarantess that systemd-aware code needs when issuing commands. Yes, I prefer that as well. (That's what I meant with distro side..) Thanks, 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
[systemd-devel] [PATCH] sysv-generator: Skip init scripts for existing native services
Hello all,
a little while ago, Jon Severinsson wrote a sysv generator
optimization to not go through all the parsing of init.d scripts and
creation of units if there already is a native unit for that name. As
they are put into generator.late they would be ignored anyway.
This is particularly relevant if you have lots of init.d scripts, like
we have on Debian. Other than that it's not a behaviour change AFAICS.
I cleaned it up a bit and added a test case.
One thing I wonder about is whether native_unit_exists() should
perhaps be moved into src/shared/? It might be useful for other stuff.
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From ae066ed5d5b7312eb8debc30970f6e56919fa7c7 Mon Sep 17 00:00:00 2001
From: Jon Severinsson j...@severinsson.net
Date: Wed, 2 Jul 2014 22:00:00 +0200
Subject: [PATCH] sysv-generator: Skip init scripts for existing native
services
There's no need to do all the parsing and creation of service files if we
already have a native systemd unit for the processed SysV init script.
---
src/sysv-generator/sysv-generator.c | 30 +-
test/sysv-generator-test.py | 12
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 673f04d..3052326 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -723,6 +723,25 @@ static int fix_order(SysvStub *s, Hashmap *all_services) {
return 0;
}
+static int native_unit_exists(LookupPaths lp, char *name) {
+char **p;
+
+STRV_FOREACH(p, lp.unit_path) {
+struct stat st;
+_cleanup_free_ char *path = NULL;
+
+path = strjoin(*p, /, name, NULL);
+if (!path)
+return -ENOMEM;
+
+if (lstat(path, st) 0)
+continue;
+
+return 1;
+}
+return 0;
+}
+
static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
char **path;
@@ -768,6 +787,14 @@ static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
if (!fpath)
return log_oom();
+r = native_unit_exists(lp, name);
+if (r 0)
+return log_oom();
+if (r 0) {
+log_debug(Native unit for %s already exists, skipping, *path);
+continue;
+}
+
service = new0(SysvStub, 1);
if (!service)
return log_oom();
@@ -852,7 +879,8 @@ static int set_dependencies_from_rcnd(LookupPaths lp, Hashmap *all_services) {
service = hashmap_get(all_services, name);
if (!service){
-log_warning(Could not find init script for %s, name);
+log_debug(Ignoring %s symlink in %s, not generating %s.,
+ de-d_name, rcnd_table[i].path, name);
continue;
}
diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py
index 5098519..89df72a 100644
--- a/test/sysv-generator-test.py
+++ b/test/sysv-generator-test.py
@@ -367,6 +367,18 @@ class SysvGeneratorTest(unittest.TestCase):
self.assert_enabled('foo.bak.service', [])
self.assert_enabled('foo.old.service', [])
+def test_existing_native_unit(self):
+'''existing native unit'''
+
+with open(os.path.join(self.unit_dir, 'foo.service'), 'w') as f:
+f.write('[Unit]\n')
+
+self.add_sysv('foo.sh', {'Provides': 'foo bar'}, enable=True)
+err, results = self.run_generator()
+self.assertEqual(list(results), [])
+# no enablement or alias links
+self.assertEqual(os.listdir(self.out_dir), [])
+
if __name__ == '__main__':
unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))
--
2.1.4
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH 2/2] localed: add LANGUAGE= fallback when LANG= is specified
Zbigniew Jędrzejewski-Szmek [2015-01-31 20:20 +0100]: I think the implementation is fine, since it is rather trivial, but I'm less certain about the implications of setting LANGUAGE in addtion to LANG. That's totally fine -- it's precisely what $LANGUAGE is meant for. One must be aware that it is a GNU extension, i. e. software like thunderbird won't respect it and thus just use $LANG (in which case you are no worse than now). But pretty much all native Linux sofware does and thus will behave as intended. We've used this schema in Ubuntu since around 2005 [1]. Now, I wish such a fallback list would be in upstream glibc so that OSes, systemd etc. wouldn't have to repeat and maintain it; but while [1] looks quite complex, it hasn't really changed in years. If this patch goes in, I'm happy to augment the list according to [1] and then move our language-selector to use that instead, to reduce duplication. Martin [1] http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/vivid/language-selector/vivid/view/head:/data/languagelist -- 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] Ensure that systemd is the init called after using bootchart
Ça va Sebastien, Sebastien Bacher [2015-02-02 16:14 +0100]: When booting with systemd-bootchart, default to call the systemd binary rather than the init binary on disk, which might be another init system. Collecting data doesn't work out of systemd. I'm not a native speaker either, but out of systemd sounds strange. only works with booting systemd.? -#define DEFAULT_INIT /sbin/init +#ifdef HAVE_SPLIT_USR +#define DEFAULT_INIT /lib/systemd/systemd +#else +#define DEFAULT_INIT /usr/lib/systemd/systemd +#endif Since ./configure allows you to specify paths independent from --enable-split-usr, IMHO it is more robust to do #define DEFAULT_INIT ROOTLIBDIR /systemd/systemd The idea sounds sane in general, though. Specifying init=/usr/lib/systemd/systemd-bootchart is pretty explicit about which init system one wants. Merci, 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] cryptsetup: Do not warn If the key is /dev/*random
Hey Cristian, Cristian Rodríguez [2015-02-02 12:06 -0300]: Using /dev/urandom as a key is valid for swap, do not warn if this devices are world readable. Thanks! Simple enough, applied. 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] cryptsetup: Do not warn If the key is /dev/*random
Lennart Poettering [2015-02-02 16:42 +0100]: I'd prefer if we'd change the check instead to only apply to S_ISREG() files. This way we wouldn't have to list all RNG device nodes. Ah, indeed. Sorry for prematurely pushing it then, I cleaned it up in http://cgit.freedesktop.org/systemd/systemd/commit/?id=3f4d56a0 This will cover regular files and symlinks, and ignore pipes and devices. 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] Avoid reloading services when shutting down
Lennart Poettering [2015-02-03 17:29 +0100]: Hmm, why precisely does this stall for 90s? The current transaction has final.target and all other jobs which need to be shut down. One of these now trigger systemctl reload postfix.service, but that reload isn't going to actually run in the same transaction but in the next one. OTOH systemctl reload waits for the reloading to finish, thus we have a deadlock. Isn't this a case where people should just use --no-block? Kind of. Not using this is the right thing while the machine is running, so that the reload is actually done after calling systemctl reload, and you can go on using postfix or whatever. --no-block should help during shutdown, or early boot (same principal bug, but slightly different patch, see http://bugs.debian.org/624599). So every time you call reload you'd have to check whether or not you are in early boot/during shutdown, or in the running system, and conditionally use --no-block. However, as such scripts should never call systemctl directly, but service foo reload (to work with other init systems or chroot), it would be also possible to move that check there, and conditionally add --no-block. It would just be another thing that every distro has to re-discover :-) Thanks, 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
[systemd-devel] [PATCH] Avoid reloading services when shutting down
Hey all,
I'm currently reviewing our Debian patches for systemd, and came
across this one which sounds important for other distributions, too.
This was reported and fixed two years ago in
https://bugs.debian.org/635777 which has all the details and logs, but
the summary is:
Distributions have quite a lot of run scripts in this .d/ directory
if stuff happens; e. g. the ISC DHCP client has
/etc/dhcp/dhclient-{enter,exit}-hooks.d/, Debian's ifupdown has
/etc/network/if-{up,down}.d/, and of course init.d scripts themselves
also occasionally call service foo reload and similar. It can happen
that when requesting a shutdown, a script of the above kind reloads or
restarts another service. In this case, postfix wants to reload itself
after a network interface goes up or down; during runtime that works
fine, but if it happens during shutdown, that systemctl reload
postfix will cause the entire shutdown to stall for 90s (the default
timeout).
Michael Stapelberg wrote the attached patch which fixes this by
disallowing unit reloads/restarts when final.target is queued. This is
admittedly not very elegant as it hardcodes the final.target name
(and also, for upstream the comment should be adjusted of course), but
it does fix the issue. I can still reproduce the problem with 218 in a
VM.
Is this something which we can fix upstream in a more elegant manner?
Thanks,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From: Tollef Fog Heen tfh...@err.no
Date: Tue, 16 Oct 2012 18:39:27 +0200
Subject: Avoid reloading services when shutting down
Doing so won't work and makes no sense. Thanks to Michael Stapelberg
for the patch. Closes: #624599.
---
src/core/manager.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/src/core/manager.c b/src/core/manager.c
index 6382400..acef626 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1147,6 +1147,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool override, sd_bus_error *e, Job **_ret) {
int r;
Transaction *tr;
+Job *j;
+Iterator i;
assert(m);
assert(type _JOB_TYPE_MAX);
@@ -1156,6 +1158,29 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove
if (mode == JOB_ISOLATE type != JOB_START)
return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, Isolate is only valid for start.);
+if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START || type == JOB_RESTART || type == JOB_TRY_RESTART) {
+HASHMAP_FOREACH(j, m-jobs, i) {
+assert(j-installed);
+
+/* If final.target is queued (happens on poweroff, reboot and
+ * halt), we will not accept new reload jobs. They would not be
+ * executed ever anyways (since the shutdown comes first), but
+ * they block the shutdown process: when systemd tries to stop
+ * a unit such as ifup@eth0.service, that unit might invoke a
+ * systemctl reload command, which blockingly waits (but only
+ * gets executed after all other queued units for the shutdown
+ * have been executed).
+ *
+ * See http://bugs.debian.org/624599 and
+ * http://bugs.debian.org/635777 */
+if (strcmp(j-unit-id, final.target) == 0) {
+log_debug(final.target is queued, ignoring %s request for unit %s, job_type_to_string(type), unit-id);
+sd_bus_error_setf(e, BUS_ERROR_SHUTTING_DOWN, final.target is queued, ignoring %s request for unit %s, job_type_to_string(type), unit-id);
+return -EINVAL;
+}
+}
+}
+
if (mode == JOB_ISOLATE !unit-allow_isolate)
return sd_bus_error_setf(e, BUS_ERROR_NO_ISOLATION, Operation refused, unit may not be isolated.);
signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Avoid reloading services when shutting down
Lennart Poettering [2015-02-03 18:10 +0100]: I am very strongly against adding hacky work-arounds like this to PID Yeah, indeed. This is why I asked for a more elegant approach, and indeed the --no-block or --job-mode=ignore-dependencies sound like slightly better approaches to this. I'll test these more thoroughly tomorrow, thanks for pointing out! 1. The deadlocks are deadlocks, and implying --no-block if we are in shutdown mode is a pretty drastic hack I think that special cases one peculiar case way too much. Right, the problem is of course more generic. Any set of jobs (i. e. a transaction) which causes (maybe through some indirection) one of its job members to get restarted/reloaded will suffer from this deadlock problem, AFAIUI. Booting and shutdown are therefore mostly affected by this as pretty much every other time there the pending transactions are only very small. My recommendation would be to stick this into your /usr/bin/service compat glue. Use the state string systemctl is-system-running outputs to check if you are shutting down. If so, add --no-block to the params you pass to systemctl. That actually sounds like just what's needed here. is-system-running will also neatly cover the corresponding case on bootup. Another option might be to pass --job-mode=ignore-dependencies instead of --no-block, which was created for usecases like this, even though it is frickin' ugly... For reload that should be fairly okay, as reload will quickly fail if the unit isn't actually running, and if it is running its dependencies already ought to be satisfied. I'll look into both, thanks for the hints! 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] sysv-generator: Skip init scripts for existing native services
Hey, Lennart Poettering [2015-02-04 13:42 +0100]: Well, but their enablement status so far is not ignored. i.e. if you drop in a unit file, as well as a sysv script, and the latter is enabled, but the former not, then systemd currently reads that so that the sysv one is overriden by the native one, and the native one is considered enabled. With this change you alter that behaviour. Is that really desired? Since it's rather confusing what happens in this case, we made systemctl sync the status to update-rc.d (the chkconfig equivalent in Debian) on enable/disable: http://anonscm.debian.org/cgit/pkg-systemd/systemd.git/tree/debian/patches/systemctl-don-t-skip-native-units-when-enabling-disa.patch That of course doesn't change the behaviour with manual rc?.d/ symlinks. But if you have a native unit, I think it's rather unexpected if you disable it with systemctl, enable it in sysv, but still get it started. It seems to me that systemctl disable should win here -- after all, that's closer to the running systemd than the rc.d links. So in that regard it would be an intended behaviour change indeed. But either way this is a corner case for sure. I just wouldn't like to carry this patch forever as it's relatively unimportant. Maybe Jon can chime in about his intentions with this? Thanks, 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] Deadlocks with reloading jobs which are part of current transaction [was: [PATCH] Avoid reloading services when shutting down]
Lennart Poettering [2015-02-04 13:27 +0100]: On Wed, 04.02.15 08:56, Martin Pitt (martin.p...@ubuntu.com) wrote: Lennart Poettering [2015-02-03 21:40 +0100]: It's really about synchronous waiting on jobs. If you synchronously wait for completion of jobs that are ordered against the job your are part of yourself, then things will deadlock. Indeed. The problem is that if you reload e. g. postfix from a DHCP or network up/down hook, such a script doesn't have the slightest idea whether it was run because the network changed at runtime (i. e. udev event or the user just selected a new network) or whether it happens as part of a systemd transaction (boot/shutdown). In the former case you do want to block, in the latter case you mustn't. I don't see why you'd want to block in either case. Networking is asynchronous anyway, there's really no point in blocking here. What does this give you? So far, if a hook or shell script calls e. g. service foo restart, it can count on the service actually being reloaded after it finishes, and thus you can then interact with it immediately. That's not important for many scripts, but we can't just always make service foo reload async by using --no-block, as that would break compatibility with scripts which do rely on that behaviour. So we do need to conditionalize it to avoid the deadlocks under systemd. Modern code, that talks directly to systemctl, that uses hook scripts, should always use --no-block for these cases. Yeah, for sure. We just don't have that luxury easily, as first these scripts don't call systemctl but service (for the Debianists: or invoke-rc.d, but same difference), it will take some time to find all these occurrences, and people will hate us for having to add stuff like if (running under systemd) systemctl --no-block reload foo else service foo reload as that special case appears rather pointless for someone who hasn't dealt with the details of systemd job transactions. That's not to say that it shouldn't happen (perhaps adding a --no-block option to service), but that takes time. I mean, you must have some code in Debian that forwards old sysv restart/reload requests to systemctl. That's probably going to be in /usr/sbin/service and maybe a few other places. Why can't you just add the --no-block logic there, conditionalized to shutdown/reboot? Well, we can; it's just as much an unreliable hack as the two existing patches :-) and I was wondering if we can't solve this in a more generic/distro agnostic way. - Don't enqueue a reload if the service to be reloaded isn't running. E. g. postfix.service inactive/dead in https://bugs.debian.org/635777 or smbd.service start/waiting in https://launchpad.net/bugs/1417010. This would completely avoid the deadlock in most situations already, and doesn't change the semantics for working use cases either, so this should even be applicable for upstream? No, this would open up the door for races. The guarantee we give around blocking operations, is that by the time they return the operation or an equivalent has been executed. More specifically, if you start a service, and it is in starting, and then issue a reload or restart, and it returns you *know* that the configuration that was on disk at the time you issued the reload or restart -- or a newer one -- is in place. If you'd suppress the reload/restart in this case, then you will not get that guarantee, because the configuration ultimately loaded might be the one from the time the daemon was first put into starting mode at. Hm, I don't quite understand this. If you reload a service which isn't currently running, systemctl will fail. It's just currently going to enqueue the reload request, and executing the job will fail due to the not active check in unit_reload(). The problem with that is just that systemctl doesn't fail immediately with the unit is not active, but blocks on the queued request. So if you don't have pending jobs, the net result is the same, but with pending jobs you can easily get a deadlock. Or in other words: if a script does this: # sed -i -e 's/something/somethingelse/' /etc/mydaemon.conf # systemctl restart mydaemon.service Then we *must* give the guarantee that when the second command returns the change just made to /etc/mydaemon.conf is in place, and not a previous config loaded. Yes, that's fine. I only said reload above, as that's what DHCP/network scripts usually do. (Now, this example is about restarts, but reload is pretty much the same reload on an inactive unit will fail, restart will start it. Also: *why* for heaven's sake? Just add --no-block when you are running from these contexts, and all is good. I really don't get why you don't want to do that. Well, I do, but special-casing that on boot/shutdown isn't enough (you can have other bigger transactions which lock up); and it's rather expensive to do
[systemd-devel] [PATCH v2] sysv-generator: Skip init scripts for existing native services
Hello all,
this update now uses the logic from src/shared/install.h, and also
documents the rationale better: The primary reason is to actually
respect a disabled native unit instead of re-enabling it through the
backdoor via a corresponding enabled sysv init script.
Thanks for considering,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 1629649e70bca3c7a7f5efec0211cb0c7dd14a58 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Wed, 2 Jul 2014 22:00:00 +0200
Subject: [PATCH] sysv-generator: Skip init scripts for existing native
services
This avoids taking the SysV init script enablement state into account if we
have native units. Otherwise systemctl disable on native unit would not
be respected in the presence of an enabled SysV script.
Also, there's no need to do all the parsing and creation of service files if we
already have a native systemd unit for the processed SysV init script.
---
src/sysv-generator/sysv-generator.c | 8 +++-
test/sysv-generator-test.py | 12
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 673f04d..6e39b44 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -768,6 +768,11 @@ static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
if (!fpath)
return log_oom();
+if (unit_file_get_state(UNIT_FILE_SYSTEM, NULL, name) = 0) {
+log_debug(Native unit for %s already exists, skipping, name);
+continue;
+}
+
service = new0(SysvStub, 1);
if (!service)
return log_oom();
@@ -852,7 +857,8 @@ static int set_dependencies_from_rcnd(LookupPaths lp, Hashmap *all_services) {
service = hashmap_get(all_services, name);
if (!service){
-log_warning(Could not find init script for %s, name);
+log_debug(Ignoring %s symlink in %s, not generating %s.,
+ de-d_name, rcnd_table[i].path, name);
continue;
}
diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py
index 5098519..09f5c01 100644
--- a/test/sysv-generator-test.py
+++ b/test/sysv-generator-test.py
@@ -367,6 +367,18 @@ class SysvGeneratorTest(unittest.TestCase):
self.assert_enabled('foo.bak.service', [])
self.assert_enabled('foo.old.service', [])
+def test_existing_native_unit(self):
+'''existing native unit'''
+
+with open(os.path.join(self.unit_dir, 'foo.service'), 'w') as f:
+f.write('[Unit]\n')
+
+self.add_sysv('foo.sh', {'Provides': 'foo bar'}, enable=True)
+err, results = self.run_generator()
+self.assertEqual(list(results), [])
+# no enablement or alias links, as native unit is disabled
+self.assertEqual(os.listdir(self.out_dir), [])
+
if __name__ == '__main__':
unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [Patch 0/6] Small adjustments to make QEMU tests work under Debian/Ubuntu
Hey Harald, Harald Hoyer [2015-02-03 10:56 +0100]: Looks good :) Pushed with 3 additional patches. Thanks for the review! Martin, can you test, if this still works for you? You might need a newer util-linux for sfdisk. Indeed, util-linux 2.25 fails with this. I'll update it to 2.26, but in the meantime I can just locally revert this. 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] language fallback: it_CH (Italian, Swiss) - it_IT (Italian, Italy)
Hey Daniele, Daniele Medri [2015-02-08 8:57 +0100]: +it_CH it_CH:it_IT Thanks! Applied with moving it up a bit to stay in alphabetical order. 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] cdrom_id: unmount media on --eject-media if media mounted
Hey Robert, thanks for working on this! Robert Milasan [2015-01-15 15:24 +0100]: +err = execl(/bin/umount, /bin/umount, node, NULL); If this succeeds, umount will completely take over the process, and cdrom_id is gone. This also isn't sufficient if there are multiple partitions, which even on CDs/DVDs isn't that uncommon (especially for operating systems). So I think this should instead find all mount points that belong to target, and call umount2(target, MNT_DETACH) on them. The actual eject program does that. ... which leads me to the question: why don't we just call the actual eject program? Just to avoid that dependency? Thanks, 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
[systemd-devel] [PATCH] sysv-generator: Handle .sh suffixes when translating Provides:
Hey all,
the recent fix for sysv-generator's Provides: handling [1] caused, or
rather uncovered, another bug which now creates symlinks to itself
foo.service - foo.service for any /etc/init.d/foo.sh.
The generator would output an error message like
Failed to create unit file path.../foo.service: File exists
instead of creating the actual foo.service file. I. e. this completely
breaks translating init scripts with .sh.
Fix with corresponding test case attached. This is a test case for the
test suite I sent in my previous mail; that might still need some
masssaging, so if you are ok with this fix, I'll commit that without
the test case, and add the test case to the suite separately.
Martin
[1] http://cgit.freedesktop.org/systemd/systemd/commit/?id=b7e7184634d5
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From cbd7f20422f044ba192d11afe2d3a29bcf7a5f69 Mon Sep 17 00:00:00 2001
From: Martin Pitt martin.p...@ubuntu.com
Date: Tue, 20 Jan 2015 16:41:31 +0100
Subject: [PATCH 2/2] sysv-generator: Handle .sh suffixes when translating
Provides:
When deciding whether the provided name equals the file name in
sysv_translate_facility(), also consider them equal if the file name has a
.sh suffix.
This was uncovered by commit b7e7184 which then created a symlink
name.service to itself for .sh suffixed init.d scripts.
For additional robustness, refuse to create symlinks to itself in add_alias().
---
src/sysv-generator/sysv-generator.c | 16 +++-
test/sysv-generator-test.py | 37 +
2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 4774981..6334fd3 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -119,6 +119,11 @@ static int add_alias(const char *service, const char *alias) {
assert(service);
assert(alias);
+if (streq(service, alias)) {
+log_error(Ignoring creation of an alias %s for itself, service);
+return 0;
+}
+
link = strjoin(arg_dest, /, alias, NULL);
if (!link)
return log_oom();
@@ -263,6 +268,7 @@ static int sysv_translate_facility(const char *name, const char *filename, char
unsigned i;
char *r;
const char *n;
+_cleanup_free_ char *filename_no_sh = NULL;
assert(name);
assert(_r);
@@ -284,6 +290,14 @@ static int sysv_translate_facility(const char *name, const char *filename, char
goto finish;
}
+/* strip .sh suffix from file name for comparison */
+filename_no_sh = strdup(filename);
+if (!filename_no_sh)
+return -ENOMEM;
+if (endswith(filename, .sh))
+filename_no_sh[strlen(filename)-3] = '\0';
+log_debug(sysv_translate_facility: translating %s fname %s no_sh %s, name, filename, filename_no_sh);
+
/* If we don't know this name, fallback heuristics to figure
* out whether something is a target or a service alias. */
@@ -293,7 +307,7 @@ static int sysv_translate_facility(const char *name, const char *filename, char
/* Facilities starting with $ are most likely targets */
r = unit_name_build(n, NULL, .target);
-} else if (filename streq(name, filename))
+} else if (filename streq(name, filename_no_sh))
/* Names equaling the file name of the services are redundant */
return 0;
else
diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py
index e281a7f..53d729a 100644
--- a/test/sysv-generator-test.py
+++ b/test/sysv-generator-test.py
@@ -172,6 +172,43 @@ class SysvGeneratorTest(unittest.TestCase):
self.assertEqual(os.readlink(os.path.join(self.out_dir, f)),
'foo.service')
+def test_sh_suffix(self):
+'''init.d script with .sh suffix'''
+
+self.add_sysv('foo.sh', {}, enable=True)
+err, results = self.run_generator()
+s = results['foo.service']
+
+self.assertEqual(s.sections(), ['Unit', 'Service'])
+# should not have a .sh
+self.assertEqual(s.get('Unit', 'Description'), 'LSB: test foo service')
+
+# calls correct script with .sh
+init_script = os.path.join(self.init_d_dir, 'foo.sh')
+self.assertEqual(s.get('Service', 'ExecStart'),
+ '%s start' % init_script)
+self.assertEqual(s.get('Service', 'ExecStop'),
+ '%s stop' % init_script)
+
+# should be enabled
+target = os.readlink(os.path.join(
+self.out_dir, 'runlevel2.target.wants', 'foo.service'))
+self.assertTrue(os.path.exists(target))
+self.assertEqual(os.path.basename(target
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
Zbigniew Jędrzejewski-Szmek [2015-01-20 16:48 +0100]: Maybe we could do this check in configure.ac/Makefile.am (add the test to the list conditinally)? Yes, that's a good idea. We already test for python presence and extract the version, so we shouldn't duplicate the tests here and have an extra wrapper. Unfortunately automake's TESTS only accepts single arguments, i. e. scripts without any arguments. But we need to call $(PYTHON) test/x.py thus we need an argument. An alternative would be to add a new TESTS_PYTHON = test/sysv-generator-test.py test/rule-syntax-check.py (and perhaps more in the future) and integrate that into Makefile.am. That would make Makefile.am more complex, but avoid the extra wrappers. WDYT? Thanks, 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
