Re: [Cluster-devel] [PATCH] fsck.gfs2: Don't rely on cluster.conf when rebuilding sb

2013-07-17 Thread Bob Peterson
- Original Message -
| You also want to get rid of this code in RHEL6 btw. It's just broken in
| many different ways.
| 
| Fabio

Perhaps Andy should open a bugzilla record so we can get his patch into RHEL6.5.

Regards,

Bob Peterson
Red Hat File Systems



Re: [Cluster-devel] [gfs2-utils PATCH 0/3] Small gfs2_edit enhancements

2013-07-17 Thread Andrew Price

Hi Bob,

On 17/07/13 16:59, Bob Peterson wrote:

Hi,

This set of three patches enhance gfs2_edit a bit to make it better for
debugging file system problems. The third patch is a bug fix that for
a segfault when directory leaf blocks have very long file names.

[gfs2-utils PATCH 1/3] gfs2_edit: Add new option to print all bitmaps
[gfs2-utils PATCH 2/3] gfs2_edit: display pointer offsets for directories
[gfs2-utils PATCH 3/3] gfs2_edit: fix a segfault with file names > 256


ACK to all three.

Thanks,
Andy



Regards,

Bob Peterson
Red Hat File Systems





Re: [Cluster-devel] [PATCH] fsck.gfs2: Don't rely on cluster.conf when rebuilding sb

2013-07-17 Thread Fabio M. Di Nitto
You also want to get rid of this code in RHEL6 btw. It's just broken in
many different ways.

Fabio

On 07/17/2013 01:51 PM, Andrew Price wrote:
> As cluster.conf no longer exists we can't sniff the locking options from
> it when rebuilding the superblock and in any case we shouldn't assume
> that fsck.gfs2 is running on the cluster the volume belongs to.
> 
> This patch removes the get_lockproto_table function and instead sets the
> lock table name to a placeholder ("unknown") and sets lockproto to
> "lock_dlm".  It warns the user at the end of the run that the locktable
> will need to be set before mounting.
> 
> Signed-off-by: Andrew Price 
> ---
>  gfs2/fsck/initialize.c | 57 
> --
>  gfs2/fsck/main.c   |  4 
>  2 files changed, 8 insertions(+), 53 deletions(-)
> 
> diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> index b01b240..869d2de 100644
> --- a/gfs2/fsck/initialize.c
> +++ b/gfs2/fsck/initialize.c
> @@ -33,6 +33,7 @@ static int was_mounted_ro = 0;
>  static uint64_t possible_root = HIGHEST_BLOCK;
>  static struct master_dir fix_md;
>  static unsigned long long blks_2free = 0;
> +extern int sb_fixed;
>  
>  /**
>   * block_mounters
> @@ -828,58 +829,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
>   return -1;
>  }
>  
> -static int get_lockproto_table(struct gfs2_sbd *sdp)
> -{
> - FILE *fp;
> - char line[PATH_MAX];
> - char *cluname, *end;
> - const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
> -
> - memset(sdp->lockproto, 0, sizeof(sdp->lockproto));
> - memset(sdp->locktable, 0, sizeof(sdp->locktable));
> - fp = fopen(cfgfile, "rt");
> - if (!fp) {
> - /* no cluster.conf; must be a stand-alone file system */
> - strcpy(sdp->lockproto, "lock_nolock");
> - log_warn(_("Lock protocol determined to be: lock_nolock\n"));
> - log_warn(_("Stand-alone file system: No need for a lock "
> -"table.\n"));
> - return 0;
> - }
> - /* We found a cluster.conf so assume it's a clustered file system */
> - log_warn(_("Lock protocol assumed to be: " GFS2_DEFAULT_LOCKPROTO
> -"\n"));
> - strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
> -
> - while (fgets(line, sizeof(line) - 1, fp)) {
> - cluname = strstr(line," - if (end)
> - *end = '\0';
> - break;
> - }
> - }
> - if (cluname == NULL || end == NULL || end - cluname < 1) {
> - log_err(_("Error: Unable to determine cluster name from %s\n"),
> -   cfgfile);
> - } else {
> - fsname = strrchr(opts.device, '/');
> - if (fsname)
> - fsname++;
> - else
> - fsname = "repaired";
> - snprintf(sdp->locktable, sizeof(sdp->locktable), "%.*s:%.16s",
> -  (int)(sizeof(sdp->locktable) - strlen(fsname) - 2),
> -  cluname, fsname);
> - log_warn(_("Lock table determined to be: %s\n"),
> -  sdp->locktable);
> - }
> - fclose(fp);
> - return 0;
> -}
> -
>  /**
>   * is_journal_copy - Is this a "real" dinode or a copy inside a journal?
>   * A real dinode will be located at the block number in its no_addr.
> @@ -1256,7 +1205,8 @@ static int sb_repair(struct gfs2_sbd *sdp)
>   }
>   }
>   /* Step 3 - Rebuild the lock protocol and file system table name */
> - get_lockproto_table(sdp);
> + strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
> + strcpy(sdp->locktable, "unknown");
>   if (query(_("Okay to fix the GFS2 superblock? (y/n)"))) {
>   log_info(_("Found system master directory at: 0x%llx\n"),
>sdp->sd_sb.sb_master_dir.no_addr);
> @@ -1280,6 +1230,7 @@ static int sb_repair(struct gfs2_sbd *sdp)
>   build_sb(sdp, uuid);
>   inode_put(&sdp->md.rooti);
>   inode_put(&sdp->master_dir);
> + sb_fixed = 1;
>   } else {
>   log_crit(_("GFS2 superblock not fixed; fsck cannot proceed "
>  "without a valid superblock.\n"));
> diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c
> index 9c3b06d..f9e7166 100644
> --- a/gfs2/fsck/main.c
> +++ b/gfs2/fsck/main.c
> @@ -36,6 +36,7 @@ struct osi_root dirtree = (struct osi_root) { NULL, };
>  struct osi_root inodetree = (struct osi_root) { NULL, };
>  int dups_found = 0, dups_found_first = 0;
>  struct gfs_sb *sbd1 = NULL;
> +int sb_fixed = 0;
>  
>  /* This function is for libgfs2's sake.  
> */
>  void print_it(const char *label, const char *fmt, const char *fmt2, ...)
> @@ -315,6 +316,9 @@ int main(int argc, char **argv)
>   log_notice( _("Writing changes to disk

[Cluster-devel] [gfs2-utils PATCH 1/3] gfs2_edit: Add new option to print all bitmaps for an rgrp

2013-07-17 Thread Bob Peterson
This patch adds a new keyword 'rgbitmaps' that causes gfs2_edit to
print all bitmaps for the given resource group. This is handy for
analysis of file system fragmentation.
---
 gfs2/edit/hexedit.c | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 281be5a..d67389e 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -2809,7 +2809,7 @@ static void dump_journal(const char *journal)
 /*  */
 static void usage(void)
 {
-   fprintf(stderr,"\nFormat is: gfs2_edit [-c 1] [-V] [-x] [-h] [identify] 
[-z <0-9>] [-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] 
[-z <0-9>] [-p structures|blocks][blocktype][blockalloc 
[val]][blockbits][blockrg][rgcount][rgflags][rgbitmaps][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");
@@ -2820,6 +2820,8 @@ static void usage(void)
fprintf(stderr,"restoremeta - restore metadata for debugging 
(DANGEROUS).\n");
fprintf(stderr,"rgcount - print how many RGs in the file system.\n");
fprintf(stderr,"rgflags rgnum [new flags] - print or modify flags for 
rg #rgnum (0 - X)\n");
+   fprintf(stderr,"rgbitmaps  - print out the bitmaps for rgrp "
+   "rgnum.\n");
fprintf(stderr,"-V   prints version number.\n");
fprintf(stderr,"-c 1 selects alternate color scheme 1\n");
fprintf(stderr,"-d   prints details (for printing journals)\n");
@@ -3097,6 +3099,29 @@ static void process_parameters(int argc, char *argv[], 
int pass)
gfs2_rgrp_free(&sbd.rgtree);
exit(EXIT_SUCCESS);
}
+   } else if (!strcmp(argv[i], "rgbitmaps")) {
+   int rg, bmap;
+   uint64_t rgblk;
+   struct rgrp_tree *rgd;
+
+   i++;
+   if (i >= argc - 1) {
+   printf("Error: rg # not specified.\n");
+   printf("Format is: %s rgbitmaps rgnum\n",
+  argv[0]);
+   gfs2_rgrp_free(&sbd.rgtree);
+   exit(EXIT_FAILURE);
+   }
+   rg = atoi(argv[i]);
+   rgblk = get_rg_addr(rg);
+   rgd = gfs2_blk2rgrpd(&sbd, rgblk);
+   if (rgd == NULL) {
+   printf("Error: rg # is invalid.\n");
+   gfs2_rgrp_free(&sbd.rgtree);
+   exit(EXIT_FAILURE);
+   }
+   for (bmap = 0; bmap < rgd->ri.ri_length; bmap++)
+   push_block(rgblk + bmap);
}
else if (!strcasecmp(argv[i], "savemeta")) {
getgziplevel(argv, &i);
@@ -3177,7 +3202,8 @@ int main(int argc, char *argv[])
if (termlines)
interactive_mode();
else { /* print all the structures requested */
-   for (i = 0; i <= blockhist; i++) {
+   i = 0;
+   while (blockhist > 0) {
block = blockstack[i + 1].block;
if (!block)
break;
@@ -3189,6 +3215,7 @@ int main(int argc, char *argv[])
eol(0);
}
block = pop_block();
+   i++;
}
}
close(fd);
-- 
1.8.3.1



[Cluster-devel] [gfs2-utils PATCH 0/3] Small gfs2_edit enhancements

2013-07-17 Thread Bob Peterson
Hi,

This set of three patches enhance gfs2_edit a bit to make it better for
debugging file system problems. The third patch is a bug fix that for
a segfault when directory leaf blocks have very long file names.

[gfs2-utils PATCH 1/3] gfs2_edit: Add new option to print all bitmaps
[gfs2-utils PATCH 2/3] gfs2_edit: display pointer offsets for directories
[gfs2-utils PATCH 3/3] gfs2_edit: fix a segfault with file names > 256

Regards,

Bob Peterson
Red Hat File Systems



[Cluster-devel] [gfs2-utils PATCH 3/3] gfs2_edit: fix a segfault with file names > 255 bytes

2013-07-17 Thread Bob Peterson
This patch fixes a segfault in gfs2_edit caused by trying to print
file names longer than 255 bytes.
---
 gfs2/edit/gfs2hex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
index 6979cd9..cc0ceb6 100644
--- a/gfs2/edit/gfs2hex.c
+++ b/gfs2/edit/gfs2hex.c
@@ -122,7 +122,7 @@ void eol(int col) /* end of line */
 void __attribute__((format (printf, 1, 2))) print_gfs2(const char *fmt, ...)
 {
va_list args;
-   char string[NAME_MAX];
+   char string[PATH_MAX];

memset(string, 0, sizeof(string));
va_start(args, fmt);
-- 
1.8.3.1



[Cluster-devel] [gfs2-utils PATCH 2/3] gfs2_edit: display pointer offsets for directory dinodes

2013-07-17 Thread Bob Peterson
This patch prints "pointer X" on the hex display for directory
dinodes like it does for indirect blocks.
---
 gfs2/edit/hexedit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index d67389e..f0d9789 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -954,7 +954,8 @@ static int hexdump(uint64_t startaddr, int len)
if (cursor_line) {
if (block_type == GFS2_METATYPE_IN ||
((block_type == GFS2_METATYPE_DI) &&
-((struct gfs2_dinode*)bh->b_data)->di_height)) {
+((struct gfs2_dinode*)bh->b_data)->di_height) ||
+S_ISDIR(di.di_mode)) {
int ptroffset = edit_row[dmode] * 16 +
edit_col[dmode];
 
-- 
1.8.3.1



Re: [Cluster-devel] [PATCH] fsck.gfs2: Don't rely on cluster.conf when rebuilding sb

2013-07-17 Thread Bob Peterson
- Original Message -
| As cluster.conf no longer exists we can't sniff the locking options from
| it when rebuilding the superblock and in any case we shouldn't assume
| that fsck.gfs2 is running on the cluster the volume belongs to.
| 
| This patch removes the get_lockproto_table function and instead sets the
| lock table name to a placeholder ("unknown") and sets lockproto to
| "lock_dlm".  It warns the user at the end of the run that the locktable
| will need to be set before mounting.
| 
| Signed-off-by: Andrew Price 
| ---
|  gfs2/fsck/initialize.c | 57
|  --
|  gfs2/fsck/main.c   |  4 
|  2 files changed, 8 insertions(+), 53 deletions(-)
| 
| diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
| index b01b240..869d2de 100644
| --- a/gfs2/fsck/initialize.c
| +++ b/gfs2/fsck/initialize.c
| @@ -33,6 +33,7 @@ static int was_mounted_ro = 0;
|  static uint64_t possible_root = HIGHEST_BLOCK;
|  static struct master_dir fix_md;
|  static unsigned long long blks_2free = 0;
| +extern int sb_fixed;
|  
|  /**
|   * block_mounters
| @@ -828,58 +829,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
|   return -1;
|  }
|  
| -static int get_lockproto_table(struct gfs2_sbd *sdp)
| -{
| - FILE *fp;
| - char line[PATH_MAX];
| - char *cluname, *end;
| - const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
| -
| - memset(sdp->lockproto, 0, sizeof(sdp->lockproto));
| - memset(sdp->locktable, 0, sizeof(sdp->locktable));
| - fp = fopen(cfgfile, "rt");
| - if (!fp) {
| - /* no cluster.conf; must be a stand-alone file system */
| - strcpy(sdp->lockproto, "lock_nolock");
| - log_warn(_("Lock protocol determined to be: lock_nolock\n"));
| - log_warn(_("Stand-alone file system: No need for a lock "
| -"table.\n"));
| - return 0;
| - }
| - /* We found a cluster.conf so assume it's a clustered file system */
| - log_warn(_("Lock protocol assumed to be: " GFS2_DEFAULT_LOCKPROTO
| -"\n"));
| - strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
| -
| - while (fgets(line, sizeof(line) - 1, fp)) {
| - cluname = strstr(line,"locktable, sizeof(sdp->locktable), "%.*s:%.16s",
| -  (int)(sizeof(sdp->locktable) - strlen(fsname) - 2),
| -  cluname, fsname);
| - log_warn(_("Lock table determined to be: %s\n"),
| -  sdp->locktable);
| - }
| - fclose(fp);
| - return 0;
| -}
| -
|  /**
|   * is_journal_copy - Is this a "real" dinode or a copy inside a journal?
|   * A real dinode will be located at the block number in its no_addr.
| @@ -1256,7 +1205,8 @@ static int sb_repair(struct gfs2_sbd *sdp)
|   }
|   }
|   /* Step 3 - Rebuild the lock protocol and file system table name */
| - get_lockproto_table(sdp);
| + strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
| + strcpy(sdp->locktable, "unknown");
|   if (query(_("Okay to fix the GFS2 superblock? (y/n)"))) {
|   log_info(_("Found system master directory at: 0x%llx\n"),
|sdp->sd_sb.sb_master_dir.no_addr);
| @@ -1280,6 +1230,7 @@ static int sb_repair(struct gfs2_sbd *sdp)
|   build_sb(sdp, uuid);
|   inode_put(&sdp->md.rooti);
|   inode_put(&sdp->master_dir);
| + sb_fixed = 1;
|   } else {
|   log_crit(_("GFS2 superblock not fixed; fsck cannot proceed "
|  "without a valid superblock.\n"));
| diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c
| index 9c3b06d..f9e7166 100644
| --- a/gfs2/fsck/main.c
| +++ b/gfs2/fsck/main.c
| @@ -36,6 +36,7 @@ struct osi_root dirtree = (struct osi_root) { NULL, };
|  struct osi_root inodetree = (struct osi_root) { NULL, };
|  int dups_found = 0, dups_found_first = 0;
|  struct gfs_sb *sbd1 = NULL;
| +int sb_fixed = 0;
|  
|  /* This function is for libgfs2's sake.
|  */
|  void print_it(const char *label, const char *fmt, const char *fmt2, ...)
| @@ -315,6 +316,9 @@ int main(int argc, char **argv)
|   log_notice( _("Writing changes to disk\n"));
|   fsync(sdp->device_fd);
|   destroy(sdp);
| + if (sb_fixed)
| + log_warn(_("Superblock was reset. Use tunegfs2 to manually "
| +"set lock table before mounting.\n"));
|   log_notice( _("gfs2_fsck complete\n"));
|  
|   if (!error) {
| --
| 1.8.1.4

Hi,

ACK. I'll pull patch1 of my set based on this. Thanks, Andy.

Regards,

Bob Peterson
Red Hat File Systems



Re: [Cluster-devel] [PATCH] fsck.gfs2: Don't rely on cluster.conf when rebuilding sb

2013-07-17 Thread Steven Whitehouse
Hi,

I think that looks like a good solution. Seems to be a bit smaller
code-wise too,

Steve.

On Wed, 2013-07-17 at 12:51 +0100, Andrew Price wrote:
> As cluster.conf no longer exists we can't sniff the locking options from
> it when rebuilding the superblock and in any case we shouldn't assume
> that fsck.gfs2 is running on the cluster the volume belongs to.
> 
> This patch removes the get_lockproto_table function and instead sets the
> lock table name to a placeholder ("unknown") and sets lockproto to
> "lock_dlm".  It warns the user at the end of the run that the locktable
> will need to be set before mounting.
> 
> Signed-off-by: Andrew Price 
> ---
>  gfs2/fsck/initialize.c | 57 
> --
>  gfs2/fsck/main.c   |  4 
>  2 files changed, 8 insertions(+), 53 deletions(-)
> 
> diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> index b01b240..869d2de 100644
> --- a/gfs2/fsck/initialize.c
> +++ b/gfs2/fsck/initialize.c
> @@ -33,6 +33,7 @@ static int was_mounted_ro = 0;
>  static uint64_t possible_root = HIGHEST_BLOCK;
>  static struct master_dir fix_md;
>  static unsigned long long blks_2free = 0;
> +extern int sb_fixed;
>  
>  /**
>   * block_mounters
> @@ -828,58 +829,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
>   return -1;
>  }
>  
> -static int get_lockproto_table(struct gfs2_sbd *sdp)
> -{
> - FILE *fp;
> - char line[PATH_MAX];
> - char *cluname, *end;
> - const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
> -
> - memset(sdp->lockproto, 0, sizeof(sdp->lockproto));
> - memset(sdp->locktable, 0, sizeof(sdp->locktable));
> - fp = fopen(cfgfile, "rt");
> - if (!fp) {
> - /* no cluster.conf; must be a stand-alone file system */
> - strcpy(sdp->lockproto, "lock_nolock");
> - log_warn(_("Lock protocol determined to be: lock_nolock\n"));
> - log_warn(_("Stand-alone file system: No need for a lock "
> -"table.\n"));
> - return 0;
> - }
> - /* We found a cluster.conf so assume it's a clustered file system */
> - log_warn(_("Lock protocol assumed to be: " GFS2_DEFAULT_LOCKPROTO
> -"\n"));
> - strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
> -
> - while (fgets(line, sizeof(line) - 1, fp)) {
> - cluname = strstr(line," - if (end)
> - *end = '\0';
> - break;
> - }
> - }
> - if (cluname == NULL || end == NULL || end - cluname < 1) {
> - log_err(_("Error: Unable to determine cluster name from %s\n"),
> -   cfgfile);
> - } else {
> - fsname = strrchr(opts.device, '/');
> - if (fsname)
> - fsname++;
> - else
> - fsname = "repaired";
> - snprintf(sdp->locktable, sizeof(sdp->locktable), "%.*s:%.16s",
> -  (int)(sizeof(sdp->locktable) - strlen(fsname) - 2),
> -  cluname, fsname);
> - log_warn(_("Lock table determined to be: %s\n"),
> -  sdp->locktable);
> - }
> - fclose(fp);
> - return 0;
> -}
> -
>  /**
>   * is_journal_copy - Is this a "real" dinode or a copy inside a journal?
>   * A real dinode will be located at the block number in its no_addr.
> @@ -1256,7 +1205,8 @@ static int sb_repair(struct gfs2_sbd *sdp)
>   }
>   }
>   /* Step 3 - Rebuild the lock protocol and file system table name */
> - get_lockproto_table(sdp);
> + strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
> + strcpy(sdp->locktable, "unknown");
>   if (query(_("Okay to fix the GFS2 superblock? (y/n)"))) {
>   log_info(_("Found system master directory at: 0x%llx\n"),
>sdp->sd_sb.sb_master_dir.no_addr);
> @@ -1280,6 +1230,7 @@ static int sb_repair(struct gfs2_sbd *sdp)
>   build_sb(sdp, uuid);
>   inode_put(&sdp->md.rooti);
>   inode_put(&sdp->master_dir);
> + sb_fixed = 1;
>   } else {
>   log_crit(_("GFS2 superblock not fixed; fsck cannot proceed "
>  "without a valid superblock.\n"));
> diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c
> index 9c3b06d..f9e7166 100644
> --- a/gfs2/fsck/main.c
> +++ b/gfs2/fsck/main.c
> @@ -36,6 +36,7 @@ struct osi_root dirtree = (struct osi_root) { NULL, };
>  struct osi_root inodetree = (struct osi_root) { NULL, };
>  int dups_found = 0, dups_found_first = 0;
>  struct gfs_sb *sbd1 = NULL;
> +int sb_fixed = 0;
>  
>  /* This function is for libgfs2's sake.  
> */
>  void print_it(const char *label, const char *fmt, const char *fmt2, ...)
> @@ -315,6 +316,9 @@ int main(int argc, char **argv)
>   log_notice( _("Writing changes t

[Cluster-devel] [PATCH] fsck.gfs2: Don't rely on cluster.conf when rebuilding sb

2013-07-17 Thread Andrew Price
As cluster.conf no longer exists we can't sniff the locking options from
it when rebuilding the superblock and in any case we shouldn't assume
that fsck.gfs2 is running on the cluster the volume belongs to.

This patch removes the get_lockproto_table function and instead sets the
lock table name to a placeholder ("unknown") and sets lockproto to
"lock_dlm".  It warns the user at the end of the run that the locktable
will need to be set before mounting.

Signed-off-by: Andrew Price 
---
 gfs2/fsck/initialize.c | 57 --
 gfs2/fsck/main.c   |  4 
 2 files changed, 8 insertions(+), 53 deletions(-)

diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index b01b240..869d2de 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -33,6 +33,7 @@ static int was_mounted_ro = 0;
 static uint64_t possible_root = HIGHEST_BLOCK;
 static struct master_dir fix_md;
 static unsigned long long blks_2free = 0;
+extern int sb_fixed;
 
 /**
  * block_mounters
@@ -828,58 +829,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
return -1;
 }
 
-static int get_lockproto_table(struct gfs2_sbd *sdp)
-{
-   FILE *fp;
-   char line[PATH_MAX];
-   char *cluname, *end;
-   const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
-
-   memset(sdp->lockproto, 0, sizeof(sdp->lockproto));
-   memset(sdp->locktable, 0, sizeof(sdp->locktable));
-   fp = fopen(cfgfile, "rt");
-   if (!fp) {
-   /* no cluster.conf; must be a stand-alone file system */
-   strcpy(sdp->lockproto, "lock_nolock");
-   log_warn(_("Lock protocol determined to be: lock_nolock\n"));
-   log_warn(_("Stand-alone file system: No need for a lock "
-  "table.\n"));
-   return 0;
-   }
-   /* We found a cluster.conf so assume it's a clustered file system */
-   log_warn(_("Lock protocol assumed to be: " GFS2_DEFAULT_LOCKPROTO
-  "\n"));
-   strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
-
-   while (fgets(line, sizeof(line) - 1, fp)) {
-   cluname = strstr(line,"locktable, sizeof(sdp->locktable), "%.*s:%.16s",
-(int)(sizeof(sdp->locktable) - strlen(fsname) - 2),
-cluname, fsname);
-   log_warn(_("Lock table determined to be: %s\n"),
-sdp->locktable);
-   }
-   fclose(fp);
-   return 0;
-}
-
 /**
  * is_journal_copy - Is this a "real" dinode or a copy inside a journal?
  * A real dinode will be located at the block number in its no_addr.
@@ -1256,7 +1205,8 @@ static int sb_repair(struct gfs2_sbd *sdp)
}
}
/* Step 3 - Rebuild the lock protocol and file system table name */
-   get_lockproto_table(sdp);
+   strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
+   strcpy(sdp->locktable, "unknown");
if (query(_("Okay to fix the GFS2 superblock? (y/n)"))) {
log_info(_("Found system master directory at: 0x%llx\n"),
 sdp->sd_sb.sb_master_dir.no_addr);
@@ -1280,6 +1230,7 @@ static int sb_repair(struct gfs2_sbd *sdp)
build_sb(sdp, uuid);
inode_put(&sdp->md.rooti);
inode_put(&sdp->master_dir);
+   sb_fixed = 1;
} else {
log_crit(_("GFS2 superblock not fixed; fsck cannot proceed "
   "without a valid superblock.\n"));
diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c
index 9c3b06d..f9e7166 100644
--- a/gfs2/fsck/main.c
+++ b/gfs2/fsck/main.c
@@ -36,6 +36,7 @@ struct osi_root dirtree = (struct osi_root) { NULL, };
 struct osi_root inodetree = (struct osi_root) { NULL, };
 int dups_found = 0, dups_found_first = 0;
 struct gfs_sb *sbd1 = NULL;
+int sb_fixed = 0;
 
 /* This function is for libgfs2's sake.  */
 void print_it(const char *label, const char *fmt, const char *fmt2, ...)
@@ -315,6 +316,9 @@ int main(int argc, char **argv)
log_notice( _("Writing changes to disk\n"));
fsync(sdp->device_fd);
destroy(sdp);
+   if (sb_fixed)
+   log_warn(_("Superblock was reset. Use tunegfs2 to manually "
+  "set lock table before mounting.\n"));
log_notice( _("gfs2_fsck complete\n"));
 
if (!error) {
-- 
1.8.1.4



Re: [Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

2013-07-17 Thread Rusty Russell
Steven Whitehouse  writes:
> Hi,
>
> On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
>> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
>> 
>> Signed-off-by: Sachin Kamat 
>> ---
>> Compile tested and based on the following tree:
>> git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
>> 
>> Dependent on [1]
>> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
>> ---
>>  fs/gfs2/inode.c |3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
>> index bbb2715..a186ebd 100644
>> --- a/fs/gfs2/inode.c
>> +++ b/fs/gfs2/inode.c
>> @@ -19,6 +19,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  
>>  #include "gfs2.h"
>> @@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct 
>> dentry *dentry,
>>  }
>>  gfs2_glock_dq_uninit(ghs);
>>  if (IS_ERR(d))
>> -return PTR_RET(d);
>> +return PTR_ERR_OR_ZERO(d);
>
> I'm not sure I follow what this is supposed to be doing... what is the
> reason for this change? This macro/function doesn't seem to be defined
> in the current kernel, so I assume that it is "coming soon" but the
> thread pointed to above wasn't very enlightening,

It's a clarification rename.

But this fix is wrong, it should just be changed to PTR_ERR(d).  It
never needed PTR_RET() in the first place.

Thanks,
Rusty.



Re: [Cluster-devel] [gfs2-utils PATCH 1/7] fsck.gfs2: Fix reference to uninitialized variable

2013-07-17 Thread Steven Whitehouse
Hi,

On Wed, 2013-07-17 at 07:30 +0200, Fabio M. Di Nitto wrote:
> On 07/16/2013 02:56 PM, Bob Peterson wrote:
> > This patch initializes a variable so that it no longer references
> > it uninitialized.
> > 
> > rhbz#984085
> > ---
> >  gfs2/fsck/initialize.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> > index b01b240..936fd5e 100644
> > --- a/gfs2/fsck/initialize.c
> > +++ b/gfs2/fsck/initialize.c
> > @@ -832,7 +832,7 @@ static int get_lockproto_table(struct gfs2_sbd *sdp)
> >  {
> > FILE *fp;
> > char line[PATH_MAX];
> > -   char *cluname, *end;
> > +   char *cluname, *end = NULL;
> > const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
> 
> Just spotted this reference to cluster.conf ^^ remember it doesn't exist
> anymore in the new era.
> 
> Fabio
> 

Yes, agreed. It looks like this is trying to repair the superblock, but
I don't think that it is a good idea to assume that lack of cluster.conf
means that the fs is not clustered. Nor should be be trying to use the
lock table name from cluster.conf since we might be repairing a
filesystem on a different cluster to the one on which it was mounted and
supposed to work, so it might just result in putting incorrect
information in to this field. Perhaps better to set it to lock_dlm with
a cluster name of unknown and then prompt the user to use gfs2tune to
reset the cluster name afterwards, or something like that,

Steve.




Re: [Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

2013-07-17 Thread Steven Whitehouse
Hi,

On Tue, 2013-07-16 at 16:05 +0930, Rusty Russell wrote:
> Steven Whitehouse  writes:
> > Hi,
> >
> > On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
> >> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
> >> 
> >> Signed-off-by: Sachin Kamat 
> >> ---
> >> Compile tested and based on the following tree:
> >> git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
> >> 
> >> Dependent on [1]
> >> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
> >> ---
> >>  fs/gfs2/inode.c |3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> >> index bbb2715..a186ebd 100644
> >> --- a/fs/gfs2/inode.c
> >> +++ b/fs/gfs2/inode.c
> >> @@ -19,6 +19,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  
> >>  #include "gfs2.h"
> >> @@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct 
> >> dentry *dentry,
> >>}
> >>gfs2_glock_dq_uninit(ghs);
> >>if (IS_ERR(d))
> >> -  return PTR_RET(d);
> >> +  return PTR_ERR_OR_ZERO(d);
> >
> > I'm not sure I follow what this is supposed to be doing... what is the
> > reason for this change? This macro/function doesn't seem to be defined
> > in the current kernel, so I assume that it is "coming soon" but the
> > thread pointed to above wasn't very enlightening,
> 
> It's a clarification rename.
> 
> But this fix is wrong, it should just be changed to PTR_ERR(d).  It
> never needed PTR_RET() in the first place.
> 
> Thanks,
> Rusty.

Ok, thanks for clarifying. I've sorted out a patch, attached below,
which I'll put in my tree unless there are any objections. I don't think
that it should affect your patch series, but let me know if there is a
problem,

Steve.

>From 1067f2a5b96d11c2c3dd5ba83e3969cc5ed51b50 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse 
Date: Wed, 17 Jul 2013 08:11:32 +0100
Subject: [PATCH] GFS2: Fix typo in gfs2_create_inode()

PTR_RET should be PTR_ERR

Reported-by: Sachin Kamat 
Cc: Rusty Russell 
Signed-off-by: Steven Whitehouse 

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index bbb2715..a01b8fd 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -594,7 +594,7 @@ static int gfs2_create_inode(struct inode *dir, struct 
dentry *dentry,
}
gfs2_glock_dq_uninit(ghs);
if (IS_ERR(d))
-   return PTR_RET(d);
+   return PTR_ERR(d);
return error;
} else if (error != -ENOENT) {
goto fail_gunlock;
-- 
1.7.4