Re: [systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck

2015-02-17 Thread Zbigniew Jędrzejewski-Szmek 
On Tue, Feb 17, 2015 at 05:26:05PM +0100, Didier Roche wrote:
 Le 14/02/2015 17:38, Zbigniew Jędrzejewski-Szmek a écrit :
 On Thu, Feb 05, 2015 at 06:09:24PM +0100, Didier Roche wrote:
  From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
 From: Didier Roche didro...@ubuntu.com
 Date: Thu, 5 Feb 2015 17:08:18 +0100
 Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
   progress fsck
 
 Try to connect and send to plymouth (if running) some checked report 
 progress,
 using direct plymouth protocole.
 
 Update message is the following:
 fsckd:num_devices: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.
 
 +r = recv(fsckd_fd, fsckd_message, 
 sizeof(FsckdMessage), 0);
 +if (r  0  fsckd_message.cancel) {
 +log_warning(Request to cancel fsck from 
 fsckd);
 log_notice or log_info. Actually log_info I think, since this is a
 legitimate user request.
 Done.
 
 +
 +n = send(current-fd, cancel_msg, sizeof(FsckdMessage), 0);
 +if (n  0 || (size_t) n  sizeof(FsckdMessage))
 +return log_warning_errno(n, Cannot send cancel to fsck on 
 (%u, %u): %m, major(current-devnum), minor(current-devnum));
 line wrap please.
 
 Done. Btw, I was wondering if there was any kind of contributor rule
 like a style guideline in systemd? I probably just missed it.
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 :)

 +
 +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

Re: [systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck

2015-02-17 Thread Didier Roche 

Le 14/02/2015 17:38, Zbigniew Jędrzejewski-Szmek a écrit :

On Thu, Feb 05, 2015 at 06:09:24PM +0100, Didier Roche wrote:

 From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Thu, 5 Feb 2015 17:08:18 +0100
Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
  progress fsck

Try to connect and send to plymouth (if running) some checked report progress,
using direct plymouth protocole.

Update message is the following:
fsckd:num_devices: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.



+r = recv(fsckd_fd, fsckd_message, 
sizeof(FsckdMessage), 0);
+if (r  0  fsckd_message.cancel) {
+log_warning(Request to cancel fsck from 
fsckd);

log_notice or log_info. Actually log_info I think, since this is a
legitimate user request.

Done.



+
+n = send(current-fd, cancel_msg, sizeof(FsckdMessage), 0);
+if (n  0 || (size_t) n  sizeof(FsckdMessage))
+return log_warning_errno(n, Cannot send cancel to fsck on (%u, %u): 
%m, major(current-devnum), minor(current-devnum));

line wrap please.


Done. Btw, I was wondering if there was any kind of contributor rule 
like a style guideline in systemd? I probably just missed it.



+
+if (!m-plymouth_cancel_sent) {
+/* indicate to plymouth that we listen to Ctrl+C */
+r = loop_write(m-plymouth_fd, PLYMOUTH_REQUEST_KEY, 
sizeof(PLYMOUTH_REQUEST_KEY), true);
+if (r  0)
+return log_warning_errno(errno, Can't send to plymouth 
cancel key: %m);
+m-plymouth_cancel_sent = true;
+plymouth_cancel_message = strappenda(fsckd-cancel-msg:, Press 
Ctrl+C to cancel all checks in progress);

...all filesystem checks...


done (will repost the i18n patch + po ones as they are impacted by that 
change)



+r = send_message_plymouth_socket(m-plymouth_fd, 
plymouth_cancel_message, false);
+if (r  0)
+log_warning_errno(r, Can't send

Re: [systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck

2015-02-14 Thread Zbigniew Jędrzejewski-Szmek 
On Thu, Feb 05, 2015 at 06:09:24PM +0100, Didier Roche wrote:
 

 From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
 From: Didier Roche didro...@ubuntu.com
 Date: Thu, 5 Feb 2015 17:08:18 +0100
 Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
  progress fsck
 
 Try to connect and send to plymouth (if running) some checked report progress,
 using direct plymouth protocole.
 
 Update message is the following:
 fsckd:num_devices: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

[systemd-devel] [PATCH 3/9] Connect to plymouth and support cancellation of in, progress fsck

2015-02-05 Thread Didier Roche 


From ec3b3d2cd4b0097f9fafa6c3f0f400e06292e21c Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Thu, 5 Feb 2015 17:08:18 +0100
Subject: [PATCH 3/9] Connect to plymouth and support cancellation of in
 progress fsck

Try to connect and send to plymouth (if running) some checked report progress,
using direct plymouth protocole.

Update message is the following:
fsckd:num_devices: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)) {
+r = recv(fsckd_fd, fsckd_message, sizeof(FsckdMessage), 0);
+if (r  0  fsckd_message.cancel) {
+log_warning(Request to cancel fsck from fsckd);
+kill(fsck_pid, SIGTERM);
+}
+}
 }
 
 return 0;
@@ -193,6 +205,7 @@ int main(int argc, char *argv[]) {
 const char *cmdline[9];
 int i = 0, r = EXIT_FAILURE, q;
 pid_t pid;
+int progress_rc;
 siginfo_t status;
 _cleanup_udev_unref_ struct udev *udev = NULL;
 _cleanup_udev_device_unref_ struct udev_device *udev_device = NULL;
@@ -335,7 +348,7 @@ int main(int argc, char *argv[]) {
 progress_pipe[1] = safe_close(progress_pipe[1]);
 
 if (progress_pipe[0] = 0) {
-process_progress(progress_pipe[0], st.st_rdev);
+progress_rc = process_progress(progress_pipe[0], pid, st.st_rdev);
 progress_pipe[0] = -1;
 }
 
@@ -345,13 +358,14 @@ int main(int argc, char *argv[]) {
 goto finish;
 }
 
-if (status.si_code != CLD_EXITED || (status.si_status  ~1)) {
+if (status.si_code != CLD_EXITED || (status.si_status  ~1) || progress_rc != 0) {
 
-if (status.si_code == CLD_KILLED || status.si_code == CLD_DUMPED)
+/* cancel will kill fsck (but process_progress returns 0) */
+if ((progress_rc != 0  status.si_code == CLD_KILLED) || status.si_code == CLD_DUMPED)
 log_error(fsck terminated by signal %s., signal_to_string(status.si_status));
 else if (status.si_code == CLD_EXITED)
 log_error(fsck failed with error code %i., status.si_status);
-else
+