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.

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

common.postinst: binary-only ldtarball fails because load_tarball returns non-zero status #362

Open
LuKePicci opened this issue Nov 8, 2023 · 6 comments

Comments

@LuKePicci
Copy link

if ! dkms ldtarball --archive "$TARBALL_ROOT/$NAME-$VERSION.dkms.tar.gz"; then

The line above expects ldtarball to exit with status 0 on success but 1 is returned. I have a binary-only tarball made by mktarball which loads and works just fine but I can't make an rpm out of it using common.postinst because it seems that load_tarball function returns non-zero status even if tarball is loaded successfully.

If the exit status of 1 is correct then the above line should be changed in order to accept that situation without failing.

@evelikov
Copy link
Collaborator

evelikov commented Nov 8, 2023

Abstracting ourselves from all the code and implementation details, can you outline a simple reproducer - the more noob friendly the better - the expected behaviour and the actual result you're seeing.

Thanks.

@LuKePicci
Copy link
Author

LuKePicci commented Nov 8, 2023

Sure, you're welcome.

We have a dkms driver which works fine compiling from sources (fedora 38, x86_64). We would like to avoid the compilation step on a specific device with fixed kernel and distro (fedora 38 as well, x86_64).

We used dkms mktarball with binaries only options to create a prebuilt tar.gz which works fine if we load it using dkms ldtarball and then dkms install -m Mname -v Mversion.

That said, we are now working on packaging such prebuilt driver as RPM package for which we have the following command in the %posttrans hook

%posttrans
/usr/lib/dkms/common.postinst %{module} %{version} $RPM_BUILD_ROOT/usr/modules %{buildarch}

According to the code of common.postinst, when $3 is defined (TARBALL_ROOT, valued /usr/modules at runtime) it is expected to load the driver from a prebuilt tarball with specific file name %{module}-%{version}.dkms.tar.gz and return success status to the rpm package installer.

However, the rpm install fails because common.postinst exits with error from the above line of code. During install, we see the ldtarball command is being executed with right arguments and is printing out normal informational messages to the console but its final exit status makes the if statement at that sae line to take the main branch and thus entering the error condition returned to the rpm package installer.

To reproduce, load and build any dkms module, use mktarball with binaries-only option to create a prebuilt tarball, remove the module from dkms system and then try using the above %posttrans command to install it again. Use echo $? to check the exit status code, you will see it is not 0, as the common.postinst expects.

@evelikov
Copy link
Collaborator

evelikov commented Nov 9, 2023

If you have done the analysis and can open a MR that would be great. I would request that it includes a test case so we don't regress things.

Alternatively a step-by-step reproducer, as requested earlier is warranted.

@anbe42
Copy link
Contributor

anbe42 commented Nov 24, 2023

Hint: You could use the minimal kernel module from test/dkms_test-1.0/ for the step-by-step reproducer.

@LuKePicci
Copy link
Author

Great I'll try to make repro steps then. I can also make a PR for the fix but before making a blind fix I would need to ask whoever authored the load_tarball function if it is expected for that function to return 1 in similar invocations. Then I would patch load_tarball rather then common.postinst

@anbe42
Copy link
Contributor

anbe42 commented Nov 24, 2023

If you are going to make a PR, you should probably start with adding some tests for mktarball and ldtarball to run_tests.sh. These should cover at least the three use cases source/binary/both. For a start it should be sufficient to look at the "success" cases (whether or not they return 0), tests covering all the possible error paths can come later.

From a sanely behaving mktarball and ldtarball subcommand I'd expect a return value of 0 on success and !0 otherwise.
I'm not sure which error case it tries to signal currently with the return of 1.

(Using sane return values is not something dkms did in the past. But it is getting better.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants