Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
On Fri, Nov 28, 2014 at 09:27:43PM +0100, Lennart Poettering wrote: > On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote: > > > On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote: > > > This adds auto detection for iSCSI and some FCoE drivers and treats > > > mounts to file-systems on those devices as remote-fs. > > > > > > Signed-off-by: Chris Leech > > No need for this. > > > > I now pushed patches 1-4 with some small changes here and there. > > Since libmount is not optional, I removed if from the version string. > > > > This patch I didn't push: this seems like something that would be better > > done through udev rules, by setting some tags. Then systemd could simply > > check for the presence of a tag on the device without having any > > special knowledge about iscsi and firends. > > Honestly, I am really not sure I like this patch. > > One one hand we now have a hard dep on libmount. Which I figure is > mostly OK. However, what I find really weird about this is that even > though libmount is supposed to abstract access to the "utab" away, it > doesn't sufficiently: we still hardcode the utab path now so that we > can watch it. I mean, either we use an abstracted interface or we > don't. THis really smells to me as either libmount should provide some > form of inotify iface to utab, or we should parse utab directly. If > libmount is the only and official API for utab, then we should > that. If the utab file however is API too, then we can well go ahead > and parse it directly. I'd like to keep the utab file private. The whole libmount API is abstraction and completely hides the difference between original /etc/mtab and new /run/mount/utab. The original Chris' purpose for the patches has been _netdev, it means to improve detection of dependence between filesystem and network. I still believe that the right way is to check for network block devices than rely on crazy _netdev userspace mount option. Wouldn't be enough to use Chris' iSCSI and FCoE auto detection? Or we can add udev rule to have NET_DEVBLK tag in udevdb, or maybe we can ask kernel developers to add /sys/block/sdb/network (as we already have "removable" in /sys). Chris, why do you want both ways (utab and the auto detection)? IMHO it would be really nice to completely kill _netdev in systemd universe ;-) It's legacy from shell init scripts. Anyway, if you still want to read userspace mount options than I can add something like: mnt_inotify_mtab_add_watch() mnt_inotify_mtab_rm_watch() mnt_inotify_mtab_changed() to hide all the paths and private mtab/utab stuff. > THe new code looks also buggy to me. For example, am I missing > something or is nobody creating /run/mount? I mean, we need to make > sure that it exists before we can install an inotify watch on it. yep, it should be hidden within libmount > Also, it leaves the cescape() calls in for what we read from libmount, > which makes a lot of alarm bells ring in my head: doesn't libmount do > that on its own? sure, libmount encodes/decodes all when read/write the files. Karel -- Karel Zak http://karelzak.blogspot.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
Hello Could you take a look at my patch? Regards Przemyslaw Kedzierski ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors
Lennart Poettering writes: > On Tue, 25.11.14 10:01, David Herrmann (dh.herrm...@gmail.com) wrote: > >> I explicitly ignore errors from verify_xkb_rmlvo() and proceed. >> libxkbcommon is still not 100% compatible to libxkb (and doesn't >> intend to be that, I guess). As we write X11 configs here, I just >> continue with a warning. >> >> If you call bus_error_setfv(), then sd-bus will return a method-error >> to the caller. However, you also send a method-return in >> method_set_x11_keyboard(). You thus end up with 2 calls. >> >> I'm not sure how to solve this. Furthermore, libxkbcommon can print a >> lot of informational warnings, and we shouldn't use just the last one. >> One idea would be to copy the same logic into localectl. You could >> also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get >> more information there. >> Yes, this would mean compiling the keymap twice, but it's for the sake >> of debugging output so I think it's fine. > > What precisely are the error messages libxkbcommon prints? Are they > really material to pass to the client? I mean, are they really about > some invalid data the client passed or just about something broken in > the way the system is set up? If it's the latter than I figure we > should return just a simple error to the client, and pass the full > logs to the logging stream... > > Lennart Thinking about it again, I don't think that it's a good idea to pass those errors to the client... A few examples: # localectl set-x11-keymap QQ Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: Couldn't find file "symbols/QQ" in include paths Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 1 include paths searched: Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: /usr/share/X11/xkb Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: Abandoning symbols file "(unnamed)" Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: Failed to compile xkb_symbols Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: Failed to compile keymap Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: Cannot compile XKB keymap for new x11 keyboard layout ('' / 'QQ' / '' / ''): Invalid argument # localectl set-x11-keymap us pc105 QQ Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: Couldn't process include statement for 'us(QQ)' Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: Abandoning symbols file "(unnamed)" Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: Failed to compile xkb_symbols Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: Failed to compile keymap Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: Cannot compile XKB keymap for new x11 keyboard layout ('pc105' / 'us' / 'QQ' / ''): Invalid argument Those errors are just stupid. Maybe they would make sense if there was also libxkbcommon code in front of me... But from the user's perspective, they say nothing useful. I added the prefix so the errors at least have some context. My original patch would say something like "No such layout 'QQ'" in the first case and "No variant 'QQ' for layout 'us'" in the second. So, is it worth logging this information? Any other ideas? Cheers, -- Jan Synacek Software Engineer, Red Hat signature.asc Description: PGP signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [WIP PATCH] Do not realize and migrate cgroups multiple times
Hello all, In my efforts to make user LXC containers work I noticed that under a "real" desktop (not just nspawn with VT login or ssh logins) my carefully set up cgroups in the non-systemd controllers get reverted. I. e. I put the session leader (and all other pids) of logind sessions (/user.slice/user-1000.slice/session-XX.scope) into all cgroup controllers, but a second after they are all back in the / cgroups (or sometimes in /user.slice). After some days of debugging (during which I learned a lot about the innards of systemd :-) I finally found out why: During unit cgroup initialization (unit_realize_cgroup), sibling cgroups are queued instead of initialized immediately. unit_create_cgroups() makes an attempt to avoid initializing and migrating a cgroup more than once: path = unit_default_cgroup_path(u); [...] r = hashmap_put(u->manager->cgroup_unit, path, u); if (r < 0) { log_error(r == -EEXIST ? "cgroup %s exists already: %s" : "hashmap_put failed for %s: %s", path, strerror(-r)); return r; } But this doesn't work: "path" always gets allocated freshly in that function, so the pointer is never in the hashmap and the -EEXISTS never actually hits. This causes -.slice to be initialized and recursively migrated a gazillion times, which explains the random punting of sub-cgroup PIDs back to /. I fixed this with an explicit hashmap_get() call, which compares values instead of pointers. I also added some debugging to that function: log_debug("unit_create_cgroups %s: path=%s realized %i hashmap %p", u->id, path, u->cgroup_realized, hashmap_get(u->manager->cgroup_unit, path)); (right before the hashmap_put() above), which illustrates the problem: systemd[1]: Starting Root Slice. systemd[1]: unit_create_cgroups -.slice: path= realized 0 hashmap (nil) systemd[1]: Created slice Root Slice. [...] pid1 systemd[1]: unit_create_cgroups session-c1.scope: path=/user.slice/user-1000.slice/session-c1.scope realized 0 hashmap (nil) systemd[1]: Started Session c1 of user martin. [... later on when most things have been started ...] systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850 systemd[1]: unit_create_cgroups -.slice: cgroup exists already systemd[1]: Failed to realize cgroups for queued unit user.slice: File exists systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850 systemd[1]: unit_create_cgroups -.slice: cgroup exists already systemd[1]: Failed to realize cgroups for queued unit grub-common.slice: File exists systemd[1]: unit_create_cgroups -.slice: path= realized 1 hashmap 0x7f0e14aa4850 systemd[1]: unit_create_cgroups -.slice: cgroup exists already systemd[1]: Failed to realize cgroups for queued unit networking.slice: File exists ... and so on, basically once for each .service. Initializing -.slice so many times is certainly an unintended effect of the peer cgroup creation. Thus the patch fixes the multiple initialization/creation, but a proper fix should also quiesce these error messages. The condition could be checked explicitly, i. e. we skip the "Failed to realize..." error for EEXISTS, or we generally tone this down to log_debug. I'm open to suggestions. And of course the log_debug should be removed; it's nice to illustrate the problem, but doesn't really belong into production code. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) From fd2f8a444c9c644a39dd3b619934e8768e03c2a3 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 1 Dec 2014 10:50:06 +0100 Subject: [PATCH] Do not realize and migrate cgroups multiple times unit_create_cgroups() tries to check if a cgroup already exists. But has the destination path is always allocated dynamically as a new string, that pointer will never already be in the hashmap, thus hashmap_put() will never actually fail with EEXISTS. Thus check for the existance of the cgroup path explicitly. Before this, "-.slice" got initialized and its child PIDs migrated many times through queuing the realization of sibling units; thiss caused any cgroup controller layout from sub-cgroups to be reverted and their pids moved back to the root cgroup in all controllers (except systemd). --- src/core/cgroup.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 8820bee..3d36080 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -614,6 +614,13 @@ static int unit_create_cgroups(Unit *u, CGroupControllerMask mask) { if (!path) return log_oom(); +log_debug("unit_create_cgroups %s: path=%s realized %i hashmap %p", u->id, path, u->cgroup_realized, hashmap_get(u->manager->cgroup_unit, path)); + +if (hashmap_get(u->manager->cgroup_unit, path)) { +log_error("unit_create_cgroups %s: cgroup %s exists already", u->id, u->cgroup_path); +
Re: [systemd-devel] [systemd-commits] src/cryptsetup
Hi, On 01/12/2014 01:12, Lennart Poettering wrote : On Mon, 24.11.14 19:25, Quentin Lefebvre (qlefebvre_...@yahoo.com) wrote: Le 24/11/2014 19:17, Zbigniew Jędrzejewski-Szmek a écrit : On Mon, Nov 24, 2014 at 07:03:27PM +0100, Quentin Lefebvre wrote: Le 24/11/2014 19:01, Zbigniew Jędrzejewski-Szmek a écrit : On Mon, Nov 24, 2014 at 06:44:25PM +0100, Quentin Lefebvre wrote: Hi, I tested your patch and actually it doesn't solve the bug. For example, if "hash=sha512" is provided in /etc/crypttab, the first > if (!streq(arg_hash, "plain")) is true, and the +} else if (!key_file) is not reached. This is be design. My patch is quite different from your patch, which I tried to make clear in the description. If you specify hash=sha512, then you get hash=sha512. Yes, and this is the problem. cryptsetup ignores the hash, so that we should obtain hash=NULL for it to work. Systemd is not going to work around a bug in a different package. Specifying a hash in the configuration if you don't want a hash is an error, please just fix it there. I understand your point. Still you have a cryptsetup tool in systemd, so I would expect it behaves as the "true" cryptsetup program. The problem here is compatibility, you do something with cryptsetup and then your system fails to boot because of a different behaviour of systemd. Note that compatibiltiy is really a problem with crypttab anyway, as there were multiple implementations of the code that reads it around: at least the one from DEbian and the one from Fedora, both of which had very different feature sets and even different syntaxes. With systemd's crypttab support we try to provide a decent amount of compatibility with both, to the level that it makes sense. Since we cannot reach 100% compat anyway, this explicitly means no bug-for-bug compatibility however. Hence I really think we should do the correct thing, rather than the traditional thing here, given that this is not the most common usecase anyway, Hope that makes sense, OK. I also read Zbigniew's answer on the bug report. I understand your point, which makes sense. I guess we'll have to document these different behaviors in Debian, so that users don't get too confused... But anyway, plain dm-crypt devices, even if they're not the most common usecase, should be handled in the long-term, as it is still a useful, and quite different setup compared to LUKS devices. Thanks for your replies and great work! Best regards, Quentin ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] x86: defconfig: Enable CONFIG_FHANDLE
On Mon, 01.12.14 14:54, Dave Chinner (da...@fromorbit.com) wrote: > On Mon, Dec 01, 2014 at 02:03:43AM +0100, Lennart Poettering wrote: > > On Mon, 01.12.14 01:41, Richard Weinberger (rich...@nod.at) wrote: > > > > > CC'ing systemd folks. > > > > > > Lennart, can you please explain why you need CONFIG_FHANDLE for systemd? > > > Maybe I'm reading the source horrible wrong. > > > > For two usecases: > > > > a) Being able to detect if something is a mount point. The traditional > >way to do this is by stat()ing the dir in question and its parent > >and comparing st_dev. That logic is not able to detect bind mounts > >however, if destination and the place the mount is at are actually > >on the same file system... Thus we check the mount id too, if we > >can get our hands on it. > > So what you really want in the mount id in st_buf.st_dev, not the > underlying device number. i.e. fstatat(dirfd, path, buf, > AT_MOUNTID)? Well, I am not a fan of overloading things, and there might be reasons why one would want to know both the mount id and the device id at the same time with one atomic call, but ultimately I don't really care, and fstatat(AT_MOUNT_ID) would certainly be at least as useful as name_to_handle_at() is. Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
On Mon, Dec 01, 2014 at 11:24:58AM +0100, Karel Zak wrote: > On Fri, Nov 28, 2014 at 09:27:43PM +0100, Lennart Poettering wrote: > > On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) > > wrote: > > > > > On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote: > > > > This adds auto detection for iSCSI and some FCoE drivers and treats > > > > mounts to file-systems on those devices as remote-fs. > > > > > > > > Signed-off-by: Chris Leech > > > No need for this. > > > > > > I now pushed patches 1-4 with some small changes here and there. > > > Since libmount is not optional, I removed if from the version string. > > > > > > This patch I didn't push: this seems like something that would be better > > > done through udev rules, by setting some tags. Then systemd could simply > > > check for the presence of a tag on the device without having any > > > special knowledge about iscsi and firends. > > > > Honestly, I am really not sure I like this patch. > > > > One one hand we now have a hard dep on libmount. Which I figure is > > mostly OK. However, what I find really weird about this is that even > > though libmount is supposed to abstract access to the "utab" away, it > > doesn't sufficiently: we still hardcode the utab path now so that we > > can watch it. I mean, either we use an abstracted interface or we > > don't. THis really smells to me as either libmount should provide some > > form of inotify iface to utab, or we should parse utab directly. If > > libmount is the only and official API for utab, then we should > > that. If the utab file however is API too, then we can well go ahead > > and parse it directly. > > I'd like to keep the utab file private. The whole libmount API is > abstraction and completely hides the difference between original > /etc/mtab and new /run/mount/utab. > > The original Chris' purpose for the patches has been _netdev, it > means to improve detection of dependence between filesystem and > network. I still believe that the right way is to check for network > block devices than rely on crazy _netdev userspace mount option. > > Wouldn't be enough to use Chris' iSCSI and FCoE auto detection? Please see previous discussion... Detecting network might not be trivial if the devices are layered and there's a network-requiring device somewhere lower in the stack. Using _netdev is supposed to be an easy mechanism which admins can use in case whatever other schemes we have in place don't work. > Or we can add udev rule to have NET_DEVBLK tag in udevdb, or maybe we > can ask kernel developers to add /sys/block/sdb/network (as we already > have "removable" in /sys). > > Chris, why do you want both ways (utab and the auto detection)? > > IMHO it would be really nice to completely kill _netdev in systemd > universe ;-) It's legacy from shell init scripts. > > > Anyway, if you still want to read userspace mount options than I can > add something like: > > mnt_inotify_mtab_add_watch() > mnt_inotify_mtab_rm_watch() > mnt_inotify_mtab_changed() > > to hide all the paths and private mtab/utab stuff. Maybe something more like what journal client code does here: mnt_mtab_get_fd() -> epoll fd mnt_mtab_get_events() -> EPOLLIN|EPOLLPRI mnt_mtab_process() -> information what changed and then mnt_mtab_wait() can be constructed on top of this, but systemd wouldn't be using it. > > THe new code looks also buggy to me. For example, am I missing > > something or is nobody creating /run/mount? I mean, we need to make > > sure that it exists before we can install an inotify watch on it. > > yep, it should be hidden within libmount > > > Also, it leaves the cescape() calls in for what we read from libmount, > > which makes a lot of alarm bells ring in my head: doesn't libmount do > > that on its own? > > sure, libmount encodes/decodes all when read/write the files. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] mate desktop user service file
Hi On Sat, Nov 29, 2014 at 10:48 AM, arnaud gaboury wrote: > I only use some settings from mate desktop (clipboard, appearance...) > thus looking for a service file to start mate-settings-daemon. > > > /home/gabx/.config/systemd/user/mate-settings-daemon.service > - > [Unit] > Description=Mate settings daemon > > [Service] > Type=daemon > ExecStart=/usr/lib/mate-settings-daemon/mate-settings-daemon > > [Install] > WantedBy=wm.target > > > The above service leave me with an error: > > -- > gabx@hortensia ➤➤ ~ % systemctl --user status mate-settings-daemon > ● mate-settings-daemon.service - Mate settings daemon >Loaded: loaded > (/home/gabx/.config/systemd/user/mate-settings-daemon.service; > disabled) >Active: failed (Result: exit-code) since Sat 2014-11-29 10:29:09 > CET; 14min ago > Process: 26343 > ExecStart=/usr/lib/mate-settings-daemon/mate-settings-daemon > (code=exited, status=1/FAILURE) > Main PID: 26343 (code=exited, status=1/FAILURE) > > Nov 29 10:29:09 hortensia systemd[914]: Started Mate settings daemon. > Nov 29 10:29:09 hortensia mate-settings-daemon[26343]: No protocol specified > Nov 29 10:29:09 hortensia mate-settings-daemon[26343]: ** > (mate-settings-daemon:26343): WARNING **: Unable to initialize GTK+ mate-settings-daemon might expect to be run from within an X-session. These errors look like DISPLAY= isn't set, which is reasonable because systemd starts programs from a clean environment. Anyway, you really should talk to the mate developers to ask them what's needed to run it from systemd. Just putting stuff into services usually does not work. Many of the X11 legacy expects to be run from within the X11 environment, not from a clean service. Thanks David > -- > > I have tried unsuccessfully to change parts of the service file. > Running manually the command < $ > /usr/lib/mate-settings-daemon/mate-settings-daemon > works. > > N.B: display is in my environment. > --- > gabx@hortensia ➤➤ ~ % systemctl --user show-environment > DBUS_SESSION_BUS_ADDRESS=/run/user/1000/dbus/user_bus_socket > DISPLAY=:0 > -- > > > I have no idea what I do wrong. Thank you for advices. > > > -- > > google.com/+arnaudgabourygabx > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] TimeoutStopSec is ignored (regression)
Hi On Sat, Nov 29, 2014 at 12:35 PM, Ross Lagerwall wrote: > Hi, > > On recent versions of systemd, unit_kill_context doesn't set > wait_for_exit to true which means that service_enter_signal sends > SIGTERM, immediately moves into stop-sigkill and sends SIGKILL, ignoring > TimeoutStopSec and often killing processes without giving them a chance > to cleanup. > > Reverting the following change, fixes the problem: > > commit 1baccdda2e954214e0c5463d6ed8f06009b33c41 > Author: Lennart Poettering > Date: Wed Feb 5 02:22:11 2014 +0100 > > core: don't wait for non-control/non-main processes when killing > processes on the host either > > Since the current kernel cgroup notification logic is easily confused by > existing subgroups, let's do the same thing as in containers before. and > just not wait for non-control and non-main processes. > > This should be corrected as soon as we have sane cgroup notifications > from the kernel. The commit-message and the comment it adds should answer your question: The kernel cgroup API does not allow us to wait for non-control processes. That is, we still honor TimeoutStopSec and friends if we have to wait for the main-process and/or control process (in those cases, wait_for_exit is still set to true). However, if there are other processes remaining in the cgroup, we now ignore it. See the commit you mentioned for an explanation. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] rules: rerun vconsole-setup when switching from vgacon to fbcon
On Friday 07 November 2014 at 16:45:02, Lennart Poettering wrote: > On Fri, 07.11.14 17:45, Ivan Shapovalov (intelfx...@gmail.com) wrote: > > > On Thursday 06 November 2014 at 11:02:44, David Herrmann wrote: > > > Hi Ray > > > > > > On Thu, Nov 6, 2014 at 10:40 AM, David Herrmann > > > wrote: > > > > On Wed, Nov 5, 2014 at 4:11 PM, Ray Strode wrote: > > > >>> So if you have no idea how to make that rule be generated only if > > > >>> ENABLE_VCONSOLE is set by configure, then we probably should take my > > > >>> patch below. > > > >> Your patch seems far better than the options above, but I think it > > > >> needs a dracut patch to make sure the new rules file gets in the > > > >> initrd too, or it won't work. > > > > > > > > I will push the patch to systemd and let Harald know. I'm not really > > > > familiar with dracut.. > > > > > > Pushed. > > > > Isn't it ugly to re-runn systemd-vconsole-setup straight from an udev rule? > > I mean, udev has a tendency to kill long-running RUN processes, and I've > > seen > > systemd-vconsole-setup.service to take >5 seconds on some systems... > > Also, these additional systemd-vconsole-setup instances aren't supervised by > > systemd... > > Is systemd-vconsole-setup really long-running? Why would it need that > long? > > To me it appears to be quite OK to be run inside a udev rule. Well, maybe it is OK to be run from an udev rule, but it is still an inconsistency between running that binary on boot (via a unit) and running the same binary when a framebuffer console is added (directly)... -- Ivan Shapovalov / intelfx / signature.asc Description: This is a digitally signed message part. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
On Mon, Dec 01, 2014 at 02:28:33PM +0100, Zbigniew Jędrzejewski-Szmek wrote: > > Wouldn't be enough to use Chris' iSCSI and FCoE auto detection? > Please see previous discussion... Detecting network might not be trivial > if the devices are layered and there's a network-requiring device somewhere > lower in the stack. Good point. (It would be possible to analyze whole stack by slave/holders relations, but I agree it's too complex and probably too fragile.) > > Anyway, if you still want to read userspace mount options than I can > > add something like: > > > > mnt_inotify_mtab_add_watch() > > mnt_inotify_mtab_rm_watch() > > mnt_inotify_mtab_changed() > > > > to hide all the paths and private mtab/utab stuff. > Maybe something more like what journal client code does here: > > mnt_mtab_get_fd() -> epoll fd > mnt_mtab_get_events() -> EPOLLIN|EPOLLPRI > mnt_mtab_process() -> information what changed Hmm.. utab is optional and very often does not exist, and library uses rename(2) to do atomic update of the file. This is reason why Chris have used inotify for /run/mount directory (and Lennart asked for inotify API). I have doubts you can use epoll to monitor directories, epoll is about I/O monitoring. Karel -- Karel Zak http://karelzak.blogspot.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH] proc, pidns: Add highpid
On Sun, Nov 30, 2014 at 11:03 PM, Konstantin Khlebnikov wrote: > Hmm. What about per-task/thread UUID? exported via separate file: > /proc/PID/uuid > It could be created at the first access, thus this wouldn't shlowdown clone(). > Also it could be droped at execve(), so it'll describe execution > context more precisely than pid. > I'd be all for this if it weren't for two issues: 1. This thing needs to work as soon as init is started, and we can't reliably generate random numbers that early. 2. Migration / CRIU is rather fundamentally at odds with making anything universally unique, as opposed to just per-user-ns. So, given that I don't think we can actually provide a universally unique pid-like thing, I'd rather not even try. That being said, I want to rework this a little bit. I think that highpid should be restricted to the range 2^32 through 2^64 - 4096. The former prevents anyone from confusing highpid with regular pid, and the latter means that we don't need to worry about confusion between errors and valid highpids (e.g. -1 will never be a highpid). Implementing that will be only mildly annoying. --Andy > On Sat, Nov 29, 2014 at 2:05 AM, Andy Lutomirski wrote: >> Pid reuse is common, which means that it's difficult or impossible >> to read information about a pid from /proc without races. >> >> This introduces a second number associated with each (task, pidns) >> pair called highpid. Highpid is a 64-bit number, and, barring >> extremely unlikely circumstances or outright error, a (highpid, pid) >> will never be reused. >> >> With just this change, a program can open /proc/PID/status, read the >> "Highpid" field, and confirm that it has the expected value. If the >> pid has been reused, then highpid will be different. >> >> The initial implementation is straightforward: highpid is simply a >> 64-bit counter. If a high-end system can fork every 3 ns (which >> would be amazing, given that just allocating a pid requires at >> atomic operation), it would take well over 1000 years for highpid to >> wrap. >> >> For CRIU's benefit, the next highpid can be set by a privileged >> user. >> >> NB: The sysctl stuff only works on 64-bit systems. If the approach >> looks good, I'll fix that somehow. >> >> Signed-off-by: Andy Lutomirski >> --- >> >> If this goes in, there's plenty of room to add new interfaces to >> make this more useful. For example, we could add a fancier tgkill >> that adds and validates hightgid and highpid, and we might want to >> add a syscall to read one's own hightgid and highpid. These would >> be quite useful for pidfiles. >> >> David, would this be useful for kdbus? >> >> CRIU people: will this be unduly difficult to support in CRIU? >> >> If you all think this is a good idea, I'll fix the sysctl stuff and >> document it. It might also be worth adding "Hightgid" to status. >> >> fs/proc/array.c | 5 - >> include/linux/pid.h | 2 ++ >> include/linux/pid_namespace.h | 1 + >> kernel/pid.c | 19 +++ >> kernel/pid_namespace.c| 22 ++ >> 5 files changed, 44 insertions(+), 5 deletions(-) >> >> diff --git a/fs/proc/array.c b/fs/proc/array.c >> index cd3653e4f35c..f1e0e69d19f9 100644 >> --- a/fs/proc/array.c >> +++ b/fs/proc/array.c >> @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct >> pid_namespace *ns, >> int g; >> struct fdtable *fdt = NULL; >> const struct cred *cred; >> + const struct upid *upid; >> pid_t ppid, tpid; >> >> rcu_read_lock(); >> @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, >> struct pid_namespace *ns, >> if (tracer) >> tpid = task_pid_nr_ns(tracer, ns); >> } >> + upid = pid_upid_ns(pid, ns); >> cred = get_task_cred(p); >> seq_printf(m, >> "State:\t%s\n" >> "Tgid:\t%d\n" >> "Ngid:\t%d\n" >> "Pid:\t%d\n" >> + "Highpid:\t%llu\n" >> "PPid:\t%d\n" >> "TracerPid:\t%d\n" >> "Uid:\t%d\t%d\t%d\t%d\n" >> @@ -183,7 +186,7 @@ static inline void task_state(struct seq_file *m, struct >> pid_namespace *ns, >> get_task_state(p), >> task_tgid_nr_ns(p, ns), >> task_numa_group_id(p), >> - pid_nr_ns(pid, ns), >> + upid ? upid->nr : 0, upid ? upid->highnr : 0, >> ppid, tpid, >> from_kuid_munged(user_ns, cred->uid), >> from_kuid_munged(user_ns, cred->euid), >> diff --git a/include/linux/pid.h b/include/linux/pid.h >> index 23705a53abba..ece70b64d04c 100644 >> --- a/include/linux/pid.h >> +++ b/include/linux/pid.h >> @@ -51,6 +51,7 @@ struct upid { >> /* Try to keep pid_chain in the same cacheline as nr for find_vpid */ >> int nr; >> struct pid_name
Re: [systemd-devel] [RFC PATCH] proc, pidns: Add highpid
On Mon, Dec 1, 2014 at 8:39 AM, Konstantin Khlebnikov wrote: > On Mon, Dec 1, 2014 at 7:21 PM, Andy Lutomirski wrote: >> On Sun, Nov 30, 2014 at 11:03 PM, Konstantin Khlebnikov >> wrote: >>> Hmm. What about per-task/thread UUID? exported via separate file: >>> /proc/PID/uuid >>> It could be created at the first access, thus this wouldn't shlowdown >>> clone(). >>> Also it could be droped at execve(), so it'll describe execution >>> context more precisely than pid. >>> >> >> I'd be all for this if it weren't for two issues: >> >> 1. This thing needs to work as soon as init is started, and we can't >> reliably generate random numbers that early. >> >> 2. Migration / CRIU is rather fundamentally at odds with making >> anything universally unique, as opposed to just per-user-ns. >> >> So, given that I don't think we can actually provide a universally >> unique pid-like thing, I'd rather not even try. > > Well... another idea: what about pid generation counter? Of course it > should be per-pid-ns. > As I see struct upid has nice 32-bit hole next to 'nr' field. (on > 64-bit systems) > I thought about that, but it has two issues: 1. Implementing it will be pain. The pid allocation algorithm is already complicated, and it wasn't obvious to me how to accurately detect wraparound without racing against other wrapping users. 2. I don't think it will be reliable. Suppose that there are pid_max - 1 processes. One of them can repeatedly clone and exit, incrementing the generation counter each time. After 2^32 iterations, which won't take all that long, the pid generation will wrap. --Andy >> >> That being said, I want to rework this a little bit. I think that >> highpid should be restricted to the range 2^32 through 2^64 - 4096. >> The former prevents anyone from confusing highpid with regular pid, >> and the latter means that we don't need to worry about confusion >> between errors and valid highpids (e.g. -1 will never be a highpid). >> >> Implementing that will be only mildly annoying. >> >> --Andy >> >>> On Sat, Nov 29, 2014 at 2:05 AM, Andy Lutomirski >>> wrote: Pid reuse is common, which means that it's difficult or impossible to read information about a pid from /proc without races. This introduces a second number associated with each (task, pidns) pair called highpid. Highpid is a 64-bit number, and, barring extremely unlikely circumstances or outright error, a (highpid, pid) will never be reused. With just this change, a program can open /proc/PID/status, read the "Highpid" field, and confirm that it has the expected value. If the pid has been reused, then highpid will be different. The initial implementation is straightforward: highpid is simply a 64-bit counter. If a high-end system can fork every 3 ns (which would be amazing, given that just allocating a pid requires at atomic operation), it would take well over 1000 years for highpid to wrap. For CRIU's benefit, the next highpid can be set by a privileged user. NB: The sysctl stuff only works on 64-bit systems. If the approach looks good, I'll fix that somehow. Signed-off-by: Andy Lutomirski --- If this goes in, there's plenty of room to add new interfaces to make this more useful. For example, we could add a fancier tgkill that adds and validates hightgid and highpid, and we might want to add a syscall to read one's own hightgid and highpid. These would be quite useful for pidfiles. David, would this be useful for kdbus? CRIU people: will this be unduly difficult to support in CRIU? If you all think this is a good idea, I'll fix the sysctl stuff and document it. It might also be worth adding "Hightgid" to status. fs/proc/array.c | 5 - include/linux/pid.h | 2 ++ include/linux/pid_namespace.h | 1 + kernel/pid.c | 19 +++ kernel/pid_namespace.c| 22 ++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index cd3653e4f35c..f1e0e69d19f9 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, int g; struct fdtable *fdt = NULL; const struct cred *cred; + const struct upid *upid; pid_t ppid, tpid; rcu_read_lock(); @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, if (tracer) tpid = task_pid_nr_ns(tracer, ns); } + upid = pid_upid_ns(pid, ns); cred = get_task_cred(p); seq_printf(m, "State:\t%s\n"
Re: [systemd-devel] [PATCH 2/5] Add a machine_id_commit call to commit on disk a, transient machine-id
On Mon, 24.11.14 12:35, Didier Roche (didro...@ubuntu.com) wrote: > > +static int is_on_temporary_fs(int fd) { > +struct statfs s; > + > +if (fstatfs(fd, &s) < 0) > +return -errno; > + > +return F_TYPE_EQUAL(s.f_type, TMPFS_MAGIC) || > + F_TYPE_EQUAL(s.f_type, RAMFS_MAGIC); > +} This should really move to util.[ch] I figure, and reuse is_temporary_fs() that we already have there. > + > +int machine_id_commit(const char *root) { > +const char *etc_machine_id; > +_cleanup_close_ int fd = -1; > +_cleanup_close_ int initial_mntns_fd = -1; > +struct stat st; > +char id[34]; /* 32 + \n + \0 */ > +int r; > + > +if (isempty(root)) > +etc_machine_id = "/etc/machine-id"; > +else { > +etc_machine_id = strappenda(root, "/etc/machine-id"); > +path_kill_slashes((char*) etc_machine_id); > +} > + > +r = path_is_mount_point(etc_machine_id, false); > +if (r <= 0) { > +log_error("We expected that %s was an independent mount.", > etc_machine_id); > +return r < 0 ? r : -ENOENT; > +} I think this should really work in idempotent style, i.e. so that you exit cleanly if the thing is already a proper file. > + > +/* read existing machine-id */ > +fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY); > +if (fd < 0) { > +log_error("Cannot open %s: %m", etc_machine_id); > +return -errno; > +} > +if (fstat(fd, &st) < 0) { > +log_error("fstat() failed: %m"); > +return -errno; > +} > +if (!S_ISREG(st.st_mode)) { > +log_error("We expected %s to be a regular file.", > etc_machine_id); > +return -EISDIR; > +} > +r = get_valid_machine_id(fd, id); > +if (r < 0) { > +log_error("We didn't find a valid machine-id."); > +return r; > +} > + > +r = is_on_temporary_fs(fd); > +if (r <= 0) { > +log_error("We expected %s to be a temporary file system.", > etc_machine_id); > +return r; > +} > + > +fd = safe_close(fd); > + > +/* store current mount namespace */ > +r = namespace_open(0, NULL, &initial_mntns_fd, NULL, NULL); > +if (r < 0) { > +log_error("Can't fetch current mount namespace: %s", > strerror(r)); > +return r; > +} > + > +/* switch to a new mount namespace, isolate ourself and unmount > etc_machine_id in our new namespace */ > +if (unshare(CLONE_NEWNS) == -1) { > + log_error("Not enough privilege to switch to another > namespace."); > + return EPERM; > +} > + > +if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) < 0) { > + log_error("Couldn't make-rslave / mountpoint in our private > namespace."); > + return EPERM; > +} > + > +r = umount(etc_machine_id); > +if (r < 0) { > +log_error("Failed to unmount transient %s file in our > private namespace: %m", etc_machine_id); > +return -errno; > +} > + > +/* update a persistent version of etc_machine_id */ > +fd = open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444); > +if (fd < 0) { > +log_error("Cannot open for writing %s. This is mandatory to > get a persistent machine-id: %m", > + etc_machine_id); > +return -errno; > +} > +if (fstat(fd, &st) < 0) { > +log_error("fstat() failed: %m"); > +return -errno; > +} > +if (!S_ISREG(st.st_mode)) { > +log_error("We expected %s to be a regular file.", > etc_machine_id); > +return -EISDIR; > +} > + > +r = write_machine_id(fd, id); > +if (r < 0) { > +log_error("Cannot write %s: %s", etc_machine_id, > strerror(r)); > +return r; > +} Since you prepared the original patch, we improved the loggic logic of systemd. It would be great if you would update the patch to make use of it. In particular, this means avoiding strerror() for cases like the above (because it is not thread-safe to use). Instead use the new log_error_errno() that takes the error value as first parameter, and then makes sure that %m actually evaluates to the error string for that error. Also it will then return the error, so that you can simplify the four lines above into: if (r < 0) return log_error_errno(r, "Cannot write %s: %m", etc_machine_id); > +fd = safe_close(fd); > + > +/* return to initial namespace and proceed a lazy tmpfs unmount */ > +r = namespace_enter(-
Re: [systemd-devel] [PATCH 1/5] Factorize some machine-id-setup functions to be reused
On Mon, 24.11.14 12:35, Didier Roche (didro...@ubuntu.com) wrote: > > +static int get_valid_machine_id(int fd, char id[34]) { > +assert(fd >= 0); > +assert(id); > + > +if (loop_read(fd, id, 33, false) == 33 && id[32] == '\n') { > +id[32] = 0; > + > +if (id128_is_valid(id)) { > +id[32] = '\n'; > +id[33] = 0; > +return 0; > +} > +} > + > +return -EINVAL; > +} As a matter of coding style we try hard to avoid functions that clobber their parameters if they fail. Please follow the same scheme here. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 3/5] Introduce machine-id-commit binary
On Mon, 24.11.14 12:35, Didier Roche (didier.ro...@canonical.com) wrote: This one looks flawless to me! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 5/5] machine-id-commit: add man pages
On Mon, 24.11.14 12:36, Didier Roche (didro...@ubuntu.com) wrote: The last two look great too! Thanks, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] rules: rerun vconsole-setup when switching from vgacon to fbcon
On Mon, 01.12.14 17:25, Ivan Shapovalov (intelfx...@gmail.com) wrote: > > To me it appears to be quite OK to be run inside a udev rule. > > Well, maybe it is OK to be run from an udev rule, but it is still an > inconsistency between running that binary on boot (via a unit) and > running the same binary when a framebuffer console is added (directly)... Yeah, we do that a couple of times however. For example the alsa volume restor tools are both run from an udev rule and as unit. The latter is important since /var (where they store their data) is not available when udev starts probing things... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Unbounded journal~ file accumulation
On Wed, Nov 27, 2014 at 09:21:00AM -0500, Zbigniew Jedrzejewski-Szmek wrote: > There were some fixes in this area. 3bfd4e0c6341b0ef946d2198f089743fa99e0a97 > might fix the unbounded size. Indeed, it does! Applying this change on top of the Debian jessie version of v215 cleaned it right up on each bootup. Thanks for locating that commit. > You can make the files smaller, ... See below. On Sat, Nov 29, 2014 at 05:44:14AM -0500, Lennart Poettering wrote: > There's now "journalctl --vacuum-size=23M" that removes journal files > until the disk usage is under the specified limit. It removes both > the clean and unclean journal files. I'll keep that in mind for when we get a version of systemd that includes that option. But, with the 3bfd4e patch, I think we're set. > Well, very small journal files are not efficient. Neither in size > (because we compress identical fields into one instance if they keep > appearing), nor in access time (since access is optimized for few > small rather than many small files, and complexity grows O(n) with > number of files). Thank you both for humoring me by discussing this. I'll plan on leaving the size alone unless further problems arise. The forced rotation (my workaround for the lack of vacuum on startup) was compounding the issue (at least two 4M files per boot instead of just one); without needing to do that, it's not nearly as bad. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Compatibility between D-Bus and kdbus
On Wed, Nov 26, 2014 at 01:25:18AM +0100, Lennart Poettering wrote: > On Tue, 25.11.14 12:01, Thiago Macieira (thi...@kde.org) wrote: [...] > > > > === KDBUS_ATTACH_NAMES === > > > > > > > > Documentation for metadata says that userspace must cope with some > > > > metadata > > > > not being delivered. Can we at least require that KDBUS_ATTACH_NAMES be > > > > delivered if requested? If the cookie in the match rule isn't provided > > > > in > > > > the message reception, having the source's names would help solve the > > > > problem of the signal delivery. > > > > > > > > The timestamp should also be mandatory. > > > > > > Yes, they are mandatory. process credentials might be suppressed > > > hwover, for example if they cannot be translated due to namespaces. > > > > Thanks. Could you clarify in the docs? > > Daniel, David? Could you add a note about this? Ok pushed a note about namespace issues, thanks! -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] networkd: Introduce Link Layer Discovery Protocol (LLDP)
On Sun, 23.11.14 10:15, Susant Sahani (sus...@redhat.com) wrote: > This patch introduces LLDP support to networkd. it implements the > receiver side of the protocol. > > The Link Layer Discovery Protocol (LLDP) is an industry-standard, > vendor-neutral method to allow networked devices to advertise > capabilities, identity, and other information onto a LAN. The Layer 2 > protocol, detailed in IEEE 802.1AB-2005.LLDP allows network devices > that operate at the lower layers of a protocol stack (such as > Layer 2 bridges and switches) to learn some of the capabilities > and characteristics of LAN devices available to higher > layer protocols. Looks great! Would love to see this systemd! TOm, what's the plan here, will you work with Susant to make sure this gets added? BTW, would be great to also expose this in networkctl right-away. i.e. change networkd to write the learned info into the state file, and then show this in "networkctl status". Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] networkd: integrate LLDP
On Sun, Nov 23, 2014 at 02:17:18PM +0100, Tom Gundersen wrote: > On Sun, Nov 23, 2014 at 12:01 PM, Tomasz Torcz wrote: > > On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote: > >> This patch integrates LLDP with networkd. > > > > In Fedora, we already have LLDP receiver/broadcaster – ladvd. > > It has this neat feature of abusing ifAlias by putting endpoint > > name there. This gives port information right at "ip l" output: > > > > 3: enp5s0: mtu 1500 qdisc fq_codel state > > UP mode DEFAULT group default qlen 1000 > > link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff > > alias connected to Core2-3b-24p (Fa0/7) > > > > Would it be possible to implement this in networkd? > > Definitely possible. However, any suggestions on how to avoid stepping > on the toes of other abusers of ifAlias, or is ladvd simply ignoring > this? I've enabled it in Fedora's ladvd almost three years ago, and received no bugreports about breaking any other software. ladvd does not take any special precautions. I think it would be safe to refrained from changing ifAlias if: - ifAlias is already set, AND - it doesn't start with "connected to " -- Tomasz Torcz"Funeral in the morning, IDE hacking xmpp: zdzich...@chrome.plin the afternoon and evening." - Alan Cox ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] networkd: Introduce Link Layer Discovery Protocol (LLDP)
On Mon, Dec 1, 2014 at 10:42 PM, Lennart Poettering wrote: > On Sun, 23.11.14 10:15, Susant Sahani (sus...@redhat.com) wrote: > >> This patch introduces LLDP support to networkd. it implements the >> receiver side of the protocol. >> >> The Link Layer Discovery Protocol (LLDP) is an industry-standard, >> vendor-neutral method to allow networked devices to advertise >> capabilities, identity, and other information onto a LAN. The Layer 2 >> protocol, detailed in IEEE 802.1AB-2005.LLDP allows network devices >> that operate at the lower layers of a protocol stack (such as >> Layer 2 bridges and switches) to learn some of the capabilities >> and characteristics of LAN devices available to higher >> layer protocols. > > Looks great! Would love to see this systemd! TOm, what's the plan > here, will you work with Susant to make sure this gets added? Yup, it is all good as far as I'm concerned, just thought I'd leave it on the ML for some time to hopefully get some reviews before I go ahead and merge it. > BTW, would be great to also expose this in networkctl > right-away. i.e. change networkd to write the learned info into the > state file, and then show this in "networkctl status". Yeah, we are just discussing this now. Susant is looking into it. -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] networkd: integrate LLDP
On Mon, Dec 1, 2014 at 11:01 PM, Tomasz Torcz wrote: > On Sun, Nov 23, 2014 at 02:17:18PM +0100, Tom Gundersen wrote: >> On Sun, Nov 23, 2014 at 12:01 PM, Tomasz Torcz wrote: >> > On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote: >> >> This patch integrates LLDP with networkd. >> > >> > In Fedora, we already have LLDP receiver/broadcaster – ladvd. >> > It has this neat feature of abusing ifAlias by putting endpoint >> > name there. This gives port information right at "ip l" output: >> > >> > 3: enp5s0: mtu 1500 qdisc fq_codel state >> > UP mode DEFAULT group default qlen 1000 >> > link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff >> > alias connected to Core2-3b-24p (Fa0/7) >> > >> > Would it be possible to implement this in networkd? >> >> Definitely possible. However, any suggestions on how to avoid stepping >> on the toes of other abusers of ifAlias, or is ladvd simply ignoring >> this? > > I've enabled it in Fedora's ladvd almost three years ago, and > received no bugreports about breaking any other software. > > ladvd does not take any special precautions. I think it would > be safe to refrained from changing ifAlias if: > - ifAlias is already set, AND > - it doesn't start with "connected to " As discussed in the other thread, we'll expose this info through networkctl anyway, so it may not be necessary to use ifAlias (or are there other benefits to this, than just to get it in ip(8) output? -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] reacting to unit failures (OnFailure)
Hello, While converting from Upstart to SystemD, came upon this issue. Is this case not covered or am I missing something? In Upstart, I can start a job when another job fails, and there's a $JOB variable that tells me what was the job that failed. start on (stopped RESULT=failed PROCESS=post-stop) script echo "this just failed: $JOB" end script In SystemD I can register a unit to be started when another unit fails. But there seems to be no way of knowing what unit failed. Is that correct? Thanks, Alexander ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] reacting to unit failures (OnFailure)
On Mon, 01.12.14 17:10, Nekrasov, Alexander (alexander.nekra...@emc.com) wrote: > Hello, > > While converting from Upstart to SystemD, came upon this issue. Is this case > not covered or am I missing something? > > In Upstart, I can start a job when another job fails, and there's a $JOB > variable that tells me what was the job that failed. > > start on (stopped RESULT=failed PROCESS=post-stop) > > script > echo "this just failed: $JOB" > end script > > In SystemD I can register a unit to be started when another unit > fails. But there seems to be no way of knowing what unit failed. Is > that correct? The idea is that you use "OnFailure=myfailureunit@%n.service". %n is automatically replaced by the name of the unit you place this in. Then, in the failure unit you can identify the instance again with %i or %I. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On Mon, 01.12.14 11:47, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote: > Hello > Could you take a look at my patch? Sorry for the delay, I have a huge backlog of unreviewed patches, and am now working through them! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On Thu, 13.11.14 18:11, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote: Looks pretty good, but I coudln't apply it. There's something wrong with the patch the deletion/renaming of the service files doesn't work. Did you create this patch with git-format-patch? > if (is_unix) { > (void) getpeercred(in_fd, &ucred); > (void) getpeersec(in_fd, &peersec); > + > +#ifdef HAVE_SMACK > +if (mac_smack_use()) { > +if (peersec) { > + > +r = mac_smack_apply_pid(getpid(), peersec); > +if (r < 0) > +log_warning("Failed to set SMACK > label %s : %s", peersec, strerror(-r)); > +} else > +log_warning("Invalid SMACK label"); > + > +r = drop_capability(CAP_MAC_ADMIN); > +if (r < 0) > +log_warning("Failed to drop CAP_MAC_ADMIN: > %s", strerror(-r)); > +} > +#endif > } Hmm, could you make this bit a function of its own please? > +m4_ifdef(`HAVE_SMACK', > +Capabilities=cap_mac_admin=i > +SecureBits=keep-caps > +) Hmm, it might be a good idea to also add some code to Makefile.am to add the capability to the file after installation in case of HAVE_SMACK. We used to do set a file cap like this on systemd-detect-virt until a while back. See commit fdd25311706bd32580ec4d43211cdf4665d2f9de for details about the setcap lines we removed back then. It should be easy to just readd those lines and adapt them to apply to systemd-bus-proxyd instead! Thanks! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] serialization bug, swap bug, etc.
On Thu, 20.11.14 09:13, Mantas Mikulėnas (graw...@gmail.com) wrote: > ~ I'm also getting this on every reload: > > systemd[1]: [/usr/lib/systemd/system/systemd-journald.service:24] Failed to > parse capability in bounding set, ignoring: CAP_AUDIT_READ > > I suppose I can ignore the message. I see that cap_audit_read was added to > kernel 3.16, but unfortunately it doesn't exist in the current libcap > release (libcap 2.24). We currently need one actual operation from libcap, plus the capability string tables. THat's all. Which really makes me wonder whether we shouldn't simply do the table and the one syscall in systemd and get rid of the dep, it's not really that complex, and we have some caps code anyway in place... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] TimeoutStopSec is ignored (regression)
On Mon, 1 Dec 2014, David Herrmann wrote: Hi On Sat, Nov 29, 2014 at 12:35 PM, Ross Lagerwall wrote: Hi, On recent versions of systemd, unit_kill_context doesn't set wait_for_exit to true which means that service_enter_signal sends SIGTERM, immediately moves into stop-sigkill and sends SIGKILL, ignoring TimeoutStopSec and often killing processes without giving them a chance to cleanup. Reverting the following change, fixes the problem: commit 1baccdda2e954214e0c5463d6ed8f06009b33c41 Author: Lennart Poettering Date: Wed Feb 5 02:22:11 2014 +0100 core: don't wait for non-control/non-main processes when killing processes on the host either Since the current kernel cgroup notification logic is easily confused by existing subgroups, let's do the same thing as in containers before. and just not wait for non-control and non-main processes. This should be corrected as soon as we have sane cgroup notifications from the kernel. The commit-message and the comment it adds should answer your question: The kernel cgroup API does not allow us to wait for non-control processes. That is, we still honor TimeoutStopSec and friends if we have to wait for the main-process and/or control process (in those cases, wait_for_exit is still set to true). However, if there are other processes remaining in the cgroup, we now ignore it. See the commit you mentioned for an explanation. What specifically would happen if wait_for_exit were kept true for other processes in the cgroup? As far as I can see they would continue to be watched for SIGCHLD (since unit_watch_all_pids should have been previously called on the unit). PID 1 may or may not get SIGCHLD for them, depending on whether they got reparented before they exited. Each time systemd gets a SIGCHLD, it can use unit_tidy_watch_pids to check the unit's entire PID list to see which ones are still present. So at best we see the PIDs go away one by one in the cgroup, and we know when it's empty ourselves. At worst we don't see the last PID's SIGCHLD, so we have to wait the entire TimeoutStopSec interval before discovering that the cgroup is empty. So I must be missing something important here, since everyone is stating emphatically that this is unsolveable until cgroup empty notifications are fixed. The only issue I can think of is that PIDs may be reused before the TimeoutStopSec interval completes. - Michael ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd: Fix wrong timestamps in rtc-in-local time mode.
On Wed, 19.11.14 18:20, Chunhui He (hchun...@mail.ustc.edu.cn) wrote: > systemd generates some timestamps before the very first call of > settimeofday(). > When we are in rtc-in-local time mode, these timestamps are wrong. > > Affected timestamps are: > Kernel, InitRD, Userspace, SecurityStart, SecurityFinish I am not really convinced that we really should try to make this work. rtc-in-local-time has so many issues, it really doesn't stop here. If people make use of this, then this is what they get really, and I am not sure we really should work around it. I mean, systemd really isn't the only component which might query the clock this early, in the initrd there might be a ton of other components too, and it's not realistic to add similar kludges to them all. In general: rtc-in-local-time is a compatibility hack, and we only want to support it to the minimal level necessary for compatibility, but not more. The proper fix for this problem I guess is to use rtc-in-utc instead! Sorry if that's disappointing, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] networkd: disable tmpfiles and sysusers bits associated with networkd
On Mon, 24.11.14 09:30, Łukasz Stelmach (l.stelm...@samsung.com) wrote: > It was <2014-11-21 pią 21:36>, when Lennart Poettering wrote: > > On Fri, 21.11.14 17:07, Łukasz Stelmach (l.stelm...@samsung.com) wrote: > > > >> On a system configured without networkd and sysusers there still needs > >> to be the unnecessary systemd-network user, otherwise systemd-tmpfiles > >> fails to start. > >> > >> Move information associated with networkd in tmpfiles.d and sysusers.d > >> to separate files. Do not install it if netwrorkd is not enabled. > > > > In principle looks OK, but I'd prefer if we would write this out with > > m4 (see etc.conf.m4) and keep it in the current files, rather than > > split this up in numerous files. > > > > Especially in the case of /run/systemd/netif this actually matters: if > > we split that out into its own tmpfiles snippet, then packagers would > > most likely put that in its own RPM/DEB if they split out those > > daemons. But this is not advisable in this case, as sd-network (which > > will eventually be a public API of libsystems) needs the directory to > > be around to install an inotify watch. If the directory doesn't exist, > > and the API is used it will fail entirely, which is suboptimal, given > > that networkd might be installed later on, and things should then just > > start to work. > > Will it be necessary for this directory to be owned by systemd-network > even without networkd? Yes. If networkd is compile-time enable the dir should exist and be properly owned, even if it networkd is split off into a separate binary package and currently not installed. Your patch in the version Zbigniew commited looks correct in this regard! Thanks! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Tue, 18.11.14 12:11, Didier Roche (didro...@ubuntu.com) wrote: > Fedora doesn't enable and start all units on package installation: there are > some preset files, based on flavors, which is basically the policy stating > which units to enable/disable by default. Some other units are always > enabled (unless masked), by using symlinks directly shipped with the package > like in /usr/lib/systemd/system/multi-users.target.wants/ for intance. > Contrary to that, Debian/Ubuntu has the policy to enable/start services and > facilities on package installation during postinst. This is done > (indirectly) via "systemctl enable", which creates symlinks in > /etc/systemd/system/*.wants/. Distros really should use "systemctl preset" for this. It's what it is for. If no preset policy is installed "systemctl preset" is actually equivalent to "systemctl enable", but it allows admins to install their own policy which then takes effect. Really, distributions should *not* use "systemctl enable" in postinst. It breaks presets for zero gain. knowledgable admins. If the admin should be able to disable/enable a service during normal operation as part of his normal workflow then it should *not* be enabled via symlinks in /usr, but instead carry [Install] and be enabled by the postinst script via "systemctl preset". > - We are mixing sys admin information and distro default choices in the same > directories, and can't tell apart what is what. You can actually. There's "systemctl preset-all" for example which brings the system back into the exact choices of the upstream distro. > We were thus thinking about having all default distro choices shipping > target symlinks in /usr/lib, and having the administrator overrides this in > /etc via systemctl. This could look similar to masking a unit, i. e. a > symlink "/etc/.../wants.d/foo -> /dev/null" overrides > "/usr/lib/.../wants.d/foo -> ../foo.service", and would be an explicit > representation of "the admin does not want foo.service to autostart" in > /etc. Overriding symlinks with symlinks to different targets is difficult, as systemd only really cares about the symlink names, and not so much about the targets. > However, we did notice the following: taking an unit, with an [Install] > section and a symlink to enable in via that target in /usr/lib: > - systemctl status will report "disabled", where it's actually > enabled and starting for that unit. This is a bug which should be fixed in > any case, as "disabled" is outright wrong. On IRC it was suggested that > those are "static" units, but then it should say at least that. As mentioned above: units that carry [Install] and are also enabled via /usr/lib/ are broken really. Either you use [Install] and thus ask systemd to manage the symlinks in /etc for you via commands like systemctl enable/disable/preset, or you say that systemctl shall not manage the symlinks and instead create a static on in /usr/lib. But having a unit that is both managed and unmanaged doesn't really make sense. > - If we ln -s /dev/null /etc/<…>/<…>.wants/, then systemctl status > will display the unit as being enabled, and it will activated once > the target is reached. This is also counterintuitive, as usually this means > to mask/completely disable the unit. Yeah, symlinks in .wants/ are just about creating dependencies, not about overriding units really... > Part of the discussion on #systemd pointed out that the admin should then > use systemctl mask . However, that means: Hmm, "systemctl mask" is really nothing we should advertise to users or admins. It's a last resort tool. Should not appear in normal workflows. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Tue, 18.11.14 13:01, Martin Pitt (martin.p...@ubuntu.com) wrote: > We can certainly ship a preset of "enable *" to reflect the policy > that in general services do get enabled by default. But this still > leaves some issues: No need to ship "enable *", btw. It's the implied default if no preset file is found or no matching line is found in them. Or in other words: the fedora default of "disable *" needs an explicit preset file, but the Debian default of "enable *" is actually the upstream default without preset file, too. > * This doesn't solve the problem of having these rather uninteresting >and cluttering symlinks in /etc at all; the wants.d symlinks would >still be as they are now, just the place that decides when to >enable them changes. If they are so uninteresting that there's no real benefit in allowing them to be modified, then I'd really recommend to simply ship the symlinks in /usr/lib, and not include an [Install] section. A lot of systemd's own units are like that. For example, since disabling udev or journald will only have the effect of making your system unbootable we don't ship [Install] sections for them, and instead hook them in statically. > I. e. my question is not so much about being able to restore the > default wants.d symlinks in /etc after a factory reset -- there are > multiple ways how this can be done: with presets or iterating over the > installed packages and re-enabling them (Debian also does some > house-keeping which unit files got enabled by postinstall scripts, > which can simply be replayed). Note that "systemctl preset-all" erally just iterates throught unit files that are installed and individually does the equivalent of "systemctl preset". There's little magic in there... > I'm interested in the reason for that. This basically cements the > status quo that one *has* to have a gazillion links in /etc in order > for your system to work, even if they are not at all specific to the > particular system or represent a deviation from the default install. It's not a gazillion really. It's 15 or so on my laptop here... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Tue, 18.11.14 14:40, Colin Guthrie (gm...@colin.guthr.ie) wrote: > Well the "upstream blessed" RPM way is to call "%systemd_post" macro in > your %post script, but (personally) I don't like this as it makes the > implementation very much embedded into the RPMs so changing the upstream > macro needs a full package rebuild. > > In Mageia we do something similar but we shell out to a script > instead. The idea was to make "systemctl preset" generic enough so that it is all we need to change should we want to change the effect of the RPM macros one day, if you follow what I mean... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Tue, 18.11.14 14:10, Tom Gundersen (t...@jklm.no) wrote: > > - We are mixing sys admin information and distro default choices in the same > > directories, and can't tell apart what is what. > > That is true. Could we perhaps improve on systemctl by printing > "enabled (preset)"/"disable (preset)" for units that are in the > default state? I know this does not change the fact that you have > distro-default (via presets) links in /etc, but it should give the > end-user a nicer experienc I guess? Sounds like a good idea. Added to TODO list. > My take on this is: make sure presets are applied on "firstboot" > (which I think they are), so empty /etc works just fine, and then > improve on systemctl to better show the distinction between 'enabled > by default' or 'enabled by choice' (and same for 'disabled'). Yes systemd applies the presets on firstboot automatically, before calculating the initial transactions. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Tue, 18.11.14 14:37, Martin Pitt (martin.p...@ubuntu.com) wrote: > > We now have: > > > > enabeld - [Install] section and symlink in /etc/**/*.wants.d/ > > disabled - [Install] section and no symlink in /etc/**/*.wants.d/ > > static - no [Install] section and symlink in /usr/lib/**/*.wants.d/ > > masked - symlink from the unit file-name to /dev/null in /etc > > > > you want in addition: > > > > preset (or something like that) - [Install] section and symlink in > > /usr/lib/**/*.wants.d/ I am not totally opposed to allowing /dev/null symlinks in .wants.d/ subdirs, and considering them a way to override dependencies. Such a "mask a dependency" concept would be OK. > I'd call this just "enabled". It follows the same "/etc/ trumps /usr" > schema that we have for udev rules, units, etc., i. e. an "enabled" > symlink should be allowed to live in /etc, /usr, and maybe even /run > (haven't though about whether this really makes sense, but perhaps for > full consistency it should be allowed). OK, let me get this right. You want an algorithm like this when somebody invokes "systemctl enable": a) if the unit file contains [Install], execute that, done b) if the unit file does not contain [Install], then delete any /dev/null symlinks in /etc/systemd/*.{wants,requires}.d/* of the same name. Then, "systemctl disable" should do this in your opinion: a) if the unit file contains [Install] remove all symlinks in /etc/systemd/*.{wants,requires}.d/* to it. b) if it doesn't, then *add* new symlinks to /dev/null in all .{wants,requires}.d/ directories where symlinks exist for it in /usr? Did I get this right? I am not convinced this is really a good idea though, as with this scheme package changes might reenable a unit that is otherwise disabled. Also, I kind like the fact that there's currently a clean error message generated when people try to disable a unit that is part of the OS itself. The scheme you propose would degrade that case to a NOP however, right? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Tue, 18.11.14 16:09, Michael Biebl (mbi...@gmail.com) wrote: > 2014-11-18 15:59 GMT+01:00 Colin Guthrie : > > Didier Roche wrote on 18/11/14 13:58: > >> This would be maybe a nice way for the admin to know what's coming from > >> a distribution default or not. However, let's say I want to ensure that > >> ssh will always be available on my server, I would (even if it's in my > >> server preset) then systemctl enable openssh, no matter whatever future > >> preset updates does (like disable it in the next batch upgrade). > > > > For the avoidance of doubt, I believe that running systemctl preset > > should only ever happen on *first* install, never on upgrade or such like. > > > > And what are you going to do, if the unit file changes? > Say v1 had > > [Install] > WantedBy=multi-user.target > > and version B has > [Install] > WantedBy=foo.target Package installs should probably not try to do something about this case, just leave things as is. I mean, let's not forget that admins should be able to define their own targets and then enabled units in them and disable them elsewhere. Package upgrades should not manipulate that. The first installation of a package should enable a unit if that's what the preset policy says, but from that point on the configuration is admin configuration and not be changed anymore automatically. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] /usr vs /etc for default distro units enablement
On Fri, 28.11.14 11:15, Didier Roche (didro...@ubuntu.com) wrote: > The distribution comes preinstalled with one dm, enable * -> enable it, have > the Alias=display-manager.service picking the right one. > However, let's say the user installed then another dm, what happens? Both > will be enabled if we systemctl preset (as the discussion was > to remove our debian enable that was conditioned on the > postinst). "systemctl preset" will fail if there are already conflicting symlinks. Hence the first installed DM wins, the second loses. > I don't think we should have systemctl preset running under > any condition as a wipe of /etc and then "systemctl preset-all" would give a > potential different result (I'm not even sure how this will work with those > alias, the first matching the alias wins and get the symlinks?) Dont follow? "wipe"? > We can of course have an ubuntu-desktop.preset which disables all dms by > lightdm, and an ubuntu-gnome.preset which disables all dms but gdm (and > having those settings conflicting with each other), but it's seems that for > every aliases, we need to maintain such a list (as we enable * by > default)? Not following here. Different flavours of Ubuntu should probably just ship different preset files. (Or well, the main ubuntu should ship one that enables lightdm, and then the gnome flavour ship another preset file, with a lower name, tht overries the lightdm line, and enables gdm instead). Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors
On Mon, 01.12.14 12:04, Jan Synacek (jsyna...@redhat.com) wrote: > Thinking about it again, I don't think that it's a good idea to pass > those errors to the client... > > A few examples: > > # localectl set-x11-keymap QQ > > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: > libxkbcommon: Couldn't find file "symbols/QQ" in include paths > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: > libxkbcommon: 1 include paths searched: > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: > libxkbcommon: /usr/share/X11/xkb > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: > libxkbcommon: Abandoning symbols file "(unnamed)" > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: > libxkbcommon: Failed to compile xkb_symbols > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: > libxkbcommon: Failed to compile keymap > Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: Cannot > compile XKB keymap for new x11 keyboard layout ('' / 'QQ' / '' / ''): Invalid > argument > > # localectl set-x11-keymap us pc105 QQ > > Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: > libxkbcommon: Couldn't process include statement for 'us(QQ)' > Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: > libxkbcommon: Abandoning symbols file "(unnamed)" > Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: > libxkbcommon: Failed to compile xkb_symbols > Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: > libxkbcommon: Failed to compile keymap > Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: Cannot > compile XKB keymap for new x11 keyboard layout ('pc105' / 'us' / 'QQ' / ''): > Invalid argument > > Those errors are just stupid. Maybe they would make sense if there was > also libxkbcommon code in front of me... But from the user's > perspective, they say nothing useful. I added the prefix so the errors > at least have some context. > > My original patch would say something like "No such layout 'QQ'" in the > first case and "No variant 'QQ' for layout 'us'" in the second. > > So, is it worth logging this information? Any other ideas? Maybe log it, but downgrade it to LOG_DEBUG? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [BUG] too many rfkill services
On Sun, 23.11.14 12:34, Andrei Borzenkov (arvidj...@gmail.com) wrote: > В Fri, 21 Nov 2014 01:26:36 +0100 > Lennart Poettering пишет: > > > On Thu, 20.11.14 19:56, Lukasz Stelmach (stl...@poczta.fm) wrote: > > > > > I talked to the kernel guys at my office and they told me that it is > > > quite usual (at least for USB devices, and my wlan and bt are USB) > > > that devices are stopped and unregistered in the kernel before > > > a system is suspended end reported as completely new ones > > > with increased numbers after machine resumes. > > > > So, I have now added some code that adds BindsTo= for the device unit > > to the service. This won't fix much though, as the service is likely > > to fail in ExecStop= because it cannot find the device anymore. > > > > Yes, you are right. It accumulates the same services but now failed > instead of active. > > Hmm ... should not systemd inform service that device it is BoundTo is > gone? I mean, while service may need to do some cleanup in this case, > it is obvious that it cannot access device which no more exists. This > would allow graceful exit, instead of returning error. Well, it's racy really. Of course systemd sends SIGTERM before shutting down a service, but this is async... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] localed: validate set-x11-keymap input
On Mon, 24.11.14 15:21, David Herrmann (dh.herrm...@gmail.com) wrote: > I now pushed something to -git, see: > > commit d4f5a1f47dbd04f26f2ddf951c97c4cb0ebbbe62 > Author: David Herrmann > Date: Mon Nov 24 15:12:42 2014 +0100 > > localed: validate xkb keymaps > > I think duplicating the xkb-parsers is something we should avoid. Ran > Benita was even working on a v2 format to drop all the legacy bits > no-one uses in the old, crufty xkb format. I dislike having a fixed > parser in systemd. Sure, our --list-layouts option needs to do this, > but maybe we can drop that some day, too. > > I pushed a fix to test-compile keymaps on set-x11-keymap calls. So > far, I only print a warning if it fails. Please feel free to post > patches to extend this. For instance, I think we might want to do that > in localectl (maybe even forward the xkb-logs then) and fail hard if > the compilation fails (even though I dislike --force options..). Wouldn't it make sense to refuse keymaps we cannot compile from the localed side? I kinda prefer doing the validity checks on the server side. Doing enumeration (which is primarily relevant fo cmdline completion) on the client side is OK, but validity checking should really be done on the server side. I think it would be great if we'd generate a simply nice error message if the compile test failed (just some error "Keyboard map could not be compiled, refusing." or so). Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] localed: validate set-x11-keymap input
On Mon, 24.11.14 13:31, Jan Synacek (jsyna...@redhat.com) wrote: > Lennart Poettering writes: > > On Fri, 14.11.14 12:42, Jan Synacek (jsyna...@redhat.com) wrote: > >> +if (look_for == LAYOUTS) { > >> +Set *s; > >> +char *k; > >> +Iterator i; > >> +/* XXX: Is there a better way to sort Hashmap keys? */ > >> +_cleanup_strv_free_ char **tmp = NULL; > >> + > >> +HASHMAP_FOREACH_KEY(s, k, keymap->x11_layouts, i) > >> +if (strv_extend(&tmp, k) < 0) > >> +(void) log_oom(); > > > > There's hashmap_get_strv() for cases like this. > > I need keys, not values. I didn't find any hashmap_get_strv() equivalent > for keys. Maybe add it then? I think this might become useful even beyond the kbd mapping part one day. > > Also, please neer invoke strv_extend() when appending to unbounded > > arrays. It's slow! > > What is a preffered way to extend a strv? In this case, I know the final > size, so it the array could manually be copied. I don't think that > that's very nice, though. Best case: you know the precise size of the array in the end. In that case, simply use realloc()/realloc_multiply() to allocate the right sized array, fill it in, done. If you don#t know the size, but you guess that it might be quite a few entries in there in the end and you might be adding a good chunk of them, please use greedy_realloc(). It grows the array exponentially, which is very beneficial performance-wise. If you don't know the size, but you are pretty sure that it will never gro to more than 10 entries or so, and that you are just going to add a couple of them, then strv_extend()/strv_push()/strv_consume() are OK. > > Humm, when clearing up hashmaps please just write a loop that steals > > the first entry, free it and then repeat. Iterating through a hashmap > > while deleting its entries is unnecessary... > > I need to free its entries *and* keys. I didn't find any function that I > could use for that, apart from hashmap_free_free(), which crashes in > my case. while ((k = hashmap_first_key())) { v = hashmap_steal_first(); free(v); free(k); } Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] make systemd service takes cpu exclusively
On Wed, 19.11.14 20:05, Andrei Borzenkov (arvidj...@gmail.com) wrote: > В Tue, 18 Nov 2014 06:25:43 + > "Cao, XinX" пишет: > > > Hi, Umut & David, > > > > My project needs the Graphical desktop to display on monitor as fast as > > possible, but I found lots of unrelated services( such as sound, network, > > ... ) are competing CPU even they are explicitly After graphical service. > > And this competion delays the startup of graphical desktop process. So, my > > opinion is to make graphical related programs runs first and the other > > unrelated services only starts after graphical program finished startup. > > Let me guess. Your Graphical desktop is defined of simple or forking, > right? So from systemd point of view service is started as soon as > process is forked and it proceeds with starting further service defined > After this one. One minor correction: "forking" actually is supposed to provide propery synchronization too, as daemons that use double-forking to daemonize should return in the parent only after the daemon is fully initialized. THis is in fact what classic sysv init scripts rely on to properly serialize startup of services that need to talk to each other. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [k]dbus: api, match replace and test extending
On Mon, 17.11.14 12:31, Rui Miguel Silva (rmf...@gmail.com) wrote: Heya, > > > - technical debt, if in the future the filter mechanism is change by > > >other than bloom. > > > so bloom maybe just be replaced with only generic filter could make more > > > sense? > > > > What do you mean by "only generic filter"? > > > Maybe I did not explain myself well, what I mean is: > Imagine that ahead we find that instead of bloom filtering mechanism, for > example, cuckoo filters are more eficient. The api have the filter > structs called struct kdbus_bloom_filter, my suggestion was to just change > that to struct kdbus_filter (and no attach to filter specific > implementation). Since they are very generic (generation and a data field) > and for the kdbus it is just a check between a mask and a filter. I had a closer look at cuckoo filters now. The lookup logic is quite different from bloom filters and involves iterating through "entries" of a bucket. Now, I am not convinced that Cuckoo filters are really something we want to do in kdbus, but should we determine one day that they in fact are, then the kernel side matching of filter against mask needs to look very different anyway, with different data structures. And in that case we really should define new items for that, that should not overload the existing kdbus_bloom_filter structures but be seperate, new structs. Hope this makes sense, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel