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

el8toel9: Warn about deprecated Xorg drivers #1078

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

ofourdan
Copy link
Contributor

@ofourdan ofourdan commented May 5, 2023

Some Xorg drivers have been deprecated in favor of the "modesetting" driver.

If Xorg is configured to use those drivers, it may not be able to work properly after the upgrade.

Add a new actor to check in the journal whether such Xorg drivers are in use and in that case, also warn if there are custom Xorg config options.

Known limitation: This actor checks the journal logs since the last boot, meaning that if Xorg was started manually from a console or if the system has been rebooted after a graphical Xorg session was used, the actor will not be able to detect the use of deprecated drivers.

@github-actions
Copy link

github-actions bot commented May 5, 2023

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please @oamg/developers to notify leapp developers of the review request
  • /packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. If you need a different version of leapp from PR#42, use /packit test oamg/leapp#42

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and latest upstream leapp build as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp*PR42* as artifacts
  • /rerun-sst to schedule sst tests using this pr build and latest upstream leapp build as artifacts
  • /rerun-sst 42 to schedule sst tests using this pr build and leapp*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

@ofourdan ofourdan force-pushed the xorg-deprecated-drivers branch from 798386e to 2d8c2e6 Compare May 5, 2023 09:53
@fernflower
Copy link
Member

/packit test

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ofourdan Thank you for the contribution. I found several problems that are describe in below with explanations. But I am also curious, how the manual removal of these drivers makes the graphical session working after the upgrade? And are these drivers deprecated or removed on RHEL 9? In case they are removed, than only thing that happened during the upgrade is that packages with these drivers will be removed. What is then difference from the manual removal? Is it than enough to just uninstall such drivers?

@ofourdan
Copy link
Contributor Author

ofourdan commented May 9, 2023

[…]But I am also curious, how the manual removal of these drivers makes the graphical session working after the upgrade?

The modesetting driver should be used instead.

And are these drivers deprecated or removed on RHEL 9?

They are removed.

In case they are removed, than only thing that happened during the upgrade is that packages with these drivers will be removed. What is then difference from the manual removal? Is it than enough to just uninstall such drivers?

This is mostly about warning users/sysadmins.

By default, the modesetting driver would be used, but in el8 we would still ship the various hardware specific Xorg drivers and people could install them and add an entry in xorg.conf to use them, sometimes even with driver specific options.

In that case, it's up to the user/sysadmin to remove the driver (and the relevant xorg,conf entries) prior to upgrade, because Xorg will refuse to start if the xorg.conf refers to a driver which is not available.

@pirat89
Copy link
Member

pirat89 commented May 10, 2023

[…]But I am also curious, how the manual removal of these drivers makes the graphical session working after the upgrade?

The modesetting driver should be used instead.

And are these drivers deprecated or removed on RHEL 9?

They are removed.

In case they are removed, than only thing that happened during the upgrade is that packages with these drivers will be removed. What is then difference from the manual removal? Is it than enough to just uninstall such drivers?

This is mostly about warning users/sysadmins.

By default, the modesetting driver would be used, but in el8 we would still ship the various hardware specific Xorg drivers and people could install them and add an entry in xorg.conf to use them, sometimes even with driver specific options.

In that case, it's up to the user/sysadmin to remove the driver (and the relevant xorg,conf entries) prior to upgrade, because Xorg will refuse to start if the xorg.conf refers to a driver which is not available.

I see. But in such a case the instructions in the report are missleading as you do not want from user just a driver removal, you want them also to update the related configuration file. IIUC

@ofourdan
Copy link
Contributor Author

I see. But in such a case the instructions in the report are missleading as you do not want from user just a driver removal, you want them also to update the related configuration file. IIUC

Yes, I'm working on it, and the refactoring :)

@ofourdan ofourdan force-pushed the xorg-deprecated-drivers branch 4 times, most recently from a36ecef to cc8f787 Compare May 17, 2023 14:03
@ofourdan ofourdan force-pushed the xorg-deprecated-drivers branch from cc8f787 to f3dd837 Compare May 22, 2023 12:58
@pirat89
Copy link
Member

pirat89 commented May 22, 2023

/packit build

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ofourdan seems good to me. Would you mind to add some unit tests for our peace of mind? Otherwise it seems good.

@ofourdan ofourdan force-pushed the xorg-deprecated-drivers branch 3 times, most recently from 3bf569a to de6d004 Compare May 23, 2023 15:26
@ofourdan
Copy link
Contributor Author

@ofourdan seems good to me. Would you mind to add some unit tests for our peace of mind? Otherwise it seems good.

Thanks! I added some unit tests for the report. For the fact/check, it's not so simple as it basically gets the data from the journal logs.

@pirat89
Copy link
Member

pirat89 commented May 29, 2023

@ofourdan thanks! In case of journal log, I think it's ok to just mock the run function to return content of some files - or just strings directly. E.g. like here:

It's not even needed to check exact the command parameters when it's not created dynamically and there are not multiple shell commands called in a tested function. In that particular example, it present also reading from files stored under ...../tests/files/ - see CUR_DIR lines. If you share some (crafted) examples of expected outputs, we can help you with the unit tests for the facts scanner.

I am sorry for late responses, we have couple of priority things on the plate last weeks and today we got anothers. I expect we will get to this PR during Thu/Fri this week.

@ofourdan ofourdan force-pushed the xorg-deprecated-drivers branch 3 times, most recently from 3c7024e to 4c6cc9d Compare May 31, 2023 07:24
Some Xorg drivers have been deprecated in favor of the "modesetting"
driver.

If Xorg is configured to use those driver, it may not be able to work
properly after the upgrade.

Add a new actor to check in the journal whether such Xorg drivers are in
use and in that case, also warn if there are custom Xorg config options.

Known limitation: This actor checks the journal logs since the last
boot, meaning that if Xorg was started manually from a console or if the
system has been rebooted after a graphical Xorg session was used, the
actor will not be able to detect the use of deprecated drivers.

Signed-off-by: Olivier Fourdan <[email protected]>
@ofourdan ofourdan force-pushed the xorg-deprecated-drivers branch from 4c6cc9d to d958eab Compare June 5, 2023 11:26
@ofourdan
Copy link
Contributor Author

ofourdan commented Jun 5, 2023

Added the Xorg log files to the codespell's list of skipped files.

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. thanks for your work! waiting for the test results. Btw, in case of failures some of upgrades tests (testing farm), it could be caused by some infrastructure issues. we will check them once they are present in case they are of failures.

@pirat89
Copy link
Member

pirat89 commented Jun 6, 2023

/packit test

@ofourdan
Copy link
Contributor Author

Hi Petr, anything else left for me to do for this?

@pirat89
Copy link
Member

pirat89 commented Jun 19, 2023

@ofourdan Hi \o sorry for the late response. seems that nothing else is needed. I will rerun the regression tests one more time. Hopefully the infrastructure is working still

@pirat89
Copy link
Member

pirat89 commented Jun 19, 2023

/packit test

@pirat89
Copy link
Member

pirat89 commented Jun 19, 2023

Checking the current test results (somewhere around 50% finished, for a reason, there are no capacities in testing farm) everything seems good -> merging. @ofourdan thanks for your work!!

@pirat89 pirat89 merged commit 948c782 into oamg:master Jun 19, 2023
@pirat89 pirat89 added this to the 8.9/9.3 milestone Jun 19, 2023
@pirat89 pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Jun 19, 2023
@ofourdan
Copy link
Contributor Author

Woohoo! Thanks!

pirat89 added a commit to pirat89/leapp-repository that referenced this pull request Aug 23, 2023
## Packaging
- Requires leapp-framework 5.0

## Upgrade handling
### Fixes
- Add el8toel9 actor to handle directory -> symlink with ruby IRB. (oamg#1076)
- Do not try to update GRUB core on IBM Z systems (oamg#1117)
- Fix failing upgrades with devtmpfs file systems specified in FSTAB (oamg#1090)
- Fix the calculation of the required free space on each partitions/volume for the upgrade transactions (oamg#1097)
- Fix the generation of the report about hybrid images (oamg#1064)
- Handle correctly the installed certificates to allow upgrades with custom repositories using HTTPs with enabled SSL verification (oamg#1106)
- Minor improvements and fixes of various reports (oamg#1066, oamg#1067, oamg#1085)
- Update error messages about leapp data files to inform user how to obtain valid data files (oamg#1121)
- Update links in various reports (oamg#1062, oamg#1086)
- Update the repomap data to cover changed repoids in RHUI Azure (oamg#1087)
- [IPU 7 -> 8] Fix false positive report about invalid symlinks on RHEL 7 (oamg#1052)
- [IPU 8 -> 9] Inhibit the upgrade when unsupported x86-64 microarchitecture is detected (oamg#1059)

### Enhancements
- Include updated leapp data files in the RPM (oamg#1046, oamg#1092, oamg#1119)
- Update the set of supported upgrade paths (oamg#1077):
  - RHEL with SAP HANA 7.9 -> 8.6, 8.8 (default: 8.6)
  - RHEL with SAP HANA 8.8 -> 9.2
- Introduce new upgrade paths:
  - RHEL 7.9 -> 8.9 (default)
  - RHEL 8.9 -> 9.3
- Correctly update grub2 when /boot resides on multiple devices aggregated in RAID (oamg#1093, oamg#1115)
- Enable upgrades for machines using RHUI on AlibabaCloud (oamg#1088)
- Introduce possibility to add kernel drivers to initramfs (oamg#1081)
- Redesign handling of information about kernel (booted and target) in preparation for new changes in RHEL 9 (oamg#1107)
- Redesign source system overlay to use disk images backed by sparse files to optimize disk space consumption (oamg#1097, oamg#1103)
- Requires leapp-framework 5.0 (oamg#1061, oamg#1116)
- Use new leapp CLI API which provides better report summary output (oamg#1061, oamg#1116)
- [IPU 8 -> 9] Detect and report use of deprecated Xorg drivers (oamg#1078)
- [IPU 8 -> 9] Introduce IPU for systems with FIPS enabled (oamg#1053)

## Additional changes interesting for devels
- Deprecated `GrubInfo.orig_device_name` field in the `GrubInfo` model (replaced by `GrubInfo.orig_devices`) (oamg#1093)
- Deprecated `InstalledTargetKernelVersion` model (replaced by `InstalledTargetKernelInfo`) (oamg#1107)
- Deprecated `leapp.libraries.common.config.version.is_rhel_realtime` (check the type in msg `KernelInfo`, field `type`) (oamg#1107)
- Deprecated `leapp.libraries.common.grub.get_grub_device()` (replaced by `leapp.libraries.common.grub.get_grub_devices()`) (oamg#1093)
- Introduced new devel envar LEAPP_DEVEL_KEEP_DISK_IMGS=1 to skip the removal of the created disk images for OVL. That's sometimes handy for the debugging. (oamg#1097)
@pirat89 pirat89 mentioned this pull request Aug 23, 2023
Rezney pushed a commit that referenced this pull request Aug 23, 2023
## Packaging
- Requires leapp-framework 5.0

## Upgrade handling
### Fixes
- Add el8toel9 actor to handle directory -> symlink with ruby IRB. (#1076)
- Do not try to update GRUB core on IBM Z systems (#1117)
- Fix failing upgrades with devtmpfs file systems specified in FSTAB (#1090)
- Fix the calculation of the required free space on each partitions/volume for the upgrade transactions (#1097)
- Fix the generation of the report about hybrid images (#1064)
- Handle correctly the installed certificates to allow upgrades with custom repositories using HTTPs with enabled SSL verification (#1106)
- Minor improvements and fixes of various reports (#1066, #1067, #1085)
- Update error messages about leapp data files to inform user how to obtain valid data files (#1121)
- Update links in various reports (#1062, #1086)
- Update the repomap data to cover changed repoids in RHUI Azure (#1087)
- [IPU 7 -> 8] Fix false positive report about invalid symlinks on RHEL 7 (#1052)
- [IPU 8 -> 9] Inhibit the upgrade when unsupported x86-64 microarchitecture is detected (#1059)

### Enhancements
- Include updated leapp data files in the RPM (#1046, #1092, #1119)
- Update the set of supported upgrade paths (#1077):
  - RHEL with SAP HANA 7.9 -> 8.6, 8.8 (default: 8.6)
  - RHEL with SAP HANA 8.8 -> 9.2
- Introduce new upgrade paths:
  - RHEL 7.9 -> 8.9 (default)
  - RHEL 8.9 -> 9.3
- Correctly update grub2 when /boot resides on multiple devices aggregated in RAID (#1093, #1115)
- Enable upgrades for machines using RHUI on AlibabaCloud (#1088)
- Introduce possibility to add kernel drivers to initramfs (#1081)
- Redesign handling of information about kernel (booted and target) in preparation for new changes in RHEL 9 (#1107)
- Redesign source system overlay to use disk images backed by sparse files to optimize disk space consumption (#1097, #1103)
- Requires leapp-framework 5.0 (#1061, #1116)
- Use new leapp CLI API which provides better report summary output (#1061, #1116)
- [IPU 8 -> 9] Detect and report use of deprecated Xorg drivers (#1078)
- [IPU 8 -> 9] Introduce IPU for systems with FIPS enabled (#1053)

## Additional changes interesting for devels
- Deprecated `GrubInfo.orig_device_name` field in the `GrubInfo` model (replaced by `GrubInfo.orig_devices`) (#1093)
- Deprecated `InstalledTargetKernelVersion` model (replaced by `InstalledTargetKernelInfo`) (#1107)
- Deprecated `leapp.libraries.common.config.version.is_rhel_realtime` (check the type in msg `KernelInfo`, field `type`) (#1107)
- Deprecated `leapp.libraries.common.grub.get_grub_device()` (replaced by `leapp.libraries.common.grub.get_grub_devices()`) (#1093)
- Introduced new devel envar LEAPP_DEVEL_KEEP_DISK_IMGS=1 to skip the removal of the created disk images for OVL. That's sometimes handy for the debugging. (#1097)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants