All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_repair: refuse to unset lazycount on V5 filesystems
@ 2015-06-18 17:01 Eric Sandeen
  2015-06-19 13:19 ` Brian Foster
  0 siblings, 1 reply; 2+ messages in thread
From: Eric Sandeen @ 2015-06-18 17:01 UTC (permalink / raw)
  To: xfs-oss; +Cc: Richard W.M. Jones

Running "xfs_admin -c 0" on a V5 fs has unfortunate results.

Before:
versionnum [0xbca5+0x18a] = ... LAZYSBCOUNT,PROJID32BIT,CRC, ...
After:
versionnum [0xbca5+0x188] = ... LAZYSBCOUNT,PROJID32BIT,CRC, ...

The on-disk feature bit is in fact changed, but xfs_db reports that the
feature is still present.  This is because xfs_sb_version_haslazysbcount()
ignores the feature bit, and always reports that the lazy-count feature is
present on a V5 fs.

i.e. the intent is that this feature is not optional on a V5 fs, yet we
still flip the bit on disk when asked.  Not good.

So, document this, and error out if the user tries to disable lazy-count
on a version 5 filesystem.

Reported-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

(hm, xfs_repair should probably also turn the bit back on, if it's off on
V5?  I'll save that for another patch.)

diff --git a/man/man8/xfs_admin.8 b/man/man8/xfs_admin.8
index b393d74..0968196 100644
--- a/man/man8/xfs_admin.8
+++ b/man/man8/xfs_admin.8
@@ -62,6 +62,10 @@ Print the current filesystem UUID (Universally Unique IDentifier).
 .TP
 .BR "\-c 0" | 1
 Enable (1) or disable (0) lazy-counters in the filesystem.
+.IP
+Lazy-counters may not be disabled on Version 5 superblock filesystems
+(i.e. those with metadata CRCs enabled).
+.IP
 This operation may take quite a bit of time on large filesystems as the
 entire filesystem needs to be scanned when this option is changed.
 .IP

diff --git a/repair/phase1.c b/repair/phase1.c
index ec75ada..9e7011d 100644
--- a/repair/phase1.c
+++ b/repair/phase1.c
@@ -121,8 +121,12 @@ phase1(xfs_mount_t *mp)
 			sb->sb_bad_features2 |= XFS_SB_VERSION2_LAZYSBCOUNTBIT;
 			primary_sb_modified = 1;
 			printf(_("Enabling lazy-counters\n"));
-		} else
-		if (!lazy_count && xfs_sb_version_haslazysbcount(sb)) {
+		} else if (!lazy_count && xfs_sb_version_haslazysbcount(sb)) {
+			if (XFS_SB_VERSION_NUM(sb) == XFS_SB_VERSION_5) {
+				printf(
+_("Cannot disable lazy-counters on V5 fs\n"));
+				exit(1);
+			}
 			sb->sb_features2 &= ~XFS_SB_VERSION2_LAZYSBCOUNTBIT;
 			sb->sb_bad_features2 &= ~XFS_SB_VERSION2_LAZYSBCOUNTBIT;
 			printf(_("Disabling lazy-counters\n"));


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] xfs_repair: refuse to unset lazycount on V5 filesystems
  2015-06-18 17:01 [PATCH] xfs_repair: refuse to unset lazycount on V5 filesystems Eric Sandeen
@ 2015-06-19 13:19 ` Brian Foster
  0 siblings, 0 replies; 2+ messages in thread
From: Brian Foster @ 2015-06-19 13:19 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Richard W.M. Jones, xfs-oss

On Thu, Jun 18, 2015 at 12:01:22PM -0500, Eric Sandeen wrote:
> Running "xfs_admin -c 0" on a V5 fs has unfortunate results.
> 
> Before:
> versionnum [0xbca5+0x18a] = ... LAZYSBCOUNT,PROJID32BIT,CRC, ...
> After:
> versionnum [0xbca5+0x188] = ... LAZYSBCOUNT,PROJID32BIT,CRC, ...
> 
> The on-disk feature bit is in fact changed, but xfs_db reports that the
> feature is still present.  This is because xfs_sb_version_haslazysbcount()
> ignores the feature bit, and always reports that the lazy-count feature is
> present on a V5 fs.
> 
> i.e. the intent is that this feature is not optional on a V5 fs, yet we
> still flip the bit on disk when asked.  Not good.
> 
> So, document this, and error out if the user tries to disable lazy-count
> on a version 5 filesystem.
> 
> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> (hm, xfs_repair should probably also turn the bit back on, if it's off on
> V5?  I'll save that for another patch.)
> 

Seems reasonable.

Brian

> diff --git a/man/man8/xfs_admin.8 b/man/man8/xfs_admin.8
> index b393d74..0968196 100644
> --- a/man/man8/xfs_admin.8
> +++ b/man/man8/xfs_admin.8
> @@ -62,6 +62,10 @@ Print the current filesystem UUID (Universally Unique IDentifier).
>  .TP
>  .BR "\-c 0" | 1
>  Enable (1) or disable (0) lazy-counters in the filesystem.
> +.IP
> +Lazy-counters may not be disabled on Version 5 superblock filesystems
> +(i.e. those with metadata CRCs enabled).
> +.IP
>  This operation may take quite a bit of time on large filesystems as the
>  entire filesystem needs to be scanned when this option is changed.
>  .IP
> 
> diff --git a/repair/phase1.c b/repair/phase1.c
> index ec75ada..9e7011d 100644
> --- a/repair/phase1.c
> +++ b/repair/phase1.c
> @@ -121,8 +121,12 @@ phase1(xfs_mount_t *mp)
>  			sb->sb_bad_features2 |= XFS_SB_VERSION2_LAZYSBCOUNTBIT;
>  			primary_sb_modified = 1;
>  			printf(_("Enabling lazy-counters\n"));
> -		} else
> -		if (!lazy_count && xfs_sb_version_haslazysbcount(sb)) {
> +		} else if (!lazy_count && xfs_sb_version_haslazysbcount(sb)) {
> +			if (XFS_SB_VERSION_NUM(sb) == XFS_SB_VERSION_5) {
> +				printf(
> +_("Cannot disable lazy-counters on V5 fs\n"));
> +				exit(1);
> +			}
>  			sb->sb_features2 &= ~XFS_SB_VERSION2_LAZYSBCOUNTBIT;
>  			sb->sb_bad_features2 &= ~XFS_SB_VERSION2_LAZYSBCOUNTBIT;
>  			printf(_("Disabling lazy-counters\n"));
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-06-19 13:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-18 17:01 [PATCH] xfs_repair: refuse to unset lazycount on V5 filesystems Eric Sandeen
2015-06-19 13:19 ` Brian Foster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.