On Tue, Apr 16, 2013 at 1:20 AM, Lucas De Marchi <lucas.demarchi@xxxxxxxxxxxxxx> wrote: > Hi Tom, Hi Lucas, Thanks for your review! > On Fri, Apr 12, 2013 at 9:15 PM, Tom Gundersen <teg@xxxxxxx> wrote: >> This tool reads modules.devname from the current kernel directory and outputs >> the information. >> >> For now only the tmpfiles.d(5) format is supported, but more could easily be >> added in the future if there is a need. >> >> When booting with systemd, the new tool is called at boot to instruct >> systemd-tmpfiles to create the necessary static modules before starting >> systemd-udevd. >> >> This means nothing but kmod needs to reads the private files under /lib/modules/. > > For me this would be the main goal of this patch or anything similar to this. Yeah, I agree. >> Cc: <linux-hotplug@xxxxxxxxxxxxxxx> >> Cc: <systemd-devel@xxxxxxxxxxxxxxxxxxxxx> >> --- >> >> v2: adressed concerns raised by Dave, and made the tool a bit more generic so >> more output formats may be added in the future as suggested by Lucas. Also >> included the systemd unit file to hook this up with systemd. >> >> Makefile.am | 20 ++++- >> configure.ac | 8 ++ >> tools/kmod-static-nodes.service.in | 16 ++++ >> tools/kmod.c | 1 + >> tools/kmod.h | 1 + >> tools/static-nodes.c | 163 +++++++++++++++++++++++++++++++++++++ >> 6 files changed, 207 insertions(+), 2 deletions(-) >> create mode 100644 tools/kmod-static-nodes.service.in >> create mode 100644 tools/static-nodes.c >> >> diff --git a/Makefile.am b/Makefile.am >> index 9feaf96..333e861 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -36,6 +36,9 @@ SED_PROCESS = \ >> %.pc: %.pc.in Makefile >> $(SED_PROCESS) >> >> +%.service: %.service.in Makefile >> + $(SED_PROCESS) >> + >> LIBKMOD_CURRENT=4 >> LIBKMOD_REVISION=2 >> LIBKMOD_AGE=2 >> @@ -88,6 +91,17 @@ pkgconfig_DATA = libkmod/libkmod.pc >> EXTRA_DIST += libkmod/libkmod.pc.in >> CLEANFILES += libkmod/libkmod.pc >> >> +if HAVE_SYSTEMD >> +systemdsystemunit_DATA = tools/kmod-static-nodes.service >> +EXTRA_DIST += tools/kmod-static-nodes.service.in >> +CLEANFILES += tools/kmod-static-nodes.service >> + >> +install-data-hook: >> + $(MKDIR_P) $(DESTDIR)$(systemdsystemunitdir)/sysinit.target.wants >> + ln -sf ../kmod-static-nodes.service \ >> + $(DESTDIR)$(systemdsystemunitdir)/sysinit.target.wants/kmod-static-nodes.service >> +endif >> + >> install-exec-hook: >> if test "$(libdir)" != "$(rootlibdir)"; then \ >> $(MKDIR_P) $(DESTDIR)$(rootlibdir) && \ >> @@ -109,7 +123,8 @@ noinst_SCRIPTS = tools/insmod tools/rmmod tools/lsmod \ >> tools_kmod_SOURCES = tools/kmod.c tools/kmod.h tools/lsmod.c \ >> tools/rmmod.c tools/insmod.c \ >> tools/modinfo.c tools/modprobe.c \ >> - tools/depmod.c tools/log.h tools/log.c >> + tools/depmod.c tools/log.h tools/log.c \ >> + tools/static-nodes.c >> tools_kmod_LDADD = libkmod/libkmod-util.la \ >> libkmod/libkmod.la >> >> @@ -211,7 +226,8 @@ testsuite-distclean: >> DISTCLEAN_LOCAL_HOOKS += testsuite-distclean >> EXTRA_DIST += testsuite/rootfs-pristine >> >> -DISTCHECK_CONFIGURE_FLAGS=--enable-gtk-doc --sysconfdir=/etc --with-zlib >> +DISTCHECK_CONFIGURE_FLAGS=--enable-gtk-doc --sysconfdir=/etc --with-zlib \ >> + --with-systemdsystemunitdir=$$dc_install_base/$(systemdsystemunitdir) >> >> distclean-local: $(DISTCLEAN_LOCAL_HOOKS) >> >> diff --git a/configure.ac b/configure.ac >> index 566b317..af5ed52 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -76,6 +76,13 @@ AS_IF([test "x$with_zlib" != "xno"], [ >> AC_MSG_NOTICE([zlib support not requested]) >> ]) >> >> +AC_ARG_WITH([systemdsystemunitdir], >> + AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]), >> + [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)]) >> +if test "x$with_systemdsystemunitdir" != xno; then >> + AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir]) >> +fi >> +AM_CONDITIONAL(HAVE_SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ]) >> >> ##################################################################### >> # --enable- >> @@ -200,6 +207,7 @@ AC_MSG_RESULT([ >> compiler: ${CC} >> cflags: ${with_cflags} ${CFLAGS} >> ldflags: ${with_ldflags} ${LDFLAGS} >> + systemdsystemunitdir: ${with_systemdsystemunitdir} >> >> tools: ${enable_tools} >> logging: ${enable_logging} >> diff --git a/tools/kmod-static-nodes.service.in b/tools/kmod-static-nodes.service.in >> new file mode 100644 >> index 0000000..be8482e >> --- /dev/null >> +++ b/tools/kmod-static-nodes.service.in > > I'm not sure we want the service file to live in kmod repository. > Maybe we could let this in systemd and in kmod we only add the support > to create this type of information? I don't really mind. For now I'll drop it, and we can agree on where to put it later. >> @@ -0,0 +1,16 @@ >> +# This file is part of kmod. >> +# >> +# kmod is free software; you can redistribute it and/or modify it >> +# under the terms of the GNU Lesser General Public License as published by >> +# the Free Software Foundation; either version 2.1 of the License, or >> +# (at your option) any later version. >> + >> +[Unit] >> +Description=Create list of static nodes to be created in /dev >> +DefaultDependencies=no >> +Before=sysinit.target systemd-static-nodes.service >> + >> +[Service] >> +Type=oneshot >> +ExecStart=/bin/mkdir -p /run/tmpfiles.d >> +ExecStart=@prefix@/bin/kmod static-nodes tmpfiles --output=/run/tmpfiles.d/kmod.conf > > The syntax is a bit cumbersome too, but since we still don't have any > command (except from the test-only "list") I think we could let this > as is and change later, before a new release. > > IMO it should be kmod static-nodes --format=... > > It's ok to implement only the tmpfiles output, though the default > should be an human readable format, just like git commands are. Thanks for the suggestions, makes sense. I resent with these changes. >> diff --git a/tools/kmod.c b/tools/kmod.c >> index ebb8875..347bb7d 100644 >> --- a/tools/kmod.c >> +++ b/tools/kmod.c >> @@ -37,6 +37,7 @@ static const struct kmod_cmd kmod_cmd_help; >> static const struct kmod_cmd *kmod_cmds[] = { >> &kmod_cmd_help, >> &kmod_cmd_list, >> + &kmod_cmd_static_nodes, >> }; >> >> static const struct kmod_cmd *kmod_compat_cmds[] = { >> diff --git a/tools/kmod.h b/tools/kmod.h >> index 80fa4c2..68a646a 100644 >> --- a/tools/kmod.h >> +++ b/tools/kmod.h >> @@ -35,5 +35,6 @@ extern const struct kmod_cmd kmod_cmd_compat_modprobe; >> extern const struct kmod_cmd kmod_cmd_compat_depmod; >> >> extern const struct kmod_cmd kmod_cmd_list; >> +extern const struct kmod_cmd kmod_cmd_static_nodes; >> >> #include "log.h" >> diff --git a/tools/static-nodes.c b/tools/static-nodes.c >> new file mode 100644 >> index 0000000..a79fc3d >> --- /dev/null >> +++ b/tools/static-nodes.c >> @@ -0,0 +1,163 @@ >> +/* >> + * kmod-static-nodes - manage modules.devname >> + * >> + * Copyright (C) 2004-2012 Kay Sievers <kay@xxxxxxxx> >> + * Copyright (C) 2011-2013 ProFUSION embedded systems >> + * Copyright (C) 2013 Tom Gundersen <teg@xxxxxxx> >> + * >> + * This program is free software: you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation, either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <stddef.h> >> +#include <getopt.h> >> +#include <errno.h> >> +#include <unistd.h> >> +#include <string.h> >> +#include <limits.h> >> +#include <sys/utsname.h> >> +#include <sys/stat.h> >> +#include <sys/types.h> >> +#include "libkmod-util.h" >> + >> +#include "kmod.h" >> + >> +static const char cmdopts_s[] = "o:h"; >> +static const struct option cmdopts[] = { >> + { "output", required_argument, 0, 'o'}, >> + { "help", no_argument, 0, 'h'}, >> + { }, >> +}; >> + >> +static void help(void) >> +{ >> + printf("Usage:\n" >> + "\t%s static-nodes format [options]\n" >> + "\n" >> + "kmod static-nodes outputs the static-node information of the currently running kernel.\n" >> + "\n" >> + "Options:\n" >> + "\t-o, --output=FILE write output to file\n" >> + "\t-h, --help show this help\n" >> + "\n" >> + "Formats:\n" >> + " tmpfiles the tmpfiles.d(5) used by systemd-tmpfiles.\n", >> + program_invocation_short_name); >> +} >> + >> +static int write_tmpfile(FILE *in, FILE *out) { >> + char buf[4096]; >> + int ret = EXIT_SUCCESS; >> + >> + while (fgets(buf, sizeof(buf), in) != NULL) { >> + char devname[PATH_MAX]; >> + char type; >> + unsigned int maj, min; >> + int matches; >> + >> + if (buf[0] == '#') >> + continue; >> + >> + matches = sscanf(buf, "%*s %s %c%u:%u", devname, &type, &maj, &min); >> + if (matches != 4 || (type != 'c' && type != 'b')) { >> + fprintf(stderr, "Error: invalid devname entry: %s", buf); >> + ret = EXIT_FAILURE; >> + continue; >> + } >> + >> + fprintf(out, "%c /dev/%s 0600 - - - %u:%u\n", type, devname, maj, min); >> + } >> + >> + return ret; >> +} >> + >> +static int do_static_nodes(int argc, char *argv[]) >> +{ >> + struct utsname kernel; >> + char modules[PATH_MAX]; >> + FILE *in = NULL, *out = stdout; >> + int ret = EXIT_SUCCESS; >> + >> + for (;;) { >> + int c, idx = 0; >> + >> + c = getopt_long(argc - 1, argv + 1, cmdopts_s, cmdopts, &idx); >> + if (c == -1) { >> + break; >> + } >> + switch (c) { >> + case 'o': >> + out = fopen(optarg, "we"); >> + if (out == NULL) { >> + fprintf(stderr, "Error: could not create %s!\n", optarg); >> + ret = EXIT_FAILURE; >> + goto finish; >> + } >> + break; >> + case 'h': >> + help(); >> + goto finish; >> + case '?': >> + ret = EXIT_FAILURE; >> + goto finish; >> + default: >> + fprintf(stderr, "Unexpected commandline option '%c'.\n", c); >> + help(); >> + ret = EXIT_FAILURE; >> + goto finish; >> + } >> + } >> + >> + if (argc < 2) { >> + help(); >> + ret = EXIT_FAILURE; >> + goto finish; >> + } >> + >> + if (!streq(argv[1], "tmpfiles")) { >> + fprintf(stderr, "Unknown format: '%s'.\n", argv[1]); >> + help(); >> + ret = EXIT_FAILURE; >> + goto finish; >> + } >> + >> + if (uname(&kernel) < 0) { >> + fputs("Error: uname failed!\n", stderr); >> + ret = EXIT_FAILURE; >> + goto finish; >> + } >> + snprintf(modules, sizeof(modules), "/lib/modules/%s/modules.devname", kernel.release); >> + in = fopen(modules, "re"); >> + if (in == NULL && errno != ENOENT) { >> + fprintf(stderr, "Error: could not open /lib/modules/%s/modules.devname!\n", kernel.release); >> + ret = EXIT_FAILURE; >> + goto finish; >> + } >> + >> + ret = write_tmpfile(in, out); >> + >> +finish: >> + if (in) >> + fclose(in); >> + if (out) >> + fclose(out); >> + return ret; >> +} >> + >> +const struct kmod_cmd kmod_cmd_static_nodes = { >> + .name = "static-nodes", >> + .cmd = do_static_nodes, >> + .help = "outputs the static-node information of the currently running kernel", >> +}; >> -- > > > Lucas De Marchi -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html