All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Yang <liezhi.yang@windriver.com>
To: Darren Hart <dvhart@linux.intel.com>, "Theodore Ts'o" <tytso@mit.edu>
Cc: <linux-ext4@vger.kernel.org>
Subject: Re: [RFC 01/10] mke2fs.c: add an option: -d root-directory
Date: Tue, 15 Oct 2013 09:38:11 +0800	[thread overview]
Message-ID: <525C9C83.70904@windriver.com> (raw)
In-Reply-To: <1381767986.9489.74.camel@dvhart-mobl4.amr.corp.intel.com>



On 10/15/2013 12:26 AM, Darren Hart wrote:
> On Sun, 2013-10-13 at 22:41 -0400, Theodore Ts'o wrote:
>> On Wed, Aug 28, 2013 at 01:25:51PM +0800, Robert Yang wrote:
>>> @@ -2773,7 +2776,6 @@ no_journal:
>>>   		       "filesystem accounting information: "));
>>>   	checkinterval = fs->super->s_checkinterval;
>>>   	max_mnt_count = fs->super->s_max_mnt_count;
>>> -	retval = ext2fs_close(fs);
>>>   	if (retval) {
>>>   		fprintf(stderr,
>>>   			_("\nWarning, had trouble writing out superblocks."));
>>
>> You can't just move the call to ext2fs_close().  You also need to move
>>
>> 	if (!quiet)
>> 		printf(_("Writing superblocks and "
>> 		       "filesystem accounting information: "));
>>
>> before the call to ext2fs_close() since this is used to print the
>> message for the progress information that will be emitted by
>> ext2fs_close(), and you also have to move the error checking:
>>
>>    	if (retval) {
>>    		fprintf(stderr,
>>    			_("\nWarning, had trouble writing out superblocks."));
>> 	...
>>
>> after the call to ext2fs_close().
>>
>>
>> This would have been ***painfully*** obvious if you had run the
>> regression test suite.  ("make -j8 ; make -j8 check"), since the
>> inconsistent move of ext2fs_close() without the preceeding printf
>> would cause all of the mke2fs tests (the m_* tests) to fail.
>>
>> This is why regression test suites are so important.  :-)
>>
>>       	    	       	    	       - Ted
>
> Robert,
>
> Can you please take Ted's feedback into account (this and his response
> to patch 10/10) and prepare another version of the patches.
>

Yes, of course:-)

> As Ted suggests here, please run the regression test suite prior to any
> future patch submissions. Looks like we missed some critical bits by not
> doing that.
>
> Ted, thank you for the response and taking the time to point out the
> test suite. Robert, could you check the README and see if anything needs
> to get updated there to make sure other developers are aware of the
> regressions suite and how to run it?
>

OK, I will, thanks Ted and Darren.

// Robert

> Thanks,
>
>

  reply	other threads:[~2013-10-15  1:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-28  5:25 [RFC 00/10] e2fsprogs/mke2fs: add an option: -d root-directory Robert Yang
2013-08-28  5:25 ` [RFC 01/10] mke2fs.c: " Robert Yang
2013-10-14  2:41   ` Theodore Ts'o
2013-10-14 12:22     ` [PATCH] mke2fs: fix up the commit "mke2fs.c: add an option: -d root-directory" Theodore Ts'o
2013-10-14 16:26     ` [RFC 01/10] mke2fs.c: add an option: -d root-directory Darren Hart
2013-10-15  1:38       ` Robert Yang [this message]
2013-08-28  5:25 ` [RFC 02/10] misc/util.c: implement populate_fs() Robert Yang
2013-08-28  5:25 ` [RFC 03/10] misc/util.c: create special file Robert Yang
2013-08-28  5:25 ` [RFC 04/10] misc/util.c: create symlink Robert Yang
2013-08-28  5:25 ` [RFC 05/10] misc/util.c: copy regular file Robert Yang
2013-08-28  5:25 ` [RFC 06/10] misc/util.c: create directory Robert Yang
2013-08-28  5:25 ` [RFC 07/10] misc/util.c: set more information for inode Robert Yang
2013-08-28  5:25 ` [RFC 08/10] misc/util.c: handle hardlinks Robert Yang
2013-08-28  5:25 ` [RFC 09/10] mke2fs.8.in: update the manual for the -d option Robert Yang
2013-08-28  5:26 ` [RFC 10/10] debugfs: use the functions in misc/util.c Robert Yang
2013-10-14 14:40   ` Theodore Ts'o
2013-09-01  3:26 ` [RFC 00/10] e2fsprogs/mke2fs: add an option: -d root-directory Zheng Liu
2013-09-02  6:46   ` Robert Yang
2013-09-02 11:55     ` Zheng Liu
2013-09-02 12:20       ` Robert Yang
2013-09-02 12:27         ` Zheng Liu
2013-09-16 21:04           ` Darren Hart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=525C9C83.70904@windriver.com \
    --to=liezhi.yang@windriver.com \
    --cc=dvhart@linux.intel.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.