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-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
