[dm-devel] [PATCH 3/4] dm-zoned: V2 metadata handling

[Damien Le Moal]

On 2020/04/03 5:01, John Dorminy wrote:
> Actually, I take that back. If it is a worry that the internal representation
> will change, it seems also unsafe to be casting the sixteen bytes as a uuid_t
> when copying into them with uuid_copy(). If the internal representation changed
> to add a new field, then there would be a buffer overrun when using uuid_copy().
> If the internal representation changed the order of the bytes, uuid_copy() would
> propagate that to the on-disk structure and yield compatibility issues.
> 
> Perhaps it would be safer to store the UUIDs on disk as a string. Or add to
> uuid.[ch] a function to get the raw bytes in a fixed order suitable for storing
> on disk?

Please have a look at file systems on-disk metadata definition and checks. There
are plenty of examples of how UUIDs are handled. For instance, XFS has in
fs/xfs/libxfs/xfs_format.h:

typedef struct xfs_dsb {
	...
	uuid_t          sb_uuid;        /* user-visible file system unique id */
	...
}

For the on-disk super block UUID and the same for the in-memory version.
And you can see code like:
memcpy(&to->sb_uuid, &from->sb_uuid, sizeof(to->sb_uuid));
uuid_copy(&to->sb_meta_uuid, &from->sb_uuid);

and checks such as:

BUILD_BUG_ON(sizeof(geo->uuid) != sizeof(sbp->sb_uuid));

for some UUID declared with:

unsigned char   uuid[16];       /* unique id of the filesystem  */
(struct xfs_fsop_geom_v1 in fs/xfs/libxfs/xfs_fs.h.

On the other hand, f2fs defines its on-disk and in-memory super block UUID as:

__u8 uuid[16];                  /* 128-bit uuid for volume */

And copies it with:

memcpy(&sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));

The general pattern is:
* For UUIDs defined as uuid_t, use uuid_copy() or memcpy()
* For UUIDs defined as an array of bytes or char, use memcpy()
* Add BUILD_BUG_ON() checks when mixing both definitions.

There is no need to go as far as using a string for the UUID.

-- 
Damien Le Moal
Western Digital Research