Re: [systemd-devel] fsckd needs to go
Le 03/04/2015 14:58, Lennart Poettering a écrit : Heya, Hey Lennart, so we discussed the whole fsckd situation a bit more here in Berlin, and we came to the conclusion that fsckd really should not exist the way it does in systemd. To start with, the code is really wrong, it should never have been merged in its current state, the read/write logic for the sockets is completely borked (I cannot even boot my own machine reliably with it!). And to my knowledge there has been no attempt to fix all of that, even though I asked for it. It also doesn't do at all what I suggested initially, as the flow of data is now fsck → systemd-fsck → systemd-fsckd → plymouth, and that's just crazy, that's two steps too many. systemd is supposed to be a few components playing well together, but certainly not a baroque network of components where data is passed though four hoops before it reaches the destination... I misunderstood first what you wanted in 2011, reading back from the mailing list. You would have noted that no comment (even on the first review which were made) raised those points in the multiple reviews that occured, hence it was merged. It's weird that it doesn't even boot your own machine reliably, as we have the first implementation running on all vivid machines by default, and it seems from the bug reports, reliably. However, I'm a bit surprised about the statement that no attempt has been done to fix it. I think you saw I have always been responsive, prioritizing your suggestions over other work to fix them. When you did your first public personal reserves about fsckd on the mainling list and I understood what flow you wanted[1], I posted fixes *the day after* (with some back and force review) to address your comments. All of them were merged by other systemd hackers and some even by ourself, but the biggest one, which directly addressed and implemented the flow of data you explicitly asked for is still waiting: http://lists.freedesktop.org/archives/systemd-devel/2015-March/029309.html (Note that this was proposed less than 48 hours after your complain about the data flow). Knowing that you were on holidays, I didn't push others too much, but Martin and I pinged you on IRC about it when you were back. Am I missing anything? Then, there's my general reservation with fsckd at all: file systems that still require offline fsck are certainly not the future, but we develop stuff for the future, and the idea to kill an fsck process while it is running is also very very questionnable. There's a reason why such functionality never existed on Fedora or RHEL: it's risky. I mean, it's all good allowing people to shoot themselves in the foot, but there's really *no* point in making that easy and giving it a fancy UI with support in the graphical boot splash. Shooting yourself in the foot should be possible, but not *easily*! And certainly not be allowed without prior authentication like you are doing it right now with the plymouth support. I can understand those points, just a little bit disappointed that wasn't stated months ago, when we started to work on it and before the whole refactoring… Thus, we decided to remove fsckd again entirely from systemd. However, if Ubuntu really wants to implement this anyway (I strongly believe that this is an absolute misfeature!), then I'd be willing to add the following for you: systemd-fsckd would try to connect to some AF_UNIX/SOCK_STREAM socket in the fs, after forking and before execing fsck in the child, and pass the connected socket to fsck via the -C switch. If the socket is not connectable it would avoid any -C switch. With this simple change you can make this work for you: simply write a daemon (outside of systemd) that listens on that sockets and reads the progress data from it. Using SO_PEERCRED you can query which fsck PID this is from and use it to kill it. You could even add this to ply natively if you wish, since it's kinda strange to bump this all off another daemon in the middle, unnecessarily. Changing this would actually make it very close to my initial suggestion, except that we would not have the receiving side for the progress data in systemd, you'd have to maintain that externally (or in ply). Not sure we are going so close to vivid finale, changing it again. We did implement all your suggestions and fixed it to match those. I'm feeling a little bit uneasy about how all this turned out, showing such good willing to get it contributed upstream we put into it, but if that's the fate of it… Didier [1] http://lists.freedesktop.org/archives/systemd-devel/2015-March/029186.html ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 3 commits - src/fsckd src/shared
Le 16/03/2015 18:41, Zbigniew Jędrzejewski-Szmek a écrit :
On Mon, Mar 16, 2015 at 10:33:54AM -0700, Tom Gundersen wrote:
src/fsckd/fsckd.c | 13 ++---
src/shared/util.c |4
src/shared/util.h |2 ++
3 files changed, 16 insertions(+), 3 deletions(-)
New commits:
commit e26169bd48c64753510a1194abdf4fb5dc907123
Author: Didier Roche
Date: Tue Mar 10 10:05:19 2015 +0100
fsckd: check if plymouth is running before attempting connection
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f24715c..6b35fc2 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -231,9 +231,12 @@ static int manager_connect_plymouth(Manager *m) {
union sockaddr_union sa = PLYMOUTH_SOCKET;
int r;
+if (!plymouth_running())
+return 0;
Why do we need to do this check? We try to connect right below, and
we'll get a connection error if plymouth is not running.
The issue is that we are raising a warning if we can't communicate to
the socket while connecting. This behavior puzzled some users looking at
their logs when they didn't get plymouth running early enough or at all
(repeating connection warnings).
There were 2 options:
- downgrading the warning to a debug, which may never give a hint if the
user is expecting to see something in plymouth and never get any connection
- have the plymouth_running() check.
(More refs at
http://lists.freedesktop.org/archives/systemd-devel/2015-March/029174.html)
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Remove explicit Plymouth integration
Le 16/03/2015 18:23, Tom Gundersen a écrit : On Mon, Mar 16, 2015 at 6:15 PM, Didier Roche wrote: Le 16/03/2015 17:53, Tom Gundersen a écrit : Thanks! Enthusiastically applied. Tom PS There is still some plymouth integration left in case anyone wants to work on getting rid of that. Hey Tom, Jasper, Note that systemd-fsckd (in the new set of patch posted last week on that ML to address Lennart's concerns) is using the plymouth_running() check to prevent some warning when the socket isn't there. I can copy the function back there, but maybe you can put that the plymouth_running() in util.c? Hi Didier, Just noticed this, sorry about that. I'll revert this hunk before applying your patches. No worry, thanks! :) Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Remove explicit Plymouth integration
Le 16/03/2015 17:53, Tom Gundersen a écrit :
Thanks! Enthusiastically applied.
Tom
PS
There is still some plymouth integration left in case anyone wants to
work on getting rid of that.
Hey Tom, Jasper,
Note that systemd-fsckd (in the new set of patch posted last week on
that ML to address Lennart's concerns) is using the plymouth_running()
check to prevent some warning when the socket isn't there. I can copy
the function back there, but maybe you can put that the
plymouth_running() in util.c?
Cheers,
Didier
On Mon, Mar 16, 2015 at 5:34 PM, Jasper St. Pierre
wrote:
Even if plymouth is running, it might have not displayed the splash yet,
so we'll see a few lines on fbcon when we should have otherwise had
nothing.
Plymouth integration was added to systemd in commit
6faa11140bf776cdaeb8d22d01816e6e48296971. That same day, Plymouth got
systemd integration [0]. As such, the Plymouth integration has always
been obsolete, and was probably only for older Plymouth's. But I can't
imagine anybody running a Plymouth from 2011 with a systemd from 2015.
Remove the Plymouth/systemd integration, and let Plymouth's code tell
systemd to print the details.
[0]
http://cgit.freedesktop.org/plymouth/commit/?id=537c16422cd49f1beeaab1ad39846a00018faec1
Signed-off-by: Jasper St. Pierre
Cc: Daniel Drake
Cc: Ray Strode
---
src/core/main.c| 2 +-
src/core/manager.c | 4 +---
src/shared/util.c | 4
src/shared/util.h | 2 --
4 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/src/core/main.c b/src/core/main.c
index 2d393de..dd8b650 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1561,7 +1561,7 @@ int main(int argc, char *argv[]) {
}
if (arg_running_as == SYSTEMD_SYSTEM && !skip_setup) {
-if (arg_show_status > 0 || plymouth_running())
+if (arg_show_status > 0)
status_welcome();
hostname_setup();
diff --git a/src/core/manager.c b/src/core/manager.c
index d33112d..1afd359 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3009,9 +3009,7 @@ static bool manager_get_show_status(Manager *m,
StatusType type) {
if (m->show_status > 0)
return true;
-/* If Plymouth is running make sure we show the status, so
- * that there's something nice to see when people press Esc */
-return plymouth_running();
+return false;
}
void manager_set_first_boot(Manager *m, bool b) {
diff --git a/src/shared/util.c b/src/shared/util.c
index 5cbbe8f..3f3ca90 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -4228,10 +4228,6 @@ bool nulstr_contains(const char*nulstr, const char
*needle) {
return false;
}
-bool plymouth_running(void) {
-return access("/run/plymouth/pid", F_OK) >= 0;
-}
-
char* strshorten(char *s, size_t l) {
assert(s);
diff --git a/src/shared/util.h b/src/shared/util.h
index d229e1e..749bd0e 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -549,8 +549,6 @@ int kill_and_sigcont(pid_t pid, int sig);
bool nulstr_contains(const char*nulstr, const char *needle);
-bool plymouth_running(void);
-
bool hostname_is_valid(const char *s) _pure_;
char* hostname_cleanup(char *s, bool lowercase);
--
2.1.0
___
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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-fsckd: Couldn't connect to plymouth: Connection refused
Le 14/03/2015 21:37, Mikhail Morfikov a écrit : This is the full log I got when I tried to mount the device: Mar 14 20:46:08 morfikownia polkitd(authority=local)[1266]: Registered Authentication Agent for unix-process:11439:94979 (system bus name :1.41 [/usr/bin/pkttyagent --notify-fd 5 --fallback], object path /org/freedesktop/PolicyKit1/AuthenticationAgent, locale en_US.UTF-8) Mar 14 20:46:08 morfikownia systemd[1]: Starting Cryptography Setup for grafi... Mar 14 20:46:08 morfikownia cryptdisks_start[11444]: Starting crypto disk...grafi (starting)... Mar 14 20:46:11 morfikownia cryptdisks_start[11444]: grafi (started)...done. Mar 14 20:46:11 morfikownia systemd[1]: Started Cryptography Setup for grafi. Mar 14 20:46:11 morfikownia systemd[1]: Found device /dev/mapper/grafi. Mar 14 20:46:11 morfikownia systemd[1]: Starting File System Check on /dev/mapper/grafi... Mar 14 20:46:11 morfikownia systemd[1]: Started File System Check Daemon to report status. Mar 14 20:46:11 morfikownia systemd[1]: Starting File System Check Daemon to report status... Mar 14 20:46:12 morfikownia systemd-fsck[11515]: grafi has been mounted 22 times without being checked, check forced. Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:12 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:13 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:13 morfikownia systemd-fsck[11515]: grafi: 21194/1966080 files (4.9% non-contiguous), 7743265/7863808 blocks Mar 14 20:46:13 morfikownia systemd-fsckd[11517]: Couldn't connect to plymouth: Connection refused Mar 14 20:46:13 morfikownia systemd[1]: Started File System Check on /dev/mapper/grafi. Mar 14 20:46:13 morfikownia systemd[1]: Mounting /media/Grafi... Mar 14 20:46:13 morfikownia systemd[1]: Mounted /media/Grafi. Mar 14 20:46:13 morfikownia kernel: EXT4-fs (dm-6): mounted filesystem with ordered data mode. Opts: errors=remount-ro,commit=10 Mar 14 20:46:13 morfikownia polkitd(authority=local)[1266]: Unregistered Authentication Agent for unix-process:11439:94979 (system bus name :1.41, object path /org/freedesktop/PolicyKit1/AuthenticationAgent, locale en_US.UTF-8) (disconnected from bus) That's an encrypted partition, and I open it sometimes after I log into the system because most of the time I don't need it, and I don't want it to be mounted at boot automatically. The device works well after mounting, but what about the systemd-fsckd message? Is there a way to get rid of that? Hey, This message is due to plymouth not running when systemd-fsckd is starts to get some output to print (it will reconnect to plymouth as soon as plymouth is starting). The message is harmless in itself. I've written a patch to only try to connect if the pid plymouth file is present (http://lists.freedesktop.org/archives/systemd-devel/2015-March/029174.html). This one triggered a more profound discussion on systemd-fsckd and I applied Lennart's advice (with this change as well) in the thread including http://lists.freedesktop.org/archives/systemd-devel/2015-March/029269.html. This set of patches is waiting for reviews since. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] path_is_mount_point: handle false positive on some fs
Le 10/03/2015 16:53, Lennart Poettering a écrit : On Mon, 09.03.15 11:27, Didier Roche (didro...@ubuntu.com) wrote: However, some file systems (seems overlayfs at least) would report a major(st_dev) as 0 on directories and not on files. The current path_is_mount_point() fallback logic would thus reports that every files is a mount point. The enclosed patch introduces a second fallback to read /proc/mounts, and ensures there is no shadowing by later mounting of parent directories. This slower path is only used when the 2 first methods failed to determine with assurance if the path is a mount point or not. Humppf, I am not convinced this is really a good idea. I mean, we use name_to_handle_at() precisely for the reason that we don't want to parse /proc/self/mountinfo, since that is really cumbersome if you want to do it properly, given that mounts can obstruct other mounts and symlink/realpath complexity and stuff. Note that the st_dev thing is the traditional way to detect whether one crosses a file system boundary. It's used for this by tools like cp, find, We slightly enhance this by using name_to_handle_at(), so that we can also detect bind mounts from file systems onto themselves. Now, if overlayfs breaks the same file system logic of all other tools, I am not convinced that systemd should not be broken by it too.. It sounds surprising that we should work around this in systemd, but not in all other tools. Or to turn this around: instead of patching systemd the better idea would probably be to teach overlayfs name_to_handle_at() support, so that there's a way to detect the mount id from such a file system in a safe way. Hmm, where precisely did you run into problems with this? The context is bug https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1411140, where systemd-machine-id-commit unit is entering in failed state (the binary handles gracefully the fact that it can't unmount the file) on a default ubuntu live system (which is a squashfs/overlayfs system). This means that a live starts in degraded mode when booted with systemd (mostly a cosmetic issue for sure, but we would rather avoid it). Seeing that the logic to check if a path is a mount point already had a fallback (the traditional way), I thought another one would be acceptable, but I understand that you were ok with the first one as it's the general way tools are detecting that case. I thought as Ryan told that glib was going to a similar approach that this fallback would be a little bit more considered as standard… I agree that the best way would be to get the kernel fixed for this, I talked to our kernel team and it doesn't seem we would gain traction on having name_to_handle_at() support on overlayfs, so quite stuck on this for now. Maybe there is a better option? Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
Le 10/03/2015 11:55, Lennart Poettering a écrit :
On Tue, 10.03.15 08:33, Martin Pitt (martin.p...@ubuntu.com) wrote:
Lennart Poettering [2015-03-09 19:11 +0100]:
Anyway, please look into fixing this, I am kinda relying on patches
for this, as I don't use this myself. Fedora isn't set up for it, nor
do I use a file system that still requires fsck at boot...
Yep, we'll fix those. But for the record, this can be tested easily
anywhere by replacing /usr/sbin/fsck with test/mocks/fsck (I'm on
btrfs myself..)
Hmm, is there also a ply mocker? That would be good!
There is none AFAIK, I'm using plymouth-x11 which is working quite well.
The only difficulty that I realized is that Control+C doesn't work for
cancelling into that X window. Consequently, everytime I changed to "c"
or any other key to test.
Well, again: fsck should *not* 'forward'. It should not be bothered at
all with processing stdout of the actual fsck tool at all. Instead it
should open a AF_UNIX/SOCK_STREAM connection to fsckd, use shutdown()
to close the read side, and then make the resulting fd the fd passed
to fsck's -C parameter. That way, the fsck tool will pass the progress
data *directly* to systemd-fsckd, and systemd-fsck will never see it
at all!
The suggestion is now implemented in the attached patch. Please note
that they rely on 0001-fsckd-Don-t-use-strjoina-on-gettext-call.patch
(just posted a new version) and
0002-fsckd-check-if-plymouth-is-running-before-attempting.patch (didn't
change that one).
It should handle what you had your mind, still enabling cancellation
through SIGTERM while removing the need for intermediates sockets.
Please note that I needed to open in systemd-fsck the socket in the
fork() children part to be able to identify the right fsck pid. I thus
used then an empty cmdline[] slot for this. Another way would be to
build the command line in the fork() children call entirely of course.
Not sure what you would prefer.
As always, I'm opened to any suggestion and hope that will satisfy your
original vision more :)
Cheers,
Didier
>From cdfcfb8490cd07c7b9129e6ce7c382c962c2c4dd Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Wed, 11 Mar 2015 12:58:39 +0100
Subject: [PATCH 3/3] fsckd: directly connect fsck output to systemd-fsckd
* Remove the intermediate structure to handle progress report and cancellation
communication from systemd-fsck to systemd-fsckd.
* Directly connect fsck -C to systemd-fsckd. systemd-fsck doesn't get any
fd connecting from fsck anymore.
* Handle cancellation in systemd-fsckd by directly SIGTERM the fsck process
(which in turn, will make systemd-fsck terminate)
* In case of premature termination of fsck, only ignore the unidentified error
if SIGTERM was sent.
---
src/fsck/fsck.c | 111 ---
src/fsckd/fsckd.c | 154 ++
src/fsckd/fsckd.h | 13 -
3 files changed, 130 insertions(+), 148 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 6e46633..b3dbd29 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "sd-bus.h"
#include "libudev.h"
@@ -130,81 +131,38 @@ static void test_files(void) {
}
-static int process_progress(int fd, pid_t fsck_pid, dev_t device_num) {
-_cleanup_fclose_ FILE *f = NULL;
-usec_t last = 0;
-_cleanup_close_ int fsckd_fd = -1;
+static int make_progress_fd(void) {
static const union sockaddr_union sa = {
.un.sun_family = AF_UNIX,
.un.sun_path = FSCKD_SOCKET_PATH,
};
+int fsck_fd;
-fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
-if (fsckd_fd < 0)
-return log_warning_errno(errno, "Cannot open fsckd socket, we won't report fsck progress: %m");
-if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0)
-return log_warning_errno(errno, "Cannot connect to fsckd socket, we won't report fsck progress: %m");
+fsck_fd = socket(AF_UNIX, SOCK_STREAM, 0);
+if (fsck_fd < 0)
+return log_warning_errno(errno, "Cannot crate fsck socket, we won't report fsck progress: %m");
-f = fdopen(fd, "r");
-if (!f)
-return log_warning_errno(errno, "Cannot connect to fsck, we won't report fsck progress: %m");
-
-while (!feof(f)) {
-int pass;
-size_t buflen;
-size_t cur, max;
-ssize_t r;
-usec_t t;
-_cleanup_free_ char *device = NULL;
-FsckProgress progress;
-FsckdMessage fsckd_message;
-
-if (fscanf(f, &
Re: [systemd-devel] [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
Le 11/03/2015 09:34, Didier Roche a écrit :
Le 11/03/2015 09:29, Martin Pitt a écrit :
Hello all,
Didier Roche [2015-03-10 17:56 +0100]:
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -272,7 +272,7 @@ static int plymouth_send_message(int
plymouth_fd, const char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char
*message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL,
*l10n_cancel_message = NULL;
int r;
As far as I can see, this would free(l10n_cancel_message) on exit, but
you must never free the result of gettext(). So split this into
_cleanup_free_ const char *plymouth_cancel_message = NULL;
const char *l10n_cancel_message;
Indeed (weird I didn't get a double free crash), but the man page says
it's statically allocated. Will fix it and report while bringing up
the other patch with the architecture modification.
Thanks!
Actually, none of then needed a cleanup_free as the second is a strjoina().
Here is the updated patch, thanks!
Didier
>From 0bcaa5707de7e71abcf507454ec98ac4dec8 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 08:58:23 +0100
Subject: [PATCH 1/3] fsckd: Don't use strjoina on gettext() call
---
src/fsckd/fsckd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3c587a3..f24715c 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -272,7 +272,7 @@ static int plymouth_send_message(int plymouth_fd, const char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+const char *plymouth_cancel_message = NULL, *l10n_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -288,7 +288,8 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"));
+l10n_cancel_message = _("Press Ctrl+C to cancel all filesystem checks in progress");
+plymouth_cancel_message = strjoina("fsckd-cancel-msg:", l10n_cancel_message);
r = plymouth_send_message(m->plymouth_fd, plymouth_cancel_message, false);
if (r < 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
Le 11/03/2015 09:29, Martin Pitt a écrit :
Hello all,
Didier Roche [2015-03-10 17:56 +0100]:
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -272,7 +272,7 @@ static int plymouth_send_message(int plymouth_fd, const
char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL,
*l10n_cancel_message = NULL;
int r;
As far as I can see, this would free(l10n_cancel_message) on exit, but
you must never free the result of gettext(). So split this into
_cleanup_free_ const char *plymouth_cancel_message = NULL;
const char *l10n_cancel_message;
Indeed (weird I didn't get a double free crash), but the man page says
it's statically allocated. Will fix it and report while bringing up the
other patch with the architecture modification.
Thanks!
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] path_is_mount_point: handle false positive on some fs
Le 10/03/2015 18:54, Lennart Poettering a écrit : On Tue, 10.03.15 18:01, Didier Roche (didro...@ubuntu.com) wrote: The context is bug https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1411140, where systemd-machine-id-commit unit is entering in failed state (the binary handles gracefully the fact that it can't unmount the file) on a default ubuntu live system (which is a squashfs/overlayfs system). Hmm, it's not clear to me by that bug what precisely fails... Any idea? Maybe it would be better to just handle that failure gracefully... Basically, ConditionPathIsMountPoint=/etc/machine-id is met in the unit and so systemd-machine-id-commit is launched. This one: - executes the double checking path_is_mount_point(etc_machine_id, false) successfully (of course) - find that machine-id fd is a temporary fs (based on the same issue), read the machine-id file content - switch namespace - call unmount(etc_machine_id), which obviously fails as there is nothing to unmount. I don't think we should shadow that failure and not returning in error here, right? Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 4/5] fsckd: check if plymouth is running before attempting connection
Le 10/03/2015 11:44, Lennart Poettering a écrit :
On Tue, 10.03.15 11:34, Didier Roche (didro...@ubuntu.com) wrote:
I think it would make more sense to return 0 when ply isn't running,
and 1 if it is, no?
Did this in the attached patch. Due to this, I needed then to return 1
even if we did not reconnect to plymouth but was already connected,
which slightly break the paradigm of "return 0 if we did nothing, and 1
if the action changed something and is successful".
Is that ok?
Cheers,
Didier
>From 73ce3f737e1211a7d182553cbc55727a04a18d4c Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 10:05:19 +0100
Subject: [PATCH 2/2] fsckd: check if plymouth is running before attempting
connection
---
src/fsckd/fsckd.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f23f272..70abb07 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -231,9 +231,12 @@ static int manager_connect_plymouth(Manager *m) {
union sockaddr_union sa = PLYMOUTH_SOCKET;
int r;
+if (!plymouth_running())
+return 0;
+
/* try to connect or reconnect if sending a message */
if (m->plymouth_fd >= 0)
-return 0;
+return 1;
m->plymouth_fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
if (m->plymouth_fd < 0)
@@ -278,6 +281,9 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
r = manager_connect_plymouth(m);
if (r < 0)
return r;
+/* 0 means that plymouth isn't running, do not send any message yet */
+else if (r == 0)
+return 0;
if (!m->plymouth_cancel_sent) {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
Le 10/03/2015 11:41, Lennart Poettering a écrit :
On Tue, 10.03.15 11:33, Didier Roche (didro...@ubuntu.com) wrote:
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3fc923b..9393379 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -220,7 +220,7 @@ static int manager_connect_plymouth(Manager *m) {
goto fail;
}
-return 1;
+return 0;
Oh, well, this doesn't matter too much I guess, since we don't check
for the precise value, but just < 0 or >= 0...
Returning 1 in this case is actually a bit of a pattern I am trying to
adopt across the codebase: return 0 if all is OK but we didn't do
anything. Return > 1 if all is OK and we actually did something. In
this case this distinction is of course entirely irrelevant, but it
was more of an automatism...
fail:
manager_disconnect_plymouth(m);
@@ -406,10 +406,8 @@ static int manager_new_connection_handler(sd_event_source
*s, int fd, uint32_t r
log_debug("New fsck client connected to fd: %d", new_client_fd);
c = new0(Client, 1);
-if (!c) {
-log_oom();
-return 0;
-}
+if (!c)
+return log_oom();
Well, I think logging and ignoring the OOM condition in this case is a
good idea. THink about the effect of returning an error up here: when
a handler callback returns an error sd-event will disable the event
source automatically. This means if fsckd hits an OOM once here, it
will become completely unresponsive, as it never processes incoming
connections anymore. However, if we simply log, close the connection,
and continue without propagating an error up, we are slightly more
robust: sure, the connection will fail, but subsequent connections
might not...
c->fd = new_client_fd;
new_client_fd = -1;
@@ -417,7 +415,7 @@ static int manager_new_connection_handler(sd_event_source
*s, int fd, uint32_t r
r = sd_event_add_io(m->event, &c->event_source, c->fd, EPOLLIN,
client_progress_handler, c);
if (r < 0) {
log_oom();
-return 0;
+return r;
Same here.
Thanks for the explanations, I dropped that patch. Note that I needed to
do a slight change to the paradigm in the replacement of patch 04_*
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
Le 10/03/2015 11:36, Lennart Poettering a écrit :
On Tue, 10.03.15 11:32, Didier Roche (didro...@ubuntu.com) wrote:
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -258,7 +258,7 @@ static int manager_send_plymouth_message(Manager *m, const
char *message) {
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"));
+plymouth_cancel_message = strjoin("fsckd-cancel-msg:", _("Press
Ctrl+C to cancel all filesystem checks in progress"), NULL);
Well, there's an OOM check missing now, as strjoin() can fail...
That said, I think using strjoina is actually a good idea here, the
string is not unbounded. Hence I think it would be a good idea to
first assign the result of _() to a const char* variable in one step,
and then concat it in a second...
Makes sense. Here is the suggested change.
Didier
PS: just posting a couple of patches from your suggestion, I'll work on
the bigger discussion of passing the fsck pipe to systemd-fsckd later in
the week.
>From ab84557e5840ff0a606b4b6f694b655b85bdcd45 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 08:58:23 +0100
Subject: [PATCH 1/2] fsckd: Don't use strjoina on gettext() call
---
src/fsckd/fsckd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3c587a3..f23f272 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -272,7 +272,7 @@ static int plymouth_send_message(int plymouth_fd, const char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL, *l10n_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -288,7 +288,8 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"));
+l10n_cancel_message = _("Press Ctrl+C to cancel all filesystem checks in progress");
+plymouth_cancel_message = strjoina("fsckd-cancel-msg:", l10n_cancel_message);
r = plymouth_send_message(m->plymouth_fd, plymouth_cancel_message, false);
if (r < 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/5] fsckd: clean up log messages
>From 32c1aec9bddf21b1380eb8f7b801468d3875e2a9 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 10:18:00 +0100
Subject: [PATCH 5/5] fsckd: clean up log messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Avoid double logs printing. Not that we don't return
manager_update_global_progress() to the handler callback as if the console or
plymouth isn't available momentarily, we still desire to handle future
fd progress events if those are available again (like cancellation, reportsâ¦)
---
src/fsckd/fsckd.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 52d69cd..5e5784b 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -354,8 +354,7 @@ static int manager_update_global_progress(Manager *m) {
/* try to connect to plymouth and send message */
r = manager_send_plymouth_message(m, fsck_message);
if (r < 0)
-log_debug("Couldn't send message to plymouth");
-
+return r;
}
return 0;
}
@@ -384,9 +383,7 @@ static int client_progress_handler(sd_event_source *s, int fd, uint32_t revents,
else {
log_warning("Closing bad behaving fsck client connection at fd %d", client->fd);
client_free(client);
-r = manager_update_global_progress(m);
-if (r < 0)
-log_warning_errno(r, "Couldn't update global progress: %m");
+manager_update_global_progress(m);
}
return 0;
}
@@ -410,9 +407,7 @@ static int client_progress_handler(sd_event_source *s, int fd, uint32_t revents,
} else
log_error_errno(r, "Unknown error while trying to read fsck data: %m");
-r = manager_update_global_progress(m);
-if (r < 0)
-log_warning_errno(r, "Couldn't update global progress: %m");
+manager_update_global_progress(m);
return 0;
}
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/5] fsckd: check if plymouth is running before attempting connection
>From fd28f24d9eaa16737cbc8f33b8fe1a806dc291b1 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 10:05:19 +0100
Subject: [PATCH 4/5] fsckd: check if plymouth is running before attempting
connection
---
src/fsckd/fsckd.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f73d23b..52d69cd 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -231,6 +231,9 @@ static int manager_connect_plymouth(Manager *m) {
union sockaddr_union sa = PLYMOUTH_SOCKET;
int r;
+if (!plymouth_running())
+return 1;
+
/* try to connect or reconnect if sending a message */
if (m->plymouth_fd >= 0)
return 0;
@@ -278,6 +281,9 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
r = manager_connect_plymouth(m);
if (r < 0)
return r;
+/* > 0 means that plymouth isn't running, do not send any message yet */
+else if (r > 0)
+return 0;
if (!m->plymouth_cancel_sent) {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/5] fsckd: Reduce the SAK window when writing to console
>From 3e877d1d493476f63fa6af7997914f93b50218bd Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 09:57:38 +0100
Subject: [PATCH 3/5] fsckd: Reduce the SAK window when writing to console
We don't want to keep /dev/console open all the time, but only open it when
needed, to avoid interfering with SAK.
---
src/fsckd/fsckd.c | 66 ---
1 file changed, 39 insertions(+), 27 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 9393379..f73d23b 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -76,12 +76,12 @@ typedef struct Manager {
LIST_HEAD(Client, clients);
unsigned n_clients;
-int clear;
+size_t clear;
int connection_fd;
sd_event_source *connection_event_source;
-FILE *console;
+bool show_status_console;
double percent;
int numdevices;
@@ -99,6 +99,36 @@ static void manager_free(Manager *m);
DEFINE_TRIVIAL_CLEANUP_FUNC(Client*, client_free);
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+static int manager_write_console(Manager *m, const char *message) {
+_cleanup_fclose_ FILE *console = NULL;
+int l;
+size_t j;
+
+assert(m);
+
+if (!m->show_status_console)
+return 0;
+
+/* Reduce the SAK window by opening and closing console on every request */
+console = fopen("/dev/console", "we");
+if (!console)
+return -errno;
+
+if (message) {
+fprintf(console, "\r%s\r%n", message, &l);
+if (m->clear < (size_t)l)
+m->clear = (size_t)l;
+} else {
+fputc('\r', console);
+for (j = 0; j < m->clear; j++)
+fputc(' ', console);
+fputc('\r', console);
+}
+fflush(console);
+
+return 0;
+}
+
static double compute_percent(int pass, size_t cur, size_t max) {
/* Values stolen from e2fsck */
@@ -284,7 +314,7 @@ static int manager_update_global_progress(Manager *m) {
Client *current = NULL;
_cleanup_free_ char *console_message = NULL;
_cleanup_free_ char *fsck_message = NULL;
-int current_numdevices = 0, l = 0, r;
+int current_numdevices = 0, r;
double current_percent = 100;
/* get the overall percentage */
@@ -311,19 +341,15 @@ static int manager_update_global_progress(Manager *m) {
if (asprintf(&fsck_message, "fsckd:%d:%3.1f:%s", m->numdevices, m->percent, console_message) < 0)
return -ENOMEM;
-/* write to console */
-if (m->console) {
-fprintf(m->console, "\r%s\r%n", console_message, &l);
-fflush(m->console);
-}
+r = manager_write_console(m, console_message);
+if (r < 0)
+return r;
/* try to connect to plymouth and send message */
r = manager_send_plymouth_message(m, fsck_message);
if (r < 0)
log_debug("Couldn't send message to plymouth");
-if (l > m->clear)
-m->clear = l;
}
return 0;
}
@@ -435,15 +461,7 @@ static void manager_free(Manager *m) {
return;
/* clear last line */
-if (m->console && m->clear > 0) {
-unsigned j;
-
-fputc('\r', m->console);
-for (j = 0; j < (unsigned) m->clear; j++)
-fputc(' ', m->console);
-fputc('\r', m->console);
-fflush(m->console);
-}
+manager_write_console(m, NULL);
sd_event_source_unref(m->connection_event_source);
safe_close(m->connection_fd);
@@ -453,9 +471,6 @@ static void manager_free(Manager *m) {
manager_disconnect_plymouth(m);
-if (m->console)
-fclose(m->console);
-
sd_event_unref(m->event);
free(m);
@@ -479,11 +494,8 @@ static int manager_new(Manager **ret, int fd) {
if (r < 0)
return r;
-if (access("/run/systemd/show-status", F_OK) >= 0) {
-m->console = fopen("/dev/console", "we");
-if (!m->console)
-return -errno;
-}
+if (access("/run/systemd/show-status", F_OK) >= 0)
+m->show_status_console = true;
r = sd_event_add_io(m->event, &m->connection_event_source, fd, EPOLLIN, manager_new_connection_handler, m);
if (r < 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
>From 689c3c1808bd286ed96d36e4fc7ff875e2477697 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 09:01:11 +0100
Subject: [PATCH 2/5] fsckd: Fix some error return values
Change slightly recent changes introduced by
0b02c7c36dbb6f2ec7434eb8d18e0410ee1cc74c and
d42688ef21a695f8a30b0d899dafdc6ff1ee0973:
- do not return 0 when log_oom() couldn't create the manager.
- returns 0 if we connect successfully to plymouth.
---
src/fsckd/fsckd.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3fc923b..9393379 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -220,7 +220,7 @@ static int manager_connect_plymouth(Manager *m) {
goto fail;
}
-return 1;
+return 0;
fail:
manager_disconnect_plymouth(m);
@@ -406,10 +406,8 @@ static int manager_new_connection_handler(sd_event_source *s, int fd, uint32_t r
log_debug("New fsck client connected to fd: %d", new_client_fd);
c = new0(Client, 1);
-if (!c) {
-log_oom();
-return 0;
-}
+if (!c)
+return log_oom();
c->fd = new_client_fd;
new_client_fd = -1;
@@ -417,7 +415,7 @@ static int manager_new_connection_handler(sd_event_source *s, int fd, uint32_t r
r = sd_event_add_io(m->event, &c->event_source, c->fd, EPOLLIN, client_progress_handler, c);
if (r < 0) {
log_oom();
-return 0;
+return r;
}
LIST_PREPEND(clients, m->clients, c);
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
Le 09/03/2015 19:11, Lennart Poettering a écrit :
So I fixed a number of issues, but there are some bits I am not really
keen to fix, mostly because it's hard for me to test as I don't use a
file system that still requires fsck...
- /dev/console needs to be opened only on demand but not continously,
to keep the SAK window as small as possible.
- A clear regime needs to be enforced: either functions log errors on
their own, or they don't. In the latter case the calling function
needs to log errors instead. Currently some of the functions (like
send_message_plymouth_socket()) log some errors but don't log
others. And sometimes if they currently do log the functions
invoking them log each error a second time! (As is the case for
send_message_plymouth() which calls send_message_plymouth_socket()).
- We should avoid mixing stack allocation calls and function
calls. See man page of alloca() about this. Hence: invoking
strjoina() around gettext calls is *not* OK.
And there's probably more to fix...
Anyway, please look into fixing this, I am kinda relying on patches
for this, as I don't use this myself. Fedora isn't set up for it, nor
do I use a file system that still requires fsck at boot...
Those 3 points should be fixed or at least enhanced with the set of
patches I'm attaching now. Please look in particular on the 02 one as I
changed back some errors values introduced by your recent commits (I'm
unsure if that was just an oversight). I tried to rationalize the log
erroring to be more coherent (and especially avoiding double erroring),
hope this is what you wanted.
I didn't touch to the communication (and so, global flow) from
systemd-fsck to systemd-fsckd point yet as changing it raises quite some
issues as discussed in the other part of the thread (how to ensure the
unit instance is in error if fsck fails or is cancel? How to even send
the cancel message request itself if we don't have any communication back?)
Feel free to point any mistake.
Didier
>From 4d279e1818dd05dccdf4595a5bf1f3dc32c3e8c2 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Tue, 10 Mar 2015 08:58:23 +0100
Subject: [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
---
src/fsckd/fsckd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index dedc828..3fc923b 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -242,7 +242,7 @@ static int plymouth_send_message(int plymouth_fd, const char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -258,7 +258,7 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"));
+plymouth_cancel_message = strjoin("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"), NULL);
r = plymouth_send_message(m->plymouth_fd, plymouth_cancel_message, false);
if (r < 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
Le 10/03/2015 08:33, Martin Pitt a écrit : Lennart Poettering [2015-03-09 19:11 +0100]: Oh, and I am only realizing now that the whole thing doesn't do *at all* what we discussed. The idea was to invoke the actual fsck tools with their stdout connected directly to fsckd. Instead the old systemd-fsck tool still is the one that parses the fsck output and which now simply forwards that info. That didn't actually come up during review, but that sounds like a nice idea! I see two structures: - s-fsck runs fsck and forwards its stdout/err fds to fsckd, and then fsckd does all the parsing. This would also automatically eliminate the intermediate socket handling from above. We will still need the cancel message back socket (so below). Or, more radically: - eliminate the current functionality of s-fsck and just make that send a request to s-fsckd to check a new device name. s-fsckd would then spawn off /usr/sbin/fsck by itself and start parsing its output. That's more complex handling of its children (but not too bad), and completely avoids passing around data through sockets, and s-fsck would be a trivial five-liner. Just a note if we go that path, then systemd-fsck*.service would always be successful as only passing the request, but then, any cancellation of a fsck in progress or worse, any fsck failing won't mark the corresponding systemd-fsck@ instance as failed. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Fsckd: only connect to plymouth if running
Hey,
use the systemd plymouth pid file detection to only try to send messages
to plymouth if present.
This prevent some warning spams which may confuse users as in
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1429171
Cheers,
Didier
>From 9ad48f85f7acc18fe1b5782058a8bb58014a3d16 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 9 Mar 2015 13:44:39 +0100
Subject: [PATCH] Fsckd: only connect to plymouth if running
Ensure that we are not trying to send plymouth messages if the plymouth pid
file is not present.
---
src/fsckd/fsckd.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 834476c..4362e10 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -238,9 +238,11 @@ static int update_global_progress(Manager *m) {
}
/* try to connect to plymouth and send message */
-r = send_message_plymouth(m, fsck_message);
-if (r < 0)
-log_debug("Couldn't send message to plymouth");
+if (plymouth_running()) {
+r = send_message_plymouth(m, fsck_message);
+if (r < 0)
+log_debug("Couldn't send message to plymouth");
+}
if (l > m->clear)
m->clear = l;
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] path_is_mount_point: handle false positive on some fs
Hey,
path_is_mount_point() can report some false positive that a file is a
mount point on some file systems (like overlayfs).
The function tries to call name_to_handle_at(), if this one isn't
supported by the file system, then a fallback using stat() is triggered
on the entity (which can be a file). This compares the parent directory
and entity st_dev.
However, some file systems (seems overlayfs at least) would report a
major(st_dev) as 0 on directories and not on files. The current
path_is_mount_point() fallback logic would thus reports that every files
is a mount point. The enclosed patch introduces a second fallback to
read /proc/mounts, and ensures there is no shadowing by later mounting
of parent directories. This slower path is only used when the 2 first
methods failed to determine with assurance if the path is a mount point
or not.
Note that from what I heard, glib is going to use a similar mechanism.
Also we could on the longer term maybe getting the whole
path_is_mount_point() logic into libmount from util-linux, using
mnt_get_mountpoint() (but this one only use st_dev comparison presently)?
As usual, I'm opened on any modification on this patch.
Cheers,
Didier
>From 47a8d7546f3fa959344431770f75d8e422192ba3 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 9 Mar 2015 09:09:30 +0100
Subject: [PATCH] path_is_mount_point: handle false positive on some fs
path_is_mount_point detection mechanism was wrongly reporting every file
as mount point on file systems like overlayfs:
- name_to_handle_at is not supported
- stat() on directories returns a 0 st_dev (and so different from file stat())
In this case, we fall back to the slower path: directly reading /proc/mounts
and matching any mount point corresponding to the path if present. Ensure that
there isn't any hiding mounts.
---
src/shared/path-util.c | 66 +-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index 12d1ec3..2464569 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -457,7 +457,9 @@ int path_is_mount_point(const char *t, bool allow_symlink) {
union file_handle_union h = FILE_HANDLE_INIT;
int mount_id = -1, mount_id_parent = -1;
-_cleanup_free_ char *parent = NULL;
+_cleanup_free_ char *parent = NULL, *path = NULL;
+_cleanup_fclose_ FILE *f = NULL;
+bool mountpoint_found = false;
struct stat a, b;
int r;
bool nosupp = false;
@@ -533,7 +535,69 @@ fallback:
if (r < 0)
return -errno;
+/* if parent major block is 0, this can be an overlay fs,
+ fallback to reading /proc/mounts */
+if (major(b.st_dev) == 0)
+goto fallback_proc_mounts;
+
return a.st_dev != b.st_dev;
+
+fallback_proc_mounts:
+if (allow_symlink) {
+r = readlink_and_make_absolute(t, &path);
+if (r < 0) {
+if (errno == ENOENT)
+return 0;
+
+return -errno;
+}
+} else
+path = strdup(t);
+
+f = fopen("/proc/mounts", "re");
+if (!f) {
+if (errno == ENOENT)
+return 0;
+return -errno;
+}
+
+for (;;) {
+_cleanup_free_ char *mountpoint = NULL, *unesc_mountpoint = NULL;
+r = fscanf(f,
+ "%*s " /* (1) fs spec */
+ "%ms " /* (2) mount point */
+ "%*[^\n]", /* not interested by other values */
+ &mountpoint);
+if (r != 1) {
+if (r == EOF)
+break;
+if (ferror(f) && errno)
+return -errno;
+
+continue;
+}
+
+unesc_mountpoint = cunescape(mountpoint);
+if (!unesc_mountpoint)
+return -ENOMEM;
+
+/* check that our mount point isn't hidden by a later mount of a parent dir */
+if (mountpoint_found) {
+size_t l = strlen(unesc_mountpoint);
+if (strlen(path) > l &&
+path_startswith(path, unesc_mountpoint) &&
+(unesc_mountpoint[l-1] == '/' || path[l] == '/'))
+mountpoint_found = false;
+}
+
+if (path_equal(path, unesc_mountpoint))
+mountpoint_found = true;
+}
+
+if (mountpoint_found)
+return 1;
+
+
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Le 06/03/2015 16:17, Michael Biebl a écrit : 2015-03-06 11:20 GMT+01:00 Didier Roche : It seems like tmp.mount unit was skipped as nothing declared any explicit dependency against it. What seems to confirm this is that if I add any enabled foo.service which declares After=tmp.mount, or if I add the After= statement to systemd-timesync.service, then I get tmp.mount reliably to start (and it was installed as the journal shows up). Does it make sense? I do have several units which have PrivateTmp=true (cups.service, timesyncd) which *are* started during boot, yet tmp.mount is not being activated. Inspecting the units via systemctl shows e.g. $ systemctl show cups.service -p After -p Requires Requires=basic.target cups.socket -.mount tmp.mount After=cups.socket -.mount system.slice tmp.mount basic.target cups.path systemd-journald.socket Why is tmp.mount then not reliably activated during boot here? Right, that's the question, my feeling is that PrivateTmp=true isn't mapped right away as corresponding to tmp.mount at boot time, and so, as nothing refers to tmp.mount explicitly, systemd doesn't take that unit into account. Then, it's present later (after boot time) and that's why starting or restarting an unit with PrivateTmp=true would then pull it. That would also explains why if you enable a service like foo.service declaring a relationship to tmp.mount (like After=tmp.mount), this one is reliably seen at boot time from systemd, and thus, reliably started, even when booting by any units declaring PrivateTmp=true. Would that be the bug? Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Le 04/03/2015 13:40, Lennart Poettering a écrit : On Wed, 04.03.15 13:19, Didier Roche (didro...@ubuntu.com) wrote: Before=systemd-timesyncd.service foo.service local-fs.target umount.target systemd-timesyncd.service though is condition failed: Condition: start condition failed at Wed 2015-03-04 13:09:09 CET; 3min 10s ago ConditionVirtualization=no was not met So, even if the condition for an unit failed, the Requires= are started? Yes. ConditionXYZ= only shortcuts the executon of the service, all its deps are pulled in. The condition is checked at the time the unit is about to be started, which means that at that time the dependencies usually are fulfilled anyway already. Also see docs about this in the man page. I noted that on boot where the tmpfs isn't mounted, systemd-timesyncd.service stays inactive: Active: inactive (dead) ExecMainStartTimestampMonotonic=0 ExecMainExitTimestampMonotonic=0 and if I try to start it (and we get the condition fail), the Requires (tmp.mount in that case) is started. It seems there are 2 issues: - systemd-timesyncd.service is seldomly activated on boot on those machines. (I'll dive into why) - if an unit as a Condition failing, the Requirements of those units are still activated. Yes, absolutely, see man pages. Thanks for your answers. So, from this, we should see systemd-timesyncd.service always trying to be activated (failing due to conditions) and bringing up tmp.mount unit with it. However, in a vm (and it seems in some systems), we are seeing different behaviors. Let's focus on the vm which has a minimal environment and the easiest to reproduce: $ systemctl show -p RequiredBy tmp.mount RequiredBy=systemd-timesyncd.service and systemd-timesyncd.service is enabled. We can end up with systemd-timesyncd.service inactive because of condition failed, but tmp.mount is inactive as well: $ systemctl status systemd-timesyncd.service ● systemd-timesyncd.service - Network Time Synchronization Loaded: loaded (/lib/systemd/system/systemd-timesyncd.service; enabled; vendor preset: enabled) Drop-In: /lib/systemd/system/systemd-timesyncd.service.d └─disable-with-time-daemon.conf Active: inactive (dead) Condition: start condition failed at Fri 2015-03-06 10:36:21 CET; 29s ago ConditionVirtualization=no was not met Docs: man:systemd-timesyncd.service(8) Mar 06 10:36:21 autopkgtest systemd[1]: ConditionFileIsExecutable=!/usr/sbin/chronyd succeeded for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: ConditionFileIsExecutable=!/usr/sbin/openntpd succeeded for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: ConditionFileIsExecutable=!/usr/sbin/ntpd succeeded for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: ConditionVirtualization=no failed for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: Starting of systemd-timesyncd.service requested but condition failed. Not starting unit. Mar 06 10:36:21 autopkgtest systemd[1]: Job systemd-timesyncd.service/start finished, result=done Mar 06 10:36:21 autopkgtest systemd[1]: Started Network Time Synchronization. $ systemctl status tmp.mount ● tmp.mount - Temporary Directory Loaded: loaded (/lib/systemd/system/tmp.mount; disabled; vendor preset: enabled) Active: inactive (dead) Where: /tmp What: tmpfs Docs: man:hier(7) http://www.freedesktop.org/wiki/Software/systemd/APIFileSystems journald content with debug mode: Mar 06 10:36:21 autopkgtest systemd[1]: ConditionFileIsExecutable=!/usr/sbin/chronyd succeeded for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: ConditionFileIsExecutable=!/usr/sbin/openntpd succeeded for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: ConditionFileIsExecutable=!/usr/sbin/ntpd succeeded for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: ConditionVirtualization=no failed for systemd-timesyncd.service. Mar 06 10:36:21 autopkgtest systemd[1]: Starting of systemd-timesyncd.service requested but condition failed. Not starting unit. Mar 06 10:36:21 autopkgtest systemd[1]: Job systemd-timesyncd.service/start finished, result=done With no mention at all about tmp.mount in the journal (even no "Installed new job tmp.mount/start as …"). Shouldn't we get tmp.mount always activated if I follow correctly what you told (and the man page)? It seems like tmp.mount unit was skipped as nothing declared any explicit dependency against it. What seems to confirm this is that if I add any enabled foo.service which declares After=tmp.mount, or if I add the After= statement to systemd-timesync.service, then I get tmp.mount reliably to start (and it was installed as the journal shows up). Does it make sense? Cheers, Didier PS: we are discussing independently about enabling or not tmp.mount in debian/ubuntu (but it's quite late
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Le 04/03/2015 16:27, Michael Biebl a écrit : 2015-03-04 15:41 GMT+01:00 Lennart Poettering : Well, just removing the symlink is kinda pointless. It might still be pulled in by anything else that implicitly depepends on /tmp. What unit is supposed to pull in tmp.mount explicitly? If a generic unit did that, this sounds like a bug. It seems to be systemd-timesyncd.service. I still don't have an answer why it's sometimes activated in my kvm env (and rightly, with condition virtualized failing) and most of the time none. I'll try to spend some time on that before the end of week. Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Le 04/03/2015 13:40, Lennart Poettering a écrit : On Wed, 04.03.15 13:19, Didier Roche (didro...@ubuntu.com) wrote: Le 04/03/2015 12:49, Lennart Poettering a écrit : On Wed, 04.03.15 09:21, Didier Roche (didro...@ubuntu.com) wrote: Hey, It seems that we discovered an issue if a service declares some relationship with a .mount unit. For instance, having tmp.mount disable (and nothing mounting /tmp as tmpfs in fstab): tmp.mount is enabled statically via the /usr/lib/systemd/system/local-fs.target.wants/tmp.mount symlink. We have a distro-patched in debian to remove this symlink. Note that otherwise, it wouldn't be started only on some boots and not on others, which shows that there is an erratic behavior. Well, it's affected by jobs later queued in. You are using "Conflicts" against the unit, apparently. Now, conflicts has different effects for later queued jobs. depending on the "mode" setting the conflicting job is either removed, or the unit stopped or the job fails. Use "systemctl show tmp.mount" to see by which dependencies it was pulled in. Nice hint! So, on boots where tmp.mount is enabled, here is what systemctl show on the unit gives: RequiredBy=systemd-timesyncd.service Conflicts=umount.target ConflictedBy=foo.service What is this ConflictedBy actually about? Why? Ihave the suspicion you assume conflicts= has different behaviour that it actually means. Ok, giving a little bit more context. So, we don't enable /tmp on tmpfs in debian/ubuntu (the symlink is removed as a distro patch). We had with sysvinit and upstart a failsafe mechanism if / is nearly full: in that case /tmp is mounted as tmpfs and marked as "overflow" (some scripts in ubuntu looks for that name and warn the user), trying to boot the system as far as we can. The idea was to recreate this functionality under systemd (bug: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1392637). * I was first proposing a generator for that: https://launchpadlibrarian.net/199267346/0001-Add-systemd-emergency-tmpfs-to-force-tmpfs-on-tmp.patch which would enable unconditionally (even if manually enabled by the sysadmin) the tmp.mount unit and add the "overflow" tag. * Martin (see bug comments) thinks that the check is too early (on the ro filesystem as our root filesystem is mounted as read only) and maybe there is a fstab mount for /tmp with more spaces. There is also the issue that we are maybe on permanent read only images (like with snappy) and so, no free space, (and then /tmp would be mounted as tmpfs via enabling tmp.mount, but we don't want to mark is as "overflow"). * For those reasons, we tried to rather go with a service started later at boot time doing that check. I tried to do a quick one: http://paste.ubuntu.com/10523961/, and that's where we started to see tmp.mount being activated at some boots, and not on others (which triggered that email). Any suggestion on how we should tackle this? (I don't really like the additional service approach and way more prefer the first declarative approach). Before=systemd-timesyncd.service foo.service local-fs.target umount.target systemd-timesyncd.service though is condition failed: Condition: start condition failed at Wed 2015-03-04 13:09:09 CET; 3min 10s ago ConditionVirtualization=no was not met So, even if the condition for an unit failed, the Requires= are started? Yes. ConditionXYZ= only shortcuts the executon of the service, all its deps are pulled in. The condition is checked at the time the unit is about to be started, which means that at that time the dependencies usually are fulfilled anyway already. Also see docs about this in the man page. I noted that on boot where the tmpfs isn't mounted, systemd-timesyncd.service stays inactive: Active: inactive (dead) ExecMainStartTimestampMonotonic=0 ExecMainExitTimestampMonotonic=0 and if I try to start it (and we get the condition fail), the Requires (tmp.mount in that case) is started. It seems there are 2 issues: - systemd-timesyncd.service is seldomly activated on boot on those machines. (I'll dive into why) - if an unit as a Condition failing, the Requirements of those units are still activated. Yes, absolutely, see man pages. Ok, that makes sense (still need to look at why we are getting in those qemu instances systemd-timesyncd started at boot some times, not on others). Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Le 04/03/2015 12:49, Lennart Poettering a écrit : On Wed, 04.03.15 09:21, Didier Roche (didro...@ubuntu.com) wrote: Hey, It seems that we discovered an issue if a service declares some relationship with a .mount unit. For instance, having tmp.mount disable (and nothing mounting /tmp as tmpfs in fstab): tmp.mount is enabled statically via the /usr/lib/systemd/system/local-fs.target.wants/tmp.mount symlink. We have a distro-patched in debian to remove this symlink. Note that otherwise, it wouldn't be started only on some boots and not on others, which shows that there is an erratic behavior. Also, note that system automatically creates the necessary deps to a mount for a variety of cases when something below that mount is referenced. Disabling a mount unit is not sufficient to not start it, if it is referenced explicitly or implicitly by another unit. Use "systemctl show tmp.mount" to see by which dependencies it was pulled in. Nice hint! So, on boots where tmp.mount is enabled, here is what systemctl show on the unit gives: RequiredBy=systemd-timesyncd.service Conflicts=umount.target ConflictedBy=foo.service Before=systemd-timesyncd.service foo.service local-fs.target umount.target systemd-timesyncd.service though is condition failed: Condition: start condition failed at Wed 2015-03-04 13:09:09 CET; 3min 10s ago ConditionVirtualization=no was not met So, even if the condition for an unit failed, the Requires= are started? I noted that on boot where the tmpfs isn't mounted, systemd-timesyncd.service stays inactive: Active: inactive (dead) ExecMainStartTimestampMonotonic=0 ExecMainExitTimestampMonotonic=0 and if I try to start it (and we get the condition fail), the Requires (tmp.mount in that case) is started. It seems there are 2 issues: - systemd-timesyncd.service is seldomly activated on boot on those machines. (I'll dive into why) - if an unit as a Condition failing, the Requirements of those units are still activated. Is the last behavior expected? Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Le 04/03/2015 09:29, Andrei Borzenkov a écrit : On Wed, Mar 4, 2015 at 11:21 AM, Didier Roche wrote: tmpfs on /tmp type tmpfs (rw) status on tmp.mount: Loaded: loaded (/lib/systemd/system/tmp.mount; disabled; vendor preset: enabled) It says "enabled" here, although I'm not sure what "vendor preset" means. The first one is "disabled", which is the current state. vendor preset is what the preset set it by default, but we don't use it (yet) on debian/ubuntu. Only the first status is important there and confirms that tmp.mount is disabled. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Just for the record: I didn't try on trunk yet, only systemd v219 that we ship in vivid. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Possible bug when a dummy service declares After= and/or Conflicts= a .mount unit?
Hey, It seems that we discovered an issue if a service declares some relationship with a .mount unit. For instance, having tmp.mount disable (and nothing mounting /tmp as tmpfs in fstab): foo.service: [Unit] After=tmp.mount [Service] ExecStart=/bin/echo foo [Install] WantedBy=multi-user.target Then, enable foo.service Many reboots will have /tmp not mounted, but after some reboots (mostly < 5), we can get: tmpfs on /tmp type tmpfs (rw) status on tmp.mount: Loaded: loaded (/lib/systemd/system/tmp.mount; disabled; vendor preset: enabled) Active: active (mounted) since Wed 2015-03-04 08:54:37 CET; 18min ago rebooting systemd with debug logs, only one mention of foo.service, but we clearly see that tmp.mount is executed, even disabled: Mar 04 08:54:37 autopkgtest systemd[1]: Deleting job tmp.mount/stop as dependency of job systemd-timesyncd.service/stop Mar 04 08:54:37 autopkgtest systemd[1]: Deleting job foo.service/start as dependency of job tmp.mount/stop Mar 04 08:54:37 autopkgtest systemd[1]: Installed new job tmp.mount/start as 50 Mar 04 08:54:37 autopkgtest systemd[1]: ConditionPathIsSymbolicLink=!/tmp succeeded for tmp.mount. Mar 04 08:54:37 autopkgtest systemd[1]: About to execute: /bin/mount tmpfs /tmp -n -t tmpfs -o mode=1777,strictatime Mar 04 08:54:37 autopkgtest systemd[1]: Forked /bin/mount as 171 Mar 04 08:54:37 autopkgtest systemd[1]: tmp.mount changed dead -> mounting Mar 04 08:54:37 autopkgtest systemd[1]: Mounting Temporary Directory... Mar 04 08:54:37 autopkgtest systemd[1]: tmp.mount changed mounting -> mounting-done Mar 04 08:54:37 autopkgtest systemd[1]: Job tmp.mount/start finished, result=done Mar 04 08:54:37 autopkgtest systemd[1]: Mounted Temporary Directory. It seems then that any relationship (at least After/Conflicts) to a mount unit from a enabled service will trigger that mount unit to executes. Did anyone else have seen something similar? Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] heads-up: chasing journal(?) related regression in 219 causing boot hang/fail
Le 20/02/2015 15:41, Michael Biebl a écrit : 2015-02-20 15:36 GMT+01:00 Martin Pitt : Hello all, Since we updated to 219 in Ubuntu, several people reported boot failures. Booting hangs a long time after starting D-Bus, in the journal you get a lot of error messages like systemd[1]: Failed to register match for Disconnected message: Connection timed out systemd-logind[749]: Failed to fully start up daemon: Connection timed out dbus[800]: [system] Failed to activate service 'org.freedesktop.PolicyKit1': timed out polkitd isn't running. This causes lots of jobs (logind, NetworkManager, avahi, etc.) to get stuck in an eternal retry loop. Unfortunately reproducing this is a real nuisance, classic heisenbug. I'm now able to trigger it (sometimes) in a VM, but I still haven't found a reliable recipe for reproducing it, so that bisecting just takes ages. I'm keeping debug log, notes, and progress in https://launchpad.net/bugs/1423811 FTR. This is mostly a heads-up for other distros in case they also get reports like this, to shortcut the debugging exercise (I already wasted 7 hours on this, and I'm not even close to the solution). Quite surprisingly it's somewhere in journald. Running 218 with journald from 219 causes the hang, 219 with journald from 218 is fine. I noticed this as well. Interestingly, it only ever happened after applying the fsckd patches and running with plymouth enabled. We get it with "quiet splash" removed as well. It just that it's random on the machine load. We already ruled out the fsckd patch yesterday, but I did retry today again after this comment on my own vms and with systemd/udev/… ubuntu package "218-8ubuntu2" which doesn't contain the fsckd patch (and have the 218 and 219 systemd-fsck writing to /dev/console) + systemd-journal binary copied from 219-1ubuntu1, I was able to reproduce the hang after 15 boots. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/9] fsckd daemon for inter-fsckd communication
Le 18/02/2015 17:00, Martin Pitt a écrit : Ça va Didier, these now apply/build/check fine against master, I tested the binaries on my laptop and in a VM, all review comments were addressed. As discussed with Zbigniew on IRC I landed them now. Thanks a lot Martin for doing this further testing and reviews! A big omission is of course the lack of German translations! :-) (I'll commit them now) You didn't want me to try to add those translations, believe me ;) Danke schön. Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 9/9] Add mock fsck process
>From 59d7202a3755dd73ebad94a20921975330a4ee83 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:46:36 +0100
Subject: [PATCH 9/9] Add mock fsck process
---
test/mocks/fsck | 27 +++
1 file changed, 27 insertions(+)
create mode 100755 test/mocks/fsck
diff --git a/test/mocks/fsck b/test/mocks/fsck
new file mode 100755
index 000..77b50d7
--- /dev/null
+++ b/test/mocks/fsck
@@ -0,0 +1,27 @@
+#!/bin/bash
+fd=0
+
+OPTIND=1
+while getopts "C:aTlM" opt; do
+case "$opt" in
+C)
+fd=$OPTARG
+;;
+\?);;
+esac
+done
+
+shift "$((OPTIND-1))"
+device=$1
+
+echo "Running fake fsck on $device"
+
+declare -a maxpass=(30 5 2 30 60)
+
+for pass in {1..5}; do
+maxprogress=${maxpass[$((pass-1))]}
+for (( current=0; current<=${maxprogress}; current++)); do
+echo "$pass $current $maxprogress $device">&$fd
+sleep 0.1
+done
+done
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 8/9] Add man page and references to it.
>From 9db0d5cd0cc20b899ddf77ccdfcbe214978eb463 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:34:59 +0100 Subject: [PATCH 8/9] Add man page and references to it. Add man page explaining the plymouth theme protocol, usage of the daemon as well as the socket activation part. Adapt existing fsck man page. --- Makefile-man.am| 12 +++ man/systemd-f...@.service.xml | 26 +++ man/systemd-fsckd.service.xml | 162 + units/systemd-fsckd.service.in | 1 + units/systemd-fsckd.socket | 2 +- 5 files changed, 189 insertions(+), 14 deletions(-) create mode 100644 man/systemd-fsckd.service.xml diff --git a/Makefile-man.am b/Makefile-man.am index d0fb9aa..7a9612e 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -66,6 +66,7 @@ MANPAGES += \ man/systemd-efi-boot-generator.8 \ man/systemd-escape.1 \ man/systemd-fsck@.service.8 \ + man/systemd-fsckd.service.8 \ man/systemd-fstab-generator.8 \ man/systemd-getty-generator.8 \ man/systemd-gpt-auto-generator.8 \ @@ -209,6 +210,8 @@ MANPAGES_ALIAS += \ man/systemd-ask-password-wall.service.8 \ man/systemd-fsck-root.service.8 \ man/systemd-fsck.8 \ + man/systemd-fsckd.8 \ + man/systemd-fsckd.socket.8 \ man/systemd-hibernate-resume.8 \ man/systemd-hibernate.service.8 \ man/systemd-hybrid-sleep.service.8 \ @@ -321,6 +324,8 @@ man/systemd-ask-password-wall.path.8: man/systemd-ask-password-console.service.8 man/systemd-ask-password-wall.service.8: man/systemd-ask-password-console.service.8 man/systemd-fsck-root.service.8: man/systemd-fsck@.service.8 man/systemd-fsck.8: man/systemd-fsck@.service.8 +man/systemd-fsckd.8: man/systemd-fsckd.service.8 +man/systemd-fsckd.socket.8: man/systemd-fsckd.service.8 man/systemd-hibernate-resume.8: man/systemd-hibernate-resume@.service.8 man/systemd-hibernate.service.8: man/systemd-suspend.service.8 man/systemd-hybrid-sleep.service.8: man/systemd-suspend.service.8 @@ -601,6 +606,12 @@ man/systemd-fsck-root.service.html: man/systemd-f...@.service.html man/systemd-fsck.html: man/systemd-f...@.service.html $(html-alias) +man/systemd-fsckd.html: man/systemd-fsckd.service.html + $(html-alias) + +man/systemd-fsckd.socket.html: man/systemd-fsckd.service.html + $(html-alias) + man/systemd-hibernate-resume.html: man/systemd-hibernate-res...@.service.html $(html-alias) @@ -1738,6 +1749,7 @@ EXTRA_DIST += \ man/systemd-escape.xml \ man/systemd-firstboot.xml \ man/systemd-f...@.service.xml \ + man/systemd-fsckd.service.xml \ man/systemd-fstab-generator.xml \ man/systemd-getty-generator.xml \ man/systemd-gpt-auto-generator.xml \ diff --git a/man/systemd-f...@.service.xml b/man/systemd-f...@.service.xml index 88e11e8..69b8fdd 100644 --- a/man/systemd-f...@.service.xml +++ b/man/systemd-f...@.service.xml @@ -81,10 +81,10 @@ last check, number of mounts, unclean unmount, etc. systemd-fsck will forward file system -checking progress to the console. If a file system check fails for -a service without nofail, emergency mode is -activated, by isolating to -emergency.target. +checking progress to systemd-fsckd.service +socket. If a file system check fails for a service without +nofail, emergency mode is activated, by isolating +to emergency.target. @@ -125,16 +125,16 @@ See Also systemd1, - fsck8, + fsck8, systemd-quotacheck.service8, - fsck.btrfs8, - fsck.cramfs8, - fsck.ext48, - fsck.fat8, - fsck.hfsplus8, - fsck.minix8, - fsck.ntfs8, - fsck.xfs8 + fsck.btrfs8, + fsck.cramfs8, + fsck.ext48, + fsck.fat8, + fsck.hfsplus8, + fsck.minix8, + fsck.ntfs8, + fsck.xfs8 diff --git a/man/systemd-fsckd.service.xml b/man/systemd-fsckd.service.xml new file mode 100644 index 000..2ad7844 --- /dev/null +++ b/man/systemd-fsckd.service.xml @@ -0,0 +1,162 @@ + + +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";> + +http://www.w3.org/2001/XInclude";> + + +systemd-fsckd.service +systemd + + + +Developer + Didier +Roche +didro...@ubuntu.com + + + + + +systemd-fsckd.service +8 + + + +systemd-fsckd.service +systemd-fsckd.socket +systemd-fsckd +File system check progress reporting + + + +systemd-fsckd.service +systemd-fsckd.socket +/usr/lib/systemd/systemd-fsckd + + + +Description + +systemd-fsckd.service is a service responsible +for receiving file system check progress, and communicating some +consolidated data to console and plymouth (if running). It also handles +possible check cancellations. + +systemd-fsckd receives messages about file +system check progress from systemd-fsck through a +UNIX domain socket. It can display the progress of the least advanced +fsck as well as
Re: [systemd-devel] [PATCH 6/9] Refresh po files
>From 03f05550e81941d2073d151aaedfe3d06c1f95b0 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:23:08 +0100 Subject: [PATCH 6/9] Refresh po files Also, add new plymouth fsckd translated strings in french. Refreshed with "make update-po". --- po/de.po| 11 +++ po/el.po| 11 +++ po/fr.po| 11 +++ po/hu.po| 11 +++ po/it.po| 11 +++ po/pl.po| 11 +++ po/pt_BR.po | 11 +++ po/ru.po| 11 +++ po/sv.po| 11 +++ po/uk.po| 12 10 files changed, 111 insertions(+) diff --git a/po/de.po b/po/de.po index 69c1fb9..0f87024 100644 --- a/po/de.po +++ b/po/de.po @@ -454,5 +454,16 @@ msgstr "" "Legitimierung ist zum Festlegen, ob Netzwerkzeitabgeich eingeschaltet sein " "soll, erforderlich." +#: ../src/fsckd/fsckd.c:186 +msgid "Press Ctrl+C to cancel all filesystem checks in progress" +msgstr "" + +#: ../src/fsckd/fsckd.c:227 +#, c-format +msgid "Checking in progress on %d disk (%3.1f%% complete)" +msgid_plural "Checking in progress on %d disks (%3.1f%% complete)" +msgstr[0] "" +msgstr[1] "" + #~ msgid "Privileged system and service manager access" #~ msgstr "Privilegierter Zugriff auf die System- und Dienstverwaltung" diff --git a/po/el.po b/po/el.po index 8f7a0ed..14ee497 100644 --- a/po/el.po +++ b/po/el.po @@ -402,3 +402,14 @@ msgid "Authentication is required to access the system and service manager." msgstr "" "ÎÏαιÏείÏαι ÏιÏÏοÏοίηÏη για να ÏÏοÏÏελάÏεÏε Ïον διαÏειÏιÏÏή ÏÏ ÏÏήμαÏÎ¿Ï ÎºÎ±Î¹ " "Ï ÏηÏεÏιÏν." + +#: ../src/fsckd/fsckd.c:186 +msgid "Press Ctrl+C to cancel all filesystem checks in progress" +msgstr "" + +#: ../src/fsckd/fsckd.c:227 +#, c-format +msgid "Checking in progress on %d disk (%3.1f%% complete)" +msgid_plural "Checking in progress on %d disks (%3.1f%% complete)" +msgstr[0] "" +msgstr[1] "" diff --git a/po/fr.po b/po/fr.po index 4d4fc6b..8e44e0c 100644 --- a/po/fr.po +++ b/po/fr.po @@ -433,3 +433,14 @@ msgid "" msgstr "" "Authentification requise pour activer ou désactiver la synchronisation de " "l'heure avec le réseau." + +#: ../src/fsckd/fsckd.c:186 +msgid "Press Ctrl+C to cancel all filesystem checks in progress" +msgstr "Appuyez sur Ctrl+C pour annuler toutes vérifications du système de fichier en cours" + +#: ../src/fsckd/fsckd.c:227 +#, c-format +msgid "Checking in progress on %d disk (%3.1f%% complete)" +msgid_plural "Checking in progress on %d disks (%3.1f%% complete)" +msgstr[0] "Vérification en cours sur %d disque (%3.1f%% complété)" +msgstr[1] "Vérification en cours sur %d disques (%3.1f%% complété)" diff --git a/po/hu.po b/po/hu.po index a914b3c..308e03d 100644 --- a/po/hu.po +++ b/po/hu.po @@ -411,3 +411,14 @@ msgstr "A systemd állapotának újratöltése" #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:8 msgid "Authentication is required to reload the systemd state." msgstr "HitelesÃtés szükséges a systemd állapotának újratöltéséhez." + +#: ../src/fsckd/fsckd.c:186 +msgid "Press Ctrl+C to cancel all filesystem checks in progress" +msgstr "" + +#: ../src/fsckd/fsckd.c:227 +#, c-format +msgid "Checking in progress on %d disk (%3.1f%% complete)" +msgid_plural "Checking in progress on %d disks (%3.1f%% complete)" +msgstr[0] "" +msgstr[1] "" diff --git a/po/it.po b/po/it.po index 93a1e79..53cccfe 100644 --- a/po/it.po +++ b/po/it.po @@ -424,3 +424,14 @@ msgstr "Autenticazione richiesta per riavviare lo stato di sistemd." #~ msgid "Privileged system and service manager access" #~ msgstr "Accesso privilegiato per la gestione del sistema e dei servizi" + +#: ../src/fsckd/fsckd.c:186 +msgid "Press Ctrl+C to cancel all filesystem checks in progress" +msgstr "" + +#: ../src/fsckd/fsckd.c:227 +#, c-format +msgid "Checking in progress on %d disk (%3.1f%% complete)" +msgid_plural "Checking in progress on %d disks (%3.1f%% complete)" +msgstr[0] "" +msgstr[1] "" diff --git a/po/pl.po b/po/pl.po index e5987be..cd7dbda 100644 --- a/po/pl.po +++ b/po/pl.po @@ -411,3 +411,14 @@ msgid "" msgstr "" "Wymagane jest uwierzytelnienie, aby kontrolowaÄ, czy wÅÄ czyÄ synchronizacjÄ " "czasu przez sieÄ." + +#: ../src/fsckd/fsckd.c:186 +msgid "Press Ctrl+C to cancel all filesystem checks in progress" +msgstr "" + +#: ../src/fsckd/fsckd.c:227 +#, c-format +msgid "Checking in p
Re: [systemd-devel] [PATCH 5/9] Translate fsckd messages for plymouth
>From 3f029b3549b736f57cfb73c016022f13185187f8 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:12:54 +0100
Subject: [PATCH 5/9] Translate fsckd messages for plymouth
For plymouth themes not supporting i18n (like .script), send translated
messages to display to user, which is equivalent to the sent machine
readable data.
---
po/POTFILES.in| 1 +
src/fsckd/fsckd.c | 10 +++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index b4c1121..70e7594 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -5,3 +5,4 @@ src/locale/org.freedesktop.locale1.policy.in
src/login/org.freedesktop.login1.policy.in
src/machine/org.freedesktop.machine1.policy.in
src/timedate/org.freedesktop.timedate1.policy.in
+src/fsckd/fsckd.c
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index dc00fc6..834476c 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
#include
#include
@@ -182,7 +183,7 @@ static int send_message_plymouth(Manager *m, const char *message) {
if (r < 0)
return log_warning_errno(errno, "Can't send to plymouth cancel key: %m");
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina("fsckd-cancel-msg:", "Press Ctrl+C to cancel all filesystem checks in progress");
+plymouth_cancel_message = strjoina("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"));
r = send_message_plymouth_socket(m->plymouth_fd, plymouth_cancel_message, false);
if (r < 0)
log_warning_errno(r, "Can't send filesystem cancel message to plymouth: %m");
@@ -222,8 +223,10 @@ static int update_global_progress(Manager *m) {
m->numdevices = current_numdevices;
m->percent = current_percent;
-if (asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
-m->numdevices, m->percent) < 0)
+if (asprintf(&console_message,
+ ngettext("Checking in progress on %d disk (%3.1f%% complete)",
+ "Checking in progress on %d disks (%3.1f%% complete)", m->numdevices),
+ m->numdevices, m->percent) < 0)
return -ENOMEM;
if (asprintf(&fsck_message, "fsckd:%d:%3.1f:%s", m->numdevices, m->percent, console_message) < 0)
return -ENOMEM;
@@ -507,6 +510,7 @@ int main(int argc, char *argv[]) {
log_set_target(LOG_TARGET_AUTO);
log_parse_environment();
log_open();
+init_gettext();
r = parse_argv(argc, argv);
if (r <= 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 7/9] Add fsckd service and socket, retarget systemd-fsck
>From 20b16b76f4197604525579cef99c8d6b8b5484e5 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:30:00 +0100 Subject: [PATCH 7/9] Add fsckd service and socket, retarget systemd-fsck systemd-fsckd can be socket-activated by systemd-fsck process. Reflect that in the different unit files. --- Makefile.am| 3 +++ units/.gitignore | 1 + units/systemd-fsck-root.service.in | 3 ++- units/systemd-f...@.service.in | 4 ++-- units/systemd-fsckd.service.in | 16 units/systemd-fsckd.socket | 14 ++ 6 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 units/systemd-fsckd.service.in create mode 100644 units/systemd-fsckd.socket diff --git a/Makefile.am b/Makefile.am index f8c425b..d553339 100644 --- a/Makefile.am +++ b/Makefile.am @@ -494,6 +494,7 @@ dist_systemunit_DATA = \ units/slices.target \ units/system.slice \ units/x-.slice \ + units/systemd-fsckd.socket \ units/systemd-initctl.socket \ units/systemd-shutdownd.socket \ units/syslog.socket \ @@ -545,6 +546,7 @@ nodist_systemunit_DATA = \ units/systemd-kexec.service \ units/systemd-fsck@.service \ units/systemd-fsck-root.service \ + units/systemd-fsckd.service \ units/systemd-machine-id-commit.service \ units/systemd-udevd.service \ units/systemd-udev-trigger.service \ @@ -598,6 +600,7 @@ EXTRA_DIST += \ units/user/systemd-exit.service.in \ units/systemd-f...@.service.in \ units/systemd-fsck-root.service.in \ + units/systemd-fsckd.service.in \ units/systemd-machine-id-commit.service.in \ units/u...@.service.m4.in \ units/debug-shell.service.in \ diff --git a/units/.gitignore b/units/.gitignore index 6fdb629..26ae965 100644 --- a/units/.gitignore +++ b/units/.gitignore @@ -28,6 +28,7 @@ /systemd-firstboot.service /systemd-fsck-root.service /systemd-fsck@.service +/systemd-fsckd.service /systemd-machine-id-commit.service /systemd-halt.service /systemd-hibernate.service diff --git a/units/systemd-fsck-root.service.in b/units/systemd-fsck-root.service.in index 6d76578..f493445 100644 --- a/units/systemd-fsck-root.service.in +++ b/units/systemd-fsck-root.service.in @@ -9,12 +9,13 @@ Description=File System Check on Root Device Documentation=man:systemd-fsck-root.service(8) DefaultDependencies=no +Wants=systemd-fsckd.socket Before=local-fs.target shutdown.target +After=systemd-fsckd.socket ConditionPathIsReadWrite=!/ [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck -StandardOutput=journal+console TimeoutSec=0 diff --git a/units/systemd-f...@.service.in b/units/systemd-f...@.service.in index 857e625..e6d98c0 100644 --- a/units/systemd-f...@.service.in +++ b/units/systemd-f...@.service.in @@ -10,12 +10,12 @@ Description=File System Check on %f Documentation=man:systemd-fsck@.service(8) DefaultDependencies=no BindsTo=%i.device -After=%i.device systemd-fsck-root.service local-fs-pre.target +Wants=systemd-fsckd.socket +After=%i.device systemd-fsck-root.service local-fs-pre.target systemd-fsckd.socket Before=shutdown.target [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck %f -StandardOutput=journal+console TimeoutSec=0 diff --git a/units/systemd-fsckd.service.in b/units/systemd-fsckd.service.in new file mode 100644 index 000..27c325f --- /dev/null +++ b/units/systemd-fsckd.service.in @@ -0,0 +1,16 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=File System Check Daemon to report status +DefaultDependencies=no +Requires=systemd-fsckd.socket +Before=shutdown.target + +[Service] +ExecStart=@rootlibexecdir@/systemd-fsckd +StandardOutput=journal+console diff --git a/units/systemd-fsckd.socket b/units/systemd-fsckd.socket new file mode 100644 index 000..a8994a1 --- /dev/null +++ b/units/systemd-fsckd.socket @@ -0,0 +1,14 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=fsck to fsckd communication Socket +Documentation=man:systemd-fsck@.service(8) man:systemd-fsck-root.service(8) +DefaultDependencies=no + +[Socket] +ListenStream=/run/systemd/fsckd -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck
Le 18/02/2015 00:30, Zbigniew Jędrzejewski-Szmek a écrit :
On Tue, Feb 17, 2015 at 05:26:05PM +0100, Didier Roche wrote:
+r = send_message_plymouth_socket(m->plymouth_fd,
plymouth_cancel_message, false);
+if (r < 0)
+log_warning_errno(r, "Can't send cancel user message to
plymouth: %m");
+} else if (m->numdevices == 0) {
+m->plymouth_cancel_sent = false;
+r = send_message_plymouth_socket(m->plymouth_fd, "", false);
+if (r < 0)
+log_warning_errno(r, "Can't clear cancel user message to
plymouth: %m");
Not *that* important, but those two log_warning_errnos are poorly worded.
If I was a user, I'd be a bit worried if I saw that somebody tried to
cancel me, even if they failed ;)
Ahah, good point! Slightly reworded.
Also "clear ... to".
isn't what I used above? "clear … to"
Well, yes, but still this sentence doesn't make sense.
Changed to "Can't clear plymouth filesystem cancel message: %m"
Didier
>From 08ea7ca56eaac8b5f0e6bcb949ea685a6166e08e Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Thu, 5 Feb 2015 17:08:18 +0100
Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
progress fsck
Try to connect and send to plymouth (if running) some checked report progress,
using direct plymouth protocole.
Update message is the following:
fsckd:::
* num_devices corresponds to the current number of devices being checked (int)
* progress corresponds to the current minimum percentage of all devices being
checked (float, from 0 to 100)
* string is a translated message ready to be displayed by the plymouth theme
displaying the information above. It can be overriden by plymouth themes
supporting i18n.
Grab in fsckd plymouth watch key Control+C, and propagate this cancel request
to systemd-fsck which will terminate fsck.
Send a message to signal to user what key we are grabbing for fsck cancel.
Message is: fsckd-cancel-msg:
Where string is a translated string ready to be displayed by the plymouth theme
indicating that Control+C can be used to cancel current checks. It can be
overriden (matching only fsckd-cancel-msg prefix) for themes supporting i18n.
---
src/fsck/fsck.c | 33
src/fsckd/fsckd.c | 146 +-
src/fsckd/fsckd.h | 4 ++
3 files changed, 173 insertions(+), 10 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 9ecba99..e718534 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -132,7 +132,7 @@ static void test_files(void) {
}
-static int process_progress(int fd, dev_t device_num) {
+static int process_progress(int fd, pid_t fsck_pid, dev_t device_num) {
_cleanup_fclose_ FILE *f = NULL;
usec_t last = 0;
_cleanup_close_ int fsckd_fd = -1;
@@ -153,11 +153,13 @@ static int process_progress(int fd, dev_t device_num) {
while (!feof(f)) {
int pass;
+size_t buflen;
size_t cur, max;
-ssize_t n;
+ssize_t r;
usec_t t;
_cleanup_free_ char *device = NULL;
FsckProgress progress;
+FsckdMessage fsckd_message;
if (fscanf(f, "%i %lu %lu %ms", &pass, &cur, &max, &device) != 4)
break;
@@ -175,9 +177,19 @@ static int process_progress(int fd, dev_t device_num) {
progress.max = max;
progress.pass = pass;
-n = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
-if (n < 0 || (size_t) n < sizeof(FsckProgress))
+r = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
+if (r < 0 || (size_t) r < sizeof(FsckProgress))
log_warning_errno(errno, "Cannot communicate fsck progress to fsckd: %m");
+
+/* get fsckd requests, only read when we have coherent size data */
+r = ioctl(fsckd_fd, FIONREAD, &buflen);
+if (r == 0 && (size_t) buflen >= sizeof(FsckdMessage)) {
+r = recv(fsckd_fd, &fsckd_message, sizeof(FsckdMessage), 0);
+if (r > 0 && fsckd_message.cancel == 1) {
+log_info("Request to cancel fsck from fsckd");
+kill(fsck_pid, SIGTERM);
+}
+}
}
return 0;
@@ -187,6 +199,7 @@ int main(int argc, char *argv[]) {
const char *cmdline[9];
int i = 0, r = EXIT_FAILURE, q;
pid_t pid;
+int progress_rc;
siginfo_t status;
_cleanup_udev_unref_ struct udev *udev = NU
Re: [systemd-devel] [PATCH 4/9] Add gettext support
>From 9227f9a919084f937fd29856935016e0ee9e754e Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Thu, 29 Jan 2015 16:12:58 +0100
Subject: [PATCH 4/9] Add gettext support
---
configure.ac | 1 +
src/shared/util.c | 8
src/shared/util.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/configure.ac b/configure.ac
index 277addb..9a2235b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -73,6 +73,7 @@ AS_IF([test -z "$INTLTOOL_POLICY_RULE"], [
GETTEXT_PACKAGE=systemd
AC_SUBST(GETTEXT_PACKAGE)
+AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$GETTEXT_PACKAGE", [systemd])
AC_PROG_MKDIR_P
AC_PROG_LN_S
diff --git a/src/shared/util.c b/src/shared/util.c
index ba035ca..deb9839 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -25,6 +25,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -74,6 +76,7 @@
#include
#endif
+#include "config.h"
#include "macro.h"
#include "util.h"
#include "ioprio.h"
@@ -5782,6 +5785,11 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
return NULL;
}
+void init_gettext(void) {
+setlocale(LC_ALL, "");
+textdomain(GETTEXT_PACKAGE);
+}
+
bool is_locale_utf8(void) {
const char *set;
static int cached_answer = -1;
diff --git a/src/shared/util.h b/src/shared/util.h
index a83b588..45cb094 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -737,6 +737,8 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
int (*compar) (const void *, const void *, void *),
void *arg);
+#define _(String) gettext (String)
+void init_gettext(void);
bool is_locale_utf8(void);
typedef enum DrawSpecialChar {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/9] systemd-fsck: always connect to systemd-fsckd
>From 6006bedd30530a91c2dee7e3822a8840799752e4 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 16:01:11 +0100
Subject: [PATCH 2/9] systemd-fsck: always connect to systemd-fsckd
Remove the plymouth running or show-status checks from systemd-fsck. Instead,
always connect to systemd-fsckd socket, and let this one decide if we display
progress or not.
---
src/fsck/fsck.c | 12
src/fsckd/fsckd.c | 8 +---
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 6ccb2e7..9ecba99 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -45,7 +45,6 @@
static bool arg_skip = false;
static bool arg_force = false;
-static bool arg_show_progress = false;
static const char *arg_repair = "-a";
static void start_target(const char *target) {
@@ -131,8 +130,6 @@ static void test_files(void) {
}
#endif
-if (access("/run/systemd/show-status", F_OK) >= 0 || plymouth_running())
-arg_show_progress = true;
}
static int process_progress(int fd, dev_t device_num) {
@@ -286,11 +283,10 @@ int main(int argc, char *argv[]) {
log_warning_errno(r, "fsck.%s cannot be used for %s: %m", type, device);
}
-if (arg_show_progress)
-if (pipe(progress_pipe) < 0) {
-log_error_errno(errno, "pipe(): %m");
-return EXIT_FAILURE;
-}
+if (pipe(progress_pipe) < 0) {
+log_error_errno(errno, "pipe(): %m");
+return EXIT_FAILURE;
+}
cmdline[i++] = "/sbin/fsck";
cmdline[i++] = arg_repair;
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 39fe899..6b2eeb0 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -259,9 +259,11 @@ static int manager_new(Manager **ret, int fd) {
return r;
m->connection_fd = fd;
-m->console = fopen("/dev/console", "we");
-if (!m->console)
-return log_warning_errno(errno, "Can't connect to /dev/console: %m");
+if (access("/run/systemd/show-status", F_OK) >= 0) {
+m->console = fopen("/dev/console", "we");
+if (!m->console)
+return log_warning_errno(errno, "Can't connect to /dev/console: %m");
+}
m->percent = 100;
*ret = m;
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/9] fsckd daemon for inter-fsckd communication
Hey,
Martin, while distro-patching ubuntu based on 219 told me that the
patches didn't apply cleanly anymore (some po, man pages and strappenda
-> strjoina changes). I did rebase on latest master then and repost here
the whole set of patches.
Also, we did notice that plymouth (at boot) and plymouth-x11 didn't
react the same way on Control+C, so I changed slightly the keys sent and
how we detect it to match what plymouth at boot expect.
With those, I hope that everything should be good to go.
Cheers,
Didier
>From 9f053f80b916642fd1049bb45fafda456a7484c3 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Wed, 4 Feb 2015 16:42:47 +0100
Subject: [PATCH 1/9] fsckd daemon for inter-fsckd communication
Add systemd-fsckd multiplexer which accepts multiple systemd-fsck
instances to connect to it and sends progress report. systemd-fsckd then
computes and writes to /dev/console the number of devices currently being
checked and the minimum fsck progress. This will be used for interactive
progress report and cancelling in plymouth.
systemd-fsckd stops on idle when no systemd-fsck is connected.
Make the necessary changes to systemd-fsck to connect to the systemd-fsckd
socket.
---
.gitignore | 1 +
Makefile.am| 13 ++
src/fsck/fsck.c| 88
src/fsckd/Makefile | 1 +
src/fsckd/fsckd.c | 404 +
src/fsckd/fsckd.h | 34 +
6 files changed, 484 insertions(+), 57 deletions(-)
create mode 12 src/fsckd/Makefile
create mode 100644 src/fsckd/fsckd.c
create mode 100644 src/fsckd/fsckd.h
diff --git a/.gitignore b/.gitignore
index 75699ca..970b9cb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,6 +77,7 @@
/systemd-evcat
/systemd-firstboot
/systemd-fsck
+/systemd-fsckd
/systemd-fstab-generator
/systemd-getty-generator
/systemd-gnome-ask-password-agent
diff --git a/Makefile.am b/Makefile.am
index 51c6d33..f8c425b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -391,6 +391,7 @@ rootlibexec_PROGRAMS = \
systemd-remount-fs \
systemd-reply-password \
systemd-fsck \
+ systemd-fsckd \
systemd-machine-id-commit \
systemd-ac-power \
systemd-sysctl \
@@ -2357,6 +2358,18 @@ systemd_fsck_LDADD = \
libsystemd-shared.la
# --
+systemd_fsckd_SOURCES = \
+ src/fsckd/fsckd.c \
+ $(NULL)
+
+systemd_fsckd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-label.la \
+ libsystemd-shared.la \
+ libudev-internal.la \
+ $(NULL)
+
+# --
systemd_machine_id_commit_SOURCES = \
src/machine-id-commit/machine-id-commit.c \
src/core/machine-id-setup.c \
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 78ceeb6..6ccb2e7 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "sd-bus.h"
#include "libudev.h"
@@ -39,6 +40,8 @@
#include "fileio.h"
#include "udev-util.h"
#include "path-util.h"
+#include "socket-util.h"
+#include "fsckd/fsckd.h"
static bool arg_skip = false;
static bool arg_force = false;
@@ -132,58 +135,36 @@ static void test_files(void) {
arg_show_progress = true;
}
-static double percent(int pass, unsigned long cur, unsigned long max) {
-/* Values stolen from e2fsck */
-
-static const int pass_table[] = {
-0, 70, 90, 92, 95, 100
+static int process_progress(int fd, dev_t device_num) {
+_cleanup_fclose_ FILE *f = NULL;
+usec_t last = 0;
+_cleanup_close_ int fsckd_fd = -1;
+static const union sockaddr_union sa = {
+.un.sun_family = AF_UNIX,
+.un.sun_path = FSCKD_SOCKET_PATH,
};
-if (pass <= 0)
-return 0.0;
-
-if ((unsigned) pass >= ELEMENTSOF(pass_table) || max == 0)
-return 100.0;
-
-return (double) pass_table[pass-1] +
-((double) pass_table[pass] - (double) pass_table[pass-1]) *
-(double) cur / (double) max;
-}
-
-static int process_progress(int fd) {
-_cleanup_fclose_ FILE *console = NULL, *f = NULL;
-usec_t last = 0;
-bool locked = false;
-int clear = 0;
+fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+if (fsckd_fd < 0)
+return log_warning_errno(errno, "Cannot open fsckd socket, we won't report fsck progress: %m");
+if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0)
+return log_warning_errno(errno, "Cannot connect to fsckd socket, we won't report fsck progress: %m");
f = fdopen(fd, "r");
-if (!f) {
-safe_close(fd);
-retur
Re: [systemd-devel] [PATCH 5/9] Translate fsckd messages for plymouth
Updated to latest (impacted by a change in 3/9)
Didier
>From b0209d20c648f6d275ed72bd78e74e3d48fc4068 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:12:54 +0100
Subject: [PATCH 5/9] Translate fsckd messages for plymouth
For plymouth themes not supporting i18n (like .script), send translated
messages to display to user, which is equivalent to the sent machine
readable data.
---
po/POTFILES.in| 1 +
src/fsckd/fsckd.c | 10 +++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index b4c1121..70e7594 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -5,3 +5,4 @@ src/locale/org.freedesktop.locale1.policy.in
src/login/org.freedesktop.login1.policy.in
src/machine/org.freedesktop.machine1.policy.in
src/timedate/org.freedesktop.timedate1.policy.in
+src/fsckd/fsckd.c
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 30c4f28..2143e1c 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
#include
#include
@@ -182,7 +183,7 @@ static int send_message_plymouth(Manager *m, const char *message) {
if (r < 0)
return log_warning_errno(errno, "Can't send to plymouth cancel key: %m");
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strappenda("fsckd-cancel-msg:", "Press Ctrl+C to cancel all filesystem checks in progress");
+plymouth_cancel_message = strappenda("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all filesystem checks in progress"));
r = send_message_plymouth_socket(m->plymouth_fd, plymouth_cancel_message, false);
if (r < 0)
log_warning_errno(r, "Can't send filesystem cancel message to plymouth: %m");
@@ -222,8 +223,10 @@ static int update_global_progress(Manager *m) {
m->numdevices = current_numdevices;
m->percent = current_percent;
-if (asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
-m->numdevices, m->percent) < 0)
+if (asprintf(&console_message,
+ ngettext("Checking in progress on %d disk (%3.1f%% complete)",
+ "Checking in progress on %d disks (%3.1f%% complete)", m->numdevices),
+ m->numdevices, m->percent) < 0)
return -ENOMEM;
if (asprintf(&fsck_message, "fsckd:%d:%3.1f:%s", m->numdevices, m->percent, console_message) < 0)
return -ENOMEM;
@@ -507,6 +510,7 @@ int main(int argc, char *argv[]) {
log_set_target(LOG_TARGET_AUTO);
log_parse_environment();
log_open();
+init_gettext();
r = parse_argv(argc, argv);
if (r <= 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/9] fsckd daemon for inter-fsckd communication
Le 14/02/2015 17:21, Zbigniew Jędrzejewski-Szmek a écrit :
Thanks for the review, reattached the patch with those fixes.
On Thu, Feb 05, 2015 at 06:06:50PM +0100, Didier Roche wrote:
+if (fsckd_fd < 0) {
+log_warning_errno(errno, "Cannot open fsckd socket, we won't report
fsck progress: %m");
+return -errno;
+}
+if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) +
strlen(sa.un.sun_path)) < 0) {
+log_warning_errno(errno, "Cannot connect to fsckd socket, we won't
report fsck progress: %m");
+return -errno;
Use 'return log_warning_errno(...)'.
+}
f = fdopen(fd, "r");
if (!f) {
-safe_close(fd);
+log_warning_errno(errno, "Cannot connect to fsck, we won't report
fsck progress: %m");
return -errno;
}
Also changed it on this last one below, thanks!
+
+/* ensure we have enough data to read */
+r = ioctl(fd, FIONREAD, &buflen);
+if (r == 0 && buflen != 0 && (size_t) buflen < sizeof(FsckProgress)) {
+if (client->buflen != buflen)
+client->buflen = buflen;
+/* we got twice the same size from a bad behaving client, kick
it off the list */
+else {
Shouldn't this be logged? Seems like this should be detected and fixed
if it happens.
Good suggestion. We don't have that much information on the client than
the fd at this stage (if we never got progress, we don't know device
major and minor numbers), but at least, we'll know that there are bad
behaving client. Added.
Didier
>From fb6d1cc4df6a27684720ccfacdeca3cddadc44ca Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Wed, 4 Feb 2015 16:42:47 +0100
Subject: [PATCH 1/9] fsckd daemon for inter-fsckd communication
Add systemd-fsckd multiplexer which accepts multiple systemd-fsck
instances to connect to it and sends progress report. systemd-fsckd then
computes and writes to /dev/console the number of devices currently being
checked and the minimum fsck progress. This will be used for interactive
progress report and cancelling in plymouth.
systemd-fsckd stops on idle when no systemd-fsck is connected.
Make the necessary changes to systemd-fsck to connect to the systemd-fsckd
socket.
---
.gitignore | 1 +
Makefile.am| 13 ++
src/fsck/fsck.c| 88
src/fsckd/Makefile | 1 +
src/fsckd/fsckd.c | 404 +
src/fsckd/fsckd.h | 34 +
6 files changed, 484 insertions(+), 57 deletions(-)
create mode 12 src/fsckd/Makefile
create mode 100644 src/fsckd/fsckd.c
create mode 100644 src/fsckd/fsckd.h
diff --git a/.gitignore b/.gitignore
index ab6d9d1..9400e75 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/systemd-evcat
/systemd-firstboot
/systemd-fsck
+/systemd-fsckd
/systemd-fstab-generator
/systemd-getty-generator
/systemd-gnome-ask-password-agent
diff --git a/Makefile.am b/Makefile.am
index c463f23..e0e8bc6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -389,6 +389,7 @@ rootlibexec_PROGRAMS = \
systemd-remount-fs \
systemd-reply-password \
systemd-fsck \
+ systemd-fsckd \
systemd-machine-id-commit \
systemd-ac-power \
systemd-sysctl \
@@ -2355,6 +2356,18 @@ systemd_fsck_LDADD = \
libsystemd-shared.la
# --
+systemd_fsckd_SOURCES = \
+ src/fsckd/fsckd.c \
+ $(NULL)
+
+systemd_fsckd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-label.la \
+ libsystemd-shared.la \
+ libudev-internal.la \
+ $(NULL)
+
+# --
systemd_machine_id_commit_SOURCES = \
src/machine-id-commit/machine-id-commit.c \
src/core/machine-id-setup.c \
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 20b7940..f6ed1d1 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "sd-bus.h"
#include "libudev.h"
@@ -39,6 +40,8 @@
#include "fileio.h"
#include "udev-util.h"
#include "path-util.h"
+#include "socket-util.h"
+#include "fsckd/fsckd.h"
static bool arg_skip = false;
static bool arg_force = false;
@@ -132,58 +135,36 @@ static void test_files(void) {
arg_show_progress = true;
}
-static double percent(int pass, unsigned long cur, unsigned long max) {
-/* Values stolen from e2fsck */
-
-static const int pass_table[] = {
-0, 70, 90, 92, 95, 100
+static int process_progress(int fd, dev_t device_num) {
+_cleanup_fclose_ FILE *f = NULL;
+usec_t last = 0;
+_cleanup_close_ in
Re: [systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck
Le 14/02/2015 17:38, Zbigniew Jędrzejewski-Szmek a écrit :
On Thu, Feb 05, 2015 at 06:09:24PM +0100, Didier Roche wrote:
From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Thu, 5 Feb 2015 17:08:18 +0100
Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
progress fsck
Try to connect and send to plymouth (if running) some checked report progress,
using direct plymouth protocole.
Update message is the following:
fsckd:::
* num_devices corresponds to the current number of devices being checked (int)
* progress corresponds to the current minimum percentage of all devices being
checked (float, from 0 to 100)
* string is a translated message ready to be displayed by the plymouth theme
displaying the information above. It can be overriden by plymouth themes
supporting i18n.
Grab in fsckd plymouth watch key Control+C, and propagate this cancel request
to systemd-fsck which will terminate fsck.
Send a message to signal to user what key we are grabbing for fsck cancel.
Message is: fsckd-cancel-msg:
Where string is a translated string ready to be displayed by the plymouth theme
indicating that Control+C can be used to cancel current checks. It can be
overriden (matching only fsckd-cancel-msg prefix) for themes supporting i18n.
---
src/fsck/fsck.c | 33 +
src/fsckd/fsckd.c | 145 +-
src/fsckd/fsckd.h | 5 ++
3 files changed, 173 insertions(+), 10 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index d5aaf9e..1f5a3de 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -132,7 +132,7 @@ static void test_files(void) {
}
-static int process_progress(int fd, dev_t device_num) {
+static int process_progress(int fd, pid_t fsck_pid, dev_t device_num) {
_cleanup_fclose_ FILE *f = NULL;
usec_t last = 0;
_cleanup_close_ int fsckd_fd = -1;
@@ -159,11 +159,13 @@ static int process_progress(int fd, dev_t device_num) {
while (!feof(f)) {
int pass;
+size_t buflen;
size_t cur, max;
-ssize_t n;
+ssize_t r;
usec_t t;
_cleanup_free_ char *device = NULL;
FsckProgress progress;
+FsckdMessage fsckd_message;
if (fscanf(f, "%i %lu %lu %ms", &pass, &cur, &max, &device) != 4)
break;
@@ -181,9 +183,19 @@ static int process_progress(int fd, dev_t device_num) {
progress.max = max;
progress.pass = pass;
-n = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
-if (n < 0 || (size_t) n < sizeof(FsckProgress))
+r = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
+if (r < 0 || (size_t) r < sizeof(FsckProgress))
log_warning_errno(errno, "Cannot communicate fsck progress
to fsckd: %m");
+
+/* get fsckd requests, only read when we have coherent size
data */
+r = ioctl(fsckd_fd, FIONREAD, &buflen);
+if (r == 0 && (size_t) buflen == sizeof(FsckdMessage)) {
Shoudlnt' this be >= ? If two messages are queued, buflen will be
bigger then one message, and we'll never read it.
Indeed, changed it.
+r = recv(fsckd_fd, &fsckd_message,
sizeof(FsckdMessage), 0);
+if (r > 0 && fsckd_message.cancel) {
+log_warning("Request to cancel fsck from
fsckd");
log_notice or log_info. Actually log_info I think, since this is a
legitimate user request.
Done.
+
+n = send(current->fd, &cancel_msg, sizeof(FsckdMessage), 0);
+if (n < 0 || (size_t) n < sizeof(FsckdMessage))
+return log_warning_errno(n, "Cannot send cancel to fsck on (%u, %u):
%m", major(current->devnum), minor(current->devnum));
line wrap please.
Done. Btw, I was wondering if there was any kind of contributor rule
like a style guideline in systemd? I probably just missed it.
+
+if (!m->plymouth_cancel_sent) {
+/* indicate to plymouth that we listen to Ctrl+C */
+r = loop_write(m->plymouth_fd, PLYMOUTH_REQUEST_KEY,
sizeof(PLYMOUTH_REQUEST_KEY), true);
+if (r < 0)
+return log_warning_errno(errno, "Can't send to plymouth
cancel key: %m");
+m->plymouth_cancel_sent = true;
+plymouth_cancel_message = strappenda("fsckd-cancel-msg:", "Press
Ctrl+C to cancel all checks in progress");
...all filesystem checks...
done (will repost the i18n patch + po ones as they are impacted by that
change)
+
Re: [systemd-devel] [PATCH 8/9] Add man page and references to it.
Le 14/02/2015 17:58, Zbigniew Jędrzejewski-Szmek a écrit : On Thu, Feb 05, 2015 at 06:11:24PM +0100, Didier Roche wrote: Thanks for the suggestions, all applied! + +The first time it connects to plymouth, a request +to grab Control+C keypresses is sent, as well as a text message. +When the cancel key is pressed, all running fscks are terminated. +It will also cancel any new connected fsck for the lifetime of +systemd-fsckd. systemd-fsckd will instruct plymouth to grab Control+C keypresses. When the key is pressed, running checks will be terminated. It will also cancel any newly connected fsck instances for the lifetime of systemd-fsckd. This raises an intereseting question. IIUC, fsckd will exit on idle after 30s, losing the information that ^C was pressed. Shouldn't the exit-on-idle be raised to something bigger like 5 minutes, to make this less likely to happen? Good point, 30s of idleness (and so keeping ^C status) sounded enough to me at boot time (the more likely this will occur), most of fsck will chain anyway (first, the root one, and then, the other disks). Do you think about some cases with some .mount unit that are activated afterwards but still while plymouth is shown, like some encrypted /home partitions before the display manager shows up (didn't include the change, happy to do it though)? + +systemd-fsckd passes the +following messages to the theme via libplymouth: Does it actually use libplymouth? argh, sorry, didn't change it. Now done. + + +See Also + + systemd1, + systemd-fsck8, + fsck8, + systemd-quotacheck.service8, Please add something like project='man-pages' (needs to be checked) to citerefentry, so the links are correct in html version. Seems to be project='man-pages' indeed. Done when referring to non systemd manpages (for the fsck* references) and same on man/systemd-f...@.service.xml Cheers, Didier >From 3f888284bb59ecaa8b8fba614161dfabb6a3463e Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:34:59 +0100 Subject: [PATCH 8/9] Add man page and references to it. Add man page explaining the plymouth theme protocol, usage of the daemon as well as the socket activation part. Adapt existing fsck man page. --- Makefile-man.am| 12 +++ man/systemd-f...@.service.xml | 24 +++--- man/systemd-fsckd.service.xml | 162 + units/systemd-fsckd.service.in | 1 + units/systemd-fsckd.socket | 2 +- 5 files changed, 189 insertions(+), 12 deletions(-) create mode 100644 man/systemd-fsckd.service.xml diff --git a/Makefile-man.am b/Makefile-man.am index 105853e..f2e13e8 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -67,6 +67,7 @@ MANPAGES += \ man/systemd-escape.1 \ man/systemd-firstboot.1 \ man/systemd-fsck@.service.8 \ + man/systemd-fsckd.service.8 \ man/systemd-fstab-generator.8 \ man/systemd-getty-generator.8 \ man/systemd-gpt-auto-generator.8 \ @@ -210,6 +211,8 @@ MANPAGES_ALIAS += \ man/systemd-firstboot.service.1 \ man/systemd-fsck-root.service.8 \ man/systemd-fsck.8 \ + man/systemd-fsckd.8 \ + man/systemd-fsckd.socket.8 \ man/systemd-hibernate-resume.8 \ man/systemd-hibernate.service.8 \ man/systemd-hybrid-sleep.service.8 \ @@ -323,6 +326,8 @@ man/systemd-ask-password-wall.service.8: man/systemd-ask-password-console.servic man/systemd-firstboot.service.1: man/systemd-firstboot.1 man/systemd-fsck-root.service.8: man/systemd-fsck@.service.8 man/systemd-fsck.8: man/systemd-fsck@.service.8 +man/systemd-fsckd.8: man/systemd-fsckd.service.8 +man/systemd-fsckd.socket.8: man/systemd-fsckd.service.8 man/systemd-hibernate-resume.8: man/systemd-hibernate-resume@.service.8 man/systemd-hibernate.service.8: man/systemd-suspend.service.8 man/systemd-hybrid-sleep.service.8: man/systemd-suspend.service.8 @@ -606,6 +611,12 @@ man/systemd-fsck-root.service.html: man/systemd-f...@.service.html man/systemd-fsck.html: man/systemd-f...@.service.html $(html-alias) +man/systemd-fsckd.html: man/systemd-fsckd.service.html + $(html-alias) + +man/systemd-fsckd.socket.html: man/systemd-fsckd.service.html + $(html-alias) + man/systemd-hibernate-resume.html: man/systemd-hibernate-res...@.service.html $(html-alias) @@ -1732,6 +1743,7 @@ EXTRA_DIST += \ man/systemd-escape.xml \ man/systemd-firstboot.xml \ man/systemd-f...@.service.xml \ + man/systemd-fsckd.service.xml \ man/systemd-fstab-generator.xml \ man/systemd-getty-generator.xml \ man/systemd-gpt-auto-generator.xml \ diff --git a/man/systemd-f...@.service.xml b/man/systemd-f...@.service.xml index ee66f37..c5b6643 100644 --- a/man/systemd-f...@.service.xml +++ b/man/systemd-f...@.service.xml @@ -87,8 +87,9 @@ check, number of mounts, unclean unmount, etc. systemd-fsck will forward -file system checking progress to the console. If a -
Re: [systemd-devel] [PATCH] preset-transient
Le 05/02/2015 17:11, Dimitri John Ledkov a écrit : Some context for this patch. Hey Dimitri, thanks for working on that. I'm just giving a broader context for everyone who followed the past discussion we had in december/january. This is a followup on our discussion and what we discussed on the "/usr vs /etc" email thread (to give the full context). I would like to support a new preset model, which has the following properties: - distribution shipped defaults are enabled - and are applied to each boot/upgrade - without overriding any user configuration Said differently, the Debian/Ubuntu needs are that distro defaults are migrated. We would prefer an upstream solution for this than the current cache implementation by our install helper tools that Dimitri described below. We should be able to handle defaults per flavors in the ubuntu case (meaning, alternatives are respected, with a priority order based on filename), admins should be able to disable services that are enabled by default on the distro and admins overrides are always respected, even if the distro changed its default policy for a service from disabled to enabled or the contrary. We need for this to work the /dev/null symlink in /etc patch to disable known services that was discussed in the previous thread and agreed on. From the past discussions and during the hackfest, we agreed that presets were the way to go forward, but with slight changes that Dimitri explains below. In many ways it is very similar to existing functionality but not quite possible to achieve all of the above. Thus, I'm introducing a new optional functionality, new unit configuration directory, and new transient-preset configurations. On each boot, if TransientPreset=yes, presets from /usr/lib/systemd/system-preset-transient/*.preset are applied into configuration path /run/systemd/system-preset-transient/. Hum, we told at the sprint that we wanted to be that available for everyone, and not having any conditions. Distros which still desires only the existing behavior would not ship files in *-preset-transient directories. An upgrade tool, sysadmin can repeat that action at appropriate points by also calling: systemctl --runtime preset-all. This command is only for sys admins, on package upgrade/installation/removal (having units file or a new preset file), we would call as a trigger systemctl --daemon-reload, which then, would purge the transient runtime directories and reapply needed changes. If distribution integrates usage of Transient Presets, it gains a few very nice properties. Fresh installations, much upgrades. User/admin modifications are preserved. And there is no additional logic required to maintain separation / diffs between system-defaults and user-modifications. At the moment distributions like Debian (where most things are enabled by default) maintain a complex state in /var/ which tracks which things were distro-enabled before/after the upgrade, as well as whether user/admin has disabled/enabled things before/after the upgrade and try hard to correctly reconcile the correct state for all units. However, with this patch, most of this segregation moves away. We discussed first that this could have gone in /var (this transient layer state is under our control) so that it's not called at every boot, however, /var isn't required at this very early stage to load units from systemd. We would like to not add those in another /etc directory on the broader goal of "empty /etc by default". The remaining part, which is not addressed in this patch series, yet, is the ability to override .wants/ symlink from a higher order configuration directory. That is if the following symlinks are present: /etc/systemd/system/foo.service.wants/bar.service -> /dev/null /usr/lib/systemd/system/foo.service.wants/bar.service -> ../bar.service There is no wants dependency added from foo.service -> bar.service. This bit is discussed in details and agreed upon on the mailing list. (Unwants thread has urls to the messages) Regards, Dimitri. Dimitri John Ledkov (1): Add support for transient presets, applied on every boot. man/systemd-system.conf.xml | 1 + src/core/main.c | 30 +++ src/core/system.conf| 1 + src/core/unit.c | 2 +- src/shared/install.c| 59 ++--- src/shared/install.h| 2 +- src/shared/path-lookup.c| 2 ++ 7 files changed, 76 insertions(+), 21 deletions(-) Right now, I think that we shouldn't have a configuration flag for it, this should apply (as stated above) to any setup, and distros can opt in or out just by shipping those transient preset files. It seems to me that this code has some flaw on first boot: As no preset file (not in the transient directory) is comparable to "enable *", that would mean that on a freshly installed system (without a /etc/machine-id file)
[systemd-devel] [PATCH 9/9] Add mock fsck process
>From 0c33545e512307774cb280cbf83e7b2c3e2137ef Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:46:36 +0100
Subject: [PATCH 9/9] Add mock fsck process
---
test/mocks/fsck | 27 +++
1 file changed, 27 insertions(+)
create mode 100755 test/mocks/fsck
diff --git a/test/mocks/fsck b/test/mocks/fsck
new file mode 100755
index 000..77b50d7
--- /dev/null
+++ b/test/mocks/fsck
@@ -0,0 +1,27 @@
+#!/bin/bash
+fd=0
+
+OPTIND=1
+while getopts "C:aTlM" opt; do
+case "$opt" in
+C)
+fd=$OPTARG
+;;
+\?);;
+esac
+done
+
+shift "$((OPTIND-1))"
+device=$1
+
+echo "Running fake fsck on $device"
+
+declare -a maxpass=(30 5 2 30 60)
+
+for pass in {1..5}; do
+maxprogress=${maxpass[$((pass-1))]}
+for (( current=0; current<=${maxprogress}; current++)); do
+echo "$pass $current $maxprogress $device">&$fd
+sleep 0.1
+done
+done
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 8/9] Add man page and references to it.
>From 2533acf15135d9db18fbd79e63de9a702e3859cc Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:34:59 +0100 Subject: [PATCH 8/9] Add man page and references to it. Add man page explaining the plymouth theme protocol, usage of the daemon as well as the socket activation part. Adapt existing fsck man page. --- Makefile-man.am| 12 +++ man/systemd-f...@.service.xml | 6 +- man/systemd-fsckd.service.xml | 165 + units/systemd-fsckd.service.in | 1 + units/systemd-fsckd.socket | 2 +- 5 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 man/systemd-fsckd.service.xml diff --git a/Makefile-man.am b/Makefile-man.am index 105853e..f2e13e8 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -67,6 +67,7 @@ MANPAGES += \ man/systemd-escape.1 \ man/systemd-firstboot.1 \ man/systemd-fsck@.service.8 \ + man/systemd-fsckd.service.8 \ man/systemd-fstab-generator.8 \ man/systemd-getty-generator.8 \ man/systemd-gpt-auto-generator.8 \ @@ -210,6 +211,8 @@ MANPAGES_ALIAS += \ man/systemd-firstboot.service.1 \ man/systemd-fsck-root.service.8 \ man/systemd-fsck.8 \ + man/systemd-fsckd.8 \ + man/systemd-fsckd.socket.8 \ man/systemd-hibernate-resume.8 \ man/systemd-hibernate.service.8 \ man/systemd-hybrid-sleep.service.8 \ @@ -323,6 +326,8 @@ man/systemd-ask-password-wall.service.8: man/systemd-ask-password-console.servic man/systemd-firstboot.service.1: man/systemd-firstboot.1 man/systemd-fsck-root.service.8: man/systemd-fsck@.service.8 man/systemd-fsck.8: man/systemd-fsck@.service.8 +man/systemd-fsckd.8: man/systemd-fsckd.service.8 +man/systemd-fsckd.socket.8: man/systemd-fsckd.service.8 man/systemd-hibernate-resume.8: man/systemd-hibernate-resume@.service.8 man/systemd-hibernate.service.8: man/systemd-suspend.service.8 man/systemd-hybrid-sleep.service.8: man/systemd-suspend.service.8 @@ -606,6 +611,12 @@ man/systemd-fsck-root.service.html: man/systemd-f...@.service.html man/systemd-fsck.html: man/systemd-f...@.service.html $(html-alias) +man/systemd-fsckd.html: man/systemd-fsckd.service.html + $(html-alias) + +man/systemd-fsckd.socket.html: man/systemd-fsckd.service.html + $(html-alias) + man/systemd-hibernate-resume.html: man/systemd-hibernate-res...@.service.html $(html-alias) @@ -1732,6 +1743,7 @@ EXTRA_DIST += \ man/systemd-escape.xml \ man/systemd-firstboot.xml \ man/systemd-f...@.service.xml \ + man/systemd-fsckd.service.xml \ man/systemd-fstab-generator.xml \ man/systemd-getty-generator.xml \ man/systemd-gpt-auto-generator.xml \ diff --git a/man/systemd-f...@.service.xml b/man/systemd-f...@.service.xml index ee66f37..d366712 100644 --- a/man/systemd-f...@.service.xml +++ b/man/systemd-f...@.service.xml @@ -87,8 +87,9 @@ check, number of mounts, unclean unmount, etc. systemd-fsck will forward -file system checking progress to the console. If a -file system check fails for a service without +file system checking progress to +systemd-fsckd.service +socket. If a file system check fails for a service without nofail, emergency mode is activated, by isolating to emergency.target. @@ -142,6 +143,7 @@ systemd1, fsck8, +systemd-fsckd.service8, systemd-quotacheck.service8, fsck.btrfs8, fsck.cramfs8, diff --git a/man/systemd-fsckd.service.xml b/man/systemd-fsckd.service.xml new file mode 100644 index 000..4a3b60d --- /dev/null +++ b/man/systemd-fsckd.service.xml @@ -0,0 +1,165 @@ + + +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";> + +http://www.w3.org/2001/XInclude";> + + +systemd-fsckd.service +systemd + + + +Developer + Didier +Roche +didro...@ubuntu.com + + + + + +systemd-fsckd.service +8 + + + +systemd-fsckd.service +systemd-fsckd.socket +systemd-fsckd +File system check progress reporting + + + +systemd-fsckd.service +systemd-fsckd.socket +/usr/lib/systemd/systemd-fsckd + + + +Description + +systemd-fsckd.service is a +service responsible for receiving file system check +progress, and communicating some consolidated data +to console and plymouth (if running). It also handles +possible check cancellations. + +systemd-fsckd accepts +systemd-fsck UNIX domain +sockets communication, fetches the lowest progress value of +all fsck running in parallel with the number of devices +being currently checked. It writes the result to +/dev/console if show status is enabled, +and communicates to the user translated strings to plymouth +ready to
[systemd-devel] [PATCH 7/9] Add fsckd service and socket, retarget systemd-fsck
>From 045e99a6865fec2a3e6167d271e01b77236c477d Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:30:00 +0100 Subject: [PATCH 7/9] Add fsckd service and socket, retarget systemd-fsck systemd-fsckd can be socket-activated by systemd-fsck process. Reflect that in the different unit files. --- Makefile.am| 3 +++ units/.gitignore | 1 + units/systemd-fsck-root.service.in | 3 ++- units/systemd-f...@.service.in | 4 ++-- units/systemd-fsckd.service.in | 16 units/systemd-fsckd.socket | 14 ++ 6 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 units/systemd-fsckd.service.in create mode 100644 units/systemd-fsckd.socket diff --git a/Makefile.am b/Makefile.am index e0e8bc6..7cf9f36 100644 --- a/Makefile.am +++ b/Makefile.am @@ -492,6 +492,7 @@ dist_systemunit_DATA = \ units/slices.target \ units/system.slice \ units/x-.slice \ + units/systemd-fsckd.socket \ units/systemd-initctl.socket \ units/systemd-shutdownd.socket \ units/syslog.socket \ @@ -543,6 +544,7 @@ nodist_systemunit_DATA = \ units/systemd-kexec.service \ units/systemd-fsck@.service \ units/systemd-fsck-root.service \ + units/systemd-fsckd.service \ units/systemd-machine-id-commit.service \ units/systemd-udevd.service \ units/systemd-udev-trigger.service \ @@ -596,6 +598,7 @@ EXTRA_DIST += \ units/user/systemd-exit.service.in \ units/systemd-f...@.service.in \ units/systemd-fsck-root.service.in \ + units/systemd-fsckd.service.in \ units/systemd-machine-id-commit.service.in \ units/u...@.service.m4.in \ units/debug-shell.service.in \ diff --git a/units/.gitignore b/units/.gitignore index 6fdb629..26ae965 100644 --- a/units/.gitignore +++ b/units/.gitignore @@ -28,6 +28,7 @@ /systemd-firstboot.service /systemd-fsck-root.service /systemd-fsck@.service +/systemd-fsckd.service /systemd-machine-id-commit.service /systemd-halt.service /systemd-hibernate.service diff --git a/units/systemd-fsck-root.service.in b/units/systemd-fsck-root.service.in index 6d76578..f493445 100644 --- a/units/systemd-fsck-root.service.in +++ b/units/systemd-fsck-root.service.in @@ -9,12 +9,13 @@ Description=File System Check on Root Device Documentation=man:systemd-fsck-root.service(8) DefaultDependencies=no +Wants=systemd-fsckd.socket Before=local-fs.target shutdown.target +After=systemd-fsckd.socket ConditionPathIsReadWrite=!/ [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck -StandardOutput=journal+console TimeoutSec=0 diff --git a/units/systemd-f...@.service.in b/units/systemd-f...@.service.in index 857e625..e6d98c0 100644 --- a/units/systemd-f...@.service.in +++ b/units/systemd-f...@.service.in @@ -10,12 +10,12 @@ Description=File System Check on %f Documentation=man:systemd-fsck@.service(8) DefaultDependencies=no BindsTo=%i.device -After=%i.device systemd-fsck-root.service local-fs-pre.target +Wants=systemd-fsckd.socket +After=%i.device systemd-fsck-root.service local-fs-pre.target systemd-fsckd.socket Before=shutdown.target [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck %f -StandardOutput=journal+console TimeoutSec=0 diff --git a/units/systemd-fsckd.service.in b/units/systemd-fsckd.service.in new file mode 100644 index 000..27c325f --- /dev/null +++ b/units/systemd-fsckd.service.in @@ -0,0 +1,16 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=File System Check Daemon to report status +DefaultDependencies=no +Requires=systemd-fsckd.socket +Before=shutdown.target + +[Service] +ExecStart=@rootlibexecdir@/systemd-fsckd +StandardOutput=journal+console diff --git a/units/systemd-fsckd.socket b/units/systemd-fsckd.socket new file mode 100644 index 000..a8994a1 --- /dev/null +++ b/units/systemd-fsckd.socket @@ -0,0 +1,14 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=fsck to fsckd communication Socket +Documentation=man:systemd-fsck@.service(8) man:systemd-fsck-root.service(8) +DefaultDependencies=no + +[Socket] +ListenStream=/run/systemd/fsckd -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/9] Translate fsckd messages for plymouth
>From e850b1cf9b49918265609da8a6ef2fd4e78541b6 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:12:54 +0100
Subject: [PATCH 5/9] Translate fsckd messages for plymouth
For plymouth themes not supporting i18n (like .script), send translated
messages to display to user, which is equivalent to the sent machine
readable data.
---
po/POTFILES.in| 1 +
src/fsckd/fsckd.c | 10 +++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index b4c1121..70e7594 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -5,3 +5,4 @@ src/locale/org.freedesktop.locale1.policy.in
src/login/org.freedesktop.login1.policy.in
src/machine/org.freedesktop.machine1.policy.in
src/timedate/org.freedesktop.timedate1.policy.in
+src/fsckd/fsckd.c
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f24f8c1..158ef3e 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
#include
#include
@@ -181,7 +182,7 @@ static int send_message_plymouth(Manager *m, const char *message) {
if (r < 0)
return log_warning_errno(errno, "Can't send to plymouth cancel key: %m");
m->plymouth_cancel_sent = true;
-plymouth_cancel_message = strappenda("fsckd-cancel-msg:", "Press Ctrl+C to cancel all checks in progress");
+plymouth_cancel_message = strappenda("fsckd-cancel-msg:", _("Press Ctrl+C to cancel all checks in progress"));
r = send_message_plymouth_socket(m->plymouth_fd, plymouth_cancel_message, false);
if (r < 0)
log_warning_errno(r, "Can't send cancel user message to plymouth: %m");
@@ -221,8 +222,10 @@ static int update_global_progress(Manager *m) {
m->numdevices = current_numdevices;
m->percent = current_percent;
-if (asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
-m->numdevices, m->percent) < 0)
+if (asprintf(&console_message,
+ ngettext("Checking in progress on %d disk (%3.1f%% complete)",
+ "Checking in progress on %d disks (%3.1f%% complete)", m->numdevices),
+ m->numdevices, m->percent) < 0)
return -ENOMEM;
if (asprintf(&fsck_message, "fsckd:%d:%3.1f:%s", m->numdevices, m->percent, console_message) < 0)
return -ENOMEM;
@@ -505,6 +508,7 @@ int main(int argc, char *argv[]) {
log_set_target(LOG_TARGET_AUTO);
log_parse_environment();
log_open();
+init_gettext();
r = parse_argv(argc, argv);
if (r <= 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck
>From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Thu, 5 Feb 2015 17:08:18 +0100
Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
progress fsck
Try to connect and send to plymouth (if running) some checked report progress,
using direct plymouth protocole.
Update message is the following:
fsckd:::
* num_devices corresponds to the current number of devices being checked (int)
* progress corresponds to the current minimum percentage of all devices being
checked (float, from 0 to 100)
* string is a translated message ready to be displayed by the plymouth theme
displaying the information above. It can be overriden by plymouth themes
supporting i18n.
Grab in fsckd plymouth watch key Control+C, and propagate this cancel request
to systemd-fsck which will terminate fsck.
Send a message to signal to user what key we are grabbing for fsck cancel.
Message is: fsckd-cancel-msg:
Where string is a translated string ready to be displayed by the plymouth theme
indicating that Control+C can be used to cancel current checks. It can be
overriden (matching only fsckd-cancel-msg prefix) for themes supporting i18n.
---
src/fsck/fsck.c | 33 +
src/fsckd/fsckd.c | 145 +-
src/fsckd/fsckd.h | 5 ++
3 files changed, 173 insertions(+), 10 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index d5aaf9e..1f5a3de 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -132,7 +132,7 @@ static void test_files(void) {
}
-static int process_progress(int fd, dev_t device_num) {
+static int process_progress(int fd, pid_t fsck_pid, dev_t device_num) {
_cleanup_fclose_ FILE *f = NULL;
usec_t last = 0;
_cleanup_close_ int fsckd_fd = -1;
@@ -159,11 +159,13 @@ static int process_progress(int fd, dev_t device_num) {
while (!feof(f)) {
int pass;
+size_t buflen;
size_t cur, max;
-ssize_t n;
+ssize_t r;
usec_t t;
_cleanup_free_ char *device = NULL;
FsckProgress progress;
+FsckdMessage fsckd_message;
if (fscanf(f, "%i %lu %lu %ms", &pass, &cur, &max, &device) != 4)
break;
@@ -181,9 +183,19 @@ static int process_progress(int fd, dev_t device_num) {
progress.max = max;
progress.pass = pass;
-n = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
-if (n < 0 || (size_t) n < sizeof(FsckProgress))
+r = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
+if (r < 0 || (size_t) r < sizeof(FsckProgress))
log_warning_errno(errno, "Cannot communicate fsck progress to fsckd: %m");
+
+/* get fsckd requests, only read when we have coherent size data */
+r = ioctl(fsckd_fd, FIONREAD, &buflen);
+if (r == 0 && (size_t) buflen == sizeof(FsckdMessage)) {
+r = recv(fsckd_fd, &fsckd_message, sizeof(FsckdMessage), 0);
+if (r > 0 && fsckd_message.cancel) {
+log_warning("Request to cancel fsck from fsckd");
+kill(fsck_pid, SIGTERM);
+}
+}
}
return 0;
@@ -193,6 +205,7 @@ int main(int argc, char *argv[]) {
const char *cmdline[9];
int i = 0, r = EXIT_FAILURE, q;
pid_t pid;
+int progress_rc;
siginfo_t status;
_cleanup_udev_unref_ struct udev *udev = NULL;
_cleanup_udev_device_unref_ struct udev_device *udev_device = NULL;
@@ -335,7 +348,7 @@ int main(int argc, char *argv[]) {
progress_pipe[1] = safe_close(progress_pipe[1]);
if (progress_pipe[0] >= 0) {
-process_progress(progress_pipe[0], st.st_rdev);
+progress_rc = process_progress(progress_pipe[0], pid, st.st_rdev);
progress_pipe[0] = -1;
}
@@ -345,13 +358,14 @@ int main(int argc, char *argv[]) {
goto finish;
}
-if (status.si_code != CLD_EXITED || (status.si_status & ~1)) {
+if (status.si_code != CLD_EXITED || (status.si_status & ~1) || progress_rc != 0) {
-if (status.si_code == CLD_KILLED || status.si_code == CLD_DUMPED)
+/* cancel will kill fsck (but process_progress returns 0) */
+if ((progress_rc != 0 && status.si_code == CLD_KILLED) || status.si_code == CLD_DUMPED)
log_error("fsck terminated by signal %s.", signal_to_string(status.si_status));
else if (status.si_code == CLD_EXITED)
[systemd-devel] [PATCH 4/9] Add gettext support
>From 94bc7097a176c90127a9ff0106e81b4fce6e9ff2 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Thu, 29 Jan 2015 16:12:58 +0100
Subject: [PATCH 4/9] Add gettext support
---
configure.ac | 1 +
src/shared/util.c | 8
src/shared/util.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/configure.ac b/configure.ac
index 12e4ab2..1a2c02c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,7 @@ AS_IF([test -z "$INTLTOOL_POLICY_RULE"], [
GETTEXT_PACKAGE=systemd
AC_SUBST(GETTEXT_PACKAGE)
+AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$GETTEXT_PACKAGE", [systemd])
AC_PROG_MKDIR_P
AC_PROG_LN_S
diff --git a/src/shared/util.c b/src/shared/util.c
index 891182a..dafec01 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -25,6 +25,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -70,6 +72,7 @@
#include
#endif
+#include "config.h"
#include "macro.h"
#include "util.h"
#include "ioprio.h"
@@ -5773,6 +5776,11 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
return NULL;
}
+void init_gettext(void) {
+setlocale(LC_ALL, "");
+textdomain(GETTEXT_PACKAGE);
+}
+
bool is_locale_utf8(void) {
const char *set;
static int cached_answer = -1;
diff --git a/src/shared/util.h b/src/shared/util.h
index ca0c2e5..4450ef5 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -740,6 +740,8 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
int (*compar) (const void *, const void *, void *),
void *arg);
+#define _(String) gettext (String)
+void init_gettext(void);
bool is_locale_utf8(void);
typedef enum DrawSpecialChar {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/9] systemd-fsck: always connect to systemd-fsckd
>From 1579acc911be682cddf4fc91646c4ded231a409a Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 16:01:11 +0100
Subject: [PATCH 2/9] systemd-fsck: always connect to systemd-fsckd
Remove the plymouth running or show-status checks from systemd-fsck. Instead,
always connect to systemd-fsckd socket, and let this one decide if we display
progress or not.
---
src/fsck/fsck.c | 12
src/fsckd/fsckd.c | 8 +---
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 9d9739b..d5aaf9e 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -45,7 +45,6 @@
static bool arg_skip = false;
static bool arg_force = false;
-static bool arg_show_progress = false;
static const char *arg_repair = "-a";
static void start_target(const char *target) {
@@ -131,8 +130,6 @@ static void test_files(void) {
}
#endif
-if (access("/run/systemd/show-status", F_OK) >= 0 || plymouth_running())
-arg_show_progress = true;
}
static int process_progress(int fd, dev_t device_num) {
@@ -292,11 +289,10 @@ int main(int argc, char *argv[]) {
log_warning_errno(r, "fsck.%s cannot be used for %s: %m", type, device);
}
-if (arg_show_progress)
-if (pipe(progress_pipe) < 0) {
-log_error_errno(errno, "pipe(): %m");
-return EXIT_FAILURE;
-}
+if (pipe(progress_pipe) < 0) {
+log_error_errno(errno, "pipe(): %m");
+return EXIT_FAILURE;
+}
cmdline[i++] = "/sbin/fsck";
cmdline[i++] = arg_repair;
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 4a16f3d..45b8d64 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -258,9 +258,11 @@ static int manager_new(Manager **ret, int fd) {
return r;
m->connection_fd = fd;
-m->console = fopen("/dev/console", "we");
-if (!m->console)
-return log_warning_errno(errno, "Can't connect to /dev/console: %m");
+if (access("/run/systemd/show-status", F_OK) >= 0) {
+m->console = fopen("/dev/console", "we");
+if (!m->console)
+return log_warning_errno(errno, "Can't connect to /dev/console: %m");
+}
m->percent = 100;
*ret = m;
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/9] fsckd daemon for inter-fsckd communication
Hey,
Posting the new set of patches for the fsck/plymouth integration,
rebased from all the comments and the systemd event loop system.
This version talks the raw plymouth protocol directly, supporting only
what is needed (sending updates, messages, requesting key listening, get
key events). It's using Control+C as the cancellation key. If plymouth
disconnects and then later respawn, the connection will be taken back.
Same for any new fsck connection incoming after a cancellation (they
will get cancelled right away). The update progress message is always
reflecting the current connection state (they will only disappear once
they are actually cleaned).
As always, I'm opened to any comments.
Cheers,
Didier
>From ac8d6f10768a5bcba0b7932547419637983637b2 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Wed, 4 Feb 2015 16:42:47 +0100
Subject: [PATCH 1/9] fsckd daemon for inter-fsckd communication
Add systemd-fsckd multiplexer which accepts multiple systemd-fsck
instances to connect to it and sends progress report. systemd-fsckd then
computes and writes to /dev/console the number of devices currently being
checked and the minimum fsck progress. This will be used for interactive
progress report and cancelling in plymouth.
systemd-fsckd stops on idle when no systemd-fsck is connected.
Make the necessary changes to systemd-fsck to connect to the systemd-fsckd
socket.
---
.gitignore | 1 +
Makefile.am| 13 ++
src/fsck/fsck.c| 88 +---
src/fsckd/Makefile | 1 +
src/fsckd/fsckd.c | 403 +
src/fsckd/fsckd.h | 34 +
6 files changed, 486 insertions(+), 54 deletions(-)
create mode 12 src/fsckd/Makefile
create mode 100644 src/fsckd/fsckd.c
create mode 100644 src/fsckd/fsckd.h
diff --git a/.gitignore b/.gitignore
index ab6d9d1..9400e75 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/systemd-evcat
/systemd-firstboot
/systemd-fsck
+/systemd-fsckd
/systemd-fstab-generator
/systemd-getty-generator
/systemd-gnome-ask-password-agent
diff --git a/Makefile.am b/Makefile.am
index c463f23..e0e8bc6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -389,6 +389,7 @@ rootlibexec_PROGRAMS = \
systemd-remount-fs \
systemd-reply-password \
systemd-fsck \
+ systemd-fsckd \
systemd-machine-id-commit \
systemd-ac-power \
systemd-sysctl \
@@ -2355,6 +2356,18 @@ systemd_fsck_LDADD = \
libsystemd-shared.la
# --
+systemd_fsckd_SOURCES = \
+ src/fsckd/fsckd.c \
+ $(NULL)
+
+systemd_fsckd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-label.la \
+ libsystemd-shared.la \
+ libudev-internal.la \
+ $(NULL)
+
+# --
systemd_machine_id_commit_SOURCES = \
src/machine-id-commit/machine-id-commit.c \
src/core/machine-id-setup.c \
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 20b7940..9d9739b 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "sd-bus.h"
#include "libudev.h"
@@ -39,6 +40,8 @@
#include "fileio.h"
#include "udev-util.h"
#include "path-util.h"
+#include "socket-util.h"
+#include "fsckd/fsckd.h"
static bool arg_skip = false;
static bool arg_force = false;
@@ -132,58 +135,42 @@ static void test_files(void) {
arg_show_progress = true;
}
-static double percent(int pass, unsigned long cur, unsigned long max) {
-/* Values stolen from e2fsck */
-
-static const int pass_table[] = {
-0, 70, 90, 92, 95, 100
+static int process_progress(int fd, dev_t device_num) {
+_cleanup_fclose_ FILE *f = NULL;
+usec_t last = 0;
+_cleanup_close_ int fsckd_fd = -1;
+static const union sockaddr_union sa = {
+.un.sun_family = AF_UNIX,
+.un.sun_path = FSCKD_SOCKET_PATH,
};
-if (pass <= 0)
-return 0.0;
-
-if ((unsigned) pass >= ELEMENTSOF(pass_table) || max == 0)
-return 100.0;
-
-return (double) pass_table[pass-1] +
-((double) pass_table[pass] - (double) pass_table[pass-1]) *
-(double) cur / (double) max;
-}
-
-static int process_progress(int fd) {
-_cleanup_fclose_ FILE *console = NULL, *f = NULL;
-usec_t last = 0;
-bool locked = false;
-int clear = 0;
+fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+if (fsckd_fd < 0) {
+log_warning_errno(errno, "Cannot open fsckd socket, we won't report fsck progress: %m");
+return -errno;
+}
+if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) {
+log_
Re: [systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
Le 04/02/2015 18:20, Lennart Poettering a écrit : On Wed, 04.02.15 17:40, Didier Roche (didro...@ubuntu.com) wrote: Le 04/02/2015 17:10, Lennart Poettering a écrit : On Wed, 04.02.15 17:05, Didier Roche (didro...@ubuntu.com) wrote: Hey, I rewrote a version of this patch including the feedback on the list. As per IRC discussion, (and after giving up the busy loop for a rewrite with epool), I did rebase it again on sd_event. I'm only proposing there up for review the 2 first patches (without plymouth communication, cancel support, i18n, man pages and the service and socket) so that I don't have to rebase all other 10 patches on a moving ground. Tom just added support for installing timer events with a NULL callback, that trigger event loop exit. I kinda prefer that solution over a new call sd_event_loop() with timeout. sd_event_add_time(event, NULL, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, NULL, NULL); So, it means that I need to reset it after any received activity, is that ok? (as this will be really frequent as each clients in parallel can send a message each 50ms). The goal is to have a global "inactivity" timeout. I didn't see a way to cancel this event source though? Oh, I see, you actually want a real idle logic, not just a normal timer. So far, for daemons like timedated, localed and so on, we are using an idle logic that is implemented in bus_event_loop_with_idle() in src/libsystemd/sd-bus/bus-util.c. It does considerably more than what you need (since it contains all the magic to racefully do exit-on-idle for bus services so that no bus messages are lost). I think the best would be to take inspiration from that code, isolate there basic minimum out of it, without all the dbus logic, and then stick that in your C file. We can generalize such exit-on-idle logic one day, somewhere between sd-bus and sd-event, but that requires considerabe design work, so that we find a generic solution that works for you and also covers this dbus case without hacks. For now it's hence better if you just take inspiration from bus_event_loop_with_idle(), drop all the bus-specific bits, and stick it in your .c code... Making sense. Done and fixed. Thanks a lot Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] fsckd daemon for inter-fsckd communication
Le 04/02/2015 17:24, Zbigniew Jędrzejewski-Szmek a écrit :
Thanks again for the quick review! Fixed if not commented (sorry, some
issues were back due to the refactoring).
On Wed, Feb 04, 2015 at 05:06:45PM +0100, Didier Roche wrote:
+typedef struct Clients {
+struct Manager *manager;
+int fd;
+dev_t devnum;
+size_t cur;
+size_t max;
+int pass;
+double percent;
+size_t buflen;
+
+LIST_FIELDS(struct Clients, clients);
+} Clients;
Would be better to call this Client.
+typedef struct Manager {
+sd_event *event;
+Clients *clients;
But still 'Client *clients' here.
Right, I can't decide (due to the list) what's the best one, what do you
think?
+if (!client)
+return log_oom();
+client->fd = new_client_fd;
+client->manager = m;
+LIST_PREPEND(clients, m->clients, client);
+r = sd_event_add_io(m->event, NULL, client->fd, EPOLLIN,
progress_handler, client);
+if (r < 0) {
+remove_client(&(m->clients), client);
+return r;
+}
I think you can do this in opposite order, and then the clean-up will
not be necessary.
I need to delete the client item and close the new fd (which is part of
remove_client()), and I can't assign the add_io event before the client
instantiated. So, it will be another free() + safe_close() rather then
remove_client(). Does it makes sense still to add this?
+typedef struct FsckProgress {
+dev_t devnum;
+unsigned long cur;
+unsigned long max;
+int pass;
+} FsckProgress;
I think I asked about that before, but not 100% sure: shouldn't those be size_t?
If they are disk offsets, than long is not enough.
Sorry, I only changed it in fsckd.c and fsck.c, done in the passing
struct now.
Cheers,
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
Le 04/02/2015 17:10, Lennart Poettering a écrit : On Wed, 04.02.15 17:05, Didier Roche (didro...@ubuntu.com) wrote: Hey, I rewrote a version of this patch including the feedback on the list. As per IRC discussion, (and after giving up the busy loop for a rewrite with epool), I did rebase it again on sd_event. I'm only proposing there up for review the 2 first patches (without plymouth communication, cancel support, i18n, man pages and the service and socket) so that I don't have to rebase all other 10 patches on a moving ground. Tom just added support for installing timer events with a NULL callback, that trigger event loop exit. I kinda prefer that solution over a new call sd_event_loop() with timeout. sd_event_add_time(event, NULL, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, NULL, NULL); So, it means that I need to reset it after any received activity, is that ok? (as this will be really frequent as each clients in parallel can send a message each 50ms). The goal is to have a global "inactivity" timeout. I didn't see a way to cancel this event source though? Didier That line should be enough to mak an even loop exit after 5s... Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
Hey,
I rewrote a version of this patch including the feedback on the list. As
per IRC discussion, (and after giving up the busy loop for a rewrite
with epool), I did rebase it again on sd_event.
I'm only proposing there up for review the 2 first patches (without
plymouth communication, cancel support, i18n, man pages and the service
and socket) so that I don't have to rebase all other 10 patches on a
moving ground.
I'm opened to any feedback and fixes to do :)
Cheers,
Didier
>From 52d3c92280ec6198a0566bd3a077c0fbb6990782 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Wed, 4 Feb 2015 16:40:44 +0100
Subject: [PATCH 1/2] Add sd_event_loop_timeout to sd_event
sd_event_loop_timeout adds a timeout parameter to sd_event_loop() to timeout
the whole event loop after some time of inactivity.
---
src/libsystemd/sd-event/sd-event.c | 10 +++---
src/systemd/sd-event.h | 1 +
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index f9fa54d..8dd3e16 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -2496,7 +2496,7 @@ _public_ int sd_event_run(sd_event *e, uint64_t timeout) {
return r;
}
-_public_ int sd_event_loop(sd_event *e) {
+_public_ int sd_event_loop_timeout(sd_event *e, uint64_t timeout) {
int r;
assert_return(e, -EINVAL);
@@ -2506,8 +2506,8 @@ _public_ int sd_event_loop(sd_event *e) {
sd_event_ref(e);
while (e->state != SD_EVENT_FINISHED) {
-r = sd_event_run(e, (uint64_t) -1);
-if (r < 0)
+r = sd_event_run(e, timeout);
+if (r < 0 || (r == 0 && timeout != (uint64_t) -1))
goto finish;
}
@@ -2518,6 +2518,10 @@ finish:
return r;
}
+_public_ int sd_event_loop(sd_event *e) {
+return sd_event_loop_timeout(e, (uint64_t) -1);
+}
+
_public_ int sd_event_get_fd(sd_event *e) {
assert_return(e, -EINVAL);
diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h
index 25a10f9..53e2382 100644
--- a/src/systemd/sd-event.h
+++ b/src/systemd/sd-event.h
@@ -90,6 +90,7 @@ int sd_event_prepare(sd_event *e);
int sd_event_wait(sd_event *e, uint64_t timeout);
int sd_event_dispatch(sd_event *e);
int sd_event_run(sd_event *e, uint64_t timeout);
+int sd_event_loop_timeout(sd_event *e, uint64_t timeout);
int sd_event_loop(sd_event *e);
int sd_event_exit(sd_event *e, int code);
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] fsckd daemon for inter-fsckd communication
>From 74291bace60f64efb96287f8170df4c38058cc46 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Wed, 4 Feb 2015 16:42:47 +0100
Subject: [PATCH 2/2] fsckd daemon for inter-fsckd communication
Add systemd-fsckd multiplexer which accepts multiple systemd-fsck
instances to connect to it and sends progress report. systemd-fsckd then
computes and writes to /dev/console the number of devices currently being
checked and the minimum fsck progress. This will be used for interactive
progress report and cancelling in plymouth.
systemd-fsckd stops on idle when no systemd-fsck is connected.
Make the necessary changes to systemd-fsck to connect to the systemd-fsckd
socket.
---
.gitignore | 1 +
Makefile.am| 13 ++
src/fsck/fsck.c| 88 +
src/fsckd/Makefile | 1 +
src/fsckd/fsckd.c | 370 +
src/fsckd/fsckd.h | 34 +
6 files changed, 453 insertions(+), 54 deletions(-)
create mode 12 src/fsckd/Makefile
create mode 100644 src/fsckd/fsckd.c
create mode 100644 src/fsckd/fsckd.h
diff --git a/.gitignore b/.gitignore
index ab6d9d1..9400e75 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/systemd-evcat
/systemd-firstboot
/systemd-fsck
+/systemd-fsckd
/systemd-fstab-generator
/systemd-getty-generator
/systemd-gnome-ask-password-agent
diff --git a/Makefile.am b/Makefile.am
index c463f23..e0e8bc6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -389,6 +389,7 @@ rootlibexec_PROGRAMS = \
systemd-remount-fs \
systemd-reply-password \
systemd-fsck \
+ systemd-fsckd \
systemd-machine-id-commit \
systemd-ac-power \
systemd-sysctl \
@@ -2355,6 +2356,18 @@ systemd_fsck_LDADD = \
libsystemd-shared.la
# --
+systemd_fsckd_SOURCES = \
+ src/fsckd/fsckd.c \
+ $(NULL)
+
+systemd_fsckd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-label.la \
+ libsystemd-shared.la \
+ libudev-internal.la \
+ $(NULL)
+
+# --
systemd_machine_id_commit_SOURCES = \
src/machine-id-commit/machine-id-commit.c \
src/core/machine-id-setup.c \
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 20b7940..9d9739b 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "sd-bus.h"
#include "libudev.h"
@@ -39,6 +40,8 @@
#include "fileio.h"
#include "udev-util.h"
#include "path-util.h"
+#include "socket-util.h"
+#include "fsckd/fsckd.h"
static bool arg_skip = false;
static bool arg_force = false;
@@ -132,58 +135,42 @@ static void test_files(void) {
arg_show_progress = true;
}
-static double percent(int pass, unsigned long cur, unsigned long max) {
-/* Values stolen from e2fsck */
-
-static const int pass_table[] = {
-0, 70, 90, 92, 95, 100
+static int process_progress(int fd, dev_t device_num) {
+_cleanup_fclose_ FILE *f = NULL;
+usec_t last = 0;
+_cleanup_close_ int fsckd_fd = -1;
+static const union sockaddr_union sa = {
+.un.sun_family = AF_UNIX,
+.un.sun_path = FSCKD_SOCKET_PATH,
};
-if (pass <= 0)
-return 0.0;
-
-if ((unsigned) pass >= ELEMENTSOF(pass_table) || max == 0)
-return 100.0;
-
-return (double) pass_table[pass-1] +
-((double) pass_table[pass] - (double) pass_table[pass-1]) *
-(double) cur / (double) max;
-}
-
-static int process_progress(int fd) {
-_cleanup_fclose_ FILE *console = NULL, *f = NULL;
-usec_t last = 0;
-bool locked = false;
-int clear = 0;
+fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+if (fsckd_fd < 0) {
+log_warning_errno(errno, "Cannot open fsckd socket, we won't report fsck progress: %m");
+return -errno;
+}
+if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) {
+log_warning_errno(errno, "Cannot connect to fsckd socket, we won't report fsck progress: %m");
+return -errno;
+}
f = fdopen(fd, "r");
if (!f) {
-safe_close(fd);
+log_warning_errno(errno, "Cannot connect to fsck, we won't report fsck progress: %m");
return -errno;
}
-console = fopen("/dev/console", "we");
-if (!console)
-return -ENOMEM;
-
while (!feof(f)) {
-int pass, m;
-unsigned long cur, max;
-_cleanup_free_ char *device = NULL;
-double p;
+
Re: [systemd-devel] [PATCH 01/12] fsckd daemon for inter-fsckd communication
Le 31/01/2015 01:34, Zbigniew Jędrzejewski-Szmek a écrit :
On Thu, Jan 29, 2015 at 06:37:53PM +0100, Didier Roche wrote:
Le 28/01/2015 15:37, Zbigniew Jędrzejewski-Szmek a écrit :
On Wed, Jan 28, 2015 at 02:20:40PM +0100, Didier Roche wrote:
\ No newline at end of file
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
new file mode 100644
index 000..3059c68
--- /dev/null
+++ b/src/fsckd/fsckd.c
@@ -0,0 +1,295 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+}
+
+static int handle_requests(int socket_fd) {
+Clients *first = NULL;
+usec_t last_activity = 0;
+int numdevices = 0, clear = 0;
+double percent = 100;
+_cleanup_fclose_ FILE *console = NULL;
NULL not necessary (and you don't have it in similar circumstances above ;))
So, after the discussion on the mailing list and to be more future
proof, I went to recheck, but I'm afraid that I didn't find any
place where I have some _cleanup_* without initializing to NULL? Did
I get it wrong?
fsckd_fd variable.
Right, done with -1 as init. Thanks!
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 04/12] Add some plymouth functionality to connect and send, messages
Le 31/01/2015 01:44, Zbigniew Jędrzejewski-Szmek a écrit :
On Thu, Jan 29, 2015 at 06:42:42PM +0100, Didier Roche wrote:
Le 28/01/2015 15:44, Zbigniew Jędrzejewski-Szmek a écrit :
On Wed, Jan 28, 2015 at 02:22:04PM +0100, Didier Roche wrote:
bool plymouth_running(void) {
return access("/run/plymouth/pid", F_OK) >= 0;
}
+
+#ifdef HAVE_PLYMOUTH
+bool plymouth_connect(void) {
Is there a particular reason why this cannot return a normal int code?
No reason, I changed to 0 for connected, -1 for can't connect.
No, please return a real return code (-ENOMEM on the allocation failure
in this case). -1 is EPERM.
Done, (even if I think that particular function is going to change with
the libplymouth dep removal.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 04/12] Add some plymouth functionality to connect and send, messages
Le 31/01/2015 01:39, Zbigniew Jędrzejewski-Szmek a écrit : On Thu, Jan 29, 2015 at 06:43:22PM +0100, Didier Roche wrote: Le 28/01/2015 21:22, Lennart Poettering a écrit : On Wed, 28.01.15 14:22, Didier Roche (didro...@ubuntu.com) wrote: # -- +have_plymouth=no +AC_ARG_ENABLE(plymouth, AS_HELP_STRING([--disable-plymouth], [disable plymouth integration])) +if test "x$enable_plymouth" != "xno"; then +PKG_CHECK_MODULES([PLYMOUTH], [ply-boot-client >= 0.8.0], +[AC_DEFINE(HAVE_PLYMOUTH, 1, [Define if plymouth is available]) have_plymouth=yes], have_plymouth=no) +if test "x$have_plymouth" = xno -a "x$enable_plymouth" = xyes; then +AC_MSG_ERROR([*** plymouth integration requested but libraries not found]) +fi +fi +AM_CONDITIONAL(HAVE_PLYMOUTH, [test "$have_plymouth" = "yes"]) Hmm, I am bit concerned about adding this dependency. So far we managed to talk to plymouth without using its library, and I am really not sure we should start doing so now. So far the messages to send were so simply that it really wasn't worth the effort to use the full library. This is doable for the first part, similar to what src/tty-ask-password-agent/tty-ask-password-agent.c is doing (using the socket directly to send update and message to it). I'm quite unsure "watch and get key events" part as looking at libplymouth code, this seems quite more complex as a protocol to achieve. If you feel that needs to be done anyway, I can look deeper at this if you really feel we should reimplement libplymouth protocol rathen than having an optional dep on it. plymouth-core-libs are 200kb in Fedora. I wouldn't sweat it too much. We discussed it during the hackfest, and got thanks to Lennart's strace suggestion the protocol figured out, will work on that before resending the patch list then (I hope, this week if I get time to look at this). Didier Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 12/12] Add mock fsck process
Le 28/01/2015 17:13, Martin Pitt a écrit :
Zbigniew Jędrzejewski-Szmek [2015-01-28 16:08 +0100]:
+#!/bin/bash
I think you can change this to /bin/sh, I don't see any bashisms.
+declare -a maxpass=(30 5 2 30 60)
I do :-) POSIX shell doesn't have arrays AFAIK, and declare -a
definitively doesn't work in e. g. dash.
+for pass in {1..5}; do
+maxprogress=${maxpass[$((pass-1))]}
That said, if using POSIX shell is a concern (probably not for this
little test dummy), this could certainly be rewritten to something
like
pass=0
for maxprogress in 30 5 2 30 60; do
pass=$((pass+1))
which isn't really more complicated and avoids arrays.
Yeah, there are some bashims. If getting POSIX shell is important for
the mock, I'm happy to do the necessary changes.
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 11/12] Add man page and references to it.
Le 28/01/2015 16:06, Zbigniew Jędrzejewski-Szmek a écrit : On Wed, Jan 28, 2015 at 02:24:55PM +0100, Didier Roche wrote: From 6b13d8fb248bf4176f1ad7e1d4736683462bf196 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:34:59 +0100 Subject: [PATCH 11/12] Add man page and references to it. --- /dev/null +++ b/man/systemd-fsckd.service.xml @@ -0,0 +1,170 @@ + + +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";> + +http://www.w3.org/2001/XInclude";> Please use 2ch indentation for new man pages. Converted to 2 characters for indentation. +progress, and communicating some consolidated data +to console and plymouth (if running). It also handles +possible check cancellations. +systemd-fsck-root.service or +systemd-fsck@.service will get the +progress from fsck and send their individual progress to +systemd-fsckd, through socket activation +by systemd-fsckd.socket. I think we don't need this kind of detail in the man page. It might change anyway. Removed then! +Progress update, sent as a plymouth update message: + fsckd:<num_devices>:<progress>:<string> + + + <num_devices> + the current number of devices + being checked (int) + + + <progress> + the current minimum percentage of + all devices being checking (float, from 0 to 100) + + + <string> + a translated message ready to be displayed + by the plymouth theme displaying the data above. It can be overriden + by themes supporting i18n. + + + + +Cancel message, sent as a traditional plymouth message: + fsckd-cancel-msg:<string> + + + <strings> + a translated string ready to be displayed + by the plymouth theme indicating that c or C can be used to cancel + current checks. It can be overriden (matching only + fsckd-cancel-msg prefix) + by themes supporting i18n. + + + This is very detailed too, but it is OK, we don't really have a good place for this kind of documentation. Yeah, some API for plymouth theme authors. I didn't find a better place (or the systemd wiki?) Thanks again for the detailed rereading :) Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 10/12] Add fsckd service and socket, retarget systemd-fsck
Le 28/01/2015 16:00, Zbigniew Jędrzejewski-Szmek a écrit : On Wed, Jan 28, 2015 at 02:24:32PM +0100, Didier Roche wrote: From 000f1b6ff4f5f80a2a13309590d255de6d6526ec Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:30:00 +0100 Subject: [PATCH 10/12] Add fsckd service and socket, retarget systemd-fsck systemd-fsckd can be socket-activated by systemd-fsck process. Reflect that in the different unit files. --- Makefile.am| 3 +++ units/.gitignore | 1 + units/systemd-fsck-root.service.in | 4 +++- units/systemd-f...@.service.in | 5 +++-- units/systemd-fsckd.service.in | 16 units/systemd-fsckd.socket | 15 +++ 6 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 units/systemd-fsckd.service.in create mode 100644 units/systemd-fsckd.socket diff --git a/Makefile.am b/Makefile.am index a9d61f1..1eeba4f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -492,6 +492,7 @@ dist_systemunit_DATA = \ units/slices.target \ units/system.slice \ units/x-.slice \ + units/systemd-fsckd.socket \ units/systemd-initctl.socket \ units/systemd-shutdownd.socket \ units/syslog.socket \ @@ -543,6 +544,7 @@ nodist_systemunit_DATA = \ units/systemd-kexec.service \ units/systemd-fsck@.service \ units/systemd-fsck-root.service \ + units/systemd-fsckd.service \ units/systemd-machine-id-commit.service \ units/systemd-udevd.service \ units/systemd-udev-trigger.service \ @@ -596,6 +598,7 @@ EXTRA_DIST += \ units/user/systemd-exit.service.in \ units/systemd-f...@.service.in \ units/systemd-fsck-root.service.in \ + units/systemd-fsckd.service.in \ units/systemd-machine-id-commit.service.in \ units/u...@.service.m4.in \ units/debug-shell.service.in \ diff --git a/units/.gitignore b/units/.gitignore index 6fdb629..26ae965 100644 --- a/units/.gitignore +++ b/units/.gitignore @@ -28,6 +28,7 @@ /systemd-firstboot.service /systemd-fsck-root.service /systemd-fsck@.service +/systemd-fsckd.service /systemd-machine-id-commit.service /systemd-halt.service /systemd-hibernate.service diff --git a/units/systemd-fsck-root.service.in b/units/systemd-fsck-root.service.in index 6d76578..bd3bdbc 100644 --- a/units/systemd-fsck-root.service.in +++ b/units/systemd-fsck-root.service.in @@ -9,12 +9,14 @@ Description=File System Check on Root Device Documentation=man:systemd-fsck-root.service(8) DefaultDependencies=no +Wants=systemd-fsckd.socket Before=local-fs.target shutdown.target +After=systemd-fsckd.socket ConditionPathIsReadWrite=!/ [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck -StandardOutput=journal+console +StandardOutput=journal Just remove the line completely to use the admin default (here and in other cases...). TimeoutSec=0 diff --git a/units/systemd-f...@.service.in b/units/systemd-f...@.service.in index 857e625..3ceb5f2 100644 --- a/units/systemd-f...@.service.in +++ b/units/systemd-f...@.service.in @@ -10,12 +10,13 @@ Description=File System Check on %f Documentation=man:systemd-fsck@.service(8) DefaultDependencies=no BindsTo=%i.device -After=%i.device systemd-fsck-root.service local-fs-pre.target +Wants=systemd-fsckd.socket +After=%i.device systemd-fsck-root.service local-fs-pre.target systemd-fsckd.socket Before=shutdown.target [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck %f -StandardOutput=journal+console +StandardOutput=journal TimeoutSec=0 diff --git a/units/systemd-fsckd.service.in b/units/systemd-fsckd.service.in new file mode 100644 index 000..27c325f --- /dev/null +++ b/units/systemd-fsckd.service.in @@ -0,0 +1,16 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=File System Check Daemon to report status +DefaultDependencies=no +Requires=systemd-fsckd.socket +Before=shutdown.target + +[Service] +ExecStart=@rootlibexecdir@/systemd-fsckd +StandardOutput=journal+console diff --git a/units/systemd-fsckd.socket b/units/systemd-fsckd.socket new file mode 100644 index 000..96a034a --- /dev/null +++ b/units/systemd-fsckd.socket @@ -0,0 +1,15 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=fsck to fsckd communication Socket +Documentation=man:systemd-fsck@.service(8
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
Le 28/01/2015 21:25, Lennart Poettering a écrit : On Wed, 28.01.15 14:22, Didier Roche (didro...@ubuntu.com) wrote: src/fsck/fsck.c | 29 + src/fsckd/fsckd.c | 43 +++ src/fsckd/fsckd.h | 5 + 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index f5dd546..0b42e3b 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -47,6 +47,8 @@ static bool arg_skip = false; static bool arg_force = false; static const char *arg_repair = "-a"; +static pid_t fsck_pid; +static bool cancel_requested = false; Please do not introduce new global variables unnecessarily. We try to keep variables locally, and keep the global state at a minimum. An exception is mostly the command line args, which by their nature are global anyway... Sure, I put fsck_pid as a function parameter. For cancel_requested, I noticed that process_progress() was already return an int that was not read, so assigned it to a variable and check for fsck KILL status and not process_progress returning 0. I didn't add checks (as there were none as of today) on process_progress() status, but can do. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
Le 28/01/2015 15:53, Zbigniew Jędrzejewski-Szmek a écrit : On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote: From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 16:40:52 +0100 Subject: [PATCH 06/12] Support cancellation of fsck in progress Grab in fsckd plymouth watch key for C or c, and propagate this cancel request to systemd-fsck which will terminate fsck. Could we bind to ^c or if this is not possible, "three c's in three seconds" instead? I'm worried that before you could press anything to little effect in plymouth, and now a single key will have significant consequences. I tried to have a look at libplymouth, and if I'm correct, it's not possible to listen and get events for compose keys, so no way to get something like Control+C. As Dimitri told, it's been quite some years we are doing that in ubuntu, and that's the reason why we show a message to ensure the user is aware about that key (and that's why this patch is doing). Is it good for you this way? Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 04/12] Add some plymouth functionality to connect and send, messages
Le 28/01/2015 21:22, Lennart Poettering a écrit : On Wed, 28.01.15 14:22, Didier Roche (didro...@ubuntu.com) wrote: # -- +have_plymouth=no +AC_ARG_ENABLE(plymouth, AS_HELP_STRING([--disable-plymouth], [disable plymouth integration])) +if test "x$enable_plymouth" != "xno"; then +PKG_CHECK_MODULES([PLYMOUTH], [ply-boot-client >= 0.8.0], +[AC_DEFINE(HAVE_PLYMOUTH, 1, [Define if plymouth is available]) have_plymouth=yes], have_plymouth=no) +if test "x$have_plymouth" = xno -a "x$enable_plymouth" = xyes; then +AC_MSG_ERROR([*** plymouth integration requested but libraries not found]) +fi +fi +AM_CONDITIONAL(HAVE_PLYMOUTH, [test "$have_plymouth" = "yes"]) Hmm, I am bit concerned about adding this dependency. So far we managed to talk to plymouth without using its library, and I am really not sure we should start doing so now. So far the messages to send were so simply that it really wasn't worth the effort to use the full library. This is doable for the first part, similar to what src/tty-ask-password-agent/tty-ask-password-agent.c is doing (using the socket directly to send update and message to it). I'm quite unsure "watch and get key events" part as looking at libplymouth code, this seems quite more complex as a protocol to achieve. If you feel that needs to be done anyway, I can look deeper at this if you really feel we should reimplement libplymouth protocol rathen than having an optional dep on it. Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 04/12] Add some plymouth functionality to connect and send, messages
Le 28/01/2015 15:44, Zbigniew Jędrzejewski-Szmek a écrit :
On Wed, Jan 28, 2015 at 02:22:04PM +0100, Didier Roche wrote:
From 7afe9270e3210668053089caaff8a1dd790a48f5 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:07:32 +0100
Subject: [PATCH 04/12] Add some plymouth functionality to connect and send
messages
diff --git a/src/shared/plymouth.c b/src/shared/plymouth.c
index c4865dd..f7155c4 100644
--- a/src/shared/plymouth.c
+++ b/src/shared/plymouth.c
@@ -19,13 +19,96 @@
along with systemd; If not, see <http://www.gnu.org/licenses/>.
***/
+#ifdef HAVE_PLYMOUTH
+#include
+#endif
+
#include
#include
#include
#include
+#include "log.h"
#include "plymouth.h"
+#ifdef HAVE_PLYMOUTH
+void plymouth_disconnected (void *user_data, ply_boot_client_t *client);
+void plymouth_update_failed(void *user_data, ply_boot_client_t *client);
+
+static ply_boot_client_t *plymouth_client = NULL;
+static ply_event_loop_t *plymouth_event_loop = NULL;
+#endif
+
bool plymouth_running(void) {
return access("/run/plymouth/pid", F_OK) >= 0;
}
+
+#ifdef HAVE_PLYMOUTH
+bool plymouth_connect(void) {
Is there a particular reason why this cannot return a normal int code?
No reason, I changed to 0 for connected, -1 for can't connect.
Other changes done, thanks!
Cheers,
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 01/12] fsckd daemon for inter-fsckd communication
Le 28/01/2015 21:16, Lennart Poettering a écrit :
Thanks for your feedbacks! Changes integrated, some remaining questions
though:
On Wed, 28.01.15 14:20, Didier Roche (didier.ro...@canonical.com) wrote:
+last_activity = now(CLOCK_MONOTONIC);
+
+for (;;) {
+int new_client_fd = 0;
+Clients *current;
+_cleanup_free_ char *console_message = NULL;
+double current_percent = 100;
+int current_numdevices = 0, m = 0;
+usec_t t;
+
+/* Initialize and list new clients */
+new_client_fd = accept4(socket_fd, NULL, NULL, SOCK_NONBLOCK);
+if (new_client_fd > 0) {
+log_debug("new fsck client connected to fd: %d",
new_client_fd);
+current = alloca0(sizeof(Clients));
It's not OK to invoke alloca() in loops. This cannot work. Please use
normal heap memoy for this.
Oh, good to know, replaced with a regular malloc. I didn't handle the
freeing of Clients as they are destructed on program close, should I
handle this (and thus looping over and freeing the new char* - which
part of the struct)?
+current = NULL;
+}
+
+LIST_FOREACH(clients, current, first) {
+ FsckProgress fsck_data;
+ int rc = 0;
+
+if (current->fd > 0)
+rc = recv(current->fd, &fsck_data,
sizeof(FsckProgress), 0);
+
+if ((current->fd != 0) && (rc == 0)) {
+log_debug("fsck client connected to
fd %d disconnected", current->fd);
Please print propery exit codes.
That was my bad, it's r rather then rc, so not a return code but number
of bytes read, or I missing something?
+
+/* write to console */
+fprintf(console, "\r%s\r%n", console_message, &m);
+fflush(console);
Hmm, what's the reason for first writing this to an alocated buffer,
and then to the console? You can write this directly to the console, no?
Once I'm adding plymouth connection in 03/12, I'm reusing the same
console_message (I can rename it if necessary).
Thanks for the review!
Cheers,
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 01/12] fsckd daemon for inter-fsckd communication
Le 28/01/2015 15:37, Zbigniew Jędrzejewski-Szmek a écrit :
On Wed, Jan 28, 2015 at 02:20:40PM +0100, Didier Roche wrote:
Hey Zbigniew,
Thanks for the quick feedbacks! Integrated your changes in the incoming
patches. Just answered to some comments here:
From d0f49b4e0994b9a1dd2738da18c5a2a6708b444f Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 15:35:50 +0100
Subject: [PATCH 01/12] fsckd daemon for inter-fsckd communication
+fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
+if (fsckd_fd < 0) {
+log_warning_errno(errno, "Cannot open fsckd socket, we won't report
fsck progress: %m");
+return -errno;
+}
+if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) +
strlen(sa.un.sun_path)) < 0) {
+log_warning_errno(errno, "Cannot connect to fsckd socket, we won't
report fsck progress: %m");
+return -errno;
+}
f = fdopen(fd, "r");
if (!f) {
-safe_close(fd);
+log_warning_errno(errno, "Cannot connect to fsck, we won't report
fsck progress: %m");
return -errno;
}
There's a double close now, since both f and fsckd_fd refer to the same fd.
Actually no, fd is the fd from the pipe between fsck -> systemd-fsck
and fsckd_fd is the fd from the socket between systemd-fsck ->
systemd-fsckd.
Or am I missing something?
\ No newline at end of file
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
new file mode 100644
index 000..3059c68
--- /dev/null
+++ b/src/fsckd/fsckd.c
@@ -0,0 +1,295 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+}
+
+static int handle_requests(int socket_fd) {
+Clients *first = NULL;
+usec_t last_activity = 0;
+int numdevices = 0, clear = 0;
+double percent = 100;
+_cleanup_fclose_ FILE *console = NULL;
NULL not necessary (and you don't have it in similar circumstances above ;))
So, after the discussion on the mailing list and to be more future
proof, I went to recheck, but I'm afraid that I didn't find any place
where I have some _cleanup_* without initializing to NULL? Did I get it
wrong?
+log_error_errno(errno, "Socket read error,
disconnecting fd %d: %m", current->fd);
+current->fd = 0;
0 is a valid file descriptor number. You must use -1. This also means that some
initialization
to -1 is missing.
The clients are always initialized when we get a valid fd (current->fd =
new_client_fd;), so no need to initiliaze to -1, I'm using now -1 though
to invalidate the fd.
+static int create_socket(void) {
Can you base this on make_socket_fd() instead?
Oh nice, didn't find it when I looked for a generic utility. Using it :)
Thanks again for this review!
Cheers,
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 11/12] Add man page and references to it.
>From 6b13d8fb248bf4176f1ad7e1d4736683462bf196 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:34:59 +0100 Subject: [PATCH 11/12] Add man page and references to it. Add man page explaining the plymouth theme protocol, usage of the daemon as well as the socket activation part. Adapt existing fsck man page. --- Makefile-man.am| 12 +++ man/systemd-f...@.service.xml | 6 +- man/systemd-fsckd.service.xml | 170 + units/systemd-fsckd.service.in | 1 + units/systemd-fsckd.socket | 2 +- 5 files changed, 188 insertions(+), 3 deletions(-) create mode 100644 man/systemd-fsckd.service.xml diff --git a/Makefile-man.am b/Makefile-man.am index 105853e..f2e13e8 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -67,6 +67,7 @@ MANPAGES += \ man/systemd-escape.1 \ man/systemd-firstboot.1 \ man/systemd-fsck@.service.8 \ + man/systemd-fsckd.service.8 \ man/systemd-fstab-generator.8 \ man/systemd-getty-generator.8 \ man/systemd-gpt-auto-generator.8 \ @@ -210,6 +211,8 @@ MANPAGES_ALIAS += \ man/systemd-firstboot.service.1 \ man/systemd-fsck-root.service.8 \ man/systemd-fsck.8 \ + man/systemd-fsckd.8 \ + man/systemd-fsckd.socket.8 \ man/systemd-hibernate-resume.8 \ man/systemd-hibernate.service.8 \ man/systemd-hybrid-sleep.service.8 \ @@ -323,6 +326,8 @@ man/systemd-ask-password-wall.service.8: man/systemd-ask-password-console.servic man/systemd-firstboot.service.1: man/systemd-firstboot.1 man/systemd-fsck-root.service.8: man/systemd-fsck@.service.8 man/systemd-fsck.8: man/systemd-fsck@.service.8 +man/systemd-fsckd.8: man/systemd-fsckd.service.8 +man/systemd-fsckd.socket.8: man/systemd-fsckd.service.8 man/systemd-hibernate-resume.8: man/systemd-hibernate-resume@.service.8 man/systemd-hibernate.service.8: man/systemd-suspend.service.8 man/systemd-hybrid-sleep.service.8: man/systemd-suspend.service.8 @@ -606,6 +611,12 @@ man/systemd-fsck-root.service.html: man/systemd-f...@.service.html man/systemd-fsck.html: man/systemd-f...@.service.html $(html-alias) +man/systemd-fsckd.html: man/systemd-fsckd.service.html + $(html-alias) + +man/systemd-fsckd.socket.html: man/systemd-fsckd.service.html + $(html-alias) + man/systemd-hibernate-resume.html: man/systemd-hibernate-res...@.service.html $(html-alias) @@ -1732,6 +1743,7 @@ EXTRA_DIST += \ man/systemd-escape.xml \ man/systemd-firstboot.xml \ man/systemd-f...@.service.xml \ + man/systemd-fsckd.service.xml \ man/systemd-fstab-generator.xml \ man/systemd-getty-generator.xml \ man/systemd-gpt-auto-generator.xml \ diff --git a/man/systemd-f...@.service.xml b/man/systemd-f...@.service.xml index ee66f37..d366712 100644 --- a/man/systemd-f...@.service.xml +++ b/man/systemd-f...@.service.xml @@ -87,8 +87,9 @@ check, number of mounts, unclean unmount, etc. systemd-fsck will forward -file system checking progress to the console. If a -file system check fails for a service without +file system checking progress to +systemd-fsckd.service +socket. If a file system check fails for a service without nofail, emergency mode is activated, by isolating to emergency.target. @@ -142,6 +143,7 @@ systemd1, fsck8, +systemd-fsckd.service8, systemd-quotacheck.service8, fsck.btrfs8, fsck.cramfs8, diff --git a/man/systemd-fsckd.service.xml b/man/systemd-fsckd.service.xml new file mode 100644 index 000..befcc45 --- /dev/null +++ b/man/systemd-fsckd.service.xml @@ -0,0 +1,170 @@ + + +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";> + +http://www.w3.org/2001/XInclude";> + + +systemd-fsckd.service +systemd + + + +Developer + Didier + Roche +didro...@ubuntu.com + + + + + +systemd-fsckd.service +8 + + + +systemd-fsckd.service +systemd-fsckd.socket +systemd-fsckd +File system check progress reporting + + + +systemd-fsckd.service +systemd-fsckd.socket +/usr/lib/systemd/systemd-fsckd + + + +Description + +systemd-fsckd.service is a +service responsible for fetching file system check +progress, and communicating some consolidated data +to console and plymouth (if runnin
[systemd-devel] [PATCH 12/12] Add mock fsck process
>From aefe0667ed62d5d7e17193c0f5ae302ed57e4727 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:46:36 +0100
Subject: [PATCH 12/12] Add mock fsck process
---
test/mocks/fsck | 27 +++
1 file changed, 27 insertions(+)
create mode 100755 test/mocks/fsck
diff --git a/test/mocks/fsck b/test/mocks/fsck
new file mode 100755
index 000..77b50d7
--- /dev/null
+++ b/test/mocks/fsck
@@ -0,0 +1,27 @@
+#!/bin/bash
+fd=0
+
+OPTIND=1
+while getopts "C:aTlM" opt; do
+case "$opt" in
+C)
+fd=$OPTARG
+;;
+\?);;
+esac
+done
+
+shift "$((OPTIND-1))"
+device=$1
+
+echo "Running fake fsck on $device"
+
+declare -a maxpass=(30 5 2 30 60)
+
+for pass in {1..5}; do
+maxprogress=${maxpass[$((pass-1))]}
+for (( current=0; current<=${maxprogress}; current++)); do
+echo "$pass $current $maxprogress $device">&$fd
+sleep 0.1
+done
+done
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 10/12] Add fsckd service and socket, retarget systemd-fsck
>From 000f1b6ff4f5f80a2a13309590d255de6d6526ec Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 26 Jan 2015 17:30:00 +0100 Subject: [PATCH 10/12] Add fsckd service and socket, retarget systemd-fsck systemd-fsckd can be socket-activated by systemd-fsck process. Reflect that in the different unit files. --- Makefile.am| 3 +++ units/.gitignore | 1 + units/systemd-fsck-root.service.in | 4 +++- units/systemd-f...@.service.in | 5 +++-- units/systemd-fsckd.service.in | 16 units/systemd-fsckd.socket | 15 +++ 6 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 units/systemd-fsckd.service.in create mode 100644 units/systemd-fsckd.socket diff --git a/Makefile.am b/Makefile.am index a9d61f1..1eeba4f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -492,6 +492,7 @@ dist_systemunit_DATA = \ units/slices.target \ units/system.slice \ units/x-.slice \ + units/systemd-fsckd.socket \ units/systemd-initctl.socket \ units/systemd-shutdownd.socket \ units/syslog.socket \ @@ -543,6 +544,7 @@ nodist_systemunit_DATA = \ units/systemd-kexec.service \ units/systemd-fsck@.service \ units/systemd-fsck-root.service \ + units/systemd-fsckd.service \ units/systemd-machine-id-commit.service \ units/systemd-udevd.service \ units/systemd-udev-trigger.service \ @@ -596,6 +598,7 @@ EXTRA_DIST += \ units/user/systemd-exit.service.in \ units/systemd-f...@.service.in \ units/systemd-fsck-root.service.in \ + units/systemd-fsckd.service.in \ units/systemd-machine-id-commit.service.in \ units/u...@.service.m4.in \ units/debug-shell.service.in \ diff --git a/units/.gitignore b/units/.gitignore index 6fdb629..26ae965 100644 --- a/units/.gitignore +++ b/units/.gitignore @@ -28,6 +28,7 @@ /systemd-firstboot.service /systemd-fsck-root.service /systemd-fsck@.service +/systemd-fsckd.service /systemd-machine-id-commit.service /systemd-halt.service /systemd-hibernate.service diff --git a/units/systemd-fsck-root.service.in b/units/systemd-fsck-root.service.in index 6d76578..bd3bdbc 100644 --- a/units/systemd-fsck-root.service.in +++ b/units/systemd-fsck-root.service.in @@ -9,12 +9,14 @@ Description=File System Check on Root Device Documentation=man:systemd-fsck-root.service(8) DefaultDependencies=no +Wants=systemd-fsckd.socket Before=local-fs.target shutdown.target +After=systemd-fsckd.socket ConditionPathIsReadWrite=!/ [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck -StandardOutput=journal+console +StandardOutput=journal TimeoutSec=0 diff --git a/units/systemd-f...@.service.in b/units/systemd-f...@.service.in index 857e625..3ceb5f2 100644 --- a/units/systemd-f...@.service.in +++ b/units/systemd-f...@.service.in @@ -10,12 +10,13 @@ Description=File System Check on %f Documentation=man:systemd-fsck@.service(8) DefaultDependencies=no BindsTo=%i.device -After=%i.device systemd-fsck-root.service local-fs-pre.target +Wants=systemd-fsckd.socket +After=%i.device systemd-fsck-root.service local-fs-pre.target systemd-fsckd.socket Before=shutdown.target [Service] Type=oneshot RemainAfterExit=yes ExecStart=@rootlibexecdir@/systemd-fsck %f -StandardOutput=journal+console +StandardOutput=journal TimeoutSec=0 diff --git a/units/systemd-fsckd.service.in b/units/systemd-fsckd.service.in new file mode 100644 index 000..27c325f --- /dev/null +++ b/units/systemd-fsckd.service.in @@ -0,0 +1,16 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=File System Check Daemon to report status +DefaultDependencies=no +Requires=systemd-fsckd.socket +Before=shutdown.target + +[Service] +ExecStart=@rootlibexecdir@/systemd-fsckd +StandardOutput=journal+console diff --git a/units/systemd-fsckd.socket b/units/systemd-fsckd.socket new file mode 100644 index 000..96a034a --- /dev/null +++ b/units/systemd-fsckd.socket @@ -0,0 +1,15 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=fsck to fsckd communication Socket +Documentation=man:systemd-fsck@.service(8) man:systemd-fsck-root.service(8) +DefaultDependencies=no +Before=sockets.target + +[Socket] +ListenStream=/run/systemd/fsckd -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 07/12] Add gettext support
>From a56e70d026565b4da79acfdad1d83d6521ec42f5 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 16:51:05 +0100
Subject: [PATCH 07/12] Add gettext support
---
configure.ac | 1 +
src/shared/util.c | 8
src/shared/util.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/configure.ac b/configure.ac
index 62f1eef..add45fc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,7 @@ AS_IF([test -z "$INTLTOOL_POLICY_RULE"], [
GETTEXT_PACKAGE=systemd
AC_SUBST(GETTEXT_PACKAGE)
+AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$GETTEXT_PACKAGE", [systemd])
AC_PROG_MKDIR_P
AC_PROG_LN_S
diff --git a/src/shared/util.c b/src/shared/util.c
index 3002224..1c6891d 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -25,6 +25,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -70,6 +72,7 @@
#include
#endif
+#include "config.h"
#include "macro.h"
#include "util.h"
#include "ioprio.h"
@@ -5769,6 +5772,11 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
return NULL;
}
+void init_gettext(void) {
+setlocale(LC_ALL, "");
+textdomain(GETTEXT_PACKAGE);
+}
+
bool is_locale_utf8(void) {
const char *set;
static int cached_answer = -1;
diff --git a/src/shared/util.h b/src/shared/util.h
index 2cdc419..6c0e561 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -738,6 +738,8 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
int (*compar) (const void *, const void *, void *),
void *arg);
+#define _(String) gettext (String)
+void init_gettext(void);
bool is_locale_utf8(void);
typedef enum DrawSpecialChar {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 08/12] Translate fsckd messages for plymouth
>From cb822ca631b8cddfe03930ef68a12c97372bb8c1 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:12:54 +0100
Subject: [PATCH 08/12] Translate fsckd messages for plymouth
For plymouth themes not supporting i18n (like .script), send translated
messages to display to user, which is equivalent to the sent machine
readable data.
---
po/POTFILES.in| 1 +
src/fsckd/fsckd.c | 10 +++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index b4c1121..70e7594 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -5,3 +5,4 @@ src/locale/org.freedesktop.locale1.policy.in
src/login/org.freedesktop.login1.policy.in
src/machine/org.freedesktop.machine1.policy.in
src/timedate/org.freedesktop.timedate1.policy.in
+src/fsckd/fsckd.c
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 5760916..b07760d 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
#include
#include
@@ -101,7 +102,7 @@ static int handle_requests(int socket_fd) {
const char *plymouth_cancel_message;
bool cancel_message_plymouth_sent = false;
-plymouth_cancel_message = strappenda("fsckd-cancel-msg:", "Press C to cancel all checks in progress");
+plymouth_cancel_message = strappenda("fsckd-cancel-msg:", _("Press C to cancel all checks in progress"));
#endif
console = fopen("/dev/console", "we");
@@ -192,7 +193,9 @@ static int handle_requests(int socket_fd) {
numdevices = current_numdevices;
percent = current_percent;
-asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
+asprintf(&console_message,
+ ngettext("Checking in progress on %d disk (%3.1f%% complete)",
+ "Checking in progress on %d disks (%3.1f%% complete)", numdevices),
numdevices, percent);
#ifdef HAVE_PLYMOUTH
asprintf(&fsck_message, "fsckd:%d:%3.1f:%s", numdevices, percent, console_message);
@@ -208,7 +211,7 @@ static int handle_requests(int socket_fd) {
/* send to plymouth */
if (!cancel_message_plymouth_sent) {
cancel_message_plymouth_sent = \
-plymouth_watch_key("Cc", plymouth_cancel_message, cancel_requested);
+plymouth_watch_key(_("Cc"), plymouth_cancel_message, cancel_requested);
}
plymouth_update(fsck_message);
#endif
@@ -341,6 +344,7 @@ int main(int argc, char *argv[]) {
log_set_target(LOG_TARGET_AUTO);
log_parse_environment();
log_open();
+init_gettext();
r = parse_argv(argc, argv);
if (r <= 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
>From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 16:40:52 +0100
Subject: [PATCH 06/12] Support cancellation of fsck in progress
Grab in fsckd plymouth watch key for C or c, and propagate this cancel request
to systemd-fsck which will terminate fsck.
Send a message to signal to user what key we are grabbing for fsck cancel.
Message is: fsckd-cancel-msg:
Where string is a translated string ready to be displayed by the plymouth theme
indicating that c or C can be used to cancel current checks. It can be
overriden (matching only fsckd-cancel-msg prefix) for themes supporting i18n.
---
src/fsck/fsck.c | 29 +
src/fsckd/fsckd.c | 43 +++
src/fsckd/fsckd.h | 5 +
3 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index f5dd546..0b42e3b 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -47,6 +47,8 @@
static bool arg_skip = false;
static bool arg_force = false;
static const char *arg_repair = "-a";
+static pid_t fsck_pid;
+static bool cancel_requested = false;
static void start_target(const char *target) {
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
@@ -165,6 +167,7 @@ static int process_progress(int fd) {
ssize_t n;
usec_t t;
FsckProgress progress;
+FsckdMessage fsckd_message;
if (fscanf(f, "%i %lu %lu %ms", &pass, &cur, &max, &device) != 4)
break;
@@ -185,6 +188,16 @@ static int process_progress(int fd) {
n = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
if (n < 0 || (size_t) n < sizeof(FsckProgress))
log_warning_errno(n, "Cannot communicate fsck progress to fsckd: %m");
+
+/* get fsckd requests */
+n = recv(fsckd_fd, &fsckd_message, sizeof(FsckdMessage), 0);
+if (n > 0) {
+if (fsckd_message.cancel) {
+log_warning("Request to cancel fsck from fsckd");
+cancel_requested = true;
+kill(fsck_pid, SIGTERM);
+}
+}
}
return 0;
@@ -193,7 +206,6 @@ static int process_progress(int fd) {
int main(int argc, char *argv[]) {
const char *cmdline[9];
int i = 0, r = EXIT_FAILURE, q;
-pid_t pid;
siginfo_t status;
_cleanup_udev_unref_ struct udev *udev = NULL;
_cleanup_udev_device_unref_ struct udev_device *udev_device = NULL;
@@ -321,11 +333,11 @@ int main(int argc, char *argv[]) {
cmdline[i++] = device;
cmdline[i++] = NULL;
-pid = fork();
-if (pid < 0) {
+fsck_pid = fork();
+if (fsck_pid < 0) {
log_error_errno(errno, "fork(): %m");
goto finish;
-} else if (pid == 0) {
+} else if (fsck_pid == 0) {
/* Child */
if (progress_pipe[0] >= 0)
safe_close(progress_pipe[0]);
@@ -340,7 +352,7 @@ int main(int argc, char *argv[]) {
progress_pipe[0] = -1;
}
-q = wait_for_terminate(pid, &status);
+q = wait_for_terminate(fsck_pid, &status);
if (q < 0) {
log_error_errno(q, "waitid(): %m");
goto finish;
@@ -348,11 +360,11 @@ int main(int argc, char *argv[]) {
if (status.si_code != CLD_EXITED || (status.si_status & ~1)) {
-if (status.si_code == CLD_KILLED || status.si_code == CLD_DUMPED)
+if ((!cancel_requested && status.si_code == CLD_KILLED) || status.si_code == CLD_DUMPED)
log_error("fsck terminated by signal %s.", signal_to_string(status.si_status));
else if (status.si_code == CLD_EXITED)
log_error("fsck failed with error code %i.", status.si_status);
-else
+else if (!cancel_requested)
log_error("fsck failed due to unknown reason.");
if (status.si_code == CLD_EXITED && (status.si_status & 2) && root_directory)
@@ -363,7 +375,8 @@ int main(int argc, char *argv[]) {
start_target(SPECIAL_EMERGENCY_TARGET);
else {
r = EXIT_SUCCESS;
-log_warning("Ignoring error.");
+if (!cancel_requested)
+log_warning("Ignoring error.");
}
} else
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsc
[systemd-devel] [PATCH 04/12] Add some plymouth functionality to connect and send, messages
>From 7afe9270e3210668053089caaff8a1dd790a48f5 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 17:07:32 +0100
Subject: [PATCH 04/12] Add some plymouth functionality to connect and send
messages
Connect to plymouth (if running).
Automatic reconnect if plymouth was disconnected (or respawned)
when trying to send a subsequent message.
---
Makefile.am | 2 ++
configure.ac | 12
src/shared/plymouth.c | 83 +++
src/shared/plymouth.h | 8 +
4 files changed, 105 insertions(+)
diff --git a/Makefile.am b/Makefile.am
index 18be607..a9d61f1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -929,10 +929,12 @@ libsystemd_shared_la_CFLAGS = \
$(AM_CFLAGS) \
$(CAP_CFLAGS) \
$(SECCOMP_CFLAGS) \
+ $(PLYMOUTH_CFLAGS) \
-pthread
libsystemd_shared_la_LIBADD = \
$(CAP_LIBS) \
+ $(PLYMOUTH_LIBS) \
-lm
# --
diff --git a/configure.ac b/configure.ac
index 12e4ab2..62f1eef 100644
--- a/configure.ac
+++ b/configure.ac
@@ -857,6 +857,18 @@ fi
AM_CONDITIONAL(HAVE_MICROHTTPD, [test "$have_microhttpd" = "yes"])
# --
+have_plymouth=no
+AC_ARG_ENABLE(plymouth, AS_HELP_STRING([--disable-plymouth], [disable plymouth integration]))
+if test "x$enable_plymouth" != "xno"; then
+PKG_CHECK_MODULES([PLYMOUTH], [ply-boot-client >= 0.8.0],
+[AC_DEFINE(HAVE_PLYMOUTH, 1, [Define if plymouth is available]) have_plymouth=yes], have_plymouth=no)
+if test "x$have_plymouth" = xno -a "x$enable_plymouth" = xyes; then
+AC_MSG_ERROR([*** plymouth integration requested but libraries not found])
+fi
+fi
+AM_CONDITIONAL(HAVE_PLYMOUTH, [test "$have_plymouth" = "yes"])
+
+# --
have_gnutls=no
AC_ARG_ENABLE(gnutls, AS_HELP_STRING([--disable-gnutls], [disable gnutls support]))
if test "x$enable_gnutls" != "xno"; then
diff --git a/src/shared/plymouth.c b/src/shared/plymouth.c
index c4865dd..f7155c4 100644
--- a/src/shared/plymouth.c
+++ b/src/shared/plymouth.c
@@ -19,13 +19,96 @@
along with systemd; If not, see <http://www.gnu.org/licenses/>.
***/
+#ifdef HAVE_PLYMOUTH
+#include
+#endif
+
#include
#include
#include
#include
+#include "log.h"
#include "plymouth.h"
+#ifdef HAVE_PLYMOUTH
+void plymouth_disconnected (void *user_data, ply_boot_client_t *client);
+void plymouth_update_failed(void *user_data, ply_boot_client_t *client);
+
+static ply_boot_client_t *plymouth_client = NULL;
+static ply_event_loop_t *plymouth_event_loop = NULL;
+#endif
+
bool plymouth_running(void) {
return access("/run/plymouth/pid", F_OK) >= 0;
}
+
+#ifdef HAVE_PLYMOUTH
+bool plymouth_connect(void) {
+
+/* Keep existing connection */
+if (plymouth_client)
+return true;
+
+/* Create the event loop */
+if (!plymouth_event_loop)
+plymouth_event_loop = ply_event_loop_new();
+
+if (!plymouth_event_loop) {
+log_error("Couldn't create event loop for plymouth");
+return false;
+}
+
+plymouth_client = ply_boot_client_new();
+
+if (!ply_boot_client_connect(plymouth_client, plymouth_disconnected, NULL)) {
+log_error("Couldn't connect to plymouth");
+ply_boot_client_free(plymouth_client);
+plymouth_client = NULL;
+plymouth_event_loop = NULL;
+return false;
+}
+
+/* attach event loop after being connected to plymouth or the disconnect handler won't be registered
+ and flush all events that may exists from an older connection if we are reconnected */
+ply_boot_client_attach_to_event_loop(plymouth_client, plymouth_event_loop);
+ply_boot_client_flush(plymouth_client);
+
+return true;
+}
+
+void plymouth_disconnect(void) {
+if (!plymouth_client)
+ return;
+ply_boot_client_disconnect(plymouth_client);
+ply_boot_client_flush(plymouth_client);
+}
+
+void plymouth_update(const char *message) {
+if (!plymouth_running() || !plymouth_connect())
+return;
+
+ply_boot_client_update_daemon(plymouth_client, message, NULL, NULL, NULL);
+ply_boot_client_flush(plymouth_client);
+}
+
+void plymouth_delete_message(void) {
+if (!plymouth_running() || !plymouth_client)
+return;
+
+ply_boot_client_tell_daemon_to_display_message (plymouth_client, "", NULL, plymouth_update_failed, NULL);
+ply_boot_client_flush(plymouth_clien
[systemd-devel] [PATCH 05/12] Connect and send to plymouth progress report
>From c60d4f41e279dd5ed7134d97d95549aac1f38e69 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 16:29:30 +0100
Subject: [PATCH 05/12] Connect and send to plymouth progress report
Try to connect and send to plymouth (if running) some check report progress,
using libplymouth.
Update message is the following:
fsckd:::
* num_devices corresponds to the current number of devices being checked (int)
* progress corresponds to the current minimum percentage of all devices being
checking (float, from 0 to 100)
* string is a translated message ready to be displayed by the plymouth theme
displaying the information above. It can be overriden by plymouth themes
supporting i18n.
---
src/fsckd/fsckd.c | 23 ++-
src/shared/plymouth.c | 15 +++
src/shared/plymouth.h | 2 ++
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index e27cd6d..b516193 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -39,6 +39,9 @@
#include "log.h"
#include "list.h"
#include "macro.h"
+#ifdef HAVE_PLYMOUTH
+#include "plymouth.h"
+#endif
#include "sd-daemon.h"
#include "time-util.h"
#include "util.h"
@@ -95,6 +98,9 @@ static int handle_requests(int socket_fd) {
int new_client_fd = 0;
Clients *current;
_cleanup_free_ char *console_message = NULL;
+#ifdef HAVE_PLYMOUTH
+_cleanup_free_ char *fsck_message = NULL;
+#endif
double current_percent = 100;
int current_numdevices = 0, m = 0;
usec_t t;
@@ -155,6 +161,9 @@ static int handle_requests(int socket_fd) {
asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
numdevices, percent);
+#ifdef HAVE_PLYMOUTH
+asprintf(&fsck_message, "fsckd:%d:%3.1f:%s", numdevices, percent, console_message);
+#endif
/* write to console */
if (show_progress) {
@@ -162,6 +171,11 @@ static int handle_requests(int socket_fd) {
fflush(console);
}
+#ifdef HAVE_PLYMOUTH
+/* send to plymouth */
+plymouth_update(fsck_message);
+#endif
+
if (m > clear)
clear = m;
}
@@ -187,6 +201,9 @@ static int handle_requests(int socket_fd) {
fputc('\r', console);
fflush(console);
}
+#ifdef HAVE_PLYMOUTH
+plymouth_disconnect();
+#endif
return 0;
}
@@ -218,7 +235,11 @@ static int create_socket(void) {
static void help(void) {
printf("%s [OPTIONS...]\n\n"
- "Capture fsck progress and forward one stream to plymouth\n\n"
+#ifdef HAVE_PLYMOUTH
+ "Capture fsck progress, log to console and forward one stream to plymouth\n\n"
+#else
+ "Capture fsck progress and log to console\n\n"
+#endif
" -h --help Show this help\n"
" --version Show package version\n",
program_invocation_short_name);
diff --git a/src/shared/plymouth.c b/src/shared/plymouth.c
index f7155c4..b43d355 100644
--- a/src/shared/plymouth.c
+++ b/src/shared/plymouth.c
@@ -92,6 +92,21 @@ void plymouth_update(const char *message) {
ply_boot_client_flush(plymouth_client);
}
+static void plymouth_key_pressed(void *callback, const char *pressed_key, ply_boot_client_t *client) {
+((keypress_callback)callback)();
+}
+
+bool plymouth_watch_key(const char *keys, const char *message, keypress_callback callback) {
+if (!plymouth_running() || !plymouth_connect())
+return false;
+
+ply_boot_client_tell_daemon_to_display_message (plymouth_client, message, NULL,
+plymouth_update_failed, NULL);
+ply_boot_client_ask_daemon_to_watch_for_keystroke (plymouth_client, keys, plymouth_key_pressed,
+ NULL, callback);
+return true;
+}
+
void plymouth_delete_message(void) {
if (!plymouth_running() || !plymouth_client)
return;
diff --git a/src/shared/plymouth.h b/src/shared/plymouth.h
index 15cecb7..f5ea00c 100644
--- a/src/shared/plymouth.h
+++ b/src/shared/plymouth.h
@@ -30,5 +30,7 @@ bool plymouth_connect(void);
void plymouth_disconnect(void);
void plymouth_update(const char *message);
+typedef void (*keypress_callback)(void);
void plymouth_delete_message(void);
+bool plymouth_watch_key(const char *keys, const char *message, keypress_callback callback);
#endif
-
[systemd-devel] [PATCH 03/12] systemd-fsck: always connect to systemd-fsckd
>From feb8f332313c3ac2542f618028f8a6a6a36daf50 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 16:01:11 +0100
Subject: [PATCH 03/12] systemd-fsck: always connect to systemd-fsckd
Remove the plymouth running or show-status checks from systemd-fsck. Instead,
always connect to systemd-fsckd socket, and let this one decide if we display
progress or not.
---
src/fsck/fsck.c | 12
src/fsckd/fsckd.c | 12 ++--
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 91dff5c..f5dd546 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -46,7 +46,6 @@
static bool arg_skip = false;
static bool arg_force = false;
-static bool arg_show_progress = false;
static const char *arg_repair = "-a";
static void start_target(const char *target) {
@@ -132,8 +131,6 @@ static void test_files(void) {
}
#endif
-if (access("/run/systemd/show-status", F_OK) >= 0 || plymouth_running())
-arg_show_progress = true;
}
static int process_progress(int fd) {
@@ -293,11 +290,10 @@ int main(int argc, char *argv[]) {
log_warning_errno(r, "fsck.%s cannot be used for %s: %m", type, device);
}
-if (arg_show_progress)
-if (pipe(progress_pipe) < 0) {
-log_error_errno(errno, "pipe(): %m");
-return EXIT_FAILURE;
-}
+if (pipe(progress_pipe) < 0) {
+log_error_errno(errno, "pipe(): %m");
+return EXIT_FAILURE;
+}
cmdline[i++] = "/sbin/fsck";
cmdline[i++] = arg_repair;
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3059c68..e27cd6d 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -45,6 +45,8 @@
#define IDLE_TIME_MINUTES 1
+static bool show_progress = false;
+
typedef struct Clients {
int fd;
char device[PATH_MAX];
@@ -155,8 +157,10 @@ static int handle_requests(int socket_fd) {
numdevices, percent);
/* write to console */
-fprintf(console, "\r%s\r%n", console_message, &m);
-fflush(console);
+if (show_progress) {
+fprintf(console, "\r%s\r%n", console_message, &m);
+fflush(console);
+}
if (m > clear)
clear = m;
@@ -291,5 +295,9 @@ int main(int argc, char *argv[]) {
if (fd <= 0)
return EXIT_FAILURE;
}
+
+if (access("/run/systemd/show-status", F_OK) >= 0)
+show_progress = true;
+
return handle_requests(fd) < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 02/12] Factorize plymouth_running in plymouth.c
>From 0ef3127531c95ad9a740cb997a1f94ef257dd4f0 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 15:45:46 +0100
Subject: [PATCH 02/12] Factorize plymouth_running in plymouth.c
Add plymouth.c/h facility which will contains all plymouth related
functionalities. This is part of the static library.
Adapt existing code to point to the new location.
---
Makefile.am | 5 -
src/core/main.c | 1 +
src/core/manager.c| 1 +
src/fsck/fsck.c | 1 +
src/shared/plymouth.c | 31 +++
src/shared/plymouth.h | 26 ++
src/shared/util.c | 4
src/shared/util.h | 2 --
8 files changed, 64 insertions(+), 7 deletions(-)
create mode 100644 src/shared/plymouth.c
create mode 100644 src/shared/plymouth.h
diff --git a/Makefile.am b/Makefile.am
index f782e66..18be607 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -905,7 +905,10 @@ libsystemd_shared_la_SOURCES = \
src/shared/sigbus.h \
src/shared/build.h \
src/shared/import-util.c \
- src/shared/import-util.h
+ src/shared/import-util.h \
+ src/shared/plymouth.c \
+ src/shared/plymouth.h \
+ $(NULL)
if HAVE_UTMP
libsystemd_shared_la_SOURCES += \
diff --git a/src/core/main.c b/src/core/main.c
index e2e1399..74cfaff 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -68,6 +68,7 @@
#include "manager.h"
#include "dbus-manager.h"
#include "load-fragment.h"
+#include "plymouth.h"
#include "mount-setup.h"
#include "loopback-setup.h"
diff --git a/src/core/manager.c b/src/core/manager.c
index e2df911..0b943b0 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -52,6 +52,7 @@
#include "macro.h"
#include "strv.h"
#include "log.h"
+#include "plymouth.h"
#include "util.h"
#include "mkdir.h"
#include "ratelimit.h"
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 4c4e150..91dff5c 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -32,6 +32,7 @@
#include "sd-bus.h"
#include "libudev.h"
+#include "plymouth.h"
#include "util.h"
#include "special.h"
#include "bus-util.h"
diff --git a/src/shared/plymouth.c b/src/shared/plymouth.c
new file mode 100644
index 000..c4865dd
--- /dev/null
+++ b/src/shared/plymouth.c
@@ -0,0 +1,31 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Canonical
+
+ 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
+#include
+#include
+#include
+
+#include "plymouth.h"
+
+bool plymouth_running(void) {
+return access("/run/plymouth/pid", F_OK) >= 0;
+}
diff --git a/src/shared/plymouth.h b/src/shared/plymouth.h
new file mode 100644
index 000..39c8c37
--- /dev/null
+++ b/src/shared/plymouth.h
@@ -0,0 +1,26 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Canonical
+
+ 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
+
+bool plymouth_running(void);
diff --git a/src/shared/util.c b/src/shared/util.c
index 891182a..3002224 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -4218,10 +4218,6 @@ bool nulstr_contains(const char*nulstr, const char *needle) {
return false;
}
-bool plymouth_running(void) {
-return access("/run/plymouth/pid", F_OK) >= 0;
-}
-
char* strshorten(char *s, size_t l) {
assert(s);
diff --git a/src/shared/util.h b/src/shared/util.h
index ca0c2e5..2cdc419 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -553,8 +55
[systemd-devel] [PATCH 01/12] fsckd daemon for inter-fsckd communication
>From d0f49b4e0994b9a1dd2738da18c5a2a6708b444f Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 26 Jan 2015 15:35:50 +0100
Subject: [PATCH 01/12] fsckd daemon for inter-fsckd communication
Add systemd-fsckd multiplexer which accept multiple systemd-fsck
instances to connect to it and send progress report. systemd-fsckd then
computes and writes to /dev/console the number of devices currently being
checked and the minimum fsck progress. This will be used for interactive
progress report and cancelling in plymouth.
systemd-fsckd stops on idling when no systemd-fsck is connected.
Make the necessary changes to systemd-fsck to connect to systemd-fsckd socket.
---
.gitignore | 1 +
Makefile.am| 11 ++
src/fsck/fsck.c| 82 ++-
src/fsckd/Makefile | 1 +
src/fsckd/fsckd.c | 295 +
src/fsckd/fsckd.h | 34 ++
6 files changed, 373 insertions(+), 51 deletions(-)
create mode 12 src/fsckd/Makefile
create mode 100644 src/fsckd/fsckd.c
create mode 100644 src/fsckd/fsckd.h
diff --git a/.gitignore b/.gitignore
index ab6d9d1..9400e75 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/systemd-evcat
/systemd-firstboot
/systemd-fsck
+/systemd-fsckd
/systemd-fstab-generator
/systemd-getty-generator
/systemd-gnome-ask-password-agent
diff --git a/Makefile.am b/Makefile.am
index c463f23..f782e66 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -389,6 +389,7 @@ rootlibexec_PROGRAMS = \
systemd-remount-fs \
systemd-reply-password \
systemd-fsck \
+ systemd-fsckd \
systemd-machine-id-commit \
systemd-ac-power \
systemd-sysctl \
@@ -2355,6 +2356,16 @@ systemd_fsck_LDADD = \
libsystemd-shared.la
# --
+systemd_fsckd_SOURCES = \
+ src/fsckd/fsckd.c \
+ $(NULL)
+
+systemd_fsckd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-shared.la \
+ $(NULL)
+
+# --
systemd_machine_id_commit_SOURCES = \
src/machine-id-commit/machine-id-commit.c \
src/core/machine-id-setup.c \
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 20b7940..4c4e150 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include "sd-bus.h"
@@ -39,6 +40,8 @@
#include "fileio.h"
#include "udev-util.h"
#include "path-util.h"
+#include "socket-util.h"
+#include "fsckd/fsckd.h"
static bool arg_skip = false;
static bool arg_force = false;
@@ -132,58 +135,42 @@ static void test_files(void) {
arg_show_progress = true;
}
-static double percent(int pass, unsigned long cur, unsigned long max) {
-/* Values stolen from e2fsck */
-
-static const int pass_table[] = {
-0, 70, 90, 92, 95, 100
-};
-
-if (pass <= 0)
-return 0.0;
-
-if ((unsigned) pass >= ELEMENTSOF(pass_table) || max == 0)
-return 100.0;
-
-return (double) pass_table[pass-1] +
-((double) pass_table[pass] - (double) pass_table[pass-1]) *
-(double) cur / (double) max;
-}
-
static int process_progress(int fd) {
-_cleanup_fclose_ FILE *console = NULL, *f = NULL;
+_cleanup_fclose_ FILE *f = NULL;
usec_t last = 0;
-bool locked = false;
-int clear = 0;
+_cleanup_close_ int fsckd_fd;
+static const union sockaddr_union sa = {
+.un.sun_family = AF_UNIX,
+.un.sun_path = FSCKD_SOCKET_PATH,
+};
+
+fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
+if (fsckd_fd < 0) {
+log_warning_errno(errno, "Cannot open fsckd socket, we won't report fsck progress: %m");
+return -errno;
+}
+if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) {
+log_warning_errno(errno, "Cannot connect to fsckd socket, we won't report fsck progress: %m");
+return -errno;
+}
f = fdopen(fd, "r");
if (!f) {
-safe_close(fd);
+log_warning_errno(errno, "Cannot connect to fsck, we won't report fsck progress: %m");
return -errno;
}
-console = fopen("/dev/console", "we");
-if (!console)
-return -ENOMEM;
-
while (!feof(f)) {
-int pass, m;
+int pass;
unsigned long cur, max;
_cleanup_free_ char *device = NULL;
-double p;
+ssize_t n;
usec_t t;
+FsckProgress progress;
if
[systemd-devel] [PATCH] Adding fsck integration to plymouth
Hi, Here is a suite of patches up to review to add fsckd integration to plymouth. This work is mostly based on Lennart's suggestion on an email thread few years ago (http://lists.freedesktop.org/archives/systemd-devel/2011-April/002063.html) where the proposal was to add a systemd-fsckd daemon. The daemon is socket-activated, each systemd-fsck instances binds and communicate its own fsck data to it. systemd-fsckd then agglomerates the results and write to /dev/console as well as propagate those values to plymouth. The protocole to communicate with the plymouth theme is described in the man page. I've modified ubuntu plymouth theme (which is a .script) to display the progress. There is as well a cancel option in case some people run on a rotational disks, hit the mount limit and need to give a talk right away (not my use case, but have seen that in practice recently) :) The plymouth integration is optional, otherwise, only /dev/console (if show-status is enabled) is written. I'm opened to any question and suggested enhancements. Cheers, Didier ___ 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
Le 05/12/2014 16:42, Lennart Poettering a écrit : On Fri, 05.12.14 16:02, Didier Roche (didro...@ubuntu.com) wrote: This would also only "fix" the newly installed case, not the upgrade with new distro defaults or various purge vs remove ones. That's why I think some kind of previous state db can help getting all those requirements met as you propose, and doing some comparisons between states (only when they deviated from defaults). Then, we can run preset on packages that matches previous defaults (meaning: where the admin didn't override the default choice) as a dpkg trigger (when new units are installed/upgraded and on a new/updated preset file shipped). Would that makes sense? Not sure I grok what you are proposing. I'd be very careful with keeping yet another layer of service enablement states (even if just as "history") in systemd upstream. I can clearly understand why you don't want to have a database of previous default enablement status. (that's what debian/ubuntu is doing right now in /var, having a bunch of symlinks to keep default state). There are 3 cases that we want to have supported (ideally with preset-only): 1. upgrade with default policy change: * foo.service is enabled in a target (with an [Install] section). Preset says 'enable *' (== no preset stenza) by default. * sysadmin didn't enable/disable it. * an upgrade of a package (not necessarily the package containing foo.service) ships a new preset file with "disable foo.service" -> We should have then foo.service disabled as per preset choice. 2. upgrade with dependency change: * foo.service was WantedBy=bar.target. It was enabled by default. * sysadmin didn't enable/disable it. * After an upgrade, the unit contains WantedBy=baz.target (still enabled by default) -> We have it still enabled by enabled (as per preset choice), with .wants symlink from baz.target From those 2 cases, we could say: "just run systemctl preset on every package upgrade or preset change", however, there is as well this case: 3. upgrade with default changes, but sysadmin overrides: * foo.service was WantedBy=bar.target. It was enabled by default. * sysadmin run systemctl disable foo.service (or mask it) * After an upgrade, the unit contains WantedBy=baz.target (still enabled by default preset policy) -> We shouldn't have foo.service enabled back as the admin did disable it. That's why I was evocating keeping a previous state database to detect "previous default state == current state", and only run preset on the package in that case (and new installs). Is this more clear? Any other idea of on an elegant way of handling those without having the previous state database? (in an upstream compatible way, if possible) Didier ___ 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
Le 05/12/2014 14:52, Lennart Poettering a écrit : On Fri, 05.12.14 11:06, Didier Roche (didro...@ubuntu.com) wrote: It seems maintaining this list in sync for all flavors would be a growing pain (this is a positive effect of the disable by default: you don't have to maintain such a list), or do you think we can come with something better? Hmm, yuck. No good suggestion. I figure this problem doesn't exist with the fedora default of everything is disabled by default... All open to ideas... Can we maybe extend the preset dictionary by having an alias (or alias-default) keyword taking a pair of arguments, like: alias display-manager.service lightdm.service Then, the preset command, for each alias, will stop at the first one it encounters. If the service doesn't exist, it's a noop, if it's there, it enables (--force in case something else was enabled for that Alias?) lightdm.service. It means of course that lightdm.service should contain: [Install] Alias=display-manager.service or preset would then generates a warning. So far this is not how presets work: they are just a database you pass in as key a service name, and it tells you whether to enable it or not, in a simple boolean way. It is something where you pass in the name of a unit you want to enable/disable, and after you got that, you do that, but you do not verify again the individual steps enabling/disabling precisely entail. It also is currently entirely stateless: you could in theory ask a web server for such a preset question, and it will always tell you the same answer, because it does *not* take your local configuration and set of packages into consideration... This kind of "disconnected" logic I'd really like to keep. Hmm, what about this proposal: Whenever the preset db is queried we'll no longer just return the verdict boolean, but also a numeric overall line number, of the line we found the verdict on. Then, when "preset-all" is invoked, we determine all the operations to execute for everything. Then, before applying them, we check if there are any conflicting operations. If so, we remove the ones with the higher line number. In effect this would mean: if you list to DMs as "enable" in the preset file, and both are to be installed, then the one that is enabled "earlier" will win in case of "preset-all". To me this appears quite a natural extension of the logic already in place. And I guess the default behavior (enable *) has a lower priority than overrides? (enable/disable). Also, I guess you concatenate all preset files as if it was a single one (ascii ordered?) to build that list. So, retaking the display-manager (on the principle they all have: Alias=display-manager.service) example, that would mean: * ubuntu-desktop would ship 99-defaults with: enable lightdm * If someone, install the gnome-ubuntu-desktop metapackage on the same install, they would get an additional preset file 50-gnome-ubuntu with: enable gdm And so, lightdm would be removed on a preset-all call as conflicting with gdm (due to sharing the same Alias) and having higher line number. Sounds like a nice way to handle Alias. Happy to have a look at this. How does this all precisely work on on ? Most of them shipped a conffile like /etc/default/ file with an ENABLED=true/false keyword. This doesn't really map in the systemd world (repetition of enablement/disablement states) * "apt-get remove" keeps conffiles * "apt-get remove --purge" deletes them. * When an upgrade occurs: - if the package conffile didn't change -> kept with the modifications if any - if the package conffile did change -> infamous debconf prompt about "a maintainer configuration file changes, do you want to apply maintainer changes/keep as it is/see the diff…" That's how all those use cases are handled on sysvinit. Of course, we could introduce that back with ExecStartPre=`grep …` but well, 2 places (systemd symlinks + a /etc/default/ file) to decide one thing isn't really appealing nor wanted :) 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...) Agreed, I already removed it from some ubuntu-only packages like whoopsie, xdiagnose… and I'm trying to investigate the right way to get a systemd-oriented solutions (while still being compatible with older init system for debian) 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
Re: [systemd-devel] /usr vs /etc for default distro units enablement
Le 05/12/2014 02:13, Lennart Poettering a écrit : On Tue, 02.12.14 12:50, Didier Roche (didro...@ubuntu.com) wrote: Just to sum up other branches of this thread: we are trying to avoid having systemctl calls in debian/ubuntu postinst (or duplicated manual symlinks logic as we currently have). systemctl preset seems the cleanest path, but we want to ensure corner cases can be handled. d/u policy is to enable newly installed package by default (difference from other distributions) Le 02/12/2014 01:59, Lennart Poettering a écrit : 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"? I meant deleting the entire "/etc" content (or I guess as you told using systemctl preset-all to reset to default): 1. lightdm and gdm were installed on my system. 2. gdm was enabled as the default display-manager. 3. I then use "systemctl preset-all" -> how the behavior of selecting the display-manager will be determined? See below implementing this with presets where enabling all services is the default. Hmm, this is actually undefined currently. THe code for preset-all iterates through the unit paths and processes the files in the order they are read by readdir(), which is pretty much undefined. We really should investigate what to do about that. Probably just order things lexicographically. I added this to the TODO list for now. See my suggestion below. So we need for any services shipping Aliases to have a preset list per flavor (if their behaviors differs) with: 99-ubuntu-desktop.preset: enable lightdm.service disable kdm.service disable gdm.service disable nodm.service (and whatnot… dm files in distro) Hmm, indeed I guess... Then, we would have 01-ubuntu-gnome.preset: enable gdm.service disable lightdm.service disable kdm.service … It seems maintaining this list in sync for all flavors would be a growing pain (this is a positive effect of the disable by default: you don't have to maintain such a list), or do you think we can come with something better? Hmm, yuck. No good suggestion. I figure this problem doesn't exist with the fedora default of everything is disabled by default... All open to ideas... Can we maybe extend the preset dictionary by having an alias (or alias-default) keyword taking a pair of arguments, like: alias display-manager.service lightdm.service Then, the preset command, for each alias, will stop at the first one it encounters. If the service doesn't exist, it's a noop, if it's there, it enables (--force in case something else was enabled for that Alias?) lightdm.service. It means of course that lightdm.service should contain: [Install] Alias=display-manager.service or preset would then generates a warning. Finally, on the "know what the administrator did on this machine", here are two cases that we can identify: I. if the administrator removes the service package, we usually keep current service state (enabled/disabled) on reinstall. So: enabled by default 1. systemctl disable foo.service 2. apt-get remove foo 3. apt-get install foo -> foo should still be disabled. However, that won't be the case as on reinstall, systemctl preset will re-enable the service as of the preset policy. Indeed, we don't have any record that the admin disabled it compared default distro policy as there is no difference between: "no previous installation state" and "service being disabled state" (no symlink). Yeah, not sure how you can provide that with the scheme we devised there in systemd. Sorry! All ears for ideas... So, I think we should really be able to fix case I. I mean, you can fix case I, by explicitly storing the state away before you remove a package. Or storing only the previous *default* state? Then, we can have a trigger updating that previous distro state (combining services installed and default preset) everytime we install/update a package containing a preset file or a an unit file? How does this all precisely work on on ? Most of them shipped a conffile like /etc/default/ file with an ENABLED=true/false keyword. This doesn't really map in the systemd world (repetition of enablement/disablement states) * "apt-get remove" keeps conffiles * "apt-get remove --purge" deletes them. * When an upgrade occurs: - if the package conffile didn't change -> kept with the modifications if any - if the package conffile did change -> infamous debconf prompt about "a maintainer configuration file changes, do you want to apply maintainer changes/keep as it is/see the diff…" That's how all those use cases are handled on sysvinit. Of course,
Re: [systemd-devel] [PATCH 2/5] Add a machine_id_commit call to commit on disk a, transient machine-id
Le 03/12/2014 03:44, Lennart Poettering a écrit : On Tue, 02.12.14 11:43, Didier Roche (didro...@ubuntu.com) wrote: Heya! Applied the patch with some changes (converted all log messages to the new errno logging). Please check if everything still works as intended. Also applied patches 3, 4, 5 after that. Hey, I saw the changes, will use that function for future patches then. (Also, I saw you changed the const casting in other part of the code already there). Everything should work as expected, I'll give it a try. Many thanks! Didier ___ 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
Just to sum up other branches of this thread: we are trying to avoid having systemctl calls in debian/ubuntu postinst (or duplicated manual symlinks logic as we currently have). systemctl preset seems the cleanest path, but we want to ensure corner cases can be handled. d/u policy is to enable newly installed package by default (difference from other distributions) Le 02/12/2014 01:59, Lennart Poettering a écrit : 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. Ok, that works with the initial install case then. However, if lightdm is installed and the admin install gdm, he will get a prompt (from postinst) asking him which dm to choose. How would you handle that (without messing manually with the symlinks or systemctl enable --force in the postinst?). Writing new presets in /etc which enables the chosen dm and disable other, then reloading preset file to reset that display-manager.service alias? 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"? I meant deleting the entire "/etc" content (or I guess as you told using systemctl preset-all to reset to default): 1. lightdm and gdm were installed on my system. 2. gdm was enabled as the default display-manager. 3. I then use "systemctl preset-all" -> how the behavior of selecting the display-manager will be determined? See below implementing this with presets where enabling all services is the default. 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). You meant disable, right? As our default is to enable all services. So we need for any services shipping Aliases to have a preset list per flavor (if their behaviors differs) with: 99-ubuntu-desktop.preset: enable lightdm.service disable kdm.service disable gdm.service disable nodm.service (and whatnot… dm files in distro) Then, we would have 01-ubuntu-gnome.preset: enable gdm.service disable lightdm.service disable kdm.service … It seems maintaining this list in sync for all flavors would be a growing pain (this is a positive effect of the disable by default: you don't have to maintain such a list), or do you think we can come with something better? Finally, on the "know what the administrator did on this machine", here are two cases that we can identify: I. if the administrator removes the service package, we usually keep current service state (enabled/disabled) on reinstall. So: enabled by default 1. systemctl disable foo.service 2. apt-get remove foo 3. apt-get install foo -> foo should still be disabled. However, that won't be the case as on reinstall, systemctl preset will re-enable the service as of the preset policy. Indeed, we don't have any record that the admin disabled it compared default distro policy as there is no difference between: "no previous installation state" and "service being disabled state" (no symlink). Same for enabling a service that is by default disabled: next systemctl call on reinstall will remove the symlinks (Alias included). II. if the adminstrator purges the service package, we usually expect that reinstalling it will reset the service to the default enablement/disablement state. So: enabled by default 1. systemctl disable foo.service 2. apt-get remove --purge foo 3. apt-get install foo -> foo should be enabled as this is the default state in distro. This use case works because the previous one doesn't :) So, I think we should really be able to fix case I. Also, we would have to condition the systemctl preset call (we have idempotent postinst script, and need to track new installs from upgrade, as we run those during postinst conf
Re: [systemd-devel] [PATCH 1/5] Factorize some machine-id-setup functions to be reused
Le 01/12/2014 18:38, Lennart Poettering a écrit :
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.
That makes sense, I rewrote the function to avoid this.
Didier
>From 7cf7322ab08c9434ba303d9958517f262b1797e0 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 24 Nov 2014 09:40:57 +0100
Subject: [PATCH 1/5] Factorize some machine-id-setup functions to be reused
---
src/core/machine-id-setup.c | 44 ++--
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index 6710038..f3763ed 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -157,6 +157,37 @@ static int generate(char id[34], const char *root) {
return 0;
}
+static int get_valid_machine_id(int fd, char id[34]) {
+char id_to_validate[34];
+
+assert(fd >= 0);
+assert(id);
+
+if (loop_read(fd, id_to_validate, 33, false) == 33 && id_to_validate[32] == '\n') {
+id_to_validate[32] = 0;
+
+if (id128_is_valid(id_to_validate)) {
+strncpy(id, id_to_validate, sizeof(id_to_validate));
+id[32] = '\n';
+id[33] = 0;
+return 0;
+}
+}
+
+return -EINVAL;
+}
+
+static int write_machine_id(int fd, char id[34]) {
+assert(fd >= 0);
+assert(id);
+lseek(fd, 0, SEEK_SET);
+
+if (loop_write(fd, id, 33, false) == 33)
+return 0;
+
+return -errno;
+}
+
int machine_id_setup(const char *root) {
const char *etc_machine_id, *run_machine_id;
_cleanup_close_ int fd = -1;
@@ -207,13 +238,8 @@ int machine_id_setup(const char *root) {
if (fstat(fd, &st) < 0)
return log_error_errno(errno, "fstat() failed: %m");
-if (S_ISREG(st.st_mode))
-if (loop_read(fd, id, 33, false) == 33 && id[32] == '\n') {
-id[32] = 0;
-
-if (id128_is_valid(id))
-return 0;
-}
+if (S_ISREG(st.st_mode) && get_valid_machine_id(fd, id) == 0)
+return 0;
/* Hmm, so, the id currently stored is not useful, then let's
* generate one */
@@ -223,9 +249,7 @@ int machine_id_setup(const char *root) {
return r;
if (S_ISREG(st.st_mode) && writable) {
-lseek(fd, 0, SEEK_SET);
-
-if (loop_write(fd, id, 33, false) == 33)
+if (write_machine_id(fd, id) == 0)
return 0;
}
--
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/5] Add a machine_id_commit call to commit on disk a, transient machine-id
Le 01/12/2014 18:37, Lennart Poettering a écrit :
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.
Thanks for your feedback.
I did introduce is_fd_on_temporary_fs() in utils.[ch] and reused
is_temporary_fs 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.
Making sense. I downgraded the error messaging to debug and always
returns 0. I tried other various use case and I think the functions
(hence, the binary) is idempotent now.
+
+/* 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 evaluat
Re: [systemd-devel] /usr vs /etc for default distro units enablement
Le 21/11/2014 12:08, Colin Guthrie a écrit : Hello again! Hey, trying to revive the topic :) Didier Roche wrote on 18/11/14 15:40: Le 18/11/2014 15:59, Colin Guthrie a écrit : Hiya, Hey, 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. This also avoids any problems here. (Of course if /etc is nuked, then reapplying the defaults from *.preset should be done!) See my Michael's answer (and my previous one) on the fact that maybe the preset files would be part of multiple packages (to disable certain units), and some being only part of packages not installed by default. Michael's case as well of a unit changing its target on a package upgrade (as a packaging fix, maybe) is valid as well. I think the distro-wide preset usage would be in a very core package that is likely installed very early on (perhaps even the package is required by the one that ships systemctl and thus has to be installed first). If you end up shipping random .preset files with the actual packages containing the units they affect (which isn't recommend as previously noted, but perhaps you still want to do it that way for some special cases) then this too will be fine. I can see some complications here but nothing that isn't manageable. This is a good news. With a shared distro/admin /etc, we have no way to take that fact into account and the next one would follow distro policy, right? Yeah, but that's assuming there *is* a next one. Once things are installed, the user should *not* be surprised by any action being taken without their consent on upgrade. FWIW, it's probably worth considering how you'd handle not changing users current preferences with regards to enabling/disabling things on upgrade. I can see this being quite hard for you roll out with your current suggestions, but you may have this covered already. Actually, this reminds me some issues we had with gconf in the past in changing distribution's default and deciphering what was users current preference or what was distro default behavior. Gnome-panel was copying its whole distro defaults in ~/.gconf. Adding/removing some default layouts and settings from the panel was then unnecessary difficult. Some changes was part of new default behaviors we wanted that the user never interacted with and was desired to be changed (because of some applets getting low maintenance, incompatible with newer technology or duplicates…) As everything was at the same place, it was difficult to know if the current value was set because of the previous default, or if the user explicitly tweaked and then selected it. I see the same issue with the shared /etc: is this unit enabled for that target because of one the preset config, or did the admin run "systemctl enable " explicitly and want to keep it? I think it's ok to change distro defaults on upgrade (will be potentially in major version upgrade of course), not user (admin here) preferences, of course. I would personally disagree with this statement that it's OK to change the distro defaults on upgrade. As an admin, whether I observe some behaviour but do not actively reinforce it (e.g. I see that installing httpd enables it by default and thus my server is working fine, so I don't need to do "systemctl enable httpd" manually) I now rely on the distro default. If that was to be changed on upgrade (whether package or whole distro), I'd personally be really annoyed! With the goal of being able to reset things (i.e. trashing /etc) if desired, the admin has a pretty good choice to start again with a clean slate after a distro upgrade if they so wish. Otherwise I'd very much expect my system to retain as much state as possible (whether that may have come from a preset or an active choice is, IMO, irrelevant - it's how the system was ultimately configured and what the admin now relies on) over the distro upgrade process. That's what we did in multiple cases in ubuntu in particular. I gave in previous emails some desktop-related email. Another one I didn't mention is when we changed from gdm to lightdm as the default dm, or gnome-panel -> unity. If the user selected another dm or another desktop session, we keep user's preference, if they switched and then switched back, we keep user's preference as well. We migrate to our new defaults if there was trace of new user settings. I guess different settings and policy f
[systemd-devel] [PATCH 5/5] machine-id-commit: add man pages
>From 9b8d7613894838ce67db5141b8012b5abf7c02e4 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 24 Nov 2014 11:14:22 +0100 Subject: [PATCH 5/5] machine-id-commit: add man pages Add man pages for systemd-machine-id-commit.service and systemd-machine-id-commit. --- Makefile-man.am | 2 + man/systemd-machine-id-commit.service.xml | 101 man/systemd-machine-id-commit.xml | 125 ++ 3 files changed, 228 insertions(+) create mode 100644 man/systemd-machine-id-commit.service.xml create mode 100644 man/systemd-machine-id-commit.xml diff --git a/Makefile-man.am b/Makefile-man.am index bdbc54f..a79292a 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -75,6 +75,7 @@ MANPAGES += \ man/systemd-inhibit.1 \ man/systemd-initctl.service.8 \ man/systemd-journald.service.8 \ + man/systemd-machine-id-commit.1 \ man/systemd-machine-id-setup.1 \ man/systemd-notify.1 \ man/systemd-nspawn.1 \ @@ -210,6 +211,7 @@ MANPAGES_ALIAS += \ man/systemd-journald.8 \ man/systemd-journald.socket.8 \ man/systemd-kexec.service.8 \ + man/systemd-machine-id-commit.service.8 \ man/systemd-poweroff.service.8 \ man/systemd-reboot.service.8 \ man/systemd-remount-fs.8 \ diff --git a/man/systemd-machine-id-commit.service.xml b/man/systemd-machine-id-commit.service.xml new file mode 100644 index 000..6da19b9 --- /dev/null +++ b/man/systemd-machine-id-commit.service.xml @@ -0,0 +1,101 @@ + + +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";> + + + + +systemd-machine-id-commit.service +systemd + + + +Developer + Didier + Roche +didro...@ubuntu.com + + + + + +systemd-machine-id-commit.service +8 + + + +systemd-machine-id-commit.service +Commit transient machine-id to disk + + + +systemd-machine-id-commit.service +/usr/lib/systemd/systemd-machine-id-commit + + + +Description + +systemd-machine-id-commit.service is +a service responsible for commiting any transient +/etc/machine-id file to a writable file +system. See +machine-id5 +for more information about this file. + +This service is started shortly after +local-fs.target if +/etc/machine-id is an independent mount +point (probably a tmpfs one) and /etc is writable. +systemd-machine-id-commit will then +write current machine ID to disk and unmount the transient +/etc/machine-id file in a race-free +manner to ensure that file is always valid for other +processes. + +Note that the traditional way to initialize the machine +ID in /etc/machine-id is to use +systemd-machine-id-setup by system +installer tools. You can also use +systemd-firstboot1 +to initialize the machine ID on mounted (but not +booted) system images. The main use case for that service is +/etc/machine-id being an empty file at +boot and initrd chaining to systemd giving it a read only file +system that will be turned read-write later during the boot +process. + +There is no consequence if that service fails other than +a newer machine-id will be generated during next system boot. + + + + +See Also + +systemd1, +systemd-machine-id-commit1, +systemd-machine-id-setup1, +machine-id5, +systemd-firstboot1 + + + + diff --git a/man/systemd-machine-id-commit.xml b/man/systemd-machine-id-commit.xml new file mode 100644 index 000..ed2a6d0 --- /dev/null +++ b/man/systemd-machine-id-commit.xml @@ -0,0 +1,125 @@ + +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";> + + + +http://www.w3.org/2001/XInclude";> + + +systemd-machine-id-commit +systemd + + + +Developer + Didier + Roche +didro...@ubuntu.com + + + + + +systemd-machine-i
[systemd-devel] [PATCH 4/5] machine-id-commit: add unit file
>From 6821788f561770ea7e49f170a5bea4a7033f6ad3 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Mon, 24 Nov 2014 10:12:06 +0100 Subject: [PATCH 4/5] machine-id-commit: add unit file The unit file only active the machine-id-commit helper if /etc is mounted writable and /etc/machine-id is an independant mount point (should be a tmpfs). --- Makefile.am| 5 + units/.gitignore | 1 + units/systemd-machine-id-commit.service.in | 20 3 files changed, 26 insertions(+) create mode 100644 units/systemd-machine-id-commit.service.in diff --git a/Makefile.am b/Makefile.am index 49e45d1..50f6f29 100644 --- a/Makefile.am +++ b/Makefile.am @@ -537,6 +537,7 @@ nodist_systemunit_DATA = \ units/systemd-kexec.service \ units/systemd-fsck@.service \ units/systemd-fsck-root.service \ + units/systemd-machine-id-commit.service \ units/systemd-udevd.service \ units/systemd-udev-trigger.service \ units/systemd-udev-settle.service \ @@ -589,6 +590,7 @@ EXTRA_DIST += \ units/user/systemd-exit.service.in \ units/systemd-f...@.service.in \ units/systemd-fsck-root.service.in \ + units/systemd-machine-id-commit.service.in \ units/u...@.service.in \ units/debug-shell.service.in \ units/systemd-suspend.service.in \ @@ -2189,6 +2191,9 @@ systemd_machine_id_commit_LDADD = \ libsystemd-internal.la \ libsystemd-shared.la +SYSINIT_TARGET_WANTS += \ + systemd-machine-id-commit.service + # -- systemd_ac_power_SOURCES = \ src/ac-power/ac-power.c diff --git a/units/.gitignore b/units/.gitignore index a1276e5..e12d299 100644 --- a/units/.gitignore +++ b/units/.gitignore @@ -25,6 +25,7 @@ /systemd-firstboot.service /systemd-fsck-root.service /systemd-fsck@.service +/systemd-machine-id-commit.service /systemd-halt.service /systemd-hibernate.service /systemd-hostnamed.service diff --git a/units/systemd-machine-id-commit.service.in b/units/systemd-machine-id-commit.service.in new file mode 100644 index 000..bed3d2e --- /dev/null +++ b/units/systemd-machine-id-commit.service.in @@ -0,0 +1,20 @@ +# This file is part of systemd. +# +# 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. + +[Unit] +Description=Commit a transient machine-id on disk +Documentation=man:systemd-machine-id-commit.service(8) +DefaultDependencies=no +Before=shutdown.target +After=local-fs.target +ConditionPathIsReadWrite=/etc +ConditionPathIsMountPoint=/etc/machine-id + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=@rootlibexecdir@/systemd-machine-id-commit -- 2.1.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/5] Introduce machine-id-commit binary
>From 6a9dfa52ea8a5564c86942ac55c9173ee06f53fe Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 24 Nov 2014 09:54:18 +0100
Subject: [PATCH 3/5] Introduce machine-id-commit binary
This binary enables to commit transient machine-id on disk if it becomes
writable.
---
.gitignore| 1 +
Makefile.am | 12
src/machine-id-commit/Makefile| 1 +
src/machine-id-commit/machine-id-commit.c | 105 ++
4 files changed, 119 insertions(+)
create mode 12 src/machine-id-commit/Makefile
create mode 100644 src/machine-id-commit/machine-id-commit.c
diff --git a/.gitignore b/.gitignore
index 2293ded..f6d3151 100644
--- a/.gitignore
+++ b/.gitignore
@@ -89,6 +89,7 @@
/systemd-kmsg-syslogd
/systemd-localed
/systemd-logind
+/systemd-machine-id-commit
/systemd-machine-id-setup
/systemd-machined
/systemd-modeset
diff --git a/Makefile.am b/Makefile.am
index 3f9f3fa..49e45d1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -385,6 +385,7 @@ rootlibexec_PROGRAMS = \
systemd-remount-fs \
systemd-reply-password \
systemd-fsck \
+ systemd-machine-id-commit \
systemd-ac-power \
systemd-sysctl \
systemd-sleep \
@@ -2178,6 +2179,17 @@ systemd_fsck_LDADD = \
libsystemd-shared.la
# --
+systemd_machine_id_commit_SOURCES = \
+ src/machine-id-commit/machine-id-commit.c \
+ src/core/machine-id-setup.c \
+ src/core/machine-id-setup.h
+
+systemd_machine_id_commit_LDADD = \
+ libsystemd-label.la \
+ libsystemd-internal.la \
+ libsystemd-shared.la
+
+# --
systemd_ac_power_SOURCES = \
src/ac-power/ac-power.c
diff --git a/src/machine-id-commit/Makefile b/src/machine-id-commit/Makefile
new file mode 12
index 000..d0b0e8e
--- /dev/null
+++ b/src/machine-id-commit/Makefile
@@ -0,0 +1 @@
+../Makefile
\ No newline at end of file
diff --git a/src/machine-id-commit/machine-id-commit.c b/src/machine-id-commit/machine-id-commit.c
new file mode 100644
index 000..c7e4de8
--- /dev/null
+++ b/src/machine-id-commit/machine-id-commit.c
@@ -0,0 +1,105 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2014 Didier Roche
+
+ 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
+#include
+#include
+#include
+#include
+
+#include "machine-id-setup.h"
+#include "log.h"
+#include "build.h"
+
+static const char *arg_root = "";
+
+static void help(void) {
+printf("%s [OPTIONS...]\n\n"
+ "Commit a transient /etc/machine-id on disk if writable.\n\n"
+ " -h --help Show this help\n"
+ " --version Show package version\n"
+ " --root=ROOTFilesystem root\n",
+ program_invocation_short_name);
+}
+
+static int parse_argv(int argc, char *argv[]) {
+
+enum {
+ARG_VERSION = 0x100,
+ARG_ROOT,
+};
+
+static const struct option options[] = {
+{ "help", no_argument, NULL, 'h' },
+{ "version", no_argument, NULL, ARG_VERSION },
+{ "root", required_argument, NULL, ARG_ROOT },
+{}
+};
+
+int c;
+
+assert(argc >= 0);
+assert(argv);
+
+while ((c = getopt_long(argc, argv, "hqcv", options, NULL)) >= 0)
+switch (c) {
+
+case 'h':
+help();
+return 0;
+
+case ARG_VERSION:
+puts(PACKAGE_STRING);
+puts(SYSTEMD_FEATURES);
+return 0;
+
+case ARG_ROOT:
+arg_root = optarg;
+break;
+
+case '?':
+return -EINVAL;
+
+default:
+assert_not_reached("Unhandled option");
+}
+
+if (opti
[systemd-devel] [PATCH 2/5] Add a machine_id_commit call to commit on disk a, transient machine-id
>From 7506756189164ad7512a896b5d74e4eb1b19a1a5 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 24 Nov 2014 09:43:29 +0100
Subject: [PATCH 2/5] Add a machine_id_commit call to commit on disk a
transient machine-id
If /etc was read only at boot time with an empty /etc/machine-id, the latter
will mounted as a tmpfs and resetted at each boot. If the system becomes rw
latter, this functionality enables to commit in a race-free manner the
transient machine-id to disk.
---
src/core/machine-id-setup.c | 122
src/core/machine-id-setup.h | 1 +
2 files changed, 123 insertions(+)
diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index 578bcfb..0cf5a73 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -25,7 +25,9 @@
#include
#include
#include
+#include
#include
+#include
#include "systemd/sd-id128.h"
@@ -187,6 +189,126 @@ static int write_machine_id(int fd, char id[34]) {
return -errno;
}
+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);
+}
+
+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;
+}
+
+/* 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;
+}
+
+fd = s
[systemd-devel] [PATCH 1/5] Factorize some machine-id-setup functions to be reused
>From b4619f01b6f25752220b2fe5c5ccd22e248f4015 Mon Sep 17 00:00:00 2001
From: Didier Roche
Date: Mon, 24 Nov 2014 09:40:57 +0100
Subject: [PATCH 1/5] Factorize some machine-id-setup functions to be reused
---
src/core/machine-id-setup.c | 41 +++--
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index ce6d8e0..578bcfb 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -159,6 +159,34 @@ static int generate(char id[34], const char *root) {
return 0;
}
+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;
+}
+
+static int write_machine_id(int fd, char id[34]) {
+assert(fd >= 0);
+assert(id);
+lseek(fd, 0, SEEK_SET);
+
+if (loop_write(fd, id, 33, false) == 33)
+return 0;
+
+return -errno;
+}
+
int machine_id_setup(const char *root) {
const char *etc_machine_id, *run_machine_id;
_cleanup_close_ int fd = -1;
@@ -211,13 +239,8 @@ int machine_id_setup(const char *root) {
return -errno;
}
-if (S_ISREG(st.st_mode))
-if (loop_read(fd, id, 33, false) == 33 && id[32] == '\n') {
-id[32] = 0;
-
-if (id128_is_valid(id))
-return 0;
-}
+if (S_ISREG(st.st_mode) && get_valid_machine_id(fd, id) == 0)
+return 0;
/* Hmm, so, the id currently stored is not useful, then let's
* generate one */
@@ -227,9 +250,7 @@ int machine_id_setup(const char *root) {
return r;
if (S_ISREG(st.st_mode) && writable) {
-lseek(fd, 0, SEEK_SET);
-
-if (loop_write(fd, id, 33, false) == 33)
+if (write_machine_id(fd, id) == 0)
return 0;
}
--
2.1.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Hosts without /etc/machine-id on boot
Le 21/11/2014 00:41, Lennart Poettering a écrit : On Thu, 20.11.14 17:23, Didier Roche (didro...@ubuntu.com) wrote: a) make /etc writable before systemd is invoked. If you use an initrd this is without risk, given that the initrd should really invoke fsck on the root disk anyway, and there's hence little reason to transition to a read-only root, rather than just doing rw right-away. Interesting, I run that through our kernel team. However, we run fsck a little bit later on in the boot process to be able to pipe the output to plymouth. At least on Fedora plymouth already runs on the initrd. If Ubuntu does the same, then there shouldn't be a difference regarding where fsck is run... Note that running fsck in the initrd for the root fs is really the right thing to do: running fsck from the file system you are about to check, which you hence cannot trust, is really wrong. I'm not sure we should then have two code paths: - one fscking from the initrd if /etc/machine-id is empty (like after a factory reset), showing the results and eventual failures to the user in some way - and then, the general use case: fscking through the systemd service via systemd-fsck-root.service before local-fs.target and piping the result in plymouth The latter is useful only really on non-initrd boots where there isn't any initrd where the fsck could run. General purpose distributions should really run fsck in the initrd. Note that systemd-fsck-root.service skips itself when it notices that the fs was already checked (via a flag file in /run). Indeed, that makes sense and we should investigate that with our kernel team in the near future. I'm going to open a thread on it. The guarantee with /etc/machine-id is really that it is valid at *any* time, in early boot and late boot and all the time in between. I think I will go that path which is an interesting one and mapping some of my thoughts. Thanks for the guidance and documentation on what's the right approach to achieve this race-free! I'll work on something around that and propose a patch. Looking forword to it. Here we go :) I did some factorization to reuse some functions on the first path and added the binary helper, unit and man page. It should follow your advice and be race-free. Tested various cases locally. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Hosts without /etc/machine-id on boot
Le 20/11/2014 13:45, Lennart Poettering a écrit : On Wed, 19.11.14 09:45, Didier Roche (didro...@ubuntu.com) wrote: Hey, Some other topic related to "empty /etc" discussions: when preparing some generic distro images, we are have the desire to ensure that all new instances will get a different /etc/machine-id file. As part of the empty /etc at boot, we first thought that removing /etc/machine-id would be sufficient, however, the instance then doesn't generate a new machine-id file and complain heavily. The new debug message of systemd 216+ helped shading some lights on it: http://cgit.freedesktop.org/systemd/systemd-stable/diff/src/core/machine-id-setup.c?h=v216-stable&id=896050eeb3acbf4106d71204a5173b4984cf1675, and adding debug statement in machine_id_setup() from src/core/machine-id-setup.c just before "open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444)" explains what happens with /proc/mounts: [2.119041] systemd[1]: rootfs / rootfs rw [2.126775] systemd[1]: /dev/disk/by-uuid/ec8166e5-d5ed-45ec-b350-6cf5773904ac / ext4 ro,relatime,data=ordered It's clear then that at this stage of the boot process / is readonly. The error message (and code) will say that in this case, what is supported is an empty /etc/machine-id. After reboot, the consequence is that /etc/machine-id is mounted as a tmpfs: Yes, generation of the machine ID is done very very early at boot, before we fork off the first non-PID1 userspace process, and hence before any file system could be remounted writable. That makes sense. tmpfs on /etc/machine-id type tmpfs (ro,relatime,size=204948k,mode=755) However, this means is that each boot of this instance will result in a different machine-id, which isn't what is desired in the empty /etc case after a factory reset. I know that there is the utility systemd-machine-id-setup that we are running on systemd postinst in debian/ubuntu, but that doesn't cover the factory reset one. Is there anything obvious that I'm missing to cover that case or anything in the pipe? You have a couple of options: a) make /etc writable before systemd is invoked. If you use an initrd this is without risk, given that the initrd should really invoke fsck on the root disk anyway, and there's hence little reason to transition to a read-only root, rather than just doing rw right-away. Interesting, I run that through our kernel team. However, we run fsck a little bit later on in the boot process to be able to pipe the output to plymouth. I'm not sure we should then have two code paths: - one fscking from the initrd if /etc/machine-id is empty (like after a factory reset), showing the results and eventual failures to the user in some way - and then, the general use case: fscking through the systemd service via systemd-fsck-root.service before local-fs.target and piping the result in plymouth b) pre-initialize the machine ID before you boot, at build time. c) live with random ids Those 2 are actually nice features, but not applying with a machine if we factory reset it with an empty /etc. d) pass in the id to use via $container_uuid (if you use a container manager), or via the DMI uuid field (if you use kvm). Then, create /etc/machine-id as an empty file, and systemd will initialize it to this ID rather than a random one. Usually, option d) is preferable in cloud setups I guess since it allows seeding the machine id from some externally used UUID, the way many container/virtualization managers define one anyway. Fully agreed (not the case I showed up here, but nice to know we can pass pre-generated uuid to containers/vms). I'd be open to add another option on top of this: e) boot up with /etc read-only and /etc/machine-id empty, so that the usualy logic of c) generates a random machine id and overmounts /etc/machine-id with it. But then, add a tiny new bootup service, that runs shortly after local-fs.target (i.e. the point where /etc/ has been made writable if it's supposed to be made writable according to fstab), and that syncs the random one used so far back to disk, so that at the next boot-up it is fully initialized. This tiny service should be properly conditioned so that it only runs if /etc/machine-id is overmounted and /etc writable (i.e. ConditionPathIsMountPoint=/etc/machine-id and ConditionPathIsReadWrite=/etc). Special care should be taken so that replacement of the mount by the normal file is race-free. (This probably means the tool should open a new ount namespace temporarily, unmount /etc/machine-id there, update the file undearneath and then return to the original mount namespace and unmount the file there too, so that at no time the filw is invalid). The guarantee with /etc/machine-id is really that it is valid at *any* time, in early boot and late boot and all the time in between. I t
[systemd-devel] Hosts without /etc/machine-id on boot
Hey, Some other topic related to "empty /etc" discussions: when preparing some generic distro images, we are have the desire to ensure that all new instances will get a different /etc/machine-id file. As part of the empty /etc at boot, we first thought that removing /etc/machine-id would be sufficient, however, the instance then doesn't generate a new machine-id file and complain heavily. The new debug message of systemd 216+ helped shading some lights on it: http://cgit.freedesktop.org/systemd/systemd-stable/diff/src/core/machine-id-setup.c?h=v216-stable&id=896050eeb3acbf4106d71204a5173b4984cf1675, and adding debug statement in machine_id_setup() from src/core/machine-id-setup.c just before "open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444)" explains what happens with /proc/mounts: [2.119041] systemd[1]: rootfs / rootfs rw [2.126775] systemd[1]: /dev/disk/by-uuid/ec8166e5-d5ed-45ec-b350-6cf5773904ac / ext4 ro,relatime,data=ordered It's clear then that at this stage of the boot process / is readonly. The error message (and code) will say that in this case, what is supported is an empty /etc/machine-id. After reboot, the consequence is that /etc/machine-id is mounted as a tmpfs: tmpfs on /etc/machine-id type tmpfs (ro,relatime,size=204948k,mode=755) However, this means is that each boot of this instance will result in a different machine-id, which isn't what is desired in the empty /etc case after a factory reset. I know that there is the utility systemd-machine-id-setup that we are running on systemd postinst in debian/ubuntu, but that doesn't cover the factory reset one. Is there anything obvious that I'm missing to cover that case or anything in the pipe? Cheers, Didier ___ 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
Le 18/11/2014 17:17, Tom Gundersen a écrit : On Tue, Nov 18, 2014 at 4:21 PM, Didier Roche wrote: Let's say as an admin that I want to disable plymouth-quit.service (which is a static unit file and symlinked in /usr/lib… on the multi-user target). Without knowing the systemd internals, my natural intent would be to use "systemctl disable plymouth-quit.service" which is no-op in this case on a static unit enabled on the multi-user target with the symlink shipped by the package. This may be perceived as unnatural to him to have to use "systemctl mask" to disable it, or am I just being too pessimistic? Right, I get it. In this case we should definitely warn when someone tries to disable a statically enabled unit. We should suggest to the user that this package is not meant to be disabled, but one can use masking instead (voiding the warranty, etc, etc). Agreed, this one is as well orthogonal to this discussion anyway and should be implemented in any case for static units, warning people to use mask for those cases (voiding the distro warranty as you told :)) /me notes that down as another patch to work on. Cheers, Didier ___ 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
Le 18/11/2014 15:59, Colin Guthrie a écrit : Hiya, Hey, 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. This also avoids any problems here. (Of course if /etc is nuked, then reapplying the defaults from *.preset should be done!) See my Michael's answer (and my previous one) on the fact that maybe the preset files would be part of multiple packages (to disable certain units), and some being only part of packages not installed by default. Michael's case as well of a unit changing its target on a package upgrade (as a packaging fix, maybe) is valid as well. With a shared distro/admin /etc, we have no way to take that fact into account and the next one would follow distro policy, right? Yeah, but that's assuming there *is* a next one. Once things are installed, the user should *not* be surprised by any action being taken without their consent on upgrade. FWIW, it's probably worth considering how you'd handle not changing users current preferences with regards to enabling/disabling things on upgrade. I can see this being quite hard for you roll out with your current suggestions, but you may have this covered already. Actually, this reminds me some issues we had with gconf in the past in changing distribution's default and deciphering what was users current preference or what was distro default behavior. Gnome-panel was copying its whole distro defaults in ~/.gconf. Adding/removing some default layouts and settings from the panel was then unnecessary difficult. Some changes was part of new default behaviors we wanted that the user never interacted with and was desired to be changed (because of some applets getting low maintenance, incompatible with newer technology or duplicates…) As everything was at the same place, it was difficult to know if the current value was set because of the previous default, or if the user explicitly tweaked and then selected it. I see the same issue with the shared /etc: is this unit enabled for that target because of one the preset config, or did the admin run "systemctl enable " explicitly and want to keep it? I think it's ok to change distro defaults on upgrade (will be potentially in major version upgrade of course), not user (admin here) preferences, of course. But we do have no way to know for sure which is which in the current system. Also, after running systemctl enable opensshn, systemctl status openssh will still say "enabled (preset)" even if the admin wanted to "stick it for good" as it's part of the preset. Not sure what you mean by "stick it for good" here, but my previous suggestion was to say "enabled [preset: enabled]" or "enabled [preset: disabled]" accordingly which might be clearer (if a bit longer). Same than in previous case: I have a preset with enable docker.socket systemctl status docker.socket -> "enabled [preset: enabled]" systectl enable docker.socket systemctl status docker.socket -> "enabled [preset: enabled]" I guess, it should then just be "enabled". Maybe that will then ask for a systemctl reset command or something like that… Doing 'enable' on a preset unit will then just delete the symlink to /dev/null from /etc (if it exists) and doing 'disable' will add it. This would also entail changing the current logic to check the target of /**/*.wants.d/ symlinks to see if they point to /dev/null, in which case they should be ignored. Did I understand that correctly? Right, maybe the implementation can diverge a little bit from that, but the intent would cover most of the admin/distro separation, while enabling sysadmin to "stick" some of the services disregarding future distro choices and keeping a clean /etc. Again, just to clarify, the current implementation intends that "systemctl preset myservice.service" is only run on the first installation of a package, not on upgrades. There should be no need to "stick" some of the services as distro choices are only applied at install time, and never on upgrade. - 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. I agree, this sh
