| Message ID | 5174C7B9.8000802@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
> /* > - * If the btrfs-progs is newer and kernel is at > - * generation_v1 then we don't touch v2 items > - * otherwise when kernel is at same or greater > - * version compared with btrfs-progs then update > - * the needed > - */ > - old_size = btrfs_item_size_nr(l, slot); > - if (old_size >= sizeof(*item)) { > - btrfs_set_root_generation_v2(item, > - btrfs_root_generation(item)); > + * If this is the first time we update the root item which originated > + * from an older kernel, we need to enlarge the item size to make room > + * for the added fields. > + */ > + if (old_len < sizeof(*item)) { With this fix. when kernel is using v1 and btrfs-progs is using v2, the disk structure will be changed to v2 after running btrfs check. I would rather take a conservative approach, by maintaining kernel intended version. Thanks, Anand -- 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
On 4/22/13 4:31 AM, Anand Jain wrote: > >> /* >> - * If the btrfs-progs is newer and kernel is at >> - * generation_v1 then we don't touch v2 items >> - * otherwise when kernel is at same or greater >> - * version compared with btrfs-progs then update >> - * the needed >> - */ >> - old_size = btrfs_item_size_nr(l, slot); >> - if (old_size >= sizeof(*item)) { >> - btrfs_set_root_generation_v2(item, >> - btrfs_root_generation(item)); >> + * If this is the first time we update the root item which originated >> + * from an older kernel, we need to enlarge the item size to make room >> + * for the added fields. >> + */ >> + if (old_len < sizeof(*item)) { > > With this fix. when kernel is using v1 and btrfs-progs is > using v2, the disk structure will be changed to v2 after > running btrfs check. I would rather take a conservative > approach, by maintaining kernel intended version. If the kernel can do it, it should be safe for userspace to do it as well. An old kernel will have a smaller btrfs_root_item size, and won't even see the new fields when it reads the roots, I think. And this has the advantage of not further diverging userspace from kernelspace which, as we've seen, leads to bugs. :) Thanks, -Eric > Thanks, Anand -- 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
On Mon, Apr 22, 2013 at 12:16:41AM -0500, Eric Sandeen wrote: > This addresses the same issue as did: > > 2bd1169 btrfs-progs: root_item generation_v2 is out of sync after btrfsck > > but rather than optionally updating generation_v2 based > on the size of the existing item, increase the size of the > item as needed, and unconditionally set generation_v2. > This matches the kernel code, and keeping things in sync is a > Good Thing. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > V2: give it a better, more descriptive subject/summary > > kdave - since you already had Anand's patch on the integration > branch, I dunno if you want to add this on top, or just replace > it with a similar patch to this, up to you. I'm going to replace the patch (the integration branches are not linear and could be rebased, the purpose is to catch such issues earlier so the patch queue is moderately clean when it gets pulled). thanks, david -- 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
diff --git a/root-tree.c b/root-tree.c index 1823918..902d75d 100644 --- a/root-tree.c +++ b/root-tree.c @@ -69,7 +69,7 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root int ret; int slot; unsigned long ptr; - u32 old_size; + u32 old_len; path = btrfs_alloc_path(); BUG_ON(!path); @@ -80,18 +80,42 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root l = path->nodes[0]; slot = path->slots[0]; ptr = btrfs_item_ptr_offset(l, slot); + old_len = btrfs_item_size_nr(l, slot); + /* - * If the btrfs-progs is newer and kernel is at - * generation_v1 then we don't touch v2 items - * otherwise when kernel is at same or greater - * version compared with btrfs-progs then update - * the needed - */ - old_size = btrfs_item_size_nr(l, slot); - if (old_size >= sizeof(*item)) { - btrfs_set_root_generation_v2(item, - btrfs_root_generation(item)); + * If this is the first time we update the root item which originated + * from an older kernel, we need to enlarge the item size to make room + * for the added fields. + */ + if (old_len < sizeof(*item)) { + btrfs_release_path(root, path); + ret = btrfs_search_slot(trans, root, key, path, + -1, 1); + if (ret < 0) { + goto out; + } + + ret = btrfs_del_item(trans, root, path); + if (ret < 0) { + goto out; + } + btrfs_release_path(root, path); + ret = btrfs_insert_empty_item(trans, root, path, + key, sizeof(*item)); + if (ret < 0) { + goto out; + } + l = path->nodes[0]; + slot = path->slots[0]; + ptr = btrfs_item_ptr_offset(l, slot); } + + /* + * Update generation_v2 so at the next mount we know the new root + * fields are valid. + */ + btrfs_set_root_generation_v2(item, btrfs_root_generation(item)); + write_extent_buffer(l, item, ptr, sizeof(*item)); btrfs_mark_buffer_dirty(path->nodes[0]); out:
This addresses the same issue as did: 2bd1169 btrfs-progs: root_item generation_v2 is out of sync after btrfsck but rather than optionally updating generation_v2 based on the size of the existing item, increase the size of the item as needed, and unconditionally set generation_v2. This matches the kernel code, and keeping things in sync is a Good Thing. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- V2: give it a better, more descriptive subject/summary kdave - since you already had Anand's patch on the integration branch, I dunno if you want to add this on top, or just replace it with a similar patch to this, up to you. Thanks! -Eric -- 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