* [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-04 18:51 ` Ramon de Carvalho Valle
0 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-04 18:51 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, xfs, hch, linux-kernel, mszeredi
[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]
The xfs_iformat function does not check if the realtime device target pointer
is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
structure.
Signed-off-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
Cc: stable <stable@kernel.org>
---
fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
1 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 1f22d65..37d3ee5 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -343,13 +343,24 @@ xfs_iformat(
return XFS_ERROR(EFSCORRUPTED);
}
+ if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
+ !ip->i_mount->m_rtdev_targp)) {
+ xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
+ "corrupt dinode %Lu, flags = 0x%x.",
+ (unsigned long long)ip->i_ino,
+ ip->i_d.di_flags);
+ XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
+ ip->i_mount, dip);
+ return XFS_ERROR(EFSCORRUPTED);
+ }
+
switch (ip->i_d.di_mode & S_IFMT) {
case S_IFIFO:
case S_IFCHR:
case S_IFBLK:
case S_IFSOCK:
if (unlikely(dip->di_format != XFS_DINODE_FMT_DEV)) {
- XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
+ XFS_CORRUPTION_ERROR("xfs_iformat(4)", XFS_ERRLEVEL_LOW,
ip->i_mount, dip);
return XFS_ERROR(EFSCORRUPTED);
}
@@ -371,7 +382,7 @@ xfs_iformat(
"corrupt inode %Lu "
"(local format for regular file).",
(unsigned long long) ip->i_ino);
- XFS_CORRUPTION_ERROR("xfs_iformat(4)",
+ XFS_CORRUPTION_ERROR("xfs_iformat(5)",
XFS_ERRLEVEL_LOW,
ip->i_mount, dip);
return XFS_ERROR(EFSCORRUPTED);
@@ -384,7 +395,7 @@ xfs_iformat(
"(bad size %Ld for local inode).",
(unsigned long long) ip->i_ino,
(long long) di_size);
- XFS_CORRUPTION_ERROR("xfs_iformat(5)",
+ XFS_CORRUPTION_ERROR("xfs_iformat(6)",
XFS_ERRLEVEL_LOW,
ip->i_mount, dip);
return XFS_ERROR(EFSCORRUPTED);
@@ -400,14 +411,14 @@ xfs_iformat(
error = xfs_iformat_btree(ip, dip, XFS_DATA_FORK);
break;
default:
- XFS_ERROR_REPORT("xfs_iformat(6)", XFS_ERRLEVEL_LOW,
+ XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW,
ip->i_mount);
return XFS_ERROR(EFSCORRUPTED);
}
break;
default:
- XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW, ip->i_mount);
+ XFS_ERROR_REPORT("xfs_iformat(8)", XFS_ERRLEVEL_LOW, ip->i_mount);
return XFS_ERROR(EFSCORRUPTED);
}
if (error) {
@@ -430,7 +441,7 @@ xfs_iformat(
"(bad attr fork size %Ld).",
(unsigned long long) ip->i_ino,
(long long) size);
- XFS_CORRUPTION_ERROR("xfs_iformat(8)",
+ XFS_CORRUPTION_ERROR("xfs_iformat(9)",
XFS_ERRLEVEL_LOW,
ip->i_mount, dip);
return XFS_ERROR(EFSCORRUPTED);
--
1.5.6.3
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-04 18:51 ` Ramon de Carvalho Valle
@ 2009-08-04 19:11 ` Eric Sandeen
-1 siblings, 0 replies; 28+ messages in thread
From: Eric Sandeen @ 2009-08-04 19:11 UTC (permalink / raw)
To: Ramon de Carvalho Valle
Cc: Christoph Hellwig, linux-kernel, mszeredi, hch, xfs
Ramon de Carvalho Valle wrote:
> The xfs_iformat function does not check if the realtime device target pointer
> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> structure.
>
> Signed-off-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
> Cc: stable <stable@kernel.org>
> ---
> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
> 1 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 1f22d65..37d3ee5 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -343,13 +343,24 @@ xfs_iformat(
> return XFS_ERROR(EFSCORRUPTED);
> }
>
> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
> + !ip->i_mount->m_rtdev_targp)) {
> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
> + "corrupt dinode %Lu, flags = 0x%x.",
> + (unsigned long long)ip->i_ino,
> + ip->i_d.di_flags);
> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> + ip->i_mount, dip);
I think I'd rather not change all the corruption text tag ordering;
it'll make it harder to track down any common occurrences of
"xfs_iformat(X)" corruption in the future if they get renumbered now.
I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
had. "realtime" is a lot more informative than "3" anyway.
-Eric
> + return XFS_ERROR(EFSCORRUPTED);
> + }
> +
> switch (ip->i_d.di_mode & S_IFMT) {
> case S_IFIFO:
> case S_IFCHR:
> case S_IFBLK:
> case S_IFSOCK:
> if (unlikely(dip->di_format != XFS_DINODE_FMT_DEV)) {
> - XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> + XFS_CORRUPTION_ERROR("xfs_iformat(4)", XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
> }
> @@ -371,7 +382,7 @@ xfs_iformat(
> "corrupt inode %Lu "
> "(local format for regular file).",
> (unsigned long long) ip->i_ino);
> - XFS_CORRUPTION_ERROR("xfs_iformat(4)",
> + XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
> @@ -384,7 +395,7 @@ xfs_iformat(
> "(bad size %Ld for local inode).",
> (unsigned long long) ip->i_ino,
> (long long) di_size);
> - XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> + XFS_CORRUPTION_ERROR("xfs_iformat(6)",
> XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
> @@ -400,14 +411,14 @@ xfs_iformat(
> error = xfs_iformat_btree(ip, dip, XFS_DATA_FORK);
> break;
> default:
> - XFS_ERROR_REPORT("xfs_iformat(6)", XFS_ERRLEVEL_LOW,
> + XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW,
> ip->i_mount);
> return XFS_ERROR(EFSCORRUPTED);
> }
> break;
>
> default:
> - XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW, ip->i_mount);
> + XFS_ERROR_REPORT("xfs_iformat(8)", XFS_ERRLEVEL_LOW, ip->i_mount);
> return XFS_ERROR(EFSCORRUPTED);
> }
> if (error) {
> @@ -430,7 +441,7 @@ xfs_iformat(
> "(bad attr fork size %Ld).",
> (unsigned long long) ip->i_ino,
> (long long) size);
> - XFS_CORRUPTION_ERROR("xfs_iformat(8)",
> + XFS_CORRUPTION_ERROR("xfs_iformat(9)",
> XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-04 19:11 ` Eric Sandeen
0 siblings, 0 replies; 28+ messages in thread
From: Eric Sandeen @ 2009-08-04 19:11 UTC (permalink / raw)
To: Ramon de Carvalho Valle
Cc: Christoph Hellwig, xfs, hch, linux-kernel, mszeredi
Ramon de Carvalho Valle wrote:
> The xfs_iformat function does not check if the realtime device target pointer
> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> structure.
>
> Signed-off-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
> Cc: stable <stable@kernel.org>
> ---
> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
> 1 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 1f22d65..37d3ee5 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -343,13 +343,24 @@ xfs_iformat(
> return XFS_ERROR(EFSCORRUPTED);
> }
>
> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
> + !ip->i_mount->m_rtdev_targp)) {
> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
> + "corrupt dinode %Lu, flags = 0x%x.",
> + (unsigned long long)ip->i_ino,
> + ip->i_d.di_flags);
> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> + ip->i_mount, dip);
I think I'd rather not change all the corruption text tag ordering;
it'll make it harder to track down any common occurrences of
"xfs_iformat(X)" corruption in the future if they get renumbered now.
I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
had. "realtime" is a lot more informative than "3" anyway.
-Eric
> + return XFS_ERROR(EFSCORRUPTED);
> + }
> +
> switch (ip->i_d.di_mode & S_IFMT) {
> case S_IFIFO:
> case S_IFCHR:
> case S_IFBLK:
> case S_IFSOCK:
> if (unlikely(dip->di_format != XFS_DINODE_FMT_DEV)) {
> - XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> + XFS_CORRUPTION_ERROR("xfs_iformat(4)", XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
> }
> @@ -371,7 +382,7 @@ xfs_iformat(
> "corrupt inode %Lu "
> "(local format for regular file).",
> (unsigned long long) ip->i_ino);
> - XFS_CORRUPTION_ERROR("xfs_iformat(4)",
> + XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
> @@ -384,7 +395,7 @@ xfs_iformat(
> "(bad size %Ld for local inode).",
> (unsigned long long) ip->i_ino,
> (long long) di_size);
> - XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> + XFS_CORRUPTION_ERROR("xfs_iformat(6)",
> XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
> @@ -400,14 +411,14 @@ xfs_iformat(
> error = xfs_iformat_btree(ip, dip, XFS_DATA_FORK);
> break;
> default:
> - XFS_ERROR_REPORT("xfs_iformat(6)", XFS_ERRLEVEL_LOW,
> + XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW,
> ip->i_mount);
> return XFS_ERROR(EFSCORRUPTED);
> }
> break;
>
> default:
> - XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW, ip->i_mount);
> + XFS_ERROR_REPORT("xfs_iformat(8)", XFS_ERRLEVEL_LOW, ip->i_mount);
> return XFS_ERROR(EFSCORRUPTED);
> }
> if (error) {
> @@ -430,7 +441,7 @@ xfs_iformat(
> "(bad attr fork size %Ld).",
> (unsigned long long) ip->i_ino,
> (long long) size);
> - XFS_CORRUPTION_ERROR("xfs_iformat(8)",
> + XFS_CORRUPTION_ERROR("xfs_iformat(9)",
> XFS_ERRLEVEL_LOW,
> ip->i_mount, dip);
> return XFS_ERROR(EFSCORRUPTED);
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-04 19:11 ` Eric Sandeen
@ 2009-08-05 3:55 ` Ramon de Carvalho Valle
-1 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-05 3:55 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, linux-kernel, mszeredi, hch, xfs
[-- Attachment #1.1: Type: text/plain, Size: 3959 bytes --]
On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
> Ramon de Carvalho Valle wrote:
> > The xfs_iformat function does not check if the realtime device target pointer
> > is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> > structure.
> >
> > Signed-off-by: Christoph Hellwig <hch@infradead.org>
> > Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
> > Cc: stable <stable@kernel.org>
> > ---
> > fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
> > 1 files changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 1f22d65..37d3ee5 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -343,13 +343,24 @@ xfs_iformat(
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> >
> > + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
> > + !ip->i_mount->m_rtdev_targp)) {
> > + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
> > + "corrupt dinode %Lu, flags = 0x%x.",
> > + (unsigned long long)ip->i_ino,
> > + ip->i_d.di_flags);
> > + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> > + ip->i_mount, dip);
>
> I think I'd rather not change all the corruption text tag ordering;
> it'll make it harder to track down any common occurrences of
> "xfs_iformat(X)" corruption in the future if they get renumbered now.
>
> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
> had. "realtime" is a lot more informative than "3" anyway.
I don't think this is a bad decision, because the corruption errors can
be easily identified by the output of xfs_fs_repair_cmn_err and the
source line. I think this is a reasonable change that will keep the code
clean and consistent.
-Ramon
>
> -Eric
>
> > + return XFS_ERROR(EFSCORRUPTED);
> > + }
> > +
> > switch (ip->i_d.di_mode & S_IFMT) {
> > case S_IFIFO:
> > case S_IFCHR:
> > case S_IFBLK:
> > case S_IFSOCK:
> > if (unlikely(dip->di_format != XFS_DINODE_FMT_DEV)) {
> > - XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> > + XFS_CORRUPTION_ERROR("xfs_iformat(4)", XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> > @@ -371,7 +382,7 @@ xfs_iformat(
> > "corrupt inode %Lu "
> > "(local format for regular file).",
> > (unsigned long long) ip->i_ino);
> > - XFS_CORRUPTION_ERROR("xfs_iformat(4)",
> > + XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> > XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
> > @@ -384,7 +395,7 @@ xfs_iformat(
> > "(bad size %Ld for local inode).",
> > (unsigned long long) ip->i_ino,
> > (long long) di_size);
> > - XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> > + XFS_CORRUPTION_ERROR("xfs_iformat(6)",
> > XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
> > @@ -400,14 +411,14 @@ xfs_iformat(
> > error = xfs_iformat_btree(ip, dip, XFS_DATA_FORK);
> > break;
> > default:
> > - XFS_ERROR_REPORT("xfs_iformat(6)", XFS_ERRLEVEL_LOW,
> > + XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW,
> > ip->i_mount);
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> > break;
> >
> > default:
> > - XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW, ip->i_mount);
> > + XFS_ERROR_REPORT("xfs_iformat(8)", XFS_ERRLEVEL_LOW, ip->i_mount);
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> > if (error) {
> > @@ -430,7 +441,7 @@ xfs_iformat(
> > "(bad attr fork size %Ld).",
> > (unsigned long long) ip->i_ino,
> > (long long) size);
> > - XFS_CORRUPTION_ERROR("xfs_iformat(8)",
> > + XFS_CORRUPTION_ERROR("xfs_iformat(9)",
> > XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
>
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 121 bytes --]
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-05 3:55 ` Ramon de Carvalho Valle
0 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-05 3:55 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, xfs, hch, linux-kernel, mszeredi
[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]
On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
> Ramon de Carvalho Valle wrote:
> > The xfs_iformat function does not check if the realtime device target pointer
> > is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> > structure.
> >
> > Signed-off-by: Christoph Hellwig <hch@infradead.org>
> > Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
> > Cc: stable <stable@kernel.org>
> > ---
> > fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
> > 1 files changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 1f22d65..37d3ee5 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -343,13 +343,24 @@ xfs_iformat(
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> >
> > + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
> > + !ip->i_mount->m_rtdev_targp)) {
> > + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
> > + "corrupt dinode %Lu, flags = 0x%x.",
> > + (unsigned long long)ip->i_ino,
> > + ip->i_d.di_flags);
> > + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> > + ip->i_mount, dip);
>
> I think I'd rather not change all the corruption text tag ordering;
> it'll make it harder to track down any common occurrences of
> "xfs_iformat(X)" corruption in the future if they get renumbered now.
>
> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
> had. "realtime" is a lot more informative than "3" anyway.
I don't think this is a bad decision, because the corruption errors can
be easily identified by the output of xfs_fs_repair_cmn_err and the
source line. I think this is a reasonable change that will keep the code
clean and consistent.
-Ramon
>
> -Eric
>
> > + return XFS_ERROR(EFSCORRUPTED);
> > + }
> > +
> > switch (ip->i_d.di_mode & S_IFMT) {
> > case S_IFIFO:
> > case S_IFCHR:
> > case S_IFBLK:
> > case S_IFSOCK:
> > if (unlikely(dip->di_format != XFS_DINODE_FMT_DEV)) {
> > - XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> > + XFS_CORRUPTION_ERROR("xfs_iformat(4)", XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> > @@ -371,7 +382,7 @@ xfs_iformat(
> > "corrupt inode %Lu "
> > "(local format for regular file).",
> > (unsigned long long) ip->i_ino);
> > - XFS_CORRUPTION_ERROR("xfs_iformat(4)",
> > + XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> > XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
> > @@ -384,7 +395,7 @@ xfs_iformat(
> > "(bad size %Ld for local inode).",
> > (unsigned long long) ip->i_ino,
> > (long long) di_size);
> > - XFS_CORRUPTION_ERROR("xfs_iformat(5)",
> > + XFS_CORRUPTION_ERROR("xfs_iformat(6)",
> > XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
> > @@ -400,14 +411,14 @@ xfs_iformat(
> > error = xfs_iformat_btree(ip, dip, XFS_DATA_FORK);
> > break;
> > default:
> > - XFS_ERROR_REPORT("xfs_iformat(6)", XFS_ERRLEVEL_LOW,
> > + XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW,
> > ip->i_mount);
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> > break;
> >
> > default:
> > - XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW, ip->i_mount);
> > + XFS_ERROR_REPORT("xfs_iformat(8)", XFS_ERRLEVEL_LOW, ip->i_mount);
> > return XFS_ERROR(EFSCORRUPTED);
> > }
> > if (error) {
> > @@ -430,7 +441,7 @@ xfs_iformat(
> > "(bad attr fork size %Ld).",
> > (unsigned long long) ip->i_ino,
> > (long long) size);
> > - XFS_CORRUPTION_ERROR("xfs_iformat(8)",
> > + XFS_CORRUPTION_ERROR("xfs_iformat(9)",
> > XFS_ERRLEVEL_LOW,
> > ip->i_mount, dip);
> > return XFS_ERROR(EFSCORRUPTED);
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-05 3:55 ` Ramon de Carvalho Valle
@ 2009-08-05 4:15 ` Eric Sandeen
-1 siblings, 0 replies; 28+ messages in thread
From: Eric Sandeen @ 2009-08-05 4:15 UTC (permalink / raw)
To: Ramon de Carvalho Valle
Cc: Christoph Hellwig, linux-kernel, mszeredi, hch, xfs
Ramon de Carvalho Valle wrote:
> On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
>> Ramon de Carvalho Valle wrote:
>>> The xfs_iformat function does not check if the realtime device target pointer
>>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
>>> structure.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
>>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
>>> Cc: stable <stable@kernel.org>
>>> ---
>>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
>>> 1 files changed, 17 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>>> index 1f22d65..37d3ee5 100644
>>> --- a/fs/xfs/xfs_inode.c
>>> +++ b/fs/xfs/xfs_inode.c
>>> @@ -343,13 +343,24 @@ xfs_iformat(
>>> return XFS_ERROR(EFSCORRUPTED);
>>> }
>>>
>>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
>>> + !ip->i_mount->m_rtdev_targp)) {
>>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
>>> + "corrupt dinode %Lu, flags = 0x%x.",
>>> + (unsigned long long)ip->i_ino,
>>> + ip->i_d.di_flags);
>>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
>>> + ip->i_mount, dip);
>> I think I'd rather not change all the corruption text tag ordering;
>> it'll make it harder to track down any common occurrences of
>> "xfs_iformat(X)" corruption in the future if they get renumbered now.
>>
>> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
>> had. "realtime" is a lot more informative than "3" anyway.
>
> I don't think this is a bad decision, because the corruption errors can
> be easily identified by the output of xfs_fs_repair_cmn_err and the
> source line. I think this is a reasonable change that will keep the code
> clean and consistent.
Until you wind up looking at a problem from some old kernel, or modified
vendor kernel, and you realize that now you really don't know which
error "xfs_iformat(6)" is anymore, and you either have to go digging
through trees that aren't handy, or you just give up and don't bother to
help because now it's too much of a pain. ;)
But I can leave it up to the folks @ sgi, I can see both sides of the
argument, and I won't care too much either way.
Thanks,
-Eric
> -Ramon
>
>> -Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-05 4:15 ` Eric Sandeen
0 siblings, 0 replies; 28+ messages in thread
From: Eric Sandeen @ 2009-08-05 4:15 UTC (permalink / raw)
To: Ramon de Carvalho Valle
Cc: Christoph Hellwig, xfs, hch, linux-kernel, mszeredi
Ramon de Carvalho Valle wrote:
> On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
>> Ramon de Carvalho Valle wrote:
>>> The xfs_iformat function does not check if the realtime device target pointer
>>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
>>> structure.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
>>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
>>> Cc: stable <stable@kernel.org>
>>> ---
>>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
>>> 1 files changed, 17 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>>> index 1f22d65..37d3ee5 100644
>>> --- a/fs/xfs/xfs_inode.c
>>> +++ b/fs/xfs/xfs_inode.c
>>> @@ -343,13 +343,24 @@ xfs_iformat(
>>> return XFS_ERROR(EFSCORRUPTED);
>>> }
>>>
>>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
>>> + !ip->i_mount->m_rtdev_targp)) {
>>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
>>> + "corrupt dinode %Lu, flags = 0x%x.",
>>> + (unsigned long long)ip->i_ino,
>>> + ip->i_d.di_flags);
>>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
>>> + ip->i_mount, dip);
>> I think I'd rather not change all the corruption text tag ordering;
>> it'll make it harder to track down any common occurrences of
>> "xfs_iformat(X)" corruption in the future if they get renumbered now.
>>
>> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
>> had. "realtime" is a lot more informative than "3" anyway.
>
> I don't think this is a bad decision, because the corruption errors can
> be easily identified by the output of xfs_fs_repair_cmn_err and the
> source line. I think this is a reasonable change that will keep the code
> clean and consistent.
Until you wind up looking at a problem from some old kernel, or modified
vendor kernel, and you realize that now you really don't know which
error "xfs_iformat(6)" is anymore, and you either have to go digging
through trees that aren't handy, or you just give up and don't bother to
help because now it's too much of a pain. ;)
But I can leave it up to the folks @ sgi, I can see both sides of the
argument, and I won't care too much either way.
Thanks,
-Eric
> -Ramon
>
>> -Eric
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-05 4:15 ` Eric Sandeen
@ 2009-08-05 13:21 ` Ramon de Carvalho Valle
-1 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-05 13:21 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, linux-kernel, mszeredi, hch, xfs
[-- Attachment #1.1: Type: text/plain, Size: 2583 bytes --]
On Tue, 2009-08-04 at 23:15 -0500, Eric Sandeen wrote:
> Ramon de Carvalho Valle wrote:
> > On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
> >> Ramon de Carvalho Valle wrote:
> >>> The xfs_iformat function does not check if the realtime device target pointer
> >>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> >>> structure.
> >>>
> >>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
> >>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
> >>> Cc: stable <stable@kernel.org>
> >>> ---
> >>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
> >>> 1 files changed, 17 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> >>> index 1f22d65..37d3ee5 100644
> >>> --- a/fs/xfs/xfs_inode.c
> >>> +++ b/fs/xfs/xfs_inode.c
> >>> @@ -343,13 +343,24 @@ xfs_iformat(
> >>> return XFS_ERROR(EFSCORRUPTED);
> >>> }
> >>>
> >>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
> >>> + !ip->i_mount->m_rtdev_targp)) {
> >>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
> >>> + "corrupt dinode %Lu, flags = 0x%x.",
> >>> + (unsigned long long)ip->i_ino,
> >>> + ip->i_d.di_flags);
> >>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> >>> + ip->i_mount, dip);
> >> I think I'd rather not change all the corruption text tag ordering;
> >> it'll make it harder to track down any common occurrences of
> >> "xfs_iformat(X)" corruption in the future if they get renumbered now.
> >>
> >> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
> >> had. "realtime" is a lot more informative than "3" anyway.
> >
> > I don't think this is a bad decision, because the corruption errors can
> > be easily identified by the output of xfs_fs_repair_cmn_err and the
> > source line. I think this is a reasonable change that will keep the code
> > clean and consistent.
>
> Until you wind up looking at a problem from some old kernel, or modified
> vendor kernel, and you realize that now you really don't know which
> error "xfs_iformat(6)" is anymore, and you either have to go digging
> through trees that aren't handy, or you just give up and don't bother to
> help because now it's too much of a pain. ;)
>
> But I can leave it up to the folks @ sgi, I can see both sides of the
> argument, and I won't care too much either way.
Yes, whatever they decide should be ok. Thanks for your feedback Eric.
-Ramon
>
> Thanks,
> -Eric
>
> > -Ramon
> >
> >> -Eric
>
>
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 121 bytes --]
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-05 13:21 ` Ramon de Carvalho Valle
0 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-05 13:21 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, xfs, hch, linux-kernel, mszeredi
[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]
On Tue, 2009-08-04 at 23:15 -0500, Eric Sandeen wrote:
> Ramon de Carvalho Valle wrote:
> > On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
> >> Ramon de Carvalho Valle wrote:
> >>> The xfs_iformat function does not check if the realtime device target pointer
> >>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> >>> structure.
> >>>
> >>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
> >>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
> >>> Cc: stable <stable@kernel.org>
> >>> ---
> >>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
> >>> 1 files changed, 17 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> >>> index 1f22d65..37d3ee5 100644
> >>> --- a/fs/xfs/xfs_inode.c
> >>> +++ b/fs/xfs/xfs_inode.c
> >>> @@ -343,13 +343,24 @@ xfs_iformat(
> >>> return XFS_ERROR(EFSCORRUPTED);
> >>> }
> >>>
> >>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
> >>> + !ip->i_mount->m_rtdev_targp)) {
> >>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
> >>> + "corrupt dinode %Lu, flags = 0x%x.",
> >>> + (unsigned long long)ip->i_ino,
> >>> + ip->i_d.di_flags);
> >>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
> >>> + ip->i_mount, dip);
> >> I think I'd rather not change all the corruption text tag ordering;
> >> it'll make it harder to track down any common occurrences of
> >> "xfs_iformat(X)" corruption in the future if they get renumbered now.
> >>
> >> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
> >> had. "realtime" is a lot more informative than "3" anyway.
> >
> > I don't think this is a bad decision, because the corruption errors can
> > be easily identified by the output of xfs_fs_repair_cmn_err and the
> > source line. I think this is a reasonable change that will keep the code
> > clean and consistent.
>
> Until you wind up looking at a problem from some old kernel, or modified
> vendor kernel, and you realize that now you really don't know which
> error "xfs_iformat(6)" is anymore, and you either have to go digging
> through trees that aren't handy, or you just give up and don't bother to
> help because now it's too much of a pain. ;)
>
> But I can leave it up to the folks @ sgi, I can see both sides of the
> argument, and I won't care too much either way.
Yes, whatever they decide should be ok. Thanks for your feedback Eric.
-Ramon
>
> Thanks,
> -Eric
>
> > -Ramon
> >
> >> -Eric
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-05 4:15 ` Eric Sandeen
@ 2009-08-05 21:53 ` Felix Blyakher
-1 siblings, 0 replies; 28+ messages in thread
From: Felix Blyakher @ 2009-08-05 21:53 UTC (permalink / raw)
To: Eric Sandeen
Cc: Linux Kernel Mailing List, mszeredi, Christoph Hellwig,
xfs mailing list, Christoph Hellwig, Ramon de Carvalho Valle
On Aug 4, 2009, at 11:15 PM, Eric Sandeen wrote:
> Ramon de Carvalho Valle wrote:
>> On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
>>> Ramon de Carvalho Valle wrote:
>>>> The xfs_iformat function does not check if the realtime device
>>>> target pointer
>>>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk
>>>> inode
>>>> structure.
>>>>
>>>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
>>>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
>>>> Cc: stable <stable@kernel.org>
>>>> ---
>>>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
>>>> 1 files changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>>>> index 1f22d65..37d3ee5 100644
>>>> --- a/fs/xfs/xfs_inode.c
>>>> +++ b/fs/xfs/xfs_inode.c
>>>> @@ -343,13 +343,24 @@ xfs_iformat(
>>>> return XFS_ERROR(EFSCORRUPTED);
>>>> }
>>>>
>>>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
>>>> + !ip->i_mount->m_rtdev_targp)) {
>>>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
>>>> + "corrupt dinode %Lu, flags = 0x%x.",
>>>> + (unsigned long long)ip->i_ino,
>>>> + ip->i_d.di_flags);
>>>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
>>>> + ip->i_mount, dip);
>>> I think I'd rather not change all the corruption text tag ordering;
>>> it'll make it harder to track down any common occurrences of
>>> "xfs_iformat(X)" corruption in the future if they get renumbered
>>> now.
>>>
>>> I'd either make this xfs_iformat(2.1) ;) or just leave it as
>>> Christoph
>>> had. "realtime" is a lot more informative than "3" anyway.
>>
>> I don't think this is a bad decision, because the corruption errors
>> can
>> be easily identified by the output of xfs_fs_repair_cmn_err and the
>> source line. I think this is a reasonable change that will keep the
>> code
>> clean and consistent.
>
> Until you wind up looking at a problem from some old kernel, or
> modified
> vendor kernel, and you realize that now you really don't know which
> error "xfs_iformat(6)" is anymore, and you either have to go digging
> through trees that aren't handy, or you just give up and don't
> bother to
> help because now it's too much of a pain. ;)
>
> But I can leave it up to the folks @ sgi, I can see both sides of the
> argument, and I won't care too much either way.
Agree with Eric, see the benefits of both approaches, but I think,
it'll be cleaner without shifting the numbering of all messages.
Otherwise, looks good.
Felix
>
>
> Thanks,
> -Eric
>
>> -Ramon
>>
>>> -Eric
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-05 21:53 ` Felix Blyakher
0 siblings, 0 replies; 28+ messages in thread
From: Felix Blyakher @ 2009-08-05 21:53 UTC (permalink / raw)
To: Eric Sandeen
Cc: Ramon de Carvalho Valle, Christoph Hellwig, xfs mailing list,
Christoph Hellwig, Linux Kernel Mailing List, mszeredi
On Aug 4, 2009, at 11:15 PM, Eric Sandeen wrote:
> Ramon de Carvalho Valle wrote:
>> On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
>>> Ramon de Carvalho Valle wrote:
>>>> The xfs_iformat function does not check if the realtime device
>>>> target pointer
>>>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk
>>>> inode
>>>> structure.
>>>>
>>>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
>>>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
>>>> Cc: stable <stable@kernel.org>
>>>> ---
>>>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
>>>> 1 files changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>>>> index 1f22d65..37d3ee5 100644
>>>> --- a/fs/xfs/xfs_inode.c
>>>> +++ b/fs/xfs/xfs_inode.c
>>>> @@ -343,13 +343,24 @@ xfs_iformat(
>>>> return XFS_ERROR(EFSCORRUPTED);
>>>> }
>>>>
>>>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
>>>> + !ip->i_mount->m_rtdev_targp)) {
>>>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
>>>> + "corrupt dinode %Lu, flags = 0x%x.",
>>>> + (unsigned long long)ip->i_ino,
>>>> + ip->i_d.di_flags);
>>>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
>>>> + ip->i_mount, dip);
>>> I think I'd rather not change all the corruption text tag ordering;
>>> it'll make it harder to track down any common occurrences of
>>> "xfs_iformat(X)" corruption in the future if they get renumbered
>>> now.
>>>
>>> I'd either make this xfs_iformat(2.1) ;) or just leave it as
>>> Christoph
>>> had. "realtime" is a lot more informative than "3" anyway.
>>
>> I don't think this is a bad decision, because the corruption errors
>> can
>> be easily identified by the output of xfs_fs_repair_cmn_err and the
>> source line. I think this is a reasonable change that will keep the
>> code
>> clean and consistent.
>
> Until you wind up looking at a problem from some old kernel, or
> modified
> vendor kernel, and you realize that now you really don't know which
> error "xfs_iformat(6)" is anymore, and you either have to go digging
> through trees that aren't handy, or you just give up and don't
> bother to
> help because now it's too much of a pain. ;)
>
> But I can leave it up to the folks @ sgi, I can see both sides of the
> argument, and I won't care too much either way.
Agree with Eric, see the benefits of both approaches, but I think,
it'll be cleaner without shifting the numbering of all messages.
Otherwise, looks good.
Felix
>
>
> Thanks,
> -Eric
>
>> -Ramon
>>
>>> -Eric
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-04 18:51 ` Ramon de Carvalho Valle
@ 2009-08-05 15:17 ` Christoph Hellwig
-1 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2009-08-05 15:17 UTC (permalink / raw)
To: Ramon de Carvalho Valle
Cc: Eric Sandeen, linux-kernel, xfs, Christoph Hellwig, mszeredi, hch
On Tue, Aug 04, 2009 at 03:51:38PM -0300, Ramon de Carvalho Valle wrote:
> The xfs_iformat function does not check if the realtime device target pointer
> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> structure.
Same as Eric I don't think there's much of a point renumbering the error
cases. Instead I'll do another patch with a couple of cleanups in this
function replacing all the numbers with short alphabetic tags.
I don't really see the point of printing the flags either, if we have
this bit flipped it's pretty clear that we had random corruption of this
dinode.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-05 15:17 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2009-08-05 15:17 UTC (permalink / raw)
To: Ramon de Carvalho Valle
Cc: Eric Sandeen, Christoph Hellwig, linux-kernel, mszeredi, hch, xfs
On Tue, Aug 04, 2009 at 03:51:38PM -0300, Ramon de Carvalho Valle wrote:
> The xfs_iformat function does not check if the realtime device target pointer
> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> structure.
Same as Eric I don't think there's much of a point renumbering the error
cases. Instead I'll do another patch with a couple of cleanups in this
function replacing all the numbers with short alphabetic tags.
I don't really see the point of printing the flags either, if we have
this bit flipped it's pretty clear that we had random corruption of this
dinode.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
2009-08-05 15:17 ` Christoph Hellwig
@ 2009-08-05 16:34 ` Ramon de Carvalho Valle
-1 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-05 16:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs, hch, Eric Sandeen, linux-kernel, mszeredi
[-- Attachment #1.1: Type: text/plain, Size: 864 bytes --]
On Wed, 2009-08-05 at 11:17 -0400, Christoph Hellwig wrote:
> On Tue, Aug 04, 2009 at 03:51:38PM -0300, Ramon de Carvalho Valle wrote:
> > The xfs_iformat function does not check if the realtime device target pointer
> > is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> > structure.
>
> Same as Eric I don't think there's much of a point renumbering the error
> cases. Instead I'll do another patch with a couple of cleanups in this
> function replacing all the numbers with short alphabetic tags.
Great. Thanks.
>
> I don't really see the point of printing the flags either, if we have
> this bit flipped it's pretty clear that we had random corruption of this
> dinode.
>
Printing the flags is just for debugging purposes and it keeps the code
consistent with the other calls to xfs_fs_repair_cmn_err.
-Ramon
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 121 bytes --]
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
@ 2009-08-05 16:34 ` Ramon de Carvalho Valle
0 siblings, 0 replies; 28+ messages in thread
From: Ramon de Carvalho Valle @ 2009-08-05 16:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Eric Sandeen, linux-kernel, mszeredi, hch, xfs
[-- Attachment #1: Type: text/plain, Size: 864 bytes --]
On Wed, 2009-08-05 at 11:17 -0400, Christoph Hellwig wrote:
> On Tue, Aug 04, 2009 at 03:51:38PM -0300, Ramon de Carvalho Valle wrote:
> > The xfs_iformat function does not check if the realtime device target pointer
> > is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
> > structure.
>
> Same as Eric I don't think there's much of a point renumbering the error
> cases. Instead I'll do another patch with a couple of cleanups in this
> function replacing all the numbers with short alphabetic tags.
Great. Thanks.
>
> I don't really see the point of printing the flags either, if we have
> this bit flipped it's pretty clear that we had random corruption of this
> dinode.
>
Printing the flags is just for debugging purposes and it keeps the code
consistent with the other calls to xfs_fs_repair_cmn_err.
-Ramon
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread