[libvirt PATCH 0/2] cpu_conf: Format vendor_id for host-model CPUs
In commit v5.9.0-400-gaf8e39921a I removed printing model's fallback and vendor_id attributes when no model is specified. However, vendor_id makes sense even without a specific CPU model (for host-model CPUs). https://bugzilla.redhat.com/show_bug.cgi?id=1804549 Jiri Denemark (2): qemuxml2xmltest: Add case for host-model vendor_id cpu_conf: Format vendor_id for host-model CPUs src/conf/cpu_conf.c | 14 +++++---- .../cpu-host-model-vendor.xml | 30 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml -- 2.25.0
This patch show a bug in our code: the <model vendor_id="Libvirt QEMU"/> element present in the source XML is lost when the parsed CPU definition is formatted back to XML. https://bugzilla.redhat.com/show_bug.cgi?id=1804549 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- .../cpu-host-model-vendor.xml | 28 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 2 files changed, 29 insertions(+) create mode 100644 tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml diff --git a/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml b/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml new file mode 100644 index 0000000000..d2447ccd10 --- /dev/null +++ b/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu mode='host-model' check='partial'/> + <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'> + <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'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 98d9ae4242..daf9b53ce8 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -266,6 +266,7 @@ mymain(void) DO_TEST("cpu-host-kvmclock", NONE); DO_TEST("cpu-host-passthrough-features", NONE); DO_TEST("cpu-host-model-features", NONE); + DO_TEST("cpu-host-model-vendor", NONE); DO_TEST("clock-catchup", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("kvmclock", NONE); DO_TEST("clock-timer-hyperv-rtc", NONE); -- 2.25.0
On Wed, Feb 19, 2020 at 02:09:03PM +0100, Jiri Denemark wrote:
This patch show a bug in our code: the <model vendor_id="Libvirt QEMU"/>
s/show/shows/
element present in the source XML is lost when the parsed CPU definition is formatted back to XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1804549
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- .../cpu-host-model-vendor.xml | 28 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 2 files changed, 29 insertions(+) create mode 100644 tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
In commit v5.9.0-400-gaf8e39921a I removed printing model's fallback and vendor_id attributes when no model is specified. However, vendor_id makes sense even without a specific CPU model (for host-model CPUs). https://bugzilla.redhat.com/show_bug.cgi?id=1804549 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 14 +++++++++----- tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml | 4 +++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 8fca3df874..07404c6fb0 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -791,10 +791,10 @@ virCPUDefFormatBuf(virBufferPtr buf, return -1; } - if (formatModel && def->model) { + if (formatModel && (def->model || def->vendor_id)) { virBufferAddLit(buf, "<model"); - if (def->type == VIR_CPU_TYPE_GUEST) { + if (def->type == VIR_CPU_TYPE_GUEST && def->model) { const char *fallback; fallback = virCPUFallbackTypeToString(def->fallback); @@ -805,11 +805,15 @@ virCPUDefFormatBuf(virBufferPtr buf, return -1; } virBufferAsprintf(buf, " fallback='%s'", fallback); - if (def->vendor_id) - virBufferEscapeString(buf, " vendor_id='%s'", def->vendor_id); } - virBufferEscapeString(buf, ">%s</model>\n", def->model); + if (def->type == VIR_CPU_TYPE_GUEST) + virBufferEscapeString(buf, " vendor_id='%s'", def->vendor_id); + + if (def->model) + virBufferEscapeString(buf, ">%s</model>\n", def->model); + else + virBufferAddLit(buf, "/>\n"); } if (formatModel && def->vendor) diff --git a/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml b/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml index d2447ccd10..2a7d0246cc 100644 --- a/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml +++ b/tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml @@ -8,7 +8,9 @@ <type arch='x86_64' machine='pc'>hvm</type> <boot dev='network'/> </os> - <cpu mode='host-model' check='partial'/> + <cpu mode='host-model' check='partial'> + <model vendor_id='Libvirt QEMU'/> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> -- 2.25.0
On Wed, Feb 19, 2020 at 02:09:04PM +0100, Jiri Denemark wrote:
In commit v5.9.0-400-gaf8e39921a I removed printing model's fallback and vendor_id attributes when no model is specified. However, vendor_id makes sense even without a specific CPU model (for host-model CPUs).
https://bugzilla.redhat.com/show_bug.cgi?id=1804549
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 14 +++++++++----- tests/qemuxml2xmloutdata/cpu-host-model-vendor.xml | 4 +++- 2 files changed, 12 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Jiri Denemark -
Ján Tomko