[PATCH 0/2] KVM features: two almost trivial cleanups
I've noticed these while reviewing a patch that touched this part of code. Michal Prívozník (2): qemuxml2xmloutdata: Turn kvm-features.xml and kvm-features-off.xml into symlinks domain_conf: Use virXMLFormatElement*() more in virDomainDefFormatFeatures() src/conf/domain_conf.c | 21 +++++----- tests/qemuxml2argvdata/kvm-features-off.xml | 7 +++- tests/qemuxml2argvdata/kvm-features.xml | 7 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 38 +------------------ tests/qemuxml2xmloutdata/kvm-features.xml | 38 +------------------ 5 files changed, 22 insertions(+), 89 deletions(-) mode change 100644 => 120000 tests/qemuxml2xmloutdata/kvm-features-off.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/kvm-features.xml -- 2.32.0
There's no real difference between input and output XMLs for kvm-features and kvm-features-off test cases. Do what we usually do in such case - turn the output file into a symlink of the input file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvdata/kvm-features-off.xml | 7 +++- tests/qemuxml2argvdata/kvm-features.xml | 7 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 38 +------------------ tests/qemuxml2xmloutdata/kvm-features.xml | 38 +------------------ 4 files changed, 12 insertions(+), 78 deletions(-) mode change 100644 => 120000 tests/qemuxml2xmloutdata/kvm-features-off.xml mode change 100644 => 120000 tests/qemuxml2xmloutdata/kvm-features.xml diff --git a/tests/qemuxml2argvdata/kvm-features-off.xml b/tests/qemuxml2argvdata/kvm-features-off.xml index fb7cbaf061..7ee6525cd9 100644 --- a/tests/qemuxml2argvdata/kvm-features-off.xml +++ b/tests/qemuxml2argvdata/kvm-features-off.xml @@ -18,17 +18,20 @@ <dirty-ring state='off'/> </kvm> </features> - <cpu mode='host-passthrough' check='none'/> + <cpu mode='host-passthrough' check='none' migratable='off'/> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-i386</emulator> - <controller type='usb' index='0'/> + <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'/> + <audio id='1' type='none'/> <memballoon model='none'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/kvm-features.xml b/tests/qemuxml2argvdata/kvm-features.xml index 900431c4ff..8ce3a2b987 100644 --- a/tests/qemuxml2argvdata/kvm-features.xml +++ b/tests/qemuxml2argvdata/kvm-features.xml @@ -18,17 +18,20 @@ <dirty-ring state='on' size='4096'/> </kvm> </features> - <cpu mode='host-passthrough' check='none'/> + <cpu mode='host-passthrough' check='none' migratable='off'/> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-i386</emulator> - <controller type='usb' index='0'/> + <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'/> + <audio id='1' type='none'/> <memballoon model='none'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/kvm-features-off.xml b/tests/qemuxml2xmloutdata/kvm-features-off.xml deleted file mode 100644 index 7ee6525cd9..0000000000 --- a/tests/qemuxml2xmloutdata/kvm-features-off.xml +++ /dev/null @@ -1,37 +0,0 @@ -<domain type='kvm'> - <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='i686' machine='pc'>hvm</type> - <boot dev='network'/> - </os> - <features> - <acpi/> - <kvm> - <hidden state='off'/> - <hint-dedicated state='off'/> - <poll-control state='off'/> - <pv-ipi state='off'/> - <dirty-ring state='off'/> - </kvm> - </features> - <cpu mode='host-passthrough' check='none' migratable='off'/> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-i386</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'/> - <audio id='1' type='none'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/kvm-features-off.xml b/tests/qemuxml2xmloutdata/kvm-features-off.xml new file mode 120000 index 0000000000..047170b59a --- /dev/null +++ b/tests/qemuxml2xmloutdata/kvm-features-off.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/kvm-features-off.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/kvm-features.xml b/tests/qemuxml2xmloutdata/kvm-features.xml deleted file mode 100644 index 8ce3a2b987..0000000000 --- a/tests/qemuxml2xmloutdata/kvm-features.xml +++ /dev/null @@ -1,37 +0,0 @@ -<domain type='kvm'> - <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='i686' machine='pc'>hvm</type> - <boot dev='network'/> - </os> - <features> - <acpi/> - <kvm> - <hidden state='on'/> - <hint-dedicated state='on'/> - <poll-control state='on'/> - <pv-ipi state='on'/> - <dirty-ring state='on' size='4096'/> - </kvm> - </features> - <cpu mode='host-passthrough' check='none' migratable='off'/> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-i386</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'/> - <audio id='1' type='none'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/kvm-features.xml b/tests/qemuxml2xmloutdata/kvm-features.xml new file mode 120000 index 0000000000..bda3acffac --- /dev/null +++ b/tests/qemuxml2xmloutdata/kvm-features.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/kvm-features.xml \ No newline at end of file -- 2.32.0
On Tue, Dec 14, 2021 at 03:01:18PM +0100, Michal Privoznik wrote:
qemuxml2xmloutdata: Turn kvm-features.xml and kvm-features-off.xml into symlinks
Maybe change this to qemuxml2xmloutdata: Turn kvm-features*.xml into symlinks for brevity's sake. Either way, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
There are few places in virDomainDefFormatFeatures() which can use virXMLFormatElement() or virXMLFormatElementEmpty() instead of writing directly into the output buffer. After this, there are still a lot of places left, but that is much bigger task. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cd87057524..060bd70de2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27783,12 +27783,12 @@ virDomainDefFormatFeatures(virBuffer *buf, break; case VIR_TRISTATE_SWITCH_ON: - virBufferAsprintf(&childBuf, "<%s state='on'/>\n", name); - break; - case VIR_TRISTATE_SWITCH_OFF: - virBufferAsprintf(&childBuf, "<%s state='off'/>\n", name); - break; + virBufferAsprintf(&tmpAttrBuf, " state='%s'", + virTristateSwitchTypeToString(def->features[i])); + + virXMLFormatElement(&childBuf, name, &tmpAttrBuf, NULL); + break; } break; @@ -27816,12 +27816,12 @@ virDomainDefFormatFeatures(virBuffer *buf, case VIR_DOMAIN_FEATURE_APIC: if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { - virBufferAddLit(&childBuf, "<apic"); if (def->apic_eoi) { - virBufferAsprintf(&childBuf, " eoi='%s'", + virBufferAsprintf(&tmpAttrBuf, " eoi='%s'", virTristateSwitchTypeToString(def->apic_eoi)); } - virBufferAddLit(&childBuf, "/>\n"); + + virXMLFormatElementEmpty(&childBuf, "apic", &tmpAttrBuf, NULL); } break; @@ -27999,11 +27999,10 @@ virDomainDefFormatFeatures(virBuffer *buf, case VIR_DOMAIN_FEATURE_GIC: if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { - virBufferAddLit(&childBuf, "<gic"); if (def->gic_version != VIR_GIC_VERSION_NONE) - virBufferAsprintf(&childBuf, " version='%s'", + virBufferAsprintf(&tmpAttrBuf, " version='%s'", virGICVersionTypeToString(def->gic_version)); - virBufferAddLit(&childBuf, "/>\n"); + virXMLFormatElementEmpty(&childBuf, "gic", &tmpAttrBuf, NULL); } break; -- 2.32.0
On Tue, Dec 14, 2021 at 03:01:19PM +0100, Michal Privoznik wrote:
case VIR_TRISTATE_SWITCH_ON: - virBufferAsprintf(&childBuf, "<%s state='on'/>\n", name); - break; - case VIR_TRISTATE_SWITCH_OFF: - virBufferAsprintf(&childBuf, "<%s state='off'/>\n", name); - break; + virBufferAsprintf(&tmpAttrBuf, " state='%s'", + virTristateSwitchTypeToString(def->features[i])); + + virXMLFormatElement(&childBuf, name, &tmpAttrBuf, NULL); + break;
You even fixed indentation as a side effect! Very nice :)
case VIR_DOMAIN_FEATURE_GIC: if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { - virBufferAddLit(&childBuf, "<gic"); if (def->gic_version != VIR_GIC_VERSION_NONE) - virBufferAsprintf(&childBuf, " version='%s'", + virBufferAsprintf(&tmpAttrBuf, " version='%s'", virGICVersionTypeToString(def->gic_version)); - virBufferAddLit(&childBuf, "/>\n"); + virXMLFormatElementEmpty(&childBuf, "gic", &tmpAttrBuf, NULL);
I think either adding an empty line before virXMLFormatElementEmpty() or braces around the check on def->gic_version would improve readability. Regardless Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Michal Privoznik