On 04/10/2015 03:09 PM, Andrea Bolognani wrote:
The tag is already marked as optional in the schema, so no changes
are needed there.
RHBZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1202606
---
Hi everyone,
this is my first contribution to libvirt so I wholeheartedly welcome
any criticism, suggestion or recommendation :)
Cheers.
src/conf/cpu_conf.c | 33 ++++++++++++++++------
tests/qemuxml2argvdata/qemuxml2argv-cpu-empty.xml | 23 +++++++++++++++
.../qemuxml2xmlout-cpu-empty.xml | 21 ++++++++++++++
tests/qemuxml2xmltest.c | 1 +
4 files changed, 69 insertions(+), 9 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-empty.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-empty.xml
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index cd3882d..8f65d55 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -435,13 +435,14 @@ virCPUDefFormatBufFull(virBufferPtr buf,
bool updateCPU)
{
int ret = -1;
+ virBuffer attributeBuf = VIR_BUFFER_INITIALIZER;
virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
int indent = virBufferGetIndent(buf, false);
if (!def)
return 0;
- virBufferAddLit(buf, "<cpu");
+ /* Format attributes */
if (def->type == VIR_CPU_TYPE_GUEST) {
const char *tmp;
@@ -451,7 +452,7 @@ virCPUDefFormatBufFull(virBufferPtr buf,
_("Unexpected CPU mode %d"), def->mode);
goto cleanup;
}
- virBufferAsprintf(buf, " mode='%s'", tmp);
+ virBufferAsprintf(&attributeBuf, " mode='%s'", tmp);
}
if (def->model &&
@@ -463,10 +464,11 @@ virCPUDefFormatBufFull(virBufferPtr buf,
def->match);
goto cleanup;
}
- virBufferAsprintf(buf, " match='%s'", tmp);
+ virBufferAsprintf(&attributeBuf, " match='%s'", tmp);
}
}
+ /* Format children */
virBufferAdjustIndent(&childrenBuf, indent + 2);
if (def->arch)
virBufferAsprintf(&childrenBuf, "<arch>%s</arch>\n",
@@ -477,16 +479,29 @@ virCPUDefFormatBufFull(virBufferPtr buf,
if (virDomainNumaDefCPUFormat(&childrenBuf, numa) < 0)
goto cleanup;
- if (virBufferUse(&childrenBuf)) {
- virBufferAddLit(buf, ">\n");
- virBufferAddBuffer(buf, &childrenBuf);
- virBufferAddLit(buf, "</cpu>\n");
- } else {
- virBufferAddLit(buf, "/>\n");
+ /* Put it all together */
+ if (virBufferUse(&attributeBuf) || virBufferUse(&childrenBuf)) {
+
+ /* Opening tag */
Just some minor nitpicks:
Although I love the idea of commenting the code to make it as much
understandable as possible :), I think this one ^^ comments the obvious...
+ virBufferAddLit(buf, "<cpu");
+
+ /* Attributes (if any) */
same here ^^...
+ if (virBufferUse(&attributeBuf))
+ virBufferAddBuffer(buf, &attributeBuf);
+
+ /* Children (if any) and closing tag */
same here ^^
+ if (virBufferUse(&childrenBuf)) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAddBuffer(buf, &childrenBuf);
+ virBufferAddLit(buf, "</cpu>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
}
ret = 0;
cleanup:
+ virBufferFreeAndReset(&attributeBuf);
virBufferFreeAndReset(&childrenBuf);
return ret;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-empty.xml
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-empty.xml
new file mode 100644
index 0000000..2a79826
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-empty.xml
@@ -0,0 +1,23 @@
+<domain type='kvm'>
+ <name>cpu-empty</name>
+ <uuid>1aed4c39-ad6e-4a78-9264-4ce996290d17</uuid>
+ <memory unit='KiB'>4000768</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-kvm</emulator>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-empty.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-empty.xml
new file mode 100644
index 0000000..e678607
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-empty.xml
@@ -0,0 +1,21 @@
+<domain type='kvm'>
+ <name>cpu-empty</name>
+ <uuid>1aed4c39-ad6e-4a78-9264-4ce996290d17</uuid>
+ <memory unit='KiB'>4000768</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-kvm</emulator>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 817e408..cd0b280 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -361,6 +361,7 @@ mymain(void)
DO_TEST("clock-utc");
DO_TEST("clock-localtime");
+ DO_TEST_DIFFERENT("cpu-empty");
DO_TEST("cpu-kvmclock");
DO_TEST("cpu-host-kvmclock");
DO_TEST("cpu-host-passthrough-features");
Other than that, it looks good, so I removed the comments marked above
and pushed. Congratulations to your first patch in libvirt ;).
Erik