diff mbox series

btrfs-progs: restore: Have -l display subvolume name

Message ID 547cf7d97e32b542f4a552c7319b167dd6b94403.1603758365.git.dxu@dxuuu.xyz
State New, archived
Headers show
Series btrfs-progs: restore: Have -l display subvolume name | expand

Commit Message

Daniel Xu Oct. 27, 2020, 12:28 a.m. UTC
This commit has `btrfs restore -l ...` display subvolume names if
applicable. Before, it only listed subvolume IDs which are not very
helpful for the user. A subvolume name is much more descriptive.

Before:
	$ btrfs restore ~/scratch/btrfs/fs -l
	 tree key (EXTENT_TREE ROOT_ITEM 0) 30425088 level 0
	 tree key (DEV_TREE ROOT_ITEM 0) 30441472 level 0
	 tree key (FS_TREE ROOT_ITEM 0) 30736384 level 0
	 tree key (CSUM_TREE ROOT_ITEM 0) 30474240 level 0
	 tree key (UUID_TREE ROOT_ITEM 0) 30785536 level 0
	 tree key (256 ROOT_ITEM 0) 30818304 level 0
	 tree key (257 ROOT_ITEM 0) 30883840 level 0
	 tree key (DATA_RELOC_TREE ROOT_ITEM 0) 30490624 level 0

After:
	$ ./btrfs restore ~/scratch/btrfs/fs -l
	 tree key (EXTENT_TREE ROOT_ITEM 0) 30425088 level 0
	 tree key (DEV_TREE ROOT_ITEM 0) 30441472 level 0
	 tree key (FS_TREE ROOT_ITEM 0) 30736384 level 0
	 tree key (CSUM_TREE ROOT_ITEM 0) 30474240 level 0
	 tree key (UUID_TREE ROOT_ITEM 0) 30785536 level 0
	 tree key (256 ROOT_ITEM 0) 30818304 level 0 subvol1
	 tree key (257 ROOT_ITEM 0) 30883840 level 0 subvol2
	 tree key (DATA_RELOC_TREE ROOT_ITEM 0) 30490624 level 0

Link: https://github.com/kdave/btrfs-progs/issues/289
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 cmds/restore.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

Comments

Josef Bacik Oct. 27, 2020, 5:37 p.m. UTC | #1
On 10/26/20 8:28 PM, Daniel Xu wrote:
> This commit has `btrfs restore -l ...` display subvolume names if
> applicable. Before, it only listed subvolume IDs which are not very
> helpful for the user. A subvolume name is much more descriptive.
> 
> Before:
> 	$ btrfs restore ~/scratch/btrfs/fs -l
> 	 tree key (EXTENT_TREE ROOT_ITEM 0) 30425088 level 0
> 	 tree key (DEV_TREE ROOT_ITEM 0) 30441472 level 0
> 	 tree key (FS_TREE ROOT_ITEM 0) 30736384 level 0
> 	 tree key (CSUM_TREE ROOT_ITEM 0) 30474240 level 0
> 	 tree key (UUID_TREE ROOT_ITEM 0) 30785536 level 0
> 	 tree key (256 ROOT_ITEM 0) 30818304 level 0
> 	 tree key (257 ROOT_ITEM 0) 30883840 level 0
> 	 tree key (DATA_RELOC_TREE ROOT_ITEM 0) 30490624 level 0
> 
> After:
> 	$ ./btrfs restore ~/scratch/btrfs/fs -l
> 	 tree key (EXTENT_TREE ROOT_ITEM 0) 30425088 level 0
> 	 tree key (DEV_TREE ROOT_ITEM 0) 30441472 level 0
> 	 tree key (FS_TREE ROOT_ITEM 0) 30736384 level 0
> 	 tree key (CSUM_TREE ROOT_ITEM 0) 30474240 level 0
> 	 tree key (UUID_TREE ROOT_ITEM 0) 30785536 level 0
> 	 tree key (256 ROOT_ITEM 0) 30818304 level 0 subvol1
> 	 tree key (257 ROOT_ITEM 0) 30883840 level 0 subvol2
> 	 tree key (DATA_RELOC_TREE ROOT_ITEM 0) 30490624 level 0
> 

Man I stared at this a few times before I realized the name came after the 
previous information.  I think I'd rather move this helper into somewhere that 
it can be used by the libbtrfs stuff, I'm sure it'll be handy elsewhere.  Thanks,

Josef
Daniel Xu Oct. 28, 2020, 11:34 p.m. UTC | #2
Hi Josef,

On Tue Oct 27, 2020 at 10:37 AM PDT, Josef Bacik wrote:
> On 10/26/20 8:28 PM, Daniel Xu wrote:
> > This commit has `btrfs restore -l ...` display subvolume names if
> > applicable. Before, it only listed subvolume IDs which are not very
> > helpful for the user. A subvolume name is much more descriptive.
> > 
> > Before:
> > 	$ btrfs restore ~/scratch/btrfs/fs -l
> > 	 tree key (EXTENT_TREE ROOT_ITEM 0) 30425088 level 0
> > 	 tree key (DEV_TREE ROOT_ITEM 0) 30441472 level 0
> > 	 tree key (FS_TREE ROOT_ITEM 0) 30736384 level 0
> > 	 tree key (CSUM_TREE ROOT_ITEM 0) 30474240 level 0
> > 	 tree key (UUID_TREE ROOT_ITEM 0) 30785536 level 0
> > 	 tree key (256 ROOT_ITEM 0) 30818304 level 0
> > 	 tree key (257 ROOT_ITEM 0) 30883840 level 0
> > 	 tree key (DATA_RELOC_TREE ROOT_ITEM 0) 30490624 level 0
> > 
> > After:
> > 	$ ./btrfs restore ~/scratch/btrfs/fs -l
> > 	 tree key (EXTENT_TREE ROOT_ITEM 0) 30425088 level 0
> > 	 tree key (DEV_TREE ROOT_ITEM 0) 30441472 level 0
> > 	 tree key (FS_TREE ROOT_ITEM 0) 30736384 level 0
> > 	 tree key (CSUM_TREE ROOT_ITEM 0) 30474240 level 0
> > 	 tree key (UUID_TREE ROOT_ITEM 0) 30785536 level 0
> > 	 tree key (256 ROOT_ITEM 0) 30818304 level 0 subvol1
> > 	 tree key (257 ROOT_ITEM 0) 30883840 level 0 subvol2
> > 	 tree key (DATA_RELOC_TREE ROOT_ITEM 0) 30490624 level 0
> > 
>
> Man I stared at this a few times before I realized the name came after
> the
> previous information. I think I'd rather move this helper into somewhere
> that
> it can be used by the libbtrfs stuff, I'm sure it'll be handy elsewhere.

libbtrfsutil seems structured to work on mounted filesystem images
whereas `btrfs restore` is for unmounted images. So I'm not sure if
libbtrfsutil would want to share that code.

Does it make sense to put the helper in common/util.c instead?

I CC'd Omar in case he has an opinion.

Thanks,
Daniel
diff mbox series

Patch

diff --git a/cmds/restore.c b/cmds/restore.c
index 025e99e9..218c6ec1 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -1197,6 +1197,41 @@  out:
 	return ret;
 }
 
+static char *get_subvol_name(struct btrfs_root *tree_root, u64 subvol_id)
+{
+	struct btrfs_root_ref *ref;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	int namelen;
+	int ret;
+	char *name = NULL;
+
+	key.objectid = BTRFS_FS_TREE_OBJECTID;
+	key.type = BTRFS_ROOT_REF_KEY;
+	key.offset = subvol_id;
+
+	btrfs_init_path(&path);
+	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
+	if (ret != 0)
+		goto out;
+
+	ref = btrfs_item_ptr(path.nodes[0], path.slots[0], struct btrfs_root_ref);
+
+	namelen = btrfs_root_ref_name_len(path.nodes[0], ref);
+	name = malloc(sizeof(char) * namelen + 1);
+	if (!name) {
+		name = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+
+	read_extent_buffer(path.nodes[0], name, (unsigned long)(ref + 1), namelen);
+	name[namelen] = 0;
+
+out:
+	btrfs_release_path(&path);
+	return name;
+}
+
 static int do_list_roots(struct btrfs_root *root)
 {
 	struct btrfs_key key;
@@ -1206,6 +1241,7 @@  static int do_list_roots(struct btrfs_root *root)
 	struct extent_buffer *leaf;
 	struct btrfs_root_item ri;
 	unsigned long offset;
+	char *name;
 	int slot;
 	int ret;
 
@@ -1244,8 +1280,16 @@  static int do_list_roots(struct btrfs_root *root)
 		read_extent_buffer(leaf, &ri, offset, sizeof(ri));
 		printf(" tree ");
 		btrfs_print_key(&disk_key);
-		printf(" %Lu level %d\n", btrfs_root_bytenr(&ri),
+		printf(" %Lu level %d", btrfs_root_bytenr(&ri),
 		       btrfs_root_level(&ri));
+
+		name = get_subvol_name(root, found_key.objectid);
+		if (name) {
+			printf(" %s", name);
+			free(name);
+		}
+
+		printf("\n");
 		path.slots[0]++;
 	}
 	btrfs_release_path(&path);