Re: [PATCH] modprobe: add --resolve-alias option

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

 



Alan Jenkins schrieb:
On 8/26/09, Thomas Bächler <thomas@xxxxxxxxxxxxx> wrote:
The --resolve-alias option prints all module names that match the
alias given on the commandline and exits. If no valid alias is specified,
it does nothing.

Why?

I.e. we already have "--verbose --dry-run" for humans, so I assume
this is for scripting purposes.  Which scripts would you like to use
this in?

- modprobe --show-depends

Will resolve dependencies, I only want the exact module names for the aliases.

- modprobe --verbose --dry-run

Will only show names for modules that are not already loaded. I want all names, regardless of whether they are loaded or not (now that I think about it, I want that for testing purposes, but in the real world it wouldn't matter). Like the above command, this one will resolve dependencies as well.

Both also show "insmod" and the full path, but you are right that some cut/basename magic will solve that. It still seems pointless to have modprobe resolve the path first and then have a script throw it away again.

I'm using this for an advanced blacklisting mechanism, that works like this:
- Resolve all module names for an alias
- For each of those module names:
  - Resolve dependencies
  - Match the module and all of its dependencies against a blacklist
  - If it doesn't match, load it

The commands mentioned above will provide me with a mix of dependencies for all module names, instead of a separate list of dependencies for each of them.

Besides this application, it would be really convenient in daily life to be able to just know "what the heck is this alias for?". It seems like an obvious feature to me.

@@ -1342,20 +1343,23 @@ int do_modprobe(char *modname,
 		if (aliases->next)
 			err = warn;
 		while (aliases) {
-			/* Add the options for this alias. */
-			char *opts = NOFAIL(strdup(cmdline_opts));
-			opts = add_extra_options(modname,
-						 opts, modoptions);
-
-			read_depends(dirname, aliases->module, &list);
-			failed |= handle_module(aliases->module,
-				&list, newname, opts, modoptions,
-				commands, cmdline_opts, err, flags);
-
+			if(flags & mit_resolve_alias) {
+				info("%s\n", aliases->module);

I think printf() would be better.

I just tried to match whatever you used in other places in modprobe, if you say I should use printf, then I have no objections.

+			} else {
+				/* Add the options for this alias. */
+				char *opts = NOFAIL(strdup(cmdline_opts));
+				opts = add_extra_options(modname,
+							 opts, modoptions);
+
+				read_depends(dirname, aliases->module, &list);
+				failed |= handle_module(aliases->module,
+					&list, newname, opts, modoptions,
+					commands, cmdline_opts, err, flags);
+				INIT_LIST_HEAD(&list);
+			}
 			aliases = aliases->next;
-			INIT_LIST_HEAD(&list);
 		}
-	} else {
+	} else if(!(flags & mit_resolve_alias)) {
 		if (flags & mit_use_blacklist
 		    && find_blacklist(modname, blacklist))
 			return failed;

Yay, more special cases and higher nesting levels :-).

How about this instead?

 	aliases = apply_blacklist(aliases, blacklist);
+	if(flags & mit_resolve_alias) {
+		while(aliases) {
+			printf("%s\n", aliases->module);
+			aliases = aliases->next;
+		}
+		return 0;
+	}

Looks better, yes, also makes the diff cleaner.

It could be even shorter if you don't mind switching the while() loops
to for() loops as well.

Like this?

for(; aliases; aliases=aliases->next)
  printf("%s\n", aliases->module);
return 0;

Regards
Alan

Should I recreate the patch with the above changes, or are you going to reject it anyway?

Best regards
Thomas

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux