Re: [PATCH V2] Btrfs-progs: add parent uuid for snapshots
I agree. Thanks for the comments.
New patch has been sent out.
-Anand
On 09/10/12 23:44, David Sterba wrote:
On Fri, Oct 05, 2012 at 10:25:22AM +0800, Anand jain wrote:
@@ -128,6 +129,11 @@ struct {
.need_print = 0,
},
{
+ .name = puuid,
+ .column_name= PUUID,
the capitalized 'P' looks like it's part of the UUID abbreviation. The
UUIDs are long, I think you can print 'parent UUID' in the header, the
name for command line argument 'puuid' is understable.
+ .need_print = 0,
+ },
+ {
.name = uuid,
.column_name= UUID,
.need_print = 0,
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -267,6 +267,7 @@ static const char * const cmd_subvol_list_usage[] = {
-p print parent ID,
-a print all the subvolumes in the filesystem.,
-u print the uuid of subvolumes (and snapshots),
+ -P print the parent uuid of snapshots,
This clashes with my efforts to make the options consistent so that we
can have a lowercase for column selection and uppercase for filter. In
case of the parent UUID, it makes sense to filter by it, eg when we
have a hierarchy of subvolumes that keep the same structure but is
replicated several times.
I suggest to pick a different letter than 'P', say 'q'. (-q is usually
used for 'no verbose output' in utilities, but it does not make much
sense in context of 'subvol list' so I hope it's ok from the UI POV).
-t print the result as a table,
-s list snapshots only in the filesystem,
-r list readonly subvolumes (including snapshots),
Thanks,
david
--
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 V2] Btrfs-progs: add parent uuid for snapshots
On Fri, Oct 05, 2012 at 10:25:22AM +0800, Anand jain wrote:
@@ -128,6 +129,11 @@ struct {
.need_print = 0,
},
{
+ .name = puuid,
+ .column_name= PUUID,
the capitalized 'P' looks like it's part of the UUID abbreviation. The
UUIDs are long, I think you can print 'parent UUID' in the header, the
name for command line argument 'puuid' is understable.
+ .need_print = 0,
+ },
+ {
.name = uuid,
.column_name= UUID,
.need_print = 0,
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -267,6 +267,7 @@ static const char * const cmd_subvol_list_usage[] = {
-p print parent ID,
-a print all the subvolumes in the filesystem.,
-u print the uuid of subvolumes (and snapshots),
+ -P print the parent uuid of snapshots,
This clashes with my efforts to make the options consistent so that we
can have a lowercase for column selection and uppercase for filter. In
case of the parent UUID, it makes sense to filter by it, eg when we
have a hierarchy of subvolumes that keep the same structure but is
replicated several times.
I suggest to pick a different letter than 'P', say 'q'. (-q is usually
used for 'no verbose output' in utilities, but it does not make much
sense in context of 'subvol list' so I hope it's ok from the UI POV).
-t print the result as a table,
-s list snapshots only in the filesystem,
-r list readonly subvolumes (including snapshots),
Thanks,
david
--
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 V2] Btrfs-progs: add parent uuid for snapshots
From: Anand Jain anand.j...@oracle.com
Reviewed-by: Dong Robin robin.k.d...@gmail.com
Signed-off-by: Anand Jain anand.j...@oracle.com
---
btrfs-list.c | 32 +++-
btrfs-list.h |1 +
cmds-subvolume.c |6 +-
3 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/btrfs-list.c b/btrfs-list.c
index b1c9714..2b7d79e 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -80,6 +80,7 @@ struct root_info {
time_t otime;
u8 uuid[BTRFS_UUID_SIZE];
+ u8 puuid[BTRFS_UUID_SIZE];
/* path from the subvol we live in to this root, including the
* root's name. This is null until we do the extra lookup ioctl.
@@ -128,6 +129,11 @@ struct {
.need_print = 0,
},
{
+ .name = puuid,
+ .column_name= PUUID,
+ .need_print = 0,
+ },
+ {
.name = uuid,
.column_name= UUID,
.need_print = 0,
@@ -435,7 +441,7 @@ static struct root_info *root_tree_search(struct
root_lookup *root_tree,
static int update_root(struct root_lookup *root_lookup,
u64 root_id, u64 ref_tree, u64 root_offset, u64 flags,
u64 dir_id, char *name, int name_len, u64 ogen, u64 gen,
- time_t ot, void *uuid)
+ time_t ot, void *uuid, void *puuid)
{
struct root_info *ri;
@@ -472,6 +478,8 @@ static int update_root(struct root_lookup *root_lookup,
ri-otime = ot;
if (uuid)
memcpy(ri-uuid, uuid, BTRFS_UUID_SIZE);
+ if (puuid)
+ memcpy(ri-puuid, puuid, BTRFS_UUID_SIZE);
return 0;
}
@@ -489,17 +497,18 @@ static int update_root(struct root_lookup *root_lookup,
* gen: the current generation of the root
* ot: the original time(create time) of the root
* uuid: uuid of the root
+ * puuid: uuid of the root parent if any
*/
static int add_root(struct root_lookup *root_lookup,
u64 root_id, u64 ref_tree, u64 root_offset, u64 flags,
u64 dir_id, char *name, int name_len, u64 ogen, u64 gen,
- time_t ot, void *uuid)
+ time_t ot, void *uuid, void *puuid)
{
struct root_info *ri;
int ret;
ret = update_root(root_lookup, root_id, ref_tree, root_offset, flags,
- dir_id, name, name_len, ogen, gen, ot, uuid);
+ dir_id, name, name_len, ogen, gen, ot, uuid, puuid);
if (!ret)
return 0;
@@ -540,6 +549,9 @@ static int add_root(struct root_lookup *root_lookup,
if (uuid)
memcpy(ri-uuid, uuid, BTRFS_UUID_SIZE);
+ if (puuid)
+ memcpy(ri-puuid, puuid, BTRFS_UUID_SIZE);
+
ret = root_tree_insert(root_lookup, ri);
if (ret) {
printf(failed to insert tree %llu\n, (unsigned long
long)root_id);
@@ -1022,6 +1034,7 @@ static int __list_subvol_search(int fd, struct
root_lookup *root_lookup)
int i;
time_t t;
u8 uuid[BTRFS_UUID_SIZE];
+ u8 puuid[BTRFS_UUID_SIZE];
root_lookup_init(root_lookup);
memset(args, 0, sizeof(args));
@@ -1075,7 +1088,7 @@ static int __list_subvol_search(int fd, struct
root_lookup *root_lookup)
add_root(root_lookup, sh-objectid, sh-offset,
0, 0, dir_id, name, name_len, 0, 0, 0,
-NULL);
+NULL, NULL);
} else if (sh-type == BTRFS_ROOT_ITEM_KEY) {
ri = (struct btrfs_root_item *)(args.buf + off);
gen = btrfs_root_generation(ri);
@@ -1085,15 +1098,17 @@ static int __list_subvol_search(int fd, struct
root_lookup *root_lookup)
t = ri-otime.sec;
ogen = btrfs_root_otransid(ri);
memcpy(uuid, ri-uuid, BTRFS_UUID_SIZE);
+ memcpy(puuid, ri-parent_uuid,
BTRFS_UUID_SIZE);
} else {
t = 0;
ogen = 0;
memset(uuid, 0, BTRFS_UUID_SIZE);
+ memset(puuid, 0, BTRFS_UUID_SIZE);
}
add_root(root_lookup, sh-objectid, 0,
sh-offset, flags, 0, NULL, 0, ogen,
-gen, t, uuid);
+gen, t, uuid, puuid);
}
off += sh-len;
@@ -1361,6 +1376,13 @@ static void print_subvolume_column(struct
