| Message ID | 20230130183324.283162-1-emil.l.velikov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | depmod: Introduce a outdir option | expand |
On Mon, Jan 30, 2023 at 06:33:24PM +0000, Emil Velikov wrote: >From: Emil Velikov <emil.velikov@collabora.com> > >This option is equivalent to basedir, with the small difference being >that's where the meta-data files are generated. In other words, this >allows us to have read-only input modules and modules.dep, while still >being able to generate the meta-data files. > >Signed-off-by: Emil Velikov <emil.velikov@collabora.com> >--- > >Hello team, > >Here's a handy feature behind the request at >https://github.com/kmod-project/kmod/issues/13 > >This is my first time hacking on kmod, so hope I didn't make too many >mistakes :-) There are a few TODO notes below where your input is >greatly appreciated. > >TODO: > - Add tests - team, any pointers or requests? yep, please add one that calls depmod with this new option. Copy and extend one of the tests in testsuite/test-depmod.c You will have to prepare a "rootfs" under testsuite/rootfs-pristine/test-depmod/<name-of-the-test>. Make sure to add the normal files coming from kernel and checking for the output in the other directory you passed as argument. > - Split the dirnamelen shorthand tenary operator change - is it worth > it? >--- > man/depmod.xml | 19 +++++++++++++++++++ > tools/depmod.c | 25 ++++++++++++++++++++++--- > 2 files changed, 41 insertions(+), 3 deletions(-) > >diff --git a/man/depmod.xml b/man/depmod.xml >index ea0be27..c53624d 100644 >--- a/man/depmod.xml >+++ b/man/depmod.xml >@@ -45,6 +45,7 @@ > <cmdsynopsis> > <command>depmod</command> > <arg><option>-b <replaceable>basedir</replaceable></option></arg> >+ <arg><option>-o <replaceable>outdir</replaceable></option></arg> > <arg><option>-e</option></arg> > <arg><option>-E <replaceable>Module.symvers</replaceable></option></arg> > <arg><option>-F <replaceable>System.map</replaceable></option></arg> >@@ -151,6 +152,24 @@ > </para> > </listitem> > </varlistentry> >+ <varlistentry> >+ <term> >+ <option>-o <replaceable>outdir</replaceable></option> >+ </term> >+ <term> >+ <option>--outdir <replaceable>outdir</replaceable></option> >+ </term> >+ <listitem> >+ <para> >+ If your modules are stored in a read-only location, you may want to >+ create the output meta-data files in another location. Setting that is probably not the only use case, I'd rephrase with something like: Set the output directory where depmod will store any generated file. <replaceable>outdir</replaceable> serves as a root to that location, similar to how <replaceable>basedir</replaceable> is used. Also this setting takes precedence and if used together with <replaceable>basedir</replaceable> it will result in the input being that directory, but the output being the one set by <replaceable>outdir</replaceable>. >+ <replaceable>outdir</replaceable> serves as a root to that location >+ similar to how we use <replaceable>basedir</replaceable>. Use this >+ option if you are a distribution vendor who needs to pre-generate >+ the meta-data files rather than running depmod again later. >+ </para> >+ </listitem> >+ </varlistentry> > <varlistentry> > <term> > <option>-C</option> >diff --git a/tools/depmod.c b/tools/depmod.c >index 364b7d4..aaf2327 100644 >--- a/tools/depmod.c >+++ b/tools/depmod.c >@@ -58,11 +58,12 @@ static const char *default_cfg_paths[] = { > NULL > }; > >-static const char cmdopts_s[] = "aAb:C:E:F:euqrvnP:wmVh"; >+static const char cmdopts_s[] = "aAb:o:C:E:F:euqrvnP:wmVh"; > static const struct option cmdopts[] = { > { "all", no_argument, 0, 'a' }, > { "quick", no_argument, 0, 'A' }, > { "basedir", required_argument, 0, 'b' }, >+ { "outdir", required_argument, 0, 'o' }, > { "config", required_argument, 0, 'C' }, > { "symvers", required_argument, 0, 'E' }, > { "filesyms", required_argument, 0, 'F' }, >@@ -104,6 +105,7 @@ static void help(void) > "\n" > "The following options are useful for people managing distributions:\n" > "\t-b, --basedir=DIR Use an image of a module tree.\n" >+ "\t-o, --outdir=DIR The output equivalent of basedir.\n" Output directory for generated files <basedir> is both input and output, so I don't think the comparison is good enough. I think making sure this is working as desired with at least one test would be good, but overall looks goot to me. thanks Lucas De Marchi > "\t-F, --filesyms=FILE Use the file instead of the\n" > "\t current kernel symbols.\n" > "\t-E, --symvers=FILE Use Module.symvers file to check\n" >@@ -467,6 +469,8 @@ struct cfg { > const char *kversion; > char dirname[PATH_MAX]; > size_t dirnamelen; >+ char outdirname[PATH_MAX]; >+ size_t outdirnamelen; > char sym_prefix; > uint8_t check_symvers; > uint8_t print_unknown; >@@ -2576,7 +2580,7 @@ static int depmod_output(struct depmod *depmod, FILE *out) > { "modules.devname", output_devname }, > { } > }; >- const char *dname = depmod->cfg->dirname; >+ const char *dname = depmod->cfg->outdirname; > int dfd, err = 0; > struct timeval tv; > >@@ -2585,6 +2589,11 @@ static int depmod_output(struct depmod *depmod, FILE *out) > if (out != NULL) > dfd = -1; > else { >+ err = mkdir_p(dname, strlen(dname), 0755); >+ if (err < 0) { >+ CRIT("could not create directory %s: %m\n", dname); >+ return err; >+ } > dfd = open(dname, O_RDONLY); > if (dfd < 0) { > err = -errno; >@@ -2898,6 +2907,7 @@ static int do_depmod(int argc, char *argv[]) > FILE *out = NULL; > int err = 0, all = 0, maybe_all = 0, n_config_paths = 0; > _cleanup_free_ char *root = NULL; >+ _cleanup_free_ char *out_root = NULL; > _cleanup_free_ const char **config_paths = NULL; > const char *system_map = NULL; > const char *module_symvers = NULL; >@@ -2927,6 +2937,11 @@ static int do_depmod(int argc, char *argv[]) > free(root); > root = path_make_absolute_cwd(optarg); > break; >+ case 'o': >+ if (out_root) >+ free(out_root); >+ out_root = path_make_absolute_cwd(optarg); >+ break; > case 'C': { > size_t bytes = sizeof(char *) * (n_config_paths + 2); > void *tmp = realloc(config_paths, bytes); >@@ -3009,7 +3024,11 @@ static int do_depmod(int argc, char *argv[]) > > cfg.dirnamelen = snprintf(cfg.dirname, PATH_MAX, > "%s/lib/modules/%s", >- root == NULL ? "" : root, cfg.kversion); >+ root ?: "", cfg.kversion); >+ >+ cfg.outdirnamelen = snprintf(cfg.outdirname, PATH_MAX, >+ "%s/lib/modules/%s", >+ out_root ?: (root ?: ""), cfg.kversion); > > if (optind == argc) > all = 1; >-- >2.39.1 >
diff --git a/man/depmod.xml b/man/depmod.xml index ea0be27..c53624d 100644 --- a/man/depmod.xml +++ b/man/depmod.xml @@ -45,6 +45,7 @@ <cmdsynopsis> <command>depmod</command> <arg><option>-b <replaceable>basedir</replaceable></option></arg> + <arg><option>-o <replaceable>outdir</replaceable></option></arg> <arg><option>-e</option></arg> <arg><option>-E <replaceable>Module.symvers</replaceable></option></arg> <arg><option>-F <replaceable>System.map</replaceable></option></arg> @@ -151,6 +152,24 @@ </para> </listitem> </varlistentry> + <varlistentry> + <term> + <option>-o <replaceable>outdir</replaceable></option> + </term> + <term> + <option>--outdir <replaceable>outdir</replaceable></option> + </term> + <listitem> + <para> + If your modules are stored in a read-only location, you may want to + create the output meta-data files in another location. Setting + <replaceable>outdir</replaceable> serves as a root to that location + similar to how we use <replaceable>basedir</replaceable>. Use this + option if you are a distribution vendor who needs to pre-generate + the meta-data files rather than running depmod again later. + </para> + </listitem> + </varlistentry> <varlistentry> <term> <option>-C</option> diff --git a/tools/depmod.c b/tools/depmod.c index 364b7d4..aaf2327 100644 --- a/tools/depmod.c +++ b/tools/depmod.c @@ -58,11 +58,12 @@ static const char *default_cfg_paths[] = { NULL }; -static const char cmdopts_s[] = "aAb:C:E:F:euqrvnP:wmVh"; +static const char cmdopts_s[] = "aAb:o:C:E:F:euqrvnP:wmVh"; static const struct option cmdopts[] = { { "all", no_argument, 0, 'a' }, { "quick", no_argument, 0, 'A' }, { "basedir", required_argument, 0, 'b' }, + { "outdir", required_argument, 0, 'o' }, { "config", required_argument, 0, 'C' }, { "symvers", required_argument, 0, 'E' }, { "filesyms", required_argument, 0, 'F' }, @@ -104,6 +105,7 @@ static void help(void) "\n" "The following options are useful for people managing distributions:\n" "\t-b, --basedir=DIR Use an image of a module tree.\n" + "\t-o, --outdir=DIR The output equivalent of basedir.\n" "\t-F, --filesyms=FILE Use the file instead of the\n" "\t current kernel symbols.\n" "\t-E, --symvers=FILE Use Module.symvers file to check\n" @@ -467,6 +469,8 @@ struct cfg { const char *kversion; char dirname[PATH_MAX]; size_t dirnamelen; + char outdirname[PATH_MAX]; + size_t outdirnamelen; char sym_prefix; uint8_t check_symvers; uint8_t print_unknown; @@ -2576,7 +2580,7 @@ static int depmod_output(struct depmod *depmod, FILE *out) { "modules.devname", output_devname }, { } }; - const char *dname = depmod->cfg->dirname; + const char *dname = depmod->cfg->outdirname; int dfd, err = 0; struct timeval tv; @@ -2585,6 +2589,11 @@ static int depmod_output(struct depmod *depmod, FILE *out) if (out != NULL) dfd = -1; else { + err = mkdir_p(dname, strlen(dname), 0755); + if (err < 0) { + CRIT("could not create directory %s: %m\n", dname); + return err; + } dfd = open(dname, O_RDONLY); if (dfd < 0) { err = -errno; @@ -2898,6 +2907,7 @@ static int do_depmod(int argc, char *argv[]) FILE *out = NULL; int err = 0, all = 0, maybe_all = 0, n_config_paths = 0; _cleanup_free_ char *root = NULL; + _cleanup_free_ char *out_root = NULL; _cleanup_free_ const char **config_paths = NULL; const char *system_map = NULL; const char *module_symvers = NULL; @@ -2927,6 +2937,11 @@ static int do_depmod(int argc, char *argv[]) free(root); root = path_make_absolute_cwd(optarg); break; + case 'o': + if (out_root) + free(out_root); + out_root = path_make_absolute_cwd(optarg); + break; case 'C': { size_t bytes = sizeof(char *) * (n_config_paths + 2); void *tmp = realloc(config_paths, bytes); @@ -3009,7 +3024,11 @@ static int do_depmod(int argc, char *argv[]) cfg.dirnamelen = snprintf(cfg.dirname, PATH_MAX, "%s/lib/modules/%s", - root == NULL ? "" : root, cfg.kversion); + root ?: "", cfg.kversion); + + cfg.outdirnamelen = snprintf(cfg.outdirname, PATH_MAX, + "%s/lib/modules/%s", + out_root ?: (root ?: ""), cfg.kversion); if (optind == argc) all = 1;