* [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover
@ 2014-06-12 2:24 Gui Hecheng
2014-06-12 2:25 ` [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover Gui Hecheng
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Gui Hecheng @ 2014-06-12 2:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: Gui Hecheng
When deal with the p & q stripes for data profile raid6, chunk-recover
forgets to insert them into the chunk record. Just insert them back
freely.
Also wrap the inert procedure into a new function, fill_chunk_up.
Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
chunk-recover.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/chunk-recover.c b/chunk-recover.c
index dfa7ff6..9b46b0b 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -1785,6 +1785,23 @@ static inline int count_devext_records(struct list_head *record_list)
return num_of_records;
}
+static int fill_chunk_up(struct chunk_record *chunk, struct list_head *devexts,
+ struct recover_control *rc)
+{
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < chunk->num_stripes; i++) {
+ if (!chunk->stripes[i].devid) {
+ ret = insert_stripe(devexts, rc, chunk, i);
+ if (ret)
+ break;
+ }
+ }
+
+ return ret;
+}
+
#define EQUAL_STRIPE (1 << 0)
static int rebuild_raid_data_chunk_stripes(struct recover_control *rc,
@@ -1919,9 +1936,9 @@ next_csum:
num_unordered = count_devext_records(&unordered);
if (chunk->type_flags & BTRFS_BLOCK_GROUP_RAID6
&& num_unordered == 2) {
- list_splice_init(&unordered, &chunk->dextents);
btrfs_release_path(&path);
- return 0;
+ ret = fill_chunk_up(chunk, &unordered, rc);
+ return ret;
}
goto next_stripe;
@@ -1966,14 +1983,7 @@ out:
& BTRFS_BLOCK_GROUP_RAID5)
|| (num_unordered == 3 && chunk->type_flags
& BTRFS_BLOCK_GROUP_RAID6)) {
- for (i = 0; i < chunk->num_stripes; i++) {
- if (!chunk->stripes[i].devid) {
- ret = insert_stripe(&unordered, rc,
- chunk, i);
- if (ret)
- break;
- }
- }
+ ret = fill_chunk_up(chunk, &unordered, rc);
}
}
fail_out:
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-12 2:24 [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Gui Hecheng
@ 2014-06-12 2:25 ` Gui Hecheng
2014-06-25 2:17 ` Eric Sandeen
2014-06-12 2:25 ` [PATCH 3/3] btrfs-progs: cleanup unused assignment " Gui Hecheng
2014-06-12 7:45 ` [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Duncan
2 siblings, 1 reply; 11+ messages in thread
From: Gui Hecheng @ 2014-06-12 2:25 UTC (permalink / raw)
To: linux-btrfs; +Cc: Gui Hecheng
When run chunk-recover on a health btrfs(data profile raid0, with
plenty of data), the program has a chance to abort on the number
of mirrors of an extent.
According to the kernel code, the max mirror number of an extent
is 3 not 2:
ctree.h: BTRFS_MAX_MIRRORS 3
chunk-recover.c : BTRFS_NUM_MIRRORS 2
just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
chunk-recover.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/chunk-recover.c b/chunk-recover.c
index 9b46b0b..d5a688e 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -42,7 +42,7 @@
#include "btrfsck.h"
#include "commands.h"
-#define BTRFS_NUM_MIRRORS 2
+#define BTRFS_NUM_MIRRORS 3
struct recover_control {
int verbose;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] btrfs-progs: cleanup unused assignment for chunk-recover
2014-06-12 2:24 [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Gui Hecheng
2014-06-12 2:25 ` [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover Gui Hecheng
@ 2014-06-12 2:25 ` Gui Hecheng
2014-06-12 7:45 ` [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Duncan
2 siblings, 0 replies; 11+ messages in thread
From: Gui Hecheng @ 2014-06-12 2:25 UTC (permalink / raw)
To: linux-btrfs; +Cc: Gui Hecheng
The 'num_unordered' will be recounted after 'goto out',
just remove it.
Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
chunk-recover.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/chunk-recover.c b/chunk-recover.c
index d5a688e..8bc5bc3 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -1905,7 +1905,6 @@ next_csum:
fprintf(stderr, "Fetch csum failed\n");
goto fail_out;
} else if (ret == 1) {
- num_unordered = count_devext_records(&unordered);
if (!(*flags & EQUAL_STRIPE))
*flags |= EQUAL_STRIPE;
goto out;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover
2014-06-12 2:24 [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Gui Hecheng
2014-06-12 2:25 ` [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover Gui Hecheng
2014-06-12 2:25 ` [PATCH 3/3] btrfs-progs: cleanup unused assignment " Gui Hecheng
@ 2014-06-12 7:45 ` Duncan
2014-06-12 8:11 ` Gui Hecheng
2 siblings, 1 reply; 11+ messages in thread
From: Duncan @ 2014-06-12 7:45 UTC (permalink / raw)
To: linux-btrfs
Gui Hecheng posted on Thu, 12 Jun 2014 10:24:59 +0800 as excerpted:
> When deal with the p & q stripes for data profile raid6, chunk-recover
> forgets to insert them into the chunk record. Just insert them back
> freely.
> Also wrap the inert procedure into a new function, fill_chunk_up.
If there's a v2 anyway: s/inert/insert/ ?
(For a moment I was contemplating the reasons one might wrap a stub,
which is how I was translating inert procedure. Then it hit me... =:^)
--
Duncan - List replies preferred. No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master." Richard Stallman
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover
2014-06-12 7:45 ` [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Duncan
@ 2014-06-12 8:11 ` Gui Hecheng
0 siblings, 0 replies; 11+ messages in thread
From: Gui Hecheng @ 2014-06-12 8:11 UTC (permalink / raw)
To: Duncan; +Cc: linux-btrfs
On Thu, 2014-06-12 at 07:45 +0000, Duncan wrote:
> Gui Hecheng posted on Thu, 12 Jun 2014 10:24:59 +0800 as excerpted:
>
> > When deal with the p & q stripes for data profile raid6, chunk-recover
> > forgets to insert them into the chunk record. Just insert them back
> > freely.
> > Also wrap the inert procedure into a new function, fill_chunk_up.
>
> If there's a v2 anyway: s/inert/insert/ ?
Oh, yes, 'insert' is the right thing. Thanks very much! I'll send a v2.
-Gui
> (For a moment I was contemplating the reasons one might wrap a stub,
> which is how I was translating inert procedure. Then it hit me... =:^)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-12 2:25 ` [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover Gui Hecheng
@ 2014-06-25 2:17 ` Eric Sandeen
2014-06-25 2:20 ` Wang Shilong
2014-06-25 2:22 ` Gui Hecheng
0 siblings, 2 replies; 11+ messages in thread
From: Eric Sandeen @ 2014-06-25 2:17 UTC (permalink / raw)
To: Gui Hecheng, linux-btrfs
On 6/11/14, 9:25 PM, Gui Hecheng wrote:
> When run chunk-recover on a health btrfs(data profile raid0, with
> plenty of data), the program has a chance to abort on the number
> of mirrors of an extent.
>
> According to the kernel code, the max mirror number of an extent
> is 3 not 2:
> ctree.h: BTRFS_MAX_MIRRORS 3
> chunk-recover.c : BTRFS_NUM_MIRRORS 2
> just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
Wouldn't it make a lot more sense, then, to change the userspace
macro to be called BTRFS_MAX_MIRRORS as well?
-Eric
> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> ---
> chunk-recover.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/chunk-recover.c b/chunk-recover.c
> index 9b46b0b..d5a688e 100644
> --- a/chunk-recover.c
> +++ b/chunk-recover.c
> @@ -42,7 +42,7 @@
> #include "btrfsck.h"
> #include "commands.h"
>
> -#define BTRFS_NUM_MIRRORS 2
> +#define BTRFS_NUM_MIRRORS 3
>
> struct recover_control {
> int verbose;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-25 2:17 ` Eric Sandeen
@ 2014-06-25 2:20 ` Wang Shilong
2014-06-25 2:22 ` Gui Hecheng
1 sibling, 0 replies; 11+ messages in thread
From: Wang Shilong @ 2014-06-25 2:20 UTC (permalink / raw)
To: Eric Sandeen, Gui Hecheng, linux-btrfs
On 06/25/2014 10:17 AM, Eric Sandeen wrote:
> On 6/11/14, 9:25 PM, Gui Hecheng wrote:
>> When run chunk-recover on a health btrfs(data profile raid0, with
>> plenty of data), the program has a chance to abort on the number
>> of mirrors of an extent.
>>
>> According to the kernel code, the max mirror number of an extent
>> is 3 not 2:
>> ctree.h: BTRFS_MAX_MIRRORS 3
>> chunk-recover.c : BTRFS_NUM_MIRRORS 2
>> just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
> Wouldn't it make a lot more sense, then, to change the userspace
> macro to be called BTRFS_MAX_MIRRORS as well?
Yeah, agree, Nice review. :-)
Wang
>
> -Eric
>
>> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
>> ---
>> chunk-recover.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/chunk-recover.c b/chunk-recover.c
>> index 9b46b0b..d5a688e 100644
>> --- a/chunk-recover.c
>> +++ b/chunk-recover.c
>> @@ -42,7 +42,7 @@
>> #include "btrfsck.h"
>> #include "commands.h"
>>
>> -#define BTRFS_NUM_MIRRORS 2
>> +#define BTRFS_NUM_MIRRORS 3
>>
>> struct recover_control {
>> int verbose;
>>
> --
> 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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-25 2:17 ` Eric Sandeen
2014-06-25 2:20 ` Wang Shilong
@ 2014-06-25 2:22 ` Gui Hecheng
2014-06-25 5:14 ` Eric Sandeen
1 sibling, 1 reply; 11+ messages in thread
From: Gui Hecheng @ 2014-06-25 2:22 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
On Tue, 2014-06-24 at 21:17 -0500, Eric Sandeen wrote:
> On 6/11/14, 9:25 PM, Gui Hecheng wrote:
> > When run chunk-recover on a health btrfs(data profile raid0, with
> > plenty of data), the program has a chance to abort on the number
> > of mirrors of an extent.
> >
> > According to the kernel code, the max mirror number of an extent
> > is 3 not 2:
> > ctree.h: BTRFS_MAX_MIRRORS 3
> > chunk-recover.c : BTRFS_NUM_MIRRORS 2
> > just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
>
> Wouldn't it make a lot more sense, then, to change the userspace
> macro to be called BTRFS_MAX_MIRRORS as well?
>
> -Eric
>
Yes, Eric, unify the names between userspace and kernelspace is really a
good point. Also, I plan to move the macro into ctree.h, what do you
think?
-Gui
> > Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> > ---
> > chunk-recover.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/chunk-recover.c b/chunk-recover.c
> > index 9b46b0b..d5a688e 100644
> > --- a/chunk-recover.c
> > +++ b/chunk-recover.c
> > @@ -42,7 +42,7 @@
> > #include "btrfsck.h"
> > #include "commands.h"
> >
> > -#define BTRFS_NUM_MIRRORS 2
> > +#define BTRFS_NUM_MIRRORS 3
> >
> > struct recover_control {
> > int verbose;
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-25 2:22 ` Gui Hecheng
@ 2014-06-25 5:14 ` Eric Sandeen
2014-06-25 5:25 ` Eric Sandeen
0 siblings, 1 reply; 11+ messages in thread
From: Eric Sandeen @ 2014-06-25 5:14 UTC (permalink / raw)
To: Gui Hecheng; +Cc: linux-btrfs
On 6/24/14, 9:22 PM, Gui Hecheng wrote:
> On Tue, 2014-06-24 at 21:17 -0500, Eric Sandeen wrote:
>> On 6/11/14, 9:25 PM, Gui Hecheng wrote:
>>> When run chunk-recover on a health btrfs(data profile raid0, with
>>> plenty of data), the program has a chance to abort on the number
>>> of mirrors of an extent.
>>>
>>> According to the kernel code, the max mirror number of an extent
>>> is 3 not 2:
>>> ctree.h: BTRFS_MAX_MIRRORS 3
>>> chunk-recover.c : BTRFS_NUM_MIRRORS 2
>>> just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
>>
>> Wouldn't it make a lot more sense, then, to change the userspace
>> macro to be called BTRFS_MAX_MIRRORS as well?
>>
>> -Eric
>>
> Yes, Eric, unify the names between userspace and kernelspace is really a
> good point. Also, I plan to move the macro into ctree.h, what do you
> think?
It's only used in chunk-recover.c, so I don't see much point to moving it
to a new file.
To be honest, I haven't paid a lot of attention to the code. The macro
*is* actually used to limit the same thing in userspace as in kernelspace,
right? ;)
-Eric
> -Gui
>
>>> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
>>> ---
>>> chunk-recover.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/chunk-recover.c b/chunk-recover.c
>>> index 9b46b0b..d5a688e 100644
>>> --- a/chunk-recover.c
>>> +++ b/chunk-recover.c
>>> @@ -42,7 +42,7 @@
>>> #include "btrfsck.h"
>>> #include "commands.h"
>>>
>>> -#define BTRFS_NUM_MIRRORS 2
>>> +#define BTRFS_NUM_MIRRORS 3
>>>
>>> struct recover_control {
>>> int verbose;
>>>
>>
>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-25 5:14 ` Eric Sandeen
@ 2014-06-25 5:25 ` Eric Sandeen
2014-06-25 6:51 ` Gui Hecheng
0 siblings, 1 reply; 11+ messages in thread
From: Eric Sandeen @ 2014-06-25 5:25 UTC (permalink / raw)
To: Gui Hecheng; +Cc: linux-btrfs
On 6/25/14, 12:14 AM, Eric Sandeen wrote:
> On 6/24/14, 9:22 PM, Gui Hecheng wrote:
>> > On Tue, 2014-06-24 at 21:17 -0500, Eric Sandeen wrote:
>>> >> On 6/11/14, 9:25 PM, Gui Hecheng wrote:
>>>> >>> When run chunk-recover on a health btrfs(data profile raid0, with
>>>> >>> plenty of data), the program has a chance to abort on the number
>>>> >>> of mirrors of an extent.
>>>> >>>
>>>> >>> According to the kernel code, the max mirror number of an extent
>>>> >>> is 3 not 2:
>>>> >>> ctree.h: BTRFS_MAX_MIRRORS 3
>>>> >>> chunk-recover.c : BTRFS_NUM_MIRRORS 2
>>>> >>> just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
>>> >>
>>> >> Wouldn't it make a lot more sense, then, to change the userspace
>>> >> macro to be called BTRFS_MAX_MIRRORS as well?
>>> >>
>>> >> -Eric
>>> >>
>> > Yes, Eric, unify the names between userspace and kernelspace is really a
>> > good point. Also, I plan to move the macro into ctree.h, what do you
>> > think?
> It's only used in chunk-recover.c, so I don't see much point to moving it
> to a new file.
Sorry, I take that back. Actually -
Yes, I think it does make sense, just so that userspace moves slightly closer to
kernelspace.
-Eric (who said long ago that he wanted to try to sync things up, but
found himself daunted by the task, and failed)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover
2014-06-25 5:25 ` Eric Sandeen
@ 2014-06-25 6:51 ` Gui Hecheng
0 siblings, 0 replies; 11+ messages in thread
From: Gui Hecheng @ 2014-06-25 6:51 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
On Wed, 2014-06-25 at 00:25 -0500, Eric Sandeen wrote:
> On 6/25/14, 12:14 AM, Eric Sandeen wrote:
> > On 6/24/14, 9:22 PM, Gui Hecheng wrote:
> >> > On Tue, 2014-06-24 at 21:17 -0500, Eric Sandeen wrote:
> >>> >> On 6/11/14, 9:25 PM, Gui Hecheng wrote:
> >>>> >>> When run chunk-recover on a health btrfs(data profile raid0, with
> >>>> >>> plenty of data), the program has a chance to abort on the number
> >>>> >>> of mirrors of an extent.
> >>>> >>>
> >>>> >>> According to the kernel code, the max mirror number of an extent
> >>>> >>> is 3 not 2:
> >>>> >>> ctree.h: BTRFS_MAX_MIRRORS 3
> >>>> >>> chunk-recover.c : BTRFS_NUM_MIRRORS 2
> >>>> >>> just change BTRFS_NUM_MIRRORS to 3, and everything goes well.
> >>> >>
> >>> >> Wouldn't it make a lot more sense, then, to change the userspace
> >>> >> macro to be called BTRFS_MAX_MIRRORS as well?
> >>> >>
> >>> >> -Eric
> >>> >>
> >> > Yes, Eric, unify the names between userspace and kernelspace is really a
> >> > good point. Also, I plan to move the macro into ctree.h, what do you
> >> > think?
> > It's only used in chunk-recover.c, so I don't see much point to moving it
> > to a new file.
>
> Sorry, I take that back. Actually -
>
> Yes, I think it does make sense, just so that userspace moves slightly closer to
> kernelspace.
>
> -Eric (who said long ago that he wanted to try to sync things up, but
> found himself daunted by the task, and failed)
Aha,it's really a huge work for one person to do the sync things.
But Rome was not built in one day by one guy. We have a long way to go
and let's take one more step now :)
-Gui
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-06-25 6:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-12 2:24 [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Gui Hecheng
2014-06-12 2:25 ` [PATCH 2/3] btrfs-progs: fix max mirror number error for chunk-recover Gui Hecheng
2014-06-25 2:17 ` Eric Sandeen
2014-06-25 2:20 ` Wang Shilong
2014-06-25 2:22 ` Gui Hecheng
2014-06-25 5:14 ` Eric Sandeen
2014-06-25 5:25 ` Eric Sandeen
2014-06-25 6:51 ` Gui Hecheng
2014-06-12 2:25 ` [PATCH 3/3] btrfs-progs: cleanup unused assignment " Gui Hecheng
2014-06-12 7:45 ` [PATCH 1/3] btrfs-progs: fix missing parity stripe for raid6 in chunk-recover Duncan
2014-06-12 8:11 ` Gui Hecheng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.