Skip to content

Commit

Permalink
Show failed TPM addition errors
Browse files Browse the repository at this point in the history
Don't just sweep them under the rug -- this is actually important for
moving to EFI firmware. It'll become even more important with an
explicit "Add TPM" action that we'll add in the next step.

Unskip `testConfigureBeforeInstallBiosTPM` on RHEL 8 and Arch, and explicitly
check how it fails there.
  • Loading branch information
martinpitt authored and jelly committed Nov 5, 2024
1 parent da34dd2 commit 06cf932
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 22 deletions.
10 changes: 1 addition & 9 deletions src/libvirtApi/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -1082,13 +1082,5 @@ export async function domainReplaceSpice({ connectionName, id: objPath }) {

export async function domainAddTPM({ connectionName, vmName }) {
const args = ["virt-xml", "-c", `qemu:///${connectionName}`, "--add-device", "--tpm", "default", vmName];
return cockpit.spawn(args, { err: "message", superuser: connectionName === "system" ? "try" : null })
// RHEL 8 does not support --tpm default; arch (and likely other system setups) fails with
// unsupported configuration: TPM version '2.0' is not supported
.catch(ex => {
if (ex.message?.includes("Unknown TPM backend type") || ex.message?.includes("unsupported configuration"))
console.log("Failed to add TPM:", ex);
else
return Promise.reject(ex);
});
return cockpit.spawn(args, { err: "message", superuser: connectionName === "system" ? "try" : null });
}
39 changes: 26 additions & 13 deletions test/check-machines-create
Original file line number Diff line number Diff line change
Expand Up @@ -2172,12 +2172,17 @@ vnc_password= "{vnc_passwd}"
b.wait_visible(".pf-v5-c-modal-box__body")
b.select_from_dropdown(".pf-v5-c-modal-box__body select", "efi")
b.click("#firmware-dialog-apply")
# ack error message if adding TPM failed
if machineslib.hasBrokenTPM(m.image):
b.wait_in_text(".pf-v5-c-modal-box .pf-v5-c-alert", "Failed to change firmware")
b.wait_in_text(".pf-v5-c-modal-box .pf-v5-c-alert", "TPM")
b.click('.pf-v5-c-modal-box__footer button:contains("Cancel")')
b.wait_not_present(".pf-v5-c-modal-box__body")
b.wait_in_text("#vm-VmNotInstalled-firmware", "UEFI")
self.assertIn("<os firmware='efi'>", m.execute("virsh dumpxml VmNotInstalled"))

# auto-enabled software TPM (doesn't work on RHEL 8 or arch)
if not m.image.startswith("rhel-8") and m.image != 'arch':
# auto-enabled software TPM
if not machineslib.hasBrokenTPM(m.image):
tpm_xml = m.execute("virsh dumpxml VmNotInstalled | xmllint --xpath /domain/devices/tpm -")
self.assertIn('model="tpm-crb"', tpm_xml)
self.assertIn('type="emulator"', tpm_xml)
Expand Down Expand Up @@ -2294,8 +2299,6 @@ vnc_password= "{vnc_passwd}"
self.assertIn("<os>", domainXML)
self.assertNotIn("efi", domainXML)

@testlib.skipImage("does not support virt-xml --tpm", "rhel-8*")
@testlib.skipImage("does not support TPM 2.0", "arch")
def testConfigureBeforeInstallBiosTPM(self):
m = self.machine
b = self.browser
Expand All @@ -2317,24 +2320,34 @@ vnc_password= "{vnc_passwd}"
b.wait_in_text(f"#vm-{vmName}-firmware", "BIOS")

# manually add TPM to BIOS
m.execute(f"virt-xml --add-device --tpm default {vmName}")
self.assertIn("tpm", m.execute(f"virsh dumpxml {vmName}"))
# this doesn't change the UI, so we have to reload to ensure that the UI picks up the change
# otherwise this is a race condition
b.reload()
b.enter_page('/machines')
if not machineslib.hasBrokenTPM(m.image):
m.execute(f"virt-xml --add-device --tpm default {vmName}")
self.assertIn("tpm", m.execute(f"virsh dumpxml {vmName}"))

# this doesn't change the UI, so we have to reload to ensure that the UI picks up the change
# otherwise this is a race condition
b.reload()
b.enter_page('/machines')

# Change the os boot firmware to EFI
b.click(f"#vm-{vmName}-firmware")
b.wait_visible(".pf-v5-c-modal-box__body")
b.select_from_dropdown(".pf-v5-c-modal-box__body select", "efi")
b.click("#firmware-dialog-apply")
# ack error message if adding TPM failed
if machineslib.hasBrokenTPM(m.image):
b.wait_in_text(".pf-v5-c-modal-box .pf-v5-c-alert", "Failed to change firmware")
b.wait_in_text(".pf-v5-c-modal-box .pf-v5-c-alert", "TPM")
b.click('.pf-v5-c-modal-box__footer button:contains("Cancel")')
b.wait_not_present(".pf-v5-c-modal-box__body")
b.wait_in_text(f"#vm-{vmName}-firmware", "UEFI")

# did not add a second TPM; that would fail in the dialog anyway, but let's double-check the backend
out = m.execute(f"virsh dumpxml {vmName} | xmllint --xpath /domain/devices/tpm - | grep --count '<tpm'")
self.assertEqual(out.strip(), "1")
if machineslib.hasBrokenTPM(m.image):
self.assertNotIn("tpm", m.execute(f"virsh dumpxml {vmName}"))
else:
# did not add a second TPM; that would fail in the dialog anyway, but let's double-check the backend
out = m.execute(f"virsh dumpxml {vmName} | xmllint --xpath /domain/devices/tpm - | grep --count '<tpm'")
self.assertEqual(out.strip(), "1")

# Install the VM
b.click(f"#vm-{vmName}-system-install")
Expand Down
5 changes: 5 additions & 0 deletions test/machineslib.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ def hasMonolithicDaemon(image):
image in ["arch"])


# software TPM doesn't work on some OSes
def hasBrokenTPM(image):
return image.startswith("rhel-8") or image == 'arch'


class VirtualMachinesCaseHelpers:
machine: testvm.Machine

Expand Down

0 comments on commit 06cf932

Please sign in to comment.