diff mbox

[v3] ndctl: add clear error support for ndctl

Message ID 20170501224158.GA21546@djiang5-desk3.ch.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Jiang May 1, 2017, 10:41 p.m. UTC
On Mon, May 01, 2017 at 03:28:18PM -0700, Kani, Toshimitsu wrote:
> On Mon, 2017-05-01 at 15:16 -0700, Dave Jiang wrote:
> >
> > On 05/01/2017 03:06 PM, Kani, Toshimitsu wrote:
> > > On Mon, 2017-05-01 at 14:23 -0700, Dave Jiang wrote:
> > >  :
> > > > +++ b/Documentation/ndctl-clear-error.txt
> > > > @@ -0,0 +1,37 @@
> > > > +ndctl-clear-error(1)
> > > > +====================
> > > > +
> > > > +NAME
> > > > +----
> > > > +ndctl-clear-error - clear badblocks for a device
> > > > +
> > > > +SYNOPSIS
> > > > +--------
> > > > +[verse]
> > > > +'ndctl clear-error' [<options>]
> > > > +
> > > > +EXAMPLES
> > > > +--------
> > > > +
> > > > +Clear poison (bad blocks) for the provided device
> > > > +[verse]
> > > > +ndctl clear-error -f /dev/dax0.0 -s 0 -l 8
> > > > +
> > > > +Clear poison (bad blocks) at block offset 0 for 8 blocks on
> > > > device
> > > > /dev/dax0.0
> > > > +
> > > > +OPTIONS
> > > > +-------
> > > > +-f::
> > > > +--file::
> > > > +The device/file to be cleared of poison (bad blocks).
> > > > +
> > > > +-s::
> > > > +--start::
> > > > +The offset where the poison (bad block) starts for this
> > > > device.
> > > > +Typically this is acquired from the sysfs badblocks file.
> > > > +
> > > > +-l::
> > > > +--len::
> > > > +The number of badblocks to clear in size of 512 bytes
> > > > increments.
> > > > +
> > >
> > > When a specified range is larger than a badblock range, the command
> > > completes with no-op without any message.  I think it should either
> > > fail with an error message or clear an inclusive badblock.
> >
> > It's suppose to just fail. Looks like I just forgot to insert an
> > error print out.
> 
> Yes, I am fine with the behavior with a proper error message. It'd be
> good to clarify it in the document as well.
> 
> Thanks,
> -Toshi

Comments

Kani, Toshi May 1, 2017, 11:06 p.m. UTC | #1
On Mon, 2017-05-01 at 15:41 -0700, Jiang, Dave wrote:
> On Mon, May 01, 2017 at 03:28:18PM -0700, Kani, Toshimitsu wrote:

> > On Mon, 2017-05-01 at 15:16 -0700, Dave Jiang wrote:

> > > 

> > > On 05/01/2017 03:06 PM, Kani, Toshimitsu wrote:

> > > > On Mon, 2017-05-01 at 14:23 -0700, Dave Jiang wrote:

> > > >  :

> > > > > +++ b/Documentation/ndctl-clear-error.txt

> > > > > @@ -0,0 +1,37 @@

> > > > > +ndctl-clear-error(1)

> > > > > +====================

> > > > > +

> > > > > +NAME

> > > > > +----

> > > > > +ndctl-clear-error - clear badblocks for a device

> > > > > +

> > > > > +SYNOPSIS

> > > > > +--------

> > > > > +[verse]

> > > > > +'ndctl clear-error' [<options>]

> > > > > +

> > > > > +EXAMPLES

> > > > > +--------

> > > > > +

> > > > > +Clear poison (bad blocks) for the provided device

> > > > > +[verse]

> > > > > +ndctl clear-error -f /dev/dax0.0 -s 0 -l 8

> > > > > +

> > > > > +Clear poison (bad blocks) at block offset 0 for 8 blocks on

> > > > > device

> > > > > /dev/dax0.0

> > > > > +

> > > > > +OPTIONS

> > > > > +-------

> > > > > +-f::

> > > > > +--file::

> > > > > +The device/file to be cleared of poison (bad blocks).

> > > > > +

> > > > > +-s::

> > > > > +--start::

> > > > > +The offset where the poison (bad block) starts for this

> > > > > device.

> > > > > +Typically this is acquired from the sysfs badblocks file.

> > > > > +

> > > > > +-l::

> > > > > +--len::

> > > > > +The number of badblocks to clear in size of 512 bytes

> > > > > increments.

> > > > > +

> > > > 

> > > > When a specified range is larger than a badblock range, the

> > > > command completes with no-op without any message.  I think it

> > > > should either fail with an error message or clear an inclusive

> > > > badblock.

> > > 

> > > It's suppose to just fail. Looks like I just forgot to insert an

> > > error print out.

> > 

> > Yes, I am fine with the behavior with a proper error message. It'd

> > be good to clarify it in the document as well.

> 

> -- 

> 

> Does something like this work for you?

> 

> diff --git a/ndctl/clear-error.c b/ndctl/clear-error.c

> index 33d930a..1a9b94a 100644

> --- a/ndctl/clear-error.c

> +++ b/ndctl/clear-error.c

> @@ -167,6 +167,7 @@ static int clear_error(struct clear_err *ce)

>         if (check_user_input_range(ce->region, clear_err.bb_start,

>                                 clear_err.bb_len) == 0) {

>                 rc = -EINVAL;

> +               error("badblocks region input out of range.\n");

>                 goto cleanup;

>         }

>  


Yes, it looks good to me as this error shows up in both zero and
inclusive badblock in the input range.  We can use the document to
describe the restriction that an input range must fit into badblocks
range.

Thanks,
-Toshi
diff mbox

Patch

diff --git a/ndctl/clear-error.c b/ndctl/clear-error.c
index 33d930a..1a9b94a 100644
--- a/ndctl/clear-error.c
+++ b/ndctl/clear-error.c
@@ -167,6 +167,7 @@  static int clear_error(struct clear_err *ce)
 	if (check_user_input_range(ce->region, clear_err.bb_start,
 				clear_err.bb_len) == 0) {
 		rc = -EINVAL;
+		error("badblocks region input out of range.\n");
 		goto cleanup;
 	}