tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

MODULAR: Add support for load-time options



Hello,

The patch below changes the new modular framework to allow the user to pass arbitrary options to the module at load time. This is done by using a proplib dictionary.

modload is changed to allow setting load-time variables, adding one flag for each supported variable type. (I tried using a single flag and specifying the type as part of its value, but that seemed too confusing.) So modload's syntax is now:

Usage: modload [-b var=boolean] [-f] [-i var=integer] [-s var=string]
       <module_name>

As part of this change, I've also killed MODCTL_FORCELOAD; in my opinion, this made little sense as a separate command. It should just be an option to MODCTL_LOAD. Now there are two ways to better handle that: either pass in a 'force' property in the new dictionary (as the code does), or stick a boolean in the syscall's arguments. I'm yet unsure as to what the best approach can be. On the one hand, keeping it in the dictionary make things simpler and does not treat the 'force' property as anything special. On the other hand, 'force' is something specific to the kernel, and not something the final modules should see, hence it may make more sense to keep it out of the dictionary.

At last, the current code is passing down the dictionary to any modules loaded as dependencies (yeah, I know they don't work at the moment). I'm also unsure if this makes sense. (If it does not, 'force' will need special handling, and this may be another reason to keep it out of the dictionary, as mentioned above.)

Any comments?

Patch below. (It is copy/pasted for easier review; blame my mailer if some lines are incorrectly cut.)

Thanks.

Index: sys/modules/example/example.c
===================================================================
RCS file: /cvsroot/src/sys/modules/example/example.c,v
retrieving revision 1.1
diff -u -p -r1.1 example.c
--- sys/modules/example/example.c       16 Jan 2008 12:34:57 -0000      1.1
+++ sys/modules/example/example.c       9 Feb 2008 08:07:43 -0000
@@ -42,6 +42,16 @@ __KERNEL_RCSID(0, "$NetBSD: example.c,v

 MODULE(MODULE_CLASS_MISC, example, NULL);

+static
+void
+handle_params(prop_dictionary_t params)
+{
+       bool force = prop_bool_true(prop_dictionary_get(params, "force"));
+
+       if (force)
+               printf("Module load was forced.\n");
+}
+
 static int
 example_modcmd(modcmd_t cmd, void *arg)
 {
@@ -49,6 +59,7 @@ example_modcmd(modcmd_t cmd, void *arg)
        switch (cmd) {
        case MODULE_CMD_INIT:
                printf("Example module loaded.\n");
+               handle_params(arg);
                break;

        case MODULE_CMD_FINI:
Index: sys/kern/kern_module.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_module.c,v
retrieving revision 1.8
diff -u -p -r1.8 kern_module.c
--- sys/kern/kern_module.c      19 Jan 2008 18:20:39 -0000      1.8
+++ sys/kern/kern_module.c      9 Feb 2008 08:07:43 -0000
@@ -73,7 +73,8 @@ static modinfo_t module_dummy;
 __link_set_add_rodata(modules, module_dummy);

 static module_t        *module_lookup(const char *);
-static int     module_do_load(const char *, bool, bool, module_t **);
+static int     module_do_load(const char *, bool, prop_dictionary_t,
+                   module_t **);
 static int     module_do_unload(const char *);
 static void    module_error(const char *, ...);
 static int     module_do_builtin(const char *, module_t **);
@@ -162,16 +163,15 @@ module_jettison(void)
 /*
  * module_load:
  *
- *     Load a single module from the file system.  If force is set,
- *     bypass the version check.
+ *     Load a single module from the file system.
  */
 int
-module_load(const char *filename, bool force)
+module_load(const char *filename, prop_dictionary_t params)
 {
        int error;

        mutex_enter(&module_lock);
-       error = module_do_load(filename, false, force, NULL);
+       error = module_do_load(filename, false, params, NULL);
        mutex_exit(&module_lock);

        return error;
@@ -374,7 +374,8 @@ module_do_builtin(const char *name, modu
  *     pushed by the boot loader.
  */
 static int
-module_do_load(const char *filename, bool isdep, bool force, module_t **modp) +module_do_load(const char *filename, bool isdep, prop_dictionary_t params,
+    module_t **modp)
 {
        static TAILQ_HEAD(,module) pending = TAILQ_HEAD_INITIALIZER(pending);
        static int depth;
@@ -389,11 +390,16 @@ module_do_load(const char *filename, boo
        size_t len;
        u_int i;
        bool closed = false;
+       bool force;

        KASSERT(mutex_owned(&module_lock));

        error = 0;

+       /* XXX Should use this to bypass version checks, but currently this
+        * is unused. */
+       force = prop_bool_true(prop_dictionary_get(params, "force"));
+
        /*
         * Avoid recursing too far.
         */
@@ -545,7 +551,7 @@ module_do_load(const char *filename, boo
                                module_error("self-dependency detected");
                                goto fail;
                        }
-                       error = module_do_load(buf, true, force,
+                       error = module_do_load(buf, true, params,
                            &mod->mod_required[mod->mod_nrequired++]);
                        if (error != 0 && error != EEXIST)
                                goto fail;
@@ -555,7 +561,7 @@ module_do_load(const char *filename, boo
        /*
         * We loaded all needed modules successfully: initialize.
         */
-       error = (*mi->mi_modcmd)(MODULE_CMD_INIT, NULL);
+       error = (*mi->mi_modcmd)(MODULE_CMD_INIT, params);
        if (error != 0) {
                module_error("modctl function returned error %d", error);
                goto fail;
Index: sys/kern/sys_module.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_module.c,v
retrieving revision 1.3
diff -u -p -r1.3 sys_module.c
--- sys/kern/sys_module.c       17 Jan 2008 22:30:54 -0000      1.3
+++ sys/kern/sys_module.c       9 Feb 2008 08:07:43 -0000
@@ -52,6 +52,40 @@ __KERNEL_RCSID(0, "$NetBSD: sys_module.c
 #include <sys/syscall.h>
 #include <sys/syscallargs.h>

+static
+int
+handle_modctl_load(void *arg)
+{
+       modctl_load_t *ml = (modctl_load_t *)arg;
+
+       char *path;
+       char params[4096];
+       int error;
+       prop_dictionary_t dict;
+
+       path = PNBUF_GET();
+       error = copyinstr(ml->ml_filename, path, MAXPATHLEN, NULL);
+       if (error != 0)
+               goto out;
+
+       error = copyinstr(ml->ml_params, params, sizeof(params), NULL);
+       if (error != 0)
+               goto out;
+
+       dict = prop_dictionary_internalize(params);
+       if (dict == NULL) {
+               error = EINVAL;
+               goto out;
+       }
+
+       error = module_load(path, dict);
+
+out:
+       PNBUF_PUT(path);
+
+       return error;
+}
+
 int
 sys_modctl(struct lwp *l, const struct sys_modctl_args *uap,
           register_t *retval)
@@ -70,13 +104,11 @@ sys_modctl(struct lwp *l, const struct s
        struct iovec iov;
        int error;
        void *arg;
-       char *path;

        arg = SCARG(uap, arg);

        switch (SCARG(uap, cmd)) {
        case MODCTL_LOAD:
-       case MODCTL_FORCELOAD:
        case MODCTL_UNLOAD:
                /* Authorize. */
                error = kauth_authorize_system(l->l_cred, KAUTH_SYSTEM_MODULE,
@@ -90,15 +122,8 @@ sys_modctl(struct lwp *l, const struct s
        }

        switch (SCARG(uap, cmd)) {
-       case MODCTL_FORCELOAD:
        case MODCTL_LOAD:
-               path = PNBUF_GET();
-               error = copyinstr(arg, path, MAXPATHLEN, NULL);
-               if (error == 0) {
-                       error = module_load(path,
-                           SCARG(uap, cmd) == MODCTL_FORCELOAD);
-               }
-               PNBUF_PUT(path);
+               error = handle_modctl_load(arg);
                break;

        case MODCTL_UNLOAD:
Index: sys/sys/module.h
===================================================================
RCS file: /cvsroot/src/sys/sys/module.h,v
retrieving revision 1.1
diff -u -p -r1.1 module.h
--- sys/sys/module.h    16 Jan 2008 12:34:54 -0000      1.1
+++ sys/sys/module.h    9 Feb 2008 08:07:43 -0000
@@ -92,6 +92,8 @@ typedef struct module {

 #include <sys/mutex.h>

+#include <prop/proplib.h>
+
 /*
* Per-module linkage. Loadable modules have a `link_set_modules' section * containing only one entry, pointing to the module's modinfo_t record.
@@ -121,7 +123,7 @@ void        module_init_class(modclass_t);
 int    module_prime(void *, size_t);
 void   module_jettison(void);

-int    module_load(const char *, bool);
+int    module_load(const char *, prop_dictionary_t);
 int    module_unload(const char *);
 int    module_hold(const char *);
 void   module_rele(const char *);
@@ -132,9 +134,13 @@ void       module_rele(const char *);

 #endif /* _KERNEL */

+typedef struct modctl_load {
+       const char *ml_filename;
+       const char *ml_params;
+} modctl_load_t;
+
 typedef enum modctl {
-       MODCTL_LOAD,            /* char *filename */
-       MODCTL_FORCELOAD,       /* char *filename */
+       MODCTL_LOAD,            /* modctl_load_t *ml */
        MODCTL_UNLOAD,          /* char *name */
        MODCTL_STAT             /* struct iovec *buffer */
 } modctl_t;
Index: sbin/modload/Makefile
===================================================================
RCS file: /cvsroot/src/sbin/modload/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- sbin/modload/Makefile       16 Jan 2008 12:34:58 -0000      1.9
+++ sbin/modload/Makefile       9 Feb 2008 08:07:43 -0000
@@ -22,6 +22,7 @@ CFLAGS+=-DUSE_AOUT
 PROG=  modload
 SRCS=  main.c
 MAN=   modload.8
+LDADD= -lprop

 .include <bsd.prog.mk>

Index: sbin/modload/main.c
===================================================================
RCS file: /cvsroot/src/sbin/modload/main.c,v
retrieving revision 1.1
diff -u -p -r1.1 main.c
--- sbin/modload/main.c 16 Jan 2008 12:34:58 -0000      1.1
+++ sbin/modload/main.c 9 Feb 2008 08:07:43 -0000
@@ -40,28 +40,55 @@ __RCSID("$NetBSD: main.c,v 1.1 2008/01/1

 #include <sys/module.h>

+#include <assert.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <err.h>

+#include <prop/proplib.h>
+
 int    main(int, char **);
+static void    parse_bool_param(prop_dictionary_t, const char *,
+                   const char *);
+static void    parse_int_param(prop_dictionary_t, const char *,
+                   const char *);
+static void    parse_param(prop_dictionary_t, const char *,
+                   void (*)(prop_dictionary_t, const char *, const char *));
+static void    parse_string_param(prop_dictionary_t, const char *,
+                   const char *);
 static void    usage(void) __dead;

 int
 main(int argc, char **argv)
 {
-       modctl_t cmd;
+       modctl_load_t cmdargs;
+       prop_dictionary_t params;
        int ch;

-       cmd = MODCTL_LOAD;
+       params = prop_dictionary_create();

-       while ((ch = getopt(argc, argv, "f")) != -1) {
+       while ((ch = getopt(argc, argv, "b:fi:s:")) != -1) {
                switch (ch) {
+               case 'b':
+                       parse_param(params, optarg, parse_bool_param);
+                       break;
+
                case 'f':
-                       cmd = MODCTL_FORCELOAD;
+                       prop_dictionary_set(params, "force",
+                           prop_bool_create(true));
+                       break;
+
+               case 'i':
+                       parse_param(params, optarg, parse_int_param);
                        break;
+
+               case 's':
+                       parse_param(params, optarg, parse_string_param);
+                       break;
+
                default:
                        usage();
                        /* NOTREACHED */
@@ -73,17 +100,98 @@ main(int argc, char **argv)
        if (argc != 1)
                usage();

-       if (modctl(cmd, argv[0])) {
+       cmdargs.ml_filename = argv[0];
+       cmdargs.ml_params = prop_dictionary_externalize(params);
+
+       if (modctl(MODCTL_LOAD, &cmdargs)) {
                err(EXIT_FAILURE, NULL);
        }

        exit(EXIT_SUCCESS);
 }

+static
+void
+parse_bool_param(prop_dictionary_t params, const char *name,
+    const char *value)
+{
+       bool boolvalue;
+
+       assert(name != NULL);
+       assert(value != NULL);
+
+       if (strcasecmp(value, "1") == 0 ||
+           strcasecmp(value, "true") == 0 ||
+           strcasecmp(value, "yes") == 0)
+               boolvalue = true;
+       else if (strcasecmp(value, "0") == 0 ||
+           strcasecmp(value, "false") == 0 ||
+           strcasecmp(value, "no") == 0)
+               boolvalue = false;
+       else
+               errx(EXIT_FAILURE, "Invalud boolean value `%s'", value);
+
+       prop_dictionary_set(params, name, prop_bool_create(boolvalue));
+}
+
+static
+void
+parse_int_param(prop_dictionary_t params, const char *name,
+    const char *value)
+{
+       int64_t intvalue;
+
+       assert(name != NULL);
+       assert(value != NULL);
+
+       if (dehumanize_number(value, &intvalue) != 0)
+               err(EXIT_FAILURE, "Invalid integer value `%s'", value);
+
+       prop_dictionary_set(params, name,
+           prop_number_create_integer(intvalue));
+}
+
+static
+void
+parse_param(prop_dictionary_t params, const char *origstr,
+    void (*fmt_handler)(prop_dictionary_t, const char *, const char *))
+{
+       char *name, *value;
+
+       name = strdup(origstr);
+
+       value = strchr(name, '=');
+       if (value == NULL) {
+               free(name);
+               errx(EXIT_FAILURE, "Invalid parameter `%s'", origstr);
+       }
+       *value++ = '\0';
+
+       fmt_handler(params, name, value);
+
+       free(name);
+}
+
+static
+void
+parse_string_param(prop_dictionary_t params, const char *name,
+    const char *value)
+{
+
+       assert(name != NULL);
+       assert(value != NULL);
+
+       prop_dictionary_set(params, name, prop_string_create_cstring(value));
+}
+
 static void
 usage(void)
 {

- (void)fprintf(stderr, "Usage: %s [-f] <module_name>\n", getprogname ());
+       (void)fprintf(stderr,
+           "Usage: %s [-b var=boolean] [-f] [-i var=integer] "
+           "[-s var=string]\n"
+           "       <module_name>\n",
+           getprogname());
        exit(EXIT_FAILURE);
 }




Home | Main Index | Thread Index | Old Index