GNOME Bugzilla – Bug 663928
Missing fallback code for /etc/machine-id
Last modified: 2011-11-18 04:10:30 UTC
gio/gdbusprivate.c has hardcoded path to /var/lib/dbus/machine-id, while it's legal to not have the file at all and have /etc/machine-id instead, for systemd compability the relavent commit is: http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-sysdeps-unix.c?id=66e52541d5bdd4927a5c702963749760643313f4 very ugly workaround: sed -i -e '/g_file_get_contents/s:/var/lib/dbus/machine-id:/etc/machine-id:' gio/gdbusprivate.c downstream bug: http://bugs.gentoo.org/390143
Created attachment 201347 [details] [review] proposed patch
Looking at how this is done in D-Bus, it appears the order you have is inverted: http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-sysdeps-unix.c?id=66e52541d5bdd4927a5c702963749760643313f4
Review of attachment 201347 [details] [review]: ::: gio/gdbusprivate.c @@ +1993,3 @@ &ret, NULL, error)) First of all, as comment 2 says, you got the check inverted. Second, you need to adjust the error, e.g. g_prefix_error (error, _("Unable to load /var/lib/dbus/machine-id: ")); so it matches the proper location.
Please use gdbus, not gio for GDBus bugs (otherwise I won't see them).
Created attachment 201426 [details] [review] proposed patch (In reply to comment #3) > First of all, as comment 2 says, you got the check inverted. Second, you need > to adjust the error, e.g. > > g_prefix_error (error, _("Unable to load /var/lib/dbus/machine-id: ")); > > so it matches the proper location. OK, here's a second attempt. A temporary GError has to be used because g_file_get_contents("/var/lib/dbus/machine-id",...)'s error should only be propagated if g_file_get_contents("/etc/machine-id",...) also fails. And g_file_get_contents("/etc/machine-id",...) does not save its error because comment #2 implies that /var/lib/dbus is the default location for machine-id, and there doesn't seem to be a clean, translation-friendly way of combining two GErrors.
Review of attachment 201426 [details] [review]: I'm not a GLib/GDBus reviewer, but I can see one problem with this: ::: gio/gdbusprivate.c @@ +2003,3 @@ { /* TODO: validate value */ + g_error_free (first_error); This will issue a critical warning (or perhaps just segfault, depending how GLib was compiled) if /var/lib/dbus/machine-id was read successfully (i.e. first_error is still NULL). Use g_clear_error (&first_error) instead. With that change, I'd be happy with this patch (but again, I'm not an official reviewer). I'll do some tests in chroots with each of the files, neither, and both.
Created attachment 201548 [details] [review] _g_dbus_get_machine_id(): check /etc/machine-id too machine-id can be in /etc or in /var/lib/dbus. [amended with slightly revised error handling -smcv] --- I've attributed this patch to Alexandre. I tested it in a chroot environment with a test program that just calls dbus_bus_get_sync(): # /etc/machine-id only reptile(sid-amd64)% sudo install ~/tmp/uuid /etc/machine-id reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out could not connect: Error spawning command line `dbus-launch --autolaunch=804eeb4e85cd6eb299ad6675000068c1 --binary-syntax --close-stderr': Failed to execute child process "dbus-launch" (No such file or directory) # this indicates that the machine-id was read correctly: my chroot # doesn't have dbus-launch # both locations reptile(sid-amd64)% sudo install -d /var/lib/dbus reptile(sid-amd64)% sudo cp /etc/machine-id /var/lib/dbus/machine-id reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out could not connect: Error spawning command line `dbus-launch --autolaunch=804eeb4e85cd6eb299ad6675000068c1 --binary-syntax --close-stderr': Failed to execute child process "dbus-launch" (No such file or directory) # traditional location only reptile(sid-amd64)% sudo rm /etc/machine-id reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out could not connect: Error spawning command line `dbus-launch --autolaunch=804eeb4e85cd6eb299ad6675000068c1 --binary-syntax --close-stderr': Failed to execute child process "dbus-launch" (No such file or directory) # neither location reptile(sid-amd64)% sudo rm /var/lib/dbus/machine-id reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out could not connect: Cannot spawn a message bus without a machine-id: Unable to load /var/lib/dbus/machine-id or /etc/machine-id: Failed to open file '/var/lib/dbus/machine-id': No such file or directory
Comment on attachment 201548 [details] [review] _g_dbus_get_machine_id(): check /etc/machine-id too Looks good to me. Thanks Alexandre and Simon!
The following fix has been pushed: dc89b51 _g_dbus_get_machine_id(): check /etc/machine-id too
Created attachment 201640 [details] [review] _g_dbus_get_machine_id(): check /etc/machine-id too machine-id can be in /etc or in /var/lib/dbus. [amended with slightly revised error handling -smcv] Bug: