skip the processing of pflash vars file when it's not set #3769

XueqiangWei · 2023-09-22T03:24:23Z

For OVMF with SEV-ES support and OVMF with TDX support, the vm can be booted without vars file. Add a workaround, skip the processing of pflash vars file when ovmf_vars_files =.

Signed-off-by: Xueqiang Wei [email protected]
ID: 1390

XueqiangWei · 2023-09-22T06:01:23Z

Tested with the code file and vars file, the results were passed.

(1/2) Host_RHEL.m9.u4.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_local.q35: STARTED
(1/2) Host_RHEL.m9.u4.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_local.q35: PASS (157.80 s)
(2/2) Host_RHEL.m9.u4.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_remote.q35: STARTED
(2/2) Host_RHEL.m9.u4.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_remote.q35: PASS (187.69 s)

(1/6) Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x8
6_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_insta
ll.aio_threads.q35: STARTED
(1/6) Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x8
6_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_insta
ll.aio_threads.q35: PASS (1117.93 s)
(2/6) Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x8
6_64.io-github-autotest-qemu.boot.q35: STARTED
(2/6) Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x8
6_64.io-github-autotest-qemu.boot.q35: PASS (72.27 s)
(3/6) signed.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(3/6) signed.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (1498.82 s)
(4/6) signed.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(4/6) signed.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (1767.79 s)
(5/6) unsigned.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(5/6) unsigned.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (1441.90 s)
(6/6) unsigned.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(6/6) unsigned.Host_RHEL.m9.u4.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (2220.95 s)

Tested with code file and without vars file, the result was passed.
(1/1) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.boot.q35: STARTED
(1/1) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.boot.q35: PASS (101.67 s)

ovmf_path = /usr/share/edk2/ovmf
ovmf_code_filename = OVMF.inteltdx.fd
ovmf_vars_filename =

qemu command line:
-blockdev '{"node-name": "file_ovmf_code", "driver": "file", "filename": "/usr/share/edk2/ovmf/OVMF.inteltdx.fd", "auto-read-only": true, "discard": "unmap"}'
-blockdev '{"node-name": "drive_ovmf_code", "driver": "raw", "read-only": true, "file": "file_ovmf_code"}'
-machine q35,pflash0=drive_ovmf_code,memory-backend=mem-machine_mem \

ovmf_path = /usr/share/edk2/ovmf
ovmf_code_filename = OVMF.amdsev.fd
ovmf_vars_filename =

-blockdev '{"node-name": "file_ovmf_code", "driver": "file", "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd", "auto-read-only": true, "discard": "unmap"}'
-blockdev '{"node-name": "drive_ovmf_code", "driver": "raw", "read-only": true, "file": "file_ovmf_code"}'
-machine q35,pflash0=drive_ovmf_code,memory-backend=mem-machine_mem \

XueqiangWei · 2023-09-22T06:04:05Z

@zhencliu @YongxueHong @zixi-chen As we discussed before, add the workaround, could you please help review it? Many thanks.

zixi-chen · 2023-09-27T07:40:58Z

Test with sev kernelHashes install case pass.
profile:
[after_headers]
mem = 4096
vm_secure_guest_type = sev
vm_sev_cbitpos = 51
vm_sev_reduced_phys_bits = 1
vm_sev_policy = 7
vm_sev_kernel_hashes = on
ovmf:
ovmf_path = /usr/share/edk2/ovmf
ovmf_code_filename = OVMF.amdsev.fd
ovmf_vars_filename =

(1/2) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
(1/2) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (773.51 s)

zhenyzha · 2023-09-27T07:57:51Z

LGTM.Acked-by: Zhenyu Zhang [email protected]

(1/1) Host_RHEL.m9.u4.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.aarch64.page_4k.io-github-autotest-qemu.boot.arm64-pci: PASS (80.99 s)

@PaulYuuu Could you pls help review when you are free. thanks.

PaulYuuu · 2023-10-10T03:23:19Z

For this workaround, I am thinking about more. Actually, code file and image name have no relevance, but we have

avocado-vt/virttest/qemu_devices/qcontainer.py

Lines 1084 to 1085 in 5e7f791

    
           firmware_path = params.get(firmware_name + "_path") 
        
           if firmware_path and images:

So if possible, make all of them can be added separately, this makes pflash more flexible.

if firmware_path:
    ...
    ...
    if pflash_code_filename:
        ...
        ...

@YongxueHong @zhencliu @XueqiangWei, what do you think?
Thanks, @PaulYuuu, looks good to me.
And I think we could decouple the previous design that decouples the vars and code parts in the pflash_handler which make the code looks clean and better to support other firmware type in the future.
for example:

        def _pflash_code_handler():
            ...

        def _pflash_var_handler():
            ...
        
        def _pflash_xxx_handler():
            ...

        def pflash_handler(firmware_name, machine_params):
            ...
            if pflash_code_filename:
                _pflash_code_handler()
            ...
            if pflash_vars_filename:
                _pflash_var_handler()
            ...
            
            if  pflash_xxx_filename:
                _pflash_xxx_handler()
            ...

Hi @XueqiangWei @zhenyzha @PaulYuuu
Please let me know what is your opinion. Thanks.

XueqiangWei · 2023-10-16T03:09:51Z

@PaulYuuu @YongxueHong As we discussed, this patch just handle "skip the processing of pflash vars file when it's not set". The next phase, I will refactoring and optimizing this part of the code.
Could you please help double check this patch? Thanks.

YongxueHong · 2023-10-19T05:34:23Z

virttest/qemu_devices/qcontainer.py

+                    # To ignore the influence from backends
+                    first_image = images[0]
+                    img_params = params.object_params(first_image)
+                    img_params["backing_chain"] = "no"
+                    img_obj = qemu_storage.QemuImg(img_params,
+                                                   current_data_dir,
+                                                   first_image)
+                    img_info = json.loads(img_obj.info(True, "json"))


According to the previous logic, the definition of this part should be independence, we should move it out of the block if pflash_vars_filename:

Updated, thanks.

XueqiangWei · 2023-10-19T17:46:53Z

Updated and retested with the code file and vars file, the results were passed.

(1/2) Host_RHEL.m9.u3.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_local.q35: STARTED
(1/2) Host_RHEL.m9.u3.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_local.q35: PASS (298.94 s)
(2/2) Host_RHEL.m9.u3.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_remote.q35: STARTED
(2/2) Host_RHEL.m9.u3.ovmf.ceph.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.remote_image_snapshot.to_remote.q35: PASS (193.98 s)
(1/6) Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-aut
otest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
(1/6) Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (597.23 s)
(2/6) Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.boot.q35: STARTED
(2/6) Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.boot.q35: PASS (91.76 s)
(3/6) signed.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(3/6) signed.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (966.48 s)
(4/6) signed.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(4/6) signed.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (1247.17 s)
(5/6) unsigned.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(5/6) unsigned.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (1011.46 s)
(6/6) unsigned.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: STARTED
(6/6) unsigned.Host_RHEL.m9.u3.v0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.uefi_secureboot.q35: PASS (1210.30 s)

Tested with code file and without vars file, the result was passed.
(1/1) Host_RHEL.m9.u3.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.boot.q35: STARTED
(1/1) Host_RHEL.m9.u3.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.boot.q35: PASS (37.64 s)

ovmf_path = /usr/share/edk2/ovmf
ovmf_code_filename = OVMF.inteltdx.fd
ovmf_vars_filename =

qemu command line:
-blockdev '{"node-name": "file_ovmf_code", "driver": "file", "filename": "/usr/share/edk2/ovmf/OVMF.inteltdx.fd", "auto-read-only": true, "discard": "unmap"}'
-blockdev '{"node-name": "drive_ovmf_code", "driver": "raw", "read-only": true, "file": "file_ovmf_code"}'
-machine q35,pflash0=drive_ovmf_code,memory-backend=mem-machine_mem \

ovmf_path = /usr/share/edk2/ovmf
ovmf_code_filename = OVMF.amdsev.fd
ovmf_vars_filename =

-blockdev '{"node-name": "file_ovmf_code", "driver": "file", "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd", "auto-read-only": true, "discard": "unmap"}'
-blockdev '{"node-name": "drive_ovmf_code", "driver": "raw", "read-only": true, "file": "file_ovmf_code"}'
-machine q35,pflash0=drive_ovmf_code,memory-backend=mem-machine_mem \

@YongxueHong Could you please help review it again? Many thanks.

YongxueHong

LGTM.

YongxueHong · 2023-10-23T02:51:11Z

Hi @PaulYuuu , @zhencliu , @zixi-chen , @zhenyzha
Would you like to have a check since there is a little updating?
Thanks.

For OVMF with SEV-ES support and OVMF with TDX support, the vm can be booted without vars file. Add a workaround, skip the processing of pflash vars file when ovmf_vars_files =. Signed-off-by: Xueqiang Wei <[email protected]>

XueqiangWei · 2023-10-25T03:01:00Z

According to Zhenchao's feedback, update the commit message, add "qcontainer:" in commit header. Thanks.

zhenyzha · 2023-10-25T03:05:22Z

ack

 (1/2) Host_RHEL.m9.u4.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.aarch64.page_4k.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.arm64-pci: STARTED
 (1/2) Host_RHEL.m9.u4.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.aarch64.page_4k.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.arm64-pci: PASS (543.16 s)
 (2/2) Host_RHEL.m9.u4.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.aarch64.page_4k.io-github-autotest-qemu.boot.arm64-pci: STARTED
 (2/2) Host_RHEL.m9.u4.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.aarch64.page_4k.io-github-autotest-qemu.boot.arm64-pci: PASS (40.67 s)

zixi-chen · 2023-10-25T05:40:03Z

@XueqiangWei I verified this with sev, it looks good to me.
profile:
[after_headers]
ovmf:
ovmf_path = /usr/share/edk2/ovmf
ovmf_code_filename = OVMF.amdsev.fd
ovmf_vars_filename =

python3 ConfigTest.py --testcase=sev_dhcert_boot --guestname=RHEL.9.4.0 --platform=x86_64 --clone=no --profile=profile

(1/1) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.sev_dhcert_boot.q35: PASS (80.19 s)

YongxueHong · 2023-10-30T02:13:42Z

Thanks all, let's merge it.

zhenyzha approved these changes Sep 27, 2023

View reviewed changes

YongxueHong reviewed Oct 19, 2023

View reviewed changes

XueqiangWei force-pushed the skip_handling_vars_file branch 2 times, most recently from ef19fb1 to 74d10c8 Compare October 19, 2023 16:58

YongxueHong approved these changes Oct 23, 2023

View reviewed changes

zhencliu approved these changes Oct 25, 2023

View reviewed changes

XueqiangWei force-pushed the skip_handling_vars_file branch from 74d10c8 to 838a212 Compare October 25, 2023 02:57

PaulYuuu approved these changes Oct 25, 2023

View reviewed changes

YongxueHong merged commit 291ae9f into avocado-framework:master Oct 30, 2023
49 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

skip the processing of pflash vars file when it's not set #3769

skip the processing of pflash vars file when it's not set #3769

XueqiangWei commented Sep 22, 2023

XueqiangWei commented Sep 22, 2023

XueqiangWei commented Sep 22, 2023

zixi-chen commented Sep 27, 2023

zhenyzha commented Sep 27, 2023

PaulYuuu commented Oct 10, 2023 •

edited by YongxueHong

Loading

XueqiangWei commented Oct 16, 2023

YongxueHong Oct 19, 2023

XueqiangWei Oct 19, 2023

XueqiangWei commented Oct 19, 2023

YongxueHong left a comment

YongxueHong commented Oct 23, 2023

XueqiangWei commented Oct 25, 2023

zhenyzha commented Oct 25, 2023

zixi-chen commented Oct 25, 2023 •

edited

Loading

YongxueHong commented Oct 30, 2023

skip the processing of pflash vars file when it's not set #3769

skip the processing of pflash vars file when it's not set #3769

Conversation

XueqiangWei commented Sep 22, 2023

XueqiangWei commented Sep 22, 2023

XueqiangWei commented Sep 22, 2023

zixi-chen commented Sep 27, 2023

zhenyzha commented Sep 27, 2023

PaulYuuu commented Oct 10, 2023 • edited by YongxueHong Loading

XueqiangWei commented Oct 16, 2023

YongxueHong Oct 19, 2023

Choose a reason for hiding this comment

XueqiangWei Oct 19, 2023

Choose a reason for hiding this comment

XueqiangWei commented Oct 19, 2023

YongxueHong left a comment

Choose a reason for hiding this comment

YongxueHong commented Oct 23, 2023

XueqiangWei commented Oct 25, 2023

zhenyzha commented Oct 25, 2023

zixi-chen commented Oct 25, 2023 • edited Loading

python3 ConfigTest.py --testcase=sev_dhcert_boot --guestname=RHEL.9.4.0 --platform=x86_64 --clone=no --profile=profile

YongxueHong commented Oct 30, 2023

PaulYuuu commented Oct 10, 2023 •

edited by YongxueHong

Loading

zixi-chen commented Oct 25, 2023 •

edited

Loading