From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from userp1040.oracle.com ([156.151.31.81]:45975 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752038AbcHXK1H (ORCPT ); Wed, 24 Aug 2016 06:27:07 -0400 Subject: Re: [PATCH 05/13] btrfs-progs: two staged filesystem creation To: David Sterba , linux-btrfs@vger.kernel.org References: <1471947917-5324-1-git-send-email-dsterba@suse.com> <1471947917-5324-6-git-send-email-dsterba@suse.com> From: Anand Jain Message-ID: <35455b29-a076-41ef-e8ee-ed001601395a@oracle.com> Date: Wed, 24 Aug 2016 18:27:44 +0800 MIME-Version: 1.0 In-Reply-To: <1471947917-5324-6-git-send-email-dsterba@suse.com> Content-Type: text/plain; charset=windows-1252; format=flowed Sender: linux-btrfs-owner@vger.kernel.org List-ID: On 08/23/2016 06:25 PM, David Sterba wrote: > The filesystem existence on a device is manifested by the signature, > during the mkfs process we write it first and then create other > structures. Such filesystem is not valid and should not be registered > during device scan nor listed among devices from blkid. > > This patch will introduce two staged creation. In the first phase, the > signature is wrong, but recognized as a partially created filesystem (by > open or scan helpers). Once we successfully create and write everything, > we fixup the signature. At this point automated scanning should find > a valid filesystem on all devices. > > We can also rely on the partially created filesystem to do better error > handling during creation. We can just bail out and do not need to clean > up. Looks good to me. Reviewed-by: Anand Jain Thanks, Anand > The partial signature is '!BHRfS_M', can be shown by > > btrfs inspect-internal dump-super -F image > > Signed-off-by: David Sterba > --- > btrfs-convert.c | 7 +++++-- > ctree.h | 1 + > disk-io.c | 35 +++++++++++++++++++++++++++++------ > disk-io.h | 11 ++++++++++- > mkfs.c | 7 ++++++- > utils.c | 10 ++++++++-- > 6 files changed, 59 insertions(+), 12 deletions(-) > > diff --git a/btrfs-convert.c b/btrfs-convert.c > index a8852d612886..b34a413de5be 100644 > --- a/btrfs-convert.c > +++ b/btrfs-convert.c > @@ -2345,7 +2345,7 @@ static int do_convert(const char *devname, int datacsum, int packing, > } > > root = open_ctree_fd(fd, devname, mkfs_cfg.super_bytenr, > - OPEN_CTREE_WRITES); > + OPEN_CTREE_WRITES | OPEN_CTREE_FS_PARTIAL); > if (!root) { > fprintf(stderr, "unable to open ctree\n"); > goto fail; > @@ -2431,11 +2431,14 @@ static int do_convert(const char *devname, int datacsum, int packing, > } > is_btrfs = 1; > > - root = open_ctree_fd(fd, devname, 0, OPEN_CTREE_WRITES); > + root = open_ctree_fd(fd, devname, 0, > + OPEN_CTREE_WRITES | OPEN_CTREE_FS_PARTIAL); > if (!root) { > fprintf(stderr, "unable to open ctree\n"); > goto fail; > } > + root->fs_info->finalize_on_close = 1; > + close_ctree(root); > close(fd); > > printf("conversion complete.\n"); > diff --git a/ctree.h b/ctree.h > index 89e47abd9d14..d0569835d0a2 100644 > --- a/ctree.h > +++ b/ctree.h > @@ -1032,6 +1032,7 @@ struct btrfs_fs_info { > unsigned int ignore_chunk_tree_error:1; > unsigned int avoid_meta_chunk_alloc:1; > unsigned int avoid_sys_chunk_alloc:1; > + unsigned int finalize_on_close:1; > > int (*free_extent_hook)(struct btrfs_trans_handle *trans, > struct btrfs_root *root, > diff --git a/disk-io.c b/disk-io.c > index 4874f0689228..dd4a34142496 100644 > --- a/disk-io.c > +++ b/disk-io.c > @@ -1225,6 +1225,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, > struct extent_buffer *eb; > int ret; > int oflags; > + unsigned sbflags = SBREAD_DEFAULT; > > if (sb_bytenr == 0) > sb_bytenr = BTRFS_SUPER_INFO_OFFSET; > @@ -1247,9 +1248,18 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, > if (flags & OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR) > fs_info->ignore_chunk_tree_error = 1; > > - ret = btrfs_scan_fs_devices(fp, path, &fs_devices, sb_bytenr, > - (flags & OPEN_CTREE_RECOVER_SUPER) ? SBREAD_RECOVER : SBREAD_DEFAULT, > - (flags & OPEN_CTREE_NO_DEVICES)); > + if ((flags & OPEN_CTREE_RECOVER_SUPER) > + && (flags & OPEN_CTREE_FS_PARTIAL)) { > + fprintf(stderr, > + "cannot open a partially created filesystem for recovery"); > + return NULL; > + } > + > + if (flags & OPEN_CTREE_FS_PARTIAL) > + sbflags = SBREAD_PARTIAL; > + > + ret = btrfs_scan_fs_devices(fp, path, &fs_devices, sb_bytenr, sbflags, > + (flags & OPEN_CTREE_NO_DEVICES)); > if (ret) > goto out; > > @@ -1272,7 +1282,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, > sb_bytenr, SBREAD_RECOVER); > else > ret = btrfs_read_dev_super(fp, disk_super, sb_bytenr, > - SBREAD_DEFAULT); > + sbflags); > if (ret) { > printk("No valid btrfs found\n"); > goto out_devices; > @@ -1401,8 +1411,12 @@ static int check_super(struct btrfs_super_block *sb, unsigned sbflags) > int csum_size; > > if (btrfs_super_magic(sb) != BTRFS_MAGIC) { > - error("superblock magic doesn't match"); > - return -EIO; > + if (btrfs_super_magic(sb) == BTRFS_MAGIC_PARTIAL) { > + if (!(sbflags & SBREAD_PARTIAL)) { > + error("superblock magic doesn't match"); > + return -EIO; > + } > + } > } > > csum_type = btrfs_super_csum_type(sb); > @@ -1747,6 +1761,15 @@ int close_ctree_fs_info(struct btrfs_fs_info *fs_info) > write_ctree_super(trans, root); > btrfs_free_transaction(root, trans); > } > + > + if (fs_info->finalize_on_close) { > + btrfs_set_super_magic(fs_info->super_copy, BTRFS_MAGIC); > + root->fs_info->finalize_on_close = 0; > + ret = write_all_supers(root); > + if (ret) > + fprintf(stderr, > + "failed to write new super block err %d\n", ret); > + } > btrfs_free_block_groups(fs_info); > > free_fs_roots_tree(&fs_info->fs_root_tree); > diff --git a/disk-io.h b/disk-io.h > index 153ef1f49641..a73dede1e8d0 100644 > --- a/disk-io.h > +++ b/disk-io.h > @@ -61,7 +61,10 @@ enum btrfs_open_ctree_flags { > * It's useful for chunk corruption case. > * Makes no sense for open_ctree variants returning btrfs_root. > */ > - OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR = (1 << 11) > + OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR = (1 << 11), > + > + /* Allow to open a partially created filesystem */ > + OPEN_CTREE_FS_PARTIAL = (1 << 12), > }; > > /* > @@ -71,6 +74,12 @@ enum btrfs_read_sb_flags { > SBREAD_DEFAULT = 0, > /* Reading superblock during recovery */ > SBREAD_RECOVER = (1 << 0), > + > + /* > + * Read superblock with the fake signature, cannot be used with > + * SBREAD_RECOVER > + */ > + SBREAD_PARTIAL = (1 << 1), > }; > > static inline u64 btrfs_sb_offset(int mirror) > diff --git a/mkfs.c b/mkfs.c > index 0b081dd44457..ee5a30daff54 100644 > --- a/mkfs.c > +++ b/mkfs.c > @@ -1708,7 +1708,7 @@ int main(int argc, char **argv) > exit(1); > } > > - root = open_ctree(file, 0, OPEN_CTREE_WRITES); > + root = open_ctree(file, 0, OPEN_CTREE_WRITES | OPEN_CTREE_FS_PARTIAL); > if (!root) { > error("open ctree failed"); > close(fd); > @@ -1858,6 +1858,11 @@ int main(int argc, char **argv) > list_all_devices(root); > } > > + /* > + * The filesystem is now fully set up, commit the remaining changes and > + * fix the signature as the last step before closing the devices. > + */ > + root->fs_info->finalize_on_close = 1; > out: > ret = close_ctree(root); > BUG_ON(ret); > diff --git a/utils.c b/utils.c > index e7195b53b015..cec7c738e088 100644 > --- a/utils.c > +++ b/utils.c > @@ -238,6 +238,9 @@ static inline int write_temp_super(int fd, struct btrfs_super_block *sb, > * > * For now sys chunk array will be empty and dev_item is empty too. > * They will be re-initialized at temp chunk tree setup. > + * > + * The superblock signature is not valid, denotes a partially created > + * filesystem, needs to be finalized. > */ > static int setup_temp_super(int fd, struct btrfs_mkfs_config *cfg, > u64 root_bytenr, u64 chunk_bytenr) > @@ -276,7 +279,7 @@ static int setup_temp_super(int fd, struct btrfs_mkfs_config *cfg, > > btrfs_set_super_bytenr(super, cfg->super_bytenr); > btrfs_set_super_num_devices(super, 1); > - btrfs_set_super_magic(super, BTRFS_MAGIC); > + btrfs_set_super_magic(super, BTRFS_MAGIC_PARTIAL); > btrfs_set_super_generation(super, 1); > btrfs_set_super_root(super, root_bytenr); > btrfs_set_super_chunk_root(super, chunk_bytenr); > @@ -1004,6 +1007,9 @@ static int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, > > /* > * @fs_uuid - if NULL, generates a UUID, returns back the new filesystem UUID > + * > + * The superblock signature is not valid, denotes a partially created > + * filesystem, needs to be finalized. > */ > int make_btrfs(int fd, struct btrfs_mkfs_config *cfg, > struct btrfs_convert_context *cctx) > @@ -1064,7 +1070,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg, > > btrfs_set_super_bytenr(&super, cfg->blocks[0]); > btrfs_set_super_num_devices(&super, 1); > - btrfs_set_super_magic(&super, BTRFS_MAGIC); > + btrfs_set_super_magic(&super, BTRFS_MAGIC_PARTIAL); > btrfs_set_super_generation(&super, 1); > btrfs_set_super_root(&super, cfg->blocks[1]); > btrfs_set_super_chunk_root(&super, cfg->blocks[3]); >