Hello Lennart,
Lennart Poettering [2014-10-27 16:09 +0100]:
> > +static const char hwdb_bin_paths[] =
> > + "/etc/udev/hwdb.bin\0"
> > + UDEVLIBEXECDIR "/hwdb.bin\0";
> > +
> > +
> > +static int open_hwdb_bin(const char **path, FILE** f) {
> > + const char* p;
> > +
> > + NULSTR_FOREACH(p, hwdb_bin_paths) {
> > + *path = p;
>
> Please do not write functions that clobber passed-in arguments on
> failure. Please store any result in a temporary variable first, and
> clobber the passed-in variables only on success.
>
> > + *f = fopen(p, "re");
>
> same here.
Done for the FILE*, but I kept the behaviour for the path as the
caller uses the path in the error message on failure. I added a
comment to clarify this.
> > - if (stat("/etc/udev/hwdb.bin", &st) < 0)
> > +
> > + /* if hwdb.bin doesn't exist anywhere, we need to update */
> > + NULSTR_FOREACH(p, hwdb_bin_paths) {
> > + if (stat(p, &st) >= 0) {
> > + found = true;
> > + break;
> > + }
> > + }
> > + if (!found)
> > return true;
>
> I'd prefer if you could use access(..., F_OK) here, for checking for
> file existance, as stat() does substantially more than just checking
> existance.
I'm aware of access(), but the code below actually uses the stat value
for timestamp comparison, so we really need stat() here.
I also changed the hwdb option to --usr now. It's not very
descriptive and wrong for /lib, but then again this is only really
being used in package maintainer scripts or custom system image
builds; users will hardly ever see this.
Thanks,
Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From e0feeb5eb9de066e6908d34dff47380e3e860d09 Mon Sep 17 00:00:00 2001 From: Martin Pitt <[email protected]> Date: Fri, 17 Oct 2014 15:01:01 +0200 Subject: [PATCH] udev hwdb: Support shipping pre-compiled database in system images In some cases it is preferable to ship system images with a pre-generated binary hwdb database, to avoid having to build it at runtime, avoid shipping the source hwdb files, or avoid storing large binary files in /etc. So if hwdb.bin does not exist in /etc/udev/, fall back to looking for it in UDEVLIBEXECDIR. This keeps the possibility to add files to /etc/udev/hwdb.d/ and re-generating the database which trumps the one in /usr/lib. Add a new --usr flag to "udevadm hwdb --update" which puts the database into UDEVLIBEXECDIR. Adjust systemd-udev-hwdb-update.service to not generate the file in /etc if we already have it in /usr. --- man/udev.xml | 4 ++- man/udevadm.xml | 9 ++++++ src/libudev/libudev-hwdb.c | 50 ++++++++++++++++++++++++++----- src/udev/udevadm-hwdb.c | 13 +++++++- units/systemd-udev-hwdb-update.service.in | 3 ++ 5 files changed, 70 insertions(+), 9 deletions(-) diff --git a/man/udev.xml b/man/udev.xml index d77cbb0..87c16c7 100644 --- a/man/udev.xml +++ b/man/udev.xml @@ -766,7 +766,9 @@ <para>The content of all hwdb files is read by <citerefentry><refentrytitle>udevadm</refentrytitle><manvolnum>8</manvolnum></citerefentry> - and compiled to a binary database located at <filename>/etc/udev/hwdb.bin</filename>. + and compiled to a binary database located at <filename>/etc/udev/hwdb.bin</filename>, + or alternatively <filename>/usr/lib/udev/hwdb.bin</filename> if you want ship the compiled + database in an immutable image. During runtime only the binary database is used.</para> </refsect1> diff --git a/man/udevadm.xml b/man/udevadm.xml index 749144d..b85d9a9 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -494,6 +494,15 @@ </listitem> </varlistentry> <varlistentry> + <term><option>--usr</option></term> + <listitem> + <para>Put the compiled database into <filename>/usr/lib/udev/hwdb.bin</filename> instead. + Use this if you want to ship a pre-compiled database in immutable system images, or + don't use <filename>/etc/udev/hwdb.d</filename> and want to avoid large binary files in + <filename>/etc</filename>.</para> + </listitem> + </varlistentry> + <varlistentry> <term><option>-t</option></term> <term><option>--test=<replaceable>string</replaceable></option></term> <listitem> diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c index 8fb7240..1089645 100644 --- a/src/libudev/libudev-hwdb.c +++ b/src/libudev/libudev-hwdb.c @@ -256,6 +256,29 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) { return 0; } +static const char hwdb_bin_paths[] = + "/etc/udev/hwdb.bin\0" + UDEVLIBEXECDIR "/hwdb.bin\0"; + + +/* path is also written to on failures, so you can use it in error messages */ +static int open_hwdb_bin(const char **path, FILE** file) { + const char* p; + FILE* f; + + NULSTR_FOREACH(p, hwdb_bin_paths) { + *path = p; + f = fopen(p, "re"); + if (f) { + *file = f; + return 0; + } + else if (errno != ENOENT) + return -errno; + } + return -errno; +} + /** * udev_hwdb_new: * @udev: udev library context @@ -266,6 +289,8 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) { **/ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) { struct udev_hwdb *hwdb; + int r; + const char *hwdb_bin_path; const char sig[] = HWDB_SIG; hwdb = new0(struct udev_hwdb, 1); @@ -275,30 +300,30 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) { hwdb->refcount = 1; udev_list_init(udev, &hwdb->properties_list, true); - hwdb->f = fopen("/etc/udev/hwdb.bin", "re"); - if (!hwdb->f) { - udev_dbg(udev, "error reading /etc/udev/hwdb.bin: %m"); + r = open_hwdb_bin(&hwdb_bin_path, &hwdb->f); + if (r < 0) { + udev_dbg(udev, "error reading %s: %s", hwdb_bin_path, strerror(-r)); udev_hwdb_unref(hwdb); return NULL; } if (fstat(fileno(hwdb->f), &hwdb->st) < 0 || (size_t)hwdb->st.st_size < offsetof(struct trie_header_f, strings_len) + 8) { - udev_dbg(udev, "error reading /etc/udev/hwdb.bin: %m"); + udev_dbg(udev, "error reading %s: %m", hwdb_bin_path); udev_hwdb_unref(hwdb); return NULL; } hwdb->map = mmap(0, hwdb->st.st_size, PROT_READ, MAP_SHARED, fileno(hwdb->f), 0); if (hwdb->map == MAP_FAILED) { - udev_dbg(udev, "error mapping /etc/udev/hwdb.bin: %m"); + udev_dbg(udev, "error mapping %s: %m", hwdb_bin_path); udev_hwdb_unref(hwdb); return NULL; } if (memcmp(hwdb->map, sig, sizeof(hwdb->head->signature)) != 0 || (size_t)hwdb->st.st_size != le64toh(hwdb->head->file_size)) { - udev_dbg(udev, "error recognizing the format of /etc/udev/hwdb.bin"); + udev_dbg(udev, "error recognizing the format of %s", hwdb_bin_path); udev_hwdb_unref(hwdb); return NULL; } @@ -352,14 +377,25 @@ _public_ struct udev_hwdb *udev_hwdb_unref(struct udev_hwdb *hwdb) { } bool udev_hwdb_validate(struct udev_hwdb *hwdb) { + bool found = false; + const char* p; struct stat st; if (!hwdb) return false; if (!hwdb->f) return false; - if (stat("/etc/udev/hwdb.bin", &st) < 0) + + /* if hwdb.bin doesn't exist anywhere, we need to update */ + NULSTR_FOREACH(p, hwdb_bin_paths) { + if (stat(p, &st) >= 0) { + found = true; + break; + } + } + if (!found) return true; + if (timespec_load(&hwdb->st.st_mtim) != timespec_load(&st.st_mtim)) return true; return false; diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c index 64273fb..3ca755e 100644 --- a/src/udev/udevadm-hwdb.c +++ b/src/udev/udevadm-hwdb.c @@ -536,14 +536,20 @@ static int import_file(struct udev *udev, struct trie *trie, const char *filenam static void help(void) { printf("Usage: udevadm hwdb OPTIONS\n" " -u,--update update the hardware database\n" + " --usr generate in " UDEVLIBEXECDIR " instead of /etc/udev\n" " -t,--test=MODALIAS query database and print result\n" " -r,--root=PATH alternative root path in the filesystem\n" " -h,--help\n\n"); } static int adm_hwdb(struct udev *udev, int argc, char *argv[]) { + enum { + ARG_USR = 0x100, + }; + static const struct option options[] = { { "update", no_argument, NULL, 'u' }, + { "usr", no_argument, NULL, ARG_USR }, { "test", required_argument, NULL, 't' }, { "root", required_argument, NULL, 'r' }, { "help", no_argument, NULL, 'h' }, @@ -551,6 +557,7 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) { }; const char *test = NULL; const char *root = ""; + const char *hwdb_bin_dir = "/etc/udev"; bool update = false; struct trie *trie = NULL; int err, c; @@ -561,6 +568,9 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) { case 'u': update = true; break; + case ARG_USR: + hwdb_bin_dir = UDEVLIBEXECDIR; + break; case 't': test = optarg; break; @@ -634,7 +644,8 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) { log_debug("strings dedup'ed: %8zu bytes (%8zu)", trie->strings->dedup_len, trie->strings->dedup_count); - if (asprintf(&hwdb_bin, "%s/etc/udev/hwdb.bin", root) < 0) { + hwdb_bin = strjoin(root, "/", hwdb_bin_dir, "/hwdb.bin", NULL); + if (!hwdb_bin) { rc = EXIT_FAILURE; goto out; } diff --git a/units/systemd-udev-hwdb-update.service.in b/units/systemd-udev-hwdb-update.service.in index cdbcd83..5b1f75d 100644 --- a/units/systemd-udev-hwdb-update.service.in +++ b/units/systemd-udev-hwdb-update.service.in @@ -13,6 +13,9 @@ Conflicts=shutdown.target After=systemd-remount-fs.service Before=sysinit.target shutdown.target systemd-update-done.service ConditionNeedsUpdate=/etc +ConditionPathExists=|!@udevlibexecdir@/hwdb.bin +ConditionPathExists=|/etc/udev/hwdb.bin +ConditionDirectoryNotEmpty=|/etc/udev/hwdb.d/ [Service] Type=oneshot -- 2.1.0
signature.asc
Description: Digital signature
_______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
