fsck: implement fsck -r {fd}
PATCH 1/1 fsck: implement fsck -r {fd}
Make possible sending of statistics to a dedicated file descriptor.
Rationale: When UI is calling fsck from a remote terminal, fsck progress needs to be sent to stdout. It is mixed there with output of statistics, and it is impossible to parse the output to get the statistics.
Now it will be possible e. g. with "fsck -C -r 3 /dev/sda1"
Note: Code in if and else is intentionally partially duplicated. Current human readable output of floats does not conform to locale conventions, and may be changed in future. But we want to keep machine readable output exactly same as it is now.
--- disk-utils/fsck.8 | 13 ++++++++++-- disk-utils/fsck.c | 63 ++++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 62 insertions(+), 14 deletions(-)
disk-utils/fsck.8 +11 -2
@@ -6,7 +6,9 @@ fsck \- check and repair a Linux filesystem .SH SYNOPSIS .B fsck -.RB [ \-lrsAVRTMNP ] +.RB [ \-lsAVRTMNP ] +.RB [ \-r +.RI [ fd ]] .RB [ \-C .RI [ fd ]] .RB [ \-t
@@ -100,13 +102,20 @@ multiple devices or for non-rotating disks. \fBfsck\fR does not lock underlying devices when executed to check stacked devices (e.g.\& MD or DM) \(en this feature is not implemented yet. .TP -.B \-r +.BR \-r \ [ \fIfd\fR ] Report certain statistics for each fsck when it completes. These statistics include the exit status, the maximum run set size (in kilobytes), the elapsed all-clock time and the user and system CPU time used by the fsck run. For example: /dev/sda1: status 0, rss 92828, real 4.002804, user 2.677592, sys 0.86186 + +GUI front-ends may specify a file descriptor +.IR fd , +in which case the progress bar information will be sent to that file descriptor +in a machine parseable format. For example: + +/dev/sda1 0 92828 4.002804 2.677592 0.86186 .TP .B \-s Serialize
disk-utils/fsck.c +51 -12
96 lines not on this page · raw message
Signature · 13 lines
2.3.5 -- Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76
Applied, thanks. See notes below.
Quoted 4 lines · Stanislav Brabec
Note: Code in if and else is intentionally partially duplicated. Current human readable output of floats does not conform to locale conventions, and may be changed in future. But we want to keep machine readable output exactly same as it is now.
The question is if we can change the human readable output. It was requested by Google and I guess they parse the output.
disk-utils/fsck.c static int progress; · line 145
Quoted 2 lines
disk-utils/fsck.c +2
-0+static int report_stats_fd; +static FILE *report_stats_file;
It seems that report_stats_fd does not have to be global variable, code depends on report_stats_file. Fixed.
Quoted 8 lines
disk-utils/fsck.c +6
-0case 'r' : report_stats =1 ; +if (arg[j+1 ]) {/* -r<fd> */ + report_stats_fd =string_to_int (arg+j+1 ); +if (report_stats_fd <0 ) + report_stats_fd =0 ; +else + goto next_arg;
disk-utils/fsck.c static void parse_argv(int argc, char *argv[]) · line 1537
Quoted 10 lines
disk-utils/fsck.c +9
-0+ }else if (i+1 < argc && *argv[i+1 ] !='-' ) {/* -r <fd> */ + report_stats_fd =string_to_int (argv[i+1 ]); +if (report_stats_fd <0 ) + report_stats_fd =0 ; +else { + ++i; + goto next_arg; + } + }break ;
...you have copied code from -C, right?
The -C code interprets errors or missing fd as 0, it's probably because fsck.extN requires file descriptor for -C, but fsck(8) has the fd as optional argument.
I don't think we need this behavior for -r too. It would be better to report all possible mistakes. Fixed.
disk-utils/fsck.c int main(int argc, char *argv[]) · line 1610
Quoted 9 lines
disk-utils/fsck.c +9
-0+/* Validate the report stats file descriptor to avoid disasters */ +if (report_stats_fd) { + report_stats_file =fdopen (report_stats_fd,"w" ); +if (!report_stats_file) +err (FSCK_EX_ERROR , +_ ("invalid argument -r %d" ), + report_stats_fd); + } +
It means that "-r 0" is unsupported, why? I have fixed this.
Karel
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Quoted 9 lines · Karel Zak
Quoted 4 lines · Stanislav Brabec
Note: Code in if and else is intentionally partially duplicated. Current human readable output of floats does not conform to locale conventions, and may be changed in future. But we want to keep machine readable output exactly same as it is now.
The question is if we can change the human readable output. It was requested by Google and I guess they parse the output.
We're not parsing the output; it's something that we look at in the log files manually (i.e., by humans when trying to debug a problem.)
The bigger deal for us not just mixing the stats and the rest of the fsck output, but if you have dozen(s) of disks after a power failure, and you're running without a journal, multiple drives will have inconsistencies that need fixing, and so we divert the output for each fsck process to a separate log file for each fsck process / file system.
The changes to separate out the fsck output into separate files is a change which I don't think ever made it upstream. IIRC, we submitted the patches upstream, there were some objections, and I never had time to get back to trying to make something that would work for us and acceptable upstream; my bad. If you want I can try to dig it up and try resubmitting it.
Alternatively, upstream e2fsck also has a way of having e2fsck direct its output to individual files, which is a bit more flexible and IMO a bit cleaner than the fsck changes. I suspect the right answer is to get something like this into fsck, but the problem is where to put the formatting information, since fsck doesn't a configuration file ala /etc/e2fsck.conf, where something like this can be dropped:
[options]
log_dir = /var/log/fsck
log_filename = e2fsck-%N.%h.%D-%T
log_dir_wait = true
report_time = true
report_verbose = true
report_features = true
See the man page for e2fsck.conf, looking at the LOGGING section and the definition of log_* in the options configuration stanza for more details.
If I recall correctly, it was the fact that fsck didn't have a configuration file which made the fsck patches a bit grotty, since there was no place to put the information to customize the logging output, and it's highly likely that what would work for Google might not be all that acceptable for other sites. That's why I ended up coding the e2fsck changes, but we never got around to switching over, since our out-of-tree fsck patches worked well enough, even if they could never go upstream. I can try resending the fsck patches, but I wouldn't blame you if you considered them too ugly to live.
Let me know if you have preferences of how you would like to proceed.
Cheers,
- Ted
Quoted 2 lines · Theodore Ts'o
We're not parsing the output; it's something that we look at in the log files manually (i.e., by humans when trying to debug a problem.)
SUSE boot scripts parse this output (well, it uses just a part of the information) to synchronize fsck and quota activation. Passing an opened file descriptor is a good solution for these purposes.
This needs just the one-line statistics generated by fsck, underlying FS specific statistics are not needed.
Quoted 6 lines
The bigger deal for us not just mixing the stats and the rest of the fsck output, but if you have dozen(s) of disks after a power failure, and you're running without a journal, multiple drives will have inconsistencies that need fixing, and so we divert the output for each fsck process to a separate log file for each fsck process / file system.
My patch just allows separating of the progress and statistics, but not splitting it per volume. (If you want human readable output, please see my reply to this note. (e. g. use -r -3).
Splitting per volumen is another issue, and use of -C {fd} is insufficient for it. But -r {fd} is probably sufficient for all use cases of the one-line-per-volume summary statistics.
fsck can now use only a subset of e2fsck features.
Quoted 6 lines
The changes to separate out the fsck output into separate files is a change which I don't think ever made it upstream. IIRC, we submitted the patches upstream, there were some objections, and I never had time to get back to trying to make something that would work for us and acceptable upstream; my bad. If you want I can try to dig it up and try resubmitting it.
The format of the machine parseable output of -C {fd} allows to separate progress for particular discs, but yes, if you want them separated and human readable, it is a bad approach to do it that way.
I can imagine better configurability of fsck, and maybe seamless integration with udev/systemd. But I have no idea what is the best way.
Keeping a possibility to send progress to an opened FD is required by GUI frontends, so it needs to be kept.
I suspect the right answer is to
Quoted 3 lines
get something like this into fsck, but the problem is where to put the formatting information, since fsck doesn't a configuration file ala
/etc/e2fsck.conf, where something like this can be dropped:
Configuration file could be a solution. SUSE could easily live even without it, so I cannot give an advice how to do it best.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76
Quoted 10 lines · Stanislav Brabec
Quoted 3 lines
We're not parsing the output; it's something that we look at in the log files manually (i.e., by humans when trying to debug a problem.)
SUSE boot scripts parse this output (well, it uses just a part of the information) to synchronize fsck and quota activation. Passing an opened file descriptor is a good solution for these purposes.
This needs just the one-line statistics generated by fsck, underlying FS specific statistics are not needed.
Actually, what of the one-line statistics do you need? Is it just "we're done checking /dev/sdXX", so you can activate quotas for /dev/sdXX? If so, one of the other patches which Frank Mayhar (one of my collegaues) implemented the ability for fsck to run a "completion handler" which was a program/shell script that would get executed for each file system the check was completed -- and which would pass the exit status for each specific file system along to the completion handler. That allowed us to take specific action on a per-file system basis if a file system couldn't be repaired, without having to parse any of the log outputs to determine which file system had uncorrectable file system problems.
I wonder if something like that would be useful for SuSE?
Regards,
- Ted
Quoted 3 lines · Theodore Ts'o
Actually, what of the one-line statistics do you need? Is it just "we're done checking
/dev/sdXX", so you can activate quotas for/dev/sdXX?
Yes, I think that it is only the device name.
Quoted 10 lines
If so, one of the other patches which Frank Mayhar (one of my collegaues) implemented the ability for fsck to run a "completion handler" which was a program/shell script that would get executed for each file system the check was completed -- and which would pass the exit status for each specific file system along to the completion handler. That allowed us to take specific action on a per-file system basis if a file system couldn't be repaired, without having to parse any of the log outputs to determine which file system had uncorrectable file system problems.
Quoted 2 lines
I wonder if something like that would be useful for SuSE?
This sounds interesting.
Quoted 1 line
From the systemd perspective, I can imagine a completely different
solution. systemd does not use a completion handlers, but it uses dependencies: When one task has fulfilled all its dependencies, then it could be run.
- fsck depends on presence of device
- mount depends on presence of the device, presence of mount point and fsck being complete.
There can be one fsck service per volume instead of one central fsck service.
But it is not a task for util-linux developers.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76
Quoted 12 lines · Karel Zak
Applied, thanks. See notes below.
Quoted 4 lines · Stanislav Brabec
Note: Code in if and else is intentionally partially duplicated. Current human readable output of floats does not conform to locale conventions, and may be changed in future. But we want to keep machine readable output exactly same as it is now.
The question is if we can change the human readable output. It was requested by Google and I guess they parse the output.
I created the alternative output format just for purpose of machine parsing: No need for scanf(), just space separated values.
I was also thinking about possibility to switch between human-readable and easily parseable output even to a specified FD, but I did not implement it.
I could be easy: Positive number would mean easily parseable output, negative number human readable. Just zero will be a problem (but we can still parse it on ASCII level and allow -r-0.
Signature · 10 lines
Best Regards / S pozdravem, Stanislav Brabec software developer --------------------------------------------------------------------- SUSE LINUX, s. r. o. e-mail: [email protected] Lihovarská 1060/12 tel: +49 911 7405384547 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.cz/ PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76