Liu Bo
2012-Jun-29 10:00 UTC
[PATCH 1/3] Btrfs-progs: add support to set subvolume/snapshot readonly
Setting subvolume/snapshot readonly has been missing for a long time.
With this patch, we can set a subvolume/snapshot readonly via:
o btrfs subvolume set-ro <path>
Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
cmds-subvolume.c | 40 ++++++++++++++++++++++++++++++++++++++++
ioctl.h | 7 +++++++
2 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 950fa8f..5ee31cb 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -512,6 +512,44 @@ static int cmd_find_new(int argc, char **argv)
return 0;
}
+static const char * const cmd_subvol_set_ro_usage[] = {
+ "btrfs subvolume set-ro <path>",
+ "Set the subvolume ro",
+ NULL
+};
+
+static int cmd_subvol_set_ro(int argc, char **argv)
+{
+ int ret=0, fd, e;
+ char *path;
+ struct btrfs_ioctl_get_set_flags_args args;
+
+ if (check_argc_exact(argc, 2))
+ usage(cmd_subvol_set_ro_usage);
+
+ path = argv[1];
+
+ memset(&args, 0, sizeof(args));
+
+ fd = open_file_or_dir(path);
+ if (fd < 0) {
+ fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
+ return 12;
+ }
+
+ args.flags |= BTRFS_SUBVOL_RDONLY;
+ args.objectid = 0;
+ ret = ioctl(fd, BTRFS_IOC_SUBVOL_SETFLAGS, &args);
+ e = errno;
+ close(fd);
+ if( ret < 0 ){
+ fprintf(stderr, "ERROR: unable to set a subvolume RO- %s\n",
+ strerror(e));
+ return 30;
+ }
+ return 0;
+}
+
const struct cmd_group subvolume_cmd_group = {
subvolume_cmd_group_usage, NULL, {
{ "create", cmd_subvol_create, cmd_subvol_create_usage, NULL, 0 },
@@ -522,6 +560,8 @@ const struct cmd_group subvolume_cmd_group = {
cmd_subvol_get_default_usage, NULL, 0 },
{ "set-default", cmd_subvol_set_default,
cmd_subvol_set_default_usage, NULL, 0 },
+ { "set-ro", cmd_subvol_set_ro,
+ cmd_subvol_set_ro_usage, NULL, 0 },
{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
{ 0, 0, 0, 0, 0 }
}
diff --git a/ioctl.h b/ioctl.h
index f2e5d8d..9c066eb 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -42,6 +42,11 @@ struct btrfs_ioctl_vol_args_v2 {
char name[BTRFS_SUBVOL_NAME_MAX + 1];
};
+struct btrfs_ioctl_get_set_flags_args {
+ __u64 objectid;
+ __u64 flags;
+};
+
#define BTRFS_FSID_SIZE 16
#define BTRFS_UUID_SIZE 16
@@ -312,6 +317,8 @@ struct btrfs_ioctl_logical_ino_args {
struct btrfs_ioctl_space_args)
#define BTRFS_IOC_SNAP_CREATE_V2 _IOW(BTRFS_IOCTL_MAGIC, 23, \
struct btrfs_ioctl_vol_args_v2)
+#define BTRFS_IOC_SUBVOL_GETFLAGS _IOW(BTRFS_IOCTL_MAGIC, 25, __u64)
+#define BTRFS_IOC_SUBVOL_SETFLAGS _IOW(BTRFS_IOCTL_MAGIC, 26, __u64)
#define BTRFS_IOC_SCRUB _IOWR(BTRFS_IOCTL_MAGIC, 27, \
struct btrfs_ioctl_scrub_args)
#define BTRFS_IOC_SCRUB_CANCEL _IO(BTRFS_IOCTL_MAGIC, 28)
--
1.6.5.2
--
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
We have both set-default and get-default, but actually our get-default
does not show which one is the default one.
This patch plus the kernel side patch fix this.
Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
Makefile | 6 +++---
btrfs-list.c | 44 +++++++++++++++++++++++++++++++++++++++++---
cmds-subvolume.c | 16 +++-------------
ioctl.h | 1 +
4 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/Makefile b/Makefile
index 79818e6..5d10026 100644
--- a/Makefile
+++ b/Makefile
@@ -4,7 +4,7 @@ CFLAGS = -g -O0
objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
root-tree.o dir-item.o file-item.o inode-item.o \
inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \
- volumes.o utils.o btrfs-list.o btrfslabel.o repair.o
+ volumes.o utils.o btrfs-list.o btrfslabel.o repair.o common.o
cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
cmds-inspect.o cmds-balance.o
@@ -39,8 +39,8 @@ all: version $(progs) manpages
version:
bash version.sh
-btrfs: $(objects) btrfs.o help.o common.o $(cmds_objects)
- $(CC) $(CFLAGS) -o btrfs btrfs.o help.o common.o $(cmds_objects) \
+btrfs: $(objects) btrfs.o help.o $(cmds_objects)
+ $(CC) $(CFLAGS) -o btrfs btrfs.o help.o $(cmds_objects) \
$(objects) $(LDFLAGS) $(LIBS) -lpthread
calc-size: $(objects) calc-size.o
diff --git a/btrfs-list.c b/btrfs-list.c
index 5f4a9be..f1baa52 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -34,6 +34,7 @@
#include "ctree.h"
#include "transaction.h"
#include "utils.h"
+#include "commands.h"
/* we store all the roots we find in an rbtree so that we can
* search for them later.
@@ -668,7 +669,42 @@ static int __list_subvol_fill_paths(int fd, struct
root_lookup *root_lookup)
return 0;
}
-int list_subvols(int fd, int print_parent)
+int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id)
+{
+ int ret = 0, e;
+ struct btrfs_ioctl_get_set_flags_args args;
+
+ memset(&args, 0, sizeof(args));
+
+ if (set && flags)
+ args.flags |= flags;
+ args.objectid = root_id;
+
+ if (set)
+ ret = ioctl(fd, BTRFS_IOC_SUBVOL_SETFLAGS, &args);
+ else
+ ret = ioctl(fd, BTRFS_IOC_SUBVOL_GETFLAGS, &args);
+ e = errno;
+ if( ret < 0 ){
+ fprintf(stderr, "ERROR: unable to %s a subvolume flags- %s\n",
+ (set) ? "set" : "get", strerror(e));
+ return 30;
+ } else if (!set) {
+ u64 flags_to_get = args.flags;
+ if (flags_to_get)
+ printf(" (");
+ if (flags_to_get & BTRFS_SUBVOL_RDONLY)
+ printf("Readonly,");
+ if (flags_to_get & BTRFS_SUBVOL_DEFAULT)
+ printf("Default,");
+ if (flags_to_get)
+ printf(")");
+ printf("\n");
+ }
+ return 0;
+}
+
+int list_subvols(int fd, int print_parent, int get_default)
{
struct root_lookup root_lookup;
struct rb_node *n;
@@ -704,15 +740,17 @@ int list_subvols(int fd, int 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",
+ printf("ID %llu parent %llu top level %llu path %s",
(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",
+ printf("ID %llu top level %llu path %s",
(unsigned long long)root_id,
(unsigned long long)level, path);
}
+ if (subvol_get_set_flags(fd, 0, 0, root_id))
+ printf("\n");
free(path);
n = rb_prev(n);
}
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 5ee31cb..8783e67 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -32,6 +32,7 @@
/* btrfs-list.c */
int list_subvols(int fd, int print_parent, int get_default);
int find_updated_files(int fd, u64 root_id, u64 oldest_gen);
+int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id);
static const char * const subvolume_cmd_group_usage[] = {
"btrfs subvolume <command> <args>",
@@ -520,33 +521,22 @@ static const char * const cmd_subvol_set_ro_usage[] = {
static int cmd_subvol_set_ro(int argc, char **argv)
{
- int ret=0, fd, e;
+ int ret=0, fd;
char *path;
- struct btrfs_ioctl_get_set_flags_args args;
if (check_argc_exact(argc, 2))
usage(cmd_subvol_set_ro_usage);
path = argv[1];
- memset(&args, 0, sizeof(args));
-
fd = open_file_or_dir(path);
if (fd < 0) {
fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
return 12;
}
- args.flags |= BTRFS_SUBVOL_RDONLY;
- args.objectid = 0;
- ret = ioctl(fd, BTRFS_IOC_SUBVOL_SETFLAGS, &args);
- e = errno;
+ ret = subvol_get_set_flags(fd, 1, BTRFS_SUBVOL_RDONLY, 0);
close(fd);
- if( ret < 0 ){
- fprintf(stderr, "ERROR: unable to set a subvolume RO- %s\n",
- strerror(e));
- return 30;
- }
return 0;
}
diff --git a/ioctl.h b/ioctl.h
index 9c066eb..936d075 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -32,6 +32,7 @@ struct btrfs_ioctl_vol_args {
};
#define BTRFS_SUBVOL_RDONLY (1ULL << 1)
+#define BTRFS_SUBVOL_DEFAULT (1ULL << 2)
#define BTRFS_SUBVOL_NAME_MAX 4039
struct btrfs_ioctl_vol_args_v2 {
--
1.6.5.2
--
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
Liu Bo
2012-Jun-29 10:00 UTC
[PATCH 3/3] Btrfs-progs: add ''s'' option for ''btrfs subvolume list''
We want ''btrfs subvolume list'' to act as
''ls'', which means that
we can not only list out all the subvolumes we have, but also list
each single one.
So this patch add ''s'' option to show a single one:
$ ./btrfs sub list /mnt/btrfs/
ID 256 top level 5 path subvol (Readonly,)
ID 257 top level 5 path snapshot
ID 258 top level 5 path subvol2
ID 259 top level 5 path subvol2/subvol3
$ ./btrfs sub list -s /mnt/btrfs/subvol
ID 256 top level 5 path subvol (Readonly,)
Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
btrfs-list.c | 41 ++++++++++++++++++++++++++++++++++++++++-
cmds-subvolume.c | 15 ++++++++++-----
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/btrfs-list.c b/btrfs-list.c
index f1baa52..3e79239 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -312,6 +312,30 @@ static int lookup_ino_path(int fd, struct root_info *ri)
return 0;
}
+/*
+ * helper function for getting the root which the file is belonged to.
+ */
+static int lookup_ino_rootid(int fd, u64 *rootid)
+{
+ struct btrfs_ioctl_ino_lookup_args args;
+ int ret, e;
+
+ memset(&args, 0, sizeof(args));
+ args.treeid = 0;
+ args.objectid = BTRFS_FIRST_FREE_OBJECTID;
+
+ ret = ioctl(fd, BTRFS_IOC_INO_LOOKUP, &args);
+ e = errno;
+ if (ret) {
+ fprintf(stderr, "ERROR: Failed to lookup root id - %s\n",
+ strerror(e));
+ return ret;
+ }
+
+ *rootid = args.treeid;
+ return 0;
+}
+
/* finding the generation for a given path is a two step process.
* First we use the inode loookup routine to find out the root id
*
@@ -704,11 +728,12 @@ int subvol_get_set_flags(int fd, int set, u64 flags, u64
root_id)
return 0;
}
-int list_subvols(int fd, int print_parent, int get_default)
+int list_subvols(int fd, int print_parent, int print_self, int get_default)
{
struct root_lookup root_lookup;
struct rb_node *n;
int ret;
+ u64 subvolid = 0;
ret = __list_subvol_search(fd, &root_lookup);
if (ret) {
@@ -725,6 +750,9 @@ int list_subvols(int fd, int print_parent, int get_default)
if (ret < 0)
return ret;
+ if (print_self)
+ lookup_ino_rootid(fd, &subvolid);
+
/* 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
@@ -739,6 +767,14 @@ int list_subvols(int fd, int print_parent, int get_default)
entry = rb_entry(n, struct root_info, rb_node);
resolve_root(&root_lookup, entry, &root_id, &parent_id,
&level, &path);
+
+ /* print this subvolume only */
+ if (print_self && subvolid != root_id) {
+ free(path);
+ n = rb_prev(n);
+ continue;
+ }
+
if (print_parent) {
printf("ID %llu parent %llu top level %llu path %s",
(unsigned long long)root_id,
@@ -753,6 +789,9 @@ int list_subvols(int fd, int print_parent, int get_default)
printf("\n");
free(path);
n = rb_prev(n);
+
+ if (print_self)
+ break;
}
return ret;
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 8783e67..f779b78 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -30,7 +30,7 @@
#include "commands.h"
/* btrfs-list.c */
-int list_subvols(int fd, int print_parent, int get_default);
+int list_subvols(int fd, int print_parent, int print_self, int get_default);
int find_updated_files(int fd, u64 root_id, u64 oldest_gen);
int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id);
@@ -218,10 +218,11 @@ static int cmd_subvol_delete(int argc, char **argv)
}
static const char * const cmd_subvol_list_usage[] = {
- "btrfs subvolume list [-p] <path>",
+ "btrfs subvolume list [-ps] <path>",
"List subvolumes (and snapshots)",
"",
"-p print parent ID",
+ "-s print this subvolume only",
NULL
};
@@ -230,11 +231,12 @@ static int cmd_subvol_list(int argc, char **argv)
int fd;
int ret;
int print_parent = 0;
+ int print_self = 0;
char *subvol;
optind = 1;
while(1) {
- int c = getopt(argc, argv, "p");
+ int c = getopt(argc, argv, "ps");
if (c < 0)
break;
@@ -242,6 +244,9 @@ static int cmd_subvol_list(int argc, char **argv)
case ''p'':
print_parent = 1;
break;
+ case ''s'':
+ print_self = 1;
+ break;
default:
usage(cmd_subvol_list_usage);
}
@@ -267,7 +272,7 @@ static int cmd_subvol_list(int argc, char **argv)
fprintf(stderr, "ERROR: can''t access
''%s''\n", subvol);
return 12;
}
- ret = list_subvols(fd, print_parent, 0);
+ ret = list_subvols(fd, print_parent, print_self, 0);
if (ret)
return 19;
return 0;
@@ -426,7 +431,7 @@ static int cmd_subvol_get_default(int argc, char **argv)
fprintf(stderr, "ERROR: can''t access
''%s''\n", subvol);
return 12;
}
- ret = list_subvols(fd, 0, 1);
+ ret = list_subvols(fd, 0, 0, 1);
if (ret)
return 19;
return 0;
--
1.6.5.2
--
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
Ilya Dryomov
2012-Jun-29 10:21 UTC
Re: [PATCH 1/3] Btrfs-progs: add support to set subvolume/snapshot readonly
On Fri, Jun 29, 2012 at 06:00:36PM +0800, Liu Bo wrote:> Setting subvolume/snapshot readonly has been missing for a long time. > > With this patch, we can set a subvolume/snapshot readonly via: > > o btrfs subvolume set-ro <path>Alexander''s ''btrfs property'' patches do exactly this, but in a much more generic and extensible way. ''btrfs property'' subgroup provides a uniform interface for getting and setting properties of filesystem objects in general, not only those of subvolumes and snapshots. It provides a much better user interface, and it also allows us to easily rethink kernel-user interface for generic get/set in future. Thanks, Ilya -- 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
Ilya Dryomov
2012-Jun-29 10:36 UTC
Re: [PATCH 2/3] Btrfs-progs: make get default report correctly
On Fri, Jun 29, 2012 at 06:00:37PM +0800, Liu Bo wrote:> We have both set-default and get-default, but actually our get-default > does not show which one is the default one. > > This patch plus the kernel side patch fix this.Are you referring to the fact that get-default in Chris'' btrfs-progs master lists all subvolumes instead of showing the default one? If so, I fixed that problem a while ago, patch is in Hugo''s integration branch. Thanks, Ilya -- 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
Liu Bo
2012-Jul-02 01:39 UTC
Re: [PATCH 2/3] Btrfs-progs: make get default report correctly
On 06/29/2012 06:36 PM, Ilya Dryomov wrote:> On Fri, Jun 29, 2012 at 06:00:37PM +0800, Liu Bo wrote: >> We have both set-default and get-default, but actually our get-default >> does not show which one is the default one. >> >> This patch plus the kernel side patch fix this. > > Are you referring to the fact that get-default in Chris'' btrfs-progs > master lists all subvolumes instead of showing the default one? If so, > I fixed that problem a while ago, patch is in Hugo''s integration branch. >So can you please show me a link about your patch? thanks, liubo> Thanks, > > Ilya > -- > 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 >-- 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
Liu Bo
2012-Jul-02 02:07 UTC
Re: [PATCH 1/3] Btrfs-progs: add support to set subvolume/snapshot readonly
On 06/29/2012 06:21 PM, Ilya Dryomov wrote:> On Fri, Jun 29, 2012 at 06:00:36PM +0800, Liu Bo wrote: >> Setting subvolume/snapshot readonly has been missing for a long time. >> >> With this patch, we can set a subvolume/snapshot readonly via: >> >> o btrfs subvolume set-ro <path> > > Alexander''s ''btrfs property'' patches do exactly this, but in a much more > generic and extensible way. ''btrfs property'' subgroup provides a > uniform interface for getting and setting properties of filesystem > objects in general, not only those of subvolumes and snapshots. It > provides a much better user interface, and it also allows us to easily > rethink kernel-user interface for generic get/set in future. >Thanks for the explanation! But I prefer keeping the current categories {subvolume,filesystem,device,...}: o Compatibility, we cannot remove the old commands until we make sure that no users will use them. o We''ve three properties {default, readonly, lable}, is it worthy making another new interface? o Current categories are clear and clean. thanks, liubo> Thanks, > > Ilya > -- > 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 >-- 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
Ilya Dryomov
2012-Jul-02 09:45 UTC
Re: [PATCH 2/3] Btrfs-progs: make get default report correctly
On Mon, Jul 02, 2012 at 09:39:45AM +0800, Liu Bo wrote:> On 06/29/2012 06:36 PM, Ilya Dryomov wrote: > > > On Fri, Jun 29, 2012 at 06:00:37PM +0800, Liu Bo wrote: > >> We have both set-default and get-default, but actually our get-default > >> does not show which one is the default one. > >> > >> This patch plus the kernel side patch fix this. > > > > Are you referring to the fact that get-default in Chris'' btrfs-progs > > master lists all subvolumes instead of showing the default one? If so, > > I fixed that problem a while ago, patch is in Hugo''s integration branch. > > > > > So can you please show me a link about your patch?I''m sorry, it turns out Hugo hasn''t pushed it yet. Here is a link: http://thread.gmane.org/gmane.comp.file-systems.btrfs/16187/ Thanks, Ilya -- 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
Ilya Dryomov
2012-Jul-02 10:00 UTC
Re: [PATCH 1/3] Btrfs-progs: add support to set subvolume/snapshot readonly
On Mon, Jul 02, 2012 at 10:07:42AM +0800, Liu Bo wrote:> On 06/29/2012 06:21 PM, Ilya Dryomov wrote: > > > On Fri, Jun 29, 2012 at 06:00:36PM +0800, Liu Bo wrote: > >> Setting subvolume/snapshot readonly has been missing for a long time. > >> > >> With this patch, we can set a subvolume/snapshot readonly via: > >> > >> o btrfs subvolume set-ro <path> > > > > Alexander''s ''btrfs property'' patches do exactly this, but in a much more > > generic and extensible way. ''btrfs property'' subgroup provides a > > uniform interface for getting and setting properties of filesystem > > objects in general, not only those of subvolumes and snapshots. It > > provides a much better user interface, and it also allows us to easily > > rethink kernel-user interface for generic get/set in future. > > > > > Thanks for the explanation! > > But I prefer keeping the current categories {subvolume,filesystem,device,...}: > > o Compatibility, we cannot remove the old commands until we make sure that no users will > use them.We are not going to remove old commands any time soon. However, adding new ones that clearly fall into get/set category, is not a good idea. Especially when there is a generic interface on its way.> > o We''ve three properties {default, readonly, lable}, is it worthy making another new interface?It''s not just about subvolumes. There will be a lot more properties on the table as filesystem matures, for example device speeds, subvolume profiles, quotas.> > o Current categories are clear and clean.Once again, it''s not just about subvolumes. Current categories are indeed clear, but adding two commands for each non-trivial property that comes up in future does not seem practical to me. Thanks, Ilya -- 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
Liu Bo
2012-Jul-03 01:27 UTC
Re: [PATCH 2/3] Btrfs-progs: make get default report correctly
On 07/02/2012 05:45 PM, Ilya Dryomov wrote:> On Mon, Jul 02, 2012 at 09:39:45AM +0800, Liu Bo wrote: >> On 06/29/2012 06:36 PM, Ilya Dryomov wrote: >> >>> On Fri, Jun 29, 2012 at 06:00:37PM +0800, Liu Bo wrote: >>>> We have both set-default and get-default, but actually our get-default >>>> does not show which one is the default one. >>>> >>>> This patch plus the kernel side patch fix this. >>> Are you referring to the fact that get-default in Chris'' btrfs-progs >>> master lists all subvolumes instead of showing the default one? If so, >>> I fixed that problem a while ago, patch is in Hugo''s integration branch. >>> >> >> So can you please show me a link about your patch? > > I''m sorry, it turns out Hugo hasn''t pushed it yet. Here is a link: > http://thread.gmane.org/gmane.comp.file-systems.btrfs/16187/ >Fairly good patches. thanks, liubo> Thanks, > > Ilya > -- > 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 >-- 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
Liu Bo
2012-Jul-03 01:46 UTC
Re: [PATCH 1/3] Btrfs-progs: add support to set subvolume/snapshot readonly
On 07/02/2012 06:00 PM, Ilya Dryomov wrote:> On Mon, Jul 02, 2012 at 10:07:42AM +0800, Liu Bo wrote: >> On 06/29/2012 06:21 PM, Ilya Dryomov wrote: >> >>> On Fri, Jun 29, 2012 at 06:00:36PM +0800, Liu Bo wrote: >>>> Setting subvolume/snapshot readonly has been missing for a long time. >>>> >>>> With this patch, we can set a subvolume/snapshot readonly via: >>>> >>>> o btrfs subvolume set-ro <path> >>> Alexander''s ''btrfs property'' patches do exactly this, but in a much more >>> generic and extensible way. ''btrfs property'' subgroup provides a >>> uniform interface for getting and setting properties of filesystem >>> objects in general, not only those of subvolumes and snapshots. It >>> provides a much better user interface, and it also allows us to easily >>> rethink kernel-user interface for generic get/set in future. >>> >> >> Thanks for the explanation! >> >> But I prefer keeping the current categories {subvolume,filesystem,device,...}: >> >> o Compatibility, we cannot remove the old commands until we make sure that no users will >> use them. > > We are not going to remove old commands any time soon. However, adding > new ones that clearly fall into get/set category, is not a good idea. > Especially when there is a generic interface on its way. > >> o We''ve three properties {default, readonly, lable}, is it worthy making another new interface? > > It''s not just about subvolumes. There will be a lot more properties on > the table as filesystem matures, for example device speeds, subvolume > profiles, quotas. > >> o Current categories are clear and clean. > > Once again, it''s not just about subvolumes. Current categories are > indeed clear, but adding two commands for each non-trivial property that > comes up in future does not seem practical to me. >I see, that''s reasonable. :) Besides set/get-ro and get-default, I also want to have ''btrfs subvolume list'' work as ''ls'', that is, it can list not only all of items, but also a single item. And I have made a patch for that (it refers to [PATCH 3/3] Btrfs-progs: add ''s'' option for ''btrfs subvolume list''). What''s your opinion about it? thanks, liubo> Thanks, > > Ilya > -- > 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 >-- 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
David Sterba
2012-Jul-04 15:41 UTC
Re: [PATCH 3/3] Btrfs-progs: add ''s'' option for ''btrfs subvolume list''
On Fri, Jun 29, 2012 at 06:00:38PM +0800, Liu Bo wrote:> We want ''btrfs subvolume list'' to act as ''ls'', which means that > we can not only list out all the subvolumes we have, but also list > each single one. > > So this patch add ''s'' option to show a single one: > > $ ./btrfs sub list /mnt/btrfs/ > ID 256 top level 5 path subvol (Readonly,) > ID 257 top level 5 path snapshot > ID 258 top level 5 path subvol2 > ID 259 top level 5 path subvol2/subvol3 > > $ ./btrfs sub list -s /mnt/btrfs/subvol > ID 256 top level 5 path subvol (Readonly,)suggestions and comments: 1) show the subvolume flags only if an option is given, similar to -p (to show parent subvol), 2) move the flags before the subvolume path -- it is of a variable length and it''s a bit easier (but not reliable) to parse it from scripts sidenote: ''find /mnt -inum 256 -print0'' will list all subvolumes in a way that''s resitent to funny characters in the path, but is slow as it has to traverse the filesystem (and ''find'' does not support a true breadth-first-search, needs to be iterated with -mindepth/maxdepth). the ''subvol list'' command could mimic the -print0 and print the subvolume paths terminated by ''\0''. 3) drop the , if there''s only one subvol property 4) the ''-s'' option on the mountpoint does not show anything, though I would expect that, eg when the mountpoint is a subvolume -- one comment to code below david> > Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> > --- > btrfs-list.c | 41 ++++++++++++++++++++++++++++++++++++++++- > cmds-subvolume.c | 15 ++++++++++----- > 2 files changed, 50 insertions(+), 6 deletions(-) > > diff --git a/btrfs-list.c b/btrfs-list.c > index f1baa52..3e79239 100644 > --- a/btrfs-list.c > +++ b/btrfs-list.c > @@ -312,6 +312,30 @@ static int lookup_ino_path(int fd, struct root_info *ri) > return 0; > } > > +/* > + * helper function for getting the root which the file is belonged to. > + */ > +static int lookup_ino_rootid(int fd, u64 *rootid) > +{ > + struct btrfs_ioctl_ino_lookup_args args; > + int ret, e; > + > + memset(&args, 0, sizeof(args)); > + args.treeid = 0; > + args.objectid = BTRFS_FIRST_FREE_OBJECTID; > + > + ret = ioctl(fd, BTRFS_IOC_INO_LOOKUP, &args); > + e = errno; > + if (ret) { > + fprintf(stderr, "ERROR: Failed to lookup root id - %s\n", > + strerror(e)); > + return ret; > + } > + > + *rootid = args.treeid; > + return 0; > +} > + > /* finding the generation for a given path is a two step process. > * First we use the inode loookup routine to find out the root id > * > @@ -704,11 +728,12 @@ int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id) > return 0; > } > > -int list_subvols(int fd, int print_parent, int get_default) > +int list_subvols(int fd, int print_parent, int print_self, int get_default) > { > struct root_lookup root_lookup; > struct rb_node *n; > int ret; > + u64 subvolid = 0; > > ret = __list_subvol_search(fd, &root_lookup); > if (ret) { > @@ -725,6 +750,9 @@ int list_subvols(int fd, int print_parent, int get_default) > if (ret < 0) > return ret; > > + if (print_self) > + lookup_ino_rootid(fd, &subvolid);you should probably check the return value> + > /* 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 > @@ -739,6 +767,14 @@ int list_subvols(int fd, int print_parent, int get_default) > entry = rb_entry(n, struct root_info, rb_node); > resolve_root(&root_lookup, entry, &root_id, &parent_id, > &level, &path); > + > + /* print this subvolume only */ > + if (print_self && subvolid != root_id) { > + free(path); > + n = rb_prev(n); > + continue; > + } > + > if (print_parent) { > printf("ID %llu parent %llu top level %llu path %s", > (unsigned long long)root_id, > @@ -753,6 +789,9 @@ int list_subvols(int fd, int print_parent, int get_default) > printf("\n"); > free(path); > n = rb_prev(n); > + > + if (print_self) > + break; > } > > return ret; > diff --git a/cmds-subvolume.c b/cmds-subvolume.c > index 8783e67..f779b78 100644 > --- a/cmds-subvolume.c > +++ b/cmds-subvolume.c > @@ -30,7 +30,7 @@ > #include "commands.h" > > /* btrfs-list.c */ > -int list_subvols(int fd, int print_parent, int get_default); > +int list_subvols(int fd, int print_parent, int print_self, int get_default); > int find_updated_files(int fd, u64 root_id, u64 oldest_gen); > int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id); > > @@ -218,10 +218,11 @@ static int cmd_subvol_delete(int argc, char **argv) > } > > static const char * const cmd_subvol_list_usage[] = { > - "btrfs subvolume list [-p] <path>", > + "btrfs subvolume list [-ps] <path>", > "List subvolumes (and snapshots)", > "", > "-p print parent ID", > + "-s print this subvolume only", > NULL > }; > > @@ -230,11 +231,12 @@ static int cmd_subvol_list(int argc, char **argv) > int fd; > int ret; > int print_parent = 0; > + int print_self = 0; > char *subvol; > > optind = 1; > while(1) { > - int c = getopt(argc, argv, "p"); > + int c = getopt(argc, argv, "ps"); > if (c < 0) > break; > > @@ -242,6 +244,9 @@ static int cmd_subvol_list(int argc, char **argv) > case ''p'': > print_parent = 1; > break; > + case ''s'': > + print_self = 1; > + break; > default: > usage(cmd_subvol_list_usage); > } > @@ -267,7 +272,7 @@ static int cmd_subvol_list(int argc, char **argv) > fprintf(stderr, "ERROR: can''t access ''%s''\n", subvol); > return 12; > } > - ret = list_subvols(fd, print_parent, 0); > + ret = list_subvols(fd, print_parent, print_self, 0); > if (ret) > return 19; > return 0; > @@ -426,7 +431,7 @@ static int cmd_subvol_get_default(int argc, char **argv) > fprintf(stderr, "ERROR: can''t access ''%s''\n", subvol); > return 12; > } > - ret = list_subvols(fd, 0, 1); > + ret = list_subvols(fd, 0, 0, 1); > if (ret) > return 19; > return 0; > -- > 1.6.5.2 > > -- > 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-- 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
Liu Bo
2012-Jul-05 12:42 UTC
Re: [PATCH 3/3] Btrfs-progs: add ''s'' option for ''btrfs subvolume list''
On 07/04/2012 11:41 PM, David Sterba wrote:> On Fri, Jun 29, 2012 at 06:00:38PM +0800, Liu Bo wrote: >> We want ''btrfs subvolume list'' to act as ''ls'', which means that >> we can not only list out all the subvolumes we have, but also list >> each single one. >> >> So this patch add ''s'' option to show a single one: >> >> $ ./btrfs sub list /mnt/btrfs/ >> ID 256 top level 5 path subvol (Readonly,) >> ID 257 top level 5 path snapshot >> ID 258 top level 5 path subvol2 >> ID 259 top level 5 path subvol2/subvol3 >> >> $ ./btrfs sub list -s /mnt/btrfs/subvol >> ID 256 top level 5 path subvol (Readonly,) > > suggestions and comments: > > 1) show the subvolume flags only if an option is given, similar to -p > (to show parent subvol), > > 2) move the flags before the subvolume path -- it is of a > variable length and it''s a bit easier (but not reliable) to parse it > from scripts > > sidenote: ''find /mnt -inum 256 -print0'' will list all subvolumes in a > way that''s resitent to funny characters in the path, but is slow as it > has to traverse the filesystem (and ''find'' does not support a true > breadth-first-search, needs to be iterated with -mindepth/maxdepth). > > the ''subvol list'' command could mimic the -print0 and print the > subvolume paths terminated by ''\0''. > > 3) drop the , if there''s only one subvol property > > 4) the ''-s'' option on the mountpoint does not show anything, though I > would expect that, eg when the mountpoint is a subvolume > > -- > > one comment to code below >Hi David, Thanks a lot for reviewing this! Really nice advice. This patch is not compatible with Alex''s ''btrfs property'' patchset, and after some discussion with Alex and Ilya, I''ve agreed to rebase this patch onto their patchset, but I''ll definitely apply all of your comments. :) thanks, liubo> david > >> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> >> --- >> btrfs-list.c | 41 ++++++++++++++++++++++++++++++++++++++++- >> cmds-subvolume.c | 15 ++++++++++----- >> 2 files changed, 50 insertions(+), 6 deletions(-) >> >> diff --git a/btrfs-list.c b/btrfs-list.c >> index f1baa52..3e79239 100644 >> --- a/btrfs-list.c >> +++ b/btrfs-list.c >> @@ -312,6 +312,30 @@ static int lookup_ino_path(int fd, struct root_info *ri) >> return 0; >> } >> >> +/* >> + * helper function for getting the root which the file is belonged to. >> + */ >> +static int lookup_ino_rootid(int fd, u64 *rootid) >> +{ >> + struct btrfs_ioctl_ino_lookup_args args; >> + int ret, e; >> + >> + memset(&args, 0, sizeof(args)); >> + args.treeid = 0; >> + args.objectid = BTRFS_FIRST_FREE_OBJECTID; >> + >> + ret = ioctl(fd, BTRFS_IOC_INO_LOOKUP, &args); >> + e = errno; >> + if (ret) { >> + fprintf(stderr, "ERROR: Failed to lookup root id - %s\n", >> + strerror(e)); >> + return ret; >> + } >> + >> + *rootid = args.treeid; >> + return 0; >> +} >> + >> /* finding the generation for a given path is a two step process. >> * First we use the inode loookup routine to find out the root id >> * >> @@ -704,11 +728,12 @@ int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id) >> return 0; >> } >> >> -int list_subvols(int fd, int print_parent, int get_default) >> +int list_subvols(int fd, int print_parent, int print_self, int get_default) >> { >> struct root_lookup root_lookup; >> struct rb_node *n; >> int ret; >> + u64 subvolid = 0; >> >> ret = __list_subvol_search(fd, &root_lookup); >> if (ret) { >> @@ -725,6 +750,9 @@ int list_subvols(int fd, int print_parent, int get_default) >> if (ret < 0) >> return ret; >> >> + if (print_self) >> + lookup_ino_rootid(fd, &subvolid); > > you should probably check the return value > >> + >> /* 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 >> @@ -739,6 +767,14 @@ int list_subvols(int fd, int print_parent, int get_default) >> entry = rb_entry(n, struct root_info, rb_node); >> resolve_root(&root_lookup, entry, &root_id, &parent_id, >> &level, &path); >> + >> + /* print this subvolume only */ >> + if (print_self && subvolid != root_id) { >> + free(path); >> + n = rb_prev(n); >> + continue; >> + } >> + >> if (print_parent) { >> printf("ID %llu parent %llu top level %llu path %s", >> (unsigned long long)root_id, >> @@ -753,6 +789,9 @@ int list_subvols(int fd, int print_parent, int get_default) >> printf("\n"); >> free(path); >> n = rb_prev(n); >> + >> + if (print_self) >> + break; >> } >> >> return ret; >> diff --git a/cmds-subvolume.c b/cmds-subvolume.c >> index 8783e67..f779b78 100644 >> --- a/cmds-subvolume.c >> +++ b/cmds-subvolume.c >> @@ -30,7 +30,7 @@ >> #include "commands.h" >> >> /* btrfs-list.c */ >> -int list_subvols(int fd, int print_parent, int get_default); >> +int list_subvols(int fd, int print_parent, int print_self, int get_default); >> int find_updated_files(int fd, u64 root_id, u64 oldest_gen); >> int subvol_get_set_flags(int fd, int set, u64 flags, u64 root_id); >> >> @@ -218,10 +218,11 @@ static int cmd_subvol_delete(int argc, char **argv) >> } >> >> static const char * const cmd_subvol_list_usage[] = { >> - "btrfs subvolume list [-p] <path>", >> + "btrfs subvolume list [-ps] <path>", >> "List subvolumes (and snapshots)", >> "", >> "-p print parent ID", >> + "-s print this subvolume only", >> NULL >> }; >> >> @@ -230,11 +231,12 @@ static int cmd_subvol_list(int argc, char **argv) >> int fd; >> int ret; >> int print_parent = 0; >> + int print_self = 0; >> char *subvol; >> >> optind = 1; >> while(1) { >> - int c = getopt(argc, argv, "p"); >> + int c = getopt(argc, argv, "ps"); >> if (c < 0) >> break; >> >> @@ -242,6 +244,9 @@ static int cmd_subvol_list(int argc, char **argv) >> case ''p'': >> print_parent = 1; >> break; >> + case ''s'': >> + print_self = 1; >> + break; >> default: >> usage(cmd_subvol_list_usage); >> } >> @@ -267,7 +272,7 @@ static int cmd_subvol_list(int argc, char **argv) >> fprintf(stderr, "ERROR: can''t access ''%s''\n", subvol); >> return 12; >> } >> - ret = list_subvols(fd, print_parent, 0); >> + ret = list_subvols(fd, print_parent, print_self, 0); >> if (ret) >> return 19; >> return 0; >> @@ -426,7 +431,7 @@ static int cmd_subvol_get_default(int argc, char **argv) >> fprintf(stderr, "ERROR: can''t access ''%s''\n", subvol); >> return 12; >> } >> - ret = list_subvols(fd, 0, 1); >> + ret = list_subvols(fd, 0, 0, 1); >> if (ret) >> return 19; >> return 0; >> -- >> 1.6.5.2 >> >> -- >> 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 > -- > 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 >-- 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
Possibly Parallel Threads
- [PATCH] add crtime to the snapshot list
- [PATCH v1 0/2] Btrfs-progs: commands "resolve inode" and "resolve logical"
- [RFC] [PATCH] Add btrfs autosnap feature
- [PATCH] BTRFS-PROG: recursively subvolume snapshot and delete
- [RFC] Improve btrfs subvolume find-new command