Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device

On 05/24/2013 04:46 PM, Eric Blake wrote: Sorry, but this needs a v3, and I'm going to hand it back to you to fix. I tried adding a testsuite, but it proved that you weren't outputting the startupPolicy during dumpxml formatting. My initial attempt to add it backfired (I'll post it below); I'm worried that we are playing too fast and loose with a union type, since even with my patch in place, 'make check' fails with problems like: 84) QEMU XML-2-XML serial-dev ... Offset 913 Expect [/> <target port='0'/> </serial> <console type='dev'> <source path='/dev/ttyS2] Actual [ startupPolicy='(null)'/> <target port='0'/> </serial> <console type='dev'> <source path='/dev/ttyS2' startupPolicy='(null)] which is a bug, since you said startupPolicy should only be tied to files and not other <serial> types. Do we need to repeat the startupPolicy on the <console> mirror of the first <serial> device, or is that unnecessary back-compat, and where we can safely declare that startupPolicy is only emitted for the <serial> side of things? All told, your patch is not complete until it passes 'make check' with a new xml2xml test that proves we can round trip the new policy. diff --git c/src/conf/domain_conf.c w/src/conf/domain_conf.c index c24a9f0..5af5e40 100644 --- c/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -14547,8 +14547,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || (def->data.file.path && !(flags & VIR_DOMAIN_XML_INACTIVE))) { - virBufferEscapeString(buf, " <source path='%s'/>\n", + virBufferEscapeString(buf, " <source path='%s'", def->data.file.path); + + if (def->data.file.path && def->data.file.startupPolicy) { + const char *policy = virDomainStartupPolicyTypeToString(def->data.file.startupPolicy); + virBufferAsprintf(buf, " startupPolicy='%s'", policy); + } + virBufferAddLit(buf, "/>\n"); } break; diff --git c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args new file mode 100644 index 0000000..24977f2 --- /dev/null +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi -boot c -usb -hdc /tmp/idedisk.img \ +-chardev file,id=charserial0,path=/tmp/serial.log \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml new file mode 100644 index 0000000..34e0eb9 --- /dev/null +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' 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</emulator> + <disk type='file' device='disk'> + <source file='/tmp/idedisk.img'/> + <target dev='hdc' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='2'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='file'> + <source path='/tmp/serial.log' startupPolicy='optional'/> + <target port='0'/> + </serial> + <console type='file'> + <source path='/tmp/serial.log'/> + <target type='serial' port='0'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff --git c/tests/qemuxml2argvtest.c w/tests/qemuxml2argvtest.c index 2c7fd01..22f4782 100644 --- c/tests/qemuxml2argvtest.c +++ w/tests/qemuxml2argvtest.c @@ -727,6 +727,8 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("console-compat-chardev", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("serial-source-optional", + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-guestfwd", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); diff --git c/tests/qemuxml2xmltest.c w/tests/qemuxml2xmltest.c index 64a271c..1147519 100644 --- c/tests/qemuxml2xmltest.c +++ w/tests/qemuxml2xmltest.c @@ -86,6 +86,8 @@ testCompareXMLToXMLHelper(const void *data) ret = testCompareXMLToXMLFiles(xml_in, info->different ? xml_out : xml_in, false); + if (ret < 0) + goto cleanup; } if (info->when & WHEN_ACTIVE) { ret = testCompareXMLToXMLFiles(xml_in, @@ -231,6 +233,7 @@ mymain(void) DO_TEST("console-virtio-many"); DO_TEST("channel-guestfwd"); DO_TEST("channel-virtio"); + DO_TEST("serial-source-optional"); DO_TEST("hostdev-usb-address"); DO_TEST("hostdev-pci-address"); -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org