Re: [PATCH v3] ndctl: add clear error support for ndctl

["Kani, Toshimitsu" <toshi.kani@hpe.com>]

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


_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm