From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Fri, 25 May 2012 11:40:07 +0100
Subject: [Cluster-devel] [PATCH 2/3] gfs2_edit: Fix find_mtype and
 support gfs1 structures
In-Reply-To: <4FBF5EAF.1070309@redhat.com>
References: <1337940424-20662-1-git-send-email-anprice@redhat.com>
	<1337940424-20662-2-git-send-email-anprice@redhat.com>
	<4FBF5EAF.1070309@redhat.com>
Message-ID: <1337942407.2692.5.camel@menhir>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

On Fri, 2012-05-25 at 11:27 +0100, Andrew Price wrote:
> On 25/05/12 11:07, Andrew Price wrote:
> > Fixes up find_mtype to really iterate over the metadata array and to
> > choose between gfs2 and gfs1 structures.
> >
> > Signed-off-by: Andrew Price<anprice@redhat.com>
> > ---
> >   gfs2/edit/hexedit.c |    9 +++++----
> >   1 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> > index 69d1940..8311534 100644
> > --- a/gfs2/edit/hexedit.c
> > +++ b/gfs2/edit/hexedit.c
> > @@ -813,14 +813,15 @@ int display_block_type(int from_restore)
> >   	return ret_type;
> >   }
> >
> > -static const struct lgfs2_metadata *find_mtype(uint32_t mtype)
> > +static const struct lgfs2_metadata *find_mtype(uint32_t mtype, int gfs1)
> >   {
> >   	const struct lgfs2_metadata *m = lgfs2_metadata;
> >   	unsigned n = 0;
> >
> >   	do {
> > -		if (m->gfs2&&  m->mh_type == mtype)
> > -			return m;
> > +		if (((gfs1&&  m[n].gfs1) || (!gfs1&&  m[n].gfs2))
> 
> I'm wondering if we should represent the version metadata as flags 
> instead of separate gfs1 and gfs2 fields and define some constants so we 
> can do the comparison in one go, e.g.
> 
>                  if ((m[n].versions & versions) ...
> 
> Where versions is some combination of defined constants passed into the 
> function like LGFS2_VER_1 | LGFS2_VER_2
> 
> But that has its own drawbacks I guess so I'm probably just splitting 
> hairs. What do you think?
> 
> Andy
> 
I think that makes sense. I don't remember now why I did that with a
bitfield and not flags, but it is probably better to convert it to
flags. We'll want to give them a longer name though, as everything in
the library will land up with a LGFS2_ or lgfs2_ prefix in the end,

Steve.