Adding IA32 to X64 pkg, because secure boot is not working on Focal

Bug #2022312 reported by Seyeong Kim
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ubuntu Cloud Archive
Invalid
Undecided
Unassigned
Yoga
Confirmed
Undecided
Unassigned
edk2 (Ubuntu)
Fix Released
Undecided
Unassigned
Focal
In Progress
Undecided
Unassigned
Jammy
Fix Released
Undecided
Unassigned

Bug Description

[Impact]

In Focal, secureboot is not working ( black screen right after instance is started )

[Test Case]
0. juju bundle for focal-yoga openstack env
- https://pastebin.ubuntu.com/p/G38JwXMX5G/
1. create custom image with cirros
- openstack image create --container-format bare --disk-format qcow2 --file cirros-0.5.1-x86_64-disk.img cirros
2. set image properties.
- $ openstack image set --property hw_machine_type=q35 --property hw_firmware_type=uefi --property os_secure_boot=required cirros
3. In focal, create instance, and enable secureboot
4. start instance.
5. you just can see only blackscreen.

[Where problems could occur]
Secureboot may have issue.

[Others]
For Jammy, it is ok

instance xml
- https://pastebin.ubuntu.com/p/MnK6nx3vwy/

#ADDED
Testing
1. Prepared cirros and cirros2 image
2. only set secure boot parameters to cirros image
3. launch instances
- instance with cirros image
- instance with cirros2 image
4. test result
- booting cirros instance doesn't work(black screen) with original OVMF_CODE_4M.secboot.fd
- booting cirros instance does work(shows uefi prompt) with patched OVMF_CODE_4M.secboot.fd
- booting cirros2 instance either cases.

Tags: patch sts
Seyeong Kim (seyeongkim)
tags: added: sts
Seyeong Kim (seyeongkim)
description: updated
description: updated
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "lp2022312_focal.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Seyeong Kim (seyeongkim)
Changed in edk2 (Ubuntu Focal):
assignee: nobody → Seyeong Kim (seyeongkim)
Changed in edk2 (Ubuntu):
status: New → Fix Released
Changed in edk2 (Ubuntu Jammy):
status: New → Fix Released
Seyeong Kim (seyeongkim)
description: updated
Revision history for this message
dann frazier (dannf) wrote :

In focal, you need to pass -global ICH9-LPC.disable_s3=1.

See https://salsa.debian.org/qemu-team/edk2/-/commit/f839ec864df811395863fdd383842ffbcba5cd56

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Hi Seyeong,

Thanks for looking at this and providing a patch.

The change to edk2/ovmf doesn't seem appropriate for a SRU (it's not adding, but switching; focal is out for ~3 years already; and a report about this only came up now).

Could you please review/address Dann's comment above (patch author)?

I'd think the change should be in the component driving the qemu command,
(in this case, nova, which builds the libvirt xml, per the pastebin),
to align with how it should be done on focal (disable s3).

Thanks!

Changed in edk2 (Ubuntu Focal):
status: New → Won't Fix
Changed in nova (Ubuntu Jammy):
status: New → Invalid
Changed in nova (Ubuntu):
status: New → Invalid
Changed in nova (Ubuntu Focal):
status: New → Incomplete
Changed in edk2 (Ubuntu Focal):
assignee: Seyeong Kim (seyeongkim) → nobody
Changed in nova (Ubuntu Focal):
assignee: nobody → Seyeong Kim (seyeongkim)
Revision history for this message
Seyeong Kim (seyeongkim) wrote :

Thanks for checking mfo.
yeah.
I thought fixing this pkg is more simple :D
The next step should be related to nova.
I'm going to analyze further.

Revision history for this message
Trent Lloyd (lathiat) wrote :

Christian wrote in this bug here that the edk2 fix was preferable, and that setting ICH9-LPC.disable_s3=1 by default may break more things than it fixes:
https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1903681

It's also notable that bug was raised in 2020 on focal's release so it has been raised (and fixed) earlier than now 3 years local.

According to the above bug, the current version of the file OVMF_CODE.ms.fd is just plainly broken in all cases. So if this patch -only- effects OVMF_CODE.ms.fd and we can confirm that the file is not usable in any situation, the SRU may be more acceptable.

The question remains whether we can safely SRU this change.

My suggestion is that:
(1) We know focal-yoga is broken, but we need to determine whether focal-ussuri (the openstack version that ships with focal) works. If it does work, we could consider a backport of edk2 from jammy into the openstack cloud-archive for yoga. The SRU still needs to be understood thoroughly to be safe but we potentially only affect openstack users, rather than all Ubuntu users.

(2) If focal-ussuri is broken, we probably need to look at actually accepting this change but need some people with much more detailed knowledge to help understand if it will be OK and help understand, detail and test the risk of the change.

Revision history for this message
Trent Lloyd (lathiat) wrote :

It seems libvirt does have code to sometimes set -global ICH9-LPC.disable_s3=1, but it's not 100% clear to me from a quick read why that might not be getting done here. Perhaps Seyeong you could check into that?

The code to do that seems quite a bit older than focal.

Example:
https://github.com/libvirt/libvirt/commit/d6d31e00ebc8c72b88f0248f1bfde7e2ff7607af#diff-b40c0e564d59de99367e1b623fb4e4cf83abe7e74f4f4e7e54358d487b552b9eR5074

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

> According to the above bug, the current version of the file OVMF_CODE.ms.fd is just plainly broken in all cases. So if this patch -only- effects OVMF_CODE.ms.fd and we can confirm that the file is not usable in any situation, the SRU may be more acceptable.

As DannF stated, it is usable if the user or the management system sets -global ICH9-LPC.disable_s3=1.
So I guess we do not get over the SRU barrier by saying "it is not working at all, so there can't be a regression.

I agree that finding out if focal-ussuri works or not is a valid data point.
But again, if you just use qemu and edk2 they will most likely work by users or management setting -global ICH9-LPC.disable_s3=1 (to be confirmed).

> it's not 100% clear to me from a quick read why that might not be getting done here

The domain needs to have something like:

<pm>
  <suspend-to-mem enabled='no'/>
</pm>

Then it should get the it set, which is depending on the rest of the guest config then either ICH9-LPC.disable_s3=1 or PIIX4_PM.disable_s3=1.

So the entry above is something that nova could set on secure boot for focal-yoga to get the case working without chaning EDK2 or the qemu defaults (as we consider those two more regression risky).

Revision history for this message
Trent Lloyd (lathiat) wrote (last edit ):

I cannot find any mention in any openstack repo of that suspend-to-mem property. Not sure if I'm missing something but perhaps OpenStack actually doesn't currently support this option?

I am surprised by that given how many people seem to have run into this issue. Has everyone else just fixed their edk2 build?

Secure Boot was definitely working at some point, maybe bionic? It's not clear to me what changed to break this in focal. Was it a change in edk2 itself or?

Is there any possibility it's safe to SRU this edk2 change?

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

I tested focal-ussuri, It doesn't work either. the same black screen there.

I think it is edk2 issue in focal. I also can't find any nova upstream work related to this(maybe i missed someting though). qemu, libvirt have related options.

Revision history for this message
dann frazier (dannf) wrote :

I'm curious, does it work if you drop "acpi-s3" from the features in /usr/share/qemu/firmware/40-edk2-x86_64-secure-enrolled.json ? If I understand that setting correctly, it should be giving libvirt the hint it needs to figure out whether or not the firmware supports S3, and in focal that setting looks wrong since we're using 64-bit PEI which does not. Hopefully libvirt has the smarts to pass '-global ICH9-LPC.disable_s3=1' when "acpi-s3" is absent. If so, correcting the descriptor in an SRU seems like a path forward.

Note: We introduced the 4M images into Ubuntu w/ 64-bit PEI at some point after focal, but we switched them to 32-bit PEI before 22.04, so I believe advertising the "acpi-s3" feature is correct in jammy. When we backported the 4M images to focal earlier this year (bug 1885662), we backported the intermediate version that still used 64-bit PEI. I regret that we didn't think to also include the switch to 32-bit PEI at that time, it would have been the safest time to do it, and consistent with jammy. But now that we've released them w/ 64-bit PEI into an LTS, I don't know how we'd go about demonstrating that no one has grown reliant upon it.

Revision history for this message
Heitor Alves de Siqueira (halves) wrote :

It seems this SRU has some pending questions that need to be addressed, and is marked as 'Incomplete'. I'm unsubscribing ~se-sponsors for now, please feel free to re-subscribe when this is ready for sponsorship!

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

updated

I just tested pure focal machine without openstack.

and put below lines for new vm

  <os>
    <type arch="x86_64" machine="pc-q35-4.2">hvm</type>
    <loader readonly="yes" secure="yes" type="pflash">/usr/share/OVMF/OVMF_CODE_4M.ms.fd</loader>
    <nvram>/var/lib/libvirt/qemu/nvram/ubuntu22.04_VARS.fd</nvram>
    <boot dev="hd"/>
    <smbios mode="sysinfo"/>
  </os>
  <features>
    <acpi/>
    <apic/>
    <vmport state="off"/>
    <smm state="on"/>
  </features>

it is booted well.

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

@dannf

I deployed test env and removed acpi-s3 from /usr/share/qemu/firmware/40-edk2-x86_64-secure-enrolled.json

then rebooted instance. but the same behaviour. is this right step to test it?

Thanks.

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

for #16, I just confirmed that there are below options with virt-manager created instance.

-global ICH9-LPC.disable_s3=1
-global ICH9-LPC.disable_s4=1

but nova created instance doesn't have it

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

in libvirt xml
  <pm>
    <suspend-to-mem enabled="no"/>
    <suspend-to-disk enabled="no"/>
  </pm>

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

so maybe belows are the way we can do

1. extra feature adding below for uca(for focal) only
- <pm><suspend-to-mem enabled="no"/><suspend-to-disk enabled="no"/></pm>
- no upstream related work so I'm not sure it can be done only for focal-uca
2. patch edk2 for focal
- not possible?
3. include patched edk2 pkg in uca
- not sure but possible solution..?

I appreciate if there are any other ideas

Seyeong Kim (seyeongkim)
Changed in nova (Ubuntu Focal):
assignee: Seyeong Kim (seyeongkim) → nobody
status: Incomplete → Invalid
Revision history for this message
Edward Hope-Morley (hopem) wrote :

Apologies i forgot to add the uca target to reflect the fact that the request is to backport to Focal Yoga UCA (so that we have parity with Jammy Yoga i.e. jammy-updates)

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Has secure boot ever worked on focal? If not, then what is being asked for is a new feature.

Revision history for this message
Corey Bryant (corey.bryant) wrote :

I'm not comfortable backporting edk2 to the focal-yoga cloud archive. The delta between the focal and jammy version is massive. I don't have an exact LOC count but a quick diff command shows the following:

diff -Naur edk2-0~20191122.bd85bf54/ edk2-2022.02/|wc -l
1954492

Revision history for this message
dann frazier (dannf) wrote :

Secure Boot does work in focal, but apparently never has through OpenStack. That's a bug or a new feature request, depending on how you look at it.

*If* we want to fix that/enable that feature, it sounds like we're back to Seyeoung's options #1 and #2 from comment #20. Option #2 - switching the 4M images to 32-bit PEI - is starting to sound like the best option. Patching Nova to do something atypical for focal feels like it would carry more risk.

We've had 4M images in focal for about 8 months (2023-01-24). The regression risk with this change would be that someone has created a VM in focal during that time that somehow needs 64-bit PEI. I'm not aware of any reason a VM would require 64-bit PEI, and I'm also unaware of any user-perceivable changes the switch would make. I haven't see any user reports about problems with that since we introduced that change in Debian/Ubuntu. If users are already booting Secure Boot VMs w/ S3 disabled to use them, they can continue doing that[*]. The fact that the firmware would now support booting them w/ S3 enabled wouldn't be a regression. And it would correct an internal inconsistency in OVMF - where it tells libvirt that the 4M images support s3 when they currently do not.

If you do move forward with this SRU Seyeoung, I'd suggest using the same or similar debian/changelog entry from when we introduced the change in 2020.11-1, as it provides more context.

[*] Of course, we need to verify that - it should be a verification test

Revision history for this message
Seyeong Kim (seyeongkim) wrote :

@dannf

Thanks for the info.

I already mentioned below link in debian/changelog
https://salsa.debian.org/qemu-team/edk2/-/commit/f839ec864df811395863fdd383842ffbcba5cd56

But you think I need to copy the string to changelog? or the other link?

I tested my patch at the time I was making the patch, and at least secure booting was fine.

but I'm going to test one more test this week.

Thanks.

Seyeong Kim (seyeongkim)
description: updated
Revision history for this message
Seyeong Kim (seyeongkim) wrote :

I updated description as with simple test result.

description: updated
description: updated
Revision history for this message
dann frazier (dannf) wrote : Re: [Bug 2022312] Re: Adding IA32 to X64 pkg, because secure boot is not working on Focal

On Mon, Sep 4, 2023 at 1:25 AM Seyeong Kim <email address hidden> wrote:
>
> @dannf
>
> Thanks for the info.
>
> I already mentioned below link in debian/changelog
> https://salsa.debian.org/qemu-team/edk2/-/commit/f839ec864df811395863fdd383842ffbcba5cd56
>
> But you think I need to copy the string to changelog? or the other link?

I prefer changelog entries to be self-contained and not require the
reader to go out to the Internet for details. Internet links break,
and users do not always have Internet access. But I'm just expressing
my preference - I'm not on the SRU team.

  -dann

Revision history for this message
Seyeong Kim (seyeongkim) wrote :
Revision history for this message
Seyeong Kim (seyeongkim) wrote :

Thanks for your feedback @dann.

@mfo
I think we(SRU) should patch edk2 pkg for focal with my patch(or modified) if there is no critical issue.

As dann and Corey mentioned, option 1 and 3 may not acceptible for now.

Could you please consider this again?

Changed in edk2 (Ubuntu Focal):
status: Won't Fix → In Progress
no longer affects: nova (Ubuntu)
no longer affects: nova (Ubuntu Jammy)
no longer affects: nova (Ubuntu Focal)
Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote (last edit ):

 I have reread this bug, the other bug 1903681, and EDK2 bug 3064 [1].

I agree with Christian's assessment in comment 11 and Dann's remark in comment 25.
That is, nova should set suspend-to-mem (disable_s3), but not by default (opt-in).

> The domain needs to have something like:
> <pm>
> <suspend-to-mem enabled='no'/>
> </pm>
>
> Then it should get the [b]it set, which is depending on the rest of the
> guest config then either ICH9-LPC.disable_s3=1 or PIIX4_PM.disable_s3=1.
>
> So the entry above is something that nova could set on secure boot for focal-yoga
> to get the case working without chan[g]ing EDK2 or the qemu defaults (as we consider
> those two more regression risky).

and

> Patching Nova to do something atypical for focal feels like it would carry more risk.

also, from [1]

> Brief summary: if you want S3 resume with SMM, then your PEI phase needs to be 32-bit
> (you need the IA32X64 build of OVMF -- "OvmfPkg/OvmfPkgIa32X64.dsc" -- if you want your DXE phase to be 64-bit).
> If you don't care about S3 resume, then you can use the purely 64-bit build of OVMF even with SMM enabled
> (like you are building X64 now), but then you need to explicitly disable S3 support on the QEMU command line (see option (2)) above.

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote (last edit ):

I think the behavior should be opt-in because there may be existing VMs
that are working (e.g., user installed other/custom UEFI/OVMF build [2],
which is not supported, but it worked), that by now have S3 enabled,
and might observe differences or even issues (?) with S3 disabled.

Thus, I propose adding a nova config option for libvirt workarounds
(this is official upstream for downstream use [3]; e.g., Ubuntu [4])
and have it emit the 'pm/suspend-to-mem' libvirt XML tags if enabled
_and_ the UEFI loader declares it `requires-smm` (ie, for Secure Boot).

It worked successfully in an UCA Focal-Yoga openstack deployment.

I'll attach the debdiff and test results in another comment, and
ask for reviews.

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

By the way, regarding Focal Ussuri (ie, Focal as in Ubuntu Archive),
I'm not sure that has to be fixed, since Secure Boot support in Nova
is only introduced in Openstack Wallaby (after Ussuri, before Yoga).
[That is actually related to the reason for upload [4], bug 1960758.]

[1] https://bugzilla.tianocore.org/show_bug.cgi?id=3064#c2
[2] https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1903681/comments/4
[3] https://git.launchpad.net/ubuntu/+source/nova/tree/nova/conf/workarounds.py?h=import/2%2521.2.4-0ubuntu2.6#n22
[4] https://git.launchpad.net/ubuntu/+source/nova/commit/?h=import/2%2521.2.4-0ubuntu2.6

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :
Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Tests 1/2:

Enable QEMU q35 machine type and UEFI firmware:
 $ openstack image set --property hw_machine_type=q35 --property hw_firmware_type=uefi jammy

Secure Boot: disabled
---------------------

The patch does not regress VMs _without_ Secure Boot, and does not change the libvirt XML.

Before: works

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

 $ openstack console log show test-jammy | grep -e '^BdsDxe' -e secureboot: -e login:
 BdsDxe: starting Boot0001 "UEFI Misc Device" from PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)
 [ 0.000000] secureboot: Secure boot disabled
 [ 0.027148] secureboot: Secure boot disabled
 test-jammy login: [...]

After/Config=False: still work

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

 $ openstack console log show test-jammy | grep -e '^BdsDxe' -e secureboot: -e login:
 BdsDxe: starting Boot0001 "UEFI Misc Device" from PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)
 [ 0.000000] secureboot: Secure boot disabled
 [ 0.026675] secureboot: Secure boot disabled
 test-jammy login: [...]

After/Config=True: still work

 @ /etc/nova/nova.conf
 [DEFAULT]
 ubuntu_libvirt_uefi_secboot_disable_s3=True

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

 $ openstack console log show test-jammy | grep -e '^BdsDxe' -e secureboot: -e login:
 BdsDxe: starting Boot0001 "UEFI Misc Device" from PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)
 [ 0.000000] secureboot: Secure boot disabled
 [ 0.028423] secureboot: Secure boot disabled
 test-jammy login: [...]

XML comparison (normalized for UUID/MAC/IP/TAP/DATETIME/etc):

 $ diff -U0 test-jammy.xml.secboot-disabled.before test-jammy.xml.secboot-disabled.after.config-false
 $

 $ diff -U0 test-jammy.xml.secboot-disabled.before test-jammy.xml.secboot-disabled.after.config-true
 $

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Tests 2/2:

Enable Secure Boot:
 $ openstack image set --property os_secure_boot=required jammy

Secure Boot: enabled
--------------------

The patch improves VMs _with_ Secure Boot (and only changes the libvirt XML) _if_ the option is enabled.

Before: fails

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

 $ openstack console log show test-jammy
 $

 $ juju ssh nova-compute/0 'top -b -n 5 1 | grep qemu-system-x86'
  115025 libvirt+ 20 0 2487320 55596 21724 S 93.8 1.4 0:57.67 qemu-system-x86
  115025 libvirt+ 20 0 2487320 55596 21724 S 99.3 1.4 1:00.66 qemu-system-x86
  115025 libvirt+ 20 0 2487320 55596 21724 S 99.7 1.4 1:03.65 qemu-system-x86
  115025 libvirt+ 20 0 2487320 55596 21724 S 99.3 1.4 1:06.64 qemu-system-x86
  115025 libvirt+ 20 0 2487320 55596 21724 S 100.0 1.4 1:09.65 qemu-system-x86

After/Config=False: still fail

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

 $ openstack console log show test-jammy
 $

 $ juju ssh nova-compute/0 'top -b -n 5 1 | grep qemu-system-x86'
  117028 libvirt+ 20 0 2487328 55684 21840 S 100.0 1.4 0:38.41 qemu-system-x86
  117028 libvirt+ 20 0 2487328 55684 21840 S 99.3 1.4 0:41.40 qemu-system-x86
  117028 libvirt+ 20 0 2487328 55684 21840 S 99.3 1.4 0:44.39 qemu-system-x86
  117028 libvirt+ 20 0 2487328 55684 21840 S 100.0 1.4 0:47.39 qemu-system-x86
  117028 libvirt+ 20 0 2487328 55684 21840 S 100.0 1.4 0:50.40 qemu-system-x86

After/Config=True: IT WORKS!

 @ /etc/nova/nova.conf
 [DEFAULT]
 ubuntu_libvirt_uefi_secboot_disable_s3=True

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

 $ openstack console log show test-jammy | grep -e '^BdsDxe' -e secureboot: -e login:
 BdsDxe: starting Boot0003 "ubuntu" from HD(15,GPT,BC341E7F-34EE-4AAE-A937-2FE87A0792CB,0x2800,0x35000)/\EFI\ubuntu\shimx64.efi
 [ 0.000000] secureboot: Secure boot enabled
 [ 0.028329] secureboot: Secure boot enabled
 test-jammy login: [...]

XML comparison (normalized for UUID/MAC/IP/TAP/DATETIME/etc):

 $ diff -U0 test-jammy.xml.secboot-enabled.before test-jammy.xml.secboot-enabled.after.config-false
 $

 $ diff -U0 test-jammy.xml.secboot-enabled.before test-jammy.xml.secboot-enabled.after.config-true
 ...
 + <pm>
 + <suspend-to-mem enabled='no'/>
 + </pm>

...

XML comparison between Secure Boot disabled (before) and Secure Boot enabled (after/config=true):

 $ diff -U0 test-jammy.xml.sb-no.before test-jammy.xml.sb-yes.after.conf-yes
 ...
 @@ -49,2 +49,2 @@
 - <loader readonly='yes' secure='no' type='pflash'>/usr/share/OVMF/OVMF_CODE_4M.fd</loader>
 - <nvram template='/usr/share/OVMF/OVMF_VARS_4M.fd'>/var/lib/libvirt/qemu/nvram/<<INSTANCE>>_VARS.fd</nvram>
 + <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE_4M.ms.fd</loader>
 + <nvram template='/usr/share/OVMF/OVMF_VARS_4M.ms.fd'>/var/lib/libvirt/qemu/nvram/<<INSTANCE>>_VARS.fd</nvram>
 @@ -56,0 +57 @@
 + <smm state='on'/>
 @@ -91,0 +93,3 @@
 + <pm>
 + <suspend-to-mem enabled='no'/>
 + </pm>

Changed in cloud-archive:
status: New → Invalid
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you Mauricio, thanks for your ping to review the situation ...

AFAICS In his comment #25 DannF was leaning towards solution #2 of comment #20 but solution #1 seemed ok as well.

I personally like if multiple things are resolved in a similar way as that is less surprising, and by making it a pure opt-in the risk isn't only low but sort of almost zero.
So IMHO you have taken an already trodden and safer path, which is great.

I'm +0.98 already

You'd get another 0.01 by an ask if we know if this is working well for the users that are affected by this. Your tests show that it "works", what I mean is that that we should avoid releasing this only to end up with people stating "But I can't change my nova.conf". Has anyone made sure this will not block this?
(Gladly solutions #1 and #2 are not mutually exclusive, we can go for #2 now and worst case still consider #1 later)

And yet another +0.01 via a thought that came to my mind about upgrade compatibility. If we add this to Nova in focal it will make it react to ubuntu_libvirt_uefi_secboot_disable_s3 config option. Once such a user upgrades to Jammy, will it break, will it silently be ignored but working the same way, will it need a patch to jammy and later to handle such upgrades well?

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :
Download full text (3.4 KiB)

Hi Christian,

Thanks for reviewing!

> AFAICS In his comment #25 DannF was leaning towards solution #2 of comment #20 but solution #1 seemed ok as well.
> ...
> (Gladly solutions #1 and #2 are not mutually exclusive, we can go for #2 now and worst case still consider #1 later)

Right. What I expressed was my opinion after considering comments #25 and #11, with a preference for your suggestion in #11 (nova to set libvirt options), while taking Dann's remark in #25 (not do something atypical -- by default).

I see that Dann provided a detailed regression risk assessment in #25 (2nd paragraph) with positive, technical observations on areas for potential regressions, and did not block an edk2 SRU from proceeding.

On that option, I think if we could run test scenarios for such areas, that would alleviate concerns a lot (eg, create Linux/Windows VMs with the existing firmware, upgrade it to test packages, power off/on VMs, and check for issues; hopefully none).

However, as you, I too think #1 seems OK (since other tools/upper layers already do it) and like a consistent/similar approach.

If you prefer option #2 (edk2) now, I think that is OK and has valid reasons too (we know some use-case is broken, and could fix it with a reasonable understanding of regression risk and sufficient testing to mitigate it).

I'll proceed with the remaining 2x +0.01 for now :)

> ... if we know if this is working well for the users that are affected by this ...
> ... we should avoid releasing this only to end up with people stating "But I can't change my nova.conf"

Good point.

We can definitely provide test packages and usage instructions for the affected users (support customer).
I'm not sure of specific scenarios that users cannot change their config files, and have previously provided a similar method (opt-in config option) that has been used.

(The usage of the 'DEFAULT' section for the workaround, instead of the traditional 'workarounds' section, is on purpose, so that the nova-compute charm options can be used to enable it; as done in the other case.)

Would an ACK from this particular affected user/customer be sufficient for this point?
I'm happy to check for additional things if needed!

> If we add this to Nova in focal it will make it react to ubuntu_libvirt_uefi_secboot_disable_s3 config option.
> Once such a user upgrades to Jammy, will it break, will it silently be ignored but working the same way, will it need a patch to jammy and later to handle such upgrades well?

Good point.
Unknown options are ignored on Focal with Yoga/UCA and Jammy (which ships Yoga), and need no additional handling in later/unaffected releases.

I have verified this today, with a juju openstack deployment on focal-yoga then do-release-upgrade the nova-compute unit from focal to jammy.

In all cases, an unknown config option was present in the DEFAULT section of the config file, and all service restarts happened successfully and the service stayed up/active.
I found no mention of errors or that option in /var/log/nova/nova-compute.log (it has a dump of all config options, and nothing mentioned it).

@ /etc/nova/nova.conf
[DEFAULT]
unknown_config_option=value

# systemctl s...

Read more...

Revision history for this message
dann frazier (dannf) wrote :

Note that edk2 upstream now supports S3 w/ 64-bit PEI, so we've now switched noble back over to it.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.