Both parser of nfsiostat and mountstats depend on the
the first word in /proc/self/mountstats to be 'device'
for them to parse through all the devices.
NFS mounts entries, in 3.1, no longer have start with
the word 'device'. They start with the actual device,
which confused both set of parsing routines.
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
tools/mountstats/mountstats.py | 9 +++++++++
tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)
--
1.7.6.2
On 09/21/2011 02:51 PM, J. Bruce Fields wrote:
> On Wed, Sep 21, 2011 at 02:31:12PM -0400, Steve Dickson wrote:
>>
>>
>> On 09/21/2011 02:25 PM, Bryan Schumaker wrote:
>>> On 09/21/2011 02:16 PM, Steve Dickson wrote:
>>>> Both parser of nfsiostat and mountstats depend on the
>>>> the first word in /proc/self/mountstats to be 'device'
>>>> for them to parse through all the devices.
>>>>
>>>> NFS mounts entries, in 3.1, no longer have start with
>>>> the word 'device'. They start with the actual device,
>>>> which confused both set of parsing routines.
>>>
>>> I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
>> Interesting... Maybe we should still apply these patches until
>> your patch shows up? They are definitely broken...
>
> May as well fix the problem on both sides if possible.
I agree this.. I can always pull these back out if/when
Bryan's patch shows up...
steved.
>
> --b.
>
>>
>> steved.
>> .
>>>
>>> - Bryan
>>>
>>> [PATCH] VFS: Add "device" tag to /proc/self/mountstats
>>>
>>> nfsiostat was failing to find mounted filesystems on kernels after
>>> 2.6.38 because of changes to show_vfsstat() by commit
>>> c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
>>> "device" tag before the nfs server entry so scripts can parse the
>>> mountstats file correctly.
>>>
>>> Signed-off-by: Bryan Schumaker <[email protected]>
>>> ---
>>> fs/namespace.c | 1 +
>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/namespace.c b/fs/namespace.c
>>> index 22bfe82..1429114 100644
>>> --- a/fs/namespace.c
>>> +++ b/fs/namespace.c
>>> @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
>>>
>>> /* device */
>>> if (mnt->mnt_sb->s_op->show_devname) {
>>> + seq_puts(m, "device ");
>>> err = mnt->mnt_sb->s_op->show_devname(m, mnt);
>>> } else {
>>> if (mnt->mnt_devname) {
>>>
>>>
>>>>
>>>> Steve Dickson (2):
>>>> nfsiostat: Breaks on 3.1 kernels
>>>> mountstats: Breaks on 3.1 kernels
>>>>
>>>> tools/mountstats/mountstats.py | 9 +++++++++
>>>> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
>>>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
mountstats depend on all devices entries in /proc/self/mountstats
to start with the word 'device'. With 3.1 kernels, NFS entries
start with the actual device (i.e. server:/export) not
the word 'device'. This change confused mountstats parsing.
Signed-off-by: Steve Dickson <[email protected]>
---
tools/mountstats/mountstats.py | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 280e15c..b95b71d 100644
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -46,6 +46,12 @@ class DeviceData:
self.__nfs_data['fstype'] = words[7]
if words[7].find('nfs') != -1:
self.__nfs_data['statvers'] = words[8]
+ elif 'nfs' in words or 'nfs4' in words:
+ self.__nfs_data['export'] = words[0]
+ self.__nfs_data['mountpoint'] = words[3]
+ self.__nfs_data['fstype'] = words[6]
+ if words[6].find('nfs') != -1:
+ self.__nfs_data['statvers'] = words[7]
elif words[0] == 'age:':
self.__nfs_data['age'] = long(words[1])
elif words[0] == 'opts:':
@@ -371,6 +377,9 @@ def parse_stats_file(filename):
if words[0] == 'device':
key = words[4]
new = [ line.strip() ]
+ elif 'nfs' in words or 'nfs4' in words:
+ key = words[3]
+ new = [ line.strip() ]
else:
new += [ line.strip() ]
ms_dict[key] = new
--
1.7.6.2
nfsiostat depend on all devices entries in /proc/self/mountstats
to start with the word 'device'. With 3.1 kernels, NFS entries
start with the actual device (i.e. server:/export) not
the word 'device'. This change confused nfsiostat parsing.
Signed-off-by: Steve Dickson <[email protected]>
---
tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
index 4e55dad..d909632 100644
--- a/tools/nfs-iostat/nfs-iostat.py
+++ b/tools/nfs-iostat/nfs-iostat.py
@@ -86,6 +86,12 @@ class DeviceData:
self.__nfs_data['fstype'] = words[7]
if words[7] == 'nfs':
self.__nfs_data['statvers'] = words[8]
+ elif 'nfs' in words or 'nfs4' in words:
+ self.__nfs_data['export'] = words[0]
+ self.__nfs_data['mountpoint'] = words[3]
+ self.__nfs_data['fstype'] = words[6]
+ if words[6] == 'nfs':
+ self.__nfs_data['statvers'] = words[7]
elif words[0] == 'age:':
self.__nfs_data['age'] = long(words[1])
elif words[0] == 'opts:':
@@ -426,6 +432,9 @@ def parse_stats_file(filename):
if words[0] == 'device':
key = words[4]
new = [ line.strip() ]
+ elif 'nfs' in words or 'nfs4' in words:
+ key = words[3]
+ new = [ line.strip() ]
else:
new += [ line.strip() ]
ms_dict[key] = new
@@ -436,7 +445,6 @@ def parse_stats_file(filename):
def print_iostat_summary(old, new, devices, time, options):
stats = {}
diff_stats = {}
-
if old:
# Trim device list to only include intersection of old and new data,
# this addresses umounts due to autofs mountpoints
@@ -553,7 +561,6 @@ client are listed.
parser.add_option_group(displaygroup)
(options, args) = parser.parse_args(sys.argv)
-
for arg in args:
if arg == sys.argv[0]:
--
1.7.6.2
On 09/21/2011 03:31 PM, Steve Dickson wrote:
>
>
> On 09/21/2011 02:51 PM, J. Bruce Fields wrote:
>> On Wed, Sep 21, 2011 at 02:31:12PM -0400, Steve Dickson wrote:
>>>
>>>
>>> On 09/21/2011 02:25 PM, Bryan Schumaker wrote:
>>>> On 09/21/2011 02:16 PM, Steve Dickson wrote:
>>>>> Both parser of nfsiostat and mountstats depend on the
>>>>> the first word in /proc/self/mountstats to be 'device'
>>>>> for them to parse through all the devices.
>>>>>
>>>>> NFS mounts entries, in 3.1, no longer have start with
>>>>> the word 'device'. They start with the actual device,
>>>>> which confused both set of parsing routines.
>>>>
>>>> I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
>>> Interesting... Maybe we should still apply these patches until
>>> your patch shows up? They are definitely broken...
>>
>> May as well fix the problem on both sides if possible.
> I agree this.. I can always pull these back out if/when
> Bryan's patch shows up...
Sounds good to me. It makes sense to have tools that work with current kernels.
>
> steved.
>
>>
>> --b.
>>
>>>
>>> steved.
>>> .
>>>>
>>>> - Bryan
>>>>
>>>> [PATCH] VFS: Add "device" tag to /proc/self/mountstats
>>>>
>>>> nfsiostat was failing to find mounted filesystems on kernels after
>>>> 2.6.38 because of changes to show_vfsstat() by commit
>>>> c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
>>>> "device" tag before the nfs server entry so scripts can parse the
>>>> mountstats file correctly.
>>>>
>>>> Signed-off-by: Bryan Schumaker <[email protected]>
>>>> ---
>>>> fs/namespace.c | 1 +
>>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/fs/namespace.c b/fs/namespace.c
>>>> index 22bfe82..1429114 100644
>>>> --- a/fs/namespace.c
>>>> +++ b/fs/namespace.c
>>>> @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
>>>>
>>>> /* device */
>>>> if (mnt->mnt_sb->s_op->show_devname) {
>>>> + seq_puts(m, "device ");
>>>> err = mnt->mnt_sb->s_op->show_devname(m, mnt);
>>>> } else {
>>>> if (mnt->mnt_devname) {
>>>>
>>>>
>>>>>
>>>>> Steve Dickson (2):
>>>>> nfsiostat: Breaks on 3.1 kernels
>>>>> mountstats: Breaks on 3.1 kernels
>>>>>
>>>>> tools/mountstats/mountstats.py | 9 +++++++++
>>>>> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
>>>>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/21/2011 02:16 PM, Steve Dickson wrote:
> Both parser of nfsiostat and mountstats depend on the
> the first word in /proc/self/mountstats to be 'device'
> for them to parse through all the devices.
>
> NFS mounts entries, in 3.1, no longer have start with
> the word 'device'. They start with the actual device,
> which confused both set of parsing routines.
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below.
- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after
2.6.38 because of changes to show_vfsstat() by commit
c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
"device" tag before the nfs server entry so scripts can parse the
mountstats file correctly.
Signed-off-by: Bryan Schumaker <[email protected]>
---
fs/namespace.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 22bfe82..1429114 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
/* device */
if (mnt->mnt_sb->s_op->show_devname) {
+ seq_puts(m, "device ");
err = mnt->mnt_sb->s_op->show_devname(m, mnt);
} else {
if (mnt->mnt_devname) {
>
> Steve Dickson (2):
> nfsiostat: Breaks on 3.1 kernels
> mountstats: Breaks on 3.1 kernels
>
> tools/mountstats/mountstats.py | 9 +++++++++
> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
On 09/21/2011 02:25 PM, Bryan Schumaker wrote:
> On 09/21/2011 02:16 PM, Steve Dickson wrote:
>> Both parser of nfsiostat and mountstats depend on the
>> the first word in /proc/self/mountstats to be 'device'
>> for them to parse through all the devices.
>>
>> NFS mounts entries, in 3.1, no longer have start with
>> the word 'device'. They start with the actual device,
>> which confused both set of parsing routines.
>
> I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
Interesting... Maybe we should still apply these patches until
your patch shows up? They are definitely broken...
steved.
.
>
> - Bryan
>
> [PATCH] VFS: Add "device" tag to /proc/self/mountstats
>
> nfsiostat was failing to find mounted filesystems on kernels after
> 2.6.38 because of changes to show_vfsstat() by commit
> c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
> "device" tag before the nfs server entry so scripts can parse the
> mountstats file correctly.
>
> Signed-off-by: Bryan Schumaker <[email protected]>
> ---
> fs/namespace.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 22bfe82..1429114 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
>
> /* device */
> if (mnt->mnt_sb->s_op->show_devname) {
> + seq_puts(m, "device ");
> err = mnt->mnt_sb->s_op->show_devname(m, mnt);
> } else {
> if (mnt->mnt_devname) {
>
>
>>
>> Steve Dickson (2):
>> nfsiostat: Breaks on 3.1 kernels
>> mountstats: Breaks on 3.1 kernels
>>
>> tools/mountstats/mountstats.py | 9 +++++++++
>> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>
>
On Sep 21, 2011, at 2:31 PM, Steve Dickson wrote:
>
>
> On 09/21/2011 02:25 PM, Bryan Schumaker wrote:
>> On 09/21/2011 02:16 PM, Steve Dickson wrote:
>>> Both parser of nfsiostat and mountstats depend on the
>>> the first word in /proc/self/mountstats to be 'device'
>>> for them to parse through all the devices.
>>>
>>> NFS mounts entries, in 3.1, no longer have start with
>>> the word 'device'. They start with the actual device,
>>> which confused both set of parsing routines.
>>
>> I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
> Interesting... Maybe we should still apply these patches until
> your patch shows up? They are definitely broken...
I think we should wait until we get some judgement from the VFS guys. Either:
o This is deemed a regression in a formal kernel API, in which case it's a kernel bug and should be fixed there, or
o This is deemed to be a permanent change to the kernel API, in which case we should address the problem in the python tools, as Steve has done
I think all the broken kernels still have stable releases, so it's possible that a kernel fix could address the whole problem. Bryan, if you are asked to repost, can you add a Cc: [email protected] below your SOB?
> steved.
> .
>>
>> - Bryan
>>
>> [PATCH] VFS: Add "device" tag to /proc/self/mountstats
>>
>> nfsiostat was failing to find mounted filesystems on kernels after
>> 2.6.38 because of changes to show_vfsstat() by commit
>> c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
>> "device" tag before the nfs server entry so scripts can parse the
>> mountstats file correctly.
>>
>> Signed-off-by: Bryan Schumaker <[email protected]>
>> ---
>> fs/namespace.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index 22bfe82..1429114 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
>>
>> /* device */
>> if (mnt->mnt_sb->s_op->show_devname) {
>> + seq_puts(m, "device ");
>> err = mnt->mnt_sb->s_op->show_devname(m, mnt);
>> } else {
>> if (mnt->mnt_devname) {
>>
>>
>>>
>>> Steve Dickson (2):
>>> nfsiostat: Breaks on 3.1 kernels
>>> mountstats: Breaks on 3.1 kernels
>>>
>>> tools/mountstats/mountstats.py | 9 +++++++++
>>> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
>>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Wed, Sep 21, 2011 at 02:31:12PM -0400, Steve Dickson wrote:
>
>
> On 09/21/2011 02:25 PM, Bryan Schumaker wrote:
> > On 09/21/2011 02:16 PM, Steve Dickson wrote:
> >> Both parser of nfsiostat and mountstats depend on the
> >> the first word in /proc/self/mountstats to be 'device'
> >> for them to parse through all the devices.
> >>
> >> NFS mounts entries, in 3.1, no longer have start with
> >> the word 'device'. They start with the actual device,
> >> which confused both set of parsing routines.
> >
> > I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
> Interesting... Maybe we should still apply these patches until
> your patch shows up? They are definitely broken...
May as well fix the problem on both sides if possible.
--b.
>
> steved.
> .
> >
> > - Bryan
> >
> > [PATCH] VFS: Add "device" tag to /proc/self/mountstats
> >
> > nfsiostat was failing to find mounted filesystems on kernels after
> > 2.6.38 because of changes to show_vfsstat() by commit
> > c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
> > "device" tag before the nfs server entry so scripts can parse the
> > mountstats file correctly.
> >
> > Signed-off-by: Bryan Schumaker <[email protected]>
> > ---
> > fs/namespace.c | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 22bfe82..1429114 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
> >
> > /* device */
> > if (mnt->mnt_sb->s_op->show_devname) {
> > + seq_puts(m, "device ");
> > err = mnt->mnt_sb->s_op->show_devname(m, mnt);
> > } else {
> > if (mnt->mnt_devname) {
> >
> >
> >>
> >> Steve Dickson (2):
> >> nfsiostat: Breaks on 3.1 kernels
> >> mountstats: Breaks on 3.1 kernels
> >>
> >> tools/mountstats/mountstats.py | 9 +++++++++
> >> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
> >> 2 files changed, 18 insertions(+), 2 deletions(-)
> >>
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/21/2011 02:51 PM, Chuck Lever wrote:
>
> On Sep 21, 2011, at 2:31 PM, Steve Dickson wrote:
>
>>
>>
>> On 09/21/2011 02:25 PM, Bryan Schumaker wrote:
>>> On 09/21/2011 02:16 PM, Steve Dickson wrote:
>>>> Both parser of nfsiostat and mountstats depend on the
>>>> the first word in /proc/self/mountstats to be 'device'
>>>> for them to parse through all the devices.
>>>>
>>>> NFS mounts entries, in 3.1, no longer have start with
>>>> the word 'device'. They start with the actual device,
>>>> which confused both set of parsing routines.
>>>
>>> I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
>> Interesting... Maybe we should still apply these patches until
>> your patch shows up? They are definitely broken...
>
> I think we should wait until we get some judgement from the VFS guys. Either:
>
> o This is deemed a regression in a formal kernel API, in which case it's a kernel bug and should be fixed there, or
>
> o This is deemed to be a permanent change to the kernel API, in which case we should address the problem in the python tools, as Steve has done
>
> I think all the broken kernels still have stable releases, so it's possible that a kernel fix could address the whole problem. Bryan, if you are asked to repost, can you add a Cc: [email protected] below your SOB?
Yeah, no problem. Should I resend it with that line anyway?
- Bryan
>
>> steved.
>> .
>>>
>>> - Bryan
>>>
>>> [PATCH] VFS: Add "device" tag to /proc/self/mountstats
>>>
>>> nfsiostat was failing to find mounted filesystems on kernels after
>>> 2.6.38 because of changes to show_vfsstat() by commit
>>> c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the
>>> "device" tag before the nfs server entry so scripts can parse the
>>> mountstats file correctly.
>>>
>>> Signed-off-by: Bryan Schumaker <[email protected]>
>>> ---
>>> fs/namespace.c | 1 +
>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/namespace.c b/fs/namespace.c
>>> index 22bfe82..1429114 100644
>>> --- a/fs/namespace.c
>>> +++ b/fs/namespace.c
>>> @@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
>>>
>>> /* device */
>>> if (mnt->mnt_sb->s_op->show_devname) {
>>> + seq_puts(m, "device ");
>>> err = mnt->mnt_sb->s_op->show_devname(m, mnt);
>>> } else {
>>> if (mnt->mnt_devname) {
>>>
>>>
>>>>
>>>> Steve Dickson (2):
>>>> nfsiostat: Breaks on 3.1 kernels
>>>> mountstats: Breaks on 3.1 kernels
>>>>
>>>> tools/mountstats/mountstats.py | 9 +++++++++
>>>> tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
>>>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>