| Message ID | 1402539901-22779-2-git-send-email-guihc.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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; > -- 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
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 > -- 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
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; > > > -- 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
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 > -- 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
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) -- 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
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 -- 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 --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;
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(-)