Patchwork [RFC,01/10] mke2fs.c: add an option: -d root-directory

login
register
mail settings
Submitter Robert Yang
Date Aug. 28, 2013, 5:25 a.m.
Message ID <1377667560-20089-2-git-send-email-liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/270349/
State Superseded
Headers show

Comments

Robert Yang - Aug. 28, 2013, 5:25 a.m.
This option is used for adding the files from the root-directory to the
filesystem, it is similiar to genext2fs, but genext2fs doesn't fully
support ext4.

This commit describes the skeleton of the implementation:
* The "struct hdlink_s" will be used for saving hard links, I
  referred this from the genext2fs.

* The do_xxx_internal will be used by both mke2fs and debugfs, most of
  their operations are similar.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 misc/mke2fs.c | 39 ++++++++++++++++++++++++++++++++++-----
 misc/util.c   | 35 +++++++++++++++++++++++++++++++++++
 misc/util.h   | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 5 deletions(-)
Theodore Ts'o - Oct. 14, 2013, 2:41 a.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darren Hart - Oct. 14, 2013, 4:26 p.m.
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.

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?

Thanks,
Robert Yang - Oct. 15, 2013, 1:38 a.m.
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,
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index d96f156..6401ae0 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -44,8 +44,6 @@  extern int optind;
 #include <errno.h>
 #endif
 #include <sys/ioctl.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <libgen.h>
 #include <limits.h>
 #include <blkid/blkid.h>
@@ -105,6 +103,7 @@  char *mount_dir;
 char *journal_device;
 int sync_kludge;	/* Set using the MKE2FS_SYNC env. option */
 char **fs_types;
+const char *root_dir;  /* Copy files from the specified directory */
 
 profile_t	profile;
 
@@ -116,7 +115,8 @@  static void usage(void)
 	fprintf(stderr, _("Usage: %s [-c|-l filename] [-b block-size] "
 	"[-C cluster-size]\n\t[-i bytes-per-inode] [-I inode-size] "
 	"[-J journal-options]\n"
-	"\t[-G flex-group-size] [-N number-of-inodes]\n"
+	"\t[-G flex-group-size] [-N number-of-inodes] "
+	"[-d root-directory]\n"
 	"\t[-m reserved-blocks-percentage] [-o creator-os]\n"
 	"\t[-g blocks-per-group] [-L volume-label] "
 	"[-M last-mounted-directory]\n\t[-O feature[,...]] "
@@ -1386,7 +1386,7 @@  profile_error:
 	}
 
 	while ((c = getopt (argc, argv,
-		    "b:cg:i:jl:m:no:qr:s:t:vC:DE:FG:I:J:KL:M:N:O:R:ST:U:V")) != EOF) {
+		    "b:cg:i:jl:m:no:qr:s:t:d:vC:DE:FG:I:J:KL:M:N:O:R:ST:U:V")) != EOF) {
 		switch (c) {
 		case 'b':
 			blocksize = parse_num_blocks2(optarg, -1);
@@ -1572,6 +1572,9 @@  profile_error:
 		case 'U':
 			fs_uuid = optarg;
 			break;
+		case 'd':
+			root_dir = optarg;
+			break;
 		case 'v':
 			verbose = 1;
 			break;
@@ -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."));
@@ -2789,5 +2791,32 @@  no_journal:
 	for (i=0; fs_types[i]; i++)
 		free(fs_types[i]);
 	free(fs_types);
+
+	/* Copy files from the specified directory */
+	if (root_dir) {
+		if (!quiet)
+			printf(_("Copying files into the device...\n"));
+
+		/*
+		 * Allocate memory for the hardlinks, we don't need free()
+		 * since the lifespan will be over after the fs populated.
+		 */
+		if ((hdlinks.hdl = (struct hdlink_s *)
+				malloc(hdlink_cnt * sizeof(struct hdlink_s))) == NULL) {
+			fprintf(stderr, _("\nNot enough memory"));
+			retval = ext2fs_close(fs);
+			return retval;
+		}
+
+		current_fs = fs;
+		root = EXT2_ROOT_INO;
+		retval = populate_fs(root, root_dir);
+		if (retval)
+			fprintf(stderr,
+				_("\nError while populating %s"), root_dir);
+	}
+
+	retval = ext2fs_close(fs);
+
 	return retval;
 }
diff --git a/misc/util.c b/misc/util.c
index 6c93e1c..cbc7cc0 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -32,8 +32,18 @@ 
 #include "ext2fs/ext2fs.h"
 #include "nls-enable.h"
 #include "blkid/blkid.h"
+
+#include <fcntl.h>
+
 #include "util.h"
 
+int	journal_size;
+int	journal_flags;
+char	*journal_device;
+
+/* For saving the hard links */
+int hdlink_cnt = HDLINK_CNT;
+
 #ifndef HAVE_STRCASECMP
 int strcasecmp (char *s1, char *s2)
 {
@@ -303,3 +313,28 @@  void dump_mmp_msg(struct mmp_struct *mmp, const char *msg)
 		       ctime(&t), mmp->mmp_nodename, mmp->mmp_bdevname);
 	}
 }
+
+/* Make a special file which is block, character and fifo */
+errcode_t do_mknod_internal(ext2_ino_t cwd, const char *name, struct stat *st)
+{
+}
+
+/* Make a symlink name -> target */
+errcode_t do_symlink_internal(ext2_ino_t cwd, const char *name, char *target)
+{
+}
+
+/* Make a directory in the fs */
+errcode_t do_mkdir_internal(ext2_ino_t cwd, const char *name, struct stat *st)
+{
+}
+
+/* Copy the native file to the fs */
+errcode_t do_write_internal(ext2_ino_t cwd, const char *src, const char *dest)
+{
+}
+
+/* Copy files from source_dir to fs */
+errcode_t populate_fs(ext2_ino_t parent_ino, const char *source_dir)
+{
+}
diff --git a/misc/util.h b/misc/util.h
index f872c38..e71caf0 100644
--- a/misc/util.h
+++ b/misc/util.h
@@ -14,6 +14,31 @@  extern int	 journal_size;
 extern int	 journal_flags;
 extern char	*journal_device;
 
+/* For struct stat */
+#include <sys/types.h>
+#include <sys/stat.h>
+
+struct hdlink_s
+{
+	ext2_ino_t src_ino;
+	ext2_ino_t dst_ino;
+};
+
+struct hdlinks_s
+{
+	int count;
+	struct hdlink_s *hdl;
+};
+
+struct hdlinks_s hdlinks;
+
+ext2_filsys	current_fs;
+ext2_ino_t	root;
+
+/* For saving the hard links */
+#define HDLINK_CNT	4
+extern int hdlink_cnt;
+
 #ifndef HAVE_STRCASECMP
 extern int strcasecmp (char *s1, char *s2);
 #endif
@@ -25,3 +50,10 @@  extern void check_mount(const char *device, int force, const char *type);
 extern unsigned int figure_journal_size(int size, ext2_filsys fs);
 extern void print_check_message(int, unsigned int);
 extern void dump_mmp_msg(struct mmp_struct *mmp, const char *msg);
+
+/* For populating the filesystem */
+extern errcode_t populate_fs(ext2_ino_t parent_ino, const char *source_dir);
+extern errcode_t do_mknod_internal(ext2_ino_t cwd, const char *name, struct stat *st);
+extern errcode_t do_symlink_internal(ext2_ino_t cwd, const char *name, char *target);
+extern errcode_t do_mkdir_internal(ext2_ino_t cwd, const char *name, struct stat *st);
+extern errcode_t do_write_internal(ext2_ino_t cwd, const char *src, const char *dest);