On Tue, Aug 20, 2019 at 05:12:49PM +0900, Damien Le Moal wrote:
> zonefs is a very simple file system exposing each zone of a zoned
> block device as a file. zonefs is in fact closer to a raw block device
> access interface than to a full feature POSIX file system.
/me cuts out a lot of stuff...
> +/*
> + * When a write error occurs in a sequenial zone, the zone write pointer
"sequential"
> + * position must be refreshed to correct the file size and zonefs inode
> + * write pointer offset.
> + */
> +static int zonefs_seq_file_write_failed(struct inode *inode, int error)
> +{
> + struct super_block *sb = inode->i_sb;
> + struct zonefs_inode_info *zi = ZONEFS_I(inode);
> + sector_t sector = zi->i_zsector;
> + unsigned int nofs_flag;
> + struct blk_zone zone;
> + int n = 1, ret;
> + loff_t pos;
<snip again>
> +/*
> + * Read super block information from the device.
> + */
> +static int zonefs_read_super(struct super_block *sb)
> +{
> + struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> + struct zonefs_super *super;
> + struct bio bio;
> + struct bio_vec bio_vec;
> + struct page *page;
> + int ret;
> +
> + page = alloc_page(GFP_KERNEL);
> + if (!page)
> + return -ENOMEM;
> +
> + bio_init(&bio, &bio_vec, 1);
> + bio.bi_iter.bi_sector = 0;
> + bio_set_dev(&bio, sb->s_bdev);
> + bio_set_op_attrs(&bio, REQ_OP_READ, 0);
> + bio_add_page(&bio, page, PAGE_SIZE, 0);
> +
> + ret = submit_bio_wait(&bio);
> + if (ret)
> + goto out;
> +
> + ret = -EINVAL;
> + super = page_address(page);
> + if (le32_to_cpu(super->s_magic) != ZONEFS_MAGIC)
> + goto out;
> + sbi->s_features = le64_to_cpu(super->s_features);
> + if (zonefs_has_feature(sbi, ZONEFS_F_UID)) {
> + sbi->s_uid = make_kuid(current_user_ns(),
> + le32_to_cpu(super->s_uid));
> + if (!uid_valid(sbi->s_uid))
> + goto out;
> + }
> + if (zonefs_has_feature(sbi, ZONEFS_F_GID)) {
> + sbi->s_gid = make_kgid(current_user_ns(),
> + le32_to_cpu(super->s_gid));
> + if (!gid_valid(sbi->s_gid))
> + goto out;
> + }
> + if (zonefs_has_feature(sbi, ZONEFS_F_PERM))
> + sbi->s_perm = le32_to_cpu(super->s_perm);
Unknown feature bits are silently ignored. Is that intentional? Will
all features be compat features? I would find it a little annoying to
format (for example) a F_UID filesystem only to have some old kernel
driver ignore it.
> +
> + uuid_copy(&sbi->s_uuid, &super->s_uuid);
> + ret = 0;
> +
> +out:
> + __free_page(page);
> +
> + return ret;
> +}
<snip>
> +/*
> + * On-disk super block (block 0).
> + */
> +struct zonefs_super {
> +
> + /* Magic number */
> + __le32 s_magic;
> +
> + /* Features */
> + __le64 s_features;
> +
> + /* 128-bit uuid */
> + uuid_t s_uuid;
> +
> + /* UID/GID to use for files */
> + __le32 s_uid;
> + __le32 s_gid;
> +
> + /* File permissions */
> + __le32 s_perm;
> +
> + /* Padding to 4K */
> + __u8 s_reserved[4056];
Hmm, I noticed that fill_super doesn't check that s_reserved is actually
zero (or any specific value). You might consider enforcing that so that
future you can add fields beyond s_perm without having to burn a
s_features bit every time you do it.
Also a little surprised there's no checksum field here to detect bit
flips and such. ;)
--D
> +
> +} __packed;
> +
> +/*
> + * Feature flags: used on disk in the s_features field of struct zonefs_super
> + * and in-memory in the s_feartures field of struct zonefs_sb_info.
> + */
> +enum zonefs_features {
> + /*
> + * Use a zone start sector value as file name.
> + */
> + ZONEFS_F_STARTSECT_NAME,
> + /*
> + * Aggregate contiguous conventional zones into a single file.
> + */
> + ZONEFS_F_AGRCNV,
> + /*
> + * Use super block specified UID for files instead of default.
> + */
> + ZONEFS_F_UID,
> + /*
> + * Use super block specified GID for files instead of default.
> + */
> + ZONEFS_F_GID,
> + /*
> + * Use super block specified file permissions instead of default 640.
> + */
> + ZONEFS_F_PERM,
> +};
> +
> +/*
> + * In-memory Super block information.
> + */
> +struct zonefs_sb_info {
> +
> + unsigned long long s_features;
> + kuid_t s_uid; /* File owner UID */
> + kgid_t s_gid; /* File owner GID */
> + umode_t s_perm; /* File permissions */
> + uuid_t s_uuid;
> +
> + loff_t s_blocksize_mask;
> + unsigned int s_nr_zones[ZONEFS_ZTYPE_MAX];
> +};
> +
> +static inline struct zonefs_sb_info *ZONEFS_SB(struct super_block *sb)
> +{
> + return sb->s_fs_info;
> +}
> +
> +static inline bool zonefs_has_feature(struct zonefs_sb_info *sbi,
> + enum zonefs_features f)
> +{
> + return sbi->s_features & (1ULL << f);
> +}
> +
> +#define zonefs_info(sb, format, args...) \
> + pr_info("zonefs (%s): " format, sb->s_id, ## args)
> +#define zonefs_err(sb, format, args...) \
> + pr_err("zonefs (%s) ERROR: " format, sb->s_id, ## args)
> +#define zonefs_warn(sb, format, args...) \
> + pr_warn("zonefs (%s) WARN: " format, sb->s_id, ## args)
> +
> +#endif
> diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
> index 1274c692e59c..3be20c774142 100644
> --- a/include/uapi/linux/magic.h
> +++ b/include/uapi/linux/magic.h
> @@ -86,6 +86,7 @@
> #define NSFS_MAGIC 0x6e736673
> #define BPF_FS_MAGIC 0xcafe4a11
> #define AAFS_MAGIC 0x5a3c69f0
> +#define ZONEFS_MAGIC 0x5a4f4653
>
> /* Since UDF 2.01 is ISO 13346 based... */
> #define UDF_SUPER_MAGIC 0x15013346
> --
> 2.21.0
>
