Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On Thu, 29.01.15 18:44, Didier Roche (didro...@ubuntu.com) wrote: Le 28/01/2015 15:53, Zbigniew Jędrzejewski-Szmek a écrit : On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote: From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001 From: Didier Roche didro...@ubuntu.com Date: Mon, 26 Jan 2015 16:40:52 +0100 Subject: [PATCH 06/12] Support cancellation of fsck in progress Grab in fsckd plymouth watch key for C or c, and propagate this cancel request to systemd-fsck which will terminate fsck. Could we bind to ^c or if this is not possible, three c's in three seconds instead? I'm worried that before you could press anything to little effect in plymouth, and now a single key will have significant consequences. I tried to have a look at libplymouth, and if I'm correct, it's not possible to listen and get events for compose keys, so no way to get something like Control+C. As Dimitri told, it's been quite some years we are doing that in ubuntu, and that's the reason why we show a message to ensure the user is aware about that key (and that's why this patch is doing). Is it good for you this way? Hmm, I thought plymouth was listening on the tty (and not the input subsystem), which means it should deliver ctrl-C as ascii character code 3, because UNIX... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On Thu, Jan 29, 2015 at 06:44:23PM +0100, Didier Roche wrote: Le 28/01/2015 15:53, Zbigniew Jędrzejewski-Szmek a écrit : On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote: From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001 From: Didier Roche didro...@ubuntu.com Date: Mon, 26 Jan 2015 16:40:52 +0100 Subject: [PATCH 06/12] Support cancellation of fsck in progress Grab in fsckd plymouth watch key for C or c, and propagate this cancel request to systemd-fsck which will terminate fsck. Could we bind to ^c or if this is not possible, three c's in three seconds instead? I'm worried that before you could press anything to little effect in plymouth, and now a single key will have significant consequences. I tried to have a look at libplymouth, and if I'm correct, it's not possible to listen and get events for compose keys, so no way to get something like Control+C. As Dimitri told, it's been quite some years we are doing that in ubuntu, and that's the reason why we show a message to ensure the user is aware about that key (and that's why this patch is doing). Is it good for you this way? I think so. We can always improve the interface later on if it's confusing for users. (If plymouth forwards the key to us, we could detect triple c within two seconds ourselves. But if you think that a single key is fine, than that's fine for me.) Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
Le 28/01/2015 15:53, Zbigniew Jędrzejewski-Szmek a écrit : On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote: From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001 From: Didier Roche didro...@ubuntu.com Date: Mon, 26 Jan 2015 16:40:52 +0100 Subject: [PATCH 06/12] Support cancellation of fsck in progress Grab in fsckd plymouth watch key for C or c, and propagate this cancel request to systemd-fsck which will terminate fsck. Could we bind to ^c or if this is not possible, three c's in three seconds instead? I'm worried that before you could press anything to little effect in plymouth, and now a single key will have significant consequences. I tried to have a look at libplymouth, and if I'm correct, it's not possible to listen and get events for compose keys, so no way to get something like Control+C. As Dimitri told, it's been quite some years we are doing that in ubuntu, and that's the reason why we show a message to ensure the user is aware about that key (and that's why this patch is doing). Is it good for you this way? Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On Wed, 28.01.15 14:22, Didier Roche (didro...@ubuntu.com) wrote: src/fsck/fsck.c | 29 + src/fsckd/fsckd.c | 43 +++ src/fsckd/fsckd.h | 5 + 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index f5dd546..0b42e3b 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -47,6 +47,8 @@ static bool arg_skip = false; static bool arg_force = false; static const char *arg_repair = -a; +static pid_t fsck_pid; +static bool cancel_requested = false; Please do not introduce new global variables unnecessarily. We try to keep variables locally, and keep the global state at a minimum. An exception is mostly the command line args, which by their nature are global anyway... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
Dimitri John Ledkov [2015-01-28 15:21 +]: Hm? an interactive message with key-binding is usually shown and then plymouth reacts to such a key prompt. This is how it has always worked on plymouth prompts since forever... thus this would not be a surprise to most plymouth users (~ 5+ years by now?!) Doing it otherwise, will, on the contrary, impede user experience. I don't see anything wrong with always making Control-C work for the Unix wizards, and then the more human friendly annouced key bindings which can be translated, and thus also work on keyboards where you don't have a c in the first place. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On 28 January 2015 at 15:31, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Wed, Jan 28, 2015 at 03:21:27PM +, Dimitri John Ledkov wrote: On 28 January 2015 at 14:53, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote: From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001 From: Didier Roche didro...@ubuntu.com Date: Mon, 26 Jan 2015 16:40:52 +0100 Subject: [PATCH 06/12] Support cancellation of fsck in progress Grab in fsckd plymouth watch key for C or c, and propagate this cancel request to systemd-fsck which will terminate fsck. Could we bind to ^c or if this is not possible, three c's in three seconds instead? I'm worried that before you could press anything to little effect in plymouth, and now a single key will have significant consequences. Hm? an interactive message with key-binding is usually shown and then plymouth reacts to such a key prompt. This is how it has always worked on plymouth prompts since forever... thus this would not be a surprise to most plymouth users (~ 5+ years by now?!) Doing it otherwise, will, on the contrary, impede user experience. If you say so. I have never interacted with plymouth except to press ESC. I'll have to give this a try. Actually it's not ESC button, but rather any escape sequence will drop into messages mode. That is shift, ctrl, alt, Fn, all of them. Thus I don't think ^c binding is actually available to plymouth clients. But my plymouth knowledge is rusty. -- Regards, Dimitri. Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On 28 January 2015 at 14:53, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote:
From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Mon, 26 Jan 2015 16:40:52 +0100
Subject: [PATCH 06/12] Support cancellation of fsck in progress
Grab in fsckd plymouth watch key for C or c, and propagate this cancel
request
to systemd-fsck which will terminate fsck.
Could we bind to ^c or if this is not possible, three c's in three
seconds instead? I'm worried that before you could press anything to little
effect in plymouth, and now a single key will have significant consequences.
Hm? an interactive message with key-binding is usually shown and then
plymouth reacts to such a key prompt.
This is how it has always worked on plymouth prompts since forever...
thus this would not be a surprise to most plymouth users (~ 5+ years
by now?!)
Doing it otherwise, will, on the contrary, impede user experience.
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 c or C can be used to cancel current checks. It can be
overriden (matching only fsckd-cancel-msg prefix) for themes supporting
i18n.
---
src/fsck/fsck.c | 29 +
src/fsckd/fsckd.c | 43 +++
src/fsckd/fsckd.h | 5 +
3 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index f5dd546..0b42e3b 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -47,6 +47,8 @@
static bool arg_skip = false;
static bool arg_force = false;
static const char *arg_repair = -a;
+static pid_t fsck_pid;
+static bool cancel_requested = false;
static void start_target(const char *target) {
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
@@ -165,6 +167,7 @@ static int process_progress(int fd) {
ssize_t n;
usec_t t;
FsckProgress progress;
+FsckdMessage fsckd_message;
if (fscanf(f, %i %lu %lu %ms, pass, cur, max,
device) != 4)
break;
@@ -185,6 +188,16 @@ static int process_progress(int fd) {
n = send(fsckd_fd, progress, sizeof(FsckProgress), 0);
if (n 0 || (size_t) n sizeof(FsckProgress))
log_warning_errno(n, Cannot communicate fsck
progress to fsckd: %m);
+
+/* get fsckd requests */
+n = recv(fsckd_fd, fsckd_message, sizeof(FsckdMessage),
0);
+if (n 0) {
+if (fsckd_message.cancel) {
+log_warning(Request to cancel fsck from
fsckd);
+cancel_requested = true;
+kill(fsck_pid, SIGTERM);
+}
+}
}
return 0;
@@ -193,7 +206,6 @@ static int process_progress(int fd) {
int main(int argc, char *argv[]) {
const char *cmdline[9];
int i = 0, r = EXIT_FAILURE, q;
-pid_t pid;
siginfo_t status;
_cleanup_udev_unref_ struct udev *udev = NULL;
_cleanup_udev_device_unref_ struct udev_device *udev_device = NULL;
@@ -321,11 +333,11 @@ int main(int argc, char *argv[]) {
cmdline[i++] = device;
cmdline[i++] = NULL;
-pid = fork();
-if (pid 0) {
+fsck_pid = fork();
+if (fsck_pid 0) {
log_error_errno(errno, fork(): %m);
goto finish;
-} else if (pid == 0) {
+} else if (fsck_pid == 0) {
/* Child */
if (progress_pipe[0] = 0)
safe_close(progress_pipe[0]);
@@ -340,7 +352,7 @@ int main(int argc, char *argv[]) {
progress_pipe[0] = -1;
}
-q = wait_for_terminate(pid, status);
+q = wait_for_terminate(fsck_pid, status);
if (q 0) {
log_error_errno(q, waitid(): %m);
goto finish;
@@ -348,11 +360,11 @@ int main(int argc, char *argv[]) {
if (status.si_code != CLD_EXITED || (status.si_status ~1)) {
-if (status.si_code == CLD_KILLED || status.si_code ==
CLD_DUMPED)
+if ((!cancel_requested status.si_code == CLD_KILLED) ||
status.si_code == CLD_DUMPED)
log_error(fsck terminated by signal %s.,
signal_to_string(status.si_status));
else if (status.si_code == CLD_EXITED)
log_error(fsck failed with error code %i.,
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On Wed, Jan 28, 2015 at 03:21:27PM +, Dimitri John Ledkov wrote: On 28 January 2015 at 14:53, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote: From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001 From: Didier Roche didro...@ubuntu.com Date: Mon, 26 Jan 2015 16:40:52 +0100 Subject: [PATCH 06/12] Support cancellation of fsck in progress Grab in fsckd plymouth watch key for C or c, and propagate this cancel request to systemd-fsck which will terminate fsck. Could we bind to ^c or if this is not possible, three c's in three seconds instead? I'm worried that before you could press anything to little effect in plymouth, and now a single key will have significant consequences. Hm? an interactive message with key-binding is usually shown and then plymouth reacts to such a key prompt. This is how it has always worked on plymouth prompts since forever... thus this would not be a surprise to most plymouth users (~ 5+ years by now?!) Doing it otherwise, will, on the contrary, impede user experience. If you say so. I have never interacted with plymouth except to press ESC. I'll have to give this a try. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 06/12] Support cancellation of fsck in progress
On Wed, Jan 28, 2015 at 02:22:54PM +0100, Didier Roche wrote:
From 104cf82ba28941e907f277a713f834ceb3d909f0 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Mon, 26 Jan 2015 16:40:52 +0100
Subject: [PATCH 06/12] Support cancellation of fsck in progress
Grab in fsckd plymouth watch key for C or c, and propagate this cancel request
to systemd-fsck which will terminate fsck.
Could we bind to ^c or if this is not possible, three c's in three
seconds instead? I'm worried that before you could press anything to little
effect in plymouth, and now a single key will have significant consequences.
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 c or C can be used to cancel current checks. It can be
overriden (matching only fsckd-cancel-msg prefix) for themes supporting i18n.
---
src/fsck/fsck.c | 29 +
src/fsckd/fsckd.c | 43 +++
src/fsckd/fsckd.h | 5 +
3 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index f5dd546..0b42e3b 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -47,6 +47,8 @@
static bool arg_skip = false;
static bool arg_force = false;
static const char *arg_repair = -a;
+static pid_t fsck_pid;
+static bool cancel_requested = false;
static void start_target(const char *target) {
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
@@ -165,6 +167,7 @@ static int process_progress(int fd) {
ssize_t n;
usec_t t;
FsckProgress progress;
+FsckdMessage fsckd_message;
if (fscanf(f, %i %lu %lu %ms, pass, cur, max, device)
!= 4)
break;
@@ -185,6 +188,16 @@ static int process_progress(int fd) {
n = send(fsckd_fd, progress, sizeof(FsckProgress), 0);
if (n 0 || (size_t) n sizeof(FsckProgress))
log_warning_errno(n, Cannot communicate fsck
progress to fsckd: %m);
+
+/* get fsckd requests */
+n = recv(fsckd_fd, fsckd_message, sizeof(FsckdMessage), 0);
+if (n 0) {
+if (fsckd_message.cancel) {
+log_warning(Request to cancel fsck from
fsckd);
+cancel_requested = true;
+kill(fsck_pid, SIGTERM);
+}
+}
}
return 0;
@@ -193,7 +206,6 @@ static int process_progress(int fd) {
int main(int argc, char *argv[]) {
const char *cmdline[9];
int i = 0, r = EXIT_FAILURE, q;
-pid_t pid;
siginfo_t status;
_cleanup_udev_unref_ struct udev *udev = NULL;
_cleanup_udev_device_unref_ struct udev_device *udev_device = NULL;
@@ -321,11 +333,11 @@ int main(int argc, char *argv[]) {
cmdline[i++] = device;
cmdline[i++] = NULL;
-pid = fork();
-if (pid 0) {
+fsck_pid = fork();
+if (fsck_pid 0) {
log_error_errno(errno, fork(): %m);
goto finish;
-} else if (pid == 0) {
+} else if (fsck_pid == 0) {
/* Child */
if (progress_pipe[0] = 0)
safe_close(progress_pipe[0]);
@@ -340,7 +352,7 @@ int main(int argc, char *argv[]) {
progress_pipe[0] = -1;
}
-q = wait_for_terminate(pid, status);
+q = wait_for_terminate(fsck_pid, status);
if (q 0) {
log_error_errno(q, waitid(): %m);
goto finish;
@@ -348,11 +360,11 @@ int main(int argc, char *argv[]) {
if (status.si_code != CLD_EXITED || (status.si_status ~1)) {
-if (status.si_code == CLD_KILLED || status.si_code ==
CLD_DUMPED)
+if ((!cancel_requested status.si_code == CLD_KILLED) ||
status.si_code == CLD_DUMPED)
log_error(fsck terminated by signal %s.,
signal_to_string(status.si_status));
else if (status.si_code == CLD_EXITED)
log_error(fsck failed with error code %i.,
status.si_status);
-else
+else if (!cancel_requested)
log_error(fsck failed due to unknown reason.);
if (status.si_code == CLD_EXITED (status.si_status 2)
root_directory)
@@ -363,7 +375,8 @@ int main(int argc, char *argv[]) {
start_target(SPECIAL_EMERGENCY_TARGET);
else {
r = EXIT_SUCCESS;
-log_warning(Ignoring error.);
+
