[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]