| Message ID | 7ca269db-a724-5938-f4d8-b85e893fd558@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote: > Currently, the top-level subvolume lacks the UUID. As a result, both > non-snapshot subvolume and snapshot of top-level subvolume do not have > Parent UUID and cannot be distinguisued. Therefore "fi show" of > top-level lists all the subvolumes which lacks the UUID in > "Snapshot(s)" filed. Also, it lacks the otime information. > > Fix this by adding the UUID and otime at the mkfs time. As a > consequence, snapshots of top-level subvolume now have a Parent UUID and > UUID tree will create an entry for top-level subvolume at mount time. > This should not cause the problem for current kernel, but user program > which relies on the empty Parent UUID may be affected by this change. Is there any way of adding a UUID to the top level subvol on an existing filesystem? It would be helpful not to have to rebuild every filesystem in the world to fix this. Hugo. > Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> > --- > This is also needed in order that "sub list -s" works properly for > non-privileged user[1] even if there are snapshots of toplevel subvolume. > > Currently the check if a subvolume is a snapshot is done by looking at the key > offset of ROOT_ITEM of subvolume (non-zero for snapshot) used by tree search ioctl. > However, non-privileged version of "sub list" won't use tree search ioctl and just > looking if parent uuid is null or not. Therefore there is no way to recognize > snapshots of toplevel subvolume. > > [1] https://marc.info/?l=linux-btrfs&m=152144463907830&w=2 > > mkfs/common.c | 14 ++++++++++++++ > mkfs/main.c | 3 +++ > 2 files changed, 17 insertions(+) > > diff --git a/mkfs/common.c b/mkfs/common.c > index 16916ca2..6924d9b7 100644 > --- a/mkfs/common.c > +++ b/mkfs/common.c > @@ -44,6 +44,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg, > u32 itemoff; > int ret = 0; > int blk; > + u8 uuid[BTRFS_UUID_SIZE]; > > memset(buf->data + sizeof(struct btrfs_header), 0, > cfg->nodesize - sizeof(struct btrfs_header)); > @@ -77,6 +78,19 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg, > btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff); > btrfs_set_item_size(buf, btrfs_item_nr(nritems), > sizeof(root_item)); > + if (blk == MKFS_FS_TREE) { > + time_t now = time(NULL); > + > + uuid_generate(uuid); > + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE); > + btrfs_set_stack_timespec_sec(&root_item.otime, now); > + btrfs_set_stack_timespec_sec(&root_item.ctime, now); > + } else { > + memset(uuid, 0, BTRFS_UUID_SIZE); > + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE); > + btrfs_set_stack_timespec_sec(&root_item.otime, 0); > + btrfs_set_stack_timespec_sec(&root_item.ctime, 0); > + } > write_extent_buffer(buf, &root_item, > btrfs_item_ptr_offset(buf, nritems), > sizeof(root_item)); > diff --git a/mkfs/main.c b/mkfs/main.c > index 5a717f70..52d92581 100644 > --- a/mkfs/main.c > +++ b/mkfs/main.c > @@ -315,6 +315,7 @@ static int create_tree(struct btrfs_trans_handle *trans, > struct btrfs_key location; > struct btrfs_root_item root_item; > struct extent_buffer *tmp; > + u8 uuid[BTRFS_UUID_SIZE] = {0}; > int ret; > > ret = btrfs_copy_root(trans, root, root->node, &tmp, objectid); > @@ -325,6 +326,8 @@ static int create_tree(struct btrfs_trans_handle *trans, > btrfs_set_root_bytenr(&root_item, tmp->start); > btrfs_set_root_level(&root_item, btrfs_header_level(tmp)); > btrfs_set_root_generation(&root_item, trans->transid); > + /* clear uuid of source tree */ > + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE); > free_extent_buffer(tmp); > > location.objectid = objectid;
On Mon, Mar 19, 2018 at 08:20:10AM +0000, Hugo Mills wrote: > On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote: > > Currently, the top-level subvolume lacks the UUID. As a result, both > > non-snapshot subvolume and snapshot of top-level subvolume do not have > > Parent UUID and cannot be distinguisued. Therefore "fi show" of > > top-level lists all the subvolumes which lacks the UUID in > > "Snapshot(s)" filed. Also, it lacks the otime information. > > > > Fix this by adding the UUID and otime at the mkfs time. As a > > consequence, snapshots of top-level subvolume now have a Parent UUID and > > UUID tree will create an entry for top-level subvolume at mount time. > > This should not cause the problem for current kernel, but user program > > which relies on the empty Parent UUID may be affected by this change. > > Is there any way of adding a UUID to the top level subvol on an > existing filesystem? It would be helpful not to have to rebuild every > filesystem in the world to fix this. We can do that by a special purpose tool. The easiest way is to set the uuid on an unmouted filesystem, but as this is a one-time action I hope this is acceptable. Added to todo, thanks for the suggestion. -- 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, Mar 19, 2018 at 02:02:23PM +0100, David Sterba wrote: > On Mon, Mar 19, 2018 at 08:20:10AM +0000, Hugo Mills wrote: > > On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote: > > > Currently, the top-level subvolume lacks the UUID. As a result, both > > > non-snapshot subvolume and snapshot of top-level subvolume do not have > > > Parent UUID and cannot be distinguisued. Therefore "fi show" of > > > top-level lists all the subvolumes which lacks the UUID in > > > "Snapshot(s)" filed. Also, it lacks the otime information. > > > > > > Fix this by adding the UUID and otime at the mkfs time. As a > > > consequence, snapshots of top-level subvolume now have a Parent UUID and > > > UUID tree will create an entry for top-level subvolume at mount time. > > > This should not cause the problem for current kernel, but user program > > > which relies on the empty Parent UUID may be affected by this change. > > > > Is there any way of adding a UUID to the top level subvol on an > > existing filesystem? It would be helpful not to have to rebuild every > > filesystem in the world to fix this. > > We can do that by a special purpose tool. The easiest way is to set the > uuid on an unmouted filesystem, but as this is a one-time action I hope > this is acceptable. Added to todo, thanks for the suggestion. Sounds good to me. Hugo.
On Mon, 2018-03-19 at 14:02 +0100, David Sterba wrote:
> We can do that by a special purpose tool.
No average user will ever run (even know) about that...
Could you perhaps either do it automatically in fsck (which is IMO als
a bad idea as fsck should be read-only per default)... or at least add
a warning to fsck, like a "Info: Please run tool foo to get bar done."?
Cheers,
Chris.
--
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, Mar 19, 2018 at 02:26:38PM +0100, Christoph Anton Mitterer wrote: > On Mon, 2018-03-19 at 14:02 +0100, David Sterba wrote: > > We can do that by a special purpose tool. > > No average user will ever run (even know) about that... > > Could you perhaps either do it automatically in fsck (which is IMO als > a bad idea as fsck should be read-only per default)... or at least add > a warning to fsck, like a "Info: Please run tool foo to get bar done."? Makes sense, check can warn and point to the command, in a similar way we have the fix-device-count. -- 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, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote: > Currently, the top-level subvolume lacks the UUID. As a result, both > non-snapshot subvolume and snapshot of top-level subvolume do not have > Parent UUID and cannot be distinguisued. Therefore "fi show" of > top-level lists all the subvolumes which lacks the UUID in > "Snapshot(s)" filed. Also, it lacks the otime information. > > Fix this by adding the UUID and otime at the mkfs time. As a > consequence, snapshots of top-level subvolume now have a Parent UUID and > UUID tree will create an entry for top-level subvolume at mount time. > This should not cause the problem for current kernel, but user program > which relies on the empty Parent UUID may be affected by this change. > > Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> So this adds uuid, ctime and otime to FS_TEEE but also to UUID_TREE and DATA_RELOC_TREE. This is harmelss, but would be nice to mention in the changelog, I'll apply the patch add that. Thanks. -- 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 2018/03/22 2:48, David Sterba wrote: > On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote: >> Currently, the top-level subvolume lacks the UUID. As a result, both >> non-snapshot subvolume and snapshot of top-level subvolume do not have >> Parent UUID and cannot be distinguisued. Therefore "fi show" of >> top-level lists all the subvolumes which lacks the UUID in >> "Snapshot(s)" filed. Also, it lacks the otime information. >> >> Fix this by adding the UUID and otime at the mkfs time. As a >> consequence, snapshots of top-level subvolume now have a Parent UUID and >> UUID tree will create an entry for top-level subvolume at mount time. >> This should not cause the problem for current kernel, but user program >> which relies on the empty Parent UUID may be affected by this change. >> >> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> > > So this adds uuid, ctime and otime to FS_TEEE but also to UUID_TREE and > DATA_RELOC_TREE. This is harmelss, but would be nice to mention in the > changelog, I'll apply the patch add that. Thanks. UUID is cleared at create_tree(), so I think you mean otime and ctime. However, other tree's ROOT_ITEM does not hold o/ctime and I'd like to clear o/ctime for UUID/DATA_RELOC_TREE too. So, I will send v2 patch and could you please use that instead of this? Regards, Tomohiro Misono -- 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/mkfs/common.c b/mkfs/common.c index 16916ca2..6924d9b7 100644 --- a/mkfs/common.c +++ b/mkfs/common.c @@ -44,6 +44,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg, u32 itemoff; int ret = 0; int blk; + u8 uuid[BTRFS_UUID_SIZE]; memset(buf->data + sizeof(struct btrfs_header), 0, cfg->nodesize - sizeof(struct btrfs_header)); @@ -77,6 +78,19 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg, btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff); btrfs_set_item_size(buf, btrfs_item_nr(nritems), sizeof(root_item)); + if (blk == MKFS_FS_TREE) { + time_t now = time(NULL); + + uuid_generate(uuid); + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE); + btrfs_set_stack_timespec_sec(&root_item.otime, now); + btrfs_set_stack_timespec_sec(&root_item.ctime, now); + } else { + memset(uuid, 0, BTRFS_UUID_SIZE); + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE); + btrfs_set_stack_timespec_sec(&root_item.otime, 0); + btrfs_set_stack_timespec_sec(&root_item.ctime, 0); + } write_extent_buffer(buf, &root_item, btrfs_item_ptr_offset(buf, nritems), sizeof(root_item)); diff --git a/mkfs/main.c b/mkfs/main.c index 5a717f70..52d92581 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -315,6 +315,7 @@ static int create_tree(struct btrfs_trans_handle *trans, struct btrfs_key location; struct btrfs_root_item root_item; struct extent_buffer *tmp; + u8 uuid[BTRFS_UUID_SIZE] = {0}; int ret; ret = btrfs_copy_root(trans, root, root->node, &tmp, objectid); @@ -325,6 +326,8 @@ static int create_tree(struct btrfs_trans_handle *trans, btrfs_set_root_bytenr(&root_item, tmp->start); btrfs_set_root_level(&root_item, btrfs_header_level(tmp)); btrfs_set_root_generation(&root_item, trans->transid); + /* clear uuid of source tree */ + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE); free_extent_buffer(tmp); location.objectid = objectid;
Currently, the top-level subvolume lacks the UUID. As a result, both non-snapshot subvolume and snapshot of top-level subvolume do not have Parent UUID and cannot be distinguisued. Therefore "fi show" of top-level lists all the subvolumes which lacks the UUID in "Snapshot(s)" filed. Also, it lacks the otime information. Fix this by adding the UUID and otime at the mkfs time. As a consequence, snapshots of top-level subvolume now have a Parent UUID and UUID tree will create an entry for top-level subvolume at mount time. This should not cause the problem for current kernel, but user program which relies on the empty Parent UUID may be affected by this change. Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> --- This is also needed in order that "sub list -s" works properly for non-privileged user[1] even if there are snapshots of toplevel subvolume. Currently the check if a subvolume is a snapshot is done by looking at the key offset of ROOT_ITEM of subvolume (non-zero for snapshot) used by tree search ioctl. However, non-privileged version of "sub list" won't use tree search ioctl and just looking if parent uuid is null or not. Therefore there is no way to recognize snapshots of toplevel subvolume. [1] https://marc.info/?l=linux-btrfs&m=152144463907830&w=2 mkfs/common.c | 14 ++++++++++++++ mkfs/main.c | 3 +++ 2 files changed, 17 insertions(+)