Discussion:
[PATCH 1/9] fsckd daemon for inter-fsckd communication
(too old to reply)
Didier Roche
2015-02-05 17:06:50 UTC
Permalink
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
Didier Roche
2015-02-05 17:08:59 UTC
Permalink
Didier Roche
2015-02-18 11:33:24 UTC
Permalink
Didier Roche
2015-02-05 17:09:24 UTC
Permalink
Zbigniew Jędrzejewski-Szmek
2015-02-14 16:38:24 UTC
Permalink
From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
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.
fsckd:<num_devices>:<progress>:<string>
* 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:<string>
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.
+ 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.
+ 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)
log_error("fsck failed with error code %i.", status.si_status);
- else
+ else if (progress_rc != 0)
log_error("fsck failed due to unknown reason.");
if (status.si_code == CLD_EXITED && (status.si_status & 2) && root_directory)
@@ -362,7 +376,8 @@ int main(int argc, char *argv[]) {
start_target(SPECIAL_EMERGENCY_TARGET);
else {
r = EXIT_SUCCESS;
- log_warning("Ignoring error.");
+ if (progress_rc != 0)
+ log_warning("Ignoring error.");
}
} else
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 45b8d64..f24f8c1 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -34,6 +34,7 @@
#include <unistd.h>
#include "build.h"
+#include "def.h"
#include "event-util.h"
#include "fsckd.h"
#include "log.h"
@@ -44,6 +45,7 @@
#include "util.h"
#define IDLE_TIME_SECONDS 30
+#define PLYMOUTH_REQUEST_KEY "K"
struct Manager;
@@ -56,6 +58,7 @@ typedef struct Client {
int pass;
double percent;
size_t buflen;
+ bool cancelled;
LIST_FIELDS(struct Client, clients);
} Client;
@@ -68,8 +71,13 @@ typedef struct Manager {
FILE *console;
double percent;
int numdevices;
+ int plymouth_fd;
+ bool plymouth_cancel_sent;
+ bool cancel_requested;
} Manager;
+static int connect_plymouth(Manager *m);
+static int update_global_progress(Manager *m);
static void manager_free(Manager *m);
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
#define _cleanup_manager_free_ _cleanup_(manager_freep)
@@ -92,6 +100,18 @@ static double compute_percent(int pass, size_t cur, size_t max) {
(double) cur / max;
}
+static int request_cancel_client(Client *current) {
+ FsckdMessage cancel_msg;
+ ssize_t n;
+ cancel_msg.cancel = true;
+
+ 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.
+ else
+ current->cancelled = true;
+ return 0;
+}
static void remove_client(Client **first, Client *item) {
LIST_REMOVE(clients, *first, item);
@@ -99,10 +119,91 @@ static void remove_client(Client **first, Client *item) {
free(item);
}
+static void on_plymouth_disconnect(Manager *m) {
+ safe_close(m->plymouth_fd);
+ m->plymouth_fd = -1;
+ m->plymouth_cancel_sent = false;
+}
+
+static int plymouth_feedback_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+ Manager *m = userdata;
+ Client *current;
+ char buffer[6];
+ int r;
+
+ assert(m);
+
+ r = read(m->plymouth_fd, buffer, sizeof(buffer));
+ if (r <= 0)
+ on_plymouth_disconnect(m);
+ else {
+ if (buffer[0] == '\15')
+ log_error("Message update to plymouth wasn't delivered successfully");
+
+ /* the only answer support type we requested is a key interruption */
+ if (buffer[0] == '\2' && buffer[5] == 'c') {
+ m->cancel_requested = true;
+ /* cancel all connected clients */
+ LIST_FOREACH(clients, current, m->clients)
+ request_cancel_client(current);
+ }
+ }
+
+ return 0;
+}
+
+static int send_message_plymouth_socket(int plymouth_fd, const char *message, bool update) {
+ _cleanup_free_ char *packet = NULL;
+ int r, n;
+ char mode = 'M';
+
+ if (update)
+ mode = 'U';
+
+ if (asprintf(&packet, "%c\002%c%s%n", mode, (int) (strlen(message) + 1), message, &n) < 0)
+ return log_oom();
+ r = loop_write(plymouth_fd, packet, n + 1, true);
+ return r;
+}
+
+
+static int send_message_plymouth(Manager *m, const char *message) {
+ int r;
+ const char *plymouth_cancel_message = NULL;
+
+ r = connect_plymouth(m);
+ if (r < 0)
+ return r;
+
+ 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...
+ 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 ;) Also "clear ... to".
+ }
+
+ r = send_message_plymouth_socket(m->plymouth_fd, message, true);
+ if (r < 0)
+ return log_warning_errno(errno, "Couldn't send %s to plymouth: %m", message);
\"%s\" would be better, otherwise this can be hard to parse.
Can the message contain non-utf-8 data?
+
+ return 0;
+}
+
static int update_global_progress(Manager *m) {
Client *current = NULL;
_cleanup_free_ char *console_message = NULL;
- int current_numdevices = 0, l = 0;
+ _cleanup_free_ char *fsck_message = NULL;
+ int current_numdevices = 0, l = 0, r;
double current_percent = 100;
/* get the overall percentage */
@@ -123,6 +224,8 @@ static int update_global_progress(Manager *m) {
if (asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
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;
/* write to console */
if (m->console) {
@@ -130,12 +233,41 @@ static int update_global_progress(Manager *m) {
fflush(m->console);
}
+ /* 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 (l > m->clear)
m->clear = l;
}
return 0;
}
+static int connect_plymouth(Manager *m) {
+ union sockaddr_union sa = PLYMOUTH_SOCKET;
+ int r;
+
+ /* try to connect or reconnect if sending a message */
+ if (m->plymouth_fd <= 0) {
+ m->plymouth_fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+ if (m->plymouth_fd < 0) {
+ return log_warning_errno(errno, "Connection to plymouth socket failed: %m");
+ }
+ if (connect(m->plymouth_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sa.un.sun_path+1)) < 0) {
+ on_plymouth_disconnect(m);
+ return log_warning_errno(errno, "Couldn't connect to plymouth: %m");
+ }
+ r = sd_event_add_io(m->event, NULL, m->plymouth_fd, EPOLLIN, plymouth_feedback_handler, m);
+ if (r < 0) {
+ on_plymouth_disconnect(m);
+ return log_warning_errno(r, "Can't listen to plymouth socket: %m");
+ }
+ }
+
+ return 0;
+}
+
static int progress_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
Client *client = userdata;
Manager *m = NULL;
@@ -146,6 +278,12 @@ static int progress_handler(sd_event_source *s, int fd, uint32_t revents, void *
assert(client);
m = client->manager;
+ /* check first if we need to cancel this client */
+ if (m->cancel_requested) {
+ if (!client->cancelled)
+ request_cancel_client(client);
+ }
+
/* ensure we have enough data to read */
r = ioctl(fd, FIONREAD, &buflen);
if (r == 0 && buflen != 0 && (size_t) buflen < sizeof(FsckProgress)) {
@@ -209,6 +347,9 @@ static int new_connection_handler(sd_event_source *s, int fd, uint32_t revents,
remove_client(&(m->clients), client);
return r;
}
+ /* only request the client to cancel now in case the request is dropped by the client (chance to recancel) */
+ if (m->cancel_requested)
+ request_cancel_client(client);
} else
return log_error_errno(errno, "Couldn't accept a new connection: %m");
@@ -232,6 +373,7 @@ static void manager_free(Manager *m) {
}
safe_close(m->connection_fd);
+ safe_close(m->plymouth_fd);
if (m->console)
fclose(m->console);
@@ -265,6 +407,7 @@ static int manager_new(Manager **ret, int fd) {
}
m->percent = 100;
+ m->plymouth_fd = -1;
*ret = m;
m = NULL;
diff --git a/src/fsckd/fsckd.h b/src/fsckd/fsckd.h
index 6fe37a7..61399cf 100644
--- a/src/fsckd/fsckd.h
+++ b/src/fsckd/fsckd.h
@@ -24,6 +24,7 @@
#define FSCKD_SOCKET_PATH "/run/systemd/fsckd"
+#include <stdbool.h>
#include "libudev.h"
typedef struct FsckProgress {
@@ -32,3 +33,7 @@ typedef struct FsckProgress {
size_t max;
int pass;
} FsckProgress;
+
+typedef struct FsckdMessage {
+ bool cancel;
+} FsckdMessage;
bool as the mechanism for data passing is not that good. Depending on
the architecture, it can have different sizes, iirc. (I can imaging
that for whatever reason user ends up runnning 32bit fsckd with 64bit
plymouth. Not likely, but better not to have to think about this at
all). Maybe you could make it uint8_t, which would have the advantage
of being trivially endianness independent.

Zbyszek
Didier Roche
2015-02-17 16:26:05 UTC
Permalink
Post by Zbigniew Jędrzejewski-Szmek
From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
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.
fsckd:<num_devices>:<progress>:<string>
* 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:<string>
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.
Post by Zbigniew Jędrzejewski-Szmek
+ 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.
Post by Zbigniew Jędrzejewski-Szmek
+
+ 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.
Post by Zbigniew Jędrzejewski-Szmek
+
+ 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)
Post by Zbigniew Jędrzejewski-Szmek
+ 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.
Post by Zbigniew Jędrzejewski-Szmek
Also "clear ... to".
isn't what I used above? "clear 
 to"
Post by Zbigniew Jędrzejewski-Szmek
+ }
+
+ r = send_message_plymouth_socket(m->plymouth_fd, message, true);
+ if (r < 0)
+ return log_warning_errno(errno, "Couldn't send %s to plymouth: %m", message);
\"%s\" would be better, otherwise this can be hard to parse.
Can the message contain non-utf-8 data?
The po files state charset=UTF-8, but if one language tweak that, I
guess that could happen. In practice, that didn't occur in ubuntu from
what I know when sent to plymouth.
Post by Zbigniew Jędrzejewski-Szmek
typedef struct FsckProgress {
@@ -32,3 +33,7 @@ typedef struct FsckProgress {
size_t max;
int pass;
} FsckProgress;
+
+typedef struct FsckdMessage {
+ bool cancel;
+} FsckdMessage;
bool as the mechanism for data passing is not that good. Depending on
the architecture, it can have different sizes, iirc. (I can imaging
that for whatever reason user ends up runnning 32bit fsckd with 64bit
plymouth. Not likely, but better not to have to think about this at
all). Maybe you could make it uint8_t, which would have the advantage
of being trivially endianness independent.
Sure, doing that change.

Not that the FsckdMessage is only sent to systemd-fsck, not plymouth, so
it's only systemd inter-services communication. I do hope that a user
won't install 2 versions of systemd compiled for different archs and one
opens the socket with a 32 bits version, the other (64 bits) write to it :)
Anyway, good advice, and changed to an uint8_t.
Post by Zbigniew Jędrzejewski-Szmek
Zbyszek
Zbigniew Jędrzejewski-Szmek
2015-02-17 23:30:52 UTC
Permalink
Post by Didier Roche
Post by Zbigniew Jędrzejewski-Szmek
From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
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.
fsckd:<num_devices>:<progress>:<string>
* 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:<string>
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.
Post by Zbigniew Jędrzejewski-Szmek
+ 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.
Post by Zbigniew Jędrzejewski-Szmek
+
+ 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.
CODING_STYLE.

There's a bit of a inconsitency about good line length limits, but
this one wraps two times in my window which I consider too much :)
Post by Didier Roche
Post by Zbigniew Jędrzejewski-Szmek
+
+ 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)
Post by Zbigniew Jędrzejewski-Szmek
+ 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.
Post by Zbigniew Jędrzejewski-Szmek
Also "clear ... to".
isn't what I used above? "clear … to"
Well, yes, but still this sentence doesn't make sense.
Post by Didier Roche
Post by Zbigniew Jędrzejewski-Szmek
+
+ r = send_message_plymouth_socket(m->plymouth_fd, message, true);
+ if (r < 0)
+ return log_warning_errno(errno, "Couldn't send %s to plymouth: %m", message);
\"%s\" would be better, otherwise this can be hard to parse.
Can the message contain non-utf-8 data?
The po files state charset=UTF-8, but if one language tweak that, I
guess that could happen. In practice, that didn't occur in ubuntu
from what I know when sent to plymouth.
Hm, I guess it's good enough for now. This is essentially
trusted data.
Post by Didier Roche
Post by Zbigniew Jędrzejewski-Szmek
typedef struct FsckProgress {
@@ -32,3 +33,7 @@ typedef struct FsckProgress {
size_t max;
int pass;
} FsckProgress;
+
+typedef struct FsckdMessage {
+ bool cancel;
+} FsckdMessage;
bool as the mechanism for data passing is not that good. Depending on
the architecture, it can have different sizes, iirc. (I can imaging
that for whatever reason user ends up runnning 32bit fsckd with 64bit
plymouth. Not likely, but better not to have to think about this at
all). Maybe you could make it uint8_t, which would have the advantage
of being trivially endianness independent.
Sure, doing that change.
Not that the FsckdMessage is only sent to systemd-fsck, not
plymouth, so it's only systemd inter-services communication. I do
hope that a user won't install 2 versions of systemd compiled for
different archs and one opens the socket with a 32 bits version, the
other (64 bits) write to it :)
Anyway, good advice, and changed to an uint8_t.
Post by Zbigniew Jędrzejewski-Szmek
Zbyszek
From e6400709f603cde406b8d3b605ca0e1dfd10ba3c Mon Sep 17 00:00:00 2001
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.
fsckd:<num_devices>:<progress>:<string>
* 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:<string>
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 c7461d2..efd6b5b 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 = NULL;
_cleanup_udev_device_unref_ struct udev_device *udev_device = NULL;
@@ -329,7 +342,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;
}
@@ -339,13 +352,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)
log_error("fsck failed with error code %i.", status.si_status);
- else
+ else if (progress_rc != 0)
log_error("fsck failed due to unknown reason.");
if (status.si_code == CLD_EXITED && (status.si_status & 2) && root_directory)
@@ -356,7 +370,8 @@ int main(int argc, char *argv[]) {
start_target(SPECIAL_EMERGENCY_TARGET);
else {
r = EXIT_SUCCESS;
- log_warning("Ignoring error.");
+ if (progress_rc != 0)
+ log_warning("Ignoring error.");
}
} else
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 6b2eeb0..30c4f28 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -34,6 +34,7 @@
#include <unistd.h>
#include "build.h"
+#include "def.h"
#include "event-util.h"
#include "fsckd.h"
#include "log.h"
@@ -44,6 +45,7 @@
#include "util.h"
#define IDLE_TIME_SECONDS 30
+#define PLYMOUTH_REQUEST_KEY "K"
struct Manager;
@@ -56,6 +58,7 @@ typedef struct Client {
int pass;
double percent;
size_t buflen;
+ bool cancelled;
LIST_FIELDS(struct Client, clients);
} Client;
@@ -68,8 +71,13 @@ typedef struct Manager {
FILE *console;
double percent;
int numdevices;
+ int plymouth_fd;
+ bool plymouth_cancel_sent;
+ bool cancel_requested;
} Manager;
+static int connect_plymouth(Manager *m);
+static int update_global_progress(Manager *m);
static void manager_free(Manager *m);
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
#define _cleanup_manager_free_ _cleanup_(manager_freep)
@@ -92,6 +100,19 @@ static double compute_percent(int pass, size_t cur, size_t max) {
(double) cur / max;
}
+static int request_cancel_client(Client *current) {
+ FsckdMessage cancel_msg;
+ ssize_t n;
+ cancel_msg.cancel = 1;
+
+ 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));
+ else
+ current->cancelled = true;
+ return 0;
+}
static void remove_client(Client **first, Client *item) {
LIST_REMOVE(clients, *first, item);
@@ -99,10 +120,91 @@ static void remove_client(Client **first, Client *item) {
free(item);
}
+static void on_plymouth_disconnect(Manager *m) {
+ safe_close(m->plymouth_fd);
+ m->plymouth_fd = -1;
+ m->plymouth_cancel_sent = false;
+}
+
+static int plymouth_feedback_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+ Manager *m = userdata;
+ Client *current;
+ char buffer[6];
+ int r;
+
+ assert(m);
+
+ r = read(m->plymouth_fd, buffer, sizeof(buffer));
+ if (r <= 0)
+ on_plymouth_disconnect(m);
+ else {
+ if (buffer[0] == '\15')
+ log_error("Message update to plymouth wasn't delivered successfully");
+
+ /* the only answer support type we requested is a key interruption */
+ if (buffer[0] == '\2' && buffer[5] == 'c') {
+ m->cancel_requested = true;
+ /* cancel all connected clients */
+ LIST_FOREACH(clients, current, m->clients)
+ request_cancel_client(current);
+ }
+ }
+
+ return 0;
+}
+
+static int send_message_plymouth_socket(int plymouth_fd, const char *message, bool update) {
+ _cleanup_free_ char *packet = NULL;
+ int r, n;
+ char mode = 'M';
+
+ if (update)
+ mode = 'U';
+
+ if (asprintf(&packet, "%c\002%c%s%n", mode, (int) (strlen(message) + 1), message, &n) < 0)
+ return log_oom();
+ r = loop_write(plymouth_fd, packet, n + 1, true);
+ return r;
+}
+
+
+static int send_message_plymouth(Manager *m, const char *message) {
+ int r;
+ const char *plymouth_cancel_message = NULL;
+
+ r = connect_plymouth(m);
+ if (r < 0)
+ return r;
+
+ 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 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");
+ } 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 filesystem cancel message to plymouth: %m");
+ }
+
+ r = send_message_plymouth_socket(m->plymouth_fd, message, true);
+ if (r < 0)
+ return log_warning_errno(errno, "Couldn't send \"%s\" to plymouth: %m", message);
+
+ return 0;
+}
+
static int update_global_progress(Manager *m) {
Client *current = NULL;
_cleanup_free_ char *console_message = NULL;
- int current_numdevices = 0, l = 0;
+ _cleanup_free_ char *fsck_message = NULL;
+ int current_numdevices = 0, l = 0, r;
double current_percent = 100;
/* get the overall percentage */
@@ -123,6 +225,8 @@ static int update_global_progress(Manager *m) {
if (asprintf(&console_message, "Checking in progress on %d disks (%3.1f%% complete)",
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;
/* write to console */
if (m->console) {
@@ -130,12 +234,41 @@ static int update_global_progress(Manager *m) {
fflush(m->console);
}
+ /* 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 (l > m->clear)
m->clear = l;
}
return 0;
}
+static int connect_plymouth(Manager *m) {
+ union sockaddr_union sa = PLYMOUTH_SOCKET;
+ int r;
+
+ /* try to connect or reconnect if sending a message */
+ if (m->plymouth_fd <= 0) {
+ m->plymouth_fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+ if (m->plymouth_fd < 0) {
+ return log_warning_errno(errno, "Connection to plymouth socket failed: %m");
+ }
+ if (connect(m->plymouth_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sa.un.sun_path+1)) < 0) {
+ on_plymouth_disconnect(m);
+ return log_warning_errno(errno, "Couldn't connect to plymouth: %m");
+ }
+ r = sd_event_add_io(m->event, NULL, m->plymouth_fd, EPOLLIN, plymouth_feedback_handler, m);
+ if (r < 0) {
+ on_plymouth_disconnect(m);
+ return log_warning_errno(r, "Can't listen to plymouth socket: %m");
+ }
+ }
+
+ return 0;
+}
+
static int progress_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
Client *client = userdata;
Manager *m = NULL;
@@ -146,6 +279,12 @@ static int progress_handler(sd_event_source *s, int fd, uint32_t revents, void *
assert(client);
m = client->manager;
+ /* check first if we need to cancel this client */
+ if (m->cancel_requested) {
+ if (!client->cancelled)
+ request_cancel_client(client);
+ }
+
/* ensure we have enough data to read */
r = ioctl(fd, FIONREAD, &buflen);
if (r == 0 && buflen != 0 && (size_t) buflen < sizeof(FsckProgress)) {
@@ -210,6 +349,9 @@ static int new_connection_handler(sd_event_source *s, int fd, uint32_t revents,
remove_client(&(m->clients), client);
return r;
}
+ /* only request the client to cancel now in case the request is dropped by the client (chance to recancel) */
+ if (m->cancel_requested)
+ request_cancel_client(client);
} else
return log_error_errno(errno, "Couldn't accept a new connection: %m");
@@ -233,6 +375,7 @@ static void manager_free(Manager *m) {
}
safe_close(m->connection_fd);
+ safe_close(m->plymouth_fd);
if (m->console)
fclose(m->console);
@@ -266,6 +409,7 @@ static int manager_new(Manager **ret, int fd) {
}
m->percent = 100;
+ m->plymouth_fd = -1;
*ret = m;
m = NULL;
diff --git a/src/fsckd/fsckd.h b/src/fsckd/fsckd.h
index 6fe37a7..8239273 100644
--- a/src/fsckd/fsckd.h
+++ b/src/fsckd/fsckd.h
@@ -32,3 +32,7 @@ typedef struct FsckProgress {
size_t max;
int pass;
} FsckProgress;
+
+typedef struct FsckdMessage {
+ uint8_t cancel;
+} FsckdMessage;
--
2.1.4
Didier Roche
2015-02-18 11:33:40 UTC
Permalink
Post by Zbigniew Jędrzejewski-Szmek
Post by Didier Roche
Post by Zbigniew Jędrzejewski-Szmek
+ 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.
Post by Zbigniew Jędrzejewski-Szmek
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
Didier Roche
2015-02-05 17:09:52 UTC
Permalink
Didier Roche
2015-02-18 11:33:46 UTC
Permalink
Didier Roche
2015-02-05 17:10:13 UTC
Permalink
Didier Roche
2015-02-17 16:26:57 UTC
Permalink
Updated to latest (impacted by a change in 3/9)

Didier
Didier Roche
2015-02-18 11:34:09 UTC
Permalink
Didier Roche
2015-02-05 17:10:36 UTC
Permalink
Didier Roche
2015-02-17 16:27:12 UTC
Permalink
Updated to latest (impacted by a change in 3/9)

Didier
Didier Roche
2015-02-18 11:34:12 UTC
Permalink
Didier Roche
2015-02-05 17:10:57 UTC
Permalink
Didier Roche
2015-02-18 11:34:17 UTC
Permalink
Didier Roche
2015-02-05 17:11:24 UTC
Permalink
Zbigniew Jędrzejewski-Szmek
2015-02-14 16:58:54 UTC
Permalink
From 2533acf15135d9db18fbd79e63de9a702e3859cc Mon Sep 17 00:00:00 2001
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-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-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-fsckd.8: man/systemd-fsckd.service.8
+man/systemd-fsckd.socket.8: man/systemd-fsckd.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-***@.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)
+
$(html-alias)
@@ -1732,6 +1743,7 @@ EXTRA_DIST += \
man/systemd-escape.xml \
man/systemd-firstboot.xml \
+ man/systemd-fsckd.service.xml \
man/systemd-fstab-generator.xml \
man/systemd-getty-generator.xml \
man/systemd-gpt-auto-generator.xml \
index ee66f37..d366712 100644
@@ -87,8 +87,9 @@
check, number of mounts, unclean unmount, etc.</para>
<para><filename>systemd-fsck</filename> will forward
- file system checking progress to the console. If a
- file system check fails for a service without
+ file system checking progress to
+ <filename>systemd-fsckd.service</filename>
+ socket. If a file system check fails for a service without
<option>nofail</option>, emergency mode is activated,
by isolating to
<filename>emergency.target</filename>.</para>
@@ -142,6 +143,7 @@
<para>
<citerefentry><refentrytitle>systemd</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry><refentrytitle>fsck</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>systemd-fsckd.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
<citerefentry><refentrytitle>systemd-quotacheck.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
<citerefentry><refentrytitle>fsck.btrfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
<citerefentry><refentrytitle>fsck.cramfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
diff --git a/man/systemd-fsckd.service.xml b/man/systemd-fsckd.service.xml
new file mode 100644
index 0000000..4a3b60d
--- /dev/null
+++ b/man/systemd-fsckd.service.xml
@@ -0,0 +1,165 @@
+<?xml version="1.0"?>
+<!--*-nxml-*-->
+<!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd">
+<!--
+ 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/>.
+-->
+<refentry id="systemd-fsckd.service" xmlns:xi="http://www.w3.org/2001/XInclude">
+
+ <refentryinfo>
+ <title>systemd-fsckd.service</title>
+ <productname>systemd</productname>
+
+ <authorgroup>
+ <author>
+ <contrib>Developer</contrib>
+ <firstname>Didier</firstname>
+ <surname>Roche</surname>
+ </author>
+ </authorgroup>
+ </refentryinfo>
+
+ <refmeta>
+ <refentrytitle>systemd-fsckd.service</refentrytitle>
+ <manvolnum>8</manvolnum>
+ </refmeta>
+
+ <refnamediv>
+ <refname>systemd-fsckd.service</refname>
+ <refname>systemd-fsckd.socket</refname>
+ <refname>systemd-fsckd</refname>
+ <refpurpose>File system check progress reporting</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+ <para><filename>systemd-fsckd.service</filename></para>
+ <para><filename>systemd-fsckd.socket</filename></para>
+ <para><filename>/usr/lib/systemd/systemd-fsckd</filename></para>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para><filename>systemd-fsckd.service</filename> 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.</para>
Reflow?
+
+ <para><filename>systemd-fsckd</filename> accepts
+ <filename>systemd-fsck</filename> 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
+ <filename>/dev/console</filename> if show status is enabled,
+ and communicates to the user translated strings to plymouth
+ ready to be used by scripted themes, not supporting i18n.
+ The compilted plymouth themes can use the raw data (see below)
+ to display their own custom messages.</para>
<para><command>systemd-fsckd</command> receives messages about file
system check progress from <command>systemd-fsck</command> through a
UNIX domain socket. It can display the progress of the least advanced
fsck as well as the total number of devices being checked in parallel
to the console. It will also send progress messages to plymouth.
Both the raw data and translated messages are sent, so compiled
plymouth themes can use the raw data to display custom messages, and
scripted themes, not supporting i18n, can display the translated
versions.</para>
+
+ <para>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
+ <filename>systemd-fsckd</filename>.</para>
<para><command>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 <filename>systemd-fsckd</filename>.</para>

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?
+ </refsect1>
+
+ <refsect1>
+ <title>Protocol with plymouth</title>
Protocol for communication with plymouth
+
+ <para><filename>systemd-fsckd</filename> passes the
+ following messages to the theme via libplymouth:</para>
Does it actually use libplymouth?
+ <variablelist>
+ <varlistentry>
+ <listitem><para>the current number of devices
+ being checked (int)</para></listitem>
+ </varlistentry>
+ <varlistentry>
+ <listitem><para>the current minimum percentage of
+ all devices being checking (float, from 0 to 100)</para></listitem>
+ </varlistentry>
+ <varlistentry>
+ <listitem><para>a translated message ready to be displayed
+ by the plymouth theme displaying the data above. It can be overriden
+ by themes supporting i18n.</para></listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+
+ <variablelist>
+ <varlistentry>
+ <listitem><para>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
+ <literal>fsckd-cancel-msg</literal> prefix)
+ by themes supporting i18n.</para></listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+ </refsect1>
+
+ <refsect1>
+ <title>Options</title>
+
+ <para>The following options are understood:</para>
+
+ <variablelist>
+ <xi:include href="standard-options.xml" xpointer="help" />
+ <xi:include href="standard-options.xml" xpointer="version" />
+ </variablelist>
+
+ </refsect1>
+
+ <refsect1>
+ <title>Exit status</title>
+
+ <para>On success, 0 is returned, a non-zero failure
+ code otherwise. Note that the daemon stays idle for
+ a while to accept new <filename>systemd-fsck</filename>
+ connections before exiting.</para>
+ </refsect1>
+
+ <refsect1>
+ <title>See Also</title>
+ <para>
+ <citerefentry><refentrytitle>systemd</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>systemd-fsck</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>systemd-quotacheck.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
Please add something like project='man-pages' (needs to be checked) to citerefentry,
so the links are correct in html version.
+ <citerefentry><refentrytitle>fsck.btrfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.cramfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.ext4</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.fat</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.hfsplus</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.minix</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.ntfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck.xfs</refentrytitle><manvolnum>8</manvolnum></citerefentry>
+ </para>
+ </refsect1>
+
+</refentry>
diff --git a/units/systemd-fsckd.service.in b/units/systemd-fsckd.service.in
index 27c325f..9c7ed51 100644
--- a/units/systemd-fsckd.service.in
+++ b/units/systemd-fsckd.service.in
@@ -7,6 +7,7 @@
[Unit]
Description=File System Check Daemon to report status
+Documentation=man:systemd-fsckd.service(8)
DefaultDependencies=no
Requires=systemd-fsckd.socket
Before=shutdown.target
diff --git a/units/systemd-fsckd.socket b/units/systemd-fsckd.socket
index a8994a1..93c4ea9 100644
--- a/units/systemd-fsckd.socket
+++ b/units/systemd-fsckd.socket
@@ -7,7 +7,7 @@
[Unit]
Description=fsck to fsckd communication Socket
DefaultDependencies=no
[Socket]
Zbyszek
Didier Roche
2015-02-17 16:27:44 UTC
Permalink
Thanks for the suggestions, all applied!
Post by Zbigniew Jędrzejewski-Szmek
+
+ <para>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
+ <filename>systemd-fsckd</filename>.</para>
<para><command>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 <filename>systemd-fsckd</filename>.</para>
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)?
Post by Zbigniew Jędrzejewski-Szmek
+
+ <para><filename>systemd-fsckd</filename> passes the
+ following messages to the theme via libplymouth:</para>
Does it actually use libplymouth?
argh, sorry, didn't change it. Now done.
Post by Zbigniew Jędrzejewski-Szmek
+
+ <refsect1>
+ <title>See Also</title>
+ <para>
+ <citerefentry><refentrytitle>systemd</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>systemd-fsck</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>fsck</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+ <citerefentry><refentrytitle>systemd-quotacheck.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
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-***@.service.xml

Cheers,
Didier
Didier Roche
2015-02-18 11:34:21 UTC
Permalink
Didier Roche
2015-02-05 17:11:46 UTC
Permalink
Didier Roche
2015-02-18 11:34:24 UTC
Permalink
Zbigniew Jędrzejewski-Szmek
2015-02-14 16:21:44 UTC
Permalink
Post by Didier Roche
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
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 120000 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 <unistd.h>
#include <fcntl.h>
#include <sys/file.h>
+#include <sys/stat.h>
#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;
Use 'return log_warning_errno(...)'.
Post by Didier Roche
+ }
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;
+ int pass;
+ size_t cur, max;
+ ssize_t n;
usec_t t;
+ _cleanup_free_ char *device = NULL;
+ FsckProgress progress;
if (fscanf(f, "%i %lu %lu %ms", &pass, &cur, &max, &device) != 4)
break;
- /* Only show one progress counter at max */
- if (!locked) {
- if (flock(fileno(console), LOCK_EX|LOCK_NB) < 0)
- continue;
-
- locked = true;
- }
-
/* Only update once every 50ms */
t = now(CLOCK_MONOTONIC);
if (last + 50 * USEC_PER_MSEC > t)
@@ -191,22 +178,15 @@ static int process_progress(int fd) {
last = t;
- p = percent(pass, cur, max);
- fprintf(console, "\r%s: fsck %3.1f%% complete...\r%n", device, p, &m);
- fflush(console);
-
- if (m > clear)
- clear = m;
- }
-
- if (clear > 0) {
- unsigned j;
+ /* send progress to fsckd */
+ progress.devnum = device_num;
+ progress.cur = cur;
+ progress.max = max;
+ progress.pass = pass;
- fputc('\r', console);
- for (j = 0; j < (unsigned) clear; j++)
- fputc(' ', console);
- fputc('\r', console);
- fflush(console);
+ n = send(fsckd_fd, &progress, sizeof(FsckProgress), 0);
+ if (n < 0 || (size_t) n < sizeof(FsckProgress))
+ log_warning_errno(errno, "Cannot communicate fsck progress to fsckd: %m");
}
return 0;
@@ -359,7 +339,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]);
+ process_progress(progress_pipe[0], st.st_rdev);
progress_pipe[0] = -1;
}
diff --git a/src/fsckd/Makefile b/src/fsckd/Makefile
new file mode 120000
index 0000000..d0b0e8e
--- /dev/null
+++ b/src/fsckd/Makefile
@@ -0,0 +1 @@
+../Makefile
\ No newline at end of file
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
new file mode 100644
index 0000000..4a16f3d
--- /dev/null
+++ b/src/fsckd/fsckd.c
@@ -0,0 +1,403 @@
+/*-*- 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 <getopt.h>
+#include <errno.h>
+#include <math.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include "build.h"
+#include "event-util.h"
+#include "fsckd.h"
+#include "log.h"
+#include "list.h"
+#include "macro.h"
+#include "sd-daemon.h"
+#include "socket-util.h"
+#include "util.h"
+
+#define IDLE_TIME_SECONDS 30
+
+struct Manager;
+
+typedef struct Client {
+ struct Manager *manager;
+ int fd;
+ dev_t devnum;
+ size_t cur;
+ size_t max;
+ int pass;
+ double percent;
+ size_t buflen;
+
+ LIST_FIELDS(struct Client, clients);
+} Client;
+
+typedef struct Manager {
+ sd_event *event;
+ Client *clients;
+ int clear;
+ int connection_fd;
+ FILE *console;
+ double percent;
+ int numdevices;
+} Manager;
+
+static void manager_free(Manager *m);
+DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+#define _cleanup_manager_free_ _cleanup_(manager_freep)
+
+static double compute_percent(int pass, size_t cur, size_t max) {
+ /* Values stolen from e2fsck */
+
+ static const double 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 pass_table[pass-1] +
+ (pass_table[pass] - pass_table[pass-1]) *
+ (double) cur / max;
+}
+
+
+static void remove_client(Client **first, Client *item) {
+ LIST_REMOVE(clients, *first, item);
+ safe_close(item->fd);
+ free(item);
+}
+
+static int update_global_progress(Manager *m) {
+ Client *current = NULL;
+ _cleanup_free_ char *console_message = NULL;
+ int current_numdevices = 0, l = 0;
+ double current_percent = 100;
+
+ /* get the overall percentage */
+ LIST_FOREACH(clients, current, m->clients) {
+ current_numdevices++;
+
+ /* right now, we only keep the minimum % of all fsckd processes. We could in the future trying to be
+ linear, but max changes and corresponds to the pass. We have all the informations into fsckd
+ already if we can treat that in a smarter way. */
+ current_percent = MIN(current_percent, current->percent);
+ }
+
+ /* update if there is anything user-visible to update */
+ if (fabs(current_percent - m->percent) > 0.001 || current_numdevices != m->numdevices) {
+ 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)
+ return -ENOMEM;
+
+ /* write to console */
+ if (m->console) {
+ fprintf(m->console, "\r%s\r%n", console_message, &l);
+ fflush(m->console);
+ }
+
+ if (l > m->clear)
+ m->clear = l;
+ }
+ return 0;
+}
+
+static int progress_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+ Client *client = userdata;
+ Manager *m = NULL;
+ FsckProgress fsck_data;
+ size_t buflen;
+ int r;
+
+ assert(client);
+ m = client->manager;
+
+ /* 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.
Post by Didier Roche
+ remove_client(&(m->clients), client);
+ r = update_global_progress(m);
+ if (r < 0)
+ log_warning_errno(r, "Couldn't update global progress: %m");
+ }
+ return 0;
+ }
+
+ /* read actual data */
+ r = recv(fd, &fsck_data, sizeof(FsckProgress), 0);
+ if (r == 0) {
+ log_debug("Fsck client connected to fd %d disconnected", client->fd);
+ remove_client(&(m->clients), client);
+ } else if (r > 0 && r != sizeof(FsckProgress))
+ log_warning("Unexpected data structure sent to fsckd socket from fd: %d. Ignoring", client->fd);
+ else if (r > 0 && r == sizeof(FsckProgress)) {
+ client->devnum = fsck_data.devnum;
+ client->cur = fsck_data.cur;
+ client->max = fsck_data.max;
+ client->pass = fsck_data.pass;
+ client->percent = compute_percent(client->pass, client->cur, client->max);
+ log_debug("Getting progress for %u:%u (%lu, %lu, %d) : %3.1f%%",
+ major(client->devnum), minor(client->devnum),
+ client->cur, client->max, client->pass, client->percent);
+ } else
+ log_error_errno(r, "Unknown error while trying to read fsck data: %m");
+
+ r = update_global_progress(m);
+ if (r < 0)
+ log_warning_errno(r, "Couldn't update global progress: %m");
+
+ return 0;
+}
+
+static int new_connection_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+ Manager *m = userdata;
+ Client *client = NULL;
+ int new_client_fd, r;
+
+ assert(m);
+
+ /* Initialize and list new clients */
+ new_client_fd = accept4(m->connection_fd, NULL, NULL, SOCK_CLOEXEC);
+ if (new_client_fd > 0) {
+ log_debug("New fsck client connected to fd: %d", new_client_fd);
+ client = new0(Client, 1);
+ 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;
+ }
+ } else
+ return log_error_errno(errno, "Couldn't accept a new connection: %m");
+
+ return 0;
+}
+
+static void manager_free(Manager *m) {
+ Client *current = NULL, *l = NULL;
+ if (!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);
+ }
+
+ safe_close(m->connection_fd);
+ if (m->console)
+ fclose(m->console);
+
+ LIST_FOREACH_SAFE(clients, current, l, m->clients)
+ remove_client(&(m->clients), current);
+
+ sd_event_unref(m->event);
+
+ free(m);
+}
+
+static int manager_new(Manager **ret, int fd) {
+ _cleanup_manager_free_ Manager *m = NULL;
+ int r;
+
+ assert(ret);
+
+ m = new0(Manager, 1);
+ if (!m)
+ return -ENOMEM;
+
+ r = sd_event_default(&m->event);
+ if (r < 0)
+ 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");
+ m->percent = 100;
+
+ *ret = m;
+ m = NULL;
+
+ return 0;
+}
+
+static int run_event_loop_with_timeout(sd_event *e, usec_t timeout) {
+ int r, code;
+
+ assert(e);
+
+ for (;;) {
+ r = sd_event_get_state(e);
+ if (r < 0)
+ return r;
+ if (r == SD_EVENT_FINISHED)
+ break;
+
+ r = sd_event_run(e, timeout);
+ if (r < 0)
+ return r;
+
+ /* timeout reached */
+ if (r == 0) {
+ sd_event_exit(e, 0);
+ break;
+ }
+ }
+
+ r = sd_event_get_exit_code(e, &code);
+ if (r < 0)
+ return r;
+
+ return code;
+}
+
+static void help(void) {
+ printf("%s [OPTIONS...]\n\n"
+ "Capture fsck progress and forward one stream to plymouth\n\n"
+ " -h --help Show this help\n"
+ " --version Show package version\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 },
+ {}
+ };
+
+ int c;
+
+ assert(argc >= 0);
+ assert(argv);
+
+ while ((c = getopt_long(argc, argv, "hv", options, NULL)) >= 0)
+ switch (c) {
+
+ help();
+ return 0;
+
+ puts(PACKAGE_STRING);
+ puts(SYSTEMD_FEATURES);
+ return 0;
+
+ return -EINVAL;
+
+ assert_not_reached("Unhandled option");
+ }
+
+ if (optind < argc) {
+ log_error("Extraneous arguments");
+ return -EINVAL;
+ }
+
+ return 1;
+}
+
+int main(int argc, char *argv[]) {
+ _cleanup_manager_free_ Manager *m = NULL;
+ int fd = -1;
+ int r, n;
+
+ log_set_target(LOG_TARGET_AUTO);
+ log_parse_environment();
+ log_open();
+
+ r = parse_argv(argc, argv);
+ if (r <= 0)
+ return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+
+ n = sd_listen_fds(0);
+ if (n > 1) {
+ log_error("Too many file descriptors received.");
+ return EXIT_FAILURE;
+ } else if (n == 1) {
+ fd = SD_LISTEN_FDS_START + 0;
+ } else {
+ fd = make_socket_fd(LOG_DEBUG, FSCKD_SOCKET_PATH, SOCK_STREAM | SOCK_CLOEXEC);
+ if (fd < 0) {
+ log_error_errno(r, "Couldn't create listening socket fd on %s: %m", FSCKD_SOCKET_PATH);
+ return EXIT_FAILURE;
+ }
+ }
+
+ r = manager_new(&m, fd);
+ if (r < 0) {
+ log_error_errno(r, "Failed to allocate manager: %m");
+ return EXIT_FAILURE;
+ }
+
+ r = sd_event_add_io(m->event, NULL, fd, EPOLLIN, new_connection_handler, m);
+ if (r < 0) {
+ log_error_errno(r, "Can't listen to connection socket: %m");
+ return EXIT_FAILURE;
+ }
+
+ r = run_event_loop_with_timeout(m->event, IDLE_TIME_SECONDS * USEC_PER_SEC);
+ if (r < 0) {
+ log_error_errno(r, "Failed to run event loop: %m");
+ return EXIT_FAILURE;
+ }
+
+ sd_event_get_exit_code(m->event, &r);
+
+ return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/src/fsckd/fsckd.h b/src/fsckd/fsckd.h
new file mode 100644
index 0000000..6fe37a7
--- /dev/null
+++ b/src/fsckd/fsckd.h
@@ -0,0 +1,34 @@
+/*-*- 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/>.
+***/
+
+#define FSCKD_SOCKET_PATH "/run/systemd/fsckd"
+
+#include "libudev.h"
+
+typedef struct FsckProgress {
+ dev_t devnum;
+ size_t cur;
+ size_t max;
+ int pass;
+} FsckProgress;
--
2.1.4
Zbyszek
Didier Roche
2015-02-17 16:25:54 UTC
Permalink
Le 14/02/2015 17:21, Zbigniew Jędrzejewski-Szmek a écrit :

Thanks for the review, reattached the patch with those fixes.
Post by Didier Roche
+ 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(...)'.
Post by Didier Roche
+ }
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!
Post by Didier Roche
Post by Didier Roche
+
+ /* 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
Didier Roche
2015-02-18 11:32:37 UTC
Permalink
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
Martin Pitt
2015-02-18 16:00:56 UTC
Permalink
Ç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.

A big omission is of course the lack of German translations! :-)
(I'll commit them now)

Merci,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Didier Roche
2015-02-18 16:09:23 UTC
Permalink
Post by Martin Pitt
Ç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!
Post by Martin Pitt
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

Loading...