Re: [PATCH V2] Btrfs-progs: add \"btrfs subvolume get-default\" subcommand
On Wed, Feb 15, 2012 at 01:43:02PM +0800, Anand Jain wrote:
>
> Hi Xin / Hugo,
> I am referring to
> git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git
>
> has miss match parameter list for list_subvols
> --
> # cd btrfs-progs
> # egrep list_subvols *
> btrfs_cmds.c:ret = list_subvols(fd, print_parent, 0);
> btrfs_cmds.c:ret = list_subvols(fd, 0, 1);
> btrfs_cmds.h:int list_subvols(int fd, int print_parent, int get_default);
> <-
> btrfs-list.c:int list_subvols(int fd, int print_parent) <-
> --
>
> --
> # egrep -A 3 list_subvols btrfs-list.c
> int list_subvols(int fd, int print_parent)
> {
> struct root_lookup root_lookup;
> struct rb_node *n;
> --
> Looks like the following patch was checked in partially.
>http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg11103.html
>
> Could you pls take a look / comment.
I looked at this issue briefly when testing the new parser. Looks like
Jan broke it with his split of list_subvols. Jan's code was probably
based on the master branch of the old progs repo, whereas get-default
was based on one of the integration branches and the subsequent merge
missed it. I'll fix it if nobody else steps up.
Thanks,
Ilya
--
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 "btrfs subvolume get-default" subcommand
> -Original Message-
> From: Goffredo Baroncelli
> [mailto:kreij...@libero.it]
> Sent: Monday, July 11, 2011 8:03 PM
> To: Zhong, Xin
> Cc: linux-btrfs@vger.kernel.org
> Subject: R: [PATCH V2] Btrfs-progs: add "btrfs subvolume get-default"
> subcommand
>
> >Messaggio originale
> >Da: xin.zh...@intel.com
> >Data: 11/07/2011 10.56
> >A:
> >Cc:
> >Ogg: [PATCH V2] Btrfs-progs: add "btrfs subvolume get-
> default"
> subcommand
> >
> >Add subcommand to get the default subvolume of btrfs filesystem
> >
> >Reported-by: Yang, Yi
> >Signed-off-by: Zhong, Xin
> >---
> > btrfs-list.c | 57
> +++--
> > btrfs.c |3 +++
> > btrfs_cmds.c | 31 ++-
> > btrfs_cmds.h |3 ++-
> > 4 files changed, 90 insertions(+), 4 deletions(-)
>
> please update the man page too.
>
> >
> >diff --git a/btrfs-list.c b/btrfs-list.c
> >index 93766a8..aa6a9b4 100644
> >--- a/btrfs-list.c
> >+++ b/btrfs-list.c
> >@@ -536,7 +536,7 @@ build:
> > return full;
> [...]
> >+/* search dir item */
> >+sk->max_type = BTRFS_DIR_ITEM_KEY;
> >+sk->min_type = BTRFS_DIR_ITEM_KEY;
> >+
> >+sk->max_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
> >+sk->min_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
> >+sk->max_offset = (u64)-1;
> >+sk->max_transid = (u64)-1;
> >+
> [...]
> >+/* go through each item to find dir item named "default" */
> >+for (i = 0; i < sk->nr_items; i++) {
> >+sh = (struct btrfs_ioctl_search_header *)(args.buf +
> >+ off);
> >+off += sizeof(*sh);
> >+if (sh->type == BTRFS_DIR_ITEM_KEY) {
> >+di = (struct btrfs_dir_item *)(args.buf + off);
> >+name_len = le16_to_cpu(di->name_len);
> >+name = (char *)di + sizeof(struct
> btrfs_dir_item);
> >+if (!strncmp("default", name, name_len)) {
> >+subvol_id = btrfs_disk_key_objectid(
> >+&di->location);
> >+break;
> >+}
> >+}
> >+
> >+off += sh->len;
> >+}
>
> I am not familiar with the "default subvolume key", but are you sure
> that the
> key is always in the first set of returned keys ?
>
It seems there's not too much dir item in the root tree. In fact, I only see it
used for for default subvolume in the root tree.
So it should be enough.
> >+}
> >+
> > /* 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
> >@@ -650,7 +698,12 @@ int list_subvols(int fd)
> > while (n) {
> > struct root_info *entry;
> > entry = rb_entry(n, struct root_info, rb_node);
> >-resolve_root(&root_lookup, entry);
> >+if(!get_default)
> >+resolve_root(&root_lookup, entry);
> >+/* we only want the default subvolume */
> >+else if(subvol_id == entry->root_id)
> >+resolve_root(&root_lookup, entry);
> >+
>
> What happens if there no is a default subvolume (for example a very old
> btrfs
> filesystem, and/or after removing the "default" subvolume) ?
> I suggest to handle this case printing something like "No default
> subvolume
> found"
>
If there's no default subvolume, the output is empty. Just the same as when you
use "btrfs subvolume list" and there's no subvolume at all.
Thanks for all the review!
>
> BR
> G.Baroncelli
>
> > n = rb_prev(n);
> > }
> >
> >diff --git a/btrfs.c b/btrfs.c
> >index 46314cf..6b73f88 100644
> >--- a/btrfs.c
> >+++ b/btrfs.c
> >@@ -73,6 +73,9 @@ static struct Command commands[] = {
> > "Set the subvolume of the filesystem which will be
> mounted\n"
> > "as default."
> > },
> >+{ do_get_default_subvol, 1, "subvolume get-default", "\n"
> >+"Get the default subvolume of a filesystem."
> >+},
> > { do_fssync, 1,
> > "filesystem sync", "\n"
> > "Force a sync on the filesystem ."
> >diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> >index 8031c58..11c56f6 100644
> >--- a/btrfs_cmds.c
> >+++ b/btrfs_cmds.c
> >@@ -301,7 +301,7 @@ int do_subvol_list(int argc, char **argv)
> > fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
> > return 12;
> > }
> >-ret = list_subvols(fd);
> >+ret = list_subvols(fd, 0);
> > if (ret)
> > return 19;
> > return 0;
> >@@ -834,6 +834,35 @@ int do_set_default_subvol(int nargs, char **argv)
> > return 0;
> > }
> >
> >+int do_get_default_subvol(int nargs, char **argv)
> >+{
> >+int fd;
> >+int ret;
> >+
RE: [PATCH V2] Btrfs-progs: add "btrfs subvolume get-default" subcommand
> -Original Message-
> From: Andreas Philipp [mailto:philipp.andr...@gmail.com]
> Sent: Monday, July 11, 2011 10:11 PM
> To: Zhong, Xin
> Cc: linux-btrfs@vger.kernel.org; Hugo Mills
> Subject: Re: [PATCH V2] Btrfs-progs: add "btrfs subvolume get-default"
> subcommand
>
>
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Would you mind rebasing your patch on Hugo Mills' integration-branch
> for
> btrfs progs at
> http://git.darksatanic.net/repo/btrfs-progs-unstable.git
> integration-20110705
> since it does not apply on top of all changes which are already there.
> Additionally, I spotted one whitespace error in the patch and marked it
> below.
Ok. I will update my patch. Thanks!
> Thanks,
> Andreas Philipp
>
> On 11.07.2011 10:56, Zhong, Xin wrote:
> > Add subcommand to get the default subvolume of btrfs filesystem
> >
> > Reported-by: Yang, Yi
> > Signed-off-by: Zhong, Xin
> > ---
> > btrfs-list.c | 57
> +++--
> > btrfs.c | 3 +++
> > btrfs_cmds.c | 31 ++-
> > btrfs_cmds.h | 3 ++-
> > 4 files changed, 90 insertions(+), 4 deletions(-)
> >
> > diff --git a/btrfs-list.c b/btrfs-list.c
> > index 93766a8..aa6a9b4 100644
> > --- a/btrfs-list.c
> > +++ b/btrfs-list.c
> > @@ -536,7 +536,7 @@ build:
> > return full;
> > }
> >
> > -int list_subvols(int fd)
> > +int list_subvols(int fd, int get_default)
> > {
> > struct root_lookup root_lookup;
> > struct rb_node *n;
> > @@ -545,10 +545,12 @@ int list_subvols(int fd)
> > struct btrfs_ioctl_search_key *sk = &args.key;
> > struct btrfs_ioctl_search_header *sh;
> > struct btrfs_root_ref *ref;
> > + struct btrfs_dir_item *di;
> > unsigned long off = 0;
> > int name_len;
> > char *name;
> > u64 dir_id;
> > + u64 subvol_id = 0;
> > int i;
> >
> > root_lookup_init(&root_lookup);
> > @@ -642,6 +644,52 @@ int list_subvols(int fd)
> > n = rb_next(n);
> > }
> >
> > + memset(&args, 0, sizeof(args));
> > +
> > + /* search in the tree of tree roots */
> > + sk->tree_id = BTRFS_ROOT_TREE_OBJECTID;
> > +
> > + /* search dir item */
> > + sk->max_type = BTRFS_DIR_ITEM_KEY;
> > + sk->min_type = BTRFS_DIR_ITEM_KEY;
> > +
> > + sk->max_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
> > + sk->min_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
> > + sk->max_offset = (u64)-1;
> > + sk->max_transid = (u64)-1;
> > +
> > + /* just a big number, doesn't matter much */
> > + sk->nr_items = 4096;
> > +
> > + /* try to get the objectid of default subvolume */
> > + if(get_default) {
> > + ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
> > + if (ret < 0) {
> > + fprintf(stderr, "ERROR: can't perform the search\n");
> > + return ret;
> > + }
> > +
> > + off = 0;
> > + /* go through each item to find dir item named "default" */
> > + for (i = 0; i < sk->nr_items; i++) {
> > + sh = (struct btrfs_ioctl_search_header *)(args.buf +
> > + off);
> > + off += sizeof(*sh);
> > + if (sh->type == BTRFS_DIR_ITEM_KEY) {
> > + di = (struct btrfs_dir_item *)(args.buf + off);
> > + name_len = le16_to_cpu(di->name_len);
> > + name = (char *)di + sizeof(struct btrfs_dir_item);
> > + if (!strncmp("default", name, name_len)) {
> > + subvol_id = btrfs_disk_key_objectid(
> > + &di->location);
> > + break;
> > + }
> > + }
> > +
> > + off += sh->len;
> > + }
> > + }
> > +
> > /* 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
> > @@ -650,7 +698,12 @@ int list_subvols(int fd)
> > while (n) {
> > struct root_info *entry;
> > entry = rb_entry(n, struct root_info, rb_node);
> > - resolve_root(&root_lookup, entry);
> > + if(!get_default)
> > + resolve_root(&root_lookup, entry);
> > + /* we only want the default subvolume */
> > + else if(subvol_id == entry->root_id)
> > + resolve_root(&root_lookup, entry);
> > +
> This line adds a whitespace error.
> > n = rb_prev(n);
> > }
> >
> > diff --git a/btrfs.c b/btrfs.c
> > index 46314cf..6b73f88 100644
> > --- a/btrfs.c
> > +++ b/btrfs.c
> > @@ -73,6 +73,9 @@ static struct Command commands[] = {
> > "Set
Re: [PATCH V2] Btrfs-progs: add "btrfs subvolume get-default" subcommand
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Would you mind rebasing your patch on Hugo Mills' integration-branch for
btrfs progs at
http://git.darksatanic.net/repo/btrfs-progs-unstable.git
integration-20110705
since it does not apply on top of all changes which are already there.
Additionally, I spotted one whitespace error in the patch and marked it
below.
Thanks,
Andreas Philipp
On 11.07.2011 10:56, Zhong, Xin wrote:
> Add subcommand to get the default subvolume of btrfs filesystem
>
> Reported-by: Yang, Yi
> Signed-off-by: Zhong, Xin
> ---
> btrfs-list.c | 57
+++--
> btrfs.c | 3 +++
> btrfs_cmds.c | 31 ++-
> btrfs_cmds.h | 3 ++-
> 4 files changed, 90 insertions(+), 4 deletions(-)
>
> diff --git a/btrfs-list.c b/btrfs-list.c
> index 93766a8..aa6a9b4 100644
> --- a/btrfs-list.c
> +++ b/btrfs-list.c
> @@ -536,7 +536,7 @@ build:
> return full;
> }
>
> -int list_subvols(int fd)
> +int list_subvols(int fd, int get_default)
> {
> struct root_lookup root_lookup;
> struct rb_node *n;
> @@ -545,10 +545,12 @@ int list_subvols(int fd)
> struct btrfs_ioctl_search_key *sk = &args.key;
> struct btrfs_ioctl_search_header *sh;
> struct btrfs_root_ref *ref;
> + struct btrfs_dir_item *di;
> unsigned long off = 0;
> int name_len;
> char *name;
> u64 dir_id;
> + u64 subvol_id = 0;
> int i;
>
> root_lookup_init(&root_lookup);
> @@ -642,6 +644,52 @@ int list_subvols(int fd)
> n = rb_next(n);
> }
>
> + memset(&args, 0, sizeof(args));
> +
> + /* search in the tree of tree roots */
> + sk->tree_id = BTRFS_ROOT_TREE_OBJECTID;
> +
> + /* search dir item */
> + sk->max_type = BTRFS_DIR_ITEM_KEY;
> + sk->min_type = BTRFS_DIR_ITEM_KEY;
> +
> + sk->max_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
> + sk->min_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
> + sk->max_offset = (u64)-1;
> + sk->max_transid = (u64)-1;
> +
> + /* just a big number, doesn't matter much */
> + sk->nr_items = 4096;
> +
> + /* try to get the objectid of default subvolume */
> + if(get_default) {
> + ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
> + if (ret < 0) {
> + fprintf(stderr, "ERROR: can't perform the search\n");
> + return ret;
> + }
> +
> + off = 0;
> + /* go through each item to find dir item named "default" */
> + for (i = 0; i < sk->nr_items; i++) {
> + sh = (struct btrfs_ioctl_search_header *)(args.buf +
> + off);
> + off += sizeof(*sh);
> + if (sh->type == BTRFS_DIR_ITEM_KEY) {
> + di = (struct btrfs_dir_item *)(args.buf + off);
> + name_len = le16_to_cpu(di->name_len);
> + name = (char *)di + sizeof(struct btrfs_dir_item);
> + if (!strncmp("default", name, name_len)) {
> + subvol_id = btrfs_disk_key_objectid(
> + &di->location);
> + break;
> + }
> + }
> +
> + off += sh->len;
> + }
> + }
> +
> /* 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
> @@ -650,7 +698,12 @@ int list_subvols(int fd)
> while (n) {
> struct root_info *entry;
> entry = rb_entry(n, struct root_info, rb_node);
> - resolve_root(&root_lookup, entry);
> + if(!get_default)
> + resolve_root(&root_lookup, entry);
> + /* we only want the default subvolume */
> + else if(subvol_id == entry->root_id)
> + resolve_root(&root_lookup, entry);
> +
This line adds a whitespace error.
> n = rb_prev(n);
> }
>
> diff --git a/btrfs.c b/btrfs.c
> index 46314cf..6b73f88 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -73,6 +73,9 @@ static struct Command commands[] = {
> "Set the subvolume of the filesystem which will be mounted\n"
> "as default."
> },
> + { do_get_default_subvol, 1, "subvolume get-default", "\n"
> + "Get the default subvolume of a filesystem."
> + },
> { do_fssync, 1,
> "filesystem sync", "\n"
> "Force a sync on the filesystem ."
> diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> index 8031c58..11c56f6 100644
> --- a/btrfs_cmds.c
> +++ b/btrfs_cmds.c
> @@ -301,7 +301,7 @@ int do_subvol_list(int argc, char **argv)
> fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
> return 12;
> }
> - ret = list_subvols(fd);
> + ret = list_subvols(fd, 0);
> if (ret)
> return 19;
> return 0;
> @@ -834,6 +834,35 @@ int do_set_default_subvol(int nargs, char **argv)
> return 0;
> }
>
> +int do_get_default_subvol(int nargs, char **argv)
> +{
> + int fd;
> + int ret;
> + char *subvol;
> +
> + subvol = argv[1];
> +
> + ret = test_issubvolume(subvol);
> + if (ret < 0) {
> + fprintf(stderr, "ERROR: error accessing '%s'\n", subvol);
> + return 12;
> + }
> + if (!ret) {
> + fprintf(stderr, "ERROR: '%s' is not a subvolume\n", subvol);
> + return 13;
> + }
> +
> + fd = open_file_or_dir(subvol);
> + if (fd < 0) {
> + fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
> + return 12;
> + }
> + ret = list_subvols(fd, 1);
> + if (ret)
> + return 19;
> + return 0;
> +}
> +
> int do_df_filesystem(int nargs, char **argv)
> {
> struct btrfs_ioctl_space_args *sargs;
> diff --git a/btrfs_cmds.h b/btrfs_cmds.h
> index 7bde191..9c
