xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/2] xfs: create new metadata UUID field and incompat flag
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Tue, 12 May 2015 07:11:45 -0400
Cc: Eric Sandeen <sandeen@xxxxxxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150511214902.GG15721@dastard>
References: <554D1F56.7030209@xxxxxxxxxx> <554D2110.8030405@xxxxxxxxxxx> <20150511161404.GA54361@xxxxxxxxxxxxxxx> <5550DE64.2030907@xxxxxxxxxxx> <20150511214902.GG15721@dastard>
User-agent: Mutt/1.5.23 (2014-03-12)
On Tue, May 12, 2015 at 07:49:02AM +1000, Dave Chinner wrote:
> 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.
> 

Ok, sounds good. Thanks for the explanation.

Brian

> > 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
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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