All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: John Dorminy <jdorminy@redhat.com>, Hannes Reinecke <hare@suse.de>
Cc: Bob Liu <bob.liu@oracle.com>,
	device-mapper development <dm-devel@redhat.com>,
	Mike Snitzer <snitzer@redhat.com>
Subject: Re: [PATCH 3/4] dm-zoned: V2 metadata handling
Date: Fri, 3 Apr 2020 05:47:09 +0000	[thread overview]
Message-ID: <CO2PR04MB2343FC2FBA8E204A289BBFC4E7C70@CO2PR04MB2343.namprd04.prod.outlook.com> (raw)
In-Reply-To: CAMeeMh8+6ssUyYqiDQL2kWpwPwjPZRQJbLa7bDUL2tYqeUKjOw@mail.gmail.com

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

  reply	other threads:[~2020-04-03  5:47 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27  7:14 [PATCH RFC 0/4] dm-zoned: Metadata V2 Hannes Reinecke
2020-03-27  7:14 ` [PATCH 1/4] dm-zoned: store zone id within the zone structure Hannes Reinecke
2020-03-31  0:57   ` Damien Le Moal
2020-03-31  8:54     ` Hannes Reinecke
2020-03-27  7:14 ` [PATCH 2/4] dm-zoned: use array for superblock zones Hannes Reinecke
2020-03-31  0:51   ` Damien Le Moal
2020-03-31  9:10   ` Bob Liu
2020-03-27  7:14 ` [PATCH 3/4] dm-zoned: V2 metadata handling Hannes Reinecke
2020-03-31  0:54   ` Damien Le Moal
2020-03-31  9:11   ` Bob Liu
2020-04-02 14:53     ` John Dorminy
2020-04-02 15:09       ` Hannes Reinecke
2020-04-02 15:52         ` John Dorminy
2020-04-02 20:01           ` John Dorminy
2020-04-03  5:47             ` Damien Le Moal [this message]
2020-03-27  7:14 ` [PATCH 4/4] dm-zoned: allow for device size smaller than the capacity Hannes Reinecke
2020-03-31  0:49   ` Damien Le Moal
2020-03-31  8:53     ` Hannes Reinecke
2020-04-02  2:45       ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO2PR04MB2343FC2FBA8E204A289BBFC4E7C70@CO2PR04MB2343.namprd04.prod.outlook.com \
    --to=damien.lemoal@wdc.com \
    --cc=bob.liu@oracle.com \
    --cc=dm-devel@redhat.com \
    --cc=hare@suse.de \
    --cc=jdorminy@redhat.com \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.