[Cluster-devel] [PATCH v5 2/3] GFS2: Introduce new gfs2_log_header_v2

[Andrew Price <anprice@redhat.com>]

On 19/01/18 14:39, Steven Whitehouse wrote:
> On 19/01/18 14:31, Andrew Price wrote:
>> On 18/01/18 16:04, Andreas Gruenbacher wrote:> diff --git 
>> a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
>>> index 09f0920f07e9..7eb73c32272f 100644
>>> --- a/include/uapi/linux/gfs2_ondisk.h
>>> +++ b/include/uapi/linux/gfs2_ondisk.h
>>> @@ -403,7 +403,15 @@ struct gfs2_ea_header {
>>> ?? * Log header structure
>>> ?? */
>>> ? -#define GFS2_LOG_HEAD_UNMOUNT??? 0x00000001??? /* log is clean */
>>> +#define GFS2_LOG_HEAD_UNMOUNT??????? 0x00000001 /* log is clean */
>>> +#define GFS2_LOG_HEAD_FLUSH_NORMAL??? 0x00000002 /* normal log flush */
>>> +#define GFS2_LOG_HEAD_FLUSH_SYNC??? 0x00000004 /* Sync log flush */
>>> +#define GFS2_LOG_HEAD_FLUSH_SHUTDOWN??? 0x00000008 /* Shutdown log 
>>> flush */
>>> +#define GFS2_LOG_HEAD_FLUSH_FREEZE??? 0x00000010 /* Freeze flush */
>>> +#define GFS2_LOG_HEAD_RECOVERY??????? 0x00000020 /* Journal recovery */
>>> +#define GFS2_LOG_HEAD_USERSPACE??????? 0x80000000 /* Written by 
>>> gfs2-utils */
>>> +
>>> +#define LH_V1_SIZE (offsetofend(struct gfs2_log_header, lh_hash))
>>> ? ? struct gfs2_log_header {
>>> ????? struct gfs2_meta_header lh_header;
>>> @@ -413,6 +421,21 @@ struct gfs2_log_header {
>>> ????? __be32 lh_tail;??????? /* Block number of log tail */
>>> ????? __be32 lh_blkno;
>>> ????? __be32 lh_hash;
>>> +
>>> +??? /* Version 2 additional fields start here */
>>> +??? __be32 lh_crc;??????? /* crc32 of whole block with this field 0 */
>>> +??? __be32 lh_nsec;??????? /* Nano second time stamp */
>>> +??? __be64 lh_sec;??????? /* Second based time stamp */
>>> +??? __be64 lh_addr;??????? /* Block addr of this log header 
>>> (absolute) */
>>> +??? __be64 lh_jinode;??? /* Journal inode number */
>>> +??? __be64 lh_statfs_addr;??? /* Local statfs inode number */
>>> +??? __be64 lh_quota_addr;??? /* Local quota change inode number */
>>> +
>>> +??? /* Statfs local changes (i.e. diff from global statfs) */
>>> +??? __be64 lh_local_total;
>>> +??? __be64 lh_local_free;
>>> +??? __be64 lh_local_dinodes;
>>> +??? __be32 lh_log_origin;??? /* The origin of this log header */
>>> ? };
>>
>> 4 bytes of padding gets added at the end of the struct. Could it be 
>> made explicit with a __pad field? It currently breaks the metadata 
>> description test with "gfs2_log_header: size mismatch between struct 
>> 128 and fields 124".
>>
>> I've pushed a gfs2-utils patch with basic support for the new log 
>> header format to the andyp-lhv2 branch. I need to test it some more 
>> but it should be useful for testing these patches already. I'll 
>> finalise it once the v2 patches are in for-next.
>>
>> Andy
>>
> It could, however nothing follows the structure, so it isn't generally 
> needed, and I suspect that the test was expecting all the structures to 
> be of fixed size (which all the others are). 

Yes, the test is checking for holes in the structure while also checking 
that the metadata description is complete.

> Still, it might be the 
> easiest way to fix that test unless we can find another solution?

Well we could update the metadata description so that we can declare 
intentional holes that it shouldn't try to dereference. Something like 
the patch below should do it. But if adding the __pad field doesn't 
cause any issues, I don't see why we shouldn't do it that way.

Andy

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 85ac74cb..a09a4b37 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -130,6 +130,7 @@ struct lgfs2_metafield {
  #define LGFS2_MFF_NSECS    0x04000     /* Units are nsecs */
  #define LGFS2_MFF_MAJOR    0x08000     /* Major device number */
  #define LGFS2_MFF_MINOR    0x10000     /* Minor device number */
+#define LGFS2_MFF_HOLE     0x20000     /* Minor device number */

         /* If it is a pointer, then this field must be set */
         const unsigned points_to;
diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
index e7a2226a..163d9d38 100644
--- a/gfs2/libgfs2/meta.c
+++ b/gfs2/libgfs2/meta.c
@@ -118,7 +118,10 @@ const unsigned int lgfs2_ld1_type_size = 
ARRAY_SIZE(lgfs2_ld1_types);

  #define INR(f,...) RF(f.no_formal_ino) \
                    RFP(f.no_addr, __VA_ARGS__)
-
+#define HOLE(sz, prevf) \
+              { .name = "(hole)", \
+                .offset = offsetof(struct STRUCT, prevf) + 
sizeof(((struct STRUCT *)(0))->prevf), \
+                .length = sz, .flags = LGFS2_MFF_HOLE },
  #define ANY_COMMON_BLOCK (1 << LGFS2_MT_DIR_LEAF) | \
                          (1 << LGFS2_MT_JRNL_DATA) | \
                          (1 << LGFS2_MT_EA_ATTR) | \
@@ -375,6 +378,7 @@ F(lh_local_total, .flags = LGFS2_MFF_FSBLOCKS)
  F(lh_local_free, .flags = LGFS2_MFF_FSBLOCKS)
  F(lh_local_dinodes, .flags = LGFS2_MFF_FSBLOCKS)
  F(lh_log_origin)
+HOLE(4, lh_log_origin)
  #endif
  };