Hello again,

the previous patch had a typo in the manpage (it said "/lib/udev"
instead of "/usr/lib/udev" at one place), and also forgot to adjust
systemd-udev-hwdb-update.service.in. Both done now.

However, the latter currently has a gotcha:

  +ConditionPathExists=!@udevlibexecdir@/hwdb.bin

This works correctly if you use this with the "factory reset"
semantics, i. e. start with an empty /etc. But it would not work if
you update /usr and have an already existing /etc/udev/hwdb.d/*. So
ideally the condition would be

  ConditionPathExists=!@udevlibexecdir@/hwdb.bin OR
  ConditionDirectoryNotEmpty=/etc/udev/hwdb.d/

but this isn't possible AFAIK. The alternative would be to change the
Exec= to call "hdwb --update --vendor" iff /etc/udev/hwdb.d/ is empty.

An alternative that was discussed at Plumbers was to make hwdb
--update automatically use @udevlibexecdir@ if there is no
/etc/udev/hwdb.d/, which would avoid this problem, avoid introducing a
new option, and still generally DTRT. I'd prefer this, so if you think
that's easier/better, I'm happy to update the patch accordingly.

Thanks,

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
>From 5c093f2552d7ebbafe079eafe9b0a9bada7615ad Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
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 --vendor 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                | 20 ++++++++++++++------
 src/udev/udevadm-hwdb.c                   | 10 ++++++++--
 units/systemd-udev-hwdb-update.service.in |  1 +
 5 files changed, 35 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..571ef7d 100644
--- a/man/udevadm.xml
+++ b/man/udevadm.xml
@@ -494,6 +494,15 @@
           </listitem>
         </varlistentry>
         <varlistentry>
+          <term><option>--vendor</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..91c05e8 100644
--- a/src/libudev/libudev-hwdb.c
+++ b/src/libudev/libudev-hwdb.c
@@ -256,6 +256,13 @@ static int trie_search_f(struct udev_hwdb *hwdb, const char *search) {
         return 0;
 }
 
+static const char *get_hwdb_bin_path(void) {
+        if (access("/etc/udev/hwdb.bin", F_OK) >= 0)
+                return "/etc/udev/hwdb.bin";
+        else
+                return UDEVLIBEXECDIR "/hwdb.bin";
+}
+
 /**
  * udev_hwdb_new:
  * @udev: udev library context
@@ -267,6 +274,7 @@ 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;
         const char sig[] = HWDB_SIG;
+        const char *hwdb_bin_path = get_hwdb_bin_path();
 
         hwdb = new0(struct udev_hwdb, 1);
         if (!hwdb)
@@ -275,30 +283,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");
+        hwdb->f = fopen(hwdb_bin_path, "re");
         if (!hwdb->f) {
-                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;
         }
 
         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;
         }
@@ -358,7 +366,7 @@ bool udev_hwdb_validate(struct udev_hwdb *hwdb) {
                 return false;
         if (!hwdb->f)
                 return false;
-        if (stat("/etc/udev/hwdb.bin", &st) < 0)
+        if (stat(get_hwdb_bin_path(), &st) < 0)
                 return true;
         if (timespec_load(&hwdb->st.st_mtim) != timespec_load(&st.st_mtim))
                 return true;
diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c
index 64273fb..2127213 100644
--- a/src/udev/udevadm-hwdb.c
+++ b/src/udev/udevadm-hwdb.c
@@ -536,6 +536,7 @@ 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"
+               "  --vendor             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");
@@ -544,6 +545,7 @@ static void help(void) {
 static int adm_hwdb(struct udev *udev, int argc, char *argv[]) {
         static const struct option options[] = {
                 { "update", no_argument,       NULL, 'u' },
+                { "vendor", no_argument,       NULL, 'V' },
                 { "test",   required_argument, NULL, 't' },
                 { "root",   required_argument, NULL, 'r' },
                 { "help",   no_argument,       NULL, 'h' },
@@ -551,16 +553,20 @@ 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;
         int rc = EXIT_SUCCESS;
 
-        while ((c = getopt_long(argc, argv, "ut:r:h", options, NULL)) >= 0)
+        while ((c = getopt_long(argc, argv, "uVt:r:h", options, NULL)) >= 0)
                 switch(c) {
                 case 'u':
                         update = true;
                         break;
+                case 'V':
+                        hwdb_bin_dir = UDEVLIBEXECDIR;
+                        break;
                 case 't':
                         test = optarg;
                         break;
@@ -634,7 +640,7 @@ 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) {
+                if (asprintf(&hwdb_bin, "%s/%s/hwdb.bin", root, hwdb_bin_dir) < 0) {
                         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..4ee5025 100644
--- a/units/systemd-udev-hwdb-update.service.in
+++ b/units/systemd-udev-hwdb-update.service.in
@@ -13,6 +13,7 @@ Conflicts=shutdown.target
 After=systemd-remount-fs.service
 Before=sysinit.target shutdown.target systemd-update-done.service
 ConditionNeedsUpdate=/etc
+ConditionPathExists=!@udevlibexecdir@/hwdb.bin
 
 [Service]
 Type=oneshot
-- 
2.1.0

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to