Re: cp --reflink with Btrfs
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
[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
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
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
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
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
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/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
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
__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()
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
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
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