[PATCH v6 0/4] mkfs: add ability to populate filesystem from directory

[PATCH v6 0/4] mkfs: add ability to populate filesystem from directory

[Luca Di Maio]

Currently the only way to pre populate an XFS partition is via the
prototype file. While it works it has some limitations like:
  - not allowed spaces in file names
  - not preserving timestamps of original inodes

This series adds a new -P option to mkfs.xfs that allows users to
populate a newly created filesystem directly from an existing directory.
While similar to the prototype functionality, this doesn't require
writing a prototype file.
The implementation preserves file and directory attributes (ownership,
permissions, timestamps) from the source directory when copying content
to the new filesystem.

[v1] -> [v2]
  remove changes to protofile spec
  ensure backward compatibility
[v2] -> [v3]
  use inode_set_[acm]time() as suggested
  avoid copying atime and ctime
  they are often problematic for reproducibility, and
  mtime is the important information to preserve anyway
[v3] -> [v4]
  rewrite functionality to populate directly from an input directory
  this is similar to mkfs.ext4 option.
[v4] -> [v5]
  reorder patch to make it easier to review
  reflow to keep code below 80 chars
  use _() macro in prints
  add SPDX headers to new files
  fix comment styling
  move from typedef to structs
  move direntry handling to own function
[v5] -> [v6]
  rebase on 6.14

Luca Di Maio (4):
  proto: expose more functions from proto
  populate: add ability to populate a filesystem from a directory
  mkfs: add -P flag to populate a filesystem from a directory
  man: document -P flag to populate a filesystem from a directory

 man/man8/mkfs.xfs.8.in |   7 +
 mkfs/Makefile          |   2 +-
 mkfs/populate.c        | 313 +++++++++++++++++++++++++++++++++++++++++
 mkfs/populate.h        |  10 ++
 mkfs/proto.c           |  33 ++---
 mkfs/proto.h           |  22 +++
 mkfs/xfs_mkfs.c        |  23 ++-
 7 files changed, 385 insertions(+), 25 deletions(-)
 create mode 100644 mkfs/populate.c
 create mode 100644 mkfs/populate.h

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>

--
2.49.0

[PATCH v6 1/4] proto: expose more functions from proto

[Luca Di Maio]

In order to minimize code duplication, we want to expose more functions
from proto. This will simplify alternative implementations of filesystem
population.

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
---
 mkfs/proto.c | 33 ++++++++++++---------------------
 mkfs/proto.h | 22 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/mkfs/proto.c b/mkfs/proto.c
index 7f56a3d..695e1a6 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -16,11 +16,7 @@
  */
 static char *getstr(char **pp);
 static void fail(char *msg, int i);
-static struct xfs_trans * getres(struct xfs_mount *mp, uint blocks);
-static void rsvfile(xfs_mount_t *mp, xfs_inode_t *ip, long long len);
 static int newregfile(char **pp, char **fname);
-static void rtinit(xfs_mount_t *mp);
-static off_t filesize(int fd);
 static int slashes_are_spaces;

 /*
@@ -115,7 +111,7 @@ res_failed(
 	fail(_("cannot reserve space"), i);
 }

-static struct xfs_trans *
+struct xfs_trans *
 getres(
 	struct xfs_mount *mp,
 	uint		blocks)
@@ -196,7 +192,7 @@ getdirentname(
 	return p;
 }

-static void
+void
 rsvfile(
 	xfs_mount_t	*mp,
 	xfs_inode_t	*ip,
@@ -242,7 +238,7 @@ rsvfile(
 		fail(_("committing space for a file failed"), error);
 }

-static void
+void
 writesymlink(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
@@ -303,7 +299,7 @@ writefile_range(
 	}
 }

-static void
+void
 writefile(
 	struct xfs_inode	*ip,
 	const char		*fname,
@@ -410,7 +406,7 @@ writeattr(
 		fail(_("setting xattr value"), error);
 }

-static void
+void
 writeattrs(
 	struct xfs_inode	*ip,
 	const char		*fname,
@@ -467,7 +463,7 @@ newregfile(
 	return fd;
 }

-static void
+void
 newdirent(
 	struct xfs_mount	*mp,
 	struct xfs_trans	*tp,
@@ -498,7 +494,7 @@ newdirent(
 	}
 }

-static void
+void
 newdirectory(
 	xfs_mount_t	*mp,
 	xfs_trans_t	*tp,
@@ -512,7 +508,7 @@ newdirectory(
 		fail(_("directory create error"), error);
 }

-static struct xfs_parent_args *
+struct xfs_parent_args *
 newpptr(
 	struct xfs_mount	*mp)
 {
@@ -526,12 +522,7 @@ newpptr(
 	return ret;
 }

-struct cred {
-	uid_t			cr_uid;
-	gid_t			cr_gid;
-};
-
-static int
+int
 creatproto(
 	struct xfs_trans	**tpp,
 	struct xfs_inode	*dp,
@@ -594,7 +585,7 @@ creatproto(
 }

 /* Create a new metadata root directory. */
-static int
+int
 create_metadir(
 	struct xfs_mount	*mp)
 {
@@ -1151,7 +1142,7 @@ rtinit_groups(
 /*
  * Allocate the realtime bitmap and summary inodes, and fill in data if any.
  */
-static void
+void
 rtinit(
 	struct xfs_mount	*mp)
 {
@@ -1161,7 +1152,7 @@ rtinit(
 		rtinit_nogroups(mp);
 }

-static off_t
+off_t
 filesize(
 	int		fd)
 {
diff --git a/mkfs/proto.h b/mkfs/proto.h
index be1ceb4..9abb9b8 100644
--- a/mkfs/proto.h
+++ b/mkfs/proto.h
@@ -5,10 +5,32 @@
  */
 #ifndef MKFS_PROTO_H_
 #define MKFS_PROTO_H_
+struct cred {
+	uid_t			cr_uid;
+	gid_t			cr_gid;
+};

 char *setup_proto(char *fname);
 void parse_proto(struct xfs_mount *mp, struct fsxattr *fsx, char **pp,
 		int proto_slashes_are_spaces);
 void res_failed(int err);
+struct xfs_trans *getres(struct xfs_mount *mp, uint blocks);
+struct xfs_parent_args *newpptr(struct xfs_mount *mp);
+void writesymlink(struct xfs_trans *tp, struct xfs_inode *ip,
+                 char *buf, int len);
+void writefile(struct xfs_inode *ip, const char *fname, int fd);
+void writeattrs(struct xfs_inode *ip, const char *fname, int fd);
+void rsvfile(xfs_mount_t *mp, xfs_inode_t *ip, long long len);
+void newdirent(struct xfs_mount *mp, struct xfs_trans *tp,
+              struct xfs_inode *pip, struct xfs_name *name,
+              struct xfs_inode *ip, struct xfs_parent_args *ppargs);
+void newdirectory(xfs_mount_t *mp, xfs_trans_t *tp,
+                 xfs_inode_t *dp, xfs_inode_t *pdp);
+int creatproto(struct xfs_trans **tpp, struct xfs_inode *dp,
+              mode_t mode, xfs_dev_t rdev, struct cred *cr,
+              struct fsxattr *fsx, struct xfs_inode **ipp);
+long filesize(int fd);
+void rtinit(struct xfs_mount *mp);
+int create_metadir(struct xfs_mount *mp);

 #endif /* MKFS_PROTO_H_ */
--
2.49.0

[PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Luca Di Maio]

This patch implements the functionality to populate a newly created XFS
filesystem directly from an existing directory structure.

The population process steps are as follows:
  - create the root inode before populating content
  - recursively process nested directories
  - handle regular files, directories, symlinks, char devices, block
    devices, and fifos
  - preserve file and directory attributes (ownership, permissions)
  - preserve timestamps from source files to maintain file history
  - preserve file extended attributes

This functionality makes it easier to create populated filesystems
without having to write protofiles manually.
It's particularly useful for reproducible builds.

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
---
 mkfs/Makefile   |   2 +-
 mkfs/populate.c | 313 ++++++++++++++++++++++++++++++++++++++++++++++++
 mkfs/populate.h |  10 ++
 3 files changed, 324 insertions(+), 1 deletion(-)
 create mode 100644 mkfs/populate.c
 create mode 100644 mkfs/populate.h

diff --git a/mkfs/Makefile b/mkfs/Makefile
index 04905bd..1611751 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -9,7 +9,7 @@ LTCOMMAND = mkfs.xfs
 XFS_PROTOFILE = xfs_protofile.py

 HFILES =
-CFILES = proto.c xfs_mkfs.c
+CFILES = populate.c proto.c xfs_mkfs.c
 CFGFILES = \
 	dax_x86_64.conf \
 	lts_4.19.conf \
diff --git a/mkfs/populate.c b/mkfs/populate.c
new file mode 100644
index 0000000..f5eacbf
--- /dev/null
+++ b/mkfs/populate.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Chainguard, Inc.
+ * All Rights Reserved.
+ * Author: Luca Di Maio <luca.dimaio1@gmail.com>
+ */
+#include <fcntl.h>
+#include <limits.h>
+#include <linux/fs.h>
+#include <stdio.h>
+#include <string.h>
+#include <dirent.h>
+#include <sys/stat.h>
+#include "libxfs.h"
+#include "proto.h"
+
+static void walk_dir(struct xfs_mount *mp, struct xfs_inode *pip,
+			    struct fsxattr *fsxp, char *cur_path);
+
+static void fail(char *msg, int i)
+{
+	fprintf(stderr, _("%s: %s [%d - %s]\n"), progname, msg, i, strerror(i));
+	exit(1);
+}
+
+static int newregfile(char *fname)
+{
+	int fd;
+	off_t size;
+
+	if ((fd = open(fname, O_RDONLY)) < 0 || (size = filesize(fd)) < 0) {
+		fprintf(stderr, _("%s: cannot open %s: %s\n"), progname, fname,
+			strerror(errno));
+		exit(1);
+	}
+
+	return fd;
+}
+
+static void writetimestamps(struct xfs_inode *ip, struct stat statbuf)
+{
+	struct timespec64 ts;
+
+	/*
+	 * Copy timestamps from source file to destination inode.
+	 *  In order to not be influenced by our own access timestamp,
+	 *  we set atime and ctime to mtime of the source file.
+	 *  Usually reproducible archives will delete or not register
+	 *  atime and ctime, for example:
+	 *     https://www.gnu.org/software/tar/manual/html_section/Reproducibility.html
+	 */
+	ts.tv_sec = statbuf.st_mtime;
+	ts.tv_nsec = statbuf.st_mtim.tv_nsec;
+	inode_set_atime_to_ts(VFS_I(ip), ts);
+	inode_set_ctime_to_ts(VFS_I(ip), ts);
+	inode_set_mtime_to_ts(VFS_I(ip), ts);
+
+	return;
+}
+
+static void create_file(struct xfs_mount *mp, struct xfs_inode *pip,
+			struct fsxattr *fsxp, int mode, struct cred creds,
+			struct xfs_name xname, int flags, struct stat file_stat,
+			xfs_dev_t rdev, int fd, char *fname, char *path)
+{
+	int error;
+	struct xfs_parent_args *ppargs = NULL;
+	struct xfs_inode *ip;
+	struct xfs_trans *tp;
+
+	tp = getres(mp, 0);
+	ppargs = newpptr(mp);
+	error = creatproto(&tp, pip, mode, rdev, &creds, fsxp, &ip);
+	if (error)
+		fail(_("Inode allocation failed"), error);
+	libxfs_trans_ijoin(tp, pip, 0);
+	newdirent(mp, tp, pip, &xname, ip, ppargs);
+
+	/*
+	 * copy over timestamps
+	 */
+	writetimestamps(ip, file_stat);
+
+	libxfs_trans_log_inode(tp, ip, flags);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		fail(_("Error encountered creating file from prototype file"),
+		     error);
+
+	libxfs_parent_finish(mp, ppargs);
+
+	/*
+	 * copy over file content, attributes and
+	 * timestamps
+	 */
+	if (fd != 0) {
+		writefile(ip, fname, fd);
+		writeattrs(ip, fname, fd);
+		close(fd);
+	}
+
+	libxfs_irele(ip);
+}
+
+static void handle_direntry(struct xfs_mount *mp, struct xfs_inode *pip,
+			    struct fsxattr *fsxp, char* cur_path, struct dirent *entry)
+{
+	char link_target[PATH_MAX];
+	char path[PATH_MAX];
+	int error;
+	int fd = -1;
+	int flags;
+	int majdev;
+	int mindev;
+	int mode;
+	off_t len;
+	struct cred creds;
+	struct stat file_stat;
+	struct xfs_name xname;
+	struct xfs_parent_args *ppargs = NULL;
+	struct xfs_inode *ip;
+	struct xfs_trans *tp;
+
+	/*
+	 * Skip "." and ".." directories
+	 */
+	if (strcmp(entry->d_name, ".") == 0 ||
+	    strcmp(entry->d_name, "..") == 0) {
+		return;
+	}
+
+	/*
+	 * Create the full path to the original file or directory
+	 */
+	snprintf(path, sizeof(path), "%s/%s", cur_path, entry->d_name);
+
+	if (lstat(path, &file_stat) < 0) {
+		printf("%s (error accessing)\n", entry->d_name);
+		return;
+	}
+
+	memset(&creds, 0, sizeof(creds));
+	creds.cr_uid = file_stat.st_uid;
+	creds.cr_gid = file_stat.st_gid;
+	xname.name = (unsigned char *)entry->d_name;
+	xname.len = strlen(entry->d_name);
+	xname.type = 0;
+	mode = file_stat.st_mode;
+	flags = XFS_ILOG_CORE;
+	switch (file_stat.st_mode & S_IFMT) {
+	case S_IFDIR:
+		tp = getres(mp, 0);
+		error = creatproto(&tp, pip, mode, 0, &creds, fsxp, &ip);
+		if (error)
+			fail(_("Inode allocation failed"), error);
+		ppargs = newpptr(mp);
+		libxfs_trans_ijoin(tp, pip, 0);
+		xname.type = XFS_DIR3_FT_DIR;
+		newdirent(mp, tp, pip, &xname, ip, ppargs);
+		libxfs_bumplink(tp, pip);
+		libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE);
+		newdirectory(mp, tp, ip, pip);
+
+		/*
+		 * copy over timestamps
+		 */
+		writetimestamps(ip, file_stat);
+
+		libxfs_trans_log_inode(tp, ip, flags);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			fail(_("Directory inode allocation failed."), error);
+
+		libxfs_parent_finish(mp, ppargs);
+		tp = NULL;
+
+		walk_dir(mp, ip, fsxp, path);
+
+		libxfs_irele(ip);
+		break;
+	case S_IFREG:
+		fd = newregfile(path);
+		xname.type = XFS_DIR3_FT_REG_FILE;
+		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
+			    0, fd, entry->d_name, path);
+		break;
+	case S_IFLNK:
+		len = readlink(path, link_target, PATH_MAX - 1);
+		tp = getres(mp, XFS_B_TO_FSB(mp, len));
+		ppargs = newpptr(mp);
+		error = creatproto(&tp, pip, mode, 0, &creds, fsxp, &ip);
+		if (error)
+			fail(_("Inode allocation failed"), error);
+		writesymlink(tp, ip, link_target, len);
+		libxfs_trans_ijoin(tp, pip, 0);
+		xname.type = XFS_DIR3_FT_SYMLINK;
+		newdirent(mp, tp, pip, &xname, ip, ppargs);
+
+		/*
+		 * copy over timestamps
+		 */
+		writetimestamps(ip, file_stat);
+
+		libxfs_trans_log_inode(tp, ip, flags);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			fail(_("Error encountered creating file from prototype file"),
+			     error);
+		libxfs_parent_finish(mp, ppargs);
+		libxfs_irele(ip);
+		break;
+	case S_IFCHR:
+		xname.type = XFS_DIR3_FT_CHRDEV;
+		majdev = major(file_stat.st_rdev);
+		mindev = minor(file_stat.st_rdev);
+		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
+			    IRIX_MKDEV(majdev, mindev), 0, entry->d_name, path);
+		break;
+	case S_IFBLK:
+		xname.type = XFS_DIR3_FT_BLKDEV;
+		majdev = major(file_stat.st_rdev);
+		mindev = minor(file_stat.st_rdev);
+		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
+			    IRIX_MKDEV(majdev, mindev), 0, entry->d_name, path);
+		break;
+	case S_IFIFO:
+		flags |= XFS_ILOG_DEV;
+		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
+			    0, 0, entry->d_name, path);
+		break;
+	default:
+		break;
+	}
+}
+
+/*
+ * walk_dir will recursively list files and directories
+ * and populate the mountpoint *mp with them using handle_direntry().
+ */
+static void walk_dir(struct xfs_mount *mp, struct xfs_inode *pip,
+			    struct fsxattr *fsxp, char *cur_path)
+{
+	DIR *dir;
+	struct dirent *entry;
+
+	/*
+	 * open input directory and iterate over all entries in it.
+	 * when another directory is found, we will recursively call
+	 * populatefromdir.
+	 */
+	if ((dir = opendir(cur_path)) == NULL)
+		fail(_("cannot open input dir"), 1);
+	while ((entry = readdir(dir)) != NULL) {
+		handle_direntry(mp, pip, fsxp, cur_path, entry);
+	}
+	closedir(dir);
+}
+
+void populate_from_dir(struct xfs_mount *mp, struct xfs_inode *pip,
+		       struct fsxattr *fsxp, char *cur_path)
+{
+	int error;
+	int mode;
+	struct cred creds;
+	struct xfs_inode *ip;
+	struct xfs_trans *tp;
+
+	/*
+	 * we first ensure we have the root inode
+	 */
+	memset(&creds, 0, sizeof(creds));
+	creds.cr_uid = 0;
+	creds.cr_gid = 0;
+	mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
+	tp = getres(mp, 0);
+	error = creatproto(&tp, pip, mode | S_IFDIR, 0, &creds, fsxp, &ip);
+	if (error)
+		fail(_("Inode allocation failed"), error);
+	pip = ip;
+	mp->m_sb.sb_rootino = ip->i_ino;
+	libxfs_log_sb(tp);
+	newdirectory(mp, tp, ip, pip);
+	libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		fail(_("Inode allocation failed"), error);
+
+	libxfs_parent_finish(mp, NULL);
+
+	/*
+	 * RT initialization.  Do this here to ensure that
+	 * the RT inodes get placed after the root inode.
+	 */
+	error = create_metadir(mp);
+	if (error)
+		fail(_("Creation of the metadata directory inode failed"),
+		     error);
+
+	rtinit(mp);
+
+	/*
+	 * now that we have a root inode, let's
+	 * walk the input dir and populate the partition
+	 */
+	walk_dir(mp, ip, fsxp, cur_path);
+
+	/*
+	 * we free up our root inode
+	 * only when we finished populating the
+	 * root filesystem
+	 */
+	libxfs_irele(ip);
+}
diff --git a/mkfs/populate.h b/mkfs/populate.h
new file mode 100644
index 0000000..d65df57
--- /dev/null
+++ b/mkfs/populate.h
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Chainguard, Inc.
+ * All Rights Reserved.
+ * Author: Luca Di Maio <luca.dimaio1@gmail.com>
+ */
+#ifndef MKFS_POPULATE_H_
+#define MKFS_POPULATE_H_
+void populate_from_dir(xfs_mount_t *mp, xfs_inode_t *pip, struct fsxattr *fsxp, char *name);
+#endif /* MKFS_POPULATE_H_ */
--
2.49.0

Re: [PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Darrick J. Wong]

On Wed, Apr 23, 2025 at 06:03:17PM +0200, Luca Di Maio wrote:
> This patch implements the functionality to populate a newly created XFS
> filesystem directly from an existing directory structure.
> 
> The population process steps are as follows:
>   - create the root inode before populating content
>   - recursively process nested directories
>   - handle regular files, directories, symlinks, char devices, block
>     devices, and fifos
>   - preserve file and directory attributes (ownership, permissions)
>   - preserve timestamps from source files to maintain file history
>   - preserve file extended attributes
> 
> This functionality makes it easier to create populated filesystems
> without having to write protofiles manually.
> It's particularly useful for reproducible builds.
> 
> Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
> ---
>  mkfs/Makefile   |   2 +-
>  mkfs/populate.c | 313 ++++++++++++++++++++++++++++++++++++++++++++++++
>  mkfs/populate.h |  10 ++
>  3 files changed, 324 insertions(+), 1 deletion(-)
>  create mode 100644 mkfs/populate.c
>  create mode 100644 mkfs/populate.h
> 
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index 04905bd..1611751 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -9,7 +9,7 @@ LTCOMMAND = mkfs.xfs
>  XFS_PROTOFILE = xfs_protofile.py
> 
>  HFILES =
> -CFILES = proto.c xfs_mkfs.c
> +CFILES = populate.c proto.c xfs_mkfs.c
>  CFGFILES = \
>  	dax_x86_64.conf \
>  	lts_4.19.conf \
> diff --git a/mkfs/populate.c b/mkfs/populate.c
> new file mode 100644
> index 0000000..f5eacbf
> --- /dev/null
> +++ b/mkfs/populate.c
> @@ -0,0 +1,313 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Chainguard, Inc.
> + * All Rights Reserved.
> + * Author: Luca Di Maio <luca.dimaio1@gmail.com>
> + */
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <linux/fs.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <dirent.h>
> +#include <sys/stat.h>
> +#include "libxfs.h"
> +#include "proto.h"
> +
> +static void walk_dir(struct xfs_mount *mp, struct xfs_inode *pip,
> +			    struct fsxattr *fsxp, char *cur_path);
> +
> +static void fail(char *msg, int i)
> +{
> +	fprintf(stderr, _("%s: %s [%d - %s]\n"), progname, msg, i, strerror(i));
> +	exit(1);
> +}
> +
> +static int newregfile(char *fname)
> +{
> +	int fd;
> +	off_t size;
> +
> +	if ((fd = open(fname, O_RDONLY)) < 0 || (size = filesize(fd)) < 0) {
> +		fprintf(stderr, _("%s: cannot open %s: %s\n"), progname, fname,
> +			strerror(errno));
> +		exit(1);
> +	}
> +
> +	return fd;
> +}

Why is this copy-pasting code from proto.c?  Put the new functions
there, and then you don't need all this externing.

> +
> +static void writetimestamps(struct xfs_inode *ip, struct stat statbuf)
> +{
> +	struct timespec64 ts;
> +
> +	/*
> +	 * Copy timestamps from source file to destination inode.
> +	 *  In order to not be influenced by our own access timestamp,
> +	 *  we set atime and ctime to mtime of the source file.
> +	 *  Usually reproducible archives will delete or not register
> +	 *  atime and ctime, for example:
> +	 *     https://www.gnu.org/software/tar/manual/html_section/Reproducibility.html
> +	 */
> +	ts.tv_sec = statbuf.st_mtime;
> +	ts.tv_nsec = statbuf.st_mtim.tv_nsec;
> +	inode_set_atime_to_ts(VFS_I(ip), ts);
> +	inode_set_ctime_to_ts(VFS_I(ip), ts);
> +	inode_set_mtime_to_ts(VFS_I(ip), ts);

This seems weird to me that you'd set [ac]time to mtime.  Why not open
the source file O_ATIME and copy atime?  And why would copying ctime not
result in a reproducible build?

Not sure what you do about crtime.

> +
> +	return;
> +}
> +
> +static void create_file(struct xfs_mount *mp, struct xfs_inode *pip,
> +			struct fsxattr *fsxp, int mode, struct cred creds,
> +			struct xfs_name xname, int flags, struct stat file_stat,
> +			xfs_dev_t rdev, int fd, char *fname, char *path)
> +{
> +	int error;
> +	struct xfs_parent_args *ppargs = NULL;
> +	struct xfs_inode *ip;
> +	struct xfs_trans *tp;
> +
> +	tp = getres(mp, 0);
> +	ppargs = newpptr(mp);
> +	error = creatproto(&tp, pip, mode, rdev, &creds, fsxp, &ip);
> +	if (error)
> +		fail(_("Inode allocation failed"), error);
> +	libxfs_trans_ijoin(tp, pip, 0);
> +	newdirent(mp, tp, pip, &xname, ip, ppargs);
> +
> +	/*
> +	 * copy over timestamps
> +	 */
> +	writetimestamps(ip, file_stat);
> +
> +	libxfs_trans_log_inode(tp, ip, flags);
> +	error = -libxfs_trans_commit(tp);
> +	if (error)
> +		fail(_("Error encountered creating file from prototype file"),
> +		     error);
> +
> +	libxfs_parent_finish(mp, ppargs);
> +
> +	/*
> +	 * copy over file content, attributes and
> +	 * timestamps
> +	 */
> +	if (fd != 0) {
> +		writefile(ip, fname, fd);
> +		writeattrs(ip, fname, fd);

Since we're adding features, should this read the fsxattr info from the
source file, override it with the set fields in *fsxp, and set that on
the file?  If you're going to slurp up a directory, you might as well
get all the non-xattr file attributes.

> +		close(fd);
> +	}
> +
> +	libxfs_irele(ip);
> +}
> +
> +static void handle_direntry(struct xfs_mount *mp, struct xfs_inode *pip,
> +			    struct fsxattr *fsxp, char* cur_path, struct dirent *entry)
> +{
> +	char link_target[PATH_MAX];
> +	char path[PATH_MAX];
> +	int error;
> +	int fd = -1;
> +	int flags;
> +	int majdev;
> +	int mindev;
> +	int mode;
> +	off_t len;
> +	struct cred creds;
> +	struct stat file_stat;
> +	struct xfs_name xname;
> +	struct xfs_parent_args *ppargs = NULL;
> +	struct xfs_inode *ip;
> +	struct xfs_trans *tp;
> +
> +	/*
> +	 * Skip "." and ".." directories
> +	 */
> +	if (strcmp(entry->d_name, ".") == 0 ||
> +	    strcmp(entry->d_name, "..") == 0) {
> +		return;
> +	}
> +
> +	/*
> +	 * Create the full path to the original file or directory
> +	 */
> +	snprintf(path, sizeof(path), "%s/%s", cur_path, entry->d_name);
> +
> +	if (lstat(path, &file_stat) < 0) {
> +		printf("%s (error accessing)\n", entry->d_name);
> +		return;
> +	}
> +
> +	memset(&creds, 0, sizeof(creds));
> +	creds.cr_uid = file_stat.st_uid;
> +	creds.cr_gid = file_stat.st_gid;
> +	xname.name = (unsigned char *)entry->d_name;
> +	xname.len = strlen(entry->d_name);
> +	xname.type = 0;
> +	mode = file_stat.st_mode;
> +	flags = XFS_ILOG_CORE;
> +	switch (file_stat.st_mode & S_IFMT) {
> +	case S_IFDIR:
> +		tp = getres(mp, 0);
> +		error = creatproto(&tp, pip, mode, 0, &creds, fsxp, &ip);
> +		if (error)
> +			fail(_("Inode allocation failed"), error);
> +		ppargs = newpptr(mp);
> +		libxfs_trans_ijoin(tp, pip, 0);
> +		xname.type = XFS_DIR3_FT_DIR;
> +		newdirent(mp, tp, pip, &xname, ip, ppargs);
> +		libxfs_bumplink(tp, pip);
> +		libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE);
> +		newdirectory(mp, tp, ip, pip);
> +
> +		/*
> +		 * copy over timestamps
> +		 */
> +		writetimestamps(ip, file_stat);
> +
> +		libxfs_trans_log_inode(tp, ip, flags);
> +		error = -libxfs_trans_commit(tp);
> +		if (error)
> +			fail(_("Directory inode allocation failed."), error);
> +
> +		libxfs_parent_finish(mp, ppargs);
> +		tp = NULL;

Shouldn't this copy xattrs and fsxattrs to directories and symlinks too?

> +
> +		walk_dir(mp, ip, fsxp, path);
> +
> +		libxfs_irele(ip);
> +		break;
> +	case S_IFREG:
> +		fd = newregfile(path);
> +		xname.type = XFS_DIR3_FT_REG_FILE;
> +		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
> +			    0, fd, entry->d_name, path);
> +		break;
> +	case S_IFLNK:
> +		len = readlink(path, link_target, PATH_MAX - 1);
> +		tp = getres(mp, XFS_B_TO_FSB(mp, len));
> +		ppargs = newpptr(mp);
> +		error = creatproto(&tp, pip, mode, 0, &creds, fsxp, &ip);
> +		if (error)
> +			fail(_("Inode allocation failed"), error);
> +		writesymlink(tp, ip, link_target, len);
> +		libxfs_trans_ijoin(tp, pip, 0);
> +		xname.type = XFS_DIR3_FT_SYMLINK;
> +		newdirent(mp, tp, pip, &xname, ip, ppargs);
> +
> +		/*
> +		 * copy over timestamps
> +		 */
> +		writetimestamps(ip, file_stat);
> +
> +		libxfs_trans_log_inode(tp, ip, flags);
> +		error = -libxfs_trans_commit(tp);
> +		if (error)
> +			fail(_("Error encountered creating file from prototype file"),
> +			     error);
> +		libxfs_parent_finish(mp, ppargs);
> +		libxfs_irele(ip);
> +		break;
> +	case S_IFCHR:
> +		xname.type = XFS_DIR3_FT_CHRDEV;
> +		majdev = major(file_stat.st_rdev);
> +		mindev = minor(file_stat.st_rdev);
> +		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
> +			    IRIX_MKDEV(majdev, mindev), 0, entry->d_name, path);
> +		break;
> +	case S_IFBLK:
> +		xname.type = XFS_DIR3_FT_BLKDEV;
> +		majdev = major(file_stat.st_rdev);
> +		mindev = minor(file_stat.st_rdev);
> +		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
> +			    IRIX_MKDEV(majdev, mindev), 0, entry->d_name, path);
> +		break;
> +	case S_IFIFO:
> +		flags |= XFS_ILOG_DEV;
> +		create_file(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
> +			    0, 0, entry->d_name, path);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +/*
> + * walk_dir will recursively list files and directories
> + * and populate the mountpoint *mp with them using handle_direntry().
> + */
> +static void walk_dir(struct xfs_mount *mp, struct xfs_inode *pip,
> +			    struct fsxattr *fsxp, char *cur_path)
> +{
> +	DIR *dir;
> +	struct dirent *entry;
> +
> +	/*
> +	 * open input directory and iterate over all entries in it.
> +	 * when another directory is found, we will recursively call
> +	 * populatefromdir.
> +	 */
> +	if ((dir = opendir(cur_path)) == NULL)
> +		fail(_("cannot open input dir"), 1);
> +	while ((entry = readdir(dir)) != NULL) {
> +		handle_direntry(mp, pip, fsxp, cur_path, entry);
> +	}
> +	closedir(dir);
> +}

nftw() ?  Which has the nice feature of constraining the number of open
dirs at any given time.

--D

> +
> +void populate_from_dir(struct xfs_mount *mp, struct xfs_inode *pip,
> +		       struct fsxattr *fsxp, char *cur_path)
> +{
> +	int error;
> +	int mode;
> +	struct cred creds;
> +	struct xfs_inode *ip;
> +	struct xfs_trans *tp;
> +
> +	/*
> +	 * we first ensure we have the root inode
> +	 */
> +	memset(&creds, 0, sizeof(creds));
> +	creds.cr_uid = 0;
> +	creds.cr_gid = 0;
> +	mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
> +	tp = getres(mp, 0);
> +	error = creatproto(&tp, pip, mode | S_IFDIR, 0, &creds, fsxp, &ip);
> +	if (error)
> +		fail(_("Inode allocation failed"), error);
> +	pip = ip;
> +	mp->m_sb.sb_rootino = ip->i_ino;
> +	libxfs_log_sb(tp);
> +	newdirectory(mp, tp, ip, pip);
> +	libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> +	error = -libxfs_trans_commit(tp);
> +	if (error)
> +		fail(_("Inode allocation failed"), error);
> +
> +	libxfs_parent_finish(mp, NULL);
> +
> +	/*
> +	 * RT initialization.  Do this here to ensure that
> +	 * the RT inodes get placed after the root inode.
> +	 */
> +	error = create_metadir(mp);
> +	if (error)
> +		fail(_("Creation of the metadata directory inode failed"),
> +		     error);
> +
> +	rtinit(mp);
> +
> +	/*
> +	 * now that we have a root inode, let's
> +	 * walk the input dir and populate the partition
> +	 */
> +	walk_dir(mp, ip, fsxp, cur_path);
> +
> +	/*
> +	 * we free up our root inode
> +	 * only when we finished populating the
> +	 * root filesystem
> +	 */
> +	libxfs_irele(ip);
> +}
> diff --git a/mkfs/populate.h b/mkfs/populate.h
> new file mode 100644
> index 0000000..d65df57
> --- /dev/null
> +++ b/mkfs/populate.h
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Chainguard, Inc.
> + * All Rights Reserved.
> + * Author: Luca Di Maio <luca.dimaio1@gmail.com>
> + */
> +#ifndef MKFS_POPULATE_H_
> +#define MKFS_POPULATE_H_
> +void populate_from_dir(xfs_mount_t *mp, xfs_inode_t *pip, struct fsxattr *fsxp, char *name);
> +#endif /* MKFS_POPULATE_H_ */
> --
> 2.49.0
> 

Re: [PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Luca Di Maio]

On Wed, Apr 23, 2025 at 01:23:58PM -0700, Darrick J. Wong wrote:
> On Wed, Apr 23, 2025 at 06:03:17PM +0200, Luca Di Maio wrote:
> > +static void fail(char *msg, int i)
> > +{
> > +   fprintf(stderr, _("%s: %s [%d - %s]\n"), progname, msg, i, strerror(i));
> > +   exit(1);
> > +}
> > +
> > +static int newregfile(char *fname)
> > +{
> > +   int fd;
> > +   off_t size;
> > +
> > +   if ((fd = open(fname, O_RDONLY)) < 0 || (size = filesize(fd)) < 0) {
> > +           fprintf(stderr, _("%s: cannot open %s: %s\n"), progname, fname,
> > +                   strerror(errno));
> > +           exit(1);
> > +   }
> > +
> > +   return fd;
> > +}
>
> Why is this copy-pasting code from proto.c?  Put the new functions
> there, and then you don't need all this externing.
>

Right, this is because with a separate flag I thought it would have been
better to keep it in a separate file.
With the new behaviour you proposed in the previous mail (one -p flag,
check if file/directory) then I can unify back into proto.c, thus
removing all the exported functions changes.

> > +
> > +static void writetimestamps(struct xfs_inode *ip, struct stat statbuf)
> > +{
> > +   struct timespec64 ts;
> > +
> > +   /*
> > +    * Copy timestamps from source file to destination inode.
> > +    *  In order to not be influenced by our own access timestamp,
> > +    *  we set atime and ctime to mtime of the source file.
> > +    *  Usually reproducible archives will delete or not register
> > +    *  atime and ctime, for example:
> > +    *     https://www.gnu.org/software/tar/manual/html_section/Reproducibility.html
> > +    */
> > +   ts.tv_sec = statbuf.st_mtime;
> > +   ts.tv_nsec = statbuf.st_mtim.tv_nsec;
> > +   inode_set_atime_to_ts(VFS_I(ip), ts);
> > +   inode_set_ctime_to_ts(VFS_I(ip), ts);
> > +   inode_set_mtime_to_ts(VFS_I(ip), ts);
>
> This seems weird to me that you'd set [ac]time to mtime.  Why not open
> the source file O_ATIME and copy atime?  And why would copying ctime not
> result in a reproducible build?
>
> Not sure what you do about crtime.
>

The problem stems from the extraction of the artifact. Usually
reproducible archives will remove [ac]time and only keep mtime, but in
the moment that a file is extracted, any filesystem will assign [ac]time
to the moment of extraction.
This will add randomness not to the filesystem itself, because it will
be reproducible if acting on the same extracted archive, but it will not
be reproducible if acting on a new extraction of the same archive.

Another approach we can do is what mkfs.ext4's populate functionality is
doing: while it preserves mtime, [cr,a,c]time is set to whatever time the
mkfs command is running.

This would make it preserve the important timestamp (mtime) and move the
"problem" of the reproducible/changing timestamp to the environment,
while keeping the behaviour of mkfs.xfs sensible

What do you think?

> > +   /*
> > +    * copy over file content, attributes and
> > +    * timestamps
> > +    */
> > +   if (fd != 0) {
> > +           writefile(ip, fname, fd);
> > +           writeattrs(ip, fname, fd);
>
> Since we're adding features, should this read the fsxattr info from the
> source file, override it with the set fields in *fsxp, and set that on
> the file?  If you're going to slurp up a directory, you might as well
> get all the non-xattr file attributes.
>

Right, I thought creatproto() did that, but now I see that this is done
only for the root inode, I'll add this for others too, thanks.

> > +           libxfs_parent_finish(mp, ppargs);
> > +           tp = NULL;
>
> Shouldn't this copy xattrs and fsxattrs to directories and symlinks too?
>

Right, will add, thanks.

> > +/*
> > + * walk_dir will recursively list files and directories
> > + * and populate the mountpoint *mp with them using handle_direntry().
> > + */
> > +static void walk_dir(struct xfs_mount *mp, struct xfs_inode *pip,
> > +                       struct fsxattr *fsxp, char *cur_path)
> > +{
> > +   DIR *dir;
> > +   struct dirent *entry;
> > +
> > +   /*
> > +    * open input directory and iterate over all entries in it.
> > +    * when another directory is found, we will recursively call
> > +    * populatefromdir.
> > +    */
> > +   if ((dir = opendir(cur_path)) == NULL)
> > +           fail(_("cannot open input dir"), 1);
> > +   while ((entry = readdir(dir)) != NULL) {
> > +           handle_direntry(mp, pip, fsxp, cur_path, entry);
> > +   }
> > +   closedir(dir);
> > +}
>
> nftw() ?  Which has the nice feature of constraining the number of open
> dirs at any given time.
>
> --D
>

The problem with nftw() is that working with callback functions, we will
need to switch to static variables for state, for example to keep track
of each ip's pip, while with the recursive approach we can have some
state and basically walk_dir() behaves similar to parseproto(), making
changes to the rest of the file minimal.
This seems to involve a lot more changes than now where we're basically
just adding a limited number of functions to proto.c.

Thanks again for the review Darrick,
I'll wait for your feedback on the walk_dir() vs nftw() and the [ac]time
approach,
thanks

L.

Re: [PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Darrick J. Wong]

On Thu, Apr 24, 2025 at 06:09:45PM +0200, Luca Di Maio wrote:
> On Wed, Apr 23, 2025 at 01:23:58PM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 23, 2025 at 06:03:17PM +0200, Luca Di Maio wrote:
> > > +static void fail(char *msg, int i)
> > > +{
> > > +   fprintf(stderr, _("%s: %s [%d - %s]\n"), progname, msg, i, strerror(i));
> > > +   exit(1);
> > > +}
> > > +
> > > +static int newregfile(char *fname)
> > > +{
> > > +   int fd;
> > > +   off_t size;
> > > +
> > > +   if ((fd = open(fname, O_RDONLY)) < 0 || (size = filesize(fd)) < 0) {
> > > +           fprintf(stderr, _("%s: cannot open %s: %s\n"), progname, fname,
> > > +                   strerror(errno));
> > > +           exit(1);
> > > +   }
> > > +
> > > +   return fd;
> > > +}
> >
> > Why is this copy-pasting code from proto.c?  Put the new functions
> > there, and then you don't need all this externing.
> >
> 
> Right, this is because with a separate flag I thought it would have been
> better to keep it in a separate file.

Common functionality goes together in a C module, regardless of how it
gets called.

> With the new behaviour you proposed in the previous mail (one -p flag,
> check if file/directory) then I can unify back into proto.c, thus
> removing all the exported functions changes.

<nod>

> > > +
> > > +static void writetimestamps(struct xfs_inode *ip, struct stat statbuf)
> > > +{
> > > +   struct timespec64 ts;
> > > +
> > > +   /*
> > > +    * Copy timestamps from source file to destination inode.
> > > +    *  In order to not be influenced by our own access timestamp,
> > > +    *  we set atime and ctime to mtime of the source file.
> > > +    *  Usually reproducible archives will delete or not register
> > > +    *  atime and ctime, for example:
> > > +    *     https://www.gnu.org/software/tar/manual/html_section/Reproducibility.html
> > > +    */
> > > +   ts.tv_sec = statbuf.st_mtime;
> > > +   ts.tv_nsec = statbuf.st_mtim.tv_nsec;
> > > +   inode_set_atime_to_ts(VFS_I(ip), ts);
> > > +   inode_set_ctime_to_ts(VFS_I(ip), ts);
> > > +   inode_set_mtime_to_ts(VFS_I(ip), ts);
> >
> > This seems weird to me that you'd set [ac]time to mtime.  Why not open
> > the source file O_ATIME and copy atime?  And why would copying ctime not
> > result in a reproducible build?
> >
> > Not sure what you do about crtime.
> >
> 
> The problem stems from the extraction of the artifact. Usually
> reproducible archives will remove [ac]time and only keep mtime, but in
> the moment that a file is extracted, any filesystem will assign [ac]time
> to the moment of extraction.
> This will add randomness not to the filesystem itself, because it will
> be reproducible if acting on the same extracted archive, but it will not
> be reproducible if acting on a new extraction of the same archive.
> 
> Another approach we can do is what mkfs.ext4's populate functionality is
> doing: while it preserves mtime, [cr,a,c]time is set to whatever time the
> mkfs command is running.
> 
> This would make it preserve the important timestamp (mtime) and move the
> "problem" of the reproducible/changing timestamp to the environment,
> while keeping the behaviour of mkfs.xfs sensible
> 
> What do you think?

The thing is, if you were relying on atime/mtime for detection of "file
data changed since last read" then /not/ copying atime into the
filesystem breaks that property in the image.

How about copying [acm]time from the source file by default, but then
add a new -p noatime option to skip the atime?

ctime/crtime should be the current time when mkfs command is running.
I assume that you have a gettimeofday type wrapper that makes it always
return the same value?

> > > +   /*
> > > +    * copy over file content, attributes and
> > > +    * timestamps
> > > +    */
> > > +   if (fd != 0) {
> > > +           writefile(ip, fname, fd);
> > > +           writeattrs(ip, fname, fd);
> >
> > Since we're adding features, should this read the fsxattr info from the
> > source file, override it with the set fields in *fsxp, and set that on
> > the file?  If you're going to slurp up a directory, you might as well
> > get all the non-xattr file attributes.
> >
> 
> Right, I thought creatproto() did that, but now I see that this is done
> only for the root inode, I'll add this for others too, thanks.

Right.

> > > +           libxfs_parent_finish(mp, ppargs);
> > > +           tp = NULL;
> >
> > Shouldn't this copy xattrs and fsxattrs to directories and symlinks too?
> >
> 
> Right, will add, thanks.

<nod>

> > > +/*
> > > + * walk_dir will recursively list files and directories
> > > + * and populate the mountpoint *mp with them using handle_direntry().
> > > + */
> > > +static void walk_dir(struct xfs_mount *mp, struct xfs_inode *pip,
> > > +                       struct fsxattr *fsxp, char *cur_path)
> > > +{
> > > +   DIR *dir;
> > > +   struct dirent *entry;
> > > +
> > > +   /*
> > > +    * open input directory and iterate over all entries in it.
> > > +    * when another directory is found, we will recursively call
> > > +    * populatefromdir.
> > > +    */
> > > +   if ((dir = opendir(cur_path)) == NULL)
> > > +           fail(_("cannot open input dir"), 1);
> > > +   while ((entry = readdir(dir)) != NULL) {
> > > +           handle_direntry(mp, pip, fsxp, cur_path, entry);
> > > +   }
> > > +   closedir(dir);
> > > +}
> >
> > nftw() ?  Which has the nice feature of constraining the number of open
> > dirs at any given time.
> >
> > --D
> >
> 
> The problem with nftw() is that working with callback functions, we will
> need to switch to static variables for state, for example to keep track
> of each ip's pip, while with the recursive approach we can have some
> state and basically walk_dir() behaves similar to parseproto(), making
> changes to the rest of the file minimal.
> This seems to involve a lot more changes than now where we're basically
> just adding a limited number of functions to proto.c.

Eck, ok.  Never mind then.  I guess we could try to bump RLIMIT_NOFILE
in that case to avoid EMFILE.

--D

> Thanks again for the review Darrick,
> I'll wait for your feedback on the walk_dir() vs nftw() and the [ac]time
> approach,
> thanks
> 
> L.
> 

Re: [PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Christoph Hellwig]

On Thu, Apr 24, 2025 at 03:00:41PM -0700, Darrick J. Wong wrote:
> The thing is, if you were relying on atime/mtime for detection of "file
> data changed since last read" then /not/ copying atime into the
> filesystem breaks that property in the image.

I don't think that matter for images, because no software will keep
running over the upgrade of the image.  Also plenty of people run
with noatime, and btrfs even defaulted to it for a while (not sure if
it still does).

At the same time having the same behavior as mkfs.ext4 is a good thing
by itself because people obviously have been using it and consistency
is always a good thing.

> How about copying [acm]time from the source file by default, but then
> add a new -p noatime option to skip the atime?

I'd probably invert the polarity.  When building an image keeping
atime especially and also ctime is usually not very useful.  But that
would give folks who need it for some reason a way to do so.

Re: [PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Darrick J. Wong]

On Fri, Apr 25, 2025 at 06:10:14AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 24, 2025 at 03:00:41PM -0700, Darrick J. Wong wrote:
> > The thing is, if you were relying on atime/mtime for detection of "file
> > data changed since last read" then /not/ copying atime into the
> > filesystem breaks that property in the image.
> 
> I don't think that matter for images, because no software will keep
> running over the upgrade of the image.  Also plenty of people run
> with noatime, and btrfs even defaulted to it for a while (not sure if
> it still does).
> 
> At the same time having the same behavior as mkfs.ext4 is a good thing
> by itself because people obviously have been using it and consistency
> is always a good thing.

I don't see where mke2fs -d actually copies i_mtime into the filesystem.
In misc/create_inode.c I see a lot of:

	now = fs->now ? fs->now : time(0);
	ext2fs_inode_xtime_set(&inode, i_atime, now);
	ext2fs_inode_xtime_set(&inode, i_ctime, now);
	ext2fs_inode_xtime_set(&inode, i_mtime, now);

which implies that all three are set to a predetermined timestamp or the
current timestamp.

Also while I'm scanning create_inode.c, do you want to preserve
hardlinks?

> > How about copying [acm]time from the source file by default, but then
> > add a new -p noatime option to skip the atime?
> 
> I'd probably invert the polarity.  When building an image keeping
> atime especially and also ctime is usually not very useful.  But that
> would give folks who need it for some reason a way to do so.

Either's fine with me.

--D

Re: [PATCH v6 2/4] populate: add ability to populate a filesystem from a directory

[Luca Di Maio]

Thanks Darrick, Christoph for the replies:

On Fri, Apr 25, 2025 at 08:00:55AM -0700, Darrick J. Wong wrote:
> On Fri, Apr 25, 2025 at 06:10:14AM -0700, Christoph Hellwig wrote:
> > On Thu, Apr 24, 2025 at 03:00:41PM -0700, Darrick J. Wong wrote:
> > > The thing is, if you were relying on atime/mtime for detection of "file
> > > data changed since last read" then /not/ copying atime into the
> > > filesystem breaks that property in the image.
> >
> > I don't think that matter for images, because no software will keep
> > running over the upgrade of the image.  Also plenty of people run
> > with noatime, and btrfs even defaulted to it for a while (not sure if
> > it still does).
> >
> > At the same time having the same behavior as mkfs.ext4 is a good thing
> > by itself because people obviously have been using it and consistency
> > is always a good thing.
>
> I don't see where mke2fs -d actually copies i_mtime into the filesystem.
> In misc/create_inode.c I see a lot of:
>
> 	now = fs->now ? fs->now : time(0);
> 	ext2fs_inode_xtime_set(&inode, i_atime, now);
> 	ext2fs_inode_xtime_set(&inode, i_ctime, now);
> 	ext2fs_inode_xtime_set(&inode, i_mtime, now);
>
> which implies that all three are set to a predetermined timestamp or the
> current timestamp.

I'm going to set them to current timestamp as a default (so noatime,
noctime will be true by default)

> Also while I'm scanning create_inode.c, do you want to preserve
> hardlinks?

Right, will work on this too

> > > How about copying [acm]time from the source file by default, but then
> > > add a new -p noatime option to skip the atime?
> >
> > I'd probably invert the polarity.  When building an image keeping
> > atime especially and also ctime is usually not very useful.  But that
> > would give folks who need it for some reason a way to do so.
>
> Either's fine with me.
>
> --D

Perfect, I'll default to current timestamp for [ac]time.

Thanks

[PATCH v6 3/4] mkfs: add -P flag to populate a filesystem from a directory

[Luca Di Maio]

Add a `-P` flag to populate a newly created filesystem from a directory.
This flag will be mutually exclusive with the `-p` prototype flag.

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
---
 mkfs/xfs_mkfs.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 3f4455d..57dbeba 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -14,6 +14,7 @@
 #include "libfrog/dahashselftest.h"
 #include "libfrog/fsproperties.h"
 #include "proto.h"
+#include "populate.h"
 #include <ini.h>

 #define TERABYTES(count, blog)	((uint64_t)(count) << (40 - (blog)))
@@ -1022,6 +1023,7 @@ struct cli_params {

 	char	*cfgfile;
 	char	*protofile;
+	char	*directory;

 	enum fsprop_autofsck autofsck;

@@ -1170,6 +1172,7 @@ usage( void )
 /* naming */		[-n size=num,version=2|ci,ftype=0|1,parent=0|1]]\n\
 /* no-op info only */	[-N]\n\
 /* prototype file */	[-p fname]\n\
+/* populate */		[-P directory]\n\
 /* quiet */		[-q]\n\
 /* realtime subvol */	[-r extsize=num,size=num,rtdev=xxx,rgcount=n,rgsize=n,\n\
 			    concurrency=num]\n\
@@ -5254,7 +5257,7 @@ main(
 	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
 	memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));

-	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV",
+	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:P:m:n:KNp:qr:s:CfV",
 					long_options, &option_index)) != EOF) {
 		switch (c) {
 		case 0:
@@ -5280,6 +5283,9 @@ main(
 				illegal(optarg, "L");
 			cfg.label = optarg;
 			break;
+		case 'P':
+			cli.directory = optarg;
+			break;
 		case 'N':
 			dry_run = 1;
 			break;
@@ -5478,9 +5484,20 @@ main(
 		initialise_ag_freespace(mp, agno, worst_freelist);

 	/*
-	 * Allocate the root inode and anything else in the proto file.
+	 * Allocate the root inode and anything else in the proto file or source
+	 * directory.
+	 * Both functions will allocate the root inode, so we use them mutually.
 	 */
-	parse_proto(mp, &cli.fsx, &protostring, cli.proto_slashes_are_spaces);
+	if (cli.protofile && cli.directory) {
+		fprintf(stderr,
+			_("%s: error: specifying both -P and -p is not supported\n"),
+			progname);
+		exit(1);
+	}
+	if (!cli.directory)
+		parse_proto(mp, &cli.fsx, &protostring, cli.proto_slashes_are_spaces);
+	else
+		populate_from_dir(mp, NULL, &cli.fsx, cli.directory);

 	/*
 	 * Protect ourselves against possible stupidity
--
2.49.0

Re: [PATCH v6 3/4] mkfs: add -P flag to populate a filesystem from a directory

[Darrick J. Wong]

On Wed, Apr 23, 2025 at 06:03:18PM +0200, Luca Di Maio wrote:
> Add a `-P` flag to populate a newly created filesystem from a directory.
> This flag will be mutually exclusive with the `-p` prototype flag.
> 
> Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
> ---
>  mkfs/xfs_mkfs.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 3f4455d..57dbeba 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -14,6 +14,7 @@
>  #include "libfrog/dahashselftest.h"
>  #include "libfrog/fsproperties.h"
>  #include "proto.h"
> +#include "populate.h"
>  #include <ini.h>
> 
>  #define TERABYTES(count, blog)	((uint64_t)(count) << (40 - (blog)))
> @@ -1022,6 +1023,7 @@ struct cli_params {
> 
>  	char	*cfgfile;
>  	char	*protofile;
> +	char	*directory;
> 
>  	enum fsprop_autofsck autofsck;
> 
> @@ -1170,6 +1172,7 @@ usage( void )
>  /* naming */		[-n size=num,version=2|ci,ftype=0|1,parent=0|1]]\n\
>  /* no-op info only */	[-N]\n\
>  /* prototype file */	[-p fname]\n\
> +/* populate */		[-P directory]\n\
>  /* quiet */		[-q]\n\
>  /* realtime subvol */	[-r extsize=num,size=num,rtdev=xxx,rgcount=n,rgsize=n,\n\
>  			    concurrency=num]\n\
> @@ -5254,7 +5257,7 @@ main(
>  	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
>  	memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));
> 
> -	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV",
> +	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:P:m:n:KNp:qr:s:CfV",
>  					long_options, &option_index)) != EOF) {
>  		switch (c) {
>  		case 0:
> @@ -5280,6 +5283,9 @@ main(
>  				illegal(optarg, "L");
>  			cfg.label = optarg;
>  			break;
> +		case 'P':
> +			cli.directory = optarg;
> +			break;

Uh... why not modify setup_proto to check the mode of the opened fd, and
call populate_from_dir if it's a directory?  Then you don't need all the
extra option parsing code.  It's not as if -p <path> has ever worked on
a directory.

--D

>  		case 'N':
>  			dry_run = 1;
>  			break;
> @@ -5478,9 +5484,20 @@ main(
>  		initialise_ag_freespace(mp, agno, worst_freelist);
> 
>  	/*
> -	 * Allocate the root inode and anything else in the proto file.
> +	 * Allocate the root inode and anything else in the proto file or source
> +	 * directory.
> +	 * Both functions will allocate the root inode, so we use them mutually.
>  	 */
> -	parse_proto(mp, &cli.fsx, &protostring, cli.proto_slashes_are_spaces);
> +	if (cli.protofile && cli.directory) {
> +		fprintf(stderr,
> +			_("%s: error: specifying both -P and -p is not supported\n"),
> +			progname);
> +		exit(1);
> +	}
> +	if (!cli.directory)
> +		parse_proto(mp, &cli.fsx, &protostring, cli.proto_slashes_are_spaces);
> +	else
> +		populate_from_dir(mp, NULL, &cli.fsx, cli.directory);
> 
>  	/*
>  	 * Protect ourselves against possible stupidity
> --
> 2.49.0
> 

Re: [PATCH v6 3/4] mkfs: add -P flag to populate a filesystem from a directory

[Luca Di Maio]

On Wed, Apr 23, 2025 at 01:09:14PM -0700, Darrick J. Wong wrote:
> On Wed, Apr 23, 2025 at 06:03:18PM +0200, Luca Di Maio wrote:
> > -	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV",
> > +	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:P:m:n:KNp:qr:s:CfV",
> >  					long_options, &option_index)) != EOF) {
> >  		switch (c) {
> >  		case 0:
> > @@ -5280,6 +5283,9 @@ main(
> >  				illegal(optarg, "L");
> >  			cfg.label = optarg;
> >  			break;
> > +		case 'P':
> > +			cli.directory = optarg;
> > +			break;
>
> Uh... why not modify setup_proto to check the mode of the opened fd, and
> call populate_from_dir if it's a directory?  Then you don't need all the
> extra option parsing code.  It's not as if -p <path> has ever worked on
> a directory.
>
> --D
>

Alright, that makes things easier yes, I'll just make sure to clearly
explain the difference in the man page and help so it's clearer,
something like:

```
-p prototype_options
Section Name: [proto]
	These options specify the prototype parameters for populating the
	filesystem.  The  valid  prototype_options are:

		[file=]

		The  file= prefix is not required for this CLI argument for
		legacy reasons.  If specified as a config file directive,
		the prefix is required.

		[file=]directory

		If the optional prototype argument is given,
		and it's a directory, mkfs.xfs will copy the contents
		of the given directory or tarball into the root
		directory of the file system.

		[file=]protofile

		If  the  optional prototype argument is given,
		and it's a file, [ ... continue with existing man ... ]
```

So it's clear from the docs that the option is there and it's the same
flag.

L.

Re: [PATCH v6 3/4] mkfs: add -P flag to populate a filesystem from a directory

[Darrick J. Wong]

On Thu, Apr 24, 2025 at 02:01:06PM +0200, Luca Di Maio wrote:
> On Wed, Apr 23, 2025 at 01:09:14PM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 23, 2025 at 06:03:18PM +0200, Luca Di Maio wrote:
> > > -	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV",
> > > +	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:P:m:n:KNp:qr:s:CfV",
> > >  					long_options, &option_index)) != EOF) {
> > >  		switch (c) {
> > >  		case 0:
> > > @@ -5280,6 +5283,9 @@ main(
> > >  				illegal(optarg, "L");
> > >  			cfg.label = optarg;
> > >  			break;
> > > +		case 'P':
> > > +			cli.directory = optarg;
> > > +			break;
> >
> > Uh... why not modify setup_proto to check the mode of the opened fd, and
> > call populate_from_dir if it's a directory?  Then you don't need all the
> > extra option parsing code.  It's not as if -p <path> has ever worked on
> > a directory.
> >
> > --D
> >
> 
> Alright, that makes things easier yes, I'll just make sure to clearly
> explain the difference in the man page and help so it's clearer,
> something like:
> 
> ```
> -p prototype_options
> Section Name: [proto]
> 	These options specify the prototype parameters for populating the
> 	filesystem.  The  valid  prototype_options are:
> 
> 		[file=]
> 
> 		The  file= prefix is not required for this CLI argument for
> 		legacy reasons.  If specified as a config file directive,
> 		the prefix is required.
> 
> 		[file=]directory
> 
> 		If the optional prototype argument is given,
> 		and it's a directory, mkfs.xfs will copy the contents
> 		of the given directory or tarball into the root
> 		directory of the file system.
> 
> 		[file=]protofile
> 
> 		If  the  optional prototype argument is given,
> 		and it's a file, [ ... continue with existing man ... ]
> ```
> 
> So it's clear from the docs that the option is there and it's the same
> flag.

Sounds good to me!

--D

> L.
> 

[PATCH v6 4/4] man: document -P flag to populate a filesystem from a directory

[Luca Di Maio]

Document the -P flag into man page.

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
---
 man/man8/mkfs.xfs.8.in | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/man/man8/mkfs.xfs.8.in b/man/man8/mkfs.xfs.8.in
index 37e3a88..507a2f9 100644
--- a/man/man8/mkfs.xfs.8.in
+++ b/man/man8/mkfs.xfs.8.in
@@ -1291,6 +1291,13 @@ creating the file system.
 .B \-K
 Do not attempt to discard blocks at mkfs time.
 .TP
+.BI \-P " directory"
+Populate the filesystem starting from a
+.IR directory .
+This will copy the contents of the given directory into the root directory of
+the filesystem. Ownership, Extended attributes, Permissions and timestamps will
+be preserved from the source.
+.TP
 .B \-V
 Prints the version number and exits.
 .SH Configuration File Format
--
2.49.0

Re: [PATCH v6 0/4] mkfs: add ability to populate filesystem from directory

[Darrick J. Wong]

On Wed, Apr 23, 2025 at 06:03:15PM +0200, Luca Di Maio wrote:
> Currently the only way to pre populate an XFS partition is via the
> prototype file. While it works it has some limitations like:
>   - not allowed spaces in file names
>   - not preserving timestamps of original inodes
> 
> This series adds a new -P option to mkfs.xfs that allows users to
> populate a newly created filesystem directly from an existing directory.
> While similar to the prototype functionality, this doesn't require
> writing a prototype file.
> The implementation preserves file and directory attributes (ownership,
> permissions, timestamps) from the source directory when copying content
> to the new filesystem.
> 
> [v1] -> [v2]
>   remove changes to protofile spec
>   ensure backward compatibility
> [v2] -> [v3]
>   use inode_set_[acm]time() as suggested
>   avoid copying atime and ctime
>   they are often problematic for reproducibility, and
>   mtime is the important information to preserve anyway
> [v3] -> [v4]
>   rewrite functionality to populate directly from an input directory
>   this is similar to mkfs.ext4 option.
> [v4] -> [v5]
>   reorder patch to make it easier to review
>   reflow to keep code below 80 chars
>   use _() macro in prints
>   add SPDX headers to new files
>   fix comment styling
>   move from typedef to structs
>   move direntry handling to own function
> [v5] -> [v6]
>   rebase on 6.14

Urrrk, one revision per day, please.

--D

> Luca Di Maio (4):
>   proto: expose more functions from proto
>   populate: add ability to populate a filesystem from a directory
>   mkfs: add -P flag to populate a filesystem from a directory
>   man: document -P flag to populate a filesystem from a directory
> 
>  man/man8/mkfs.xfs.8.in |   7 +
>  mkfs/Makefile          |   2 +-
>  mkfs/populate.c        | 313 +++++++++++++++++++++++++++++++++++++++++
>  mkfs/populate.h        |  10 ++
>  mkfs/proto.c           |  33 ++---
>  mkfs/proto.h           |  22 +++
>  mkfs/xfs_mkfs.c        |  23 ++-
>  7 files changed, 385 insertions(+), 25 deletions(-)
>  create mode 100644 mkfs/populate.c
>  create mode 100644 mkfs/populate.h
> 
> Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
> 
> --
> 2.49.0
>