UEFI libvirt servers can't boot on Ubuntu 20.04 hypervisors with Ussuri/Victoria

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/828978

Change abandoned by "Mauricio Faria de Oliveira <email address hidden>" on branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/828978
Reason: resubmit

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/828979

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/828980

https://review.opendev.org/c/openstack/nova/+/828979 # victoria
https://review.opendev.org/c/openstack/nova/+/828980 # ussuri

Technical Restriction:
---

OVMF_CODE.secboot.fd is built w/ SMM_REQUIRE, so Secure Boot is actually secure.

It's possible to build w/ just SMM_ENABLE, but then it's not really Secure Boot
(thus this type usually isn't distributed, but the SB+SMM "OVMF_CODE.secboot.fd").

The requirement is SMM emulation from q35 machine (pc machine: too small SMRAM).

References:
---

- [2015] Securing secure boot: system management mode in KVM and Tiano Core by Paolo Bonzini

 """
 Securing OVMF
 ...
 Target: Q35 (440FX SMRAM too small)
 """

 @ https://youtu.be/IxLvxP1O8T8?t=945 (15:45)
 @ http://www.linux-kvm.org/images/c/ce/03x06-Aspen-Paolo_Bonzini-Securing_secure_boot.pdf (slide 15)

- Allow Secure Boot (SB) for QEMU- and KVM-based guests

 """
 Another feature that can be built into OVMF is called “SMM” (Secure Management Mode).
 [...] Once the SMM feature is built into OVMF, then SMM emulation by the QEMU platform
 is non-optional, it is required.
 ...
 For the SMM (System Management Mode) feature, only the QEMU Q35 machine type is supported.
 """

 @ https://specs.openstack.org/openstack/nova-specs/specs/wallaby/implemented/allow-secure-boot-for-qemu-kvm-guests.html

- OVMF's SMM support

 """
 * The minimum required QEMU machine type is "pc-q35-2.5".
 ...
 * Flags common to all SMM scenarios (only the Q35 machine type is supported):
 """

 @ https://github.com/tianocore/edk2/blob/master/OvmfPkg/README#L120

- [edk2] [PATCH v2 41/41] OvmfPkg: README: document SMM status

 """
 +OVMF's SMM support is subject to the following by-design limitations:
 +- only the q35 machine type of QEMU is supported,
 """

 @ https://<email address hidden>/msg02802.html

- Laszlo Ersek:

 """
 If you rebuild OVMF with "-D SECURE_BOOT_ENABLE" but remove "-D SMM_REQUIRE",
 that will break the security of the Secure Boot feature. Don't do it.
 ...
 The actual security of the Secure Boot feature in OVMF is ensured by SMM emulation.
 For this, OVMF must be built to include the edk2 SMM driver stack (hence -D SMM_REQUIRE).

 Furthermore, QEMU and KVM both must provide SMM/SMRAM emulation.
 The RHEL7 host kernel (KVM) provides SMM/SMRAM emulation, but qemu-kvm in base RHEL7 does not.

 For that, the "pc-q35-rhel7.3.0" or later QEMU machine type is necessary,
 """

 @ https://access.redhat.com/discussions/3175901#comment-1588571

- Laszlo Ersek:

 """
 SmmAccessPeiEntryPoint: no SMRAM with host bridge DID=0x1237; only DID=0x29C0 (Q35) is supported
 ASSERT .../OvmfPkg/SmmAccess/SmmAccessPei.c(390): ((BOOLEAN)(0==1))
 """

 @ https://access.redhat.com/discussions/2958371#comment-1155681

- Bug 1308678 - clearly separate SB-less, SMM-less OVMF binary from SB+SMM OVMF binary

 """
 "OVMF_CODE.nosmm.fd" would keep the same functionality as before
 (Secure Boot enabled, SMM disabled),
 while "OVMF_CODE.smm.fd" would be Secure Boot Enabled plus SMM enabled.

 ... Nonetheless the breakage was thought justified, because an SMM-less
 firmware binary (whose Secure Boot would not be in fact secure) ...
 """

 @ https://bugzilla.redhat.com/show_bug.cgi?id=1308678

Test Results:
---

OVMF_CODE.fd boots "everywhere", on q35/pc with SMM on/off.

OVMF_CODE.secboot.fd boots _only_ on q35 _and_ with SMM on.

Tests:
---

for SMM in on off; do
    for MACH in q35 pc; do
        for FW in OVMF_CODE.fd OVMF_CODE.secboot.fd; do

          echo
          echo "- SMM $SMM / $MACH / $FW"

          cp /usr/share/OVMF/OVMF_VARS.fd .
          timeout 5 \
          qemu-system-x86_64 \
              -M $MACH,smm=$SMM,accel=kvm \
              -drive if=pflash,format=raw,unit=0,file=/usr/share/OVMF/$FW,readonly=on \
              -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd \
              -global ICH9-LPC.disable_s3=1 \
              -nodefaults -nographic -no-user-config \
              -serial stdio 2>&1 \
          | grep 'UEFI.*EDK'

        done
    done
done

- SMM on / q35 / OVMF_CODE.fd
UEFI v2.70 (EDK II, 0x00010000)
Terminated

- SMM on / q35 / OVMF_CODE.secboot.fd
UEFI v2.70 (EDK II, 0x00010000)
Terminated

- SMM on / pc / OVMF_CODE.fd
UEFI v2.70 (EDK II, 0x00010000)
Terminated

- SMM on / pc / OVMF_CODE.secboot.fd
Terminated

- SMM off / q35 / OVMF_CODE.fd
UEFI v2.70 (EDK II, 0x00010000)
Terminated

- SMM off / q35 / OVMF_CODE.secboot.fd
Terminated

- SMM off / pc / OVMF_CODE.fd
UEFI v2.70 (EDK II, 0x00010000)
Terminated

- SMM off / pc / OVMF_CODE.secboot.fd
Terminated

Change abandoned by "Mauricio Faria de Oliveira <email address hidden>" on branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/828980
Reason: will do cherry-pick from victoria if/once it's merged

This turned out not to be needed in practice; marking as Won't Fix.

Hello, we are hitting this issue in a customers stack.

Focal/Ussuri

q35 is set
uefi is set

Nova thinks the instance has booted, but theres no logs, no guest console displayed, and it sits there.

Test steps/results with the attached patch
------------------------------------------

Create server with BIOS firmware (it boots):

 $ openstack server create --image jammy --flavor m1.small --network private test

 $ openstack console log show test | grep -o 'test login:'
 test login:

 $ openstack server show test | grep -e vm_state -e status -e task_state -e instance_name
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000002 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

 $ openstack server stop test
 $ openstack server delete test

Create server with UEFI firmware (it does _not_ boot):

 $ openstack image set jammy --property hw_firmware_type=uefi

 $ openstack server create --image jammy --flavor m1.small --network private test

 $ openstack console log show test | grep -o 'test login:'
 $

 $ openstack server show test | grep -e vm_state -e status -e task_state -e instance_name
 | OS-EXT-SRV-ATTR:instance_name | instance-00000003 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

(*) Note it uses the secure-boot loader (secboot), for which
    support has not been introduced in Ussuri, but in Wallaby.

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000003 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.secboot.fd</loader>
     <nvram>/var/lib/libvirt/qemu/nvram/instance-00000003_VARS.fd</nvram>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

The QEMU process keeps spinning at 100% CPU utilization.

 $ juju ssh nova-compute/0 top -b -d1 -n5 | grep qemu
  355887 libvirt+ 20 0 2482736 52228 21776 S 100.0 1.3 2:23.33 qemu-system-x86
  355887 libvirt+ 20 0 2482736 52228 21776 S 100.0 1.3 2:24.34 qemu-system-x86
  355887 libvirt+ 20 0 2482736 52228 21776 S 99.0 1.3 2:25.34 qemu-system-x86
  355887 libvirt+ 20 0 2482736 52228 21776 S 99.0 1.3 2:26.34 qemu-system-x86
  355887 libvirt+ 20 0 2482736 52228 21776 S 98.0 1.3 2:27.32 qemu-system-x86

 $ openstack server stop test

 $ openstack server show test | grep -e vm_state -e status -e task_state -e instance_name
 | OS-EXT-SRV-ATTR:instance_name | instance-00000003 |
 | OS-EXT-STS:task_state | None ...

The proposed/new config option is described in sample config in test package.

$ dpkg-deb -x nova-common_21.2.4-0ubuntu2.6_all.deb nova-common-deb.new

$ grep -B2 ubuntu_libvirt_uefi_loader_path nova-common-deb.new/etc/nova/nova.conf
# Determine UEFI loader path list for images with hw_firmware_type=uefi. For
# more information, refer to the documentation. (boolean value)
#ubuntu_libvirt_uefi_loader_path = false

Proposed MR for Ubuntu Focal (Ussuri)

https://code.launchpad.net/~mfo/ubuntu/+source/nova/+git/nova/+merge/447799

Test steps/results (Ussuri)
---------------------------

Ussuri:

 $ juju status | grep nova-compute | grep stable
 nova-compute 21.2.4 active 1 nova-compute ussuri/stable 682 no Unit is ready

Create server with BIOS firmware (it boots):

 $ openstack server create --image jammy --flavor m1.small --network private test

 $ openstack console log show test | grep -o 'test login:'
 test login:

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000001 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000001 2>&1 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

 $ openstack server stop test
 $ openstack server delete test

Create server with UEFI firmware (it does NOT boot):

 $ openstack image set jammy --property hw_firmware_type=uefi

 $ openstack server create --image jammy --flavor m1.small --network private test

 $ openstack console log show test | grep -o 'test login:'
 $

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

(*) Note it uses the secure-boot loader (secboot), for which
    support has not been introduced in Ussuri, but in Wallaby.

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000002 2>&1 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.secboot.fd</loader>
     <nvram>/var/lib/libvirt/qemu/nvram/instance-00000002_VARS.fd</nvram>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

The QEMU process keeps spinning at ~100% CPU utilization.

 $ juju ssh nova-compute/0 'for i in {1..5}; do ps -ho pcpu,cmd $(pidof qemu-system-x86_64) | cut -d, -f-1; sleep 1; done'
 99.4 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 99.4 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 99.4 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 99.4 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 99.4 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002

 $ openstack server stop test

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 ...

Test steps/results (Victoria)
-----------------------------

Victoria:

 $ juju status | grep nova-compute | grep stable
 nova-compute 22.4.0 active 1 nova-compute victoria/stable 679 no Unit is ready

Create server with BIOS firmware (it boots):

 $ openstack server create --image jammy --flavor m1.small --network private test

 $ openstack console log show test | grep -o 'test login:'
 test login:

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000001 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000001 2>&1 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

 $ openstack server stop test
 $ openstack server delete test

Create server with UEFI firmware (it does NOT boot):

 $ openstack image set jammy --property hw_firmware_type=uefi

 $ openstack server create --image jammy --flavor m1.small --network private test

 $ openstack console log show test | grep -o 'test login:'
 $

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

(*) Note it uses the secure-boot loader (secboot), for which
    support has not been introduced in Ussuri, but in Wallaby.

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000002 2>&1 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.secboot.fd</loader>
     <nvram>/var/lib/libvirt/qemu/nvram/instance-00000002_VARS.fd</nvram>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

The QEMU process keeps spinning at ~100% CPU utilization.

 $ juju ssh nova-compute/0 'for i in {1..5}; do ps -ho pcpu,cmd $(pidof qemu-system-x86_64) | cut -d, -f-1; sleep 1; done'
 96.1 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 96.2 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 96.3 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 96.4 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002
 96.5 /usr/bin/qemu-system-x86_64 -name guest=instance-00000002

 $ openstack server stop test

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 ...

Test steps/results (Upgrade: Ussuri -> Victoria)
------------------------------------------------

The 'test' server with UEFI was started on Ussuri
with the patch applied (i.e., it boots correctly;
see comment #15).

Now we upgrade from Openstack Ussuri to Victoria,
apply the patch, and restart the 'test' server
(it still boots correctly; as in comment #16).

Upgrade Reference:
 https://docs.openstack.org/project-deploy-guide/charm-deployment-guide/wallaby/upgrade-openstack.html

Commands and versions before/after:

 $ juju config rabbitmq-server source=cloud:focal-victoria

 $ juju config keystone openstack-origin=cloud:focal-victoria
  keystone 17.0.1 ...
  keystone 18.1.0 ...

 $ juju config cinder openstack-origin=cloud:focal-victoria
  cinder 16.4.2 ...
  cinder 17.4.0 ...

 $ juju config glance openstack-origin=cloud:focal-victoria
  glance 20.2.0 ...
  glance 21.1.0 ...

 $ juju config neutron-api openstack-origin=cloud:focal-victoria
  neutron-api 16.4.2 ...
  neutron-api 17.4.1 ...

 $ juju config neutron-gateway openstack-origin=cloud:focal-victoria
  neutron-gateway 16.4.2 ...
  neutron-gateway 17.4.1 ...

 $ juju config placement openstack-origin=cloud:focal-victoria
  placement 3.0.1 ...
  placement 4.0.0 ...

 $ juju config nova-cloud-controller openstack-origin=cloud:focal-victoria
  nova-cloud-controller 21.2.4 ...
  nova-cloud-controller 22.4.0 ...

 $ juju config nova-compute openstack-origin=cloud:focal-victoria
  nova-compute 21.2.4 ...
  nova-compute 22.4.0 ...

Verified that the config option is still in nova.conf:
 /etc/nova/nova.conf:ubuntu_libvirt_uefi_loader_path = True

Applied the changes.
Restarted nova-compute.service.

The old XML from Ussuri (VM is still running)

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000002 2>&1 | sed -n '/<os>/,/<\/os>/p'
   <os>
     <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
     <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
     <nvram>/var/lib/libvirt/qemu/nvram/instance-00000002_VARS.fd</nvram>
     <boot dev='hd'/>
     <smbios mode='sysinfo'/>
   </os>

Stop/Start the server:

 $ openstack server stop test
 $ openstack server show test | grep -e vm_state -e status -e task_state -e instance_name
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | stopped |
 | status | SHUTOFF |

 $ openstack server start test
 $ openstack server show test | grep -e vm_state -e status -e task_state -e instance_name
 | OS-EXT-SRV-ATTR:instance_name | in...

Patch updated in MR [1].
[1] https://code.launchpad.net/~mfo/ubuntu/+source/nova/+git/nova/+merge/447799

Thanks, Corey, for all your help.

The patches are merged in UCA:
- Victoria (focal-victoria): [1]
- Ussuri (focal): [2]

And uploaded to Focal. [3]

[1] https://git.launchpad.net/~ubuntu-openstack-dev/ubuntu/+source/nova/commit/?id=5b5542885a797e091a7b5c33c39fcbeff7a5ef20
[2] https://git.launchpad.net/~ubuntu-openstack-dev/ubuntu/+source/nova/commit/?id=5f9113b078c8cda9e543ec8a492e4a14be648d55
[3] https://launchpad.net/ubuntu/focal/+upload/31668221/+files/nova_21.2.4-0ubuntu2.6_source.changes

For non-juju installations of nova-common (which ships /etc/nova/nova.conf), if there was a local change in nova.conf, this update will trigger a dpkg conf prompt:

 ==> Modified (by you or by a script) since installation.
 ==> Package distributor has shipped an updated version.
   What would you like to do about it ? Your options are:
    Y or I : install the package maintainer's version
    N or O : keep your currently-installed version
      D : show the differences between the versions
      Z : start a shell to examine the situation
 The default action is to keep your current version.
*** nova.conf (Y/I/N/O/D/Z) [default=N] ? d
--- /etc/nova/nova.conf 2023-08-23 18:33:23.107167993 +0000
+++ /etc/nova/nova.conf.dpkg-new 2023-07-25 20:34:00.000000000 +0000
@@ -6,7 +6,6 @@
 #
 # From nova.conf
 #
-# andreas was here

 #
 # Availability zone for internal services. For more information, refer to the
@@ -659,6 +658,18 @@
 # mc - Memcache ServiceGroup driver
 #servicegroup_driver = db

+# DEPRECATED:
+# Determine UEFI loader path list for images with hw_firmware_type=uefi. For
+# more information, refer to the documentation. (boolean value)
+# This option is deprecated for removal since 21.2.4.
+# Its value may be silently ignored in the future.
+# Reason:
+# This option was added to work around issues with booting UEFI instances
+# in Ussuri and Victoria (LP: #1960758) if the Secure Boot loader is used
+# (happens by default on Ubuntu 20.04). This option does not exist nor is
+# needed in Wallaby and later, which actually support Secure Boot.
+#ubuntu_libvirt_uefi_loader_path = false
+
 #
 # From oslo.log
 #

Usually this is not desirable, specially if a new default value won't change. Is there another way to fix this? Is there a precedent for such a change in the history of updates of this stack of packages? Or is it very unlikely that nova.conf will have been changed locally?
\

Hi Andreas,

Thanks for reviewing!

Summary: among all points you raised, the least-negative answer/option seems to be, indeed, this is another case of changes to nova.conf (i.e., there are precedents; 6 versions of the file in the 21 package versions since focal-release).

...

Details:

> For non-juju installations of nova-common (which ships /etc/nova/nova.conf), if there was a local change in nova.conf, this update will trigger a dpkg conf prompt:

Nice catch; thanks for the careful review, as always!

> Usually this is not desirable, specially if a new default value won't change. [snip]

Agreed, it's not desirable.

> [snip] Is there another way to fix this?

- If 'this' is nova.conf, probably not in an elegant way, as it's automatically generated at build-time (so it's likely a hack in `debian/rules`).

- Now, if 'this' is the technical approach, probably not, because an opt-in is probably the best compromise between stability (no behavior change/less regression risk) and functionality (fix), in this case.

> [snip] Is there a precedent for such a change in the history of updates of this stack of packages? [snip]

There _are_ 5 precedents of nova.conf changes since the version in focal-release (total of 6 unique md5sums; see shell commands below).

> [snip] Or is it very unlikely that nova.conf will have been changed locally?

It's very _likely_ that nova.conf will have been changed.
The nova docs for installation [1, 2] change nova.conf immediately after package install:

  1. Install the packages:
  2. Edit the /etc/nova/nova.conf file and complete the following actions:

Thanks again,
Mauricio

...

$ curl -s 'https://launchpad.net/ubuntu/focal/+source/nova' \
  | grep -o '/ubuntu/+source/nova/[^"]\+' | sort -Vu \
  | sed -n '/2:21.0.0~b3~git2020041013.57ff308d6d-0ubuntu2/,$ p' \
  | cut -d/ -f5- | cut -d: -f2- \
  | while read VERSION; do \
      wget "https://launchpad.net/ubuntu/+archive/primary/+files/nova-common_${VERSION}_all.deb"; \
    done

$ for deb in *.deb; do dir=${deb%_*}; dpkg-deb -x $deb $dir; done

$ md5sum nova-common_*/etc/nova/nova.conf | sort
0c12bc3f6253cb10b1e25fc43bec7edb nova-common_21.0.0-0ubuntu0.20.04.1/etc/nova/nova.conf
0c12bc3f6253cb10b1e25fc43bec7edb nova-common_21.0.0-0ubuntu0.20.04.2/etc/nova/nova.conf
0c12bc3f6253cb10b1e25fc43bec7edb nova-common_21.0.0-0ubuntu0.20.04.3/etc/nova/nova.conf
34e79bb12168a44ce1bd8d824a309788 nova-common_21.1.0-0ubuntu1/etc/nova/nova.conf
34e79bb12168a44ce1bd8d824a309788 nova-common_21.1.0-0ubuntu2/etc/nova/nova.conf
34e79bb12168a44ce1bd8d824a309788 nova-common_21.1.1-0ubuntu1/etc/nova/nova.conf
34e79bb12168a44ce1bd8d824a309788 nova-common_21.1.1-0ubuntu2/etc/nova/nova.conf
5b99773742952b3633791b641a36aa5e nova-common_21.2.2-0ubuntu1/etc/nova/nova.conf
5b99773742952b3633791b641a36aa5e nova-common_21.2.3-0ubuntu1/etc/nova/nova.conf
5b99773742952b3633791b641a36aa5e nova-common_21.2.4-0ubuntu1/etc/nova/nova.conf
5b99773742952b3633791b641a36aa5e nova-common_21.2.4-0ubuntu2/etc/nova/nova.conf
79b5bb5ba349227ec756340b673b8e4c nova-common_21.2.4-0ubuntu2.1/etc/nova/nova.conf
79b5bb5ba349227ec756340b673b8e4c nova-common_21.2.4-0ubuntu2.2/etc/nova/nova.conf
79b5bb5...

Thanks Mauricio. I'm interested to hear what Andreas thinks about the previous conffile changes and its acceptability this time.

One other side of this is that this patch is being added as a workaround to Focal only, right? Are there any scenarios where users might carry forward this parameter to newer releases? What would happen if they did, and what are our expectations about that?

Hey Robie,

Thanks for reviewing!

> One other side of this is that this patch is being added as a workaround to Focal only, right?

Right.

> Are there any scenarios where users might carry forward this parameter to newer releases?

No; the next supported release, Jammy, ships Yoga, which is not affected, because
(since Wallaby) nova uses QEMU firmware metadata files (/usr/share/qemu/firmware/),
with proper check/handling of secure boot and the required changes to libvirt XML.

@ nova/virt/libvirt/driver.py:_configure_guest_by_virt_type() now calls host.py:get_loader()

> What would happen if they did, and what are our expectations about that?

This parameter is a no-op in newer releases (it doesn't need/have code there).

I think the expectation is that users who enable such option (as it's opt-in)
have read its doc/comments in the config file, which explains this option is
deprecated and is not available/needed in newer releases.

Thanks,
Mauricio

Hello Mauricio, or anyone else affected,

Accepted nova into victoria-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:victoria-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-victoria-needed to verification-victoria-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-victoria-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

When I see a new config file option being added already in a deprecated state, I have to wonder if it's the right fix: "NEW config file option! But it's deprecated, please try not to use it" :)

If we need a workaround, maybe it shouldn't be in the config file like that. It's opt-in, the default value is such that it doesn't change behavior, sounds like the config file is just a means for documentation. If this is indeed the only way to address this, could it not be in the default config file, and perhaps just be a "hidden" option that will be honored in this version? Its documentation could be elsewhere.

Hi Andreas o/

Thanks for the discussion! Addressing your points:

> When I see a new config file option being added already in a deprecated
> state, I have to wonder if it's the right fix: [...]

The usage of a deprecated config option is arbitrary (it may be not used),
but the semantics are actually correct [1] for the scenario of this issue
(as the config option is not available in future releases):

  class oslo_config.cfg.Opt(...)

    deprecated_for_removal – indicates whether this opt is planned for removal in a future release
    deprecated_reason – indicates why this opt is planned for removal in a future release. [...]
    deprecated_since – indicates which release this opt was deprecated in. [...]

> If we need a workaround, maybe it shouldn't be in the config file like
> that. It's opt-in, the default value is such that it doesn't change
> behavior, sounds like the config file is just a means for documentation.

I can appreciate your opinion (and have a similar take on this, in general),
however, the package/project goes with all options being in the config file
(with docs), including a dedicated 'workarounds' section, with many opt-ins:

  $ grep -e '^\[' -e '^#[^ ]' nova.conf
  ...
  [workarounds]
  #disable_rootwrap = false
  #disable_libvirt_livesnapshot = false
  #handle_virt_lifecycle_events = true
  #disable_group_policy_check_upcall = false
  #enable_numa_live_migration = false
  #ensure_libvirt_rbd_instance_dir_cleanup = false
  #disable_fallback_pcpu_query = false
  #never_download_image_if_on_rbd = false
  #disable_native_luksv1 = false
  #rbd_volume_local_attach = false
  #reserve_disk_resource_for_image_cache = false
  ...

(There are technical reasons this workaround is in the defaults section,
instead of workarounds: for integration with the juju charm, see MR [2].)

> If this is indeed the only way to address this, could it not be in the
> default config file, and perhaps just be a "hidden" option that will be
> honored in this version? Its documentation could be elsewhere.

Although there are technical means to achieve that suggestion (eg, d/rules
sed, or non-standard usage of the config option library in the source code):
- that does not seem to be what the users of this package expect; and
- hiding it could hinder someone affected to find out the solution needed.

So, I'd suggest that we continue with that the package/project already use.

...

Hopefully the points above address your questions and make the upload be
considered for SRU acceptance again, as-is.

Summary:
- there have been other config file changes in the past (~25% of uploads
after focal-release, as in #21);
- the package/project already describes all config options in nova.conf
(with documentation), including workarounds and deprecations, which are
correct/in-line with the release-scope of the introduced config option.

Thanks again,
Mauricio

[1] https://docs.openstack.org/oslo.config/ussuri/reference/api/oslo_config.html#oslo_config.cfg.ConfigOpts
[2] https://code.launchpad.net/~mfo/ubuntu/+source/nova/+git/nova/+merge/447799/comments/1198731

Ok regarding the config file change then, that ship has sailed already a long time ago.

Ack; thanks, Andreas!

Hello Mauricio, or anyone else affected,

Accepted nova into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nova/2:21.2.4-0ubuntu2.6 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Verification done for focal-proposed and
(via upgrade to) focal-victoria/proposed,
later upgraded to focal-wallaby/proposed
(to confirm no changes are needed there).

focal-proposed:
--------------

First, (juju) deployed openstack on focal, and verified with steps in comment #15.

With focal-updates, an uefi server does NOT boot:

 $ juju ssh nova-compute/0 'dpkg -s nova-compute | grep ^Version:' 2>/dev/null
 Version: 2:21.2.4-0ubuntu2.5

 $ openstack image set jammy --property hw_firmware_type=uefi
 $ openstack server create --image jammy --flavor m1.small --network private test

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000002 2>&1 | sed -n '/<os>/,/<\/os>/p'
 ...
     <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.secboot.fd</loader>
 ...

 $ openstack console log show test
 $

With focal-proposed, the server does boot successfully:

$ juju ssh nova-compute/0 'sudo add-apt-repository --yes "deb http://archive.ubuntu.com/ubuntu focal-proposed main"'

I didn't find an option to "upgrade" to focal-proposed
(just to later openstack releases/cloud archive), thus
I manually run the `apt install` command as the charm.

The config change isn't an issue with the nova-compute
charm due to dpkg options `--force-conf{new,def}`:

 @ charm-nova-compute/hooks/nova_compute_utils.py

  769 dpkg_opts = [
  770 '--option', 'Dpkg::Options::=--force-confnew',
  771 '--option', 'Dpkg::Options::=--force-confdef',
  772 ]

@ dpkg(1):

 confnew: If a conffile has been modified and the version in the package did change,
 always install the new version without prompting, unless the --force-confdef is also specified,
 in which case the default action is preferred.

And the charm does rewrite the configs anyway (`configs.write_all()`),
so I installed and triggered the config change/rewrite w/ our option.

 $ juju ssh nova-compute/0 'sudo apt install --yes --option=Dpkg::Options::=--force-confnew --option=Dpkg::Options::=--force-confdef nova-compute'
 ...
 Configuration file '/etc/nova/nova.conf'
  ==> Modified (by you or by a script) since installation.
  ==> Package distributor has shipped an updated version.
  ==> Keeping old config file as default.
 ...

 $ juju ssh nova-compute/0 'dpkg -s nova-compute | grep ^Version:' 2>/dev/null
 Version: 2:21.2.4-0ubuntu2.6

 $ juju config nova-compute config-flags='ubuntu_libvirt_uefi_loader_path=True'

 $ juju ssh nova-compute/0 sudo grep ubuntu_libvirt_uefi_loader_path /etc/nova/nova.conf
 ubuntu_libvirt_uefi_loader_path = True

And the server boot correctly:

 $ openstack server stop test
 $ openstack server start test

 $ openstack server show test | grep -e instance_name -e task_state -e vm_state -e status
 | OS-EXT-SRV-ATTR:instance_name | instance-00000002 |
 | OS-EXT-STS:task_state | None |
 | OS-EXT-STS:vm_state | active |
 | status | ACTIVE |

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000002...

Hello Mauricio, or anyone else affected,

Accepted nova into ussuri-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:ussuri-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-ussuri-needed to verification-ussuri-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-ussuri-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

The verification of the Stable Release Update for nova has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package nova - 2:22.4.0-0ubuntu1~cloud5
---------------

 nova (2:22.4.0-0ubuntu1~cloud5) focal-victoria; urgency=medium
 .
   * d/p/lp1960758-ubuntu-uefi-loader-path.patch: add config option
     'ubuntu_libvirt_uefi_loader_path' to restrict UEFI loaders to
     only those shipped/supported in Ubuntu/Ussuri. (LP: #1960758)

Change abandoned by "Mauricio Faria de Oliveira <email address hidden>" on branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/828979
Reason: Different/downstream patch used in bug 1960758.

This bug was fixed in the package nova - 2:21.2.4-0ubuntu2.6

---------------
nova (2:21.2.4-0ubuntu2.6) focal; urgency=medium

  * d/p/lp1960758-ubuntu-uefi-loader-path.patch: add config option
    'ubuntu_libvirt_uefi_loader_path' to restrict UEFI loaders to
    only those shipped/supported in Ubuntu/Ussuri. (LP: #1960758)

 -- Mauricio Faria de Oliveira <email address hidden> Tue, 25 Jul 2023 17:34:00 -0300

Hi Corey,

While verifying (bionic) ussuri-proposed, I noticed bionic doesn't necessarily need the fix, since its ovmf package does not ship OVMF_CODE.secboot.fd (introduced later, present in focal).

Keeping the fix is safe, though, as it is a no-op by default (no functional change),
and may be desired to get nova in bionic-ussuri back in sync with focal, after the regression handling for CVE-2023-2088.patch, but I'm not sure of all the details involved.

I'll not mark this as verification-done-ussuri yet, in order to wait for your confirmation.

Thanks,
Mauricio

...

Verification on bionic:

With ussuri-updates, an uefi server _does_ boot (differently than focal):

 $ juju ssh nova-compute/0 'dpkg -s nova-compute | grep ^Version:' 2>/dev/null
 Version: 2:21.2.4-0ubuntu2.2~cloud0

 $ openstack image set jammy --property hw_firmware_type=uefi
 $ openstack server create --image jammy --flavor m1.small --network private test

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000001 2>&1 | sed -n '/<os>/,/<\/os>/p'
 ...
 <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
 ...

 $ openstack console log show test | grep -o 'test login:'
 test login:

This works because the problematic loader (OVMF_CODE.secboot.fd) is not yet available on bionic, only in focal:

 $ ls -1 /usr/share/OVMF/
 OVMF_CODE.fd
 OVMF_VARS.fd

 $ dpkg -S /usr/share/OVMF/OVMF_CODE.fd
 ovmf: /usr/share/OVMF/OVMF_CODE.fd

 $ dpkg -L ovmf | grep -F .fd
 /usr/share/OVMF/OVMF_CODE.fd
 /usr/share/OVMF/OVMF_VARS.fd
 /usr/share/ovmf/OVMF.fd
 /usr/share/qemu/OVMF.fd

 $ lsb_release -cs
 bionic

 $ pull-lp-debs ovmf focal
 $ dpkg-deb -c ovmf_*.deb
 ...
 -rw-r--r-- root/root 1966080 2022-11-08 05:40 ./usr/share/OVMF/OVMF_CODE.secboot.fd
 ...

With ussuri-proposed and new option disabled (default), an uefi server still boots (no regression by default):

 $ juju ssh nova-compute/0 'sudo add-apt-repository --yes "deb http://ubuntu-cloud.archive.canonical.com/ubuntu bionic-proposed/ussuri main"'
 $ juju ssh nova-compute/0 'sudo apt install --yes --option=Dpkg::Options::=--force-confnew --option=Dpkg::Options::=--force-confdef nova-compute'
 $ juju ssh nova-compute/0 'sudo systemctl restart nova-compute.service' 2>/dev/null

 $ juju ssh nova-compute/0 'dpkg -s nova-compute | grep ^Version:' 2>/dev/null
 Version: 2:21.2.4-0ubuntu2.6~cloud0

 $ openstack server stop test
 $ openstack server start test

 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000001 2>&1 | sed -n '/<os>/,/<\/os>/p'
 ...
     <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
 ...

 $ openstack console log show test | grep -o 'test login:'
 test login:

With ussuri-proposed and the new option enabled (changed), an uefi server still boots (no regression)

 $ juju config nova-compute config-flags='ubuntu_libvirt_uefi_loader_path=True'
 $ juju ssh nova-compute/0 sudo grep ubuntu_libvirt_uefi_loader_path /etc/nova/nova.conf 2>/dev/null
 ubuntu_libvirt_uefi_loader_path = True

 $ openstack server stop test
 $ openstack server start test
 $ juju ssh nova-compute/0 sudo virsh dumpxml instance-00000001 2>&1 | sed -n '/<os>/,/<\/os>/p'
 ...
     <loader readonly='y...

Hi Mauricio, Thanks for your work on this. I think that's fine. It's behind a feature config anyway. There shouldn't be any implications with that CVE. The CVE is dealing with disconnecting volumes.

Hey Corey. Thanks for confirming. I've updated the tags.

The verification of the Stable Release Update for nova has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package nova - 2:21.2.4-0ubuntu2.6~cloud0
---------------

 nova (2:21.2.4-0ubuntu2.6~cloud0) bionic-ussuri; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 nova (2:21.2.4-0ubuntu2.6) focal; urgency=medium
 .
   * d/p/lp1960758-ubuntu-uefi-loader-path.patch: add config option
     'ubuntu_libvirt_uefi_loader_path' to restrict UEFI loaders to
     only those shipped/supported in Ubuntu/Ussuri. (LP: #1960758)