ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

PATCH Superseded piaojun 4 participants 6 messages Dec 26, 2017 – Dec 27, 2017 OCFS2

Share & tools

piaojun

author

2017-12-26 11:09 UTC #0 lore raw

PATCH v2 1/1 ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

If metadata is corrupted such as 'invalid inode block', we will get failed by calling 'mount()' and then set filesystem readonly as below:

ocfs2_mount
ocfs2_initialize_super

    ocfs2_init_global_system_inodes
      ocfs2_iget
        ocfs2_read_locked_inode
          ocfs2_validate_inode_block
            ocfs2_error
              ocfs2_handle_error
                ocfs2_set_ro_flag(osb, 0);  // set readonly

In this situation we need return -EROFS to 'mount.ocfs2', so that user can fix it by fsck and mount again rather than doing meaningless retry. In addition, 'mount.ocfs2' should be updated correspondingly as it only return 1 for all errno. And I will post a patch for 'mount.ocfs2' too.

Signed-off-by Jun Piao <[email protected]>

Reviewed-by Alex Chen <[email protected]>

---
 fs/ocfs2/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/ocfs2/super.c +2 -2

index 040bbb6..f46f6a6 100644
@@ -474,7 +474,7 @@ static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb)
 		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
 		if (!new) {
 			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
 			mlog_errno(status);
 			/* FIXME: Should ERROR_RO_FS */
 			mlog(ML_ERROR, "Unable to load system inode %d, "

@@ -505,7 +505,7 @@ static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb)
 		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
 		if (!new) {
 			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
 			mlog(ML_ERROR, "status=%d, sysfile=%d, slot=%d\n",
 			     status, i, osb->slot_num);
 			goto bail;

Changwei Ge

2017-12-26 11:18 UTC #1 lore raw

PATCH v2 1/1 ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

Hi Jun,

Quoted 29 lines · piaojun

If metadata is corrupted such as 'invalid inode block', we will get failed by calling 'mount()' and then set filesystem readonly as below:
ocfs2_mount
ocfs2_initialize_super
     ocfs2_init_global_system_inodes
       ocfs2_iget
         ocfs2_read_locked_inode
           ocfs2_validate_inode_block
             ocfs2_error
               ocfs2_handle_error
                 ocfs2_set_ro_flag(osb, 0);  // set readonly
In this situation we need return -EROFS to 'mount.ocfs2', so that user can fix it by fsck and mount again rather than doing meaningless retry. In addition, 'mount.ocfs2' should be updated correspondingly as it only return 1 for all errno. And I will post a patch for 'mount.ocfs2' too.
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Alex Chen <[email protected]>
---
  fs/ocfs2/super.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
fs/ocfs2/super.c +0 -0
index 040bbb6..f46f6a6 100644

fs/ocfs2/super.c static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb) · line 479

Quoted 7 lines

fs/ocfs2/super.c +1 -1

@@ -474,7 +474,7 @@ static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb)
  		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
  		if (!new) {
  			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
  			mlog_errno(status);
  			/* FIXME: Should ERROR_RO_FS */

If your patch is applied, shall we remove above comment line?

Thanks,
Changwei

Quoted 11 lines

fs/ocfs2/super.c +1 -1

  			mlog(ML_ERROR, "Unable to load system inode %d, "

@@ -505,7 +505,7 @@ static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb)
  		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
  		if (!new) {
  			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
  			mlog(ML_ERROR, "status=%d, sysfile=%d, slot=%d\n",
  			     status, i, osb->slot_num);
  			goto bail;

piaojun

author

2017-12-26 11:22 UTC #2 lore raw

PATCH v2 1/1 ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

Hi Changwei,

Quoted 42 lines · Changwei Ge

Hi Jun,
Quoted 29 lines · piaojun
If metadata is corrupted such as 'invalid inode block', we will get failed by calling 'mount()' and then set filesystem readonly as below:
ocfs2_mount
ocfs2_initialize_super
     ocfs2_init_global_system_inodes
       ocfs2_iget
         ocfs2_read_locked_inode
           ocfs2_validate_inode_block
             ocfs2_error
               ocfs2_handle_error
                 ocfs2_set_ro_flag(osb, 0);  // set readonly
In this situation we need return -EROFS to 'mount.ocfs2', so that user can fix it by fsck and mount again rather than doing meaningless retry. In addition, 'mount.ocfs2' should be updated correspondingly as it only return 1 for all errno. And I will post a patch for 'mount.ocfs2' too.
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Alex Chen <[email protected]>
---
  fs/ocfs2/super.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
fs/ocfs2/super.c +0 -0
index 040bbb6..f46f6a6 100644
fs/ocfs2/super.c static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb) · line 479
Quoted 7 lines
fs/ocfs2/super.c +1 -1
@@ -474,7 +474,7 @@ static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb)
  		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
  		if (!new) {
  			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
  			mlog_errno(status);
  			/* FIXME: Should ERROR_RO_FS */
If your patch is applied, shall we remove above comment line?

Quoted 3 lines

Thanks,
Changwei

Good suggestion! If nobody rejects it, I will remove that comment.

thanks,
Jun

Quoted 14 lines

fs/ocfs2/super.c static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb) · line 511

Quoted 11 lines

fs/ocfs2/super.c +1 -1

  			mlog(ML_ERROR, "Unable to load system inode %d, "

@@ -505,7 +505,7 @@ static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb)
  		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
  		if (!new) {
  			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
  			mlog(ML_ERROR, "status=%d, sysfile=%d, slot=%d\n",
  			     status, i, osb->slot_num);
  			goto bail;

Changwei Ge

2017-12-26 11:27 UTC #3 lore raw

PATCH v2 1/1 ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

Quoted 45 lines · piaojun

Hi Changwei,
Quoted 42 lines · Changwei Ge
Hi Jun,
Quoted 29 lines · piaojun
If metadata is corrupted such as 'invalid inode block', we will get failed by calling 'mount()' and then set filesystem readonly as below:
ocfs2_mount
    ocfs2_initialize_super
      ocfs2_init_global_system_inodes
        ocfs2_iget
          ocfs2_read_locked_inode
            ocfs2_validate_inode_block
              ocfs2_error
                ocfs2_handle_error
                  ocfs2_set_ro_flag(osb, 0);  // set readonly
In this situation we need return -EROFS to 'mount.ocfs2', so that user can fix it by fsck and mount again rather than doing meaningless retry. In addition, 'mount.ocfs2' should be updated correspondingly as it only return 1 for all errno. And I will post a patch for 'mount.ocfs2' too.
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Alex Chen <[email protected]>
---
   fs/ocfs2/super.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
fs/ocfs2/super.c +0 -0
index 040bbb6..f46f6a6 100644
fs/ocfs2/super.c static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb) · line 479
Quoted 7 lines
fs/ocfs2/super.c +1 -1
@@ -474,7 +474,7 @@ static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb)
   		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
   		if (!new) {
   			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
   			mlog_errno(status);
   			/* FIXME: Should ERROR_RO_FS */
If your patch is applied, shall we remove above comment line?

Quoted 4 lines

Quoted 3 lines
Thanks,
Changwei
Good suggestion! If nobody rejects it, I will remove that comment.

After removing that comment line, please feel free to use my

Reviewed-by Changwei Ge <[email protected]>

Thanks,
Changwei

Quoted 19 lines

thanks,
Jun

Quoted 14 lines

fs/ocfs2/super.c static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb) · line 511

Quoted 11 lines

fs/ocfs2/super.c +1 -1

   			mlog(ML_ERROR, "Unable to load system inode %d, "

@@ -505,7 +505,7 @@ static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb)
   		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
   		if (!new) {
   			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
   			mlog(ML_ERROR, "status=%d, sysfile=%d, slot=%d\n",
   			     status, i, osb->slot_num);
   			goto bail;

Joseph Qi

2017-12-26 11:31 UTC #4 lore raw

PATCH v2 1/1 ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

In reply to piaojun

Quoted 13 lines · piaojun

If metadata is corrupted such as 'invalid inode block', we will get failed by calling 'mount()' and then set filesystem readonly as below:
ocfs2_mount
ocfs2_initialize_super
    ocfs2_init_global_system_inodes
      ocfs2_iget
        ocfs2_read_locked_inode
          ocfs2_validate_inode_block
            ocfs2_error
              ocfs2_handle_error
                ocfs2_set_ro_flag(osb, 0);  // set readonly

Quoted 7 lines

In this situation we need return -EROFS to 'mount.ocfs2', so that user can fix it by fsck and mount again rather than doing meaningless retry. In addition, 'mount.ocfs2' should be updated correspondingly as it only return 1 for all errno. And I will post a patch for 'mount.ocfs2' too.
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Alex Chen <[email protected]>

Looks good to me.

Reviewed-by Joseph Qi <[email protected]>

Quoted 27 lines

---
 fs/ocfs2/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/ocfs2/super.c +2 -2

index 040bbb6..f46f6a6 100644
@@ -474,7 +474,7 @@ static int ocfs2_init_global_system_inodes(struct ocfs2_super *osb)
 		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
 		if (!new) {
 			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
 			mlog_errno(status);
 			/* FIXME: Should ERROR_RO_FS */
 			mlog(ML_ERROR, "Unable to load system inode %d, "

@@ -505,7 +505,7 @@ static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb)
 		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
 		if (!new) {
 			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
 			mlog(ML_ERROR, "status=%d, sysfile=%d, slot=%d\n",
 			     status, i, osb->slot_num);
 			goto bail;

Gang He

2017-12-27 01:59 UTC #5 lore raw

PATCH v2 1/1 ocfs2: return -EROFS to mount.ocfs2 if inode block is invalid

Quoted 25 lines

Quoted 4 lines
Quoted 4 lines
Quoted 18 lines · piaojun
If metadata is corrupted such as 'invalid inode block', we will get failed by calling 'mount()' and then set filesystem readonly as below:
ocfs2_mount
ocfs2_initialize_super
    ocfs2_init_global_system_inodes
      ocfs2_iget
        ocfs2_read_locked_inode
          ocfs2_validate_inode_block
            ocfs2_error
              ocfs2_handle_error
                ocfs2_set_ro_flag(osb, 0);  // set readonly
In this situation we need return -EROFS to 'mount.ocfs2', so that user can fix it by fsck and mount again rather than doing meaningless retry. In addition, 'mount.ocfs2' should be updated correspondingly as it only return 1 for all errno. And I will post a patch for 'mount.ocfs2' too.

Quoted 4 lines

Quoted 2 lines
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Alex Chen <[email protected]>
Looks good to me.
Reviewed-by Joseph Qi <[email protected]>

Reviewed-by Gang He <[email protected]>

Quoted 35 lines

fs/ocfs2/super.c static int ocfs2_init_global_system_inodes(struct
Quoted 10 lines
---
 fs/ocfs2/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/ocfs2/super.c +0 -0
index 040bbb6..f46f6a6 100644
@@ -474,7 +474,7 @@ static int ocfs2_init_global_system_inodes(struct 
ocfs2_super *osb)
fs/ocfs2/super.c static int ocfs2_init_local_system_inodes(struct
Quoted 9 lines
fs/ocfs2/super.c +1 -1
 		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
 		if (!new) {
 			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
 			mlog_errno(status);
 			/* FIXME: Should ERROR_RO_FS */
 			mlog(ML_ERROR, "Unable to load system inode %d, "
@@ -505,7 +505,7 @@ static int ocfs2_init_local_system_inodes(struct 
ocfs2_super *osb)
fs/ocfs2/super.c static int ocfs2_init_local_system_inodes(struct ocfs2_super *osb) · line 511
Quoted 9 lines
fs/ocfs2/super.c +1 -1
 		new = ocfs2_get_system_file_inode(osb, i, osb->slot_num);
 		if (!new) {
 			ocfs2_release_system_inodes(osb);
-			status = -EINVAL;
+			status = ocfs2_is_soft_readonly(osb) ? -EROFS : -EINVAL;
 			mlog(ML_ERROR, "status=%d, sysfile=%d, slot=%d\n",
 			     status, i, osb->slot_num);
 			goto bail;
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel