Skip to content
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.

Sign up for GitHub

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

Jump to bottom

libvmi: fix libxenctrl dynamic loading #45865

Merged
merged 2 commits into from Sep 4, 2018
Merged

libvmi: fix libxenctrl dynamic loading #45865

merged 2 commits into from Sep 4, 2018

Conversation

lschuermann
Copy link
Member

Motivation for this change

libvmi uses dlopen() to only require libxenctrl if the Xen hypervisor is used. However, dlopen does not work with Nix: https://nixos.org/nix-dev/2011-December/007413.html

Therefore, if Xen support is required, libvmi will be force-linked to libxenctrl. This seems to work correctly.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

libvmi: fix libxenctrl dynamic loading
f0906c5 
libvmi uses dlopen() to only require libxenctrl if the Xen hypervisor is
used. However, dlopen does not work with Nix:
https://nixos.org/nix-dev/2011-December/007413.html
Therefore, if Xen support is required, libvmi will be force-linked to
libxenctrl.
@xeji
Copy link
Contributor

xeji commented Sep 1, 2018

@GrahamcOfBorg build libvmi

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: libvmi

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: libvmi

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: libvmi

Partial log (click to expand)

shrinking /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/bin/vmi-win-guid
shrinking /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/bin/vmi-dump-memory
shrinking /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/bin/vmi-module-list
shrinking /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/bin/vmi-process-list
shrinking /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/lib/libvmi.so.0.0.12
strip is /nix/store/h0lbngpv6ln56hjj59i6l77vxq25flbz-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/lib  /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0/bin
patching script interpreter paths in /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0
checking for references to /build in /nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0...
/nix/store/4ikqxxfzvh2j0lvnakjrl0l64pm9pa0b-libvmi-0.12.0

@xeji
Copy link
Contributor

xeji commented Sep 1, 2018

However, dlopen does not work with Nix: https://nixos.org/nix-dev/2011-December/007413.html

That's not entirely true, and your reference is rather old.

Instead of patching the source to avoid dlopen (which works), the issue can be fixed by applying patchelf to fix the library's rpath after building.

Including autoPatchelfHook provides a convenient way to do so, see https://nixos.org/nixpkgs/manual/#ssec-setup-hooks. If it works, we generally preferred this approach because it avoids patching the source. Can you please give it a try?

@Mic92 Mic92 added this to the 18.09 milestone Sep 3, 2018
@lschuermann
Copy link
Member Author

Thank you very much for the info, @xeji! I didn't know about autoPatchelfHook.
I looked at a ton of packages that use it, but can't get it to work for libvmi.

What I did so far:

  • Add autoPatchelfHook to the nativeBuildInputs
  • Add xen (which contains the missing libxenctrl.so) to the runtimeDependencies
  • Set dontPatchELF to either true or false

However, the resulting ELF doesn't even have an rpath and the runpath shows no notice of xen. Do you know what I'm doing wrong here?

@xeji
Copy link
Contributor

xeji commented Sep 4, 2018

autoPatchelfHook doesn't work in all situations (I can't explain exactly when it does).
You can get xen into the runpath by manually applying patchelf with this:

diff --git a/pkgs/development/libraries/libvmi/default.nix b/pkgs/development/libraries/libvmi/default.nix
index 672804b7f39..d5a65b34e44 100644
--- a/pkgs/development/libraries/libvmi/default.nix
+++ b/pkgs/development/libraries/libvmi/default.nix
@@ -23,13 +23,17 @@ stdenv.mkDerivation rec {
     sha256 = "0wbi2nasb1gbci6cq23g6kq7i10rwi1y7r44rl03icr5prqjpdyv";
   };

-  patches = optional xenSupport ./libxenctrl.diff;
-
   buildInputs = [ glib libvirt json_c ] ++ (optional xenSupport xen);
   nativeBuildInputs = [ autoreconfHook bison flex pkgconfig ];

   configureFlags = optional (!xenSupport) "--disable-xen";

+  postFixup = ''
+    libvmi="$out/lib/libvmi.so.0.0.12"
+    oldrpath=$(patchelf --print-rpath "$libvmi")
+    patchelf --set-rpath "$oldrpath:${makeLibraryPath [ xen ]}" "$libvmi"
+  '';
+
   meta = with stdenv.lib; {
     homepage = "http://libvmi.com/";
     description = "A C library for virtual machine introspection";

With this change the runpath looks ok:

$ readelf -d libvmi.so

Dynamic section at offset 0x376c8 contains 35 entries:
  Tag        Type                         Name/Value
...
 0x000000000000001d (RUNPATH)            Library runpath: [/nix/store/kcsi934n6paxnhzqh1hjir4glm9qbrc0-glib-2.56.0/lib:/nix/store/ywwqxvba637x0hc5dj9p2k009p09wrzk-pcre-8.41/lib:/nix/store/sc0pnl3nvw7y46hbbm9zkbxbv6zifxlr-json-c-0.13.1/lib:/nix/store/83lrbvbmxrgv7iz49mgd42yvhi473xp6-glibc-2.27/lib:/nix/store/5a42h1jpphnkygvi5r6mmdxyx7wm6394-xen-4.8.3/lib]

Please test if this fixes your dlopen issue.

@xeji
Copy link
Contributor

xeji commented Sep 4, 2018

(btw, the patchelf parameter --set-rpath actually sets RUNPATH, it's a slightly misleading historic name.)

@lschuermann
Copy link
Member Author

Please test if this fixes your dlopen issue.

Works like a charm! Thank you very much.
I've added a libVersion field as apparently they aren't consistent when it comes to versioning the shared objects. In addition to that, the fix-up is only done when Xen support is requested.

@xeji
Copy link
Contributor

xeji commented Sep 4, 2018

Please also remove the obsolete libxenctrl.diff.

libvmi: fix RUNPATH for loading libxenctrl
60e62f9 
This integrates the changes proposed by @xeji here:
NixOS#45865 (comment)
@lschuermann
Copy link
Member Author

Please also remove the obsolete libxenctrl.diff.

Sorry, forgot about that. It's done.

@xeji
Copy link
Contributor

xeji commented Sep 4, 2018

@GrahamcOfBorg build libvmi

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: libvmi

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: libvmi

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: libvmi

Partial log (click to expand)

shrinking /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/lib/libvmi.so.0.0.12
shrinking /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/bin/vmi-process-list
shrinking /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/bin/vmi-module-list
shrinking /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/bin/vmi-win-guid
shrinking /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/bin/vmi-dump-memory
strip is /nix/store/h0lbngpv6ln56hjj59i6l77vxq25flbz-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/lib  /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0/bin
patching script interpreter paths in /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0
checking for references to /build in /nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0...
/nix/store/rgjymhisz1mvrhvralr0b96gwwymlfqy-libvmi-0.12.0

@xeji xeji merged commit 4295e22 into NixOS:master Sep 4, 2018
@xeji
Copy link
Contributor

xeji commented Sep 4, 2018

During merge I squashed the commits and edited the commit message to fit the implemented solution.

@lschuermann lschuermann deleted the libvmi branch October 14, 2018 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
18.09
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lschuermann @xeji @GrahamcOfBorg @Mic92