[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] modprobe: implement -d DIR flag like the modprobe in kmod
From:       tito <farmatito () tiscali ! it>
Date:       2024-04-25 7:33:09
Message-ID: 20240425093309.424f74dd () devuan
[Download RAW message or body]

On Thu, 25 Apr 2024 09:13:09 +0200
tito <farmatito@tiscali.it> wrote:

> On Wed, 24 Apr 2024 21:22:28 -0700
> Adam Joseph <adam@westernsemico.com> wrote:
> 
> > This patch implements support for the `-d DIR` flag, like kmod has.  When
> > provided, `DIR` is a directory *beneath which* modprobe will expect to find
> > $CONFIG_DEFAULT_MODULES_DIR/$(uname -r) (e.g. lib/modules/$(uname -r)).
> > 
> > This is required on nixpkgs-based systems, which do not normally have a
> > /lib/modules directory.  Instead, a symbolic link at some ephemeral location
> > in /run points to the proper location.  For example, with mdevd instead of
> > systemd and the following in mdev.conf:
> > 
> > mdevd-conf = ''
> > ...
> > $MODALIAS=.* root:root 660 @${pkgs.busybox}/bin/modprobe -d \
> >                 /run/booted-system/kernel-modules -b "$MODALIAS"
> > ...
> > '';
> > 
> > Compared to setting CONFIG_DEFAULT_MODULES_DIR, a command line flag is
> > preferable since it allows the same busybox binary to be used both in early
> > userspace (where the initrd layout often follows FHS and uses /lib/modules) as
> > well as after switch_root.
> > ---
> > modutils/modprobe.c | 24 +++++++++++++++++-------
> > 1 file changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/modutils/modprobe.c b/modutils/modprobe.c
> > index 543f53e99..d3690ebc1 100644
> > --- a/modutils/modprobe.c
> > +++ b/modutils/modprobe.c
> > @@ -114,6 +114,7 @@
> > //usage:	" MODULE" IF_FEATURE_CMDLINE_MODULE_OPTIONS(" [SYMBOL=VALUE]...")
> > //usage:#define modprobe_full_usage "\n\n"
> > //usage:       "	-a	Load multiple MODULEs"
> > +//usage:     "\n	-d DIR	Use DIR as filesystem root"
> > //usage:     "\n	-l	List (MODULE is a pattern)"
> > //usage:     "\n	-r	Remove MODULE (stacks) or do autoclean"
> > //usage:     "\n	-q	Quiet"
> > @@ -130,7 +131,7 @@
> > * Note2: -b is always accepted, but if !FEATURE_MODPROBE_BLACKLIST,
> > * it is a no-op.
> > */
> > -#define MODPROBE_OPTS  "alrDb"
> > +#define MODPROBE_OPTS  "ad:lrDb"
> > /* -a and -D _are_ in fact compatible */
> > #define MODPROBE_COMPLEMENTARY "q-v:v-q:l--arD:r--alD:a--lr:D--rl"
> > //#define MODPROBE_OPTS  "acd:lnrt:C:b"
> > @@ -138,20 +139,21 @@
> > enum {
> > 	OPT_INSERT_ALL   = (INSMOD_OPT_UNUSED << 0), /* a */
> > 	//OPT_DUMP_ONLY  = (INSMOD_OPT_UNUSED << x), /* c */
> > -	//OPT_DIRNAME    = (INSMOD_OPT_UNUSED << x), /* d */
> > -	OPT_LIST_ONLY    = (INSMOD_OPT_UNUSED << 1), /* l */
> > +	OPT_DIRNAME      = (INSMOD_OPT_UNUSED << 1), /* d */
> > +	OPT_LIST_ONLY    = (INSMOD_OPT_UNUSED << 2), /* l */
> > 	//OPT_SHOW_ONLY  = (INSMOD_OPT_UNUSED << x), /* n */
> > -	OPT_REMOVE       = (INSMOD_OPT_UNUSED << 2), /* r */
> > +	OPT_REMOVE       = (INSMOD_OPT_UNUSED << 3), /* r */
> > 	//OPT_RESTRICT   = (INSMOD_OPT_UNUSED << x), /* t */
> > 	//OPT_VERONLY    = (INSMOD_OPT_UNUSED << x), /* V */
> > 	//OPT_CONFIGFILE = (INSMOD_OPT_UNUSED << x), /* C */
> > -	OPT_SHOW_DEPS    = (INSMOD_OPT_UNUSED << 3), /* D */
> > -	OPT_BLACKLIST    = (INSMOD_OPT_UNUSED << 4) * \
> > ENABLE_FEATURE_MODPROBE_BLACKLIST, +	OPT_SHOW_DEPS    = (INSMOD_OPT_UNUSED << 4), \
> > /* D */ +	OPT_BLACKLIST    = (INSMOD_OPT_UNUSED << 5) * \
> > ENABLE_FEATURE_MODPROBE_BLACKLIST, };
> > #if ENABLE_LONG_OPTS
> > static const char modprobe_longopts[] ALIGN1 =
> > 	/* nobody asked for long opts (yet) */
> > 	// "all\0"          No_argument "a"
> > +	// "dirname\0"      required_argument "d"
> > 	// "list\0"         No_argument "l"
> > 	// "remove\0"       No_argument "r"
> > 	// "quiet\0"        No_argument "q"
> > @@ -559,17 +561,25 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
> > 	int rc;
> > 	unsigned opt;
> > 	struct module_entry *me;
> > +	const char* dirname;
> > 
> > 	INIT_G();
> > 
> > 	opt = getopt32long(argv, "^" INSMOD_OPTS MODPROBE_OPTS "\0" \
> > MODPROBE_COMPLEMENTARY,  modprobe_longopts
> > 			INSMOD_ARGS
> > +			, &dirname
> > 	);
> > 	argv += optind;
> > 
> > 	/* Goto modules location */
> > -	xchdir(CONFIG_DEFAULT_MODULES_DIR);
> > +	if (opt & OPT_DIRNAME) {
> Hi,
> just out of curiosity what are you trying to achieve here?
> To which directory you want to change? dirname or  &CONFIG_DEFAULT_MODULES_DIR[1]?
> and why not xchdir(CONFIG_DEFAULT_MODULES_DIR) like later on in the code?
> Also I would like to point out that if the first xchdir call fails the program will \
> exit and the second will never be executed.
> 
> Ciao,
> Tito

Hi again, I see now what this is about something like:

if (opt & OPT_DIRNAME) {
	xchdir(dirname);
	/* remove first / */
	CONFIG_DEFAULT_MODULES_DIR+=1;
}
xchdir(CONFIG_DEFAULT_MODULES_DIR);

This spares a xchdir call and from my point of view
makes it easier to understand.

Sorry for the noise.

Ciao,
Tito


> 
> > +		xchdir(dirname);
> > +		xchdir(&CONFIG_DEFAULT_MODULES_DIR[1]);
> 
> 
> > +	} else {
> > +		xchdir(CONFIG_DEFAULT_MODULES_DIR);
> > +	}
> > +
> > 	uname(&G.uts);
> > 	xchdir(G.uts.release);
> > 
> 
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]