[PATCH V2] xfs_io: Document shutdown -f

[PATCH V2] xfs_io: Document shutdown -f

[Carlos Maiolino]

Add help and manpage information for xfs_io shutdown -f

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

V2:
	Add more detailed information about what shutdown command does

 io/shutdown.c     | 16 ++++++++++++++++
 man/man8/xfs_io.8 | 16 +++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/io/shutdown.c b/io/shutdown.c
index 022a0e9a..8ac64d63 100644
--- a/io/shutdown.c
+++ b/io/shutdown.c
@@ -47,6 +47,21 @@ shutdown_f(
 	return 0;
 }
 
+static void
+shutdown_help(void)
+{
+	printf(_(
+"\n"
+" Shuts down the filesystem and prevents any further IO from occurring.\n"
+"\n"
+" With no arguments, shutdown will not attempt to flush completed transactions\n"
+" to disk before marking the log as being shut down. Simulating an IO error\n"
+" behavior.\n"
+"\n"
+" -f -- Flush completed transactions to disk before shut down.\n"
+"\n"));
+}
+
 void
 shutdown_init(void)
 {
@@ -56,6 +71,7 @@ shutdown_init(void)
 	shutdown_cmd.argmax = 1;
 	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
 	shutdown_cmd.args = _("[-f]");
+	shutdown_cmd.help = shutdown_help;
 	shutdown_cmd.oneline =
 		_("shuts down the filesystem where the current file resides");
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index c3ab532d..d4c28a0a 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1007,8 +1007,22 @@ Note \-\- this can be useful for exercising out of space behavior.
 Only available in expert mode and requires privileges.
 .TP
 .BR shutdown " [ " \-f " ]"
-Force the filesystem to shutdown (with or without flushing the log).
+Force a shutdown of the filesystem, but keep consistency. It does not unmount
+the filesystem, but prevents any other IO to happen on it.
 Only available in expert mode and requires privileges.
+.PP
+.RS
+By default, XFS will not attempt to flush completed transactions to disk before
+marking the log as being shut down.
+.RE
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.B \-f
+Force XFS to flush all completed transactions to disk before marking the log as
+shut down.
+.PD
+.RE
 .TP
 .BR stat " [ " \-v "|" \-r " ]"
 Selected statistics from
-- 
2.14.3

Re: [PATCH V2] xfs_io: Document shutdown -f

[Darrick J. Wong]

On Thu, May 10, 2018 at 11:10:30AM +0200, Carlos Maiolino wrote:
> Add help and manpage information for xfs_io shutdown -f
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> V2:
> 	Add more detailed information about what shutdown command does
> 
>  io/shutdown.c     | 16 ++++++++++++++++
>  man/man8/xfs_io.8 | 16 +++++++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/io/shutdown.c b/io/shutdown.c
> index 022a0e9a..8ac64d63 100644
> --- a/io/shutdown.c
> +++ b/io/shutdown.c
> @@ -47,6 +47,21 @@ shutdown_f(
>  	return 0;
>  }
>  
> +static void
> +shutdown_help(void)
> +{
> +	printf(_(
> +"\n"
> +" Shuts down the filesystem and prevents any further IO from occurring.\n"
> +"\n"
> +" With no arguments, shutdown will not attempt to flush completed transactions\n"
> +" to disk before marking the log as being shut down. Simulating an IO error\n"

"This simulates an IO error behavior." ?

Might also be worth mentioning that this simulates a system crash too?

> +" behavior.\n"
> +"\n"
> +" -f -- Flush completed transactions to disk before shut down.\n"
> +"\n"));
> +}
> +
>  void
>  shutdown_init(void)
>  {
> @@ -56,6 +71,7 @@ shutdown_init(void)
>  	shutdown_cmd.argmax = 1;
>  	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
>  	shutdown_cmd.args = _("[-f]");
> +	shutdown_cmd.help = shutdown_help;
>  	shutdown_cmd.oneline =
>  		_("shuts down the filesystem where the current file resides");
>  
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index c3ab532d..d4c28a0a 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1007,8 +1007,22 @@ Note \-\- this can be useful for exercising out of space behavior.
>  Only available in expert mode and requires privileges.
>  .TP
>  .BR shutdown " [ " \-f " ]"
> -Force the filesystem to shutdown (with or without flushing the log).
> +Force a shutdown of the filesystem, but keep consistency. It does not unmount

I would've thought we maintain metadata consistency no matter if we
flush the log or not, at least if the log is working correctly.

Perhaps shorten this to 'Force the filesystem to shut down.'?

--D

> +the filesystem, but prevents any other IO to happen on it.
>  Only available in expert mode and requires privileges.
> +.PP
> +.RS
> +By default, XFS will not attempt to flush completed transactions to disk before
> +marking the log as being shut down.
> +.RE
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.B \-f
> +Force XFS to flush all completed transactions to disk before marking the log as
> +shut down.
> +.PD
> +.RE
>  .TP
>  .BR stat " [ " \-v "|" \-r " ]"
>  Selected statistics from
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] xfs_io: Document shutdown -f

[Carlos Maiolino]

On Thu, May 10, 2018 at 09:33:17AM -0700, Darrick J. Wong wrote:
> On Thu, May 10, 2018 at 11:10:30AM +0200, Carlos Maiolino wrote:
> > Add help and manpage information for xfs_io shutdown -f
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > 
> > V2:
> > 	Add more detailed information about what shutdown command does
> > 
> >  io/shutdown.c     | 16 ++++++++++++++++
> >  man/man8/xfs_io.8 | 16 +++++++++++++++-
> >  2 files changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/io/shutdown.c b/io/shutdown.c
> > index 022a0e9a..8ac64d63 100644
> > --- a/io/shutdown.c
> > +++ b/io/shutdown.c
> > @@ -47,6 +47,21 @@ shutdown_f(
> >  	return 0;
> >  }
> >  
> > +static void
> > +shutdown_help(void)
> > +{
> > +	printf(_(
> > +"\n"
> > +" Shuts down the filesystem and prevents any further IO from occurring.\n"
> > +"\n"
> > +" With no arguments, shutdown will not attempt to flush completed transactions\n"
> > +" to disk before marking the log as being shut down. Simulating an IO error\n"
> 
> "This simulates an IO error behavior." ?
> 
> Might also be worth mentioning that this simulates a system crash too?
> 

Not sure, I don't see it as a system crash, after all, the system doesn't crash
at all?

I may be seeing it in different way though.

> > +" behavior.\n"
> > +"\n"
> > +" -f -- Flush completed transactions to disk before shut down.\n"
> > +"\n"));
> > +}
> > +
> >  void
> >  shutdown_init(void)
> >  {
> > @@ -56,6 +71,7 @@ shutdown_init(void)
> >  	shutdown_cmd.argmax = 1;
> >  	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
> >  	shutdown_cmd.args = _("[-f]");
> > +	shutdown_cmd.help = shutdown_help;
> >  	shutdown_cmd.oneline =
> >  		_("shuts down the filesystem where the current file resides");
> >  
> > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> > index c3ab532d..d4c28a0a 100644
> > --- a/man/man8/xfs_io.8
> > +++ b/man/man8/xfs_io.8
> > @@ -1007,8 +1007,22 @@ Note \-\- this can be useful for exercising out of space behavior.
> >  Only available in expert mode and requires privileges.
> >  .TP
> >  .BR shutdown " [ " \-f " ]"
> > -Force the filesystem to shutdown (with or without flushing the log).
> > +Force a shutdown of the filesystem, but keep consistency. It does not unmount
> 
> I would've thought we maintain metadata consistency no matter if we
> flush the log or not, at least if the log is working correctly.
> 

Sorry, I thought that's exactly what I meant there.

"Force a shutdown of the filesystem, but keep consistency (whether -f is used or
not).

I thought the parenthesis were implicit there.

> Perhaps shorten this to 'Force the filesystem to shut down.'?

Can be too, I think it could be important to say 'keep consistency', but well,
not sure how important it would be if somebody is actually using shutdown

> 
> --D
> 
> > +the filesystem, but prevents any other IO to happen on it.
> >  Only available in expert mode and requires privileges.
> > +.PP
> > +.RS
> > +By default, XFS will not attempt to flush completed transactions to disk before
> > +marking the log as being shut down.
> > +.RE
> > +.RS 1.0i
> > +.PD 0
> > +.TP 0.4i
> > +.B \-f
> > +Force XFS to flush all completed transactions to disk before marking the log as
> > +shut down.
> > +.PD
> > +.RE
> >  .TP
> >  .BR stat " [ " \-v "|" \-r " ]"
> >  Selected statistics from
> > -- 
> > 2.14.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

Re: [PATCH V2] xfs_io: Document shutdown -f

[Eric Sandeen]

On 5/10/18 2:10 AM, Carlos Maiolino wrote:
> Add help and manpage information for xfs_io shutdown -f
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> V2:
> 	Add more detailed information about what shutdown command does
> 
>  io/shutdown.c     | 16 ++++++++++++++++
>  man/man8/xfs_io.8 | 16 +++++++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/io/shutdown.c b/io/shutdown.c
> index 022a0e9a..8ac64d63 100644
> --- a/io/shutdown.c
> +++ b/io/shutdown.c
> @@ -47,6 +47,21 @@ shutdown_f(
>  	return 0;
>  }
>  
> +static void
> +shutdown_help(void)
> +{
> +	printf(_(
> +"\n"
> +" Shuts down the filesystem and prevents any further IO from occurring.\n"
> +"\n"
> +" With no arguments, shutdown will not attempt to flush completed transactions\n"
> +" to disk before marking the log as being shut down. Simulating an IO error\n"
> +" behavior.\n"

I'd like to get this merged, but I think something like:

" Shuts down the filesystem and prevents any further IO from occurring.\n"
"\n"
" By default, shutdown will not flush completed transactions to disk\n"
" before marking the log as being shut down, simulating a disk failure\n"
" or system crash.\n"
" With -f, the log will be flushed to disk, matching XFS behavior when\n"
" metadata corruption is encountered.\n"
"\n"
" -f -- Flush completed transactions to disk before shut down.\n"

is more accurate.  I can edit the manpage to match, probably mentioning that
other filesystem implementations may vary.

(by default we're NOLOGFLUSH, meaning /nothing/ goes to disk.  no user
data, no log.  It's the same as unplugging a disk (or crashing the system).

With -f we do flush the log, which is what a metadata IO error would do
when it causes a shutdown.  Right?)

Sound OK?

-Eric

> +"\n"
> +" -f -- Flush completed transactions to disk before shut down.\n"
> +"\n"));
> +}
> +
>  void
>  shutdown_init(void)
>  {
> @@ -56,6 +71,7 @@ shutdown_init(void)
>  	shutdown_cmd.argmax = 1;
>  	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
>  	shutdown_cmd.args = _("[-f]");
> +	shutdown_cmd.help = shutdown_help;
>  	shutdown_cmd.oneline =
>  		_("shuts down the filesystem where the current file resides");
>  
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index c3ab532d..d4c28a0a 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1007,8 +1007,22 @@ Note \-\- this can be useful for exercising out of space behavior.
>  Only available in expert mode and requires privileges.
>  .TP
>  .BR shutdown " [ " \-f " ]"
> -Force the filesystem to shutdown (with or without flushing the log).
> +Force a shutdown of the filesystem, but keep consistency. It does not unmount
> +the filesystem, but prevents any other IO to happen on it.
>  Only available in expert mode and requires privileges.
> +.PP
> +.RS
> +By default, XFS will not attempt to flush completed transactions to disk before
> +marking the log as being shut down.
> +.RE
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.B \-f
> +Force XFS to flush all completed transactions to disk before marking the log as
> +shut down.
> +.PD
> +.RE
>  .TP
>  .BR stat " [ " \-v "|" \-r " ]"
>  Selected statistics from
> 

[PATCH V3] xfs_io: Document shutdown -f

[Eric Sandeen]

From: Carlos Maiolino <cmaiolino@redhat.com>

Add help and manpage information for xfs_io shutdown -f
[sandeen: more editing for clarity]

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Add more detailed information about what shutdown command does
V3: more editing

diff --git a/io/shutdown.c b/io/shutdown.c
index 7ceed7a..052d3de 100644
--- a/io/shutdown.c
+++ b/io/shutdown.c
@@ -35,6 +35,22 @@ shutdown_f(
 	return 0;
 }
 
+static void
+shutdown_help(void)
+{
+	printf(_(
+"\n"
+" Shuts down the filesystem and prevents any further IO from occurring.\n"
+"\n"
+" By default, shutdown will not flush completed transactions to disk\n"
+" before shutting the filesystem down, simulating a disk failure or crash.\n"
+" With -f, the log will be flushed to disk, matching XFS behavior when\n"
+" metadata corruption is encountered.\n"
+"\n"
+" -f -- Flush completed transactions to disk before shut down.\n"
+"\n"));
+}
+
 void
 shutdown_init(void)
 {
@@ -44,6 +60,7 @@ shutdown_init(void)
 	shutdown_cmd.argmax = 1;
 	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
 	shutdown_cmd.args = _("[-f]");
+	shutdown_cmd.help = shutdown_help;
 	shutdown_cmd.oneline =
 		_("shuts down the filesystem where the current file resides");
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index dd91589..54a077c 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1017,8 +1017,23 @@ Note \-\- this can be useful for exercising out of space behavior.
 Only available in expert mode and requires privileges.
 .TP
 .BR shutdown " [ " \-f " ]"
-Force the filesystem to shutdown (with or without flushing the log).
+Force the filesystem to shut down, preventing any further IO.
+XFS and other filesystems implement this functionality, although implementation
+details may differ slightly.
 Only available in expert mode and requires privileges.
+.PP
+.RS
+By default, XFS will not attempt to flush completed transactions to disk before
+shutting down the filesystem.  This simulates a disk failure or crash.
+.RE
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.B \-f
+Force XFS to flush all completed transactions to disk before shutting down,
+matching XFS behavior when critical corruption is encountered.
+.PD
+.RE
 .TP
 .BR stat " [ " \-v "|" \-r " ]"
 Selected statistics from

Re: [PATCH V3] xfs_io: Document shutdown -f

[Carlos Maiolino]

On Wed, Jul 25, 2018 at 05:07:13PM -0700, Eric Sandeen wrote:
> From: Carlos Maiolino <cmaiolino@redhat.com>
> 
> Add help and manpage information for xfs_io shutdown -f
> [sandeen: more editing for clarity]
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

I'm fine with the changes, thanks for fixing and sending the V3 Eric.

Since the patch has mine SoB too, I think I'm not supposed to add a reviewed by
just due your updates, but FWIW, I'm ok with them :P


> 
> V2: Add more detailed information about what shutdown command does
> V3: more editing
> 
> diff --git a/io/shutdown.c b/io/shutdown.c
> index 7ceed7a..052d3de 100644
> --- a/io/shutdown.c
> +++ b/io/shutdown.c
> @@ -35,6 +35,22 @@ shutdown_f(
>  	return 0;
>  }
>  
> +static void
> +shutdown_help(void)
> +{
> +	printf(_(
> +"\n"
> +" Shuts down the filesystem and prevents any further IO from occurring.\n"
> +"\n"
> +" By default, shutdown will not flush completed transactions to disk\n"
> +" before shutting the filesystem down, simulating a disk failure or crash.\n"
> +" With -f, the log will be flushed to disk, matching XFS behavior when\n"
> +" metadata corruption is encountered.\n"
> +"\n"
> +" -f -- Flush completed transactions to disk before shut down.\n"
> +"\n"));
> +}
> +
>  void
>  shutdown_init(void)
>  {
> @@ -44,6 +60,7 @@ shutdown_init(void)
>  	shutdown_cmd.argmax = 1;
>  	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
>  	shutdown_cmd.args = _("[-f]");
> +	shutdown_cmd.help = shutdown_help;
>  	shutdown_cmd.oneline =
>  		_("shuts down the filesystem where the current file resides");
>  
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index dd91589..54a077c 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1017,8 +1017,23 @@ Note \-\- this can be useful for exercising out of space behavior.
>  Only available in expert mode and requires privileges.
>  .TP
>  .BR shutdown " [ " \-f " ]"
> -Force the filesystem to shutdown (with or without flushing the log).
> +Force the filesystem to shut down, preventing any further IO.
> +XFS and other filesystems implement this functionality, although implementation
> +details may differ slightly.
>  Only available in expert mode and requires privileges.
> +.PP
> +.RS
> +By default, XFS will not attempt to flush completed transactions to disk before
> +shutting down the filesystem.  This simulates a disk failure or crash.
> +.RE
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.B \-f
> +Force XFS to flush all completed transactions to disk before shutting down,
> +matching XFS behavior when critical corruption is encountered.
> +.PD
> +.RE
>  .TP
>  .BR stat " [ " \-v "|" \-r " ]"
>  Selected statistics from
> 

-- 
Carlos