qemu: Automatically create swtpm device #1834

kalikiana · 2021-10-22T17:18:59Z

Create device if QEMUTPM is set
Recommend swtpm packages

See: https://progress.opensuse.org/issues/101015

backend/qemu.pm

dist/rpm/os-autoinst.spec

backend/qemu.pm

codecov · 2021-10-25T17:22:37Z

Codecov Report

Merging #1834 (7ae93f9) into master (2585008) will increase coverage by 0.10%.
The diff coverage is 63.15%.

@@            Coverage Diff             @@
##           master    #1834      +/-   ##
==========================================
+ Coverage   67.36%   67.47%   +0.10%     
==========================================
  Files          62       62              
  Lines        6877     6887      +10     
==========================================
+ Hits         4633     4647      +14     
+ Misses       2244     2240       -4

Impacted Files	Coverage Δ
backend/qemu.pm	`51.40% <63.15%> (+0.98%)`	⬆️
consoles/virtio_terminal.pm	`98.64% <0.00%> (+1.35%)`	⬆️
backend/driver.pm	`100.00% <0.00%> (+2.77%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2585008...7ae93f9. Read the comment docs.

backend/qemu.pm

kalikiana · 2021-11-08T15:46:22Z

FYI expecting one test failure for the "default" case. I'm guessing I'm missing something obvious but I can't make much sense of the output and I don't see the issue in my code.

Martchus

I suppose it would work but have you conducted a complete test run?

kalikiana · 2021-11-09T15:13:23Z

FYI expecting one test failure for the "default" case. I'm guessing I'm missing something obvious but I can't make much sense of the output and I don't see the issue in my code.

Seems like that was artifacts from debug code I'd forgotten to revert. However I did forget to commit the update-deps changes.
Not sure what's causing the OBS error, though 🤔

error: /home/abuild/rpmbuild/SOURCES/os-autoinst.spec:154: Got a %endif with no %if

I suppose it would work but have you conducted a complete test run?

FYI the test here runs qemu and swtpm, although it doesn't examine the device further. I was going to run the test from poo#101015 but that deleted, and I couldn't find existing tests relying on swtpm. Trying to get that from Richard.

dist/rpm/os-autoinst.spec

dependencies.yaml

okurz · 2021-11-09T20:19:55Z

https://build.opensuse.org/package/show/devel:openQA:TestGithub:OPR-1834/os-autoinst shows that swtpm can not be resolved so this won't work. Do we actually need swtpm for testing? I would prefer if we don't need it at all. If you are just checking the log for what would be called I guess we should be ok :)

kalikiana · 2021-11-10T08:51:04Z

https://build.opensuse.org/package/show/devel:openQA:TestGithub:OPR-1834/os-autoinst shows that swtpm can not be resolved so this won't work. Do we actually need swtpm for testing? I would prefer if we don't need it at all. If you are just checking the log for what would be called I guess we should be ok :)

I think it's rather good to know that it works. And we run a real qemu already

If the benefit is not obvious enough, remember what checking for hypothetical commands got us for image compression

mergify · 2021-11-15T09:35:47Z

This pull request is now in conflicts. Could you fix it? 🙏

kalikiana · 2021-11-15T12:26:44Z

I was confused why setup_tpm needed the re-defined sp and qemu_params_ofw doesn't. After fixing the deprecation message for nowait the test passes w/o it.

okurz

Please rephrase the git commit message subject line to answer the "Why", not the "What". Consider reading the changelog on openqa.opensuse.org/changelog which only shows the git commit message subject line. Think about what you would want to read as a user.

Also, what changes did you apply regarding #1834 (comment) ?

It might be better to do d5b4b9d in a separate PR as changing the commands on the command line might introduce backward incompatible changes. You might fix a warning on Tumbleweed but introduce problems for older Leap or SLE for example.

kalikiana · 2021-11-15T15:28:34Z

Also, what changes did you apply regarding #1834 (comment) ?

I made the dependency Tumbleweed-specific similar to what we do with e.g. opencv. We don't run the test on OBS either way.

dist/rpm/os-autoinst.spec

kalikiana · 2021-11-16T18:30:55Z

I suppose it would work but have you conducted a complete test run?

FYI the test here runs qemu and swtpm, although it doesn't examine the device further. I was going to run the test from poo#101015 but that deleted, and I couldn't find existing tests relying on swtpm. Trying to get that from Richard.

openqa-clone-job --from http://openqa.suse.de --skip-deps --host http://openqa.suse.de 7619588 --skip-download _GROUP=0 CASEDIR="https://github.com/rfan1/os-autoinst-distri-opensuse.git#swtpm_test_only"
Created job #7691337: sle-15-SP4-Online-x86_64-Build61.1-create_swtpm_hdd@64bit -> http://openqa.suse.de/t7691337

Unfortunately it fails like so in thebuild_hdd module:

fpmS7
/tmp/scriptfpmS7.sh: line 1: ll: command not found

@rfan1 Can you confirm if this is the right reproducer?

rfan1 · 2021-11-17T02:15:36Z

I suppose it would work but have you conducted a complete test run?

FYI the test here runs qemu and swtpm, although it doesn't examine the device further. I was going to run the test from poo#101015 but that deleted, and I couldn't find existing tests relying on swtpm. Trying to get that from Richard.
openqa-clone-job --from http://openqa.suse.de --skip-deps --host http://openqa.suse.de 7619588 --skip-download _GROUP=0 CASEDIR="https://github.com/rfan1/os-autoinst-distri-opensuse.git#swtpm_test_only"
Created job #7691337: sle-15-SP4-Online-x86_64-Build61.1-create_swtpm_hdd@64bit -> http://openqa.suse.de/t7691337
Unfortunately it fails like so in thebuild_hdd module:
fpmS7
/tmp/scriptfpmS7.sh: line 1: ll: command not found
@rfan1 Can you confirm if this is the right reproducer?

No problem, let me revise my code and see.

rfan1 · 2021-11-17T04:16:54Z

I suppose it would work but have you conducted a complete test run?

FYI the test here runs qemu and swtpm, although it doesn't examine the device further. I was going to run the test from poo#101015 but that deleted, and I couldn't find existing tests relying on swtpm. Trying to get that from Richard.
openqa-clone-job --from http://openqa.suse.de --skip-deps --host http://openqa.suse.de 7619588 --skip-download _GROUP=0 CASEDIR="https://github.com/rfan1/os-autoinst-distri-opensuse.git#swtpm_test_only"
Created job #7691337: sle-15-SP4-Online-x86_64-Build61.1-create_swtpm_hdd@64bit -> http://openqa.suse.de/t7691337
Unfortunately it fails like so in thebuild_hdd module:
fpmS7
/tmp/scriptfpmS7.sh: line 1: ll: command not found
@rfan1 Can you confirm if this is the right reproducer?
No problem, let me revise my code and see.

@kalikiana you can start a new job with the below one and add needed "swtpm" configuration: :)

openqa-clone-job --from http://openqa.suse.de --skip-deps --host http://openqa.suse.de 7656835 --skip-download _GROUP=0 CASEDIR="https://github.com/rfan1/os-autoinst-distri-opensuse.git#swtpm_test_only"

kalikiana · 2021-11-25T17:34:13Z

openqa-clone-job --from http://openqa.suse.de --skip-deps --host http://openqa.suse.de 7656835 --skip-download _GROUP=0 CASEDIR="https://github.com/rfan1/os-autoinst-distri-opensuse.git#swtpm_test_only"

That seems to work, see https://progress.opensuse.org/issues/101015#note-15

- Create device if QEMUTPM is set - Recommend swtpm packages See: https://progress.opensuse.org/issues/101015

kalikiana · 2021-11-26T08:21:28Z

The test is not fully covered because of the if ($arch eq 'aarch64') { handling... this is not new code, though, I moved it into a new function, so I wonder why it wasn't covered before 🤔

okurz

so I wonder why [this code] wasn't covered before thinking

Because automated tests did not exist before 2021 maybe ;P

Martchus · 2021-11-26T09:56:15Z

It is just that it looks like this branch is actually covered. Maybe the coverage (of the forked process) isn't collected.

okurz · 2021-11-27T10:04:16Z

@kalikiana why did you merge now? I am ok if you merge and state why you want to overrule checks. IMHO would have been a good opportunity to increase test coverage beyond the patch coverage check limit

okurz requested changes Oct 22, 2021

View reviewed changes

backend/qemu.pm Outdated Show resolved Hide resolved

dist/rpm/os-autoinst.spec Outdated Show resolved Hide resolved

Martchus requested changes Oct 25, 2021

View reviewed changes

backend/qemu.pm Outdated Show resolved Hide resolved

backend/qemu.pm Outdated Show resolved Hide resolved

kalikiana force-pushed the qemutpm_device branch 3 times, most recently from 9bd83cd to 813154f Compare October 25, 2021 16:01

kalikiana marked this pull request as ready for review October 25, 2021 16:02

kalikiana force-pushed the qemutpm_device branch from 813154f to 0f98be1 Compare October 25, 2021 17:09

okurz requested changes Oct 27, 2021

View reviewed changes

backend/qemu.pm Outdated Show resolved Hide resolved

kalikiana force-pushed the qemutpm_device branch from 0f98be1 to 54d92ec Compare November 8, 2021 15:44

Martchus reviewed Nov 8, 2021

View reviewed changes

okurz approved these changes Nov 8, 2021

View reviewed changes

perlpunk reviewed Nov 9, 2021

View reviewed changes

dist/rpm/os-autoinst.spec Outdated Show resolved Hide resolved

perlpunk reviewed Nov 9, 2021

View reviewed changes

dependencies.yaml Outdated Show resolved Hide resolved

kalikiana force-pushed the qemutpm_device branch from 54d92ec to dc3864b Compare November 9, 2021 16:53

kalikiana force-pushed the qemutpm_device branch from dc3864b to d5b4b9d Compare November 15, 2021 12:24

okurz requested changes Nov 15, 2021

View reviewed changes

kalikiana mentioned this pull request Nov 15, 2021

qemu: Use wait=off instead of the deprecated nowait #1853

Closed

kalikiana force-pushed the qemutpm_device branch from d5b4b9d to 15851c2 Compare November 15, 2021 13:08

okurz requested changes Nov 15, 2021

View reviewed changes

dist/rpm/os-autoinst.spec Outdated Show resolved Hide resolved

kalikiana force-pushed the qemutpm_device branch 2 times, most recently from af02059 to f42df28 Compare November 16, 2021 18:05

qemu: Automatically create swtpm device

7ae93f9

- Create device if QEMUTPM is set - Recommend swtpm packages See: https://progress.opensuse.org/issues/101015

kalikiana force-pushed the qemutpm_device branch from f42df28 to 7ae93f9 Compare November 25, 2021 17:34

okurz approved these changes Nov 26, 2021

View reviewed changes

kalikiana requested a review from Martchus November 26, 2021 09:16

Martchus approved these changes Nov 26, 2021

View reviewed changes

kalikiana merged commit 755f4e5 into os-autoinst:master Nov 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qemu: Automatically create swtpm device #1834

qemu: Automatically create swtpm device #1834

kalikiana commented Oct 22, 2021 •

edited

codecov bot commented Oct 25, 2021 •

edited

kalikiana commented Nov 8, 2021

Martchus left a comment

kalikiana commented Nov 9, 2021

okurz commented Nov 9, 2021

kalikiana commented Nov 10, 2021 •

edited

mergify bot commented Nov 15, 2021

kalikiana commented Nov 15, 2021

okurz left a comment •

edited

kalikiana commented Nov 15, 2021

kalikiana commented Nov 16, 2021 •

edited

rfan1 commented Nov 17, 2021

rfan1 commented Nov 17, 2021 •

edited by kalikiana

kalikiana commented Nov 25, 2021

kalikiana commented Nov 26, 2021

okurz left a comment

Martchus commented Nov 26, 2021

okurz commented Nov 27, 2021

qemu: Automatically create swtpm device #1834

qemu: Automatically create swtpm device #1834

Conversation

kalikiana commented Oct 22, 2021 • edited

codecov bot commented Oct 25, 2021 • edited

Codecov Report

kalikiana commented Nov 8, 2021

Martchus left a comment

Choose a reason for hiding this comment

kalikiana commented Nov 9, 2021

okurz commented Nov 9, 2021

kalikiana commented Nov 10, 2021 • edited

mergify bot commented Nov 15, 2021

kalikiana commented Nov 15, 2021

okurz left a comment • edited

Choose a reason for hiding this comment

kalikiana commented Nov 15, 2021

kalikiana commented Nov 16, 2021 • edited

rfan1 commented Nov 17, 2021

rfan1 commented Nov 17, 2021 • edited by kalikiana

kalikiana commented Nov 25, 2021

kalikiana commented Nov 26, 2021

okurz left a comment

Choose a reason for hiding this comment

Martchus commented Nov 26, 2021

okurz commented Nov 27, 2021

kalikiana commented Oct 22, 2021 •

edited

codecov bot commented Oct 25, 2021 •

edited

kalikiana commented Nov 10, 2021 •

edited

okurz left a comment •

edited

kalikiana commented Nov 16, 2021 •

edited

rfan1 commented Nov 17, 2021 •

edited by kalikiana