[Cluster-devel] [PATCH 2/3] gfs2_edit: Fix find_mtype and support gfs1 structures

[Steven Whitehouse]

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 at 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.