[PATCH 4/4] fsck: Add support for "completion" scripts.

[PATCH 4/4] fsck: Add support for "completion" scripts.

[Frank Mayhar]

Add support for scripts that are run when each individual fsck
completes.  This allows certain general or file- or device-specific
actions to be taken when each fsck finishes.

Signed-off-by: Frank Mayhar <fmayhar@google.com>

 fsck/fsck.8 |   60
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fsck/fsck.c |   62
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fsck/fsck.h |    4 ++-
 3 files changed, 125 insertions(+), 1 deletions(-)

diff --git a/fsck/fsck.8 b/fsck/fsck.8
index d56b0d7..7e5b55b 100644
--- a/fsck/fsck.8
+++ b/fsck/fsck.8
@@ -413,6 +413,60 @@ and
 do not support the
 .B -y
 option as of this writing.
+.SH COMPLETION SCRIPTS
+.B fsck
+has a mechanism available by which to run a script when each check
completes.
+If when
+.B fsck
+starts the directory
+.IR /etc/fsck.d
+(or see FSCK_COMPLETION_PATH below) exists and is readable,
+.B fsck
+will use it to find such scripts.  Scripts there must be named one of
+.IR /etc/fsck.d/device/name ", " /etc/fsck.d/fstype/name ,
+or
+.IR /etc/fsck.d/completion ,
+where
+.I name
+is either the last component of the device path (e.g.
+.IR /etc/fsck.d/device/hda1 )
+or a file system type (e.g.
+.IR /etc/fsck.d/fstype/ext4 ).
+.PP
+When a file system check completes,
+.B fsck
+looks for a script that corresponds to the device that was checked; if
such
+a script is not found it looks for a script that corresponds to the
+type of that file system.  If neither of those is found it looks for a
+script named
+.IR /etc/fsck.d/completion .
+When
+.B fsck
+finds such a script and if the script is executable (and the
+.B \-N
+option was not specified), it runs the script via the
+.I system(3)
+function, passing the following set of parameters on the command line:
+.br
+\	device\	\-\ Device path of the device checked
+.br
+\	type\	\	\-\ File system type
+.br
+\	status\	\-\ Exit status
+.br
+\	elapsed\	\-\ Elapsed wall-clock time
+.br
+\	utime\	\-\ User CPU time
+.br
+\	stime\	\-\ System CPU time
+.br
+\	maxrss\	\-\ Maximum run-set-size
+.br
+\	logpath\	\-\ Path to logfile if the \-L option was specified
+.PP
+For example:
+.br
+  /etc/fsck.d/device/hdc1 /dev/hdc1 ext4 0 1.60470 0.41193 0.76588 9828
 .SH AUTHOR
 .UR tytso\@mit.edu
 Theodore Ts'o
@@ -429,6 +483,12 @@ The
 .B fsck
 program's behavior is affected by the following environment variables:
 .TP
+.B FSCK_COMPLETION_PATH
+If this environment variable is set,
+.B fsck
+will look for completion scripts in the indicated directory rather than
in
+.IR /etc/fsck.d .
+.TP
 .B FSCK_FORCE_ALL_PARALLEL
 If this environment variable is set,
 .B fsck
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 28b7016..58c7183 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -135,6 +135,8 @@ struct fsck_instance *instance_list;
 const char fsck_prefix_path[] = FS_SEARCH_PATH;
 char *fsck_path = 0;
 
+char *completion_path = COMPLETION_PATH;
+
 int force_timeout = 0;
 int timeout_secs = 0;
 int timeout_active = 0;
@@ -567,6 +569,59 @@ static int timeval_diff(struct timeval *result,
 		(x->tv_sec == y->tv_sec && x->tv_usec < y->tv_usec);
 }
 
+/*
+ * Choose and run a completion script for the completed fsck instance
+ * Prefer device match, then file system type, then generic script.
+ */
+static void run_completion(struct fsck_instance *inst)
+{
+	char *cmd, *cp, *devc;
+	struct timeval time_diff;
+
+	/*
+	 * If we're not actually doing fsck or we don't have a completion
+	 * path, don't run completions.
+	 */
+	if (noexecute || !completion_path)
+		return;
+	if ((devc = strrchr(inst->fs->device, '/')) == NULL)
+		devc = inst->fs->device;
+	else
+		devc++;
+	if ((cmd = malloc(1024)) == NULL)
+		return;
+	cp = cmd;
+	cp += sprintf(cmd, "%s/device/%s", completion_path, devc);
+	if (access(cmd, R_OK|X_OK) < 0) {
+		cp = cmd;
+		cp += sprintf(cmd, "%s/fstype/%s", completion_path, inst->fs->type);
+		if (access(cmd, R_OK|X_OK) < 0) {
+			cp = cmd;
+			cp += sprintf(cmd, "%s/completion", completion_path);
+			if (access(cmd, R_OK|X_OK) < 0) {
+				free(cmd);
+				return;
+			}
+		}
+	}
+	cp += sprintf(cp, " %s", inst->fs->device);
+	cp += sprintf(cp, " %s", inst->fs->type);
+	cp += sprintf(cp, " %d", inst->exit_status);
+	timeval_diff(&time_diff, &inst->end_time, &inst->start_time);
+	cp += sprintf(cp, " %d.%06d", (int)time_diff.tv_sec,
+			(int)time_diff.tv_usec);
+	cp += sprintf(cp, " %d.%06d", (int)inst->rusage.ru_utime.tv_sec,
+			(int)inst->rusage.ru_utime.tv_usec);
+	cp += sprintf(cp, " %d.%06d", (int)inst->rusage.ru_stime.tv_sec,
+			(int)inst->rusage.ru_stime.tv_usec);
+	cp += sprintf(cp, " %ld", inst->rusage.ru_maxrss);
+	if (log_output)
+		cp += sprintf(cp, " %s", inst->log_file);
+	if (system(cmd) < 0)
+		perror(cmd);
+	free(cmd);
+}
+
 /* Forward reference. */
 static void restart_earliest_timeout(NOARGS);
 
@@ -962,6 +1017,7 @@ ret_inst:
 		prev->next = inst->next;
 	else
 		instance_list = inst->next;
+	run_completion(inst);
 	report_fsck_stats(inst);
 	if (inst->log_fd)
 		close(inst->log_fd);
@@ -1694,6 +1750,12 @@ int main(int argc, char *argv[])
 		fstab = _PATH_MNTTAB;
 	load_fs_info(fstab);
 
+	completion_path = getenv("FSCK_COMPLETION_PATH");
+	if (!completion_path)
+		completion_path = COMPLETION_PATH;
+	if (access(completion_path, R_OK|X_OK) < 0)
+		completion_path = NULL;
+
 	/* Update our search path to include uncommon directories. */
 	if (oldpath) {
 		fsck_path = xmalloc (strlen (fsck_prefix_path) + 1 +
diff --git a/fsck/fsck.h b/fsck/fsck.h
index 6e41f40..93356bc 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -33,8 +33,10 @@
 #define EXIT_TIMEOUT     64
 #define EXIT_LIBRARY     128
 
+#define COMPLETION_PATH	"/etc/fsck.d/"
+
 /*
- * Internal structure for mount tabel entries.
+ * Internal structure for mount table entries.
  */
 
 struct fs_info {

-- 
Frank Mayhar
fmayhar@google.com

Re: [PATCH 4/4] fsck: Add support for "completion" scripts.

[Karel Zak]

On Tue, Feb 07, 2012 at 01:10:55PM -0800, Frank Mayhar wrote:
> Add support for scripts that are run when each individual fsck
> completes.  This allows certain general or file- or device-specific
> actions to be taken when each fsck finishes.
> 
> Signed-off-by: Frank Mayhar <fmayhar@google.com>
> 
>  fsck/fsck.8 |   60
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fsck/fsck.c |   62
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fsck/fsck.h |    4 ++-
>  3 files changed, 125 insertions(+), 1 deletions(-)
> 
> diff --git a/fsck/fsck.8 b/fsck/fsck.8
> index d56b0d7..7e5b55b 100644
> --- a/fsck/fsck.8
> +++ b/fsck/fsck.8
> @@ -413,6 +413,60 @@ and
>  do not support the
>  .B -y
>  option as of this writing.
> +.SH COMPLETION SCRIPTS
> +.B fsck
> +has a mechanism available by which to run a script when each check
> completes.
> +If when
> +.B fsck
> +starts the directory
> +.IR /etc/fsck.d
> +(or see FSCK_COMPLETION_PATH below) exists and is readable,
> +.B fsck
> +will use it to find such scripts.  Scripts there must be named one of
> +.IR /etc/fsck.d/device/name ", " /etc/fsck.d/fstype/name ,

 The problem is that "device name" is whatever from fsck command line
 or from first fstab column.

 Unfortunately the current fsck code does not canonicalize the device
 name -- this is bug because in the function ignore() we stat() the
 device... I'll fix this problem ASAP.

 Maybe it would be better to somehow standardize the names used for
 fsck-log and the completion scripts.

 What about to use

    /etc/fsck.d/uuid/<uuid>
    /var/log/fsck/fsck-<uuid>

 rather than devname? I guess that all usable filesystems provides
 UUIDs now and fsck is already linked with libblkid.

 If you agree than ignore this problem for now, I'll implement it.

> +.I system(3)

 Why we need to execute the scripts by system(3)? Why not classic execv?

> +function, passing the following set of parameters on the command line:
> +.br
> +\	device\	\-\ Device path of the device checked
> +.br
> +\	type\	\	\-\ File system type
> +.br
> +\	status\	\-\ Exit status
> +.br
> +\	elapsed\	\-\ Elapsed wall-clock time
> +.br
> +\	utime\	\-\ User CPU time
> +.br
> +\	stime\	\-\ System CPU time
> +.br
> +\	maxrss\	\-\ Maximum run-set-size
> +.br
> +\	logpath\	\-\ Path to logfile if the \-L option was specified
> +.PP
> +For example:
> +.br
> +  /etc/fsck.d/device/hdc1 /dev/hdc1 ext4 0 1.60470 0.41193 0.76588 9828

 I don't like the fixed set of parameters, it will be difficult to extend
 such solution in the future.

 What about to set environment variables FSCK_{DEVICE,FSTYPE,UTIME,...}
 for the script or use regular command line options, so:

    /etc/fsck.d/completion --device=/dev/hda1 --fstype=ext4 ...


    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 4/4] fsck: Add support for "completion" scripts.

[Frank Mayhar]

On Wed, 2012-02-08 at 17:22 +0100, Karel Zak wrote:
> On Tue, Feb 07, 2012 at 01:10:55PM -0800, Frank Mayhar wrote:
> > Add support for scripts that are run when each individual fsck
> > completes.  This allows certain general or file- or device-specific
> > actions to be taken when each fsck finishes.
> > 
> > Signed-off-by: Frank Mayhar <fmayhar@google.com>
> > 
> >  fsck/fsck.8 |   60
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fsck/fsck.c |   62
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fsck/fsck.h |    4 ++-
> >  3 files changed, 125 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fsck/fsck.8 b/fsck/fsck.8
> > index d56b0d7..7e5b55b 100644
> > --- a/fsck/fsck.8
> > +++ b/fsck/fsck.8
> > @@ -413,6 +413,60 @@ and
> >  do not support the
> >  .B -y
> >  option as of this writing.
> > +.SH COMPLETION SCRIPTS
> > +.B fsck
> > +has a mechanism available by which to run a script when each check
> > completes.
> > +If when
> > +.B fsck
> > +starts the directory
> > +.IR /etc/fsck.d
> > +(or see FSCK_COMPLETION_PATH below) exists and is readable,
> > +.B fsck
> > +will use it to find such scripts.  Scripts there must be named one of
> > +.IR /etc/fsck.d/device/name ", " /etc/fsck.d/fstype/name ,
> 
>  The problem is that "device name" is whatever from fsck command line
>  or from first fstab column.
> 
>  Unfortunately the current fsck code does not canonicalize the device
>  name -- this is bug because in the function ignore() we stat() the
>  device... I'll fix this problem ASAP.

If the device name is canonicalized, I think the problem generally goes
away.

>  Maybe it would be better to somehow standardize the names used for
>  fsck-log and the completion scripts.
> 
>  What about to use
> 
>     /etc/fsck.d/uuid/<uuid>
>     /var/log/fsck/fsck-<uuid>

Adding the uuid as a potential completion script makes a lot of sense;
I'm a little hesitant about using the uuid in the logfile name, since
it's typically pretty long and could easily make the filename unwieldy.
That's part of what the completion script is for, too:  To potentially
canonicalize the logfile name into whatever form makes sense locally.

I would want to keep the devname, fstype and generic completion scripts
the same, though.  Just add the uuid one as you indicate.

>  rather than devname? I guess that all usable filesystems provides
>  UUIDs now and fsck is already linked with libblkid.
> 
>  If you agree than ignore this problem for now, I'll implement it.
> 
> > +.I system(3)
> 
>  Why we need to execute the scripts by system(3)? Why not classic execv?

The completion script could as easily be an executable binary; I just
figured I would let system() take care of it for me.  If you have a
strong objection, I could change it but at the moment I see no strong
reason to worry about it.

> > +  /etc/fsck.d/device/hdc1 /dev/hdc1 ext4 0 1.60470 0.41193 0.76588 9828
> 
>  I don't like the fixed set of parameters, it will be difficult to extend
>  such solution in the future.

Mostly I wanted something that would be trivial to parse.  I thought
about regular command line options but that makes the completion script
a lot bigger (all the option-parsing code) and my goal was to keep
things as simple as possible.  Setting environment variables might be
sufficient, though.

One thing I remembered last night that I forgot to document is that the
last parameter is optional and is the logfile name (including path) if
the -L option was given.  I'll generate one more patch to fsck.8 (based
on the current set of patches) to fix that, although if we go with
environment variables then it's no longer a problem.

For my own purposes, I would really like to keep the invocation as-is
and fall back to environment variables if there are no arguments on the
command line.  This would make my life a _lot_ easier.
-- 
Frank Mayhar
fmayhar@google.com

Re: [PATCH 4/4] fsck: Add support for "completion" scripts.

[Karel Zak]

On Wed, Feb 08, 2012 at 08:59:34AM -0800, Frank Mayhar wrote:
> >  The problem is that "device name" is whatever from fsck command line
> >  or from first fstab column.
> > 
> >  Unfortunately the current fsck code does not canonicalize the device
> >  name -- this is bug because in the function ignore() we stat() the
> >  device... I'll fix this problem ASAP.
> 
> If the device name is canonicalized, I think the problem generally goes
> away.

 You miss the point. Yes, we can resolve the problem with UUID= or
 LABEL= in /etc/fstab, but the problem is that nothing guarantees that
 the canonical kernel device name (e.g. /dev/sda) will the same after
 reboot.  This is reason why we use things like UUID= or
 /dev/disk/by-* for block devices.

 Anyway, no problem to support /etc/fsck.d/<devname>/ but preferred and
 recommended will be definitely <uuid>.

> >  Maybe it would be better to somehow standardize the names used for
> >  fsck-log and the completion scripts.
> > 
> >  What about to use
> > 
> >     /etc/fsck.d/uuid/<uuid>
> >     /var/log/fsck/fsck-<uuid>
> 
> Adding the uuid as a potential completion script makes a lot of sense;
> I'm a little hesitant about using the uuid in the logfile name, since
> it's typically pretty long and could easily make the filename unwieldy.
> That's part of what the completion script is for, too:  To potentially
> canonicalize the logfile name into whatever form makes sense locally.
> 
> I would want to keep the devname, fstype and generic completion scripts
> the same, though.  Just add the uuid one as you indicate.
> 
> >  rather than devname? I guess that all usable filesystems provides
> >  UUIDs now and fsck is already linked with libblkid.
> > 
> >  If you agree than ignore this problem for now, I'll implement it.
> > 
> > > +.I system(3)
> > 
> >  Why we need to execute the scripts by system(3)? Why not classic execv?
> 
> The completion script could as easily be an executable binary; I just
> figured I would let system() take care of it for me.  If you have a
> strong objection, I could change it but at the moment I see no strong
> reason to worry about it.
> 
> > > +  /etc/fsck.d/device/hdc1 /dev/hdc1 ext4 0 1.60470 0.41193 0.76588 9828
> > 
> >  I don't like the fixed set of parameters, it will be difficult to extend
> >  such solution in the future.
> 
> Mostly I wanted something that would be trivial to parse.  I thought
> about regular command line options but that makes the completion script
> a lot bigger (all the option-parsing code)

 Good point.

> and my goal was to keep
> things as simple as possible.  Setting environment variables might be
> sufficient, though.
>
> One thing I remembered last night that I forgot to document is that the
> last parameter is optional and is the logfile name (including path) if
> the -L option was given.  I'll generate one more patch to fsck.8 (based
> on the current set of patches) to fix that, although if we go with
> environment variables then it's no longer a problem.
> 
> For my own purposes, I would really like to keep the invocation as-is
> and fall back to environment variables if there are no arguments on the
> command line.  This would make my life a _lot_ easier.

 I understand, but I'd like to have a solution which is usable for
 more people and easy to maintain for upstream :-)  What about

   /etc/fsck.d/completion <devname> <status>

 and the rest of information by FSCK_* env.variables ? The env.
 variables should be pretty simple to use in the script.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 4/4] fsck: Add support for "completion" scripts.

[Frank Mayhar]

On Wed, 2012-02-08 at 20:50 +0100, Karel Zak wrote:
> On Wed, Feb 08, 2012 at 08:59:34AM -0800, Frank Mayhar wrote:
> > >  The problem is that "device name" is whatever from fsck command line
> > >  or from first fstab column.
> > > 
> > >  Unfortunately the current fsck code does not canonicalize the device
> > >  name -- this is bug because in the function ignore() we stat() the
> > >  device... I'll fix this problem ASAP.
> > 
> > If the device name is canonicalized, I think the problem generally goes
> > away.
> 
>  You miss the point. Yes, we can resolve the problem with UUID= or
>  LABEL= in /etc/fstab, but the problem is that nothing guarantees that
>  the canonical kernel device name (e.g. /dev/sda) will the same after
>  reboot.  This is reason why we use things like UUID= or
>  /dev/disk/by-* for block devices.

This is true in the general case but is more of an administrative issue
(in my opinion) and depends heavily on the configuration (of both the
machine and the distribution).  (In my _particular_ case, it isn't true,
in fact.)  Regardless, however, the real reason I'm resistant to using
the uuid for the logfile name is that it's just not very human-friendly.
That may also be true for certain kinds of device names, but to me
"fsck-d0c0t0s0" or "fsck-sda1" is much easier to deal with than
"fsck-bfb8fb54-5b31-11df-98e8-00e081337e9a".

>  Anyway, no problem to support /etc/fsck.d/<devname>/ but preferred and
>  recommended will be definitely <uuid>.

My inclination is to leave it up to the individual admin, but I have no
strong feeling about it.

> > and my goal was to keep
> > things as simple as possible.  Setting environment variables might be
> > sufficient, though.
> >
> > One thing I remembered last night that I forgot to document is that the
> > last parameter is optional and is the logfile name (including path) if
> > the -L option was given.  I'll generate one more patch to fsck.8 (based
> > on the current set of patches) to fix that, although if we go with
> > environment variables then it's no longer a problem.
> > 
> > For my own purposes, I would really like to keep the invocation as-is
> > and fall back to environment variables if there are no arguments on the
> > command line.  This would make my life a _lot_ easier.
> 
>  I understand, but I'd like to have a solution which is usable for
>  more people and easy to maintain for upstream :-)  What about
> 
>    /etc/fsck.d/completion <devname> <status>
> 
>  and the rest of information by FSCK_* env.variables ? The env.
>  variables should be pretty simple to use in the script.

Yeah, I could go with this.  Do you want to code it up or shall I?
-- 
Frank Mayhar
fmayhar@google.com

Re: [PATCH 4/4] fsck: Add support for "completion" scripts.

[Karel Zak]

On Wed, Feb 08, 2012 at 01:25:36PM -0800, Frank Mayhar wrote:
> Yeah, I could go with this.  Do you want to code it up or shall I?

 No, I'll play with it later and it seems that we can cleanup more
 things in fsck (it's too late for v2.21 now).

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com