[Cluster-devel] [PATCH] gfs2_edit: Add compression to savemeta and restoremeta

Andrew Price anprice at redhat.com
Thu Apr 28 16:40:31 UTC 2011


On 04/28/2011 03:55 PM, Steven Whitehouse wrote:
> Hi,
>
> Some comments...
>
> On Thu, 2011-04-14 at 18:08 +0100, Andrew Price wrote:
>> savemeta now outputs a gzip-compressed file by default and there is a
>> -nocompress option to turn off compression. restoremeta can restore from a
>> gzip-compressed or raw metadata file without any extra options.
>>
>> The file opening, closing and writing code from savemeta has been abstracted
>> away into savemeta{open,close,write} functions to abstract away compressed and
>> non-compressed file output.
>>
>> Adds a dependency on zlib.
>>
>> rhbz#695767
>>
> Is there a way to control the compression level? Something like the gzip
> -1 to -9 options would be good. It should probably default to -9

Yeah that's doable. Considering how the existing opt/arg parsing code 
works, I would prefer to replace the -nocompress option with -z <N> and 
default to -z 9. Then -z 0 could mean the same as -nocompress.

>> Signed-off-by: Andrew Price<anprice at redhat.com>
>> ---
>>   configure.ac          |    1 +
>>   gfs2/edit/Makefile.am |    4 +-
>>   gfs2/edit/hexedit.c   |   16 +++--
>>   gfs2/edit/hexedit.h   |    2 +-
>>   gfs2/edit/savemeta.c  |  211 +++++++++++++++++++++++++++++++++++-------------
>>   gfs2/man/gfs2_edit.8  |   42 +++++-----
>>   6 files changed, 190 insertions(+), 86 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 10bcc95..3fc02d0 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -119,6 +119,7 @@ PKG_CHECK_MODULES([cfg],[libcfg])
>>   PKG_CHECK_MODULES([fenced],[libfenced])
>>   PKG_CHECK_MODULES([dlmcontrol],[libdlmcontrol])
>>   PKG_CHECK_MODULES([quorum],[libquorum])
>> +PKG_CHECK_MODULES([zlib],[zlib])
>>
>>   # old versions of ncurses don't ship pkg-config files
>>   PKG_CHECK_MODULES([ncurses],[ncurses],,
>> diff --git a/gfs2/edit/Makefile.am b/gfs2/edit/Makefile.am
>> index 9701c91..c4814dc 100644
>> --- a/gfs2/edit/Makefile.am
>> +++ b/gfs2/edit/Makefile.am
>> @@ -10,8 +10,8 @@ gfs2_edit_CPPFLAGS	= -D_FILE_OFFSET_BITS=64 -DHELPER_PROGRAM \
>>   			  -I$(top_srcdir)/gfs2/include \
>>   			  -I$(top_srcdir)/gfs2/libgfs2
>>
>> -gfs2_edit_CFLAGS	= $(ncurses_CFLAGS)
>> +gfs2_edit_CFLAGS	= $(ncurses_CFLAGS) $(zlib_CFLAGS)
>>
>> -gfs2_edit_LDFLAGS	= $(ncurses_LIBS)
>> +gfs2_edit_LDFLAGS	= $(ncurses_LIBS) $(zlib_LIBS)
>>
>>   gfs2_edit_LDADD		= $(top_builddir)/gfs2/libgfs2/libgfs2.la
>> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
>> index d8f8841..8f0630b 100644
>> --- a/gfs2/edit/hexedit.c
>> +++ b/gfs2/edit/hexedit.c
>> @@ -41,6 +41,7 @@ struct gfs2_log_header *llh;
>>   struct gfs2_log_descriptor *lld;
>>   int pgnum;
>>   int details = 0;
>> +int compressmeta = 1;
>>
>>   int display(int identify_only);
>>
>> @@ -3294,7 +3295,7 @@ static void dump_journal(const char *journal)
>>   /* ------------------------------------------------------------------------ */
>>   static void usage(void)
>>   {
>> -	fprintf(stderr,"\nFormat is: gfs2_edit [-c 1] [-V] [-x] [-h] [identify] [-p structures|blocks][blocktype][blockalloc [val]][blockbits][blockrg][find sb|rg|rb|di|in|lf|jd|lh|ld|ea|ed|lb|13|qc][field<f>[val]] /dev/device\n\n");
>> +	fprintf(stderr,"\nFormat is: gfs2_edit [-c 1] [-V] [-x] [-h] [identify] [-nocompress] [-p structures|blocks][blocktype][blockalloc [val]][blockbits][blockrg][find sb|rg|rb|di|in|lf|jd|lh|ld|ea|ed|lb|13|qc][field<f>[val]] /dev/device\n\n");
>>   	fprintf(stderr,"If only the device is specified, it enters into hexedit mode.\n");
>>   	fprintf(stderr,"identify - prints out only the block type, not the details.\n");
>>   	fprintf(stderr,"printsavedmeta - prints out the saved metadata blocks from a savemeta file.\n");
>> @@ -3335,6 +3336,7 @@ static void usage(void)
>>   	fprintf(stderr,"-p<b>  find sb|rg|rb|di|in|lf|jd|lh|ld|ea|ed|lb|"
>>   		"13|qc - find block of given type after block<b>\n");
>>   	fprintf(stderr,"<b>  specifies the starting block for search\n");
>> +	fprintf(stderr,"-nocompress  don't compress savemeta/savergs output\n");
>>   	fprintf(stderr,"-s   specifies a starting block such as root, rindex, quota, inum.\n");
>>   	fprintf(stderr,"-x   print in hexmode.\n");
>>   	fprintf(stderr,"-h   prints this help.\n\n");
>> @@ -3361,8 +3363,8 @@ static void usage(void)
>>   	fprintf(stderr,"     gfs2_edit -p quota find di /dev/x/y\n");
>>   	fprintf(stderr,"   To set the Resource Group flags for rg #7 to 3.\n");
>>   	fprintf(stderr,"     gfs2_edit rgflags 7 3 /dev/sdc2\n");
>> -	fprintf(stderr,"   To save off all metadata for /dev/vg/lv:\n");
>> -	fprintf(stderr,"     gfs2_edit savemeta /dev/vg/lv /tmp/metasave\n");
>> +	fprintf(stderr,"   To save off all metadata for /dev/vg/lv without compression:\n");
>> +	fprintf(stderr,"     gfs2_edit -nocompress savemeta /dev/vg/lv /tmp/metasave\n");
>>   }/* usage */
>>
>>   /* ------------------------------------------------------------------------ */
>> @@ -3395,6 +3397,8 @@ static void parameterpass1(int argc, char *argv[], int i)
>>   	else if (!strcasecmp(argv[i], "-d") ||
>>   		 !strcasecmp(argv[i], "-details"))
>>   		details = 1;
>> +	else if (!strcasecmp(argv[i], "-nocompress"))
>> +		compressmeta = 0;
>>   	else if (!strcasecmp(argv[i], "savemeta"))
>>   		termlines = 0;
>>   	else if (!strcasecmp(argv[i], "savemetaslow"))
>> @@ -3559,11 +3563,11 @@ static void process_parameters(int argc, char *argv[], int pass)
>>   			}
>>   		}
>>   		else if (!strcasecmp(argv[i], "savemeta"))
>> -			savemeta(argv[i+2], 0);
>> +			savemeta(argv[i+2], 0, compressmeta);
>>   		else if (!strcasecmp(argv[i], "savemetaslow"))
>> -			savemeta(argv[i+2], 1);
>> +			savemeta(argv[i+2], 1, compressmeta);
>>   		else if (!strcasecmp(argv[i], "savergs"))
>> -			savemeta(argv[i+2], 2);
>> +			savemeta(argv[i+2], 2, compressmeta);
>>   		else if (isdigit(argv[i][0])) { /* decimal addr */
>>   			sscanf(argv[i], "%"SCNd64,&temp_blk);
>>   			push_block(temp_blk);
>> diff --git a/gfs2/edit/hexedit.h b/gfs2/edit/hexedit.h
>> index a7a3109..53a6670 100644
>> --- a/gfs2/edit/hexedit.h
>> +++ b/gfs2/edit/hexedit.h
>> @@ -341,7 +341,7 @@ extern void gfs_log_header_in(struct gfs_log_header *head,
>>   			      struct gfs2_buffer_head *bh);
>>   extern void gfs_log_header_print(struct gfs_log_header *lh);
>>   extern void gfs_dinode_in(struct gfs_dinode *di, struct gfs2_buffer_head *bh);
>> -extern void savemeta(char *out_fn, int saveoption);
>> +extern void savemeta(char *out_fn, int saveoption, int compressmeta);
>>   extern void restoremeta(const char *in_fn, const char *out_device,
>>   			uint64_t printblocksonly);
>>   extern int display(int identify_only);
>> diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
>> index 4d9f591..b96537e 100644
>> --- a/gfs2/edit/savemeta.c
>> +++ b/gfs2/edit/savemeta.c
>> @@ -18,6 +18,7 @@
>>   #include<limits.h>
>>   #include<sys/time.h>
>>   #include<linux/gfs2_ondisk.h>
>> +#include<zlib.h>
>>
>>   #include "osi_list.h"
>>   #include "gfs2hex.h"
>> @@ -34,6 +35,13 @@ struct saved_metablock {
>>   	char buf[BUFSIZE];
>>   };
>>
>> +struct metafd {
>> +	int fd;
>> +	gzFile gzfd;
>> +	const char *filename;
>> +	int compressed;
>> +};
>> +
>>   struct saved_metablock *savedata;
>>   uint64_t last_fs_block, last_reported_block, blks_saved, total_out, pct;
>>   uint64_t journal_blocks[MAX_JOURNALS_SAVED];
>> @@ -193,7 +201,98 @@ static void warm_fuzzy_stuff(uint64_t wfsblock, int force)
>>   	}
>>   }
>>
>> -static int save_block(int fd, int out_fd, uint64_t blk)
>> +/**
>> + * Open a file and prepare it for writing by savemeta()
>> + * out_fn: the path to the file, which will be truncated if it exists
>> + * compressed: 0 - do not compress the file, 1 - compress the file
>> + * Returns a struct metafd containing the opened file descriptor
>> + */
>> +static struct metafd savemetaopen(char *out_fn, int compressed)
>> +{
>> +	struct metafd mfd;
>> +
>> +	if (!out_fn) {
>> +		out_fn = strdup(DFT_SAVE_FILE);
> why strdup? I can't see where this memory is freed

This code was already in savemeta(); I just moved it into the new 
function. I guess the strdup was used because mkstemp() requires a 
modifiable char array. I'll polish this up in my next patch revision.

> Otherwise looks good,
>
> Steve.

Cheers,

Andy



More information about the Cluster-devel mailing list