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

using rpm_safe_upgrade correctly? #25

Open
bitness opened this issue Jun 13, 2017 · 11 comments

Comments

4 participants
@bitness
Copy link

commented Jun 13, 2017

Hello,

I have an RPM that uses dkms with --rpm_safe_upgrade in its %post and %preun sections, and it uses them essentially the same way as in dkms's sample.spec:

%post
dkms add -m modname -v %{modver} --rpm_safe_upgrade
dkms build -m modname -v %{modver}
dkms install -m modname -v %{modver}

%preun
dkms remove -m modname -v %{modver} --all --rpm_safe_upgrade

As far as I can tell, though, this doesn't work when upgrading from one release of the package to the next, without the module version changing. dkms add with --rpm_safe_upgrade exits out with a complaint that the module version is already added, but it exits before writing the rpm_safe_upgrade lock file. When dkms remove is called later in the%preun section, it sees no lock file and so uninstalls the module.

Shouldn't add_module() write the rpm_safe_upgrade lock file before exiting due to an existing module with the same version? I hacked my /usr/sbin/dkms to do this and it all worked as expected.

Thanks!
-lars

@yuzaipiaofei

This comment has been minimized.

Copy link
Collaborator

commented Jul 7, 2017

--rpm_safe-upgrade prevents the uninstallation of the driver in case a package with the same version-release is reinstalled.

@yuzaipiaofei

This comment has been minimized.

Copy link
Collaborator

commented Jul 7, 2017

You can refer to dkms package policy.
agile.us.dell.com/Agile

@yuzaipiaofei

This comment has been minimized.

Copy link
Collaborator

commented Nov 14, 2017

If you do not have any more questions about this .i would like to close this case in this week.

@bitness

This comment has been minimized.

Copy link
Author

commented Nov 14, 2017

Hi,

Sorry, I don't see any content at http://agile.us.dell.com/Agile.

The dkms man page says this about rpm_safe_upgrade:

Its use makes sure that when upgrading between different releases of an RPM for the same , DKMS does not do anything dumb

So it's not just intended for uninstallation protection during reinstallation of the same version-release, it's specifically called out as the right thing to do when the module-version hasn't changed, but the release has.

What I've been finding in that circumstance is that the %post script for the new package is run, dkms add -m <module> -v <version> --rpm_safe_upgrade is called, and then dkms exits immediately because the same module-version has already been added (which is true when we're just changing the release). At this point it hasn't written out the rpm_safe_upgrade lock file. Then, in the same RPM transaction, the %preun for the old package is run, and dkms remove -m <module> -v <version> --all --rpm_safe_upgrade gets called. Now, rpm_safe_upgrade doesn't see the lock file since the new package's dkms add never wrote it, so it happily removes the module, and the installation of the new package is therefore broken.

The current behavior of dkms add exiting on a module-version match before writing the rpm_safe_upgrade flag seems broken to me, and as far as I can see doesn't do what the documentation says it should do.

It seems like for dkms to work as advertised with rpm_safe_upgrade, it should write out the rpm_safe_upgrade file before exiting due to module-version match, like in the patch I'm attaching here.

Thanks,
-lars

proposed.patch.txt

@bitness

This comment has been minimized.

Copy link
Author

commented Nov 15, 2017

I cloned my own dkms repo and made a branch to make it easier to see the change, which you can find here.

Thanks!

@bitness

This comment has been minimized.

Copy link
Author

commented Jan 4, 2018

Hello, have you had a chance to look at this patch? I still believe that rpm_safe_upgrade is broken without it, and would love to stop adding a hack to work around it in each of my dkms-based spec files.

Thanks,
-lars

@scaronni

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

I confirm the fix is needed when using --rpm_safe_upgrade. This is the test I made:

I have the following package installed:

$ rpm -q dkms-nvidia
dkms-nvidia-390.12-1.fc27.x86_64

With this content in the %post and %preun sections:

%post
dkms add -m %{dkms_name} -v %{version} --rpm_safe_upgrade || :
# Rebuild and make available for the currently running kernel
dkms build -m %{dkms_name} -v %{version} -q || :
dkms install -m %{dkms_name} -v %{version} -q --force || :

%preun
# Remove all versions from DKMS registry
dkms remove -m %{dkms_name} -v %{version} -q --all --rpm_safe_upgrade || :

Then I roll out a patch for the dkms-nvidia module and the RPM release bumps to 2 (from 1). Without this patch, this is what happens:

$ sudo rpm -Uvh /home/mock/fedora-27-x86_64/result/dkms-nvidia-390.12-2.fc27.x86_64.rpm 
Preparing...                          ################################# [100%]
Updating / installing...
   1:dkms-nvidia-2:390.12-2.fc27      ################################# [ 50%]
Error! DKMS tree already contains: nvidia-390.12
You cannot add the same module/version combo more than once.
Cleaning up / removing...
   2:dkms-nvidia-2:390.12-1.fc27      ################################# [100%]

Basically my modules will not have a patch, which is not the intended behaviour.
With the patch, the upgrade is fine.

@scaronni

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

As a side note to this, wouldn't be clean and less complicated to just use these in the %post and %preun sections and just get rid of --rpm_safe_upgrade entirely?

%post
dkms add -m %{dkms_name} -v %{version} -q || :
# Rebuild and make available for the currently running kernel
dkms build -m %{dkms_name} -v %{version} -q || :
dkms install -m %{dkms_name} -v %{version} -q --force || :

%preun
# Remove all versions from DKMS registry
dkms remove -m %{dkms_name} -v %{version} -q --all || :

You basically got the same result without extra code and extra options.

@scaronni

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

@yuzaipiaofei @bitness what do you think? I'm in favor of entirely removing the --rpm_safe_upgrade parameter and associated code/manpage entry. I can make a PR that also updates the sample.spec file if we agree on this.

@scaronni scaronni self-assigned this Jan 24, 2018

@scaronni scaronni added bug and removed help wanted labels Jan 24, 2018

@bitness

This comment has been minimized.

Copy link
Author

commented Jan 24, 2018

Hi Simone,

After testing the setup you suggest for getting rid of -rpm_safe_upgrade, I still saw the module getting erroneously removed (the old package's %post runs before the new package's %preun).

But what if, instead, we put the dkms add, build, and install commands into %posttrans? That works as I want it to in my testing, without using --rpm-safe-upgrade. The only downside is that the module is needlessly removed and reinstalled on a cross-release update, but that happens so rarely that I don't think it's an issue (for me, anyway).

@tonyhutter tonyhutter referenced this issue Dec 4, 2018

Closed

Fix missing dkms modules after upgrades #8160

1 of 11 tasks complete
@marmarek

This comment has been minimized.

Copy link

commented Jun 5, 2019

Any chance for merging #44 (or alternative fix)? I'm seeing this issue too.

There is also another (similar) issue - the check on "remove" action isn't done, if no kernel is installed - for example in chroot build environment. But that's a minor issue and easy to workaround - simply install some kernel package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.