[PATCH 1/2] btrfs-progs: don't report internal dev replace result if ioctl failed
If BTRFS_IOC_DEV_REPLACE ioctl failed, there's no result returned to
fill args.result, it doesn't make sense to report this internal result
to user.
And the arg has been initialized with 0, the result is always 0, which
is BTRFS_IOCTL_DEV_REPLACE_REPLACE_NO_ERROR, and the resulting error
message looks confusing too:
ERROR: ioctl(DEV_REPLACE_START) failed on /mnt/btrfs: No such file or
directory, no error
So just skip the internal dev replace result if the whole ioctl failed.
Signed-off-by: Eryu Guan guane...@gmail.com
---
cmds-replace.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/cmds-replace.c b/cmds-replace.c
index 9fe7ad8..51b7aaf 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -187,9 +187,8 @@ static int cmd_start_replace(int argc, char **argv)
ret = ioctl(fdmnt, BTRFS_IOC_DEV_REPLACE, status_args);
if (ret) {
fprintf(stderr,
- ERROR: ioctl(DEV_REPLACE_STATUS) failed on \%s\: %s,
%s\n,
- path, strerror(errno),
- replace_dev_result2string(status_args.result));
+ ERROR: ioctl(DEV_REPLACE_STATUS) failed on \%s\:
%s\n,
+ path, strerror(errno));
goto leave_with_error;
}
@@ -305,9 +304,8 @@ static int cmd_start_replace(int argc, char **argv)
if (do_not_background) {
if (ret) {
fprintf(stderr,
- ERROR: ioctl(DEV_REPLACE_START) failed on
\%s\: %s, %s\n,
- path, strerror(errno),
- replace_dev_result2string(start_args.result));
+ ERROR: ioctl(DEV_REPLACE_START) failed on
\%s\: %s\n,
+ path, strerror(errno));
if (errno == EOPNOTSUPP)
fprintf(stderr,
@@ -404,9 +402,8 @@ static int print_replace_status(int fd, const char *path,
int once)
args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_STATUS;
ret = ioctl(fd, BTRFS_IOC_DEV_REPLACE, args);
if (ret) {
- fprintf(stderr, ERROR: ioctl(DEV_REPLACE_STATUS)
failed on \%s\: %s, %s\n,
- path, strerror(errno),
- replace_dev_result2string(args.result));
+ fprintf(stderr, ERROR: ioctl(DEV_REPLACE_STATUS)
failed on \%s\: %s\n,
+ path, strerror(errno));
return ret;
}
@@ -554,9 +551,8 @@ static int cmd_cancel_replace(int argc, char **argv)
e = errno;
close_file_or_dir(fd, dirstream);
if (ret) {
- fprintf(stderr, ERROR: ioctl(DEV_REPLACE_CANCEL) failed on
\%s\: %s, %s\n,
- path, strerror(e),
- replace_dev_result2string(args.result));
+ fprintf(stderr, ERROR: ioctl(DEV_REPLACE_CANCEL) failed on
\%s\: %s\n,
+ path, strerror(e));
return 1;
}
if (args.result == BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED) {
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] btrfs-progs: return error when canonicalize_path failed
Error out cmd_start_replace() if canonicalize_path() dstdev failed, add
the missing goto leave_with_error;
Signed-off-by: Eryu Guan guane...@gmail.com
---
cmds-replace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cmds-replace.c b/cmds-replace.c
index 51b7aaf..d5648d5 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -213,6 +213,7 @@ static int cmd_start_replace(int argc, char **argv)
fprintf(stderr,
ERROR: Could not canonicalize path '%s': %s\n,
argv[optind + 1], strerror(errno));
+ goto leave_with_error;
}
if (is_numerical(srcdev)) {
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Fix and enhance merge_extent_mapping() to insert best fitted extent map
On Fri, Sep 19, 2014 at 1:31 AM, Qu Wenruo quwen...@cn.fujitsu.com wrote:
Original Message
Subject: Re: [PATCH] btrfs: Fix and enhance merge_extent_mapping() to insert
best fitted extent map
From: Filipe David Manana fdman...@gmail.com
To: Qu Wenruo quwen...@cn.fujitsu.com
Date: 2014年09月18日 21:16
On Wed, Sep 17, 2014 at 4:53 AM, Qu Wenruo quwen...@cn.fujitsu.com
wrote:
The following commit enhanced the merge_extent_mapping() to reduce
fragment in extent map tree, but it can't handle case which existing
lies before map_start:
51f39 btrfs: Use right extent length when inserting overlap extent map.
[BUG]
When existing extent map's start is before map_start,
the em-len will be minus, which will corrupt the extent map and fail to
insert the new extent map.
This will happen when someone get a large extent map, but when it is
going to insert it into extent map tree, some one has already commit
some write and split the huge extent into small parts.
This sounds like very deterministic to me.
Any reason to not add tests to the sanity tests that exercise
this/these case/cases?
Yes, thanks for the informing.
Will add the test case for it soon.
Hi Qu,
Any progress on the test?
This is a very important one IMHO, not only because of the bad
consequences of the bug (extent map corruption, leading to all sorts
of chaos), but also because this problem was not found by the full
xfstests suite on several developer machines.
thanks
Thanks,
Qu
Thanks
[REPRODUCER]
It is very easy to tiger using filebench with randomrw personality.
It is about 100% to reproduce when using 8G preallocated file in 60s
randonrw test.
[FIX]
This patch can now handle any existing extent position.
Since it does not directly use existing-start, now it will find the
previous and next extent around map_start.
So the old existing-start map_start bug will never happen again.
[ENHANCE]
This patch will insert the best fitted extent map into extent map tree,
other than the oldest [map_start, map_start + sectorsize) or the
relatively newer but not perfect [map_start, existing-start).
The patch will first search existing extent that does not intersects with
the desired map range [map_start, map_start + len).
The existing extent will be either before or behind map_start, and based
on the existing extent, we can find out the previous and next extent
around map_start.
So the best fitted extent would be [prev-end, next-start).
For prev or next is not found, em-start would be prev-end and em-end
wold be next-start.
With this patch, the fragment in extent map tree should be reduced much
more than the 51f39 commit and reduce an unneeded extent map tree search.
Reported-by: Tsutomu Itoh t-i...@jp.fujitsu.com
Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com
---
fs/btrfs/inode.c | 79
1 file changed, 57 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 016c403..8039021 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6191,21 +6191,60 @@ out_fail_inode:
goto out_fail;
}
+/* Find next extent map of a given extent map, caller needs to ensure
locks */
+static struct extent_map *next_extent_map(struct extent_map *em)
+{
+ struct rb_node *next;
+
+ next = rb_next(em-rb_node);
+ if (!next)
+ return NULL;
+ return container_of(next, struct extent_map, rb_node);
+}
+
+static struct extent_map *prev_extent_map(struct extent_map *em)
+{
+ struct rb_node *prev;
+
+ prev = rb_prev(em-rb_node);
+ if (!prev)
+ return NULL;
+ return container_of(prev, struct extent_map, rb_node);
+}
+
/* helper for btfs_get_extent. Given an existing extent in the tree,
+ * the existing extent is the nearest extent to map_start,
* and an extent that you want to insert, deal with overlap and insert
- * the new extent into the tree.
+ * the best fitted new extent into the tree.
*/
static int merge_extent_mapping(struct extent_map_tree *em_tree,
struct extent_map *existing,
struct extent_map *em,
u64 map_start)
{
+ struct extent_map *prev;
+ struct extent_map *next;
+ u64 start;
+ u64 end;
u64 start_diff;
BUG_ON(map_start em-start || map_start =
extent_map_end(em));
- start_diff = map_start - em-start;
- em-start = map_start;
- em-len = existing-start - em-start;
+
+ if (existing-start map_start) {
+ next = existing;
+ prev = prev_extent_map(next);
+ } else {
+ prev = existing;
+ next = next_extent_map(prev);
+ }
+
+ start = prev ? extent_map_end(prev) : em-start;
+ start = max_t(u64, start, em-start);
+
Re: Two uncorrectable errors across RAID1 at same logical block?
On Mon, Oct 06, 2014 at 07:18:06PM -0700, Rich Rauenzahn wrote: On 10/6/2014 7:05 PM, Liu Bo wrote: btrfs inspect-internal logical-resolve 58464632832 $ sudo btrfs inspect-internal logical-resolve 58464632832 / ...no output? Hmm...have you tried the latest btrfs-progs? You can pull it or get a tar ball from git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git thanks, -liubo -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Two uncorrectable errors across RAID1 at same logical block?
On 10/8/2014 7:20 AM, Liu Bo wrote: On Mon, Oct 06, 2014 at 07:18:06PM -0700, Rich Rauenzahn wrote: On 10/6/2014 7:05 PM, Liu Bo wrote: btrfs inspect-internal logical-resolve 58464632832 $ sudo btrfs inspect-internal logical-resolve 58464632832 / ...no output? Hmm...have you tried the latest btrfs-progs? You can pull it or get a tar ball from git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git thanks, -liubo Still no output: $ sudo ./btrfs inspect-internal logical-resolve 58464632832 / Could it be a deleted file? -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What is the vision for btrfs fs repair?
I was looking at Marc's post: http://marc.merlins.org/perso/btrfs/post_2014-03-19_Btrfs-Tips_-Btrfs-Scrub-and-Btrfs-Filesystem-Repair.html and it feels like there isn't exactly a cohesive, overarching vision for repair of a corrupted btrfs filesystem. In other words - I'm an admin cruising along, when the kernel throws some fs corruption error, or for whatever reason btrfs fails to mount. What should I do? Marc lays out several steps, but to me this highlights that there seem to be a lot of disjoint mechanisms out there to deal with these problems; mostly from Marc's blog, with some bits of my own: * btrfs scrub Errors are corrected along if possible (what *is* possible?) * mount -o recovery Enable autorecovery attempts if a bad tree root is found at mount time. * mount -o degraded Allow mounts to continue with missing devices. (This isn't really a way to recover from corruption, right?) * btrfs-zero-log remove the log tree if log tree is corrupt * btrfs rescue Recover a damaged btrfs filesystem chunk-recover super-recover How does this relate to btrfs check? * btrfs check repair a btrfs filesystem --repair --init-csum-tree --init-extent-tree How does this relate to btrfs rescue? * btrfs restore try to salvage files from a damaged filesystem (not really repair, it's disk-scraping) What's the vision for, say, scrub vs. check vs. rescue? Should they repair the same errors, only online vs. offline? If not, what class of errors does one fix vs. the other? How would an admin know? Can btrfs check recover a bad tree root in the same way that mount -o recovery does? How would I know if I should use --init-*-tree, or chunk-recover, and what are the ramifications of using these options? It feels like recovery tools have been badly splintered, and if there's an overarching design or vision for btrfs fs repair, I can't tell what it is. Can anyone help me? Thanks, -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Fix and enhance merge_extent_mapping() to insert best fitted extent map
Original Message
Subject: Re: [PATCH] btrfs: Fix and enhance merge_extent_mapping() to
insert best fitted extent map
From: Filipe David Manana fdman...@gmail.com
To: Qu Wenruo quwen...@cn.fujitsu.com
Date: 2014年10月08日 20:08
On Fri, Sep 19, 2014 at 1:31 AM, Qu Wenruo quwen...@cn.fujitsu.com wrote:
Original Message
Subject: Re: [PATCH] btrfs: Fix and enhance merge_extent_mapping() to insert
best fitted extent map
From: Filipe David Manana fdman...@gmail.com
To: Qu Wenruo quwen...@cn.fujitsu.com
Date: 2014年09月18日 21:16
On Wed, Sep 17, 2014 at 4:53 AM, Qu Wenruo quwen...@cn.fujitsu.com
wrote:
The following commit enhanced the merge_extent_mapping() to reduce
fragment in extent map tree, but it can't handle case which existing
lies before map_start:
51f39 btrfs: Use right extent length when inserting overlap extent map.
[BUG]
When existing extent map's start is before map_start,
the em-len will be minus, which will corrupt the extent map and fail to
insert the new extent map.
This will happen when someone get a large extent map, but when it is
going to insert it into extent map tree, some one has already commit
some write and split the huge extent into small parts.
This sounds like very deterministic to me.
Any reason to not add tests to the sanity tests that exercise
this/these case/cases?
Yes, thanks for the informing.
Will add the test case for it soon.
Hi Qu,
Any progress on the test?
This is a very important one IMHO, not only because of the bad
consequences of the bug (extent map corruption, leading to all sorts
of chaos), but also because this problem was not found by the full
xfstests suite on several developer machines.
thanks
Still trying to reproduce it under xfstest framework.
But even followiiing the FileBench randomrw behavior(1 thread random
read 1 thread random write on preallocated space),
I still failed to reproduce it.
Still investigating how to reproduce it.
Worst case may be add a new C program into src of xfstests?
Thanks,
Qu
Thanks,
Qu
Thanks
[REPRODUCER]
It is very easy to tiger using filebench with randomrw personality.
It is about 100% to reproduce when using 8G preallocated file in 60s
randonrw test.
[FIX]
This patch can now handle any existing extent position.
Since it does not directly use existing-start, now it will find the
previous and next extent around map_start.
So the old existing-start map_start bug will never happen again.
[ENHANCE]
This patch will insert the best fitted extent map into extent map tree,
other than the oldest [map_start, map_start + sectorsize) or the
relatively newer but not perfect [map_start, existing-start).
The patch will first search existing extent that does not intersects with
the desired map range [map_start, map_start + len).
The existing extent will be either before or behind map_start, and based
on the existing extent, we can find out the previous and next extent
around map_start.
So the best fitted extent would be [prev-end, next-start).
For prev or next is not found, em-start would be prev-end and em-end
wold be next-start.
With this patch, the fragment in extent map tree should be reduced much
more than the 51f39 commit and reduce an unneeded extent map tree search.
Reported-by: Tsutomu Itoh t-i...@jp.fujitsu.com
Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com
---
fs/btrfs/inode.c | 79
1 file changed, 57 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 016c403..8039021 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6191,21 +6191,60 @@ out_fail_inode:
goto out_fail;
}
+/* Find next extent map of a given extent map, caller needs to ensure
locks */
+static struct extent_map *next_extent_map(struct extent_map *em)
+{
+ struct rb_node *next;
+
+ next = rb_next(em-rb_node);
+ if (!next)
+ return NULL;
+ return container_of(next, struct extent_map, rb_node);
+}
+
+static struct extent_map *prev_extent_map(struct extent_map *em)
+{
+ struct rb_node *prev;
+
+ prev = rb_prev(em-rb_node);
+ if (!prev)
+ return NULL;
+ return container_of(prev, struct extent_map, rb_node);
+}
+
/* helper for btfs_get_extent. Given an existing extent in the tree,
+ * the existing extent is the nearest extent to map_start,
* and an extent that you want to insert, deal with overlap and insert
- * the new extent into the tree.
+ * the best fitted new extent into the tree.
*/
static int merge_extent_mapping(struct extent_map_tree *em_tree,
struct extent_map *existing,
struct extent_map *em,
u64 map_start)
{
+ struct extent_map *prev;
+ struct extent_map *next;
+ u64 start;
+ u64 end;
u64 start_diff;
BUG_ON(map_start em-start || map_start =
