All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical"
@ 2011-07-07 16:01 Jan Schmidt
  2011-07-07 16:01 ` [PATCH v1 1/2] btrfs-list: split list_subvols Jan Schmidt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jan Schmidt @ 2011-07-07 16:01 UTC (permalink / raw)
  To: chris.mason, linux-btrfs

The kernel patch series just sent (Subject: "Btrfs: scrub: print path to
corrupted files and trigger nodatasum fixup") introduces two new ioctls to
do in-kernel filesystem path construction. This series provides the
corresponding userspace changes, adding two new commands to the btrfs utility:

--
btrfs resolve inode [-v] <inode> <path>
	resolves an <inode> to all filesystem paths local to the fs mounted
	at <path>.
		-v  print count of returned and missed paths

btrfs resolve logical [-v] [-P] <logical> <path>
	resolves a <logical> address to all filesystem paths in the file
	system mounted at <path> and all its subvolumes.
		-v  print count of returned and missed inode/offset/root
		    triples
		-P  do not resolve the path but stop after finding all
		    inodes at this logical address and print them instead
--

These patches are based on Hugo's current integration branch.

Please try them out and report bugs here. I'll send an update to the manpages
later.

-Jan

Jan Schmidt (2):
  btrfs-list: split list_subvols
  added ioctls and commands to resolve inodes and logical addresses

 btrfs-list.c |  139 ++++++++++++++++++++++++++++++++++------------
 btrfs.c      |   10 +++
 btrfs_cmds.c |  177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 btrfs_cmds.h |    3 +
 ioctl.h      |   29 ++++++++++
 5 files changed, 323 insertions(+), 35 deletions(-)

-- 
1.7.3.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1 1/2] btrfs-list: split list_subvols
  2011-07-07 16:01 [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Jan Schmidt
@ 2011-07-07 16:01 ` Jan Schmidt
  2011-07-07 16:01 ` [PATCH v1 2/2] added ioctls and commands to resolve inodes and logical addresses Jan Schmidt
  2011-07-07 23:19 ` [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Goffredo Baroncelli
  2 siblings, 0 replies; 6+ messages in thread
From: Jan Schmidt @ 2011-07-07 16:01 UTC (permalink / raw)
  To: chris.mason, linux-btrfs

split list_subvols to separate functions and allow printing only in the
containing function. lets us make use of those functions when resolving
logical addresses.

Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
---
 btrfs-list.c |  104 ++++++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/btrfs-list.c b/btrfs-list.c
index 07b179a..dd685c2 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -199,10 +199,9 @@ static int add_root(struct root_lookup *root_lookup,
  * This can't be called until all the root_info->path fields are filled
  * in by lookup_ino_path
  */
-static int resolve_root(struct root_lookup *rl, struct root_info *ri, int print_parent)
+static int resolve_root(struct root_lookup *rl, struct root_info *ri,
+			u64 *root_id, u64 *parent_id, u64 *top_id, char **path)
 {
-	u64 top_id;
-	u64 parent_id = 0;
 	char *full_path = NULL;
 	int len = 0;
 	struct root_info *found;
@@ -211,6 +210,7 @@ static int resolve_root(struct root_lookup *rl, struct root_info *ri, int print_
 	 * we go backwards from the root_info object and add pathnames
 	 * from parent directories as we go.
 	 */
+	*parent_id = 0;
 	found = ri;
 	while (1) {
 		char *tmp;
@@ -234,13 +234,12 @@ static int resolve_root(struct root_lookup *rl, struct root_info *ri, int print_
 
 		next = found->ref_tree;
 		/* record the first parent */
-		if ( parent_id == 0 ) {
-			parent_id = next;
-		}
+		if (*parent_id == 0)
+			*parent_id = next;
 
 		/* if the ref_tree refers to ourselves, we're at the top */
 		if (next == found->root_id) {
-			top_id = next;
+			*top_id = next;
 			break;
 		}
 
@@ -250,20 +249,15 @@ static int resolve_root(struct root_lookup *rl, struct root_info *ri, int print_
 		 */
 		found = tree_search(&rl->root, next);
 		if (!found) {
-			top_id = next;
+			*top_id = next;
 			break;
 		}
 	}
-	if (print_parent) {
-		printf("ID %llu parent %llu top level %llu path %s\n",
-		       (unsigned long long)ri->root_id, (unsigned long long)parent_id, (unsigned long long)top_id,
-			full_path);
-	} else {
-		printf("ID %llu top level %llu path %s\n",
-		       (unsigned long long)ri->root_id, (unsigned long long)top_id,
-			full_path);
-	}
-	free(full_path);
+
+	*root_id = ri->root_id;
+	*parent_id = ri->root_id;
+	*path = full_path;
+
 	return 0;
 }
 
@@ -560,10 +554,8 @@ build:
 	return full;
 }
 
-int list_subvols(int fd, int print_parent)
+static int __list_subvol_search(int fd, struct root_lookup *root_lookup)
 {
-	struct root_lookup root_lookup;
-	struct rb_node *n;
 	int ret;
 	struct btrfs_ioctl_search_args args;
 	struct btrfs_ioctl_search_key *sk = &args.key;
@@ -574,9 +566,11 @@ int list_subvols(int fd, int print_parent)
 	char *name;
 	u64 dir_id;
 	int i;
-	int e;
 
-	root_lookup_init(&root_lookup);
+	root_lookup_init(root_lookup);
+	memset(&args, 0, sizeof(args));
+
+	root_lookup_init(root_lookup);
 
 	memset(&args, 0, sizeof(args));
 
@@ -603,12 +597,8 @@ int list_subvols(int fd, int print_parent)
 
 	while(1) {
 		ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
-		e = errno;
-		if (ret < 0) {
-			fprintf(stderr, "ERROR: can't perform the search - %s\n",
-				strerror(e));
+		if (ret < 0)
 			return ret;
-		}
 		/* the ioctl returns the number of item it found in nr_items */
 		if (sk->nr_items == 0)
 			break;
@@ -629,7 +619,7 @@ int list_subvols(int fd, int print_parent)
 				name = (char *)(ref + 1);
 				dir_id = btrfs_stack_root_ref_dirid(ref);
 
-				add_root(&root_lookup, sh->objectid, sh->offset,
+				add_root(root_lookup, sh->objectid, sh->offset,
 					 dir_id, name, name_len);
 			}
 
@@ -657,11 +647,15 @@ int list_subvols(int fd, int print_parent)
 		} else
 			break;
 	}
-	/*
-	 * now we have an rbtree full of root_info objects, but we need to fill
-	 * in their path names within the subvol that is referencing each one.
-	 */
-	n = rb_first(&root_lookup.root);
+
+	return 0;
+}
+
+static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup)
+{
+	struct rb_node *n;
+
+	n = rb_first(&root_lookup->root);
 	while (n) {
 		struct root_info *entry;
 		int ret;
@@ -672,6 +666,30 @@ int list_subvols(int fd, int print_parent)
 		n = rb_next(n);
 	}
 
+	return 0;
+}
+
+int list_subvols(int fd, int print_parent)
+{
+	struct root_lookup root_lookup;
+	struct rb_node *n;
+	int ret;
+
+	ret = __list_subvol_search(fd, &root_lookup);
+	if (ret) {
+		fprintf(stderr, "ERROR: can't perform the search - %s\n",
+				strerror(errno));
+		return ret;
+	}
+
+	/*
+	 * now we have an rbtree full of root_info objects, but we need to fill
+	 * in their path names within the subvol that is referencing each one.
+	 */
+	ret = __list_subvol_fill_paths(fd, &root_lookup);
+	if (ret < 0)
+		return ret;
+
 	/* now that we have all the subvol-relative paths filled in,
 	 * we have to string the subvols together so that we can get
 	 * a path all the way back to the FS root
@@ -679,8 +697,24 @@ int list_subvols(int fd, int print_parent)
 	n = rb_last(&root_lookup.root);
 	while (n) {
 		struct root_info *entry;
+		u64 root_id;
+		u64 level;
+		u64 parent_id;
+		char *path;
 		entry = rb_entry(n, struct root_info, rb_node);
-		resolve_root(&root_lookup, entry, print_parent);
+		resolve_root(&root_lookup, entry, &root_id, &parent_id,
+				&level, &path);
+		if (print_parent) {
+			printf("ID %llu parent %llu top level %llu path %s\n",
+				(unsigned long long)root_id,
+				(unsigned long long)parent_id,
+				(unsigned long long)level, path);
+		} else {
+			printf("ID %llu top level %llu path %s\n",
+				(unsigned long long)root_id,
+				(unsigned long long)level, path);
+		}
+		free(path);
 		n = rb_prev(n);
 	}
 
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 2/2] added ioctls and commands to resolve inodes and logical addresses
  2011-07-07 16:01 [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Jan Schmidt
  2011-07-07 16:01 ` [PATCH v1 1/2] btrfs-list: split list_subvols Jan Schmidt
@ 2011-07-07 16:01 ` Jan Schmidt
  2011-07-07 23:19 ` [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Goffredo Baroncelli
  2 siblings, 0 replies; 6+ messages in thread
From: Jan Schmidt @ 2011-07-07 16:01 UTC (permalink / raw)
  To: chris.mason, linux-btrfs

two new commands that make use of the new path resolving functions
implemented for scrub, doing the resolving in-kernel. the result for both
commands is a list of files belonging to that inode / logical address.

Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
---
 btrfs-list.c |   35 ++++++++++++
 btrfs.c      |   10 +++
 btrfs_cmds.c |  177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 btrfs_cmds.h |    3 +
 ioctl.h      |   29 ++++++++++
 5 files changed, 254 insertions(+), 0 deletions(-)

diff --git a/btrfs-list.c b/btrfs-list.c
index dd685c2..cbf6a08 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -900,3 +900,38 @@ int find_updated_files(int fd, u64 root_id, u64 oldest_gen)
 	printf("transid marker was %llu\n", (unsigned long long)max_found);
 	return ret;
 }
+
+char *path_for_root(int fd, u64 root)
+{
+	struct root_lookup root_lookup;
+	struct rb_node *n;
+	char *ret_path = NULL;
+	int ret;
+
+	ret = __list_subvol_search(fd, &root_lookup);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ret = __list_subvol_fill_paths(fd, &root_lookup);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	n = rb_last(&root_lookup.root);
+	while (n) {
+		struct root_info *entry;
+		u64 root_id;
+		u64 parent_id;
+		u64 level;
+		char *path;
+		entry = rb_entry(n, struct root_info, rb_node);
+		resolve_root(&root_lookup, entry, &root_id, &parent_id, &level,
+				&path);
+		if (root_id == root)
+			ret_path = path;
+		else
+			free(path);
+		n = rb_prev(n);
+	}
+
+	return ret_path;
+}
diff --git a/btrfs.c b/btrfs.c
index 67d6f6f..86d356b 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -178,6 +178,16 @@ static struct Command commands[] = {
 		"Remove a device from a filesystem.",
 	  NULL
 	},
+	{ do_ino_to_path, -2,
+	  "resolve inode", "[-v] <inode> <path>\n"
+		"get file system paths for the given inode.",
+	  NULL
+	},
+	{ do_logical_to_ino, -2,
+	  "resolve logical", "[-v] [-P] <logical> <path>\n"
+		"get file system paths for the given logical address.",
+	  NULL
+	},
 	{ 0, 0, 0, 0 }
 };
 
diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 0612f34..2db5d31 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -1545,3 +1545,180 @@ int do_df_filesystem(int nargs, char **argv)
 
 	return 0;
 }
+
+static int __ino_to_path_fd(u64 inum, int fd, int verbose, const char *prepend)
+{
+	int ret;
+	int i;
+	struct btrfs_ioctl_ino_path_args ipa;
+	struct btrfs_data_container *fspath;
+
+	fspath = malloc(4096);
+	if (!fspath)
+		return 1;
+
+	ipa.inum = inum;
+	ipa.size = 4096;
+	ipa.fspath = fspath;
+
+	ret = ioctl(fd, BTRFS_IOC_INO_PATHS, &ipa);
+	if (ret) {
+		printf("ioctl ret=%d, error: %s\n", ret, strerror(errno));
+		goto out;
+	}
+
+	if (verbose)
+		printf("ioctl ret=%d, size=%d, cnt=%d, missed=%d\n", ret,
+			fspath->size, fspath->elem_cnt, fspath->elem_missed);
+
+	for (i = 0; i < fspath->elem_cnt; ++i) {
+		fspath->str[i] += (unsigned long)fspath->str;
+		if (prepend)
+			printf("%s/%s\n", prepend, fspath->str[i]);
+		else
+			printf("%s\n", fspath->str[i]);
+	}
+
+out:
+	free(fspath);
+	return ret;
+}
+
+int do_ino_to_path(int nargs, char **argv)
+{
+	int fd;
+	int verbose = 0;
+
+	optind = 1;
+	while (1) {
+		int c = getopt(nargs, argv, "v");
+		if (c < 0)
+			break;
+		switch (c) {
+		case 'v':
+			verbose = 1;
+			break;
+		default:
+			fprintf(stderr, "invalid arguments for ipath\n");
+			return 1;
+		}
+	}
+	if (nargs - optind != 2) {
+		fprintf(stderr, "invalid arguments for ipath\n");
+		return 1;
+	}
+
+	fd = open_file_or_dir(argv[optind+1]);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind+1]);
+		return 12;
+	}
+
+	return __ino_to_path_fd(atoll(argv[optind]), fd, verbose,
+				argv[optind+1]);
+}
+
+int do_logical_to_ino(int nargs, char **argv)
+{
+	int ret;
+	int fd;
+	int i;
+	int verbose = 0;
+	int getpath = 1;
+	int bytes_left;
+	struct btrfs_ioctl_logical_ino_args loi;
+	struct btrfs_data_container *inodes;
+	char full_path[4096];
+	char *path_ptr;
+
+	optind = 1;
+	while (1) {
+		int c = getopt(nargs, argv, "Pv");
+		if (c < 0)
+			break;
+		switch (c) {
+		case 'P':
+			getpath = 0;
+			break;
+		case 'v':
+			verbose = 1;
+			break;
+		default:
+			fprintf(stderr, "invalid arguments for ipath\n");
+			return 1;
+		}
+	}
+	if (nargs - optind != 2) {
+		fprintf(stderr, "invalid arguments for ipath\n");
+		return 1;
+	}
+
+	inodes = malloc(4096);
+	if (!inodes)
+		return 1;
+
+	loi.logical = atoll(argv[optind]);
+	loi.size = 4096;
+	loi.inodes = inodes;
+
+	fd = open_file_or_dir(argv[optind+1]);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind+1]);
+		ret = 12;
+		goto out;
+	}
+
+	ret = ioctl(fd, BTRFS_IOC_LOGICAL_INO, &loi);
+	if (ret) {
+		printf("ioctl ret=%d, error: %s\n", ret, strerror(errno));
+		goto out;
+	}
+
+	if (verbose)
+		printf("ioctl ret=%d, size=%d, cnt=%d, missed=%d\n", ret,
+			inodes->size, inodes->elem_cnt, inodes->elem_missed);
+
+	bytes_left = sizeof(full_path);
+	ret = snprintf(full_path, bytes_left, "%s/", argv[optind+1]);
+	path_ptr = full_path + ret;
+	bytes_left -= ret + 1;
+	BUG_ON(bytes_left < 0);
+
+	for (i = 0; i < inodes->elem_cnt; i += 3) {
+		u64 inum = inodes->val[i];
+		u64 offset = inodes->val[i+1];
+		u64 root = inodes->val[i+2];
+		int path_fd;
+		char *name;
+
+		if (getpath) {
+			name = path_for_root(fd, root);
+			if (IS_ERR(name))
+				return PTR_ERR(name);
+			if (!name) {
+				path_ptr[-1] = '\0';
+				path_fd = fd;
+			} else {
+				path_ptr[-1] = '/';
+				ret = snprintf(path_ptr, bytes_left, "%s",
+						name);
+				BUG_ON(ret >= bytes_left);
+				free(name);
+				path_fd = open_file_or_dir(full_path);
+				if (path_fd < 0) {
+					fprintf(stderr, "ERROR: can't access "
+						"'%s'\n", full_path);
+					goto out;
+				}
+			}
+			__ino_to_path_fd(inum, path_fd, verbose, full_path);
+		} else {
+			printf("inode %llu offset %llu root %llu\n", inum,
+				offset, root);
+		}
+	}
+
+out:
+	free(inodes);
+	return ret;
+}
diff --git a/btrfs_cmds.h b/btrfs_cmds.h
index 83faa5b..0a3f88f 100644
--- a/btrfs_cmds.h
+++ b/btrfs_cmds.h
@@ -40,3 +40,6 @@ int find_updated_files(int fd, u64 root_id, u64 oldest_gen);
 int do_find_newer(int argc, char **argv);
 int do_change_label(int argc, char **argv);
 int open_file_or_dir(const char *fname);
+int do_ino_to_path(int nargs, char **argv);
+int do_logical_to_ino(int nargs, char **argv);
+char *path_for_root(int fd, u64 root);
diff --git a/ioctl.h b/ioctl.h
index 4accbfd..267b597 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -240,6 +240,30 @@ struct btrfs_ioctl_balance_start {
 					   * bytes for future expansion */
 };
 
+struct btrfs_data_container {
+	__s32	size;		/* out */
+	__u32	elem_cnt;	/* out */
+	__u32	elem_missed;	/* out */
+	union {
+		char	*str[0];	/* out */
+		__u64	val[0];		/* out */
+	};
+};
+
+struct btrfs_ioctl_ino_path_args {
+	__u64				inum;		/* in */
+	__s32				size;		/* in */
+	__u64				reserved[4];
+	struct btrfs_data_container	*fspath;	/* out */
+};
+
+struct btrfs_ioctl_logical_ino_args {
+	__u64				logical;	/* in */
+	__s32				size;		/* in */
+	__u64				reserved[4];
+	struct btrfs_data_container	*inodes;	/* out */
+};
+
 /* BTRFS_IOC_SNAP_CREATE is no longer used by the btrfs command */
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
@@ -294,4 +318,9 @@ struct btrfs_ioctl_balance_start {
 #define BTRFS_IOC_BALANCE_CANCEL _IO(BTRFS_IOCTL_MAGIC, 33)
 #define BTRFS_IOC_BALANCE_FILTERED _IOWR(BTRFS_IOCTL_MAGIC, 34, \
 				struct btrfs_ioctl_balance_start)
+#define BTRFS_IOC_INO_PATHS _IOWR(BTRFS_IOCTL_MAGIC, 35, \
+					struct btrfs_ioctl_ino_path_args)
+#define BTRFS_IOC_LOGICAL_INO _IOWR(BTRFS_IOCTL_MAGIC, 36, \
+					struct btrfs_ioctl_ino_path_args)
+
 #endif
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical"
  2011-07-07 16:01 [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Jan Schmidt
  2011-07-07 16:01 ` [PATCH v1 1/2] btrfs-list: split list_subvols Jan Schmidt
  2011-07-07 16:01 ` [PATCH v1 2/2] added ioctls and commands to resolve inodes and logical addresses Jan Schmidt
@ 2011-07-07 23:19 ` Goffredo Baroncelli
  2011-07-08  8:24   ` Jan Schmidt
  2 siblings, 1 reply; 6+ messages in thread
From: Goffredo Baroncelli @ 2011-07-07 23:19 UTC (permalink / raw)
  To: Jan Schmidt; +Cc: chris.mason, linux-btrfs

On 07/07/2011 06:01 PM, Jan Schmidt wrote:
> The kernel patch series just sent (Subject: "Btrfs: scrub: print path to
> corrupted files and trigger nodatasum fixup") introduces two new ioctls to
> do in-kernel filesystem path construction. This series provides the
> corresponding userspace changes, adding two new commands to the btrfs utility:

Which is the aim of these commands ? It seems more a "debug" utilities
than a standard command. If so, these commands may be put under a new
group called "debug" or "test" or whichever we decided to use. But,
please, highlight the fact that these commands aren't for a general use.

I suggest to use

	btrfs debug resolve ...

Or better

	btrfs inspect resolve ...

> 
> --
> btrfs resolve inode [-v] <inode> <path>
> 	resolves an <inode> to all filesystem paths local to the fs mounted
> 	at <path>.
> 		-v  print count of returned and missed paths
> 
> btrfs resolve logical [-v] [-P] <logical> <path>
> 	resolves a <logical> address to all filesystem paths in the file
> 	system mounted at <path> and all its subvolumes.
> 		-v  print count of returned and missed inode/offset/root
> 		    triples
> 		-P  do not resolve the path but stop after finding all
> 		    inodes at this logical address and print them instead
> --
> 
> These patches are based on Hugo's current integration branch.
> 
> Please try them out and report bugs here. I'll send an update to the manpages
> later.

Please update the man pages at the same time of the code. Develop the
man page coupled with the code may help to design a "good interface"
(from an user point of view) and to explain better the aim of the new
command.


BR
G.Baroncelli


> 
> -Jan
> 
> Jan Schmidt (2):
>   btrfs-list: split list_subvols
>   added ioctls and commands to resolve inodes and logical addresses
> 
>  btrfs-list.c |  139 ++++++++++++++++++++++++++++++++++------------
>  btrfs.c      |   10 +++
>  btrfs_cmds.c |  177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  btrfs_cmds.h |    3 +
>  ioctl.h      |   29 ++++++++++
>  5 files changed, 323 insertions(+), 35 deletions(-)
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical"
  2011-07-07 23:19 ` [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Goffredo Baroncelli
@ 2011-07-08  8:24   ` Jan Schmidt
  2011-07-08 16:42     ` Goffredo Baroncelli
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Schmidt @ 2011-07-08  8:24 UTC (permalink / raw)
  To: kreijack, linux-btrfs; +Cc: Chris Mason

On 08.07.2011 01:19, Goffredo Baroncelli wrote:
> On 07/07/2011 06:01 PM, Jan Schmidt wrote:
>> The kernel patch series just sent (Subject: "Btrfs: scrub: print path to
>> corrupted files and trigger nodatasum fixup") introduces two new ioctls to
>> do in-kernel filesystem path construction. This series provides the
>> corresponding userspace changes, adding two new commands to the btrfs utility:
> 
> Which is the aim of these commands ? It seems more a "debug" utilities
> than a standard command. If so, these commands may be put under a new
> group called "debug" or "test" or whichever we decided to use. But,
> please, highlight the fact that these commands aren't for a general use.

Well, in my opinion, they are. Finding files for an inode is not that
exotic, is it?

The command to find files for logical addresses is useful until each and
every error message in the kernel log does that resolving itself.
Currently, we log logical addresses everywhere. Once that's changed,
this one becomes more like a debug command, I agree.

> I suggest to use
> 
> 	btrfs debug resolve ...
> 
> Or better
> 
> 	btrfs inspect resolve ...

I don't give much in command names (I admit they should make sense), I'm
happy to insert an "inspect" there. Although "btrfs inspect resolve
inode" sounds strange to me. Any other opinions? I suggest we'd better
do the naming process in #btrfs.

>>
>> --
>> btrfs resolve inode [-v] <inode> <path>
>> 	resolves an <inode> to all filesystem paths local to the fs mounted
>> 	at <path>.
>> 		-v  print count of returned and missed paths
>>
>> btrfs resolve logical [-v] [-P] <logical> <path>
>> 	resolves a <logical> address to all filesystem paths in the file
>> 	system mounted at <path> and all its subvolumes.
>> 		-v  print count of returned and missed inode/offset/root
>> 		    triples
>> 		-P  do not resolve the path but stop after finding all
>> 		    inodes at this logical address and print them instead
>> --
>>
>> These patches are based on Hugo's current integration branch.
>>
>> Please try them out and report bugs here. I'll send an update to the manpages
>> later.
> 
> Please update the man pages at the same time of the code. Develop the
> man page coupled with the code may help to design a "good interface"
> (from an user point of view) and to explain better the aim of the new
> command.

Agreed. Next version will come with man pages.

-Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical"
  2011-07-08  8:24   ` Jan Schmidt
@ 2011-07-08 16:42     ` Goffredo Baroncelli
  0 siblings, 0 replies; 6+ messages in thread
From: Goffredo Baroncelli @ 2011-07-08 16:42 UTC (permalink / raw)
  To: Jan Schmidt; +Cc: kreijack, linux-btrfs, Chris Mason

On 07/08/2011 10:24 AM, Jan Schmidt wrote:
> On 08.07.2011 01:19, Goffredo Baroncelli wrote:
>> On 07/07/2011 06:01 PM, Jan Schmidt wrote:
>>> The kernel patch series just sent (Subject: "Btrfs: scrub: print path to
>>> corrupted files and trigger nodatasum fixup") introduces two new ioctls to
>>> do in-kernel filesystem path construction. This series provides the
>>> corresponding userspace changes, adding two new commands to the btrfs utility:
>>
>> Which is the aim of these commands ? It seems more a "debug" utilities
>> than a standard command. If so, these commands may be put under a new
>> group called "debug" or "test" or whichever we decided to use. But,
>> please, highlight the fact that these commands aren't for a general use.
> 
> Well, in my opinion, they are. Finding files for an inode is not that
> exotic, is it?

In my opinion it is a bit exotic. But there would be some interesting
case, like: find all the link which point to an inode.

> 
> The command to find files for logical addresses is useful until each and
> every error message in the kernel log does that resolving itself.
> Currently, we log logical addresses everywhere. Once that's changed,
> this one becomes more like a debug command, I agree.

I am not fully aligned to you, but the difference is so small that
doesn't matter anyway.

> 
>> I suggest to use
>>
>> 	btrfs debug resolve ...
>>
>> Or better
>>
>> 	btrfs inspect resolve ...
> 
> I don't give much in command names (I admit they should make sense), I'm
> happy to insert an "inspect" there. Although "btrfs inspect resolve
> inode" sounds strange to me. Any other opinions? I suggest we'd better
> do the naming process in #btrfs.

For the name my I can suggest:

	btrfs inspect-internal inode-resolve

	btrfs inspect-internal logical-resolve

which may be abbreviate to

	btrfs inspect inode

	btrfs inspect logical

which are (IMHO) a good balancing between shortness and clarity

The command group "inspect-internal" (IMHO) is quite generic and in the
future other commands may be inserted here. And the name is sufficent
"strange" to highlight that these command are not for "faint of heart"'
admins.



> 
>>>
>>> --
>>> btrfs resolve inode [-v] <inode> <path>
>>> 	resolves an <inode> to all filesystem paths local to the fs mounted
>>> 	at <path>.
>>> 		-v  print count of returned and missed paths
>>>
>>> btrfs resolve logical [-v] [-P] <logical> <path>
>>> 	resolves a <logical> address to all filesystem paths in the file
>>> 	system mounted at <path> and all its subvolumes.
>>> 		-v  print count of returned and missed inode/offset/root
>>> 		    triples
>>> 		-P  do not resolve the path but stop after finding all
>>> 		    inodes at this logical address and print them instead
>>> --
>>>
>>> These patches are based on Hugo's current integration branch.
>>>
>>> Please try them out and report bugs here. I'll send an update to the manpages
>>> later.
>>
>> Please update the man pages at the same time of the code. Develop the
>> man page coupled with the code may help to design a "good interface"
>> (from an user point of view) and to explain better the aim of the new
>> command.
> 
> Agreed. Next version will come with man pages.
> 
> -Jan
> --
> 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] 6+ messages in thread

end of thread, other threads:[~2011-07-08 16:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-07 16:01 [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Jan Schmidt
2011-07-07 16:01 ` [PATCH v1 1/2] btrfs-list: split list_subvols Jan Schmidt
2011-07-07 16:01 ` [PATCH v1 2/2] added ioctls and commands to resolve inodes and logical addresses Jan Schmidt
2011-07-07 23:19 ` [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical" Goffredo Baroncelli
2011-07-08  8:24   ` Jan Schmidt
2011-07-08 16:42     ` Goffredo Baroncelli

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.