Date: Wed, 4 Jan 2012 18:42:17 -0500 From: Jung-uk Kim <jkim@FreeBSD.org> To: freebsd-hackers@FreeBSD.org Cc: Alexander Sack <alex@niksun.com> Subject: [PATCH] makefs from multiple directories Message-ID: <201201041842.18854.jkim@FreeBSD.org>
next in thread | raw e-mail | index | archive | help
--Boundary-00=_aPOBPLYw2Mozxms Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline My colleague at work wrote a nice patch for makefs(8) to support multiple directories as mkisofs does. Then, I cleaned it up and made it committable to head (also attached here): http://people.freebsd.org/~jkim/makefs.diff Actually, he submitted this PR first: http://www.freebsd.org/cgi/query-pr.cgi?pr=162338 After I reviewed it, I told him it isn't right because it messes up with source directories. Instead, I suggested him to add the new feature. ;-) Please review and let me know what you think. Cheers! Jung-uk Kim PS: Do we care about submitting it back to NetBSD? Actually, it seems to be the first thing in their TODO list for very long time: http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/TODO.diff?r1=1.6&r2=1.7&only_with_tag=MAIN However, our version looks quite different from their head version. :-/ Note: His original e-mail is attached here as a reference. ---------- Description: Create an ISO image of multiple directory hierarchies passed on the command line. The patch attempts to overlay this feature with the least amount of structural changes to the existing code path. An attempt was made to support both cd9660 and ffs filesystem formats as appropriate (see below for a disclaimer). The makefs tool uses a tree of struct _fsnode's to represent a given root filesystem hierarchy. This tree is then optionally converted to a filesystem specific data structure that is parsed and written out as a disk image. This patch will take all filesystems passed on the command line and aggregate them into one fsnode based filesystem structure. The first directory passed on the command line is the root of the image. Any additional directories are attached to this root and then converted. When the image is written out, the base path if it exists is used to find the real file to copy out. Notes: CD9660 handle collision code is very buggy (take a look, there are large sections commented out). The mkisofs tool which was used previously doesn't even support collisions within the hierarchy (e.g. 'mkisofs -R -o test.iso /tmp/foo /tmp/foo' will immediately fail). I made a compromise in this patch, I allow file collisions but not directories. Example, let's say we have the following two hierarchies: /tmp/foo/1 /tmp/foo/2 /tmp/foo/3 /tmp/bar/1 /tmp/bar/2 /tmp/bar/3 So 'makefs -t cd9660 -o rockridge test.iso /tmp/foo /tmp/bar' will work as originally designed by the cd9660 filesystem driver. The namespace collision will be handled by internally renaming the ISO9660 filename and creating links. But 'makefs -t cd9660 -o rockridge test.iso /tmp/foo /tmp/foo' will not work as subdirectory foo is duplicated within the hierarchies. If we really want to support renaming directories via links, it will be in my opinion a much more significant re-work of the existing code with little gain. I can't see a single reason why you would want to duplicate directory names within the same ISO9660 image? The ffs driver handles collisions as standard links so everything works as designed. The concept of a link changes depending on the type of ISO9660 image you create (whether you get Rockridge POSIX extended attributes or not, etc.). This is why detection of duplicate directory entires is not handled at the point of fsnode tree creation but at driver specific time. Let me know what you think, I believe this restores POLA to the build while adding a nice feature. Thanks! -aps --Boundary-00=_aPOBPLYw2Mozxms Content-Type: text/plain; charset="iso-8859-1"; name="makefs.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="makefs.diff" Index: walk.c =================================================================== --- walk.c (revision 229517) +++ walk.c (working copy) @@ -166,10 +166,29 @@ create_fsnode(const char *name, struct stat *stbuf cur->type = stbuf->st_mode & S_IFMT; cur->inode->nlink = 1; cur->inode->st = *stbuf; + cur->base_path = NULL; return (cur); } /* + * free_fsnode -- + * Frees memory allocated for a single fsnode + */ +void +free_fsnode(fsnode *node) +{ + + if (node->inode->nlink-- == 1) + free(node->inode); + if (node->symlink) + free(node->symlink); + if (node->base_path) + free(node->base_path); + free(node->name); + free(node); +} + +/* * free_fsnodes -- * Removes node from tree and frees it and all of * its descendants. @@ -207,12 +226,7 @@ free_fsnodes(fsnode *node) cur->child->parent = NULL; free_fsnodes(cur->child); } - if (cur->inode->nlink-- == 1) - free(cur->inode); - if (cur->symlink) - free(cur->symlink); - free(cur->name); - free(cur); + free_fsnode(cur); } } @@ -519,8 +533,8 @@ dump_fsnodes(const char *dir, fsnode *root) errx(1, "Pathname too long."); if (debug & DEBUG_DUMP_FSNODES_VERBOSE) - printf("cur=%8p parent=%8p first=%8p ", - cur, cur->parent, cur->first); + printf("cur=%8p parent=%8p first=%8p base_path=%s", + cur, cur->parent, cur->first, cur->base_path); printf("%7s: %s", inode_type(cur->type), path); if (S_ISLNK(cur->type)) { assert(cur->symlink != NULL); @@ -647,3 +661,30 @@ link_check(fsinode *entry) htused++; return NULL; } + +/* + * mark_base_path -- + * Given a hierarchy of fsnodes, mark the real path + * of the underlying filesystem to copy from. + */ +void +mark_base_path(char *base_path, fsnode *root) +{ + char path[MAXPATHLEN + 1]; + fsnode *cur; + + for (cur = root; cur != NULL; cur = cur->next) { + snprintf(path, sizeof(path), "%s/%s", base_path, + cur->name); + + if (cur->base_path == NULL) { + cur->base_path = malloc(MAXPATHLEN + 1); + if (cur->base_path == NULL) + err(1, "Memory allocation error"); + } + + strncpy(cur->base_path, path, strlen(path) + 1); + if (cur->child != NULL) + mark_base_path(path, cur->child); + } +} Index: makefs.c =================================================================== --- makefs.c (revision 229517) +++ makefs.c (working copy) @@ -86,8 +86,13 @@ main(int argc, char *argv[]) struct timeval start; fstype_t *fstype; fsinfo_t fsoptions; + fsnode *extra_root; + fsnode *last; fsnode *root; int ch, len; + char *extra_root_dir; + char *image_dir; + char *image_name; char *subtree; char *specfile; @@ -241,32 +246,68 @@ main(int argc, char *argv[]) argc -= optind; argv += optind; - if (argc != 2) + if (argc < 2) usage(); + else { + image_name = argv[0]; + image_dir = argv[1]; + argc -= 2; + argv += 2; + } /* -x must be accompanied by -F */ if (fsoptions.onlyspec != 0 && specfile == NULL) errx(1, "-x requires -F mtree-specfile."); /* Accept '-' as meaning "read from standard input". */ - if (strcmp(argv[1], "-") == 0) + if (strcmp(image_dir, "-") == 0) sb.st_mode = S_IFREG; else { - if (stat(argv[1], &sb) == -1) - err(1, "Can't stat `%s'", argv[1]); + if (stat(image_dir, &sb) == -1) + err(1, "Can't stat `%s'", image_dir); } switch (sb.st_mode & S_IFMT) { case S_IFDIR: /* walk the tree */ - subtree = argv[1]; + subtree = image_dir; TIMER_START(start); root = walk_dir(subtree, NULL); TIMER_RESULTS(start, "walk_dir"); + + /* Assume any left over arguments are extra directories */ + while (argc > 0) { + extra_root_dir = argv[0]; + if (stat(extra_root_dir, &sb) == -1) + err(1, "Can't stat `%s'", extra_root_dir); + if (!S_ISDIR(sb.st_mode)) + err(1, "%s' is not a valid directory", + extra_root_dir); + + TIMER_START(start); + extra_root = walk_dir(extra_root_dir, NULL); + TIMER_RESULTS(start, "walk_dir"); + mark_base_path(extra_root_dir, extra_root); + + /* + * Now attach our fsnode tree to the real root + * hierarchy. Order does not matter here, so we + * attach it to the last child skipping the extra + * root "." directory. + */ + last = root; + while (last->next != NULL) + last = last->next; + last->next = extra_root->next; + free_fsnode(extra_root); + + argc--; + argv++; + } break; case S_IFREG: /* read the manifest file */ subtree = "."; TIMER_START(start); - root = read_mtree(argv[1], NULL); + root = read_mtree(image_dir, NULL); TIMER_RESULTS(start, "manifest"); break; default: @@ -288,7 +329,7 @@ main(int argc, char *argv[]) /* build the file system */ TIMER_START(start); - fstype->make_fs(argv[0], subtree, root, &fsoptions); + fstype->make_fs(image_name, subtree, root, &fsoptions); TIMER_RESULTS(start, "make_fs"); free_fsnodes(root); @@ -336,7 +377,7 @@ usage(void) "usage: %s [-t fs-type] [-o fs-options] [-d debug-mask] [-B endian]\n" "\t[-S sector-size] [-M minimum-size] [-m maximum-size] [-s image-size]\n" "\t[-b free-blocks] [-f free-files] [-F mtree-specfile] [-x]\n" -"\t[-N userdb-dir] image-file directory | manifest\n", +"\t[-N userdb-dir] image-file directory | manifest [extra-directory ...]\n", prog); exit(1); } Index: makefs.h =================================================================== --- makefs.h (revision 229517) +++ makefs.h (working copy) @@ -94,6 +94,7 @@ typedef struct _fsnode { fsinode *inode; /* actual inode data */ char *symlink; /* symlink target */ char *contents; /* file to provide contents */ + char *base_path; /* path different from fsnode root */ char *name; /* file name */ int flags; /* misc flags */ } fsnode; @@ -148,10 +149,12 @@ typedef struct { void apply_specfile(const char *, const char *, fsnode *, int); void dump_fsnodes(const char *, fsnode *); +void mark_base_path(char *, fsnode *); const char * inode_type(mode_t); fsnode * read_mtree(const char *, fsnode *); int set_option(option_t *, const char *, const char *); fsnode * walk_dir(const char *, fsnode *); +void free_fsnode(fsnode *); void free_fsnodes(fsnode *); void ffs_prep_opts(fsinfo_t *); Index: cd9660.c =================================================================== --- cd9660.c (revision 229517) +++ cd9660.c (working copy) @@ -1017,6 +1017,7 @@ cd9660_handle_collisions(cd9660node *colliding, in int temp_skip; int flag = 0; cd9660node *end_of_range; + char *temp_file_name; for (iter = TAILQ_FIRST(&colliding->cn_children); iter != NULL && (next = TAILQ_NEXT(iter, cn_next_child)) != NULL;) { @@ -1025,6 +1026,15 @@ cd9660_handle_collisions(cd9660node *colliding, in iter = TAILQ_NEXT(iter, cn_next_child); continue; } + if (S_ISDIR(iter->node->type)) { + temp_file_name = malloc(CD9660MAXPATH + 1); + if (temp_file_name == NULL) + errx(EXIT_FAILURE, "%s: malloc", __func__); + cd9660_compute_full_filename(iter, temp_file_name, 0); + errx(EXIT_FAILURE, + "%s has a duplicate entry in source filesystem", + temp_file_name); + } flag = 1; temp_skip = skip = cd9660_count_collisions(iter); end_of_range = iter; Index: ffs.c =================================================================== --- ffs.c (revision 229517) +++ ffs.c (working copy) @@ -780,9 +780,14 @@ ffs_populate_dir(const char *dir, fsnode *root, fs cur->inode->flags |= FI_WRITTEN; if (cur->contents == NULL) { - if (snprintf(path, sizeof(path), "%s/%s", dir, - cur->name) >= sizeof(path)) - errx(1, "Pathname too long."); + if (!S_ISDIR(cur->type) && cur->base_path) + strncpy(path, cur->base_path, + strlen(cur->base_path) + 1); + else { + if (snprintf(path, sizeof(path), "%s/%s", dir, + cur->name) >= sizeof(path)) + errx(1, "Pathname too long."); + } } if (cur->child != NULL) Index: cd9660/cd9660_write.c =================================================================== --- cd9660/cd9660_write.c (revision 229517) +++ cd9660/cd9660_write.c (working copy) @@ -261,6 +261,7 @@ cd9660_write_path_tables(FILE *fd) static int cd9660_write_file(FILE *fd, cd9660node *writenode) { + char *base_path; char *buf; char *temp_file_name; int ret; @@ -294,9 +295,15 @@ cd9660_write_file(FILE *fd, cd9660node *writenode) INODE_WARNX(("%s: writing inode %d blocks at %" PRIu32, __func__, (int)inode->st.st_ino, inode->ino)); inode->flags |= FI_WRITTEN; - if (writenode->node->contents == NULL) - cd9660_compute_full_filename(writenode, - temp_file_name, 0); + if (writenode->node->contents == NULL) { + if (writenode->node->base_path) { + base_path = writenode->node->base_path; + strncpy(temp_file_name, base_path, + strlen(base_path) + 1); + } else + cd9660_compute_full_filename(writenode, + temp_file_name, 0); + } ret = cd9660_copy_file(fd, writenode->fileDataSector, (writenode->node->contents != NULL) ? writenode->node->contents : temp_file_name); Index: makefs.8 =================================================================== --- makefs.8 (revision 229517) +++ makefs.8 (working copy) @@ -35,7 +35,7 @@ .\" .\" $FreeBSD$ .\" -.Dd January 10, 2009 +.Dd January 4, 2012 .Dt MAKEFS 8 .Os .Sh NAME @@ -58,6 +58,7 @@ .Op Fl t Ar fs-type .Ar image-file .Ar directory | manifest +.Op Ar extra-directory ... .Sh DESCRIPTION The utility .Nm @@ -67,6 +68,13 @@ from the directory tree .Ar directory or from the mtree manifest .Ar manifest . +If optional directory tree +.Ar extra-directory +is passed, then the directory tree of each argument will be merged +into the +.Ar directory +first before creating +.Ar image-file . No special devices or privileges are required to perform this task. .Pp The options are as follows: --Boundary-00=_aPOBPLYw2Mozxms--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201201041842.18854.jkim>