[PATCH 0/5] VFS API for getting filesystem's UUID

[PATCH 0/5] VFS API for getting filesystem's UUID

[Amir Goldstein]

Miklos,

As you observed, the sb->s_uuid field is not always filled by filesystems.

Consumers, like overlayfs, that wish to use this field can check if is
zeroed out as an indication for valid value.

Christoph suggested to make the test more explicit and require the
filesystems that fill the s_uuid field to set a super block flag.

Do you agree with the proposed API?

The first patch in the series defines the flag.
If you push this patch through your tree to Al or Linus, then filesystem
maintainers could later pick the individual patches to their trees.

The xfs patch is based on a patch I already sent to Darrick for filling
out the s_uuid field.

Thanks,
Amir.

Amir Goldstein (5):
  vfs: define a flag to indicate sb->s_uuid is available
  ext4: set the super block SB_I_HAVE_UUID flag
  f2fs: set the super block SB_I_HAVE_UUID flag
  ocfs2: set the super block SB_I_HAVE_UUID flag
  xfs: set the super block SB_I_HAVE_UUID flag

 fs/ext4/super.c    | 1 +
 fs/f2fs/super.c    | 1 +
 fs/ocfs2/super.c   | 1 +
 fs/xfs/xfs_mount.c | 1 +
 include/linux/fs.h | 3 +++
 5 files changed, 7 insertions(+)

-- 
2.7.4

[PATCH 1/5] vfs: define a flag to indicate sb->s_uuid is available

[Amir Goldstein]

Overlayfs would like to make use of underlying filesystem's
sb->s_uuid, but not all filesytems fill this field.

Define a flag to be set by filesystems that do fill the s_uuid
field, so let consumers like overlayfs know that the content
of this field is valid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 include/linux/fs.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 80daadf..de913b4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1272,6 +1272,9 @@ struct mm_struct;
 /* sb->s_iflags to limit user namespace mounts */
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
 
+/* sb->s_iflags for optional information available in super_block struct */
+#define SB_I_HAVE_UUID			0x00000100 /* s_uuid */
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
-- 
2.7.4

[PATCH 2/5] ext4: set the super block SB_I_HAVE_UUID flag

[Amir Goldstein]

Let consumers (i.e. overlayfs) know that the content of the field
sb->s_uuid is valid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/ext4/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2e03a0a..98e8fb0 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3925,6 +3925,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
 #endif
 	memcpy(sb->s_uuid, es->s_uuid, sizeof(es->s_uuid));
+	sb->s_iflags |= SB_I_HAVE_UUID;
 
 	INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
 	mutex_init(&sbi->s_orphan_lock);
-- 
2.7.4

[PATCH 3/5] f2fs: set the super block SB_I_HAVE_UUID flag

[Amir Goldstein]

Let consumers (i.e. overlayfs) know that the content of the field
sb->s_uuid is valid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/f2fs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 96fe8ed..9dca352 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1911,6 +1911,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
 		(test_opt(sbi, POSIX_ACL) ? MS_POSIXACL : 0);
 	memcpy(sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
+	sb->s_iflags |= SB_I_HAVE_UUID;
 
 	/* init f2fs-specific super block info */
 	sbi->valid_super_block = valid_super_block;
-- 
2.7.4

[PATCH 4/5] ocfs2: set the super block SB_I_HAVE_UUID flag

[Amir Goldstein]

Let consumers (i.e. overlayfs) know that the content of the field
sb->s_uuid is valid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/ocfs2/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index ca1646f..9c5e701 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2064,6 +2064,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	sb->s_maxbytes = ocfs2_max_file_offset(bbits, cbits);
 	memcpy(sb->s_uuid, di->id2.i_super.s_uuid,
 	       sizeof(di->id2.i_super.s_uuid));
+	sb->s_iflags |= SB_I_HAVE_UUID;
 
 	osb->osb_dx_mask = (1 << (cbits - bbits)) - 1;
 
-- 
2.7.4

[PATCH 5/5] xfs: set the super block SB_I_HAVE_UUID flag

[Amir Goldstein]

Let consumers (i.e. overlayfs) know that the content of the field
sb->s_uuid is valid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/xfs_mount.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 29e45a0..bb1d325 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -103,6 +103,7 @@ xfs_uuid_mount(
 	/* Publish UUID in struct super_block */
 	BUILD_BUG_ON(sizeof(mp->m_super->s_uuid) != sizeof(uuid_t));
 	memcpy(&mp->m_super->s_uuid, uuid, sizeof(uuid_t));
+	mp->m_super->s_iflags |= SB_I_HAVE_UUID;
 	return 0;
 
  out_duplicate:
-- 
2.7.4

Re: [PATCH 0/5] VFS API for getting filesystem's UUID

[Miklos Szeredi]

On Thu, Apr 27, 2017 at 10:59 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> Miklos,
>
> As you observed, the sb->s_uuid field is not always filled by filesystems.
>
> Consumers, like overlayfs, that wish to use this field can check if is
> zeroed out as an indication for valid value.
>
> Christoph suggested to make the test more explicit and require the
> filesystems that fill the s_uuid field to set a super block flag.
>
> Do you agree with the proposed API?

Sure.

Thanks,
Miklos

Re: [PATCH 1/5] vfs: define a flag to indicate sb->s_uuid is available

[Darrick J. Wong]

On Thu, Apr 27, 2017 at 11:59:30AM +0300, Amir Goldstein wrote:
> Overlayfs would like to make use of underlying filesystem's
> sb->s_uuid, but not all filesytems fill this field.
> 
> Define a flag to be set by filesystems that do fill the s_uuid
> field, so let consumers like overlayfs know that the content
> of this field is valid.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  include/linux/fs.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 80daadf..de913b4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1272,6 +1272,9 @@ struct mm_struct;
>  /* sb->s_iflags to limit user namespace mounts */
>  #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
>  
> +/* sb->s_iflags for optional information available in super_block struct */
> +#define SB_I_HAVE_UUID			0x00000100 /* s_uuid */

SB_I_CAN_HAZ_UUID, heh. :)

There seems to be demand for a "Yes I really filled this out" flag, so
for the vfs and xfs parts,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

(I wonder why we can't just leave s_uuid as nil and to heck with anyone
who deliberately sets their fs to have a nil uuid (like we do now), but
whatever... :))

--D

> +
>  /* Possible states of 'frozen' field */
>  enum {
>  	SB_UNFROZEN = 0,		/* FS is unfrozen */
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] VFS API for getting filesystem's UUID

[Dave Chinner]

On Thu, Apr 27, 2017 at 11:59:29AM +0300, Amir Goldstein wrote:
> Miklos,
> 
> As you observed, the sb->s_uuid field is not always filled by filesystems.
> 
> Consumers, like overlayfs, that wish to use this field can check if is
> zeroed out as an indication for valid value.
> 
> Christoph suggested to make the test more explicit and require the
> filesystems that fill the s_uuid field to set a super block flag.
> 
> Do you agree with the proposed API?
> 
> The first patch in the series defines the flag.
> If you push this patch through your tree to Al or Linus, then filesystem
> maintainers could later pick the individual patches to their trees.
> 
> The xfs patch is based on a patch I already sent to Darrick for filling
> out the s_uuid field.
> 
> Thanks,
> Amir.
> 
> Amir Goldstein (5):
>   vfs: define a flag to indicate sb->s_uuid is available
>   ext4: set the super block SB_I_HAVE_UUID flag
>   f2fs: set the super block SB_I_HAVE_UUID flag
>   ocfs2: set the super block SB_I_HAVE_UUID flag
>   xfs: set the super block SB_I_HAVE_UUID flag
> 
>  fs/ext4/super.c    | 1 +
>  fs/f2fs/super.c    | 1 +
>  fs/ocfs2/super.c   | 1 +
>  fs/xfs/xfs_mount.c | 1 +
>  include/linux/fs.h | 3 +++
>  5 files changed, 7 insertions(+)

Doesn't this make the struct export_opierations .get_uuid method
somewhat redundant?  Shouldn't that now be replaced with generic
functions that looks at SB_I_HAVE_UUID before allowing PNFS export
is allowed and then just use s_uuid directly in the PNFS code?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/5] VFS API for getting filesystem's UUID

[Al Viro]

On Sun, Apr 30, 2017 at 09:01:59AM +1000, Dave Chinner wrote:

> Doesn't this make the struct export_opierations .get_uuid method
> somewhat redundant?  Shouldn't that now be replaced with generic
> functions that looks at SB_I_HAVE_UUID before allowing PNFS export
> is allowed and then just use s_uuid directly in the PNFS code?

Do we even need a flag?  Checking if 16 bytes are not all zeroes isn't hard,
after all...

Re: [PATCH 0/5] VFS API for getting filesystem's UUID

[Amir Goldstein]

On Sun, Apr 30, 2017 at 2:01 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Thu, Apr 27, 2017 at 11:59:29AM +0300, Amir Goldstein wrote:
>> Miklos,
>>
>> As you observed, the sb->s_uuid field is not always filled by filesystems.
>>
>> Consumers, like overlayfs, that wish to use this field can check if is
>> zeroed out as an indication for valid value.
>>
>> Christoph suggested to make the test more explicit and require the
>> filesystems that fill the s_uuid field to set a super block flag.
>>
>> Do you agree with the proposed API?
>>
>> The first patch in the series defines the flag.
>> If you push this patch through your tree to Al or Linus, then filesystem
>> maintainers could later pick the individual patches to their trees.
>>
>> The xfs patch is based on a patch I already sent to Darrick for filling
>> out the s_uuid field.
>>
>> Thanks,
>> Amir.
>>
>> Amir Goldstein (5):
>>   vfs: define a flag to indicate sb->s_uuid is available
>>   ext4: set the super block SB_I_HAVE_UUID flag
>>   f2fs: set the super block SB_I_HAVE_UUID flag
>>   ocfs2: set the super block SB_I_HAVE_UUID flag
>>   xfs: set the super block SB_I_HAVE_UUID flag
>>
>>  fs/ext4/super.c    | 1 +
>>  fs/f2fs/super.c    | 1 +
>>  fs/ocfs2/super.c   | 1 +
>>  fs/xfs/xfs_mount.c | 1 +
>>  include/linux/fs.h | 3 +++
>>  5 files changed, 7 insertions(+)
>
> Doesn't this make the struct export_opierations .get_uuid method
> somewhat redundant?  Shouldn't that now be replaced with generic
> functions that looks at SB_I_HAVE_UUID before allowing PNFS export
> is allowed and then just use s_uuid directly in the PNFS code?
>

There's the tiny issue that get_uuid also returns the offset of the uuid
on disk. Not sure how much that really matters for PNFS?

Amir.

Re: [PATCH 0/5] VFS API for getting filesystem's UUID

[Amir Goldstein]

On Sun, Apr 30, 2017 at 3:54 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Apr 30, 2017 at 09:01:59AM +1000, Dave Chinner wrote:
>
>> Doesn't this make the struct export_opierations .get_uuid method
>> somewhat redundant?  Shouldn't that now be replaced with generic
>> functions that looks at SB_I_HAVE_UUID before allowing PNFS export
>> is allowed and then just use s_uuid directly in the PNFS code?
>
> Do we even need a flag?  Checking if 16 bytes are not all zeroes isn't hard,
> after all...

Nope, it's not hard and that is what we do now.
I posted the flag per Christoph's suggestion to see what others  have to say.

Amir.