| Message ID | 1377667560-20089-2-git-send-email-liezhi.yang@windriver.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
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
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,
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
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);
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(-)