Re: [PATCH] ext4: add a line break for proc mb_groups display

From: Andreas Dilger <adilger@dilger.ca>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Joe Perches <joe@perches.com>,
	Huaitong Han <huaitong.han@intel.com>,
	tytso@mit.edu, adilger.kernel@dilger.ca,
	linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ext4: add a line break for proc mb_groups display
Date: Wed, 3 Feb 2016 14:12:15 -0700	[thread overview]
Message-ID: <41C709B0-476E-4DE8-B973-1F7237102708@dilger.ca> (raw)
In-Reply-To: <20160203200706.GG22352@birch.djwong.org>

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

On Feb 3, 2016, at 1:07 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> On Wed, Feb 03, 2016 at 11:38:33AM -0800, Joe Perches wrote:
>> On Wed, 2016-02-03 at 11:32 -0800, Darrick J. Wong wrote:
>>> On Wed, Feb 03, 2016 at 10:30:32AM -0800, Joe Perches wrote:
>>>> On Wed, 2016-02-03 at 10:13 -0800, Darrick J. Wong wrote:
>>>>> On Wed, Feb 03, 2016 at 09:19:06PM +0800, Huaitong Han wrote:
>>>>>> This patch adds a line break for proc mb_groups display.
>>>> 
>>>> Using 2 lines for output might break any existing users.
>>>> 
>>>> Are there any?
>>> 
>>> It's a multiline file if you have more than one blockgroup; this just makes it
>>> so that you don't have to special-case BG 0.
>> 
>> And existing scripts might do that now and might fail
>> to do properly after this change.
> 
> Or they might have sed -e 's/]#0/]\n#0/g' in which case they won't be affected.

I suspect that any scripts which handled this in the past probably didn't
even notice and just missed the bg=0 line at the end of the header.  Users
looking at the output probably saw it line-wrapped by the terminal and
didn't notice either.

>>> IOW: mb_groups scripts already had to parse multiple lines, and most likely any
>>> script parsing it would inject a newline after the header.
>> 
>> I've no dog in this fight really.  I just wanted to make
>> it clear that this could cause existing scripts to fail.
>> 
>> proc output is supposed to be unchanging except maybe
>> adding new fields to existing lines.
>> 
>> Your choice.
> 
> Ted's, really.  I have no idea which scripts do with various per-fs /proc
> files.  Usually poking in mb_groups is only done as part of failure report data
> collection to see what's mucked up the fs this time.
> 
> Anyway, I'll defer to the maintainer. :)

I think it makes sense to accept the patch, since I doubt any scripts will be broken, and it is the "right thing to do" rather than perpetuate a bug.

Cheers, Andreas

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]