search

oVirt / vdsm

The Virtual Desktop Server Manager
GNU General Public License v2.0
161 stars 202 forks source link

Remove SPDX header from lvmlocal.conf #331

Closed mz-pdm closed 2 years ago

mz-pdm commented 2 years ago

It is suspected to cause OST failure on el9stream.

mz-pdm commented 2 years ago

/ost basic-suite-master el9stream

aesteve-rh commented 2 years ago

It would be better to add a .license file and no remove the reuse linter:

contrib/add-spdx-header.sh --explicit-license static/usr/share/vdsm/lvmlocal.conf

That should create a static/usr/share/vdsm/lvmlocal.conf.license file with the SPDX header, and keep the reuse linter happy.

mz-pdm commented 2 years ago

Thanks for the advice. At this point, I'm trying to find out whether this actually causes the OST failure or not. I guess it doesn't but it's hard to tell due to general OST unreliability. We need more runs with different Vdsm versions to get some at least partially convincing statistics.

mz-pdm commented 2 years ago

Not needed, something else is broken.

nirs commented 2 years ago

It is suspected to cause OST failure on el9stream.

Removing license from file and disabling reuse check is a bit too much without evidence. The only code accessing the comments is in vdsm and it does not depend on system version.

I added a simple test in #332 that runs on all tested environment to verify that the builtin lvmlocal.conf can be parsed by vdsm tool.

Vdsm tests do not run only on centos stream 8 now. If we want to support centos stream 9 the first step would be to add centos stream 9 CI.

mz-pdm commented 2 years ago

Please note this PR was just a draft to test OST behavior and to see whether a quick fix was needed for OST (posting a PR here is the easiest way to run OST), not a request for real reviews. Apparently people don't notice the draft status, I'll add a note to the PR title next time.