| Message ID | 20180130040720.20172-3-casantos@datacom.ind.br |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/3] eudev: fix error handling init script | expand |
> From: "Carlos Santos" <casantos@datacom.ind.br> > To: "buildroot" <buildroot@buildroot.org> > Cc: "Bernd Kuhls" <bernd.kuhls@t-online.de>, "Eric Le Bihan" <eric.le.bihan.dev@free.fr> > Sent: Tuesday, January 30, 2018 2:07:20 AM > Subject: [Buildroot] [PATCH 3/3] eudev: generate hwdb.bin at system startup > Programs that use libudev (e.g. lsusb, from usbutils) need the hwdb.bin > file to work properly. Ping. ;-) https://patchwork.ozlabs.org/patch/867381/
Carlos, Yann, On Mon, Oct 15, 2018 at 2:43 PM Carlos Santos <casantos@datacom.ind.br> wrote: > > Programs that use libudev (e.g. lsusb, from usbutils) need the hwdb.bin > file to work properly. > > If BR2_PACKAGE_EUDEV_ENABLE_HWDB is selected then the eudev installation > populates /etc/udev/hwdb.d/ but does not genarete /etc/udev/hwdb.bin. It > must be created running "udevadm hwdb --update" on the target device but > this does not work on with read-only /etc, so we need these changes: > > - Add the BR2_PACKAGE_EUDEV_HWDB_BIN_PATH config, allowing the user to > set the location of hwdb.bin. > - Patch the configuration script, allowing to set the hwdb.bin path by > means of an environment variable (udevhwdbbinpath). > - Pass the value set in BR2_PACKAGE_EUDEV_HWDB_BIN_PATH to the configure > script by means of the udevhwdbbinpath variable. > - Make the S10udev init script run "udevadm hwdb --update" to recreate > hwdb.bin or print an error message if the operation fails (e.g. no > write permission on the filesystem). > (aarch qemu target) Tested-by: Matt Weber <matthew.weber@rockwellcollins.com> > Signed-off-by: Carlos Santos <casantos@datacom.ind.br> > --- > package/eudev/Config.in | 9 +++++++++ > package/eudev/S10udev | 8 +++++++- > package/eudev/eudev.mk | 13 ++++++++++++- > 3 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/package/eudev/Config.in b/package/eudev/Config.in > index 2220265a55..a589a14af3 100644 > --- a/package/eudev/Config.in > +++ b/package/eudev/Config.in > @@ -32,6 +32,15 @@ config BR2_PACKAGE_EUDEV_ENABLE_HWDB > help > Enables hardware database installation to /etc/udev/hwdb.d > > +config BR2_PACKAGE_EUDEV_HWDB_BIN_PATH Should this depend on BR2_PACKAGE_EUDEV_ENABLE_HWDB? What does it do if you don't enable this option or have a write-able location for the .bin? > + string "hwdb.bin path" > + default "/etc/udev/hwdb.bin" > + help > + Location of the hwdb.bin file, which is re-generated at system > + startup. The default is /etc/udev/hwdb.bin but you may want to > + move it elsewhere (e.g. /var/run/udev/hwdb.bin) if /etc is in > + a read-only filesystem. > + > endif > > comment "eudev needs eudev /dev management" > diff --git a/package/eudev/S10udev b/package/eudev/S10udev > index 640fec625b..026ab6ae49 100755 > --- a/package/eudev/S10udev > +++ b/package/eudev/S10udev > @@ -25,14 +25,20 @@ UDEV_CONFIG=/etc/udev/udev.conf This section of the patch no-longer applies (cosmetic changes required). > test -r $UDEV_CONFIG || exit 6 > . $UDEV_CONFIG > > +UDEV_HWDB_BIN=%%UDEV_HWDB_BIN%% > + > case "$1" in > start) > printf "Populating %s using udev: " "${udev_root:-/dev}" > printf '\000\000\000\000' > /proc/sys/kernel/hotplug > $UDEV_BIN -d || { echo "FAIL"; exit 1; } > + echo "done" > udevadm trigger --type=subsystems --action=add > udevadm trigger --type=devices --action=add > - udevadm settle --timeout=30 || echo "udevadm settle failed" > + udevadm settle --timeout=30 || { echo "udevadm settle failed" && exit 1; } > + echo "done" > + printf "Compiling hardware database information $UDEV_HWDB_BIN: " > + udevadm hwdb --update || { echo "FAIL" && exit 1; } Yann had a good point on IRC, this probably could be prebuild offline. > echo "done" > ;; > stop) > diff --git a/package/eudev/eudev.mk b/package/eudev/eudev.mk > index d08b9bb0c9..25745008d2 100644 > --- a/package/eudev/eudev.mk > +++ b/package/eudev/eudev.mk > @@ -47,8 +47,19 @@ else > EUDEV_CONF_OPTS += --disable-selinux > endif > > +define EUDEV_POST_PATCH > + $(SED) 's,$${udevconfdir}/hwdb.bin,$${udevhwdbbinpath:-$${udevconfdir}/hwdb.bin},' \ > + $(@D)/configure > +endef > +EUDEV_POST_PATCH_HOOKS += EUDEV_POST_PATCH > + > +EUDEV_CONF_ENV += udevhwdbbinpath=$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH) > + > define EUDEV_INSTALL_INIT_SYSV > - $(INSTALL) -D -m 0755 package/eudev/S10udev $(TARGET_DIR)/etc/init.d/S10udev > + $(INSTALL) -D -m 0755 package/eudev/S10udev \ > + $(TARGET_DIR)/etc/init.d/S10udev > + $(SED) 's,%%UDEV_HWDB_BIN%%,$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH),' \ > + $(TARGET_DIR)/etc/init.d/S10udev Instead of a SED approach, could we add a /etc/defaults/udev cfg file instead and just set the value in that file if present. Otherwise default the value? (Like what was done in the recent S01logging suggested updates). Matt
> From: "Matthew Weber" <matthew.weber@rockwellcollins.com> > To: "Carlos Santos" <casantos@datacom.ind.br> > Cc: "buildroot" <buildroot@buildroot.org>, "Bernd Kuhls" <bernd.kuhls@t-online.de>, "Eric Le Bihan" > <eric.le.bihan.dev@free.fr>, "Yann Morin" <yann.morin.1998@free.fr> > Sent: Monday, October 15, 2018 4:55:51 PM > Subject: Re: [Buildroot] [PATCH 3/3] eudev: generate hwdb.bin at system startup > Carlos, Yann, > > On Mon, Oct 15, 2018 at 2:43 PM Carlos Santos <casantos@datacom.ind.br> wrote: >> >> Programs that use libudev (e.g. lsusb, from usbutils) need the hwdb.bin >> file to work properly. >> >> If BR2_PACKAGE_EUDEV_ENABLE_HWDB is selected then the eudev installation >> populates /etc/udev/hwdb.d/ but does not genarete /etc/udev/hwdb.bin. It >> must be created running "udevadm hwdb --update" on the target device but >> this does not work on with read-only /etc, so we need these changes: >> >> - Add the BR2_PACKAGE_EUDEV_HWDB_BIN_PATH config, allowing the user to >> set the location of hwdb.bin. >> - Patch the configuration script, allowing to set the hwdb.bin path by >> means of an environment variable (udevhwdbbinpath). >> - Pass the value set in BR2_PACKAGE_EUDEV_HWDB_BIN_PATH to the configure >> script by means of the udevhwdbbinpath variable. >> - Make the S10udev init script run "udevadm hwdb --update" to recreate >> hwdb.bin or print an error message if the operation fails (e.g. no >> write permission on the filesystem). >> > > (aarch qemu target) > Tested-by: Matt Weber <matthew.weber@rockwellcollins.com> > >> Signed-off-by: Carlos Santos <casantos@datacom.ind.br> >> --- >> package/eudev/Config.in | 9 +++++++++ >> package/eudev/S10udev | 8 +++++++- >> package/eudev/eudev.mk | 13 ++++++++++++- >> 3 files changed, 28 insertions(+), 2 deletions(-) >> >> diff --git a/package/eudev/Config.in b/package/eudev/Config.in >> index 2220265a55..a589a14af3 100644 >> --- a/package/eudev/Config.in >> +++ b/package/eudev/Config.in >> @@ -32,6 +32,15 @@ config BR2_PACKAGE_EUDEV_ENABLE_HWDB >> help >> Enables hardware database installation to /etc/udev/hwdb.d >> >> +config BR2_PACKAGE_EUDEV_HWDB_BIN_PATH > > Should this depend on BR2_PACKAGE_EUDEV_ENABLE_HWDB? What does it do > if you don't enable this option or have a write-able location for the > .bin? > >> + string "hwdb.bin path" >> + default "/etc/udev/hwdb.bin" >> + help >> + Location of the hwdb.bin file, which is re-generated at system >> + startup. The default is /etc/udev/hwdb.bin but you may want to >> + move it elsewhere (e.g. /var/run/udev/hwdb.bin) if /etc is in >> + a read-only filesystem. >> + >> endif >> >> comment "eudev needs eudev /dev management" >> diff --git a/package/eudev/S10udev b/package/eudev/S10udev >> index 640fec625b..026ab6ae49 100755 >> --- a/package/eudev/S10udev >> +++ b/package/eudev/S10udev >> @@ -25,14 +25,20 @@ UDEV_CONFIG=/etc/udev/udev.conf > > This section of the patch no-longer applies (cosmetic changes required). I will update the patch accordingly. >> test -r $UDEV_CONFIG || exit 6 >> . $UDEV_CONFIG >> >> +UDEV_HWDB_BIN=%%UDEV_HWDB_BIN%% >> + >> case "$1" in >> start) >> printf "Populating %s using udev: " "${udev_root:-/dev}" >> printf '\000\000\000\000' > /proc/sys/kernel/hotplug >> $UDEV_BIN -d || { echo "FAIL"; exit 1; } >> + echo "done" >> udevadm trigger --type=subsystems --action=add >> udevadm trigger --type=devices --action=add >> - udevadm settle --timeout=30 || echo "udevadm settle failed" >> + udevadm settle --timeout=30 || { echo "udevadm settle failed" && exit >> 1; } >> + echo "done" >> + printf "Compiling hardware database information $UDEV_HWDB_BIN: " >> + udevadm hwdb --update || { echo "FAIL" && exit 1; } > > Yann had a good point on IRC, this probably could be prebuild offline. Does "offline" mean "at build time" or "asynchronous to the initialization"? >> echo "done" >> ;; >> stop) >> diff --git a/package/eudev/eudev.mk b/package/eudev/eudev.mk >> index d08b9bb0c9..25745008d2 100644 >> --- a/package/eudev/eudev.mk >> +++ b/package/eudev/eudev.mk >> @@ -47,8 +47,19 @@ else >> EUDEV_CONF_OPTS += --disable-selinux >> endif >> >> +define EUDEV_POST_PATCH >> + $(SED) >> 's,$${udevconfdir}/hwdb.bin,$${udevhwdbbinpath:-$${udevconfdir}/hwdb.bin},' \ >> + $(@D)/configure >> +endef >> +EUDEV_POST_PATCH_HOOKS += EUDEV_POST_PATCH >> + >> +EUDEV_CONF_ENV += udevhwdbbinpath=$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH) >> + >> define EUDEV_INSTALL_INIT_SYSV >> - $(INSTALL) -D -m 0755 package/eudev/S10udev >> $(TARGET_DIR)/etc/init.d/S10udev >> + $(INSTALL) -D -m 0755 package/eudev/S10udev \ >> + $(TARGET_DIR)/etc/init.d/S10udev >> + $(SED) 's,%%UDEV_HWDB_BIN%%,$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH),' \ >> + $(TARGET_DIR)/etc/init.d/S10udev > > Instead of a SED approach, could we add a /etc/defaults/udev cfg file > instead and just set the value in that file if present. Otherwise > default the value? (Like what was done in the recent S01logging > suggested updates). I like this approach. I will update the patch to implement it.
Carlos, On Mon, Oct 15, 2018 at 3:09 PM Carlos Santos <casantos@datacom.com.br> wrote: > > > From: "Matthew Weber" <matthew.weber@rockwellcollins.com> > > To: "Carlos Santos" <casantos@datacom.ind.br> > > Cc: "buildroot" <buildroot@buildroot.org>, "Bernd Kuhls" <bernd.kuhls@t-online.de>, "Eric Le Bihan" > > <eric.le.bihan.dev@free.fr>, "Yann Morin" <yann.morin.1998@free.fr> > > Sent: Monday, October 15, 2018 4:55:51 PM > > Subject: Re: [Buildroot] [PATCH 3/3] eudev: generate hwdb.bin at system startup > > > Carlos, Yann, > > > > On Mon, Oct 15, 2018 at 2:43 PM Carlos Santos <casantos@datacom.ind.br> wrote: > >> > >> Programs that use libudev (e.g. lsusb, from usbutils) need the hwdb.bin > >> file to work properly. > >> > >> If BR2_PACKAGE_EUDEV_ENABLE_HWDB is selected then the eudev installation > >> populates /etc/udev/hwdb.d/ but does not genarete /etc/udev/hwdb.bin. It > >> must be created running "udevadm hwdb --update" on the target device but > >> this does not work on with read-only /etc, so we need these changes: > >> > >> - Add the BR2_PACKAGE_EUDEV_HWDB_BIN_PATH config, allowing the user to > >> set the location of hwdb.bin. > >> - Patch the configuration script, allowing to set the hwdb.bin path by > >> means of an environment variable (udevhwdbbinpath). > >> - Pass the value set in BR2_PACKAGE_EUDEV_HWDB_BIN_PATH to the configure > >> script by means of the udevhwdbbinpath variable. > >> - Make the S10udev init script run "udevadm hwdb --update" to recreate > >> hwdb.bin or print an error message if the operation fails (e.g. no > >> write permission on the filesystem). > >> > > > > (aarch qemu target) > > Tested-by: Matt Weber <matthew.weber@rockwellcollins.com> > > > >> Signed-off-by: Carlos Santos <casantos@datacom.ind.br> > >> --- > >> package/eudev/Config.in | 9 +++++++++ > >> package/eudev/S10udev | 8 +++++++- > >> package/eudev/eudev.mk | 13 ++++++++++++- > >> 3 files changed, 28 insertions(+), 2 deletions(-) > >> > >> diff --git a/package/eudev/Config.in b/package/eudev/Config.in > >> index 2220265a55..a589a14af3 100644 > >> --- a/package/eudev/Config.in > >> +++ b/package/eudev/Config.in > >> @@ -32,6 +32,15 @@ config BR2_PACKAGE_EUDEV_ENABLE_HWDB > >> help > >> Enables hardware database installation to /etc/udev/hwdb.d > >> > >> +config BR2_PACKAGE_EUDEV_HWDB_BIN_PATH > > > > Should this depend on BR2_PACKAGE_EUDEV_ENABLE_HWDB? What does it do > > if you don't enable this option or have a write-able location for the > > .bin? > > > >> + string "hwdb.bin path" > >> + default "/etc/udev/hwdb.bin" > >> + help > >> + Location of the hwdb.bin file, which is re-generated at system > >> + startup. The default is /etc/udev/hwdb.bin but you may want to > >> + move it elsewhere (e.g. /var/run/udev/hwdb.bin) if /etc is in > >> + a read-only filesystem. > >> + > >> endif > >> > >> comment "eudev needs eudev /dev management" > >> diff --git a/package/eudev/S10udev b/package/eudev/S10udev > >> index 640fec625b..026ab6ae49 100755 > >> --- a/package/eudev/S10udev > >> +++ b/package/eudev/S10udev > >> @@ -25,14 +25,20 @@ UDEV_CONFIG=/etc/udev/udev.conf > > > > This section of the patch no-longer applies (cosmetic changes required). > > I will update the patch accordingly. > > >> test -r $UDEV_CONFIG || exit 6 > >> . $UDEV_CONFIG > >> > >> +UDEV_HWDB_BIN=%%UDEV_HWDB_BIN%% > >> + > >> case "$1" in > >> start) > >> printf "Populating %s using udev: " "${udev_root:-/dev}" > >> printf '\000\000\000\000' > /proc/sys/kernel/hotplug > >> $UDEV_BIN -d || { echo "FAIL"; exit 1; } > >> + echo "done" > >> udevadm trigger --type=subsystems --action=add > >> udevadm trigger --type=devices --action=add > >> - udevadm settle --timeout=30 || echo "udevadm settle failed" > >> + udevadm settle --timeout=30 || { echo "udevadm settle failed" && exit > >> 1; } > >> + echo "done" > >> + printf "Compiling hardware database information $UDEV_HWDB_BIN: " > >> + udevadm hwdb --update || { echo "FAIL" && exit 1; } > > > > Yann had a good point on IRC, this probably could be prebuild offline. > > Does "offline" mean "at build time" or "asynchronous to the initialization"? > Build time via adding eudev as a host tool. > >> echo "done" > >> ;; > >> stop) > >> diff --git a/package/eudev/eudev.mk b/package/eudev/eudev.mk > >> index d08b9bb0c9..25745008d2 100644 > >> --- a/package/eudev/eudev.mk > >> +++ b/package/eudev/eudev.mk > >> @@ -47,8 +47,19 @@ else > >> EUDEV_CONF_OPTS += --disable-selinux > >> endif > >> > >> +define EUDEV_POST_PATCH > >> + $(SED) > >> 's,$${udevconfdir}/hwdb.bin,$${udevhwdbbinpath:-$${udevconfdir}/hwdb.bin},' \ > >> + $(@D)/configure > >> +endef > >> +EUDEV_POST_PATCH_HOOKS += EUDEV_POST_PATCH > >> + > >> +EUDEV_CONF_ENV += udevhwdbbinpath=$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH) > >> + > >> define EUDEV_INSTALL_INIT_SYSV > >> - $(INSTALL) -D -m 0755 package/eudev/S10udev > >> $(TARGET_DIR)/etc/init.d/S10udev > >> + $(INSTALL) -D -m 0755 package/eudev/S10udev \ > >> + $(TARGET_DIR)/etc/init.d/S10udev > >> + $(SED) 's,%%UDEV_HWDB_BIN%%,$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH),' \ > >> + $(TARGET_DIR)/etc/init.d/S10udev > > > > Instead of a SED approach, could we add a /etc/defaults/udev cfg file > > instead and just set the value in that file if present. Otherwise > > default the value? (Like what was done in the recent S01logging > > suggested updates). > > I like this approach. I will update the patch to implement it. > Matt
diff --git a/package/eudev/Config.in b/package/eudev/Config.in index 2220265a55..a589a14af3 100644 --- a/package/eudev/Config.in +++ b/package/eudev/Config.in @@ -32,6 +32,15 @@ config BR2_PACKAGE_EUDEV_ENABLE_HWDB help Enables hardware database installation to /etc/udev/hwdb.d +config BR2_PACKAGE_EUDEV_HWDB_BIN_PATH + string "hwdb.bin path" + default "/etc/udev/hwdb.bin" + help + Location of the hwdb.bin file, which is re-generated at system + startup. The default is /etc/udev/hwdb.bin but you may want to + move it elsewhere (e.g. /var/run/udev/hwdb.bin) if /etc is in + a read-only filesystem. + endif comment "eudev needs eudev /dev management" diff --git a/package/eudev/S10udev b/package/eudev/S10udev index 640fec625b..026ab6ae49 100755 --- a/package/eudev/S10udev +++ b/package/eudev/S10udev @@ -25,14 +25,20 @@ UDEV_CONFIG=/etc/udev/udev.conf test -r $UDEV_CONFIG || exit 6 . $UDEV_CONFIG +UDEV_HWDB_BIN=%%UDEV_HWDB_BIN%% + case "$1" in start) printf "Populating %s using udev: " "${udev_root:-/dev}" printf '\000\000\000\000' > /proc/sys/kernel/hotplug $UDEV_BIN -d || { echo "FAIL"; exit 1; } + echo "done" udevadm trigger --type=subsystems --action=add udevadm trigger --type=devices --action=add - udevadm settle --timeout=30 || echo "udevadm settle failed" + udevadm settle --timeout=30 || { echo "udevadm settle failed" && exit 1; } + echo "done" + printf "Compiling hardware database information $UDEV_HWDB_BIN: " + udevadm hwdb --update || { echo "FAIL" && exit 1; } echo "done" ;; stop) diff --git a/package/eudev/eudev.mk b/package/eudev/eudev.mk index d08b9bb0c9..25745008d2 100644 --- a/package/eudev/eudev.mk +++ b/package/eudev/eudev.mk @@ -47,8 +47,19 @@ else EUDEV_CONF_OPTS += --disable-selinux endif +define EUDEV_POST_PATCH + $(SED) 's,$${udevconfdir}/hwdb.bin,$${udevhwdbbinpath:-$${udevconfdir}/hwdb.bin},' \ + $(@D)/configure +endef +EUDEV_POST_PATCH_HOOKS += EUDEV_POST_PATCH + +EUDEV_CONF_ENV += udevhwdbbinpath=$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH) + define EUDEV_INSTALL_INIT_SYSV - $(INSTALL) -D -m 0755 package/eudev/S10udev $(TARGET_DIR)/etc/init.d/S10udev + $(INSTALL) -D -m 0755 package/eudev/S10udev \ + $(TARGET_DIR)/etc/init.d/S10udev + $(SED) 's,%%UDEV_HWDB_BIN%%,$(BR2_PACKAGE_EUDEV_HWDB_BIN_PATH),' \ + $(TARGET_DIR)/etc/init.d/S10udev endef # Required by default rules for input devices
Programs that use libudev (e.g. lsusb, from usbutils) need the hwdb.bin file to work properly. If BR2_PACKAGE_EUDEV_ENABLE_HWDB is selected then the eudev installation populates /etc/udev/hwdb.d/ but does not genarete /etc/udev/hwdb.bin. It must be created running "udevadm hwdb --update" on the target device but this does not work on with read-only /etc, so we need these changes: - Add the BR2_PACKAGE_EUDEV_HWDB_BIN_PATH config, allowing the user to set the location of hwdb.bin. - Patch the configuration script, allowing to set the hwdb.bin path by means of an environment variable (udevhwdbbinpath). - Pass the value set in BR2_PACKAGE_EUDEV_HWDB_BIN_PATH to the configure script by means of the udevhwdbbinpath variable. - Make the S10udev init script run "udevadm hwdb --update" to recreate hwdb.bin or print an error message if the operation fails (e.g. no write permission on the filesystem). Signed-off-by: Carlos Santos <casantos@datacom.ind.br> --- package/eudev/Config.in | 9 +++++++++ package/eudev/S10udev | 8 +++++++- package/eudev/eudev.mk | 13 ++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-)