diff mbox

[4/4] btrfs-progs: correct manpage option description for scrub

Message ID 1405564839-28702-4-git-send-email-guihc.fnst@cn.fujitsu.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Gui Hecheng July 17, 2014, 2:40 a.m. UTC
The -f option of scrub means to skip checking running scrub,
not to force checking.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 Documentation/btrfs-scrub.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Satoru Takeuchi July 30, 2014, 5:20 a.m. UTC | #1
Hi Gui,

(2014/07/17 11:40), Gui Hecheng wrote:
> The -f option of scrub means to skip checking running scrub,
> not to force checking.
> 
> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> ---
>   Documentation/btrfs-scrub.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/btrfs-scrub.txt b/Documentation/btrfs-scrub.txt
> index 7b27d63..1af9b9f 100644
> --- a/Documentation/btrfs-scrub.txt
> +++ b/Documentation/btrfs-scrub.txt
> @@ -47,7 +47,7 @@ manpage).
>   -n <ioprio_classdata>::::
>   Set IO priority classdata (see `ionice`(1) manpage).
>   -f::::
> -force to check whether scrub has started or resumed in userspace.
> +force to skip checking whether scrub has started or resumed in userspace.

I consider "scrub has started and resumed" is not user-friendly
expression. First, it can be replaced with more easy one,
"scrub is running". Second, there in no explanation about
this checking behavior before "-f" option's description.

So, how about the following idea?

 Fix 1. Add "If scrub is already running running, it fails."
        to the description before `Options` section
 Fix 2. Replace "force to check ..." with
        "force starting new scrub even if scrub is already running."
 Fix 3. Fix cmd_scrub_start_usage too.

Thanks,
Satoru

>   this is useful when scrub stat record file is damaged.
>   
>   *cancel* <path>|<device>::
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gui Hecheng July 30, 2014, 5:39 a.m. UTC | #2
On Wed, 2014-07-30 at 14:20 +0900, Satoru Takeuchi wrote:
> Hi Gui,
> 
> (2014/07/17 11:40), Gui Hecheng wrote:
> > The -f option of scrub means to skip checking running scrub,
> > not to force checking.
> > 
> > Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> > ---
> >   Documentation/btrfs-scrub.txt | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/btrfs-scrub.txt b/Documentation/btrfs-scrub.txt
> > index 7b27d63..1af9b9f 100644
> > --- a/Documentation/btrfs-scrub.txt
> > +++ b/Documentation/btrfs-scrub.txt
> > @@ -47,7 +47,7 @@ manpage).
> >   -n <ioprio_classdata>::::
> >   Set IO priority classdata (see `ionice`(1) manpage).
> >   -f::::
> > -force to check whether scrub has started or resumed in userspace.
> > +force to skip checking whether scrub has started or resumed in userspace.

Hi Satoru,
Thanks for your comments first. My opinions are as follows:

> I consider "scrub has started and resumed" is not user-friendly
> expression. First, it can be replaced with more easy one,
> "scrub is running". Second, there in no explanation about
> this checking behavior before "-f" option's description.

Yes, "Scrub is running" is more precise.

> So, how about the following idea?
> 
>  Fix 1. Add "If scrub is already running running, it fails."
>         to the description before `Options` section
  This is really a valuable idea. 

>  Fix 2. Replace "force to check ..." with
>         "force starting new scrub even if scrub is already running."
  This is more precise.

>  Fix 3. Fix cmd_scrub_start_usage too.
  Of course, thanks for reminding me.

So please let me rework this patch, could I add your sign-off-by then?

Thanks,
Gui
> 
> Thanks,
> Satoru
> 
> >   this is useful when scrub stat record file is damaged.
> >   
> >   *cancel* <path>|<device>::
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Satoru Takeuchi July 30, 2014, 6:09 a.m. UTC | #3
Hi Gui,

(2014/07/30 14:39), Gui Hecheng wrote:
> On Wed, 2014-07-30 at 14:20 +0900, Satoru Takeuchi wrote:
>> Hi Gui,
>>
>> (2014/07/17 11:40), Gui Hecheng wrote:
>>> The -f option of scrub means to skip checking running scrub,
>>> not to force checking.
>>>
>>> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
>>> ---
>>>    Documentation/btrfs-scrub.txt | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/btrfs-scrub.txt b/Documentation/btrfs-scrub.txt
>>> index 7b27d63..1af9b9f 100644
>>> --- a/Documentation/btrfs-scrub.txt
>>> +++ b/Documentation/btrfs-scrub.txt
>>> @@ -47,7 +47,7 @@ manpage).
>>>    -n <ioprio_classdata>::::
>>>    Set IO priority classdata (see `ionice`(1) manpage).
>>>    -f::::
>>> -force to check whether scrub has started or resumed in userspace.
>>> +force to skip checking whether scrub has started or resumed in userspace.
>
> Hi Satoru,
> Thanks for your comments first. My opinions are as follows:
>
>> I consider "scrub has started and resumed" is not user-friendly
>> expression. First, it can be replaced with more easy one,
>> "scrub is running". Second, there in no explanation about
>> this checking behavior before "-f" option's description.
>
> Yes, "Scrub is running" is more precise.
>
>> So, how about the following idea?
>>
>>   Fix 1. Add "If scrub is already running running, it fails."
>>          to the description before `Options` section
>    This is really a valuable idea.
>
>>   Fix 2. Replace "force to check ..." with
>>          "force starting new scrub even if scrub is already running."
>    This is more precise.
>
>>   Fix 3. Fix cmd_scrub_start_usage too.
>    Of course, thanks for reminding me.
>
> So please let me rework this patch,

Thank you for reworking.

>could I add your sign-off-by then?

Yes, please.

Thanks,
Satoru

>
> Thanks,
> Gui
>>
>> Thanks,
>> Satoru
>>
>>>    this is useful when scrub stat record file is damaged.
>>>
>>>    *cancel* <path>|<device>::
>>>
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/btrfs-scrub.txt b/Documentation/btrfs-scrub.txt
index 7b27d63..1af9b9f 100644
--- a/Documentation/btrfs-scrub.txt
+++ b/Documentation/btrfs-scrub.txt
@@ -47,7 +47,7 @@  manpage).
 -n <ioprio_classdata>::::
 Set IO priority classdata (see `ionice`(1) manpage).
 -f::::
-force to check whether scrub has started or resumed in userspace.
+force to skip checking whether scrub has started or resumed in userspace.
 this is useful when scrub stat record file is damaged.
 
 *cancel* <path>|<device>::