[PATCH 0/4] Rework <tpm/> formatting

Please note that the test suite is temporarily broken after 2/4 but fixed in 3/4. This could be resolved be swapping those two patches, but I figured I keep the order to demonstrate the bug. However, I can do the swap if desired. Michal Prívozník (4): qemuxml2xmltest: Introduce tpm-emulator-spapr test qemuxml2xmloutdata: Turn tpm-*.xml files into symlinks conf: Rework <tpm/> formatting conf: Make virDomainTPMDefFormat() return void src/conf/domain_conf.c | 65 +++++++--------- tests/qemuxml2argvdata/tpm-emulator-spapr.xml | 74 +++++++++++-------- .../tpm-emulator-tpm2-enc.xml | 12 ++- .../tpm-emulator-tpm2-pstate.xml | 12 ++- tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 13 +++- tests/qemuxml2argvdata/tpm-emulator.xml | 12 ++- .../qemuxml2argvdata/tpm-passthrough-crb.xml | 12 ++- tests/qemuxml2argvdata/tpm-passthrough.xml | 12 ++- .../tpm-emulator-spapr.ppc64-latest.xml | 1 + .../tpm-emulator-tpm2-enc.x86_64-latest.xml | 41 +--------- ...tpm-emulator-tpm2-pstate.x86_64-latest.xml | 39 +--------- .../tpm-emulator-tpm2.x86_64-latest.xml | 44 +---------- .../tpm-emulator.x86_64-latest.xml | 39 +--------- .../tpm-passthrough-crb.x86_64-latest.xml | 41 +--------- .../tpm-passthrough.x86_64-latest.xml | 41 +--------- tests/qemuxml2xmltest.c | 1 + 16 files changed, 138 insertions(+), 321 deletions(-) create mode 120000 tests/qemuxml2xmloutdata/tpm-emulator-spapr.ppc64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml -- 2.34.1

We already have the input xml because of xml2arg test. However, the corresponding xml2xml test case is missing. Make the expected XML a symlink to the input XML and clean the latter up a bit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvdata/tpm-emulator-spapr.xml | 74 +++++++++++-------- .../tpm-emulator-spapr.ppc64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 3 files changed, 44 insertions(+), 32 deletions(-) create mode 120000 tests/qemuxml2xmloutdata/tpm-emulator-spapr.ppc64-latest.xml diff --git a/tests/qemuxml2argvdata/tpm-emulator-spapr.xml b/tests/qemuxml2argvdata/tpm-emulator-spapr.xml index f9cda19727..1b4b825e2c 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-spapr.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-spapr.xml @@ -12,49 +12,59 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>POWER9</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-ppc64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/tmp/scsidisk.img'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='1' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='ibmvscsi'> + <address type='spapr-vio' reg='0x00002000'/> + </controller> + <controller type='scsi' index='1' model='ibmvscsi'> + <address type='spapr-vio' reg='0x00003000'/> + </controller> + <controller type='usb' index='0' model='pci-ohci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <serial type='pty'> + <target type='spapr-vio-serial' port='0'> + <model name='spapr-vty'/> + </target> + <address type='spapr-vio' reg='0x30000000'/> + </serial> + <serial type='pty'> + <target type='spapr-vio-serial' port='1'> + <model name='spapr-vty'/> + </target> + <address type='spapr-vio' reg='0x30001000'/> + </serial> <console type='pty'> - <address type="spapr-vio"/> + <target type='serial' port='0'/> + <address type='spapr-vio' reg='0x30000000'/> </console> - - <!-- Two serials, first is the console --> - <serial type="pty"> - <address type="spapr-vio"/> - </serial> - <serial type="pty"> - <address type="spapr-vio"/> - </serial> - - <!-- One disk --> - <disk type="file" device="disk"> - <driver name="qemu" type="raw"/> - <source file="/tmp/scsidisk.img"/> - <target dev="sda" bus="scsi"/> - <address type="drive" controller="1"/> - </disk> - - <!-- Two SCSI controllers --> - <controller type="scsi" index="1"> - <address type="spapr-vio"/> - </controller> - <controller type="scsi" index="0"> - <address type="spapr-vio"/> - </controller> - - <nvram> - <address type='spapr-vio' reg='0x4000'/> - </nvram> - - <!-- TPM emulator --> <tpm model='tpm-spapr'> <backend type='emulator' version='2.0'/> + <address type='spapr-vio' reg='0x00005000'/> </tpm> - + <audio id='1' type='none'/> <memballoon model='none'/> + <nvram> + <address type='spapr-vio' reg='0x00004000'/> + </nvram> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-spapr.ppc64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-spapr.ppc64-latest.xml new file mode 120000 index 0000000000..d69355b81e --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-spapr.ppc64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-emulator-spapr.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 45e76bf9cc..c11d415e98 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -759,6 +759,7 @@ mymain(void) DO_TEST_CAPS_LATEST("tpm-passthrough"); DO_TEST_CAPS_LATEST("tpm-passthrough-crb"); DO_TEST_CAPS_LATEST("tpm-emulator"); + DO_TEST_CAPS_ARCH_LATEST("tpm-emulator-spapr", "ppc64"); DO_TEST_CAPS_LATEST("tpm-emulator-tpm2"); DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc"); DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate"); -- 2.34.1

On Tue, Jan 04, 2022 at 09:14:27 +0100, Michal Privoznik wrote:
We already have the input xml because of xml2arg test. However, the corresponding xml2xml test case is missing. Make the expected XML a symlink to the input XML and clean the latter up a bit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvdata/tpm-emulator-spapr.xml | 74 +++++++++++-------- .../tpm-emulator-spapr.ppc64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 3 files changed, 44 insertions(+), 32 deletions(-) create mode 120000 tests/qemuxml2xmloutdata/tpm-emulator-spapr.ppc64-latest.xml
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Make the tpm-*.xml files symlinks to their respective input XMLs from qemuxml2argvdata/ directory. This uncovers a bug in our <tpm/> formatter which formats an invalid XML if both <encryption/> and <active_pcr_banks/> elements are present for <backend/>. This is going to be addressed in the next commit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .../tpm-emulator-tpm2-enc.xml | 12 ++++- .../tpm-emulator-tpm2-pstate.xml | 12 ++++- tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 14 +++++- tests/qemuxml2argvdata/tpm-emulator.xml | 12 ++++- .../qemuxml2argvdata/tpm-passthrough-crb.xml | 12 ++++- tests/qemuxml2argvdata/tpm-passthrough.xml | 12 ++++- .../tpm-emulator-tpm2-enc.x86_64-latest.xml | 41 +---------------- ...tpm-emulator-tpm2-pstate.x86_64-latest.xml | 39 +--------------- .../tpm-emulator-tpm2.x86_64-latest.xml | 44 +------------------ .../tpm-emulator.x86_64-latest.xml | 39 +--------------- .../tpm-passthrough-crb.x86_64-latest.xml | 41 +---------------- .../tpm-passthrough.x86_64-latest.xml | 41 +---------------- 12 files changed, 68 insertions(+), 251 deletions(-) mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml index d889aae4f6..9c2279b28b 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml @@ -12,13 +12,18 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> @@ -27,6 +32,9 @@ <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/> </backend> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml index 45fc4c0e1a..42e93cfcbe 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml @@ -12,19 +12,27 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> <backend type='emulator' version='2.0' persistent_state='yes'/> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 68db8b9232..59dd68311f 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -12,24 +12,34 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> + <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> + > <active_pcr_banks> <sha256/> <sha512/> </active_pcr_banks> </backend> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2argvdata/tpm-emulator.xml b/tests/qemuxml2argvdata/tpm-emulator.xml index defc3789ad..b98a3693b7 100644 --- a/tests/qemuxml2argvdata/tpm-emulator.xml +++ b/tests/qemuxml2argvdata/tpm-emulator.xml @@ -12,19 +12,27 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> <backend type='emulator' version='1.2'/> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2argvdata/tpm-passthrough-crb.xml b/tests/qemuxml2argvdata/tpm-passthrough-crb.xml index 2fce5ca342..47c622bd84 100644 --- a/tests/qemuxml2argvdata/tpm-passthrough-crb.xml +++ b/tests/qemuxml2argvdata/tpm-passthrough-crb.xml @@ -12,13 +12,18 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> @@ -27,6 +32,9 @@ <device path='/dev/tpm0'/> </backend> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2argvdata/tpm-passthrough.xml b/tests/qemuxml2argvdata/tpm-passthrough.xml index 036091d44f..1555de4e86 100644 --- a/tests/qemuxml2argvdata/tpm-passthrough.xml +++ b/tests/qemuxml2argvdata/tpm-passthrough.xml @@ -12,13 +12,18 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> @@ -27,6 +32,9 @@ <device path='/dev/tpm0'/> </backend> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml deleted file mode 100644 index 9c2279b28b..0000000000 --- a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml +++ /dev/null @@ -1,40 +0,0 @@ -<domain type='qemu'> - <name>TPM-VM</name> - <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> - <memory unit='KiB'>2097152</memory> - <currentMemory unit='KiB'>512288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> - <boot dev='hd'/> - <bootmenu enable='yes'/> - </os> - <features> - <acpi/> - </features> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <tpm model='tpm-tis'> - <backend type='emulator' version='2.0'> - <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/> - </backend> - </tpm> - <audio id='1' type='none'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml new file mode 120000 index 0000000000..030f4f373d --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-emulator-tpm2-enc.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml deleted file mode 100644 index 42e93cfcbe..0000000000 --- a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml +++ /dev/null @@ -1,38 +0,0 @@ -<domain type='qemu'> - <name>TPM-VM</name> - <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> - <memory unit='KiB'>2097152</memory> - <currentMemory unit='KiB'>512288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> - <boot dev='hd'/> - <bootmenu enable='yes'/> - </os> - <features> - <acpi/> - </features> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <tpm model='tpm-tis'> - <backend type='emulator' version='2.0' persistent_state='yes'/> - </tpm> - <audio id='1' type='none'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml new file mode 120000 index 0000000000..eb65b59aac --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml deleted file mode 100644 index edab6db123..0000000000 --- a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml +++ /dev/null @@ -1,43 +0,0 @@ -<domain type='qemu'> - <name>TPM-VM</name> - <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> - <memory unit='KiB'>2097152</memory> - <currentMemory unit='KiB'>512288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> - <boot dev='hd'/> - <bootmenu enable='yes'/> - </os> - <features> - <acpi/> - </features> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <tpm model='tpm-tis'> - <backend type='emulator' version='2.0'> - <active_pcr_banks> - <sha256/> - <sha512/> - </active_pcr_banks> - </backend> - </tpm> - <audio id='1' type='none'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml new file mode 120000 index 0000000000..b8f1123553 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-emulator-tpm2.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml deleted file mode 100644 index b98a3693b7..0000000000 --- a/tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml +++ /dev/null @@ -1,38 +0,0 @@ -<domain type='qemu'> - <name>TPM-VM</name> - <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> - <memory unit='KiB'>2097152</memory> - <currentMemory unit='KiB'>512288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> - <boot dev='hd'/> - <bootmenu enable='yes'/> - </os> - <features> - <acpi/> - </features> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <tpm model='tpm-tis'> - <backend type='emulator' version='1.2'/> - </tpm> - <audio id='1' type='none'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml new file mode 120000 index 0000000000..3ddc89fc94 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-emulator.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml deleted file mode 100644 index 47c622bd84..0000000000 --- a/tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml +++ /dev/null @@ -1,40 +0,0 @@ -<domain type='qemu'> - <name>TPM-VM</name> - <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> - <memory unit='KiB'>2097152</memory> - <currentMemory unit='KiB'>512288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> - <boot dev='hd'/> - <bootmenu enable='yes'/> - </os> - <features> - <acpi/> - </features> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <tpm model='tpm-crb'> - <backend type='passthrough'> - <device path='/dev/tpm0'/> - </backend> - </tpm> - <audio id='1' type='none'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml new file mode 120000 index 0000000000..2f5f021ee6 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-passthrough-crb.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-passthrough-crb.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml deleted file mode 100644 index 1555de4e86..0000000000 --- a/tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml +++ /dev/null @@ -1,40 +0,0 @@ -<domain type='qemu'> - <name>TPM-VM</name> - <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> - <memory unit='KiB'>2097152</memory> - <currentMemory unit='KiB'>512288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> - <boot dev='hd'/> - <bootmenu enable='yes'/> - </os> - <features> - <acpi/> - </features> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <tpm model='tpm-tis'> - <backend type='passthrough'> - <device path='/dev/tpm0'/> - </backend> - </tpm> - <audio id='1' type='none'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml new file mode 120000 index 0000000000..2fbd46cad0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-passthrough.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/tpm-passthrough.xml \ No newline at end of file -- 2.34.1

On Tue, Jan 04, 2022 at 09:14:28 +0100, Michal Privoznik wrote:
Make the tpm-*.xml files symlinks to their respective input XMLs from qemuxml2argvdata/ directory. This uncovers a bug in our <tpm/> formatter which formats an invalid XML if both <encryption/> and <active_pcr_banks/> elements are present for <backend/>. This is going to be addressed in the next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
[...]
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 68db8b9232..59dd68311f 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -12,24 +12,34 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> + <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> + >
Summary of Failures: 253/315 libvirt / virschematest FAIL 1.47s exit status 1
<active_pcr_banks> <sha256/> <sha512/> </active_pcr_banks> </backend> </tpm> - <memballoon model='virtio'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> </devices> </domain>

On 1/4/22 09:30, Peter Krempa wrote:
On Tue, Jan 04, 2022 at 09:14:28 +0100, Michal Privoznik wrote:
Make the tpm-*.xml files symlinks to their respective input XMLs from qemuxml2argvdata/ directory. This uncovers a bug in our <tpm/> formatter which formats an invalid XML if both <encryption/> and <active_pcr_banks/> elements are present for <backend/>. This is going to be addressed in the next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
[...]
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 68db8b9232..59dd68311f 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -12,24 +12,34 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> + <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> + >
Summary of Failures:
253/315 libvirt / virschematest FAIL 1.47s exit status 1
To cite from the cover letter: Please note that the test suite is temporarily broken after 2/4 but fixed in 3/4. This could be resolved be swapping those two patches, but I figured I keep the order to demonstrate the bug. However, I can do the swap if desired. Michal

On Tue, Jan 04, 2022 at 09:31:39 +0100, Michal Prívozník wrote:
On 1/4/22 09:30, Peter Krempa wrote:
On Tue, Jan 04, 2022 at 09:14:28 +0100, Michal Privoznik wrote:
Make the tpm-*.xml files symlinks to their respective input XMLs from qemuxml2argvdata/ directory. This uncovers a bug in our <tpm/> formatter which formats an invalid XML if both <encryption/> and <active_pcr_banks/> elements are present for <backend/>. This is going to be addressed in the next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
[...]
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 68db8b9232..59dd68311f 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -12,24 +12,34 @@ <features> <acpi/> </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> + <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> + >
Summary of Failures:
253/315 libvirt / virschematest FAIL 1.47s exit status 1
To cite from the cover letter:
Please note that the test suite is temporarily broken after 2/4 but fixed in 3/4. This could be resolved be swapping those two patches, but I figured I keep the order to demonstrate the bug. However, I can do the swap if desired.
https://www.libvirt.org/hacking.html#preparing-patches "If you're going to submit multiple patches, the automated tests must pass after each patch, not just after the last one."

On 1/4/22 09:33, Peter Krempa wrote:
https://www.libvirt.org/hacking.html#preparing-patches
"If you're going to submit multiple patches, the automated tests must pass after each patch, not just after the last one."
Alright then. Let me swap those two patches and post v2. Michal

The <tpm/> element formatting is handled in virDomainTPMDefFormat() which uses the "old style" - appending strings directly into the output buffer. With this, it's easy to get conditions that tell when an element has ended wrong. In this particular case, if both <encryption/> and <active_pcr_banks/> are to be formatted the current code puts a stray '>' into the output buffer, resulting in invalid XML. Rewrite the function to use virXMLFormatElement() which is more clever. https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 53 ++++++++------------ tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 1 - 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 716c6d2240..b8fef8586c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25481,63 +25481,54 @@ virDomainTPMDefFormat(virBuffer *buf, virDomainTPMDef *def, unsigned int flags) { - virBufferAsprintf(buf, "<tpm model='%s'>\n", + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) backendAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) backendBuf = VIR_BUFFER_INIT_CHILD(&childBuf); + + virBufferAsprintf(&attrBuf, " model='%s'", virDomainTPMModelTypeToString(def->model)); - virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<backend type='%s'", + + virBufferAsprintf(&backendAttrBuf, " type='%s'", virDomainTPMBackendTypeToString(def->type)); switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<device path='%s'/>\n", + virBufferEscapeString(&backendBuf, "<device path='%s'/>\n", def->data.passthrough.source->data.file.path); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</backend>\n"); break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: - virBufferAsprintf(buf, " version='%s'", + virBufferAsprintf(&backendAttrBuf, " version='%s'", virDomainTPMVersionTypeToString(def->version)); if (def->data.emulator.persistent_state) - virBufferAddLit(buf, " persistent_state='yes'"); + virBufferAddLit(&backendAttrBuf, " persistent_state='yes'"); if (def->data.emulator.hassecretuuid) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<encryption secret='%s'/>\n", - virUUIDFormat(def->data.emulator.secretuuid, uuidstr)); - virBufferAdjustIndent(buf, -2); + + virBufferAsprintf(&backendBuf, "<encryption secret='%s'/>\n", + virUUIDFormat(def->data.emulator.secretuuid, uuidstr)); } if (def->data.emulator.activePcrBanks) { + g_auto(virBuffer) activePcrBanksBuf = VIR_BUFFER_INIT_CHILD(&backendBuf); size_t i; - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferAddLit(buf, "<active_pcr_banks>\n"); - virBufferAdjustIndent(buf, 2); + for (i = VIR_DOMAIN_TPM_PCR_BANK_SHA1; i < VIR_DOMAIN_TPM_PCR_BANK_LAST; i++) { if ((def->data.emulator.activePcrBanks & (1 << i))) - virBufferAsprintf(buf, "<%s/>\n", + virBufferAsprintf(&activePcrBanksBuf, "<%s/>\n", virDomainTPMPcrBankTypeToString(i)); } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</active_pcr_banks>\n"); - virBufferAdjustIndent(buf, -2); + + virXMLFormatElement(&backendBuf, "active_pcr_banks", NULL, &activePcrBanksBuf); } - if (def->data.emulator.hassecretuuid || - def->data.emulator.activePcrBanks) - virBufferAddLit(buf, "</backend>\n"); - else - virBufferAddLit(buf, "/>\n"); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } - virDomainDeviceInfoFormat(buf, &def->info, flags); + virXMLFormatElement(&childBuf, "backend", &backendAttrBuf, &backendBuf); + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</tpm>\n"); + virXMLFormatElement(buf, "tpm", &attrBuf, &childBuf); return 0; } diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 59dd68311f..79acde218b 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -30,7 +30,6 @@ <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> - > <active_pcr_banks> <sha256/> <sha512/> -- 2.34.1

On Tue, Jan 04, 2022 at 09:14:29 +0100, Michal Privoznik wrote:
The <tpm/> element formatting is handled in virDomainTPMDefFormat() which uses the "old style" - appending strings directly into the output buffer. With this, it's easy to get conditions that tell when an element has ended wrong. In this particular case, if both <encryption/> and <active_pcr_banks/> are to be formatted the current code puts a stray '>' into the output buffer, resulting in invalid XML.
Rewrite the function to use virXMLFormatElement() which is more clever.
https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 53 ++++++++------------ tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 1 - 2 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 716c6d2240..b8fef8586c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25481,63 +25481,54 @@ virDomainTPMDefFormat(virBuffer *buf, virDomainTPMDef *def, unsigned int flags) { - virBufferAsprintf(buf, "<tpm model='%s'>\n", + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) backendAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) backendBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
Cannonically this would be 'backendChildBuf'. [...] Above code: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 59dd68311f..79acde218b 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -30,7 +30,6 @@ <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> - > <active_pcr_banks> <sha256/> <sha512/> -- 2.34.1

The virDomainTPMDefFormat() function can't fail really. There's no point in it returning an integer then. Make it return void and fix both places which check for its retval. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8fef8586c..509f74c092 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25476,9 +25476,9 @@ virDomainSoundCodecDefFormat(virBuffer *buf, return 0; } -static int +static void virDomainTPMDefFormat(virBuffer *buf, - virDomainTPMDef *def, + const virDomainTPMDef *def, unsigned int flags) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; @@ -25529,8 +25529,6 @@ virDomainTPMDefFormat(virBuffer *buf, virDomainDeviceInfoFormat(&childBuf, &def->info, flags); virXMLFormatElement(buf, "tpm", &attrBuf, &childBuf); - - return 0; } @@ -28517,8 +28515,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, } for (n = 0; n < def->ntpms; n++) { - if (virDomainTPMDefFormat(buf, def->tpms[n], flags) < 0) - return -1; + virDomainTPMDefFormat(buf, def->tpms[n], flags); } for (n = 0; n < def->ngraphics; n++) { @@ -29748,7 +29745,8 @@ virDomainDeviceDefCopy(virDomainDeviceDef *src, rc = virDomainChrDefFormat(&buf, src->data.chr, flags); break; case VIR_DOMAIN_DEVICE_TPM: - rc = virDomainTPMDefFormat(&buf, src->data.tpm, flags); + virDomainTPMDefFormat(&buf, src->data.tpm, flags); + rc = 0; break; case VIR_DOMAIN_DEVICE_PANIC: virDomainPanicDefFormat(&buf, src->data.panic); -- 2.34.1

On Tue, Jan 04, 2022 at 09:14:30 +0100, Michal Privoznik wrote:
The virDomainTPMDefFormat() function can't fail really. There's no point in it returning an integer then. Make it return void and fix both places which check for its retval.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8fef8586c..509f74c092 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25476,9 +25476,9 @@ virDomainSoundCodecDefFormat(virBuffer *buf, return 0; }
-static int +static void virDomainTPMDefFormat(virBuffer *buf, - virDomainTPMDef *def, + const virDomainTPMDef *def,
This isn't accounted for in the commit message.
unsigned int flags) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; @@ -25529,8 +25529,6 @@ virDomainTPMDefFormat(virBuffer *buf, virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
virXMLFormatElement(buf, "tpm", &attrBuf, &childBuf); - - return 0; }
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

On Tue, Jan 04, 2022 at 09:14:26AM +0100, Michal Privoznik wrote:
Please note that the test suite is temporarily broken after 2/4 but fixed in 3/4. This could be resolved be swapping those two patches, but I figured I keep the order to demonstrate the bug. However, I can do the swap if desired.
They certainly must be swapped before this is pushed, otherwise it breaks 'git bisect' for anyone using 'meson test' as their validator for the bisect. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Daniel P. Berrangé
-
Michal Privoznik
-
Michal Prívozník
-
Peter Krempa