Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device
Eric,
> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Eric Blake
> Sent: Tuesday, May 28, 2013 7:08 PM
> Cc: libvir-list@redhat.com; dle-deve...@lists.sourceforge.net; Tomoki Sekiyama
> Subject: Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev
> device
>
> On 05/24/2013 04:46 PM, Eric Blake wrote:
> > From: Seiji Aguchi
> >
> > [Problem]
> > Currently, guest OS's messages can be logged to a local disk of host OS
> > by creating chadevs with options below.
> > -chardev file,id=charserial0,path= -device
> > isa-serial,chardev=chardevserial0,id=serial0
>
> >
> > Seiji's patch plus my initial review comments; I've ack'd this much,
> > but want to add a round-trip xml2xml test before pushing anything
> > (Seiji, if you want to use this as a starting point, rather than
> > waiting for my long weekend, feel free to post a v3).
>
>
> 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);
Thank you for handling/testing my patch.
>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 [/>
>
>
>
>
>
>
>
>new xml2xml test that proves we can round trip the new policy.
Anyway, I will post the v3 patch by fixing the issue.
Seiji
>
> 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, " \n",
> +virBufferEscapeString(buf, " 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 000..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 000..34e0eb9
> --- /dev/null
> +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
> @@ -0,0 +1,35 @@
> +
> + QEMUGuest1
> + c7a5fdbd-edaf-9455-926a-d65c16db1809
> + 219136
> + 219136
> + 1
> +
> +hvm
> +
> +
> +
> + destroy
> + restart
> + destroy
> +
> +/usr/bin/qemu
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> 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_C
Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device
On 05/24/2013 04:46 PM, Eric Blake wrote:
> From: Seiji Aguchi
>
> [Problem]
> Currently, guest OS's messages can be logged to a local disk of host OS
> by creating chadevs with options below.
> -chardev file,id=charserial0,path= -device
> isa-serial,chardev=chardevserial0,id=serial0
>
> Seiji's patch plus my initial review comments; I've ack'd this much,
> but want to add a round-trip xml2xml test before pushing anything
> (Seiji, if you want to use this as a starting point, rather than
> waiting for my long weekend, feel free to post a v3).
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 [/>
type != VIR_DOMAIN_CHR_TYPE_PTY ||
(def->data.file.path &&
!(flags & VIR_DOMAIN_XML_INACTIVE))) {
-virBufferEscapeString(buf, " \n",
+virBufferEscapeString(buf, " 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 000..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 000..34e0eb9
--- /dev/null
+++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
@@ -0,0 +1,35 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
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
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
