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
