New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of /etc/machine-id causes test failure #100

Closed
markhindley opened this Issue Nov 24, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@markhindley
Copy link
Contributor

markhindley commented Nov 24, 2018

Sven,

I am wanting to enable the tests for our De[vu|bi]an package builds. I have been debugging the few failures (test-id128 [sd_id128_get_machine_app_specific], test-fs-util [chase_symlinks], test-process-util [mount MS_PRIVATE|MS_REC] and test-login [sd_seat_get_active]). They all seem to be related to the use of /etc/machine-id. This file is not present in debootstrap chroots and its presence cannot be relied upon for systems that have never been booted with systemd.

I have tried with dbus installed in the chroot and using /var/lib/dbus/machine-id and all the tests pass.

Would you consider this patch?

--- a/src/libelogind/sd-id128/sd-id128.c
+++ b/src/libelogind/sd-id128/sd-id128.c
@@ -90,7 +90,7 @@
assert_return(ret, -EINVAL);

     if (sd_id128_is_null(saved_machine_id)) {
  •            r = id128_read("/etc/machine-id", ID128_PLAIN, &saved_machine_id);
    
  •            r = id128_read("/var/lib/dbus/machine-id", ID128_PLAIN, &saved_machine_id);
               if (r < 0)
                       return r;
    

--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -174,7 +174,7 @@
assert_se(streq(result, "/test-chase.fsldajfl"));
result = mfree(result);

  •    r = chase_symlinks("/etc/machine-id/foo", NULL, 0, &result);
    
  •    r = chase_symlinks("/var/lib/dbus/machine-id/foo", NULL, 0, &result);
       assert_se(r == -ENOTDIR);
       result = mfree(result);
    

I see systemd has an environment variable to override the machine-id path, but that seems less attractive to me?

Thanks

Mark

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Nov 24, 2018

Well, the environment variable would not catch here, as this is one of (far too) many hard coded paths in systemd.

I would favour to do the default, and then add an "#if 1" block to check the dbus path if the default one failed. This way the change would not interfere with future updates when I migrate commits from systemd, as my migration tools can handle that

To be a bit more concrete:

        if (sd_id128_is_null(saved_machine_id)) {
                r = id128_read("/etc/machine-id", ID128_PLAIN, &saved_machine_id);
#if 1 /// elogind also checks the path of the dbus provided machine id
                if (r < 0)
                        r = id128_read("/var/lib/dbus/machine-id", ID128_PLAIN, &saved_machine_id);
#endif // 1
                if (r < 0)
                        return r;

                if (sd_id128_is_null(saved_machine_id))
                        return -ENOMEDIUM;
        }

That's what the idea would look like. The second example would be changed accordingly.
I hope I can test this tonight.

What do you say?

@Yamakuzure Yamakuzure self-assigned this Nov 24, 2018

@markhindley

This comment has been minimized.

Copy link
Contributor Author

markhindley commented Nov 24, 2018

Yamakuzure added a commit that referenced this issue Nov 26, 2018

Prep v239.3: Also search machine-id in the dbus default path. (#100)
In setups where the machine-id file is not generated in /etc/, but in
the default path /var/lib/dbus, both sd_id128_get_machine() and
test-fs-util fail.

The proposed solution is to check the dbus default path, if the file
is not found in /etc/.

Bug: #100
Signed-Off-By: Sven Eden <sven.eden@prydeworx.com>

Yamakuzure added a commit that referenced this issue Nov 26, 2018

Prep v238.3: Also search machine-id in the dbus default path. (#100)
In setups where the machine-id file is not generated in /etc/, but in
the default path /var/lib/dbus, both sd_id128_get_machine() and
test-fs-util fail.

The proposed solution is to check the dbus default path, if the file
is not found in /etc/.

Bug: #100
Signed-Off-By: Sven Eden <sven.eden@prydeworx.com>

Yamakuzure added a commit that referenced this issue Dec 7, 2018

Version 239.3 Release
This is a service release, bringing the following changes and fixes:

Fixes:
 * Clean up bus_creds_dump()
 * Fix documentation to refer to elogind. (#98)
 * Fixed another Unicode test that has slithered in. (#99)

Changes / Additions:
 * sd_bus_creds_get_[user_]slice() simplified.
 * Updated documentation to be more consistent (#98)
 * Also search machine-id in the dbus default path. (#100)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment