From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Thu, 28 Apr 2011 17:40:31 +0100 Subject: [Cluster-devel] [PATCH] gfs2_edit: Add compression to savemeta and restoremeta In-Reply-To: <1304002521.2649.16.camel@dolmen> References: <1302800939-21883-1-git-send-email-anprice@redhat.com> <1304002521.2649.16.camel@dolmen> Message-ID: <4DB9987F.4090407@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 and default to -z 9. Then -z 0 could mean the same as -nocompress. >> Signed-off-by: Andrew Price >> --- >> 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[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[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 find sb|rg|rb|di|in|lf|jd|lh|ld|ea|ed|lb|" >> "13|qc - find block of given type after block\n"); >> fprintf(stderr," 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 >> #include >> #include >> +#include >> >> #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