Re: [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 01/08/2018 01:25 PM, Qu Wenruo wrote:


On 2018年01月08日 13:13, Anand Jain wrote:


On 01/08/2018 01:08 PM, Qu Wenruo wrote:


On 2018年01月08日 13:04, Anand Jain wrote:
Userland sets SUPER_FLAG_CHANGING_FSID and resets it only when changing
fsid is complete. Its not a good idea to mount the device anything in
between, so this patch fails the mount if SB SUPER_FLAG_CHANGING_FSID
is set.

Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
    cc: wqu@xxxxxxxx
---
v1->v2: Oops. Fix cut and paste error, remove ~. My bad.

   fs/btrfs/disk-io.c              | 7 ++++++-
   include/uapi/linux/btrfs_tree.h | 1 +
   2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a69e5944dc08..0dd215258ff9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -61,7 +61,8 @@
                    BTRFS_HEADER_FLAG_RELOC |\
                    BTRFS_SUPER_FLAG_ERROR |\
                    BTRFS_SUPER_FLAG_SEEDING |\
-                 BTRFS_SUPER_FLAG_METADUMP)
+                 BTRFS_SUPER_FLAG_METADUMP |\
+                 BTRFS_SUPER_FLAG_CHANGING_FSID)

Still the same problem here.

Since that super flag is excluded in btrfs_check_super_valid(), I didn't
see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
confuse later BTRFS_SUPER_FLAG_SUPP users.


  Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
  just warns. So when there is a new flag which is not supported
  we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
  BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not supported.
  However since we are supporting BTRFS_SUPER_FLAG_CHANGING_FSID, so
  adding to BTRFS_SUPER_FLAG_SUPP make sense to me. ?

We still refuse to mount the fs if CHANGING_FSID is set, which I believe
should be called "unsupported"
>
On the other hand it would be better to include METADUMP_V2 into SUPP,
as we allow it to be mounted and now kernel knows that flag, output a
warning doesn't seems necessary now.

It would be even better to change the later SUPP flags checks, into
something not only output some meaningless numeric flags, but also
output the human readable flags if it's recognized but unsupported. (For
CHANGIND_FSID).

 Originally [1] looks like there isn't any design specific reason not to
 fail the mount (which I didn't want to alter to fail) but just to warn.
 Will change it to fail.

[1]
 commit 319e4d0661e5323c9f9945f0f8fb5905e5fe74c3
    btrfs: Enhance super validation check

Thanks, Anand

Thanks,
Qu


Thanks, Anand


Thanks,
Qu

     static const struct extent_io_ops btree_extent_io_ops;
   static void end_workqueue_fn(struct btrfs_work *work);
@@ -3906,6 +3907,10 @@ static int btrfs_check_super_valid(struct
btrfs_fs_info *fs_info)
           btrfs_err(fs_info, "no valid FS found");
           ret = -EINVAL;
       }
+    if (btrfs_super_flags(sb) & BTRFS_SUPER_FLAG_CHANGING_FSID) {
+        btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
+        ret = -EINVAL;
+    }
       if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
           btrfs_warn(fs_info, "unrecognized super flag: %llu",
                   btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
diff --git a/include/uapi/linux/btrfs_tree.h
b/include/uapi/linux/btrfs_tree.h
index 38ab0e06259a..aff1356c2bb8 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -457,6 +457,7 @@ struct btrfs_free_space_header {
   #define BTRFS_SUPER_FLAG_SEEDING    (1ULL << 32)
   #define BTRFS_SUPER_FLAG_METADUMP    (1ULL << 33)
   #define BTRFS_SUPER_FLAG_METADUMP_V2    (1ULL << 34)
+#define BTRFS_SUPER_FLAG_CHANGING_FSID    (1ULL << 35)
       /*



--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux