xfs
[Top] [All Lists]

Re: [PATCH 1/2] xfs: create new metadata UUID field and incompat flag

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 1/2] xfs: create new metadata UUID field and incompat flag
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 12 May 2015 07:49:02 +1000
Cc: Brian Foster <bfoster@xxxxxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5550DE64.2030907@xxxxxxxxxxx>
References: <554D1F56.7030209@xxxxxxxxxx> <554D2110.8030405@xxxxxxxxxxx> <20150511161404.GA54361@xxxxxxxxxxxxxxx> <5550DE64.2030907@xxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, May 11, 2015 at 11:52:52AM -0500, Eric Sandeen wrote:
> On 5/11/15 11:14 AM, Brian Foster wrote:
> > On Fri, May 08, 2015 at 03:48:16PM -0500, Eric Sandeen wrote:
> >> > This adds a new superblock field, sb_meta_uuid.  If set, along with
> >> > a new incompat flag, the code will use that field on a V5 filesystem
> >> > to compare to metadata UUIDs, which allows us to change the user-
> >> > visible UUID at will.  Userspace handles the setting or clearing
> >> > of the incompat flag as appropriate, as the UUID gets written.
> >> > 
> >> > If the incompat flag is not set, copy the user-visible UUID into
> >> > into the meta_uuid slot in memory; this is not written back to disk in
> >> > this case.
> >> > 
> >> > The remainder of this patch simply switches verifiers, initializers,
> >> > etc to use the new sb_meta_uuid field.
> >> > 
> >> > Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> >> > ---
> > This all looks fine to me. The only thing that confuses me is why we
> > continue to use sb_uuid for the log. Eric points out on irc that we
> > can't modify the uuid when the log is dirty, so we can't actually break
> > anything in this manner. In other words, we effectively already handle a
> > modified uuid with respect to the log.
> > 
> > That said, it seems inconsistent to me to create a metadata uuid field
> > and not use it for all metadata. My expectation from such a change is to
> > see sb_uuid now used only for user facing bits (e.g., mount and get geo
> > related) and sb_meta_uuid used everywhere else. Instead, we use sb_uuid
> > for the user facing bits, sb_meta_uuid for the internal bits that can't
> > handle a uuid change and sb_uuid for the internal bits that can.
> > 
> > I suppose I could be convinced otherwise if there's context I'm missing,
> > but otherwise this seems a bit confusing to me...
> > 
> > Brian
> 
> Yeah, I'm sympathetic to that argument.  I can look into using sb_meta_uuid
> for the log too, if the incompat flag is set.

The log uuid is user facing, like the superblock uuid. i.e. there
are tools that identify external log devices by checking that the
uuid in the log matches the user facing uuid in the superblock.
Hence the log uuid needs to use the uuid that is reported to
userspace.

> I had skipped it just because that use of the UUID predated the CRC changes.

Changing the uuid results in the log being zeroed, so the uuid in
the log can be changed without issue, even if we are using CRCs.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>