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

[Andrew Price]

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

> +		&&  m[n].mh_type == mtype)
> +			return&m[n];
>   		n++;
>   	} while (n<  lgfs2_metadata_size);
>
> @@ -934,7 +935,7 @@ static int hexdump(uint64_t startaddr, int len)
>   		}
>   		print_gfs2("] ");
>   		if (print_field>= 0) {
> -			const struct lgfs2_metadata *m = find_mtype(get_block_type(bh));
> +			const struct lgfs2_metadata *m = find_mtype(get_block_type(bh), sbd.gfs1);
>   			if (m) {
>   				const struct lgfs2_metafield *f;
>   				unsigned n;