Ocfs2 devel - May 2011 - [PATCH 0/4] ocfs2: bugfix for hard readonly mount

Hi, All,

These four patches are all related to ocfs2 on hard readonly mount.
patch 1 fix oops when umount ocfs2 on hard readonly device.
Because ocfs2_dismount_volume() will call ocfs2_cluster_hangup() and 
then call ocfs2_stack_driver_put(), will hit BUG_ON(active_stack == NULL).

patch 2 fix oops when do ls or cat in ocfs2 on hard readonly device.
Because ocfs2_open_lock() will call ocfs2_cluster_lock() and then call 
ocfs2_dlm_lock(), but active_stack is NULL.

patch 3 fix bug of http://oss.oracle.com/bugzilla/show_bug.cgi?id=1322
ocfs2_statfs, ocfs2_fiemap, ocfs2_get_acl, ocfs2_listxattr and 
ocfs2_xattr_get need ocfs2_inode_lock to get dinode buffer head, but on 
hard readonly mount, they did not get they expected, so lead to oops. 
ocfs2_init_security_and_acl and ocfs2_test_inode_bit also called 
ocfs2_inode_lock(inode, &bh, 0), but they are safe, because  
ocfs2_reflink and ocfs2_get_dentry  check hard read only before them.

patch 4 fix problem of running ls on hard readonly mount  can't get the 
right result.
Because ocfs2_dentry_lock() return -EROFS. It should return 0.

best regards,
Tiger

As no cluster service in hard readonly mount, so skip hangup
cluster, otherwise ocfs2_stack_driver_put will hit BUG_ON.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/super.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 5a521c7..395b26f 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1973,7 +1973,8 @@ static void ocfs2_dismount_volume(struct super_block *sb,
int mnt_err)
 	 * If we failed before we got a uuid_str yet, we can't stop
 	 * heartbeat.  Otherwise, do it.
 	 */
-	if (!mnt_err && !ocfs2_mount_local(osb) && osb->uuid_str)
+	if (!mnt_err && !ocfs2_mount_local(osb) && osb->uuid_str
&&
+	    !ocfs2_is_hard_readonly(osb))
 		hangup_needed = 1;
 
 	if (osb->cconn)
-- 
1.7.4.4

As no dlm lock in hard readonly mount, so add hard readonly check
in open lock, otherwise ocfs2_cluster_lock leads to oops.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/dlmglue.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 7642d7c..09e8b30 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -1692,7 +1692,7 @@ int ocfs2_open_lock(struct inode *inode)
 	mlog(0, "inode %llu take PRMODE open lock\n",
 	     (unsigned long long)OCFS2_I(inode)->ip_blkno);
 
-	if (ocfs2_mount_local(osb))
+	if (ocfs2_is_hard_readonly(osb) || ocfs2_mount_local(osb))
 		goto out;
 
 	lockres = &OCFS2_I(inode)->ip_open_lockres;
@@ -1718,6 +1718,12 @@ int ocfs2_try_open_lock(struct inode *inode, int write)
 	     (unsigned long long)OCFS2_I(inode)->ip_blkno,
 	     write ? "EXMODE" : "PRMODE");
 
+	if (ocfs2_is_hard_readonly(osb)) {
+		if (write)
+			status = -EROFS;
+		goto out;
+	}
+
 	if (ocfs2_mount_local(osb))
 		goto out;
 
-- 
1.7.4.4

ocfs2_statfs, ocfs2_fiemap, ocfs2_get_acl, ocfs2_listxattr and
ocfs2_xattr_get need ocfs2_inode_lock to get dinode buffer head
even on hard readonly mount.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/dlmglue.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 09e8b30..717f8d3 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2304,7 +2304,7 @@ int ocfs2_inode_lock_full_nested(struct inode *inode,
 	if (ocfs2_is_hard_readonly(osb)) {
 		if (ex)
 			status = -EROFS;
-		goto bail;
+		goto getbh;
 	}
 
 	if (ocfs2_mount_local(osb))
@@ -2362,7 +2362,7 @@ local:
 			mlog_errno(status);
 		goto bail;
 	}
-
+getbh:
 	if (ret_bh) {
 		status = ocfs2_assign_bh(inode, ret_bh, local_bh);
 		if (status < 0) {
-- 
1.7.4.4

As some readonly operations such as ls, cat are allowed to
run on hard readonly mount, so change return value to 0 to
allow them to get fake readonly dentry lock.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/dlmglue.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 717f8d3..da103f5 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2634,8 +2634,11 @@ int ocfs2_dentry_lock(struct dentry *dentry, int ex)
 
 	BUG_ON(!dl);
 
-	if (ocfs2_is_hard_readonly(osb))
-		return -EROFS;
+	if (ocfs2_is_hard_readonly(osb)) {
+		if (ex)
+			return -EROFS;
+		return 0;
+	}
 
 	if (ocfs2_mount_local(osb))
 		return 0;
@@ -2653,7 +2656,7 @@ void ocfs2_dentry_unlock(struct dentry *dentry, int ex)
 	struct ocfs2_dentry_lock *dl = dentry->d_fsdata;
 	struct ocfs2_super *osb = OCFS2_SB(dentry->d_sb);
 
-	if (!ocfs2_mount_local(osb))
+	if (!ocfs2_is_hard_readonly(osb) && !ocfs2_mount_local(osb))
 		ocfs2_cluster_unlock(osb, &dl->dl_lockres, level);
 }
 
-- 
1.7.4.4

On 05/26/2011 02:40 AM, Tiger Yang wrote:
> These four patches are all related to ocfs2 on hard readonly mount.
> patch 1 fix oops when umount ocfs2 on hard readonly device.
> Because ocfs2_dismount_volume() will call ocfs2_cluster_hangup() and 
> then call ocfs2_stack_driver_put(), will hit BUG_ON(active_stack == 
> NULL).
>
> patch 2 fix oops when do ls or cat in ocfs2 on hard readonly device.
> Because ocfs2_open_lock() will call ocfs2_cluster_lock() and then call 
> ocfs2_dlm_lock(), but active_stack is NULL.
>
> patch 3 fix bug of http://oss.oracle.com/bugzilla/show_bug.cgi?id=1322
> ocfs2_statfs, ocfs2_fiemap, ocfs2_get_acl, ocfs2_listxattr and 
> ocfs2_xattr_get need ocfs2_inode_lock to get dinode buffer head, but 
> on hard readonly mount, they did not get they expected, so lead to 
> oops. ocfs2_init_security_and_acl and ocfs2_test_inode_bit also called 
> ocfs2_inode_lock(inode, &bh, 0), but they are safe, because  
> ocfs2_reflink and ocfs2_get_dentry  check hard read only before them.
>
> patch 4 fix problem of running ls on hard readonly mount  can't get 
> the right result.
> Because ocfs2_dentry_lock() return -EROFS. It should return 0.
Tiger, Thanks. Was this tested with all fs-features enabled? I am
worried about quotas. If not, please test that.

Tristan, Please can you cross check these patches. Enable all features
in a volume, make it readonly and do mount, ls -lR, umount, read, stat,
stat -f. This calls for a test script. We should be able to use loopback.
losetup -rf.

Thanks
Sunil