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

snp_basic_config: Add snp basic config test case #4224

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

zixi-chen
Copy link
Contributor

@zixi-chen zixi-chen commented Dec 2, 2024

ID: 2863

@zixi-chen zixi-chen force-pushed the snp_basic branch 2 times, most recently from 73ba367 to c184836 Compare December 2, 2024 09:26
@zixi-chen
Copy link
Contributor Author

zixi-chen commented Dec 3, 2024

@XueqiangWei @zhencliu @JinLiul Could you please help to review this?

@XueqiangWei
Copy link
Contributor

@zixi-chen Commit signature has been enabled, please repush the pullreq with -S. Thanks.

@zixi-chen
Copy link
Contributor Author

@zixi-chen Commit signature has been enabled, please repush the pullreq with -S. Thanks.

Done, thanks.

@zixi-chen
Copy link
Contributor Author

Update a retry function to fetch certs from the server.
...
ERROR: Unable to fetch VCEK from URL: 429
Error: Unable to fetch VCEK from URL: 429
Command 'snpguest fetch vcek pem genoa ./ attestation-report.bin' failed. Retry 1/3 in 20s...
The AMD ARK was self-signed!
The AMD ASK was signed by the AMD ARK!
The VCEK was signed by the AMD ASK!
...

@XueqiangWei
Copy link
Contributor

@zixi-chen two commits, only one signed, please check it, thanks.

@zixi-chen zixi-chen force-pushed the snp_basic branch 3 times, most recently from 210e960 to 36fc358 Compare December 18, 2024 10:38
@zixi-chen
Copy link
Contributor Author

zixi-chen commented Dec 19, 2024

@zhenchao
@XueqiangWei
Update the changes based on the comments, Could you please review them again? Thanks!
(1/3) Host_RHEL.m9.u6.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9..0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_default.q35: STARTED
(1/3) Host_RHEL.m9.u6.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9..0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_default.q35: PASS (93.14 s)
(2/3) Host_RHEL.m9.u6.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9..0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_debug.q35: STARTED
(2/3) Host_RHEL.m9.u6.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9..0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_debug.q35: PASS (91.80 s)
(3/3) Host_RHEL.m9.u6.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_singlesocket.q35: STARTED
(3/3) Host_RHEL.m9.u6.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9..0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_singlesocket.q35: CANCEL: Host cpu has more than 1 socket, skip the case. (3.10 s)

@zixi-chen zixi-chen force-pushed the snp_basic branch 2 times, most recently from 37ed748 to c54fe62 Compare December 19, 2024 05:59
@zixi-chen zixi-chen force-pushed the snp_basic branch 2 times, most recently from 7d2c4b3 to 01d25dc Compare December 19, 2024 08:25
zhencliu
zhencliu previously approved these changes Dec 19, 2024
snp_module_path = "/sys/module/kvm_amd/parameters/sev_snp"
module_status = Y y 1
snp_guest_check = "journalctl|grep -i -w snp"
guest_tool_install = "dnf install -y snpguest"
Copy link
Contributor

Choose a reason for hiding this comment

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

Has the snpguest package already been supported in RHEL repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, snpguest package targets 9.6, so don't need to do a workaround here. To run this case, guest image should install snpguest in prior. Thanks.

@JinLiul
Copy link
Contributor

JinLiul commented Dec 30, 2024

(1/3) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_default.q35: STARTED
(1/3) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_default.q35: PASS (88.53 s)
(2/3) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_debug.q35: STARTED
(2/3) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_debug.q35: PASS (88.71 s)
(3/3) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_singlesocket.q35: STARTED
(3/3) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.snp_basic_config.policy_singlesocket.q35: CANCEL: Host cpu has more than 1 socket, skip the case. (2.55 s)
LGTM, thanks.

@XueqiangWei
Copy link
Contributor

@zixi-chen This case is only for linux, right? If yes, we'd better add "only Linux".

if int(process.getoutput(socket_count_cmd, shell=True)) != 1:
test.cancel("Host cpu has more than 1 socket, skip the case.")

vm_name = params["main_vm"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add env_process.preprocess_vm(test, params, env, vm_name) after line 39

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Xueqiang, this is discussed with zhenchao, since I don't change configs during the test, the preprocess can be removed since VT will do it by default at the beginning of the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, make sense.

Signed-off-by: Zixi Chen <zixchen@redhat.com>
Signed-off-by: Zixi Chen <zixchen@redhat.com>
@zixi-chen
Copy link
Contributor Author

@zixi-chen This case is only for linux, right? If yes, we'd better add "only Linux".

@XueqiangWei Updated, tried --guestname=Win11 won't run the case.

Copy link
Contributor

@XueqiangWei XueqiangWei left a comment

Choose a reason for hiding this comment

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

LGTM,
Acked-by: xuwei@redhat.com

@XueqiangWei XueqiangWei merged commit d0cbb78 into autotest:master Jan 9, 2025
7 checks passed
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

Successfully merging this pull request may close these issues.

4 participants