On Wed, Jan 28, 2015 at 02:20:40PM +0100, Didier Roche wrote: > > From d0f49b4e0994b9a1dd2738da18c5a2a6708b444f Mon Sep 17 00:00:00 2001 > From: Didier Roche <didro...@ubuntu.com> > 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 accepts
> instances to connect to it and send progress report. systemd-fsckd then sends > 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. idle > > Make the necessary changes to systemd-fsck to connect to systemd-fsckd socket. to connect to the > --- > .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 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..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 <errno.h> > #include <unistd.h> > #include <fcntl.h> > +#include <linux/limits.h> > #include <sys/file.h> > > #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; > } There's a double close now, since both f and fsckd_fd refer to the same fd. > > - 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 (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 */ > + strncpy(progress.device, device, PATH_MAX); > + 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(n, "Cannot communicate fsck > progress to fsckd: %m"); > } > > return 0; > 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..3059c68 > --- /dev/null > +++ b/src/fsckd/fsckd.c > @@ -0,0 +1,295 @@ > +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ > + > +/*** > + This file is part of systemd. > + > + Copyright 2015 Canonical > + > + Author: > + Didier Roche <didro...@ubuntu.com> > + > + 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 <linux/limits.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 "fsckd.h" > +#include "log.h" > +#include "list.h" > +#include "macro.h" > +#include "sd-daemon.h" > +#include "time-util.h" > +#include "util.h" > + > +#define IDLE_TIME_MINUTES 1 > + > +typedef struct Clients { > + int fd; > + char device[PATH_MAX]; Maybe nicer to make this char*? PATH_MAX is a full page, overkill in most circumstances. > + unsigned long cur; > + unsigned long max; size_t? > + int pass; > + double percent; > + > + LIST_FIELDS(struct Clients, clients); > +} Clients; > + > +static double compute_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; Just the (double) in front of cur should be enough. > +} > + > +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 ;)) > + > + console = fopen("/dev/console", "we"); > + if (!console) { > + log_error("Can't connect to /dev/console"); > + return -ENOMEM; > + } Please don't make up the return value. > + > + 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); Capital "N". > + current = alloca0(sizeof(Clients)); > + current->fd = new_client_fd; > + if (!first) > + LIST_INIT(clients, current); > + else > + LIST_PREPEND(clients, first, current); > + first = current; > + current = NULL; > + } > + > + LIST_FOREACH(clients, current, first) { > + FsckProgress fsck_data; > + int rc = 0; Please use r for the return code. Indentation is wrong. > + > + 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); > + current->fd = 0; > + current->percent = 100; > + } else if ((rc < 0) && (errno != EWOULDBLOCK)) { Those parens are unnecessary. > + 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. > + current->percent = 100; > + } else if (rc > 0) { > + strncpy(current->device, fsck_data.device, > PATH_MAX); You should check that enough bytes have been read, and not read unitialized data. > + current->cur = fsck_data.cur; > + current->max = fsck_data.max; > + current->pass = fsck_data.pass; > + current->percent = > compute_percent(current->pass, current->cur, current->max); > + > + log_debug("Getting progress for %s: (%lu, > %lu, %d) : %3.1f%%", > + current->device, current->cur, > current->max, current->pass, current->percent); > + } > + > + /* update global (eventually cached) values. */ > + if (current->fd > 0) Wrong condition here too. > + 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); > + } > + > + if ((fabs(current_percent - percent) > 0.000001) || > (current_numdevices != numdevices)) { > + numdevices = current_numdevices; > + percent = current_percent; > + > + asprintf(&console_message, "Checking in progress on > %d disks (%3.1f%% complete)", > + numdevices, percent); oom check missing. > + > + /* write to console */ > + fprintf(console, "\r%s\r%n", console_message, &m); > + fflush(console); > + > + if (m > clear) > + clear = m; > + } > + > + /* idle out after IDLE_TIME_MINUTES minutes with no > connected device */ > + t = now(CLOCK_MONOTONIC); > + if (numdevices == 0) { > + if (t > last_activity + IDLE_TIME_MINUTES * > USEC_PER_MINUTE) { > + log_debug("No fsck in progress for the last %d > minutes, shutting down.", IDLE_TIME_MINUTES); > + break; > + } > + } else > + last_activity = t; > + } > + > + /* clear last line */ > + if (clear > 0) { > + unsigned j; > + > + fputc('\r', console); > + for (j = 0; j < (unsigned) clear; j++) > + fputc(' ', console); > + fputc('\r', console); > + fflush(console); > + } > + > + return 0; > +} > + > +static int create_socket(void) { Can you base this on make_socket_fd() instead? > + /* Create the socket manually for testing */ > + union { > + struct sockaddr sa; > + struct sockaddr_un un; > + } sa; > + int fd; > + > + fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); > + if (fd < 0) > + return log_error_errno(errno, "socket() failed: %m"); > + > + memset(&sa, 0, sizeof(sa)); > + sa.un.sun_family = AF_UNIX; > + strncpy(sa.un.sun_path, FSCKD_SOCKET_PATH, sizeof(sa.un.sun_path)); > + unlink(FSCKD_SOCKET_PATH); > + if (bind(fd, &sa.sa, sizeof(sa)) < 0) > + return log_error_errno(errno, "binding %s failed: %m", > FSCKD_SOCKET_PATH); > + > + if (listen(fd, 5) < 0) > + return log_error_errno(errno, "listening on %s failed: %m", > FSCKD_SOCKET_PATH); > + > + return fd; > +} > + > +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) { > + > + case 'h': > + help(); > + return 0; > + > + case ARG_VERSION: > + puts(PACKAGE_STRING); > + puts(SYSTEMD_FEATURES); > + return 0; > + > + case '?': > + return -EINVAL; > + > + default: > + assert_not_reached("Unhandled option"); > + } > + > + if (optind < argc) { > + log_error("Extraneous arguments"); > + return -EINVAL; > + } > + > + return 1; > +} > + > +int main(int argc, char *argv[]) { > + int r; > + int fd, n; > + int flags; > + > + 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; > + flags = fcntl(fd, F_GETFL, 0); > + fcntl(fd, F_SETFL, flags | O_NONBLOCK); > + } else { > + fd = create_socket(); > + if (fd <= 0) > + return EXIT_FAILURE; > + } > + return handle_requests(fd) < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > +} > diff --git a/src/fsckd/fsckd.h b/src/fsckd/fsckd.h > new file mode 100644 > index 0000000..e8cd014 > --- /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 > + > + Author: > + Didier Roche <didro...@ubuntu.com> > + > + 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 <linux/limits.h> > + > +#define FSCKD_SOCKET_PATH "/run/systemd/fsckd" > + > +typedef struct FsckProgress { > + unsigned long cur; > + unsigned long max; > + int pass; > + char device[PATH_MAX]; > +} FsckProgress; > -- > 2.1.4 > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel