Re: [Cluster-devel] [PATCH 02/17] quota: Wire up -quota_{enable, disable} callbacks into Q_QUOTA{ON, OFF}

2015-01-19 Thread Christoph Hellwig
On Fri, Jan 16, 2015 at 01:47:36PM +0100, Jan Kara wrote:
 Make Q_QUOTAON / Q_QUOTAOFF quotactl call -quota_enable /
 -quota_disable callback when provided. To match current behavior of
 ocfs2  ext4 we make these quotactls turn on / off quota enforcement for
 appropriate quota type.
 
 Signed-off-by: Jan Kara j...@suse.cz
 ---
  fs/quota/quota.c | 31 +++
  include/linux/quotaops.h |  2 ++
  2 files changed, 29 insertions(+), 4 deletions(-)
 
 diff --git a/fs/quota/quota.c b/fs/quota/quota.c
 index 5b307e2b5719..748716ffee48 100644
 --- a/fs/quota/quota.c
 +++ b/fs/quota/quota.c
 @@ -66,18 +66,43 @@ static int quota_sync_all(int type)
   return ret;
  }
  
 +unsigned int qtype_limit_flag(int type)
 +{
 + switch (type) {
 + case USRQUOTA:
 + return FS_QUOTA_UDQ_ENFD;
 + case GRPQUOTA:
 + return FS_QUOTA_GDQ_ENFD;
 + case PRJQUOTA:
 + return FS_QUOTA_PDQ_ENFD;
 + }
 + return 0;


What's the limit_ in the name supposed to mean?

Otherwise looks good:

Reviewed-by: Christoph Hellwig h...@lst.de



Re: [Cluster-devel] [PATCH 01/17] quota: Split -set_xstate callback into two

2015-01-19 Thread Christoph Hellwig
On Fri, Jan 16, 2015 at 01:47:35PM +0100, Jan Kara wrote:
 Split -set_xstate callback into two callbacks - one for turning quotas
 on (-quota_enable) and one for turning quotas off (-quota_disable). That
 way we don't have to pass quotactl command into the callback which seems
 cleaner.
 
 Signed-off-by: Jan Kara j...@suse.cz

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de



Re: [Cluster-devel] [PATCH 04/17] ext4: Use generic helpers for quotaon and quotaoff

2015-01-19 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de



Re: [Cluster-devel] [PATCH 03/17] quota: Add -quota_{enable, disable} callbacks for VFS quotas

2015-01-19 Thread Christoph Hellwig
On Fri, Jan 16, 2015 at 01:47:37PM +0100, Jan Kara wrote:
 +EXPORT_SYMBOL(dquot_quota_enable);

 +EXPORT_SYMBOL(dquot_quota_disable);

I can't find any modular users of this (in fact none outside this
file), so I'd suggest to keep these local.



Re: [Cluster-devel] [PATCH 12/17] xfs: Convert to using -get_state callback

2015-01-19 Thread Christoph Hellwig
 +static void xfs_qm_fill_state(struct qc_type_state *tstate,

Normal xfs style would be to keep the static void  on a separate line,
as well as the arguments, e.g.

static void
xfs_qm_fill_state(
struct qc_type_state*tstate,


 +   struct xfs_mount *mp,
 +   struct xfs_inode *ip,
 +   xfs_ino_t ino)

No need to pass mp, as it can be derived as ip-i_mount.

Btw, I think this code should move into xfs_quotaops.c now
that it ties into the Linux quota interface, and xfs_qm_scall_getstate
should be folded into xfs_fs_get_quota_state.



Re: [Cluster-devel] [PATCH 09/17] quota: Make Q_XQUOTASYNC support VFS quota syncing

2015-01-19 Thread Christoph Hellwig
On Fri, Jan 16, 2015 at 01:47:43PM +0100, Jan Kara wrote:
 Call -quota_sync method from Q_XQUOTASYNC for better userspace
 compatibility.

Q_XQUOTASYNC never did the equivalent to -quota_sync, but rather was
the equivalent to sys_syncfs which also happens to write out quotas.

Unless you have a really strong reason for wiring it up, I'd rather keep
it as-is.



Re: [Cluster-devel] [PATCH 05/17] ocfs2: Use generic helpers for quotaon and quotaoff

2015-01-19 Thread Christoph Hellwig
On Fri, Jan 16, 2015 at 01:47:39PM +0100, Jan Kara wrote:
 Ocfs2 can just use the generic helpers provided by quota code for
 turning quotas on and off when quota files are stored as system inodes.
 The only difference is the feature test in ocfs2_quota_on() and that is
 covered by dquot_quota_enable() checking whether usage tracking is
 enabled (which can happen only if the filesystem has the quota feature
 set).
 
 Signed-off-by: Jan Kara j...@suse.cz
 ---
  fs/ocfs2/super.c | 32 +---
  1 file changed, 1 insertion(+), 31 deletions(-)
 
 diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
 index 83723179e1ec..706c71c2955d 100644
 --- a/fs/ocfs2/super.c
 +++ b/fs/ocfs2/super.c
 @@ -1000,36 +1000,6 @@ static void ocfs2_disable_quotas(struct ocfs2_super 
 *osb)
   }
  }
  
 -/* Handle quota on quotactl */
 -static int ocfs2_quota_on(struct super_block *sb, int type, int format_id)
 -{
 - unsigned int feature[OCFS2_MAXQUOTAS] = {
 - OCFS2_FEATURE_RO_COMPAT_USRQUOTA,
 - OCFS2_FEATURE_RO_COMPAT_GRPQUOTA};
 -
 - if (!OCFS2_HAS_RO_COMPAT_FEATURE(sb, feature[type]))
 - return -EINVAL;

Where are we doing this feature check now?



Re: [Cluster-devel] [PATCH 14/17] quota: Remove -get_xstate and -get_xstatev callbacks

2015-01-19 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de



Re: [Cluster-devel] [PATCH 07/17] quota: Switch -get_dqblk() and -set_dqblk() to use bytes as space units

2015-01-19 Thread Christoph Hellwig
 diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
 index 18dc721ca19f..f718ba1f2ccb 100644
 --- a/fs/xfs/libxfs/xfs_fs.h
 +++ b/fs/xfs/libxfs/xfs_fs.h
 @@ -559,18 +559,4 @@ typedef struct xfs_swapext
  /*   XFS_IOC_GETFSUUID -- deprecated 140  */
  
  
 -#ifndef HAVE_BBMACROS
 -/*
 - * Block I/O parameterization.   A basic block (BB) is the lowest size of
 - * filesystem allocation, and must equal 512.  Length units given to bio
 - * routines are in BB's.
 - */
 -#define BBSHIFT  9
 -#define BBSIZE   (1BBSHIFT)
 -#define BBMASK   (BBSIZE-1)
 -#define BTOBB(bytes) (((__u64)(bytes) + BBSIZE - 1)  BBSHIFT)
 -#define BTOBBT(bytes)((__u64)(bytes)  BBSHIFT)
 -#define BBTOB(bbs)   ((bbs)  BBSHIFT)
 -#endif

Please don't move these defintions around and just opencode them in
the quota code.

Otherwise looks good:

Reviewed-by: Christoph Hellwig h...@lst.de



Re: [Cluster-devel] [PATCH 10/17] quota: Make VFS quotas use new interface for getting quota info

2015-01-19 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de



Re: [Cluster-devel] [PATCH 06/17] quota: Remove quota_on_meta callback

2015-01-19 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de



[Cluster-devel] [fsck.gfs2 patch] fsck.gfs2: Rebuild system files if they don't have the SYS bit set

2015-01-19 Thread Bob Peterson
Hi,

This patch checks for the GFS2_DIF_SYSTEM bit on system dinodes. If the
bit is not set, the dinode is assumed to be corrupt and rebuilt.
If the jindex is rebuilt, it needs to be re-read.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson rpete...@redhat.com 
---
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 2841b8c..4348683 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -27,6 +27,7 @@
 #include util.h
 #include link.h
 #include metawalk.h
+#include fs_recovery.h
 
 struct special_blocks gfs1_rindex_blks;
 
@@ -1241,7 +1242,8 @@ static int check_system_inode(struct gfs2_sbd *sdp,
  struct gfs2_inode **sysinode,
  const char *filename,
  int builder(struct gfs2_sbd *sdp),
- enum gfs2_mark_block mark)
+ enum gfs2_mark_block mark,
+ struct gfs2_inode *sysdir, int needs_sysbit)
 {
uint64_t iblock = 0;
struct dir_status ds = {0};
@@ -1282,6 +1284,24 @@ static int check_system_inode(struct gfs2_sbd *sdp,
if (mark == gfs2_inode_dir)
dirtree_insert((*sysinode)-i_di.di_num);
}
+   /* Make sure it's marked as a system file/directory */
+   if (needs_sysbit 
+   !((*sysinode)-i_di.di_flags  GFS2_DIF_SYSTEM)) {
+   log_err( _(System inode %s is missing the 'system' 
+  flag. It should be rebuilt.\n), filename);
+   if (sysdir  query(_(Delete the corrupt %s system 
+ inode? (y/n) ), filename)) {
+   inode_put(sysinode);
+   gfs2_dirent_del(sysdir, filename,
+   strlen(filename));
+   /* Set the blockmap (but not bitmap) back to
+  'free' so that it gets checked like any
+  normal dinode. */
+   gfs2_blockmap_set(bl, iblock, gfs2_block_free);
+   log_err( _(Removed system inode \%s\.\n),
+filename);
+   }
+   }
} else
log_info( _(System inode for '%s' is corrupt or missing.\n),
  filename);
@@ -1290,18 +1310,22 @@ static int check_system_inode(struct gfs2_sbd *sdp,
   lost+found then, but we *need* our system inodes before we can
   do any of that. */
if (!(*sysinode) || ds.q != mark) {
-   log_err( _(Invalid or missing %s system inode (should be %d, 
-  is %d).\n), filename, mark, ds.q);
+   log_err( _(Invalid or missing %s system inode (should be 
+  '%s', is '%s').\n), filename,
+block_type_string(mark),
+block_type_string(ds.q));
if (query(_(Create new %s system inode? (y/n) ), filename)) {
log_err( _(Rebuilding system file \%s\\n),
 filename);
error = builder(sdp);
if (error) {
-   log_err( _(Error trying to rebuild system 
-  file %s: Cannot continue\n),
+   log_err( _(Error rebuilding system 
+  inode %s: Cannot continue\n),
 filename);
return error;
}
+   if (*sysinode == sdp-md.jiinode)
+   ji_update(sdp);
fsck_blockmap_set(*sysinode,
  (*sysinode)-i_di.di_num.no_addr,
  filename, mark);
@@ -1367,7 +1391,8 @@ static int check_system_inodes(struct gfs2_sbd *sdp)
  sdp-master_dir-i_di.di_num.no_addr,
  master, gfs2_inode_dir);
if (check_system_inode(sdp, sdp-master_dir, master,
-  build_master, gfs2_inode_dir)) {
+  build_master, gfs2_inode_dir,
+  NULL, 1)) {
stack;
return -1;
}
@@ -1377,39 +1402,41 @@ static int check_system_inodes(struct gfs2_sbd *sdp)
fsck_blockmap_set(sdp-md.rooti, sdp-md.rooti-i_di.di_num.no_addr,
  root, gfs2_inode_dir);
if (check_system_inode(sdp, sdp-md.rooti, root, build_root,
-  gfs2_inode_dir)) {
+ 

[Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Check the integrity of the journal index

2015-01-19 Thread Bob Peterson
Hi,

This patch checks the jindex system directory to make sure the entries
all start with journal and so forth. If not, the jindex is deleted
and rebuilt. As part of this patch, I moved where we read in the rindex
file and rgrps to an earlier point in time, before the journals are
replayed. This allows us to remove a dummied-up rgrp kludge in the code.
However, if the replayed journal block is part of an rgrp, we need to
refresh the rgrp based on the values rewritten from the journal.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson rpete...@redhat.com 
---
diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index 095d118..4eaba1e 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -96,6 +96,30 @@ void gfs2_revoke_clean(struct gfs2_sbd *sdp)
}
 }
 
+static void refresh_rgrp(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
+struct gfs2_buffer_head *bh, uint64_t blkno)
+{
+   int i;
+
+   log_debug(_(Block is part of rgrp 0x%llx; refreshing the rgrp.\n),
+ (unsigned long long)rgd-ri.ri_addr);
+   for (i = 0; i  rgd-ri.ri_length; i++) {
+   if (rgd-bits[i].bi_bh-b_blocknr != blkno)
+   continue;
+
+   memcpy(rgd-bits[i].bi_bh-b_data, bh-b_data, sdp-bsize);
+   bmodified(rgd-bits[i].bi_bh);
+   if (i == 0) { /* this is the rgrp itself */
+   if (sdp-gfs1)
+   gfs_rgrp_in((struct gfs_rgrp *)rgd-rg,
+   rgd-bits[0].bi_bh);
+   else
+   gfs2_rgrp_in(rgd-rg, rgd-bits[0].bi_bh);
+   }
+   break;
+   }
+}
+
 static int buf_lo_scan_elements(struct gfs2_inode *ip, unsigned int start,
struct gfs2_log_descriptor *ld, __be64 *ptr,
int pass)
@@ -105,6 +129,7 @@ static int buf_lo_scan_elements(struct gfs2_inode *ip, 
unsigned int start,
struct gfs2_buffer_head *bh_log, *bh_ip;
uint64_t blkno;
int error = 0;
+   struct rgrp_tree *rgd;
 
if (pass != 1 || be32_to_cpu(ld-ld_type) != GFS2_LOG_DESC_METADATA)
return 0;
@@ -147,6 +172,9 @@ static int buf_lo_scan_elements(struct gfs2_inode *ip, 
unsigned int start,
error = -EIO;
} else {
bmodified(bh_ip);
+   rgd = gfs2_blk2rgrpd(sdp, blkno);
+   if (rgd  blkno  rgd-ri.ri_data0)
+   refresh_rgrp(sdp, rgd, bh_ip, blkno);
}
 
brelse(bh_log);
@@ -676,28 +704,8 @@ int replay_journals(struct gfs2_sbd *sdp, int preen, int 
force_check,
 
for(i = 0; i  sdp-md.journals; i++) {
if (sdp-md.journal[i]) {
-   struct rgrp_tree rgd;
-   struct gfs2_bitmap bits;
-
-   /* The real rgrp tree hasn't been built at this point,
-* so we need to dummy one up that covers the whole
-* file system so basic functions in check_metatree
-* don't segfault. */
-   rgd.start = sdp-sb_addr + 1;
-   rgd.length = 1;
-   bits.bi_bh = NULL;
-   bits.bi_start = 0;
-   bits.bi_len = sdp-fssize / GFS2_NBBY;
-   rgd.bits = bits;
-   rgd.ri.ri_addr = sdp-sb_addr + 1;
-   rgd.ri.ri_length = 1;
-   rgd.ri.ri_data0 = sdp-sb_addr + 2;
-   rgd.ri.ri_data = sdp-fssize - (sdp-sb_addr + 2);
-
-   sdp-rgtree.osi_node = (struct osi_node *)rgd;
error = check_metatree(sdp-md.journal[i],
   rangecheck_journal);
-   sdp-rgtree.osi_node = NULL;
if (error)
/* Don't use fsck_inode_put here because it's a
   system file and we need to dismantle it. */
@@ -707,8 +715,7 @@ int replay_journals(struct gfs2_sbd *sdp, int preen, int 
force_check,
if (!sdp-md.journal[i]) {
log_err(_(File system journal \journal%d\ is 
  missing or corrupt: pass1 will try to 
- recreate it.\n),
-   i);
+ recreate it.\n), i);
continue;
}
if (!error) {
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 4e52262..043917c 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -142,7 +142,7 @@ static int set_block_ranges(struct gfs2_sbd *sdp)
uint64_t rmin = 0;

[Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: rgrp block count reform

2015-01-19 Thread Bob Peterson
Hi,

In the past, pass5 has kept track of different counts for the different
metadata types, but the counts did not correspond to the type in the
bitmap. This patch changes pass5 so that it makes a little more sense.
There should be a straightforward blockmap-to-bitmap conversion, and
the counts should be based on the bitmap type. For example, before the
patch, the rgrp's inode count would be kept in count[1], but in the
bitmap, dinodes are of type 3. So this patch reworks that system so
that the count of dinodes is kept in count[3] and so forth, and it
uses a simple blockmap-to-bitmap to figure out the index to count.
This breaks down for GFS1 which keeps a few extra numbers in the rgrp,
however this patch compensates for that. This patch is being done
as a precursor to another patch that reduces fsck's memory requirements
by using a blockmap that's 1:1 with the bitmaps rather than today's
blockmap that's 4:1.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson rpete...@redhat.com 
---
diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c
index b2e8adf..2b8536d 100644
--- a/gfs2/fsck/pass5.c
+++ b/gfs2/fsck/pass5.c
@@ -12,94 +12,6 @@
 #include fsck.h
 #include util.h
 
-static int gfs1_convert_mark(uint8_t q, uint32_t *count)
-{
-   switch(q) {
-
-   case gfs2_meta_inval:
-   case gfs2_inode_invalid:
-   /* Convert invalid metadata to free blocks */
-   case gfs2_block_free:
-   count[0]++;
-   return GFS2_BLKST_FREE;
-
-   case gfs2_block_used:
-   count[2]++;
-   return GFS2_BLKST_USED;
-
-   case gfs2_inode_dir:
-   case gfs2_inode_file:
-   case gfs2_inode_lnk:
-   case gfs2_inode_device:
-   case gfs2_inode_fifo:
-   case gfs2_inode_sock:
-   count[1]++;
-   return GFS2_BLKST_DINODE;
-
-   case gfs2_indir_blk:
-   case gfs2_leaf_blk:
-   /*case gfs2_meta_rgrp:*/
-   case gfs2_jdata: /* gfs1 jdata blocks count as metadata and gfs1
-   metadata is marked the same as gfs2 inode in the
-   bitmap. */
-   case gfs2_meta_eattr:
-   count[3]++;
-   return GFS2_BLKST_DINODE;
-
-   case gfs2_freemeta:
-   count[4]++;
-   return GFS2_BLKST_UNLINKED;
-
-   default:
-   log_err( _(Invalid block type %d found\n), q);
-   }
-   return -1;
-}
-
-static int gfs2_convert_mark(uint8_t q, uint32_t *count)
-{
-   switch(q) {
-
-   case gfs2_meta_inval:
-   case gfs2_inode_invalid:
-   /* Convert invalid metadata to free blocks */
-   case gfs2_block_free:
-   count[0]++;
-   return GFS2_BLKST_FREE;
-
-   case gfs2_block_used:
-   count[2]++;
-   return GFS2_BLKST_USED;
-
-   case gfs2_inode_dir:
-   case gfs2_inode_file:
-   case gfs2_inode_lnk:
-   case gfs2_inode_device:
-   case gfs2_jdata: /* gfs1 jdata blocks count as metadata and gfs1
-   metadata is marked the same as gfs2 inode in the
-   bitmap. */
-   case gfs2_inode_fifo:
-   case gfs2_inode_sock:
-   count[1]++;
-   return GFS2_BLKST_DINODE;
-
-   case gfs2_indir_blk:
-   case gfs2_leaf_blk:
-   case gfs2_meta_eattr:
-   count[2]++;
-   return GFS2_BLKST_USED;
-
-   case gfs2_freemeta:
-   log_err( _(Invalid freemeta type %d found\n), q);
-   count[4]++;
-   return -1;
-
-   default:
-   log_err( _(Invalid block type %d found\n), q);
-   }
-   return -1;
-}
-
 static int check_block_status(struct gfs2_sbd *sdp, char *buffer,
  unsigned int buflen, uint64_t *rg_block,
  uint64_t rg_data, uint32_t *count)
@@ -107,7 +19,6 @@ static int check_block_status(struct gfs2_sbd *sdp, char 
*buffer,
unsigned char *byte, *end;
unsigned int bit;
unsigned char rg_status;
-   int block_status;
uint8_t q;
uint64_t block;
 
@@ -123,26 +34,33 @@ static int check_block_status(struct gfs2_sbd *sdp, char 
*buffer,
if (skip_this_pass || fsck_abort) /* if asked to skip the rest 
*/
return 0;
 
-   q = block_type(block);
-
-   if (sdp-gfs1)
-   block_status = gfs1_convert_mark(q, count);
-   else
-   block_status = gfs2_convert_mark(q, count);
-
-   if (block_status  0) {
-   log_err( _(Invalid status for block %llu (0x%llx).\n),
-(unsigned long long)block,
-(unsigned long long)block);
-   return block_status;
+   q = blockmap_to_bitmap(block_type(block), sdp-gfs1);
+   /*