[Cluster-devel] [PATCH 2/3] GFS2: Fix race during filesystem mount

2011-07-14 Thread Steven Whitehouse
There is a potential race during filesystem mounting which has recently
been reported. It occurs when the userland gfs_controld is able to
process requests fast enough that it tries to use the sysfs interface
before the lock module is properly initialised. This is a pretty
unusual case as normally the lock module initialisation is very quick
compared with gfs_controld.

This patch adds an interruptible completion which is used to ensure that
userland will wait for the initialisation of the lock module to
complete.

There are other potential solutions to this problem, but this is the
quickest at this stage and has been tested both with and without
mount.gfs2 present in the system.

Signed-off-by: Steven Whitehouse swhit...@redhat.com
Reported-by: David Booher dboo...@adams.net

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 0a064e9..81206e7 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -17,6 +17,7 @@
 #include linux/buffer_head.h
 #include linux/rcupdate.h
 #include linux/rculist_bl.h
+#include linux/completion.h
 
 #define DIO_WAIT   0x0010
 #define DIO_METADATA   0x0020
@@ -546,6 +547,7 @@ struct gfs2_sbd {
struct gfs2_glock *sd_trans_gl;
wait_queue_head_t sd_glock_wait;
atomic_t sd_glock_disposal;
+   struct completion sd_locking_init;
 
/* Inode Stuff */
 
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 8ac9ae1..2a77071 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -72,6 +72,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
 
init_waitqueue_head(sdp-sd_glock_wait);
atomic_set(sdp-sd_glock_disposal, 0);
+   init_completion(sdp-sd_locking_init);
spin_lock_init(sdp-sd_statfs_spin);
 
spin_lock_init(sdp-sd_rindex_spin);
@@ -1017,11 +1018,13 @@ hostdata_error:
fsname++;
if (lm-lm_mount == NULL) {
fs_info(sdp, Now mounting FS...\n);
+   complete(sdp-sd_locking_init);
return 0;
}
ret = lm-lm_mount(sdp, fsname);
if (ret == 0)
fs_info(sdp, Joined cluster. Now mounting FS...\n);
+   complete(sdp-sd_locking_init);
return ret;
 }
 
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index e20eab3..443cabc 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -338,6 +338,9 @@ static ssize_t lkfirst_store(struct gfs2_sbd *sdp, const 
char *buf, size_t len)
rv = sscanf(buf, %u, first);
if (rv != 1 || first  1)
return -EINVAL;
+   rv = wait_for_completion_killable(sdp-sd_locking_init);
+   if (rv)
+   return rv;
spin_lock(sdp-sd_jindex_spin);
rv = -EBUSY;
if (test_bit(SDF_NOJOURNALID, sdp-sd_flags) == 0)
@@ -414,7 +417,9 @@ static ssize_t jid_store(struct gfs2_sbd *sdp, const char 
*buf, size_t len)
rv = sscanf(buf, %d, jid);
if (rv != 1)
return -EINVAL;
-
+   rv = wait_for_completion_killable(sdp-sd_locking_init);
+   if (rv)
+   return rv;
spin_lock(sdp-sd_jindex_spin);
rv = -EINVAL;
if (sdp-sd_lockstruct.ls_ops-lm_mount == NULL)
-- 
1.7.4



[Cluster-devel] GFS2: Pre-pull patch posting (fixes)

2011-07-14 Thread Steven Whitehouse
Hi,

The following three fixes should help ensure that 3.0 is the best
release yet, GFS2-wise at least. I've had the first two queued for
some time waiting for the final one of this set. All three are relatively
short, although the third is a bit longer than the others, mainly
due to the extra comments added describing the inode eviction process,

Steve.



[Cluster-devel] [PATCH 3/3] GFS2: Resolve inode eviction and ail list interaction bug

2011-07-14 Thread Steven Whitehouse
This patch contains a few misc fixes which resolve a recently
reported issue. This patch has been a real team effort and has
received a lot of testing.

The first issue is that the ail lock needs to be held over a few
more operations. The lock thats added into gfs2_releasepage() may
possibly be a candidate for replacing with RCU at some future
point, but at this stage we've gone for the obvious fix.

The second issue is that gfs2_write_inode() can end up calling
a glock recursively when called from gfs2_evict_inode() via the
syncing code, so it needs a guard added.

The third issue is that we either need to not truncate the metadata
pages of inodes which have zero link count, but which we cannot
deallocate due to them still being in use by other nodes, or we need
to ensure that those pages have all made it through the journal and
ail lists first. This patch takes the former approach, but the
latter has also been tested and there is nothing to choose between
them performance-wise. So again, we could revise that decision
in the future.

Also, the inode eviction process is now better documented.

Signed-off-by: Steven Whitehouse swhit...@redhat.com
Tested-by: Bob Peterson rpete...@redhat.com
Tested-by: Abhijith Das a...@redhat.com
Reported-by: Barry J. Marson bmar...@redhat.com
Reported-by: David Teigland teigl...@redhat.com

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 802ac5e..f9fbbe9 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -1069,6 +1069,7 @@ int gfs2_releasepage(struct page *page, gfp_t gfp_mask)
return 0;
 
gfs2_log_lock(sdp);
+   spin_lock(sdp-sd_ail_lock);
head = bh = page_buffers(page);
do {
if (atomic_read(bh-b_count))
@@ -1080,6 +1081,7 @@ int gfs2_releasepage(struct page *page, gfp_t gfp_mask)
goto not_possible;
bh = bh-b_this_page;
} while(bh != head);
+   spin_unlock(sdp-sd_ail_lock);
gfs2_log_unlock(sdp);
 
head = bh = page_buffers(page);
@@ -1112,6 +1114,7 @@ not_possible: /* Should never happen */
WARN_ON(buffer_dirty(bh));
WARN_ON(buffer_pinned(bh));
 cannot_release:
+   spin_unlock(sdp-sd_ail_lock);
gfs2_log_unlock(sdp);
return 0;
 }
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 712b722..2cca293 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -47,10 +47,10 @@ static void __gfs2_ail_flush(struct gfs2_glock *gl)
bd_ail_gl_list);
bh = bd-bd_bh;
gfs2_remove_from_ail(bd);
-   spin_unlock(sdp-sd_ail_lock);
-
bd-bd_bh = NULL;
bh-b_private = NULL;
+   spin_unlock(sdp-sd_ail_lock);
+
bd-bd_blkno = bh-b_blocknr;
gfs2_log_lock(sdp);
gfs2_assert_withdraw(sdp, !buffer_busy(bh));
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 903115f..85c6292 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -903,6 +903,7 @@ void gfs2_meta_syncfs(struct gfs2_sbd *sdp)
if (gfs2_ail1_empty(sdp))
break;
}
+   gfs2_log_flush(sdp, NULL);
 }
 
 static inline int gfs2_jrnl_flush_reqd(struct gfs2_sbd *sdp)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index ed540e7..fb0edf7 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -757,13 +757,17 @@ static int gfs2_write_inode(struct inode *inode, struct 
writeback_control *wbc)
struct timespec atime;
struct gfs2_dinode *di;
int ret = -EAGAIN;
+   int unlock_required = 0;
 
/* Skip timestamp update, if this is from a memalloc */
if (current-flags  PF_MEMALLOC)
goto do_flush;
-   ret = gfs2_glock_nq_init(ip-i_gl, LM_ST_EXCLUSIVE, 0, gh);
-   if (ret)
-   goto do_flush;
+   if (!gfs2_glock_is_locked_by_me(ip-i_gl)) {
+   ret = gfs2_glock_nq_init(ip-i_gl, LM_ST_EXCLUSIVE, 0, gh);
+   if (ret)
+   goto do_flush;
+   unlock_required = 1;
+   }
ret = gfs2_trans_begin(sdp, RES_DINODE, 0);
if (ret)
goto do_unlock;
@@ -780,7 +784,8 @@ static int gfs2_write_inode(struct inode *inode, struct 
writeback_control *wbc)
}
gfs2_trans_end(sdp);
 do_unlock:
-   gfs2_glock_dq_uninit(gh);
+   if (unlock_required)
+   gfs2_glock_dq_uninit(gh);
 do_flush:
if (wbc-sync_mode == WB_SYNC_ALL)
gfs2_log_flush(GFS2_SB(inode), ip-i_gl);
@@ -1427,7 +1432,20 @@ out:
return error;
 }
 
-/*
+/**
+ * gfs2_evict_inode - Remove an inode from cache
+ * @inode: The inode to evict
+ *
+ * There are three cases to consider:
+ * 1. i_nlink == 0, we are final opener (and must deallocate)
+ * 2. i_nlink == 0, we are not the final opener (and cannot deallocate)
+ * 3. i_nlink  0
+ *
+ * If the fs is read only, then we have to treat all cases as per #3
+ * since we are unable to 

[Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device

2011-07-14 Thread Steven Whitehouse

This is designed to address Red Hat bz #719124 and #719126

The help message is updated to include all supported options.
Also, the return codes are now taken from sysexits.h rather
than trying to pass negative errno values as per kernel
code. There is not an exact error code for all potential
events, but they are close enough I think.

The code also checks that there is exactly one non-option
argument (i.e. the device) given before attempting to open
it.

Signed-off-by: Steven Whitehouse swhit...@redhat.com
Reported-by: Nathan Straz nst...@redhat.com

diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
index 48fd59f..6a0daff 100644
--- a/gfs2/tune/main.c
+++ b/gfs2/tune/main.c
@@ -1,6 +1,7 @@
 #include stdio.h
 #include stdlib.h
 #include libgen.h
+#include sysexits.h
 
 #include sys/types.h
 #include sys/stat.h
@@ -51,8 +52,8 @@ void parse_mount_options(char *arg)
 
 static void usage(char *name)
 {
-   printf(Usage: %s -L volume label -U UUID -l -o 
-   mount options device \n, basename(name));
+   printf(Usage: %s [-hlV] [-L volume_label] [-U UUID]\n\t
+   [-o mount_options] device \n, basename(name));
 }
 
 static void version(void)
@@ -62,14 +63,14 @@ static void version(void)
 
 int main(int argc, char **argv)
 {
-   int c, status = 0;
+   int c, status;
 
memset(tfs, 0, sizeof(struct tunegfs2));
while((c = getopt(argc, argv, hL:U:lo:V)) != -1) {
switch(c) {
case 'h':
usage(argv[0]);
-   break;
+   return 0;
case 'L':
tfs-opt_label = 1;
tfs-label = optarg;
@@ -86,23 +87,27 @@ int main(int argc, char **argv)
break;
case 'V':
version();
-   break;
+   return 0;
default:
fprintf(stderr, _(Invalid option.\n));
usage(argv[0]);
-   status = -EINVAL;
-   goto out;
+   return EX_USAGE;
}
}
 
+   if ((argc - optind) != 1) {
+   fprintf(stderr, _(Incorrect number of arguments\n));
+   usage(argv[0]);
+   return EX_USAGE;
+   }
+
tfs-devicename = argv[optind];
tfs-fd = open(tfs-devicename, O_RDWR); 
 
if (tfs-fd  0) {
fprintf(stderr, _(Unable to open device %s\n),
tfs-devicename);
-   status = -EIO;
-   goto out;
+   return EX_IOERR;
}
 
status = read_super(tfs);
diff --git a/gfs2/tune/super.c b/gfs2/tune/super.c
index 056e868..9d67054 100644
--- a/gfs2/tune/super.c
+++ b/gfs2/tune/super.c
@@ -1,6 +1,6 @@
 #include stdio.h
 #include stdlib.h
-
+#include sysexits.h
 #include sys/types.h
 #include sys/stat.h
 #include errno.h
@@ -63,7 +63,7 @@ static int str2uuid(const char *newval, char *uuid)
 
if (strlen(newval) != 36) {
fprintf(stderr, _(Invalid UUID specified.\n));
-   return -EINVAL;
+   return EX_DATAERR;
}
 
cp = uuid;
@@ -74,13 +74,13 @@ static int str2uuid(const char *newval, char *uuid)
continue;
fprintf(stderr, _(uuid %s has an invalid format.),
newval);
-   return -EINVAL;
+   return EX_DATAERR;
}
if (!isxdigit(newval[i])) {
fprintf(stderr, _(uuid %s has an invalid hex 
digit '%c' at offset %d.\n),
newval, newval[i], i + 1);
-   return -EINVAL;
+   return EX_DATAERR;
}
*cp = str_to_hexchar(newval[i++]);
cp++;
@@ -96,13 +96,13 @@ int read_super(struct tunegfs2 *tfs)
block = malloc(sizeof(char) * GFS2_DEFAULT_BSIZE);
n = pread(tfs-fd, block, GFS2_DEFAULT_BSIZE, tfs-sb_start);
if (n  0) {
-   fprintf(stderr, _(Error reading from device));
-   return errno;
+   perror(read_super: pread);
+   return EX_IOERR;
}
tfs-sb = block;
if (be32_to_cpu(tfs-sb-sb_header.mh_magic) != GFS2_MAGIC) {
fprintf(stderr, _(Not a GFS/GFS2 device\n));
-   return -EINVAL;
+   return EX_IOERR;
}
return 0;
 }
@@ -144,9 +144,9 @@ int write_super(const struct tunegfs2 *tfs)
 {
int n;
n = pwrite(tfs-fd, tfs-sb, GFS2_DEFAULT_BSIZE, tfs-sb_start);
-   if (n0) {
-   fprintf(stderr, _(Unable to write super block\n));
-   return -errno;
+   if (n  0) {
+   perror(write_super: 

Re: [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device

2011-07-14 Thread Andrew Price

Hi Steve,

This looks good to me. For consistency, should we start using sysexits.h 
values throughout gfs2-utils?


Andy

On 14/07/11 11:23, Steven Whitehouse wrote:


This is designed to address Red Hat bz #719124 and #719126

The help message is updated to include all supported options.
Also, the return codes are now taken from sysexits.h rather
than trying to pass negative errno values as per kernel
code. There is not an exact error code for all potential
events, but they are close enough I think.

The code also checks that there is exactly one non-option
argument (i.e. the device) given before attempting to open
it.

Signed-off-by: Steven Whitehouseswhit...@redhat.com
Reported-by: Nathan Straznst...@redhat.com

diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
index 48fd59f..6a0daff 100644
--- a/gfs2/tune/main.c
+++ b/gfs2/tune/main.c
@@ -1,6 +1,7 @@
  #includestdio.h
  #includestdlib.h
  #includelibgen.h
+#includesysexits.h

  #includesys/types.h
  #includesys/stat.h
@@ -51,8 +52,8 @@ void parse_mount_options(char *arg)

  static void usage(char *name)
  {
-   printf(Usage: %s -Lvolume label  -UUUID  -l -o 
-   mount options  device  \n, basename(name));
+   printf(Usage: %s [-hlV] [-Lvolume_label] [-UUUID]\n\t
+   [-omount_options]device  \n, basename(name));
  }

  static void version(void)
@@ -62,14 +63,14 @@ static void version(void)

  int main(int argc, char **argv)
  {
-   int c, status = 0;
+   int c, status;

memset(tfs, 0, sizeof(struct tunegfs2));
while((c = getopt(argc, argv, hL:U:lo:V)) != -1) {
switch(c) {
case 'h':
usage(argv[0]);
-   break;
+   return 0;
case 'L':
tfs-opt_label = 1;
tfs-label = optarg;
@@ -86,23 +87,27 @@ int main(int argc, char **argv)
break;
case 'V':
version();
-   break;
+   return 0;
default:
fprintf(stderr, _(Invalid option.\n));
usage(argv[0]);
-   status = -EINVAL;
-   goto out;
+   return EX_USAGE;
}
}

+   if ((argc - optind) != 1) {
+   fprintf(stderr, _(Incorrect number of arguments\n));
+   usage(argv[0]);
+   return EX_USAGE;
+   }
+
tfs-devicename = argv[optind];
tfs-fd = open(tfs-devicename, O_RDWR);

if (tfs-fd  0) {
fprintf(stderr, _(Unable to open device %s\n),
tfs-devicename);
-   status = -EIO;
-   goto out;
+   return EX_IOERR;
}

status = read_super(tfs);
diff --git a/gfs2/tune/super.c b/gfs2/tune/super.c
index 056e868..9d67054 100644
--- a/gfs2/tune/super.c
+++ b/gfs2/tune/super.c
@@ -1,6 +1,6 @@
  #includestdio.h
  #includestdlib.h
-
+#includesysexits.h
  #includesys/types.h
  #includesys/stat.h
  #includeerrno.h
@@ -63,7 +63,7 @@ static int str2uuid(const char *newval, char *uuid)

if (strlen(newval) != 36) {
fprintf(stderr, _(Invalid UUID specified.\n));
-   return -EINVAL;
+   return EX_DATAERR;
}

cp = uuid;
@@ -74,13 +74,13 @@ static int str2uuid(const char *newval, char *uuid)
continue;
fprintf(stderr, _(uuid %s has an invalid format.),
newval);
-   return -EINVAL;
+   return EX_DATAERR;
}
if (!isxdigit(newval[i])) {
fprintf(stderr, _(uuid %s has an invalid hex 
digit '%c' at offset %d.\n),
newval, newval[i], i + 1);
-   return -EINVAL;
+   return EX_DATAERR;
}
*cp = str_to_hexchar(newval[i++]);
cp++;
@@ -96,13 +96,13 @@ int read_super(struct tunegfs2 *tfs)
block = malloc(sizeof(char) * GFS2_DEFAULT_BSIZE);
n = pread(tfs-fd, block, GFS2_DEFAULT_BSIZE, tfs-sb_start);
if (n  0) {
-   fprintf(stderr, _(Error reading from device));
-   return errno;
+   perror(read_super: pread);
+   return EX_IOERR;
}
tfs-sb = block;
if (be32_to_cpu(tfs-sb-sb_header.mh_magic) != GFS2_MAGIC) {
fprintf(stderr, _(Not a GFS/GFS2 device\n));
-   return -EINVAL;
+   return EX_IOERR;
}
return 0;
  }
@@ -144,9 +144,9 @@ int write_super(const struct tunegfs2 *tfs)
  {
int n;
n = pwrite(tfs-fd, tfs-sb, GFS2_DEFAULT_BSIZE, tfs-sb_start);
-   if (n0) {

Re: [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device

2011-07-14 Thread Steven Whitehouse
Hi,

On Thu, 2011-07-14 at 11:52 +0100, Andrew Price wrote:
 Hi Steve,
 
 This looks good to me. For consistency, should we start using sysexits.h 
 values throughout gfs2-utils?
 
 Andy
 
Yes, thats not a bad plan, although I think fsck has some specific
values it has to use which may or may not match sysexits,

Steve.

 On 14/07/11 11:23, Steven Whitehouse wrote:
 
  This is designed to address Red Hat bz #719124 and #719126
 
  The help message is updated to include all supported options.
  Also, the return codes are now taken from sysexits.h rather
  than trying to pass negative errno values as per kernel
  code. There is not an exact error code for all potential
  events, but they are close enough I think.
 
  The code also checks that there is exactly one non-option
  argument (i.e. the device) given before attempting to open
  it.
 
  Signed-off-by: Steven Whitehouseswhit...@redhat.com
  Reported-by: Nathan Straznst...@redhat.com
 
  diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
  index 48fd59f..6a0daff 100644
  --- a/gfs2/tune/main.c
  +++ b/gfs2/tune/main.c
  @@ -1,6 +1,7 @@
#includestdio.h
#includestdlib.h
#includelibgen.h
  +#includesysexits.h
 
#includesys/types.h
#includesys/stat.h
  @@ -51,8 +52,8 @@ void parse_mount_options(char *arg)
 
static void usage(char *name)
{
  -   printf(Usage: %s -Lvolume label  -UUUID  -l -o 
  -   mount options  device  \n, basename(name));
  +   printf(Usage: %s [-hlV] [-Lvolume_label] [-UUUID]\n\t
  +   [-omount_options]device  \n, basename(name));
}
 
static void version(void)
  @@ -62,14 +63,14 @@ static void version(void)
 
int main(int argc, char **argv)
{
  -   int c, status = 0;
  +   int c, status;
 
  memset(tfs, 0, sizeof(struct tunegfs2));
  while((c = getopt(argc, argv, hL:U:lo:V)) != -1) {
  switch(c) {
  case 'h':
  usage(argv[0]);
  -   break;
  +   return 0;
  case 'L':
  tfs-opt_label = 1;
  tfs-label = optarg;
  @@ -86,23 +87,27 @@ int main(int argc, char **argv)
  break;
  case 'V':
  version();
  -   break;
  +   return 0;
  default:
  fprintf(stderr, _(Invalid option.\n));
  usage(argv[0]);
  -   status = -EINVAL;
  -   goto out;
  +   return EX_USAGE;
  }
  }
 
  +   if ((argc - optind) != 1) {
  +   fprintf(stderr, _(Incorrect number of arguments\n));
  +   usage(argv[0]);
  +   return EX_USAGE;
  +   }
  +
  tfs-devicename = argv[optind];
  tfs-fd = open(tfs-devicename, O_RDWR);
 
  if (tfs-fd  0) {
  fprintf(stderr, _(Unable to open device %s\n),
  tfs-devicename);
  -   status = -EIO;
  -   goto out;
  +   return EX_IOERR;
  }
 
  status = read_super(tfs);
  diff --git a/gfs2/tune/super.c b/gfs2/tune/super.c
  index 056e868..9d67054 100644
  --- a/gfs2/tune/super.c
  +++ b/gfs2/tune/super.c
  @@ -1,6 +1,6 @@
#includestdio.h
#includestdlib.h
  -
  +#includesysexits.h
#includesys/types.h
#includesys/stat.h
#includeerrno.h
  @@ -63,7 +63,7 @@ static int str2uuid(const char *newval, char *uuid)
 
  if (strlen(newval) != 36) {
  fprintf(stderr, _(Invalid UUID specified.\n));
  -   return -EINVAL;
  +   return EX_DATAERR;
  }
 
  cp = uuid;
  @@ -74,13 +74,13 @@ static int str2uuid(const char *newval, char *uuid)
  continue;
  fprintf(stderr, _(uuid %s has an invalid format.),
  newval);
  -   return -EINVAL;
  +   return EX_DATAERR;
  }
  if (!isxdigit(newval[i])) {
  fprintf(stderr, _(uuid %s has an invalid hex 
  digit '%c' at offset %d.\n),
  newval, newval[i], i + 1);
  -   return -EINVAL;
  +   return EX_DATAERR;
  }
  *cp = str_to_hexchar(newval[i++]);
  cp++;
  @@ -96,13 +96,13 @@ int read_super(struct tunegfs2 *tfs)
  block = malloc(sizeof(char) * GFS2_DEFAULT_BSIZE);
  n = pread(tfs-fd, block, GFS2_DEFAULT_BSIZE, tfs-sb_start);
  if (n  0) {
  -   fprintf(stderr, _(Error reading from device));
  -   return errno;
  +   perror(read_super: pread);
  +   return EX_IOERR;
  }
  tfs-sb = block;
  if (be32_to_cpu(tfs-sb-sb_header.mh_magic) != GFS2_MAGIC) {
  fprintf(stderr, _(Not a GFS/GFS2 device\n));
  -   return -EINVAL;
  +   return EX_IOERR;
  }
  return 0;
}
  @@ -144,9 +144,9 

[Cluster-devel] tunegfs2: Fix label/locktable setting code

2011-07-14 Thread Steven Whitehouse
This patch is aimed at fixing Red Hat bz #719135

The code here seemed a bit confused. I've added a check for the
format of the locktable/label based on the same code in mkfs.
In addition we set the lock proto first in order that we use the
new value of the lock proto when checking the format of
the lock table.

There is no need to have a separate function for setting the
label to the locktable, since they are both setting the same
data.

The on-disk locktable and lockproto always contain trailing
NULLs and when we read the sb from disk, we now always add NULLs
at the end of the strings so that we can rely on this later on.

The patch also checks the size of the lock table when it is
set to ensure that it is not too large.

Signed-off-by: Steven Whitehouse swhit...@redhat.com
Reported-by: Nathan Straznst...@redhat.com

diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
index 6a0daff..dc9d6a5 100644
--- a/gfs2/tune/main.c
+++ b/gfs2/tune/main.c
@@ -20,13 +20,13 @@ struct tunegfs2 tunegfs2_struct;
 struct tunegfs2 *tfs = tunegfs2_struct;
 

-void parse_mount_options(char *arg)
+static void parse_mount_options(char *arg)
 {
struct opt_map *m;
char *s, *c;
int l;
struct opt_map {
-   char *tag;
+   const char *tag;
int *flag;
char **val;
} map[]= {
@@ -101,6 +101,12 @@ int main(int argc, char **argv)
return EX_USAGE;
}
 
+   if (tfs-opt_label  tfs-opt_table) {
+   fprintf(stderr, _(The -L and -o locktable= options are 
mutually exclusive\n));
+   usage(argv[0]);
+   return EX_USAGE;
+   }
+
tfs-devicename = argv[optind];
tfs-fd = open(tfs-devicename, O_RDWR); 
 
@@ -120,29 +126,26 @@ int main(int argc, char **argv)
goto out;
}
 
-   /* Keep label and table together because they are the same field
-* in the superblock */
-
-   if (tfs-opt_label) {
-   status = change_label(tfs, tfs-label);
+   if (tfs-opt_proto) {
+   status = change_lockproto(tfs, tfs-proto);
if (status)
goto out;
}
 
-   if (tfs-opt_table) {
-   status = change_locktable(tfs, tfs-table);
+   if (tfs-opt_label) {
+   status = change_locktable(tfs, tfs-label);
if (status)
goto out;
}
 
-   if (tfs-opt_proto) {
-   status = change_lockproto(tfs, tfs-proto);
+   if (tfs-opt_table) {
+   status = change_locktable(tfs, tfs-table);
if (status)
goto out;
}
 
-   if (tfs-opt_label || tfs-opt_uuid ||
-   tfs-opt_table || tfs-opt_proto) {
+   if (tfs-opt_label || tfs-opt_uuid || tfs-opt_table ||
+   tfs-opt_proto) {
status = write_super(tfs);
if (status)
goto out;
diff --git a/gfs2/tune/super.c b/gfs2/tune/super.c
index 9d67054..b912f11 100644
--- a/gfs2/tune/super.c
+++ b/gfs2/tune/super.c
@@ -104,6 +104,9 @@ int read_super(struct tunegfs2 *tfs)
fprintf(stderr, _(Not a GFS/GFS2 device\n));
return EX_IOERR;
}
+   /* Ensure that table and proto are NULL terminated */
+   tfs-sb-sb_lockproto[GFS2_LOCKNAME_LEN - 1] = '\0';
+   tfs-sb-sb_locktable[GFS2_LOCKNAME_LEN - 1] = '\0';
return 0;
 }
 
@@ -115,16 +118,10 @@ static int is_gfs2(const struct tunegfs2 *tfs)
 int print_super(const struct tunegfs2 *tfs)
 {
char *fsname = NULL;
-   int table_len = 0, fsname_len = 0;
 
fsname = strchr(tfs-sb-sb_locktable, ':');
-   if (fsname) {
-   table_len = fsname - tfs-sb-sb_locktable;
-   fsname_len = GFS2_LOCKNAME_LEN - table_len - 1;
-   fsname++;
-   }
-
-   printf(_(Filesystem volume name: %.*s\n), fsname_len, fsname);
+   if (fsname)
+   printf(_(Filesystem volume name: %s\n), ++fsname);
if (is_gfs2(tfs))
printf(_(Filesystem UUID: %s\n), uuid2str(tfs-sb-sb_uuid));
printf( _(Filesystem magic number: 0x%X\n), 
be32_to_cpu(tfs-sb-sb_header.mh_magic));
@@ -133,9 +130,8 @@ int print_super(const struct tunegfs2 *tfs)
printf(_(Root inode: %llu\n), (unsigned long 
long)be64_to_cpu(tfs-sb-sb_root_dir.no_addr));
if (is_gfs2(tfs))
printf(_(Master inode: %llu\n), (unsigned long 
long)be64_to_cpu(tfs-sb-sb_master_dir.no_addr));
-   printf(_(Lock Protocol: %.*s\n), GFS2_LOCKNAME_LEN,
-   tfs-sb-sb_lockproto);
-   printf(_(Lock table: %.*s\n), table_len, tfs-sb-sb_locktable);
+   printf(_(Lock Protocol: %s\n), tfs-sb-sb_lockproto);
+   printf(_(Lock table: %s\n), tfs-sb-sb_locktable);
 
return 0;
 }
@@ -151,26 +147,6 @@ int write_super(const struct tunegfs2 *tfs)
return 0;
 }
 
-int 

[Cluster-devel] GFS2: Pull request (fixes)

2011-07-14 Thread Steven Whitehouse
Hi,

Please consider pulling the following three fixes,

Steve.


The following changes since commit 620917de59eeb934b9f8cf35cc2d95c1ac8ed0fc:

  Linux 3.0-rc7 (2011-07-11 16:51:52 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/steve/gfs2-2.6-fixes.git master

Benjamin Marzinski (1):
  GFS2: force a log flush when invalidating the rindex glock

Steven Whitehouse (2):
  GFS2: Fix race during filesystem mount
  GFS2: Resolve inode eviction and ail list interaction bug

 fs/gfs2/aops.c   |3 +++
 fs/gfs2/glops.c  |8 +---
 fs/gfs2/incore.h |2 ++
 fs/gfs2/log.c|1 +
 fs/gfs2/ops_fstype.c |3 +++
 fs/gfs2/super.c  |   36 ++--
 fs/gfs2/sys.c|7 ++-
 7 files changed, 50 insertions(+), 10 deletions(-)