Re: cp --reflink with Btrfs

2009-12-14 Thread Josef Bacik
On Sun, Dec 13, 2009 at 12:29:03AM +, Jason White wrote:
> I am testing a Btrfs root file system with Debian (kernel 2.6.32) under KVM.
> 
> ja...@vrtl:~$ touch testfile
> ja...@vrtl:~$ cp --reflink testfile /tmp
> cp: failed to clone `/tmp/testfile': Invalid argument
> 
> This is with GNU Coreutils 8.0 taken from debian Sid.
> 
> Is this a Coreutils issue, a Btrfs problem or something in my local
> configuration?
>

Try using bcp, if that works then its likely a problem with coreutils.

> All the other issues that I have encountered so far are known limitations, for
> example, lack of Btrfs support in Grub 2 and the current inability of the
> initrd scripts from Debian to recognize the file system type. All of this will
> be fixed, no doubt, over time.
> 

Yeah the initrd stuff is all debian "problems".  When I added btrfs support to
fedora there were alot of various utilities and such that needed to be changed
in order to accomodate it, so the same sort of thing likely needs to be done
with debian.  There are patches for Grub1 since thats what Fedora uses, I
suppose that there will be Grub2 patches in the future, but since the author of
the grub patches uses fedora and we're on grub1 it's all we care about :).
Thanks,

Josef
--
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: BUG: Link from sub volume, then remove the subvolume -> wrong link

2009-12-14 Thread Andrew Lutomirski
[cc: Chris -- This, or something like it, should probably go to
stable, but it needs to make it to upstream somewhere first.]

On Sun, Dec 13, 2009 at 6:42 PM, TARUISI Hiroaki
 wrote:
> Hi,
>
> We can see the patch in ML archive or at 'Patchwork' site.
>
>  http://patchwork.kernel.org/patch/59519/

That fixes the bug, but shouldn't the error be EXDEV?  It has nothing
to do with permissions.

--Andy
--
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] Btrfs: fix various things with the listing ioctl

2009-12-14 Thread Josef Bacik
With slab poisoning on you could panic the box simply by running btrfsctl -l
multiple times in a row on the same volume.  This patch fixes up the ioctl stuff
to be a bit cleaner, makes sure we always call btrfs_free_path() instead of
kfree(path) and make sure we do not kfree() our work names before we are done
using them.  There were several memory leaks and use after free problems
previously, they appear to be gone now, and as an added bonus doing btrfsctl -l
no longer panic's the box.  Thanks,

Signed-off-by: Josef Bacik 
---
 fs/btrfs/ioctl.c |   88 +++--
 1 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c157eb7..9778324 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -759,18 +759,29 @@ static noinline int btrfs_search_path_in_tree(struct 
btrfs_fs_info *info,
 
if (dirid == BTRFS_FIRST_FREE_OBJECTID) {
name[0]='\0';
-   ret = 0;
-   goto out_direct;
+   return 0;
}
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
+
name_stack = kzalloc(BTRFS_PATH_NAME_MAX+1, GFP_NOFS);
+   if (!name_stack) {
+   btrfs_free_path(path);
+   return -ENOMEM;
+   }
+
ptr = &name_stack[BTRFS_PATH_NAME_MAX];
 
key.objectid = tree_id;
key.type = BTRFS_ROOT_ITEM_KEY;
key.offset = (u64)-1;
root = btrfs_read_fs_root_no_name(info, &key);
+   if (IS_ERR(root)) {
+   printk(KERN_ERR "could not find root %llu\n", tree_id);
+   return -ENOENT;
+   }
 
key.objectid = dirid;
key.type = BTRFS_INODE_REF_KEY;
@@ -778,14 +789,14 @@ static noinline int btrfs_search_path_in_tree(struct 
btrfs_fs_info *info,
 
while(1) {
ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-   if (ret<0)
+   if (ret < 0)
goto out;
 
l = path->nodes[0];
slot = path->slots[0];
btrfs_item_key_to_cpu(l, &key, slot);
 
-   if (ret>0 && (key.objectid != dirid ||
+   if (ret > 0 && (key.objectid != dirid ||
key.type != BTRFS_INODE_REF_KEY))
goto out;
 
@@ -814,11 +825,8 @@ static noinline int btrfs_search_path_in_tree(struct 
btrfs_fs_info *info,
name[total_len]='\0';
ret = 0;
 out:
-   btrfs_release_path(root, path);
-   kfree(path);
+   btrfs_free_path(path);
kfree(name_stack);
-
-out_direct:
return ret;
 }
 
@@ -857,6 +865,7 @@ static noinline int btrfs_ioctl_snap_listing(struct file 
*file,
 {
struct btrfs_ioctl_subvol_leaf *leaf;
struct btrfs_ioctl_subvol_args *svol;
+   struct inode *inode;
int rest, offset, idx, name_len, i;
struct btrfs_root *tree_root;
struct btrfs_root_ref *ref;
@@ -878,18 +887,18 @@ static noinline int btrfs_ioctl_snap_listing(struct file 
*file,
 
work_path = kzalloc(BTRFS_PATH_NAME_MAX + 1, GFP_NOFS);
if (!work_path) {
-   kfree(path);
+   btrfs_free_path(path);
return -ENOMEM;
}
 
svol = memdup_user(arg, sizeof(struct btrfs_ioctl_subvol_args));
if (IS_ERR(svol)) {
-   kfree(path);
+   btrfs_free_path(path);
kfree(work_path);
return PTR_ERR(svol);
}
if (svol->len < BTRFS_SUBVOL_LEAF_SIZE_MIN) {
-   kfree(path);
+   btrfs_free_path(path);
kfree(work_path);
kfree(svol);
return -EINVAL;
@@ -897,44 +906,42 @@ static noinline int btrfs_ioctl_snap_listing(struct file 
*file,
 
leaf = memdup_user(svol->leaf, svol->len);
if (IS_ERR(leaf)) {
-   kfree(path);
+   btrfs_free_path(path);
kfree(work_path);
kfree(svol);
return PTR_ERR(leaf);
}
-   if (leaf->len != svol->len)
-   goto out_inval;
+   if (leaf->len != svol->len) {
+   ret = -EINVAL;
+   goto out;
+   }
 
-   tree_root =
-   BTRFS_I(fdentry(file)->d_inode)->root->fs_info->tree_root;
+   inode = fdentry(file)->d_inode;
+   tree_root = BTRFS_I(inode)->root->fs_info->tree_root;
if (!leaf->parent_tree) {
-   leaf->parent_tree =
-   
BTRFS_I(fdentry(file)->d_inode)->root->root_key.objectid;
+   leaf->parent_tree = BTRFS_I(inode)->root->root_key.objectid;
+
if (svol->base_path) {
-   work_path = kzalloc(BTRFS_PATH_NAME_MAX+1, GFP_NOFS);
-   if (!work_path) {
-   ret = -ENOMEM;
-   goto out;
-   

[PATCH] Btrfs: make subvol=0 mount the original default root

2009-12-14 Thread Josef Bacik
Since theres not a good way to make sure the user sees the original default root
tree id, and not to mention it's 5 so is way different than any other volume,
just make subvol=0 mount the original default root.  This makes it a bit easier
for users to handle in the long run.  Thanks,

Signed-off-by: Josef Bacik 
---
 fs/btrfs/super.c |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index bcbdc1b..ccc0380 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -305,9 +305,15 @@ static int btrfs_parse_early_options(const char *options, 
fmode_t flags,
switch (token) {
case Opt_subvol:
intarg = 0;
-   match_int(&args[0], &intarg);
-   if (intarg)
-   *subvol_objectid = intarg;
+   error = match_int(&args[0], &intarg);
+   if (!error) {
+   /* we want the original fs_tree */
+   if (!intarg)
+   *subvol_objectid =
+   BTRFS_FS_TREE_OBJECTID;
+   else
+   *subvol_objectid = intarg;
+   }
break;
case Opt_device:
error = btrfs_scan_one_device(match_strdup(&args[0]),
-- 
1.5.4.3

--
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] btrfs: Show discard option in /proc/mounts

2009-12-14 Thread Matthew Wilcox

Christoph's patch e244a0aeb6a599c19a7c802cda6e2d67c847b154 doesn't display
the discard option in /proc/mounts, leading to some confusion for me.
Here's the missing bit.

Signed-off-by: Matthew Wilcox 

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 752a546..5e5bd29 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -450,6 +450,8 @@ static int btrfs_show_options(struct seq_file *seq, struct 
vfsmount *vfs)
seq_puts(seq, ",notreelog");
if (btrfs_test_opt(root, FLUSHONCOMMIT))
seq_puts(seq, ",flushoncommit");
+   if (btrfs_test_opt(root, DISCARD))
+   seq_puts(seq, ",discard");
if (!(root->fs_info->sb->s_flags & MS_POSIXACL))
seq_puts(seq, ",noacl");
return 0;

-- 
Matthew Wilcox  Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
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] btrfs: Show discard option in /proc/mounts

2009-12-14 Thread Christoph Hellwig
On Mon, Dec 14, 2009 at 03:01:12PM -0700, Matthew Wilcox wrote:
> 
> Christoph's patch e244a0aeb6a599c19a7c802cda6e2d67c847b154 doesn't display
> the discard option in /proc/mounts, leading to some confusion for me.
> Here's the missing bit.
> 
> Signed-off-by: Matthew Wilcox 

Looks good, sorry for forgetting that bit.

--
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: BUG: Link from sub volume, then remove the subvolume -> wrong link

2009-12-14 Thread TARUISI Hiroaki
Hi,

I think message:'Operation not permitted' is more
proper for this problem than 'Invalid cross-device
link' simply because this link is not cross-device
link.

Of course, this operation is prohibited not by security
policy but by inner limitation of btrfs, this usage of
EPERM may be a kind of abuses...

Regards,
taruisi

(2009/12/15 3:43), Andrew Lutomirski wrote:
> [cc: Chris -- This, or something like it, should probably go to
> stable, but it needs to make it to upstream somewhere first.]
> 
> On Sun, Dec 13, 2009 at 6:42 PM, TARUISI Hiroaki
>  wrote:
>> Hi,
>>
>> We can see the patch in ML archive or at 'Patchwork' site.
>>
>>  http://patchwork.kernel.org/patch/59519/
> 
> That fixes the bug, but shouldn't the error be EXDEV?  It has nothing
> to do with permissions.
> 
> --Andy
> --
> 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

--
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] Btrfs: fix various things with the listing ioctl

2009-12-14 Thread TARUISI Hiroaki
(2009/12/15 4:17), Josef Bacik wrote:
> With slab poisoning on you could panic the box simply by running btrfsctl -l
> multiple times in a row on the same volume.  This patch fixes up the ioctl 
> stuff
> to be a bit cleaner, makes sure we always call btrfs_free_path() instead of
> kfree(path) and make sure we do not kfree() our work names before we are done
> using them.  There were several memory leaks and use after free problems
> previously, they appear to be gone now, and as an added bonus doing btrfsctl 
> -l
> no longer panic's the box.  Thanks,
> 
> Signed-off-by: Josef Bacik 
> ---

Thank you for your patch. I've tested this patch and
I found it works. And I'd like to add one line to
prevent from another tiny leak.

Signed-off-by: TARUISI Hiroaki 
---
 fs/btrfs/ioctl.c |1 +
 1 file changed, 1 insertion(+)

Index: b/fs/btrfs/ioctl.c
===
--- a/fs/btrfs/ioctl.c  2009-12-15 11:31:18.0 +0900
+++ b/fs/btrfs/ioctl.c  2009-12-15 11:33:28.0 +0900
@@ -989,6 +989,7 @@ static noinline int btrfs_ioctl_snap_lis
if (rest < sizeof(struct btrfs_ioctl_subvol_items) +
name_len + strlen(work_path) + 1) {
svol->next_len = name_len + strlen(work_path);
+   kfree(name);
if (copy_to_user(arg, svol, sizeof(*svol))) {
ret = -EFAULT;
goto out;

--
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: BUG: Link from sub volume, then remove the subvolume -> wrong link

2009-12-14 Thread Andy Lutomirski

TARUISI Hiroaki wrote:

Hi,

I think message:'Operation not permitted' is more
proper for this problem than 'Invalid cross-device
link' simply because this link is not cross-device
link.

Of course, this operation is prohibited not by security
policy but by inner limitation of btrfs, this usage of
EPERM may be a kind of abuses...



EOPNOTSUPP?  EINVAL?

Seriously, though, something should go in.  Chris, any opinion?

--Andy


Regards,
taruisi

(2009/12/15 3:43), Andrew Lutomirski wrote:

[cc: Chris -- This, or something like it, should probably go to
stable, but it needs to make it to upstream somewhere first.]

On Sun, Dec 13, 2009 at 6:42 PM, TARUISI Hiroaki
 wrote:

Hi,

We can see the patch in ML archive or at 'Patchwork' site.

 http://patchwork.kernel.org/patch/59519/

That fixes the bug, but shouldn't the error be EXDEV?  It has nothing
to do with permissions.

--Andy
--
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


--
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



--
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 1/4] btrfs: fix the bug that __tree_search() returns the wrong result in extent_map.c

2009-12-14 Thread Miao Xie
__tree_search() returns the reverse result about the prev node and the next 
node. And we can
get the prev node and the next node directly by rb_prev() and rb_next(), so it 
is unnecessary
to use while loop to get them.

This patch fixes this bug of the wrong result.

Signed-off-by: Miao Xie 
---
 fs/btrfs/extent_map.c |   13 -
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 46bea0f..16744f4 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -136,20 +136,15 @@ static struct rb_node *__tree_search(struct rb_root 
*root, u64 offset,
 
if (prev_ret) {
orig_prev = prev;
-   while (prev && offset >= extent_map_end(prev_entry)) {
-   prev = rb_next(prev);
-   prev_entry = rb_entry(prev, struct extent_map, rb_node);
-   }
+   if (prev && offset < prev_entry->start)
+   prev = rb_prev(prev);
*prev_ret = prev;
prev = orig_prev;
}
 
if (next_ret) {
-   prev_entry = rb_entry(prev, struct extent_map, rb_node);
-   while (prev && offset < prev_entry->start) {
-   prev = rb_prev(prev);
-   prev_entry = rb_entry(prev, struct extent_map, rb_node);
-   }
+   if (prev && offset >= extent_map_end(prev_entry))
+   prev = rb_next(prev);
*next_ret = prev;
}
return NULL;
-- 
1.6.5.2


--
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 2/4] btrfs: cleanup the code in lookup_extent_mapping()

2009-12-14 Thread Miao Xie
It is unnecessary to check whether the range intersects the prev extent, because
the end of the prev extent must be less than or equal the start of the range to
search.

This patch cleanups those unnecessary check code in lookup_extent_mapping()

Signed-off-by: Miao Xie 
---
 fs/btrfs/extent_map.c |5 -
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 16744f4..ef49e31 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -330,11 +330,6 @@ struct extent_map *lookup_extent_mapping(struct 
extent_map_tree *tree,
u64 end = range_end(start, len);
 
rb_node = __tree_search(&tree->map, start, &prev, &next);
-   if (!rb_node && prev) {
-   em = rb_entry(prev, struct extent_map, rb_node);
-   if (end > em->start && start < extent_map_end(em))
-   goto found;
-   }
if (!rb_node && next) {
em = rb_entry(next, struct extent_map, rb_node);
if (end > em->start && start < extent_map_end(em))
-- 
1.6.5.2


--
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 3/4] btrfs: cleanup the unnecessary code in __tree_search() in ordered-data.c

2009-12-14 Thread Miao Xie
It is unnecessary to get the prev node by getting the next node first, because
we can get the prev node directly by rb_prev(). And it is also unnecessary to 
use
while loop to get the prev node.

This patch cleanups those unnecessary code in __tree_search() in ordered-data.c

Signed-off-by: Miao Xie 
---
 fs/btrfs/ordered-data.c |   24 ++--
 1 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 5799bc4..74128f6 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -69,7 +69,6 @@ static struct rb_node *__tree_search(struct rb_root *root, 
u64 file_offset,
 {
struct rb_node *n = root->rb_node;
struct rb_node *prev = NULL;
-   struct rb_node *test;
struct btrfs_ordered_extent *entry;
struct btrfs_ordered_extent *prev_entry = NULL;
 
@@ -88,28 +87,9 @@ static struct rb_node *__tree_search(struct rb_root *root, 
u64 file_offset,
if (!prev_ret)
return NULL;
 
-   while (prev && file_offset >= entry_end(prev_entry)) {
-   test = rb_next(prev);
-   if (!test)
-   break;
-   prev_entry = rb_entry(test, struct btrfs_ordered_extent,
- rb_node);
-   if (file_offset < entry_end(prev_entry))
-   break;
+   if (prev && file_offset < prev_entry->file_offset)
+   prev = rb_prev(prev);
 
-   prev = test;
-   }
-   if (prev)
-   prev_entry = rb_entry(prev, struct btrfs_ordered_extent,
- rb_node);
-   while (prev && file_offset < entry_end(prev_entry)) {
-   test = rb_prev(prev);
-   if (!test)
-   break;
-   prev_entry = rb_entry(test, struct btrfs_ordered_extent,
- rb_node);
-   prev = test;
-   }
*prev_ret = prev;
return NULL;
 }
-- 
1.6.5.2


--
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 4/4] btrfs: remove tree_search() in extent_map.c

2009-12-14 Thread Miao Xie
This patch removes tree_search() in extent_map.c because it is not called by
anything.

Signed-off-by: Miao Xie 
---
 fs/btrfs/extent_map.c |   14 --
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 07ec673..fe4bb5e 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -150,20 +150,6 @@ static struct rb_node *__tree_search(struct rb_root *root, 
u64 offset,
return NULL;
 }
 
-/*
- * look for an offset in the tree, and if it can't be found, return
- * the first offset we can find smaller than 'offset'.
- */
-static inline struct rb_node *tree_search(struct rb_root *root, u64 offset)
-{
-   struct rb_node *prev;
-   struct rb_node *ret;
-   ret = __tree_search(root, offset, &prev, NULL);
-   if (!ret)
-   return prev;
-   return ret;
-}
-
 /* check to see if two extent_map structs are adjacent and safe to merge */
 static int mergable_maps(struct extent_map *prev, struct extent_map *next)
 {
-- 
1.6.2.2


--
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