* [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super
@ 2016-09-20 2:19 Qu Wenruo
2016-09-20 2:19 ` [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function Qu Wenruo
2016-09-23 13:22 ` [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super David Sterba
0 siblings, 2 replies; 5+ messages in thread
From: Qu Wenruo @ 2016-09-20 2:19 UTC (permalink / raw)
To: linux-btrfs; +Cc: hawken
btrfs_read_dev_super() only returns 0 or -1, which doesn't really help,
caller won't know if it's caused by bad superblock or superblock out of
range.
Change it to return -ENOENT for superblock which is out of range, and
return -EIO for bad superblocks.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
disk-io.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/disk-io.c b/disk-io.c
index 854c285..26852ee 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1590,14 +1590,15 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr,
if (sb_bytenr != BTRFS_SUPER_INFO_OFFSET) {
ret = pread64(fd, buf, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
+ /* Special return value for out of dev/file range */
if (ret < BTRFS_SUPER_INFO_SIZE)
- return -1;
+ return -ENOENT;
if (btrfs_super_bytenr(buf) != sb_bytenr)
- return -1;
+ return -EIO;
if (check_super(buf, sbflags))
- return -1;
+ return -EIO;
memcpy(sb, buf, BTRFS_SUPER_INFO_SIZE);
return 0;
}
@@ -1641,7 +1642,7 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr,
}
}
- return transid > 0 ? 0 : -1;
+ return transid > 0 ? 0 : -EIO;
}
static int write_dev_supers(struct btrfs_root *root,
--
2.10.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function
2016-09-20 2:19 [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super Qu Wenruo
@ 2016-09-20 2:19 ` Qu Wenruo
2016-09-23 13:26 ` David Sterba
2016-09-23 13:22 ` [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super David Sterba
1 sibling, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2016-09-20 2:19 UTC (permalink / raw)
To: linux-btrfs; +Cc: hawken
We have enhanced super block validation check in disk_io.c, while
super-recover doesn't use that function and use a custom one.
That is duplicated, so reuse btrfs_read_dev_super to handle it, which
not only enhanced super check, but also reduce the code length.
Reported-by: hawken <hawken@thehawken.org>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
super-recover.c | 39 +++++++++++----------------------------
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/super-recover.c b/super-recover.c
index ecd1215..5298d46 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -88,24 +88,6 @@ void free_recover_superblock(struct btrfs_recover_superblock *recover)
}
}
-static int check_super(u64 bytenr, struct btrfs_super_block *sb)
-{
- int csum_size = btrfs_super_csum_size(sb);
- u8 result[csum_size];
- u32 crc = ~(u32)0;
-
- if (btrfs_super_bytenr(sb) != bytenr)
- return 0;
- if (sb->magic != cpu_to_le64(BTRFS_MAGIC))
- return 0;
-
- crc = btrfs_csum_data(NULL, (char *)sb + BTRFS_CSUM_SIZE,
- crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
- btrfs_csum_final(crc, result);
-
- return !memcmp(sb, &result, csum_size);
-}
-
static int add_superblock_record(struct btrfs_super_block *sb, char *fname,
u64 bytenr, struct list_head *head)
{
@@ -133,24 +115,20 @@ read_dev_supers(char *filename, struct btrfs_recover_superblock *recover)
int i, ret, fd;
u8 buf[BTRFS_SUPER_INFO_SIZE];
u64 max_gen, bytenr;
+ struct btrfs_super_block *sb = (struct btrfs_super_block *)buf;
+
/* just ignore errno that were set in btrfs_scan_fs_devices() */
errno = 0;
- struct btrfs_super_block *sb = (struct btrfs_super_block *)buf;
-
fd = open(filename, O_RDONLY);
if (fd < 0)
return -errno;
for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
bytenr = btrfs_sb_offset(i);
- ret = pread64(fd, buf, sizeof(buf), bytenr);
- if (ret < sizeof(buf)) {
- ret = -errno;
- goto out;
- }
- ret = check_super(bytenr, sb);
- if (ret) {
+
+ ret = btrfs_read_dev_super(fd, sb, bytenr, SBREAD_DEFAULT);
+ if (!ret) {
ret = add_superblock_record(sb, filename, bytenr,
&recover->good_supers);
if (ret)
@@ -158,13 +136,18 @@ read_dev_supers(char *filename, struct btrfs_recover_superblock *recover)
max_gen = btrfs_super_generation(sb);
if (max_gen > recover->max_generation)
recover->max_generation = max_gen;
- } else {
+ } else if (ret == -EIO){
+ /*
+ * Skip superblock which doesn't exist, only adds
+ * really corrupted superblock
+ */
ret = add_superblock_record(sb, filename, bytenr,
&recover->bad_supers);
if (ret)
goto out;
}
}
+ ret = 0;
out:
close(fd);
return ret;
--
2.10.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super
2016-09-20 2:19 [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super Qu Wenruo
2016-09-20 2:19 ` [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function Qu Wenruo
@ 2016-09-23 13:22 ` David Sterba
1 sibling, 0 replies; 5+ messages in thread
From: David Sterba @ 2016-09-23 13:22 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, hawken
On Tue, Sep 20, 2016 at 10:19:01AM +0800, Qu Wenruo wrote:
> btrfs_read_dev_super() only returns 0 or -1, which doesn't really help,
> caller won't know if it's caused by bad superblock or superblock out of
> range.
>
> Change it to return -ENOENT for superblock which is out of range, and
> return -EIO for bad superblocks.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> disk-io.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/disk-io.c b/disk-io.c
> index 854c285..26852ee 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1590,14 +1590,15 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr,
>
> if (sb_bytenr != BTRFS_SUPER_INFO_OFFSET) {
> ret = pread64(fd, buf, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
pread64 sets errno, should be returned
> + /* Special return value for out of dev/file range */
> if (ret < BTRFS_SUPER_INFO_SIZE)
> - return -1;
> + return -ENOENT;
>
> if (btrfs_super_bytenr(buf) != sb_bytenr)
> - return -1;
> + return -EIO;
>
> if (check_super(buf, sbflags))
> - return -1;
> + return -EIO;
and we should return what check_super returns, though it's -EIO in most
cases.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function
2016-09-20 2:19 ` [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function Qu Wenruo
@ 2016-09-23 13:26 ` David Sterba
2016-09-26 3:05 ` Qu Wenruo
0 siblings, 1 reply; 5+ messages in thread
From: David Sterba @ 2016-09-23 13:26 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, hawken
On Tue, Sep 20, 2016 at 10:19:02AM +0800, Qu Wenruo wrote:
> We have enhanced super block validation check in disk_io.c, while
> super-recover doesn't use that function and use a custom one.
>
> That is duplicated, so reuse btrfs_read_dev_super to handle it, which
> not only enhanced super check, but also reduce the code length.
Yeah, the existing check_super does way more checks compared to this
one. Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function
2016-09-23 13:26 ` David Sterba
@ 2016-09-26 3:05 ` Qu Wenruo
0 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2016-09-26 3:05 UTC (permalink / raw)
To: dsterba, linux-btrfs, hawken
At 09/23/2016 09:26 PM, David Sterba wrote:
> On Tue, Sep 20, 2016 at 10:19:02AM +0800, Qu Wenruo wrote:
>> We have enhanced super block validation check in disk_io.c, while
>> super-recover doesn't use that function and use a custom one.
>>
>> That is duplicated, so reuse btrfs_read_dev_super to handle it, which
>> not only enhanced super check, but also reduce the code length.
>
> Yeah, the existing check_super does way more checks compared to this
> one. Applied, thanks.
>
>
Sorry David, this patch can't be applied without previous patch.
As this patch relies on previous patch to get meaningful return value,
to check if it's a real error or that superblock just doesn't exist.
I'll update previous patch soon.
Thanks,
Qu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-26 3:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20 2:19 [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super Qu Wenruo
2016-09-20 2:19 ` [PATCH 2/2] btrfs-progs: super-recover: Reuse btrfs_read_dev_super function Qu Wenruo
2016-09-23 13:26 ` David Sterba
2016-09-26 3:05 ` Qu Wenruo
2016-09-23 13:22 ` [PATCH 1/2] btrfs-progs: Return more meaningful value for btrfs_read_dev_super David Sterba
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.