[PATCH 1/4] btrfs-progs: judge the return value of check_mounted more accurately

[PATCH 1/4] btrfs-progs: judge the return value of check_mounted more accurately

[Gui Hecheng]

For btrfs-convert, btrfstune, btrfs rescue, they report "device busy"
when given a device that does not actually exist e.g.

	# btrfstune -x abcdefg (this device does not exist)
	$ ...device busy...

We deal with this case by add "ret < 0" error check when
judging the return value of check_mounted.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 btrfs-convert.c |  7 ++++++-
 btrfstune.c     |  7 ++++++-
 cmds-rescue.c   | 14 +++++++++++---
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index 1b66de7..a8b2c51 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2729,7 +2729,12 @@ int main(int argc, char *argv[])
 	}
 
 	file = argv[optind];
-	if (check_mounted(file)) {
+	ret = check_mounted(file);
+	if (ret < 0) {
+		fprintf(stderr, "Could not check mount status: %s\n",
+			strerror(-ret));
+		return 1;
+	} else if (ret) {
 		fprintf(stderr, "%s is mounted\n", file);
 		return 1;
 	}
diff --git a/btrfstune.c b/btrfstune.c
index da82f36..2e623f3 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -151,7 +151,12 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	if (check_mounted(device)) {
+	ret = check_mounted(device);
+	if (ret < 0) {
+		fprintf(stderr, "Could not check mount status: %s\n",
+			strerror(-ret));
+		return 1;
+	} else if (ret) {
 		fprintf(stderr, "%s is mounted\n", device);
 		return 1;
 	}
diff --git a/cmds-rescue.c b/cmds-rescue.c
index e18eb98..f20a206 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -80,9 +80,13 @@ int cmd_chunk_recover(int argc, char *argv[])
 	file = argv[optind];
 
 	ret = check_mounted(file);
-	if (ret) {
+	if (ret < 0) {
+		fprintf(stderr, "Could not check mount status: %s\n",
+			strerror(-ret));
+		return 1;
+	} else if (ret) {
 		fprintf(stderr, "the device is busy\n");
-		return ret;
+		return 1;
 	}
 
 	ret = btrfs_recover_chunk_tree(file, verbose, yes);
@@ -133,7 +137,11 @@ int cmd_super_recover(int argc, char **argv)
 
 	dname = argv[optind];
 	ret = check_mounted(dname);
-	if (ret) {
+	if (ret < 0) {
+		fprintf(stderr, "Could not check mount status: %s\n",
+			strerror(-ret));
+		return 1;
+	} else if (ret) {
 		fprintf(stderr, "the device is busy\n");
 		return 1;
 	}
-- 
1.8.1.4

[PATCH 2/4] btrfs-progs: fix segment fault when exec btrfs-debug-tree as non-root

[Gui Hecheng]

When exec btrfs-debug-tree as non-root user, we get a segment fault.
Because the btrfs_scan_block_devices return a success 0 when we fail
to open a device. Now we just return the errno if this case happens.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 utils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/utils.c b/utils.c
index 97e23d5..1878abc 100644
--- a/utils.c
+++ b/utils.c
@@ -1517,7 +1517,8 @@ scan_again:
 		scans++;
 		goto scan_again;
 	}
-	return 0;
+
+	return errno ? -errno : 0;
 }
 
 u64 parse_size(char *s)
-- 
1.8.1.4

Re: [PATCH 2/4] btrfs-progs: fix segment fault when exec btrfs-debug-tree as non-root

[Mike Fleetwood]

On 20 February 2014 02:49, Gui Hecheng <guihc.fnst@cn.fujitsu.com> wrote:
> When exec btrfs-debug-tree as non-root user, we get a segment fault.
> Because the btrfs_scan_block_devices return a success 0 when we fail
> to open a device. Now we just return the errno if this case happens.
>
> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> ---
>  utils.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/utils.c b/utils.c
> index 97e23d5..1878abc 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1517,7 +1517,8 @@ scan_again:
>                 scans++;
>                 goto scan_again;
>         }
> -       return 0;
> +
> +       return errno ? -errno : 0;
>  }
>
>  u64 parse_size(char *s)
> --
> 1.8.1.4

Hi Gui,

This strikes me as not not right because errno is only documented as
being set when open() returns -1 on failure.  In the success case
errno is not set so you can't assume it will be 0.

I think the following might work:
1) Initilase ret = 0 at the start of btrfs_scan_block_devices()
2) In the open(fullpath, O_RDONLY) failure case set ret = fd
3) return ret

Thanks,
Mike

Re: [PATCH 2/4] btrfs-progs: fix segment fault when exec btrfs-debug-tree as non-root

[Gui Hecheng]

On Thu, 2014-02-20 at 08:48 +0000, Mike Fleetwood wrote:
> On 20 February 2014 02:49, Gui Hecheng <guihc.fnst@cn.fujitsu.com> wrote:
> > When exec btrfs-debug-tree as non-root user, we get a segment fault.
> > Because the btrfs_scan_block_devices return a success 0 when we fail
> > to open a device. Now we just return the errno if this case happens.
> >
> > Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> > ---
> >  utils.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/utils.c b/utils.c
> > index 97e23d5..1878abc 100644
> > --- a/utils.c
> > +++ b/utils.c
> > @@ -1517,7 +1517,8 @@ scan_again:
> >                 scans++;
> >                 goto scan_again;
> >         }
> > -       return 0;
> > +
> > +       return errno ? -errno : 0;
> >  }
> >
> >  u64 parse_size(char *s)
> > --
> > 1.8.1.4
> 
> Hi Gui,
> 
> This strikes me as not not right because errno is only documented as
> being set when open() returns -1 on failure.  In the success case
> errno is not set so you can't assume it will be 0.
> 
> I think the following might work:
> 1) Initilase ret = 0 at the start of btrfs_scan_block_devices()
> 2) In the open(fullpath, O_RDONLY) failure case set ret = fd
> 3) return ret
> 
> Thanks,
> Mike
Hi Mike,
Thanks for your comment, I think it is a reasonable way to fix the ret
value problem.
But I am sorry that my patch does not really address the segmentfault
problem.

Please *ignore* this patch...

-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

[PATCH 3/4] btrfs-progs: fix wrong error msg for exec btrfsck as non-root

[Gui Hecheng]

When exec btrfsck as non-root user on a disk, btrfsck will always
warn that "No such file or directory", despite that a directory
(e.g. /dev/vboxusb)actually exists. We just have no permission.
In this case, return the -errno set by the opendir call in
btrfs_scan_one_dir rather than blindly return -ENOENT.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils.c b/utils.c
index 1878abc..12ed2f4 100644
--- a/utils.c
+++ b/utils.c
@@ -1114,7 +1114,7 @@ again:
 	dirp = opendir(dirname);
 	if (!dirp) {
 		fprintf(stderr, "Unable to open %s for scanning\n", dirname);
-		ret = -ENOENT;
+		ret = -errno;
 		goto fail;
 	}
 	while(1) {
-- 
1.8.1.4

[PATCH 4/4] btrfs-progs: clean the unnecessary error msg of btrfs-file-show

[Gui Hecheng]

If the new version of btrfs-file-show fails, it will try to use the
old version. But before that, an error msg of the failed ioctl
shows up which causes xfstests case btrfs/006 to get a mismatch output.
I think we can just remove the error msg since the btrfs-file-show
still works with right outputs. Showing an error msg together with
the right result is really awkward.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 utils.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/utils.c b/utils.c
index 12ed2f4..9408885 100644
--- a/utils.c
+++ b/utils.c
@@ -2206,8 +2206,6 @@ again:
 		free(fsargs);
 		goto again;
 	} else if (ret < 0) {
-		printf("ERROR: scan_fsid ioctl failed - %s\n",
-			strerror(e));
 		ret = -e;
 		goto out;
 	}
-- 
1.8.1.4