* [PATCH 1/2] xfs_fsr: extra debugging info
@ 2012-03-30 14:54 Eric Sandeen
2012-03-30 15:01 ` [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff Eric Sandeen
0 siblings, 1 reply; 7+ messages in thread
From: Eric Sandeen @ 2012-03-30 14:54 UTC (permalink / raw)
To: xfs-oss
Provide 2 new pieces of information when -d is specified to
xfs_fsr:
* If we needed to grow the forkoffset and couldn't
* If we were unable to match the fork offset
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 2db2224..d83bdc9 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1021,6 +1021,7 @@ fsr_setup_attr_fork(
{
struct stat64 tstatbuf;
int i;
+ int diff = 0;
int last_forkoff = 0;
int no_change_cnt = 0;
int ret;
@@ -1056,7 +1057,6 @@ fsr_setup_attr_fork(
xfs_bstat_t tbstat;
xfs_ino_t ino;
char name[64];
- int diff;
/*
* bulkstat the temp inode to see what the forkoff is. Use
@@ -1122,6 +1122,8 @@ fsr_setup_attr_fork(
* non-contiguous offsets.
*/
/* XXX: unimplemented! */
+ if (dflag)
+ printf(_("data fork growth unimplemented\n"));
goto out;
}
@@ -1137,6 +1139,10 @@ fsr_setup_attr_fork(
out:
if (dflag)
fsrprintf(_("set temp attr\n"));
+ /* We failed to resolve the fork difference */
+ if (dflag && diff)
+ fsrprintf(_("failed to match fork offset\n"));;
+
return 0;
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff
2012-03-30 14:54 [PATCH 1/2] xfs_fsr: extra debugging info Eric Sandeen
@ 2012-03-30 15:01 ` Eric Sandeen
2012-03-30 18:11 ` Eric Sandeen
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Eric Sandeen @ 2012-03-30 15:01 UTC (permalink / raw)
To: xfs-oss; +Cc: Gabriel VLASIU
In some cases the target file may have a non-local attribute,
but the temp file gets assigned a local attribute on creation,
due to selinux, for example.
In this case, the large-ish selinux attr will create a forkoff
in the temp file smaller than the forkoff in the target file,
because the FMT_EXTENTS attr takes up less space. There is
no mechanism to grow the forkoff to match, so we can end up
failing to swap these 2 inodes if the result is not enough
data space in the temp inode as a result.
After testing the target file for a non-local extent, and
checking to see if the forkoff needs to be grown on the first
pass, we can add a large attr to knock all attributes of the
temp file out of local format, and grow the fork offset.
This passes xfstest 227, and also resolves issues seen on
a metadata image provided by Gabriel.
Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index d83bdc9..e8ab028 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1022,6 +1022,8 @@ fsr_setup_attr_fork(
struct stat64 tstatbuf;
int i;
int diff = 0;
+ struct fsxattr fsx;
+ int target_attr_local = 0;
int last_forkoff = 0;
int no_change_cnt = 0;
int ret;
@@ -1052,6 +1054,11 @@ fsr_setup_attr_fork(
return -1;
}
+ memset(&fsx, 0, sizeof(fsx));
+ ioctl(fd, XFS_IOC_FSGETXATTR, &fsx);
+ if (fsx.fsx_nextents == 0)
+ target_attr_local = 1;
+
i = 0;
do {
xfs_bstat_t tbstat;
@@ -1072,6 +1079,7 @@ fsr_setup_attr_fork(
if (dflag)
fsrprintf(_("orig forkoff %d, temp forkoff %d\n"),
bstatp->bs_forkoff, tbstat.bs_forkoff);
+ diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
snprintf(name, sizeof(name), "user.%d", i);
@@ -1086,6 +1094,21 @@ fsr_setup_attr_fork(
return -1;
}
continue;
+ } else if (i == 0 && diff < 0 && target_attr_local == 0) {
+ /*
+ * A small attr may exist from eg selinux but, if the
+ * target is not local, write a big attr to tempfile
+ * to knock it out of local format to match target.
+ * This should actually increase the temp forkoffset.
+ */
+ char val[2048];
+ memset(val, 'X', 2048);
+ ret = fsetxattr(tfd, name, val, 2048, XATTR_CREATE);
+ if (ret) {
+ fsrprintf(_("could not set large ATTR\n"));
+ return -1;
+ }
+ continue;
}
/*
@@ -1100,7 +1123,6 @@ fsr_setup_attr_fork(
last_forkoff = tbstat.bs_forkoff;
/* work out which way to grow the fork */
- diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
if (abs(diff) > fsgeom.inodesize - sizeof(struct xfs_dinode)) {
fsrprintf(_("forkoff diff %d too large!\n"), diff);
return -1;
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff
2012-03-30 15:01 ` [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff Eric Sandeen
@ 2012-03-30 18:11 ` Eric Sandeen
2012-05-12 1:30 ` Eric Sandeen
2013-10-10 3:11 ` Eric Sandeen
2 siblings, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2012-03-30 18:11 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Gabriel VLASIU, xfs-oss
On 3/30/12 10:01 AM, Eric Sandeen wrote:
> In some cases the target file may have a non-local attribute,
> but the temp file gets assigned a local attribute on creation,
> due to selinux, for example.
>
> In this case, the large-ish selinux attr will create a forkoff
> in the temp file smaller than the forkoff in the target file,
> because the FMT_EXTENTS attr takes up less space. There is
> no mechanism to grow the forkoff to match, so we can end up
> failing to swap these 2 inodes if the result is not enough
> data space in the temp inode as a result.
>
> After testing the target file for a non-local extent, and
sorry that should read "non-local attribute."
> checking to see if the forkoff needs to be grown on the first
> pass, we can add a large attr to knock all attributes of the
> temp file out of local format, and grow the fork offset.
>
> This passes xfstest 227, and also resolves issues seen on
> a metadata image provided by Gabriel.
>
> Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index d83bdc9..e8ab028 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -1022,6 +1022,8 @@ fsr_setup_attr_fork(
> struct stat64 tstatbuf;
> int i;
> int diff = 0;
> + struct fsxattr fsx;
> + int target_attr_local = 0;
> int last_forkoff = 0;
> int no_change_cnt = 0;
> int ret;
> @@ -1052,6 +1054,11 @@ fsr_setup_attr_fork(
> return -1;
> }
>
> + memset(&fsx, 0, sizeof(fsx));
> + ioctl(fd, XFS_IOC_FSGETXATTR, &fsx);
> + if (fsx.fsx_nextents == 0)
> + target_attr_local = 1;
> +
> i = 0;
> do {
> xfs_bstat_t tbstat;
> @@ -1072,6 +1079,7 @@ fsr_setup_attr_fork(
> if (dflag)
> fsrprintf(_("orig forkoff %d, temp forkoff %d\n"),
> bstatp->bs_forkoff, tbstat.bs_forkoff);
> + diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
>
> snprintf(name, sizeof(name), "user.%d", i);
>
> @@ -1086,6 +1094,21 @@ fsr_setup_attr_fork(
> return -1;
> }
> continue;
> + } else if (i == 0 && diff < 0 && target_attr_local == 0) {
> + /*
> + * A small attr may exist from eg selinux but, if the
> + * target is not local, write a big attr to tempfile
> + * to knock it out of local format to match target.
> + * This should actually increase the temp forkoffset.
> + */
> + char val[2048];
> + memset(val, 'X', 2048);
> + ret = fsetxattr(tfd, name, val, 2048, XATTR_CREATE);
> + if (ret) {
> + fsrprintf(_("could not set large ATTR\n"));
> + return -1;
> + }
> + continue;
> }
>
> /*
> @@ -1100,7 +1123,6 @@ fsr_setup_attr_fork(
> last_forkoff = tbstat.bs_forkoff;
>
> /* work out which way to grow the fork */
> - diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
> if (abs(diff) > fsgeom.inodesize - sizeof(struct xfs_dinode)) {
> fsrprintf(_("forkoff diff %d too large!\n"), diff);
> return -1;
>
> _______________________________________________
> 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] 7+ messages in thread
* Re: [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff
2012-03-30 15:01 ` [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff Eric Sandeen
2012-03-30 18:11 ` Eric Sandeen
@ 2012-05-12 1:30 ` Eric Sandeen
2013-10-10 3:11 ` Eric Sandeen
2 siblings, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2012-05-12 1:30 UTC (permalink / raw)
Cc: xfs-oss
On 3/30/12 10:01 AM, Eric Sandeen wrote:
> In some cases the target file may have a non-local attribute,
> but the temp file gets assigned a local attribute on creation,
> due to selinux, for example.
>
> In this case, the large-ish selinux attr will create a forkoff
> in the temp file smaller than the forkoff in the target file,
> because the FMT_EXTENTS attr takes up less space. There is
> no mechanism to grow the forkoff to match, so we can end up
> failing to swap these 2 inodes if the result is not enough
> data space in the temp inode as a result.
>
> After testing the target file for a non-local extent, and
> checking to see if the forkoff needs to be grown on the first
> pass, we can add a large attr to knock all attributes of the
> temp file out of local format, and grow the fork offset.
>
> This passes xfstest 227, and also resolves issues seen on
> a metadata image provided by Gabriel.
Ping on this one? Would be nice to push it out.
-Eric
> Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index d83bdc9..e8ab028 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -1022,6 +1022,8 @@ fsr_setup_attr_fork(
> struct stat64 tstatbuf;
> int i;
> int diff = 0;
> + struct fsxattr fsx;
> + int target_attr_local = 0;
> int last_forkoff = 0;
> int no_change_cnt = 0;
> int ret;
> @@ -1052,6 +1054,11 @@ fsr_setup_attr_fork(
> return -1;
> }
>
> + memset(&fsx, 0, sizeof(fsx));
> + ioctl(fd, XFS_IOC_FSGETXATTR, &fsx);
> + if (fsx.fsx_nextents == 0)
> + target_attr_local = 1;
> +
> i = 0;
> do {
> xfs_bstat_t tbstat;
> @@ -1072,6 +1079,7 @@ fsr_setup_attr_fork(
> if (dflag)
> fsrprintf(_("orig forkoff %d, temp forkoff %d\n"),
> bstatp->bs_forkoff, tbstat.bs_forkoff);
> + diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
>
> snprintf(name, sizeof(name), "user.%d", i);
>
> @@ -1086,6 +1094,21 @@ fsr_setup_attr_fork(
> return -1;
> }
> continue;
> + } else if (i == 0 && diff < 0 && target_attr_local == 0) {
> + /*
> + * A small attr may exist from eg selinux but, if the
> + * target is not local, write a big attr to tempfile
> + * to knock it out of local format to match target.
> + * This should actually increase the temp forkoffset.
> + */
> + char val[2048];
> + memset(val, 'X', 2048);
> + ret = fsetxattr(tfd, name, val, 2048, XATTR_CREATE);
> + if (ret) {
> + fsrprintf(_("could not set large ATTR\n"));
> + return -1;
> + }
> + continue;
> }
>
> /*
> @@ -1100,7 +1123,6 @@ fsr_setup_attr_fork(
> last_forkoff = tbstat.bs_forkoff;
>
> /* work out which way to grow the fork */
> - diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
> if (abs(diff) > fsgeom.inodesize - sizeof(struct xfs_dinode)) {
> fsrprintf(_("forkoff diff %d too large!\n"), diff);
> return -1;
>
> _______________________________________________
> 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] 7+ messages in thread
* Re: [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff
2012-03-30 15:01 ` [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff Eric Sandeen
2012-03-30 18:11 ` Eric Sandeen
2012-05-12 1:30 ` Eric Sandeen
@ 2013-10-10 3:11 ` Eric Sandeen
2013-10-10 3:21 ` Dave Chinner
2 siblings, 1 reply; 7+ messages in thread
From: Eric Sandeen @ 2013-10-10 3:11 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
On 3/30/12 10:01 AM, Eric Sandeen wrote:
> In some cases the target file may have a non-local attribute,
> but the temp file gets assigned a local attribute on creation,
> due to selinux, for example.
>
> In this case, the large-ish selinux attr will create a forkoff
> in the temp file smaller than the forkoff in the target file,
> because the FMT_EXTENTS attr takes up less space. There is
> no mechanism to grow the forkoff to match, so we can end up
> failing to swap these 2 inodes if the result is not enough
> data space in the temp inode as a result.
>
> After testing the target file for a non-local extent, and
> checking to see if the forkoff needs to be grown on the first
> pass, we can add a large attr to knock all attributes of the
> temp file out of local format, and grow the fork offset.
>
> This passes xfstest 227, and also resolves issues seen on
> a metadata image provided by Gabriel.
>
> Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
Ping #2? This was a real bug once, IIRC. Probably still is...
Patch 1/2 helped identify the problem, so ping on that too I guess.
-Eric
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index d83bdc9..e8ab028 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -1022,6 +1022,8 @@ fsr_setup_attr_fork(
> struct stat64 tstatbuf;
> int i;
> int diff = 0;
> + struct fsxattr fsx;
> + int target_attr_local = 0;
> int last_forkoff = 0;
> int no_change_cnt = 0;
> int ret;
> @@ -1052,6 +1054,11 @@ fsr_setup_attr_fork(
> return -1;
> }
>
> + memset(&fsx, 0, sizeof(fsx));
> + ioctl(fd, XFS_IOC_FSGETXATTR, &fsx);
> + if (fsx.fsx_nextents == 0)
> + target_attr_local = 1;
> +
> i = 0;
> do {
> xfs_bstat_t tbstat;
> @@ -1072,6 +1079,7 @@ fsr_setup_attr_fork(
> if (dflag)
> fsrprintf(_("orig forkoff %d, temp forkoff %d\n"),
> bstatp->bs_forkoff, tbstat.bs_forkoff);
> + diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
>
> snprintf(name, sizeof(name), "user.%d", i);
>
> @@ -1086,6 +1094,21 @@ fsr_setup_attr_fork(
> return -1;
> }
> continue;
> + } else if (i == 0 && diff < 0 && target_attr_local == 0) {
> + /*
> + * A small attr may exist from eg selinux but, if the
> + * target is not local, write a big attr to tempfile
> + * to knock it out of local format to match target.
> + * This should actually increase the temp forkoffset.
> + */
> + char val[2048];
> + memset(val, 'X', 2048);
> + ret = fsetxattr(tfd, name, val, 2048, XATTR_CREATE);
> + if (ret) {
> + fsrprintf(_("could not set large ATTR\n"));
> + return -1;
> + }
> + continue;
> }
>
> /*
> @@ -1100,7 +1123,6 @@ fsr_setup_attr_fork(
> last_forkoff = tbstat.bs_forkoff;
>
> /* work out which way to grow the fork */
> - diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
> if (abs(diff) > fsgeom.inodesize - sizeof(struct xfs_dinode)) {
> fsrprintf(_("forkoff diff %d too large!\n"), diff);
> return -1;
>
> _______________________________________________
> 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] 7+ messages in thread
* Re: [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff
2013-10-10 3:11 ` Eric Sandeen
@ 2013-10-10 3:21 ` Dave Chinner
2013-10-10 3:24 ` Eric Sandeen
0 siblings, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2013-10-10 3:21 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss
On Wed, Oct 09, 2013 at 10:11:29PM -0500, Eric Sandeen wrote:
> On 3/30/12 10:01 AM, Eric Sandeen wrote:
> > In some cases the target file may have a non-local attribute,
> > but the temp file gets assigned a local attribute on creation,
> > due to selinux, for example.
> >
> > In this case, the large-ish selinux attr will create a forkoff
> > in the temp file smaller than the forkoff in the target file,
> > because the FMT_EXTENTS attr takes up less space. There is
> > no mechanism to grow the forkoff to match, so we can end up
> > failing to swap these 2 inodes if the result is not enough
> > data space in the temp inode as a result.
> >
> > After testing the target file for a non-local extent, and
> > checking to see if the forkoff needs to be grown on the first
> > pass, we can add a large attr to knock all attributes of the
> > temp file out of local format, and grow the fork offset.
> >
> > This passes xfstest 227, and also resolves issues seen on
> > a metadata image provided by Gabriel.
> >
> > Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > ---
>
> Ping #2? This was a real bug once, IIRC. Probably still is...
>
> Patch 1/2 helped identify the problem, so ping on that too I guess.
Can you repost all these old patches as a new series to make it easy
to apply and test them?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff
2013-10-10 3:21 ` Dave Chinner
@ 2013-10-10 3:24 ` Eric Sandeen
0 siblings, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2013-10-10 3:24 UTC (permalink / raw)
To: Dave Chinner; +Cc: Eric Sandeen, xfs-oss
On Oct 9, 2013, at 10:21 PM, Dave Chinner <david@fromorbit.com> wrote:
>
>> On Wed, Oct 09, 2013 at 10:11:29PM -0500, Eric Sandeen wrote:
>>> On 3/30/12 10:01 AM, Eric Sandeen wrote:
>>> In some cases the target file may have a non-local attribute,
>>> but the temp file gets assigned a local attribute on creation,
>>> due to selinux, for example.
>>>
>>> In this case, the large-ish selinux attr will create a forkoff
>>> in the temp file smaller than the forkoff in the target file,
>>> because the FMT_EXTENTS attr takes up less space. There is
>>> no mechanism to grow the forkoff to match, so we can end up
>>> failing to swap these 2 inodes if the result is not enough
>>> data space in the temp inode as a result.
>>>
>>> After testing the target file for a non-local extent, and
>>> checking to see if the forkoff needs to be grown on the first
>>> pass, we can add a large attr to knock all attributes of the
>>> temp file out of local format, and grow the fork offset.
>>>
>>> This passes xfstest 227, and also resolves issues seen on
>>> a metadata image provided by Gabriel.
>>>
>>> Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>
>> Ping #2? This was a real bug once, IIRC. Probably still is...
>>
>> Patch 1/2 helped identify the problem, so ping on that too I guess.
>
> Can you repost all these old patches as a new series to make it easy
> to apply and test them?
>
Ok, good idea.
Eric
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-10-10 3:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-30 14:54 [PATCH 1/2] xfs_fsr: extra debugging info Eric Sandeen
2012-03-30 15:01 ` [PATCH 2/2] xfs_fsr: create extent-based attr to grow forkoff Eric Sandeen
2012-03-30 18:11 ` Eric Sandeen
2012-05-12 1:30 ` Eric Sandeen
2013-10-10 3:11 ` Eric Sandeen
2013-10-10 3:21 ` Dave Chinner
2013-10-10 3:24 ` Eric Sandeen
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.