[libvirt] [PATCH] Introduce startupPolicy for chardev to make guest OS bootable when hardware failure happens in log disk
[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=<log file's path> -device isa-serial,chardev=chardevserial0,id=serial0 When a hardware failure happens in the disk, qemu-kvm can't create the chardevs. In this case, guest OS doesn't boot up. Actually, there are users who don't desire that guest OS goes down due to a hardware failure of a log disk only.Therefore, qemu should offer some way to boot guest OS up even if the log disk is broken. [Solution] This patch supports startupPolicy for chardev. The starupPolicy is introduced just in case where chardev is "file" because this patch aims for making guest OS boot up when a hardware failure happens. In other cases ,pty, dev, pipe and unix, it is not introduced because they don't access to hardware. The policy works as follows. - If the value is "optional", guestOS boots up by dropping the chardev. - If other values are specified, guestOS fails to boot up. (the default) Description about original startupPolicy attribute: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=e5a84d74a2789a917bf394f... Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com> --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 25 ++++++++++++++++++++++++- tests/virt-aa-helper-test | 3 +++ 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f325c3c..1e1bf27 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4044,13 +4044,20 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. + It is possible to define policy whether guestOS boots up + if the file is not accessible. This is done by a startupPolicy + attribute: + <ul> + <li>If the vaule is "optional", guestOS boots up by dropping the file.</li> + <li>If other values are specified, guestOS fails to boot up. (the default)</li> + </ul> </p> <pre> ... <devices> <serial type="file"> - <source path="/var/log/vm/vm-serial.log"/> + <source path="/var/log/vm/vm-serial.log" startupPolicy="optional"/> <target port="1"/> </serial> </devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10596dc..6fc0a3c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2706,6 +2706,9 @@ </optional> <optional> <attribute name="path"/> + <optional> + <ref name='startupPolicy'/> + </optional> </optional> <optional> <attribute name="host"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a8b5dfd..6680f15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6467,6 +6467,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *path = NULL; char *mode = NULL; char *protocol = NULL; + char *startupPolicy = NULL; int remaining = 0; while (cur != NULL) { @@ -6487,6 +6488,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, !(flags & VIR_DOMAIN_XML_INACTIVE))) path = virXMLPropString(cur, "path"); + if (startupPolicy == NULL && + def->type == VIR_DOMAIN_CHR_TYPE_FILE) + startupPolicy = virXMLPropString(cur, "startupPolicy"); break; case VIR_DOMAIN_CHR_TYPE_UDP: @@ -6559,6 +6563,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, def->data.file.path = path; path = NULL; + + def->data.file.startupPolicy = + virDomainStartupPolicyTypeFromString(startupPolicy); + startupPolicy = NULL; break; case VIR_DOMAIN_CHR_TYPE_STDIO: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a0f23a..e709951 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1052,6 +1052,7 @@ struct _virDomainChrSourceDef { /* no <source> for null, vc, stdio */ struct { char *path; + int startupPolicy; /* enum virDomainStartupPolicy */ } file; /* pty, file, pipe, or device */ struct { char *host; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e75c8c9..6e2f78e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2442,7 +2442,30 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virReportSystemError(errno, _("Unable to pre-create chardev file '%s'"), dev->source.data.file.path); - return -1; + if (dev->source.data.file.startupPolicy != + VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + return -1; + } + VIR_FREE(dev->source.data.file.path); + /* + * Change a destination to /dev/null to boot guest OS up + * even if a log disk is broken. + */ + VIR_WARN("Switch the destination to /dev/null"); + dev->source.data.file.path = strdup("/dev/null"); + + if (!(dev->source.data.file.path)) { + virReportOOMError(); + return -1; + } + + if ((fd = open(dev->source.data.file.path, + O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { + virReportSystemError(errno, + _("Unable to pre-create chardev file '%s'"), + dev->source.data.file.path); + return -1; + } } VIR_FORCE_CLOSE(fd); diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index af91c61..7172fd6 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -255,6 +255,9 @@ testme "0" "disk (empty cdrom)" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log' startupPolicy='optional'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" +testme "0" "serial" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='pty'><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial (pty)" "-r -u $valid_uuid" "$test_xml" -- 1.7.1
Any comments?
-----Original Message----- From: Seiji Aguchi Sent: Thursday, May 02, 2013 1:26 PM To: libvir-list@redhat.com Cc: dle-develop@lists.sourceforge.net; Tomoki Sekiyama (tomoki.sekiyama@hds.com) Subject: [PATCH] Introduce startupPolicy for chardev to make guest OS bootable when hardware failure happens in log disk
[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=<log file's path> -device isa-serial,chardev=chardevserial0,id=serial0
When a hardware failure happens in the disk, qemu-kvm can't create the chardevs. In this case, guest OS doesn't boot up.
Actually, there are users who don't desire that guest OS goes down due to a hardware failure of a log disk only.Therefore, qemu should offer some way to boot guest OS up even if the log disk is broken.
[Solution] This patch supports startupPolicy for chardev.
The starupPolicy is introduced just in case where chardev is "file" because this patch aims for making guest OS boot up when a hardware failure happens.
In other cases ,pty, dev, pipe and unix, it is not introduced because they don't access to hardware.
The policy works as follows. - If the value is "optional", guestOS boots up by dropping the chardev. - If other values are specified, guestOS fails to boot up. (the default)
Description about original startupPolicy attribute: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=e5a84d74a2789a917bf394f...
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com> --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 25 ++++++++++++++++++++++++- tests/virt-aa-helper-test | 3 +++ 6 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f325c3c..1e1bf27 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4044,13 +4044,20 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. + It is possible to define policy whether guestOS boots up + if the file is not accessible. This is done by a startupPolicy + attribute: + <ul> + <li>If the vaule is "optional", guestOS boots up by dropping the file.</li> + <li>If other values are specified, guestOS fails to boot up. (the default)</li> + </ul> </p>
<pre> ... <devices> <serial type="file"> - <source path="/var/log/vm/vm-serial.log"/> + <source path="/var/log/vm/vm-serial.log" startupPolicy="optional"/> <target port="1"/> </serial> </devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10596dc..6fc0a3c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2706,6 +2706,9 @@ </optional> <optional> <attribute name="path"/> + <optional> + <ref name='startupPolicy'/> + </optional> </optional> <optional> <attribute name="host"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a8b5dfd..6680f15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6467,6 +6467,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *path = NULL; char *mode = NULL; char *protocol = NULL; + char *startupPolicy = NULL; int remaining = 0;
while (cur != NULL) { @@ -6487,6 +6488,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, !(flags & VIR_DOMAIN_XML_INACTIVE))) path = virXMLPropString(cur, "path");
+ if (startupPolicy == NULL && + def->type == VIR_DOMAIN_CHR_TYPE_FILE) + startupPolicy = virXMLPropString(cur, "startupPolicy"); break;
case VIR_DOMAIN_CHR_TYPE_UDP: @@ -6559,6 +6563,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
def->data.file.path = path; path = NULL; + + def->data.file.startupPolicy = + virDomainStartupPolicyTypeFromString(startupPolicy); + startupPolicy = NULL; break;
case VIR_DOMAIN_CHR_TYPE_STDIO: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a0f23a..e709951 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1052,6 +1052,7 @@ struct _virDomainChrSourceDef { /* no <source> for null, vc, stdio */ struct { char *path; + int startupPolicy; /* enum virDomainStartupPolicy */ } file; /* pty, file, pipe, or device */ struct { char *host; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e75c8c9..6e2f78e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2442,7 +2442,30 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virReportSystemError(errno, _("Unable to pre-create chardev file '%s'"), dev->source.data.file.path); - return -1; + if (dev->source.data.file.startupPolicy != + VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + return -1; + } + VIR_FREE(dev->source.data.file.path); + /* + * Change a destination to /dev/null to boot guest OS up + * even if a log disk is broken. + */ + VIR_WARN("Switch the destination to /dev/null"); + dev->source.data.file.path = strdup("/dev/null"); + + if (!(dev->source.data.file.path)) { + virReportOOMError(); + return -1; + } + + if ((fd = open(dev->source.data.file.path, + O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { + virReportSystemError(errno, + _("Unable to pre-create chardev file '%s'"), + dev->source.data.file.path); + return -1; + } }
VIR_FORCE_CLOSE(fd); diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index af91c61..7172fd6 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -255,6 +255,9 @@ testme "0" "disk (empty cdrom)" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial" "-r -u $valid_uuid" "$test_xml"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log' startupPolicy='optional'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" +testme "0" "serial" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='pty'><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial (pty)" "-r -u $valid_uuid" "$test_xml"
-- 1.7.1
On 05/10/2013 06:10 PM, Seiji Aguchi wrote:
Any comments?
Apologies that no one has spoken up yet, but this is on my list of things to review. I got waylaid by debugging two nasty races today, or I would have had time to actually give comments; but come Monday, you should have some better feedback.
-----Original Message----- From: Seiji Aguchi Sent: Thursday, May 02, 2013 1:26 PM To: libvir-list@redhat.com Cc: dle-develop@lists.sourceforge.net; Tomoki Sekiyama (tomoki.sekiyama@hds.com) Subject: [PATCH] Introduce startupPolicy for chardev to make guest OS bootable when hardware failure happens in log disk
[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=<log file's path> -device isa-serial,chardev=chardevserial0,id=serial0
When a hardware failure happens in the disk, qemu-kvm can't create the chardevs. In this case, guest OS doesn't boot up.
Actually, there are users who don't desire that guest OS goes down due to a hardware failure of a log disk only.Therefore, qemu should offer some way to boot guest OS up even if the log disk is broken.
[Solution] This patch supports startupPolicy for chardev.
The starupPolicy is introduced just in case where chardev is "file" because this patch aims for making guest OS boot up when a hardware failure happens.
In other cases ,pty, dev, pipe and unix, it is not introduced because they don't access to hardware.
The policy works as follows. - If the value is "optional", guestOS boots up by dropping the chardev. - If other values are specified, guestOS fails to boot up. (the default)
Description about original startupPolicy attribute: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=e5a84d74a2789a917bf394f...
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com> --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 25 ++++++++++++++++++++++++- tests/virt-aa-helper-test | 3 +++ 6 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f325c3c..1e1bf27 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4044,13 +4044,20 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. + It is possible to define policy whether guestOS boots up + if the file is not accessible. This is done by a startupPolicy + attribute: + <ul> + <li>If the vaule is "optional", guestOS boots up by dropping the file.</li> + <li>If other values are specified, guestOS fails to boot up. (the default)</li> + </ul> </p>
<pre> ... <devices> <serial type="file"> - <source path="/var/log/vm/vm-serial.log"/> + <source path="/var/log/vm/vm-serial.log" startupPolicy="optional"/> <target port="1"/> </serial> </devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10596dc..6fc0a3c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2706,6 +2706,9 @@ </optional> <optional> <attribute name="path"/> + <optional> + <ref name='startupPolicy'/> + </optional> </optional> <optional> <attribute name="host"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a8b5dfd..6680f15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6467,6 +6467,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *path = NULL; char *mode = NULL; char *protocol = NULL; + char *startupPolicy = NULL; int remaining = 0;
while (cur != NULL) { @@ -6487,6 +6488,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, !(flags & VIR_DOMAIN_XML_INACTIVE))) path = virXMLPropString(cur, "path");
+ if (startupPolicy == NULL && + def->type == VIR_DOMAIN_CHR_TYPE_FILE) + startupPolicy = virXMLPropString(cur, "startupPolicy"); break;
case VIR_DOMAIN_CHR_TYPE_UDP: @@ -6559,6 +6563,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
def->data.file.path = path; path = NULL; + + def->data.file.startupPolicy = + virDomainStartupPolicyTypeFromString(startupPolicy); + startupPolicy = NULL; break;
case VIR_DOMAIN_CHR_TYPE_STDIO: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a0f23a..e709951 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1052,6 +1052,7 @@ struct _virDomainChrSourceDef { /* no <source> for null, vc, stdio */ struct { char *path; + int startupPolicy; /* enum virDomainStartupPolicy */ } file; /* pty, file, pipe, or device */ struct { char *host; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e75c8c9..6e2f78e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2442,7 +2442,30 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virReportSystemError(errno, _("Unable to pre-create chardev file '%s'"), dev->source.data.file.path); - return -1; + if (dev->source.data.file.startupPolicy != + VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + return -1; + } + VIR_FREE(dev->source.data.file.path); + /* + * Change a destination to /dev/null to boot guest OS up + * even if a log disk is broken. + */ + VIR_WARN("Switch the destination to /dev/null"); + dev->source.data.file.path = strdup("/dev/null"); + + if (!(dev->source.data.file.path)) { + virReportOOMError(); + return -1; + } + + if ((fd = open(dev->source.data.file.path, + O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { + virReportSystemError(errno, + _("Unable to pre-create chardev file '%s'"), + dev->source.data.file.path); + return -1; + } }
VIR_FORCE_CLOSE(fd); diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index af91c61..7172fd6 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -255,6 +255,9 @@ testme "0" "disk (empty cdrom)" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial" "-r -u $valid_uuid" "$test_xml"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log' startupPolicy='optional'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" +testme "0" "serial" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='pty'><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial (pty)" "-r -u $valid_uuid" "$test_xml"
-- 1.7.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/02/2013 11:26 AM, Seiji Aguchi wrote: The subject line is a bit long for a patch. 'git shortlog -30' will give you an idea of typical subject lines; I'm modifying this to: xml: introduce startupPolicy for chardev device
[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=<log file's path> -device isa-serial,chardev=chardevserial0,id=serial0
This long line is okay (quoting a command line)...
When a hardware failure happens in the disk, qemu-kvm can't create the chardevs. In this case, guest OS doesn't boot up.
Actually, there are users who don't desire that guest OS goes down due to a hardware failure of a log disk only.Therefore, qemu should offer some way to boot guest OS up even if the log
...but here, wrapping below 80 characters is desirable. Also, space after '.' ending a sentence, in English. So I rewrapped it.
disk is broken.
[Solution] This patch supports startupPolicy for chardev.
The starupPolicy is introduced just in case where chardev is "file"
s/case/cases/
because this patch aims for making guest OS boot up when a hardware failure happens.
In other cases ,pty, dev, pipe and unix, it is not introduced because they don't access to hardware.
The policy works as follows. - If the value is "optional", guestOS boots up by dropping the chardev. - If other values are specified, guestOS fails to boot up. (the default)
Description about original startupPolicy attribute: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=e5a84d74a2789a917bf394f...
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com> --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 3 +++
Good - adding documentation alongside the feature.
src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 25 ++++++++++++++++++++++++-
It's a bit nicer to split the XML addition and a driver's implementation of the XML (makes backporting easier if we later implement in a different driver, and want to backport the second implementation without the first); but this patch is small enough that I'm not too worried about it.
tests/virt-aa-helper-test | 3 +++
This added one test, but not quite enough. I generally also expect a .xml and .args that shows the new XML in use, and that there is no corresponding change to a generated qemu command line (as the automatic dropping is handled solely by libvirt reworking the XML). I can probably do that on your behalf.
6 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f325c3c..1e1bf27 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4044,13 +4044,20 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. + It is possible to define policy whether guestOS boots up
s/guestOS/the guest/
+ if the file is not accessible. This is done by a startupPolicy + attribute: + <ul>
Fails 'make check' with: Validating formatdomain.html formatdomain.html.tmp:4516: element p: validity error : Element ul is not declared in p list of possible children </p> ^ Solved by closing the <p> before starting the <ul>, or ditching <ul> altogether.
+ <li>If the vaule is "optional", guestOS boots up by dropping the file.</li>
s/vaule/value/; s/guestOS/guest/
+ <li>If other values are specified, guestOS fails to boot up. (the default)</li> + </ul>
Needs a "since 1.0.6" notation. Rather than leaving it as 'optional' and a nebulous 'anything else', I'd rather we match existing practice, and specifically call out the same states as elsewhere ('optional'|'requisite'|'mandatory'); especially since your RNG change reused that same enum.
+++ b/src/qemu/qemu_process.c @@ -2442,7 +2442,30 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virReportSystemError(errno, _("Unable to pre-create chardev file '%s'"), dev->source.data.file.path); - return -1; + if (dev->source.data.file.startupPolicy != + VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + return -1; + }
This issues an error even when policy said we want a replacement and thus succeed; we only want to issue an error when we can't succeed, or else clear the error when we try the fallback.
+ VIR_FREE(dev->source.data.file.path); + /* + * Change a destination to /dev/null to boot guest OS up + * even if a log disk is broken.
Not the typical comment style, and should come before we release the old name.
+ */ + VIR_WARN("Switch the destination to /dev/null"); + dev->source.data.file.path = strdup("/dev/null"); + + if (!(dev->source.data.file.path)) { + virReportOOMError(); + return -1; + }
VIR_STRDUP has gone in since you posted, simplifying this.
+ + if ((fd = open(dev->source.data.file.path, + O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) {
O_CREAT and O_APPEND are pointless now that we know we are opening /dev/null. POSIX requires that you use at least one of O_RDONLY, O_WRONLY, or O_RDWR (yes, I know you were copying existing bad code, but no need to repeat that mistake). Just because O_RDONLY is 0 on Linux doesn't make it correct to omit an access mode in your code. For that matter, we KNOW that /dev/null already exists (POSIX says it does, and any system where it doesn't has lots more problems on its hand), so we can exit early as soon as we've malloc'd the replacement file name.
+ virReportSystemError(errno, + _("Unable to pre-create chardev file '%s'"),
Since you've already changed the file name, this error message is not as useful as if it were the user's original file name; but changing that is a bit harder. Here's what I've changed so far; I won't push until I've also added a testcase that proves xml2xml round-trip works on the new attribute (but it's late for me on a holiday weekend, so that might not be until Tuesday). Since I've reviewed the patch prior to code freeze for 1.0.6, I think it will be okay pushing next week even if I don't finish the testsuite addition before DV does the freeze. Thanks again for a first time contribution, and apologies for the delays in getting it reviewed (I think others will agree with me that we've had a LOT of list traffic lately). Interdiff to your original mail (I'll send the complete patch, as-is, as a reply). diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 92fc496..299fa49 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -4212,14 +4212,20 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. - It is possible to define policy whether guestOS boots up - if the file is not accessible. This is done by a startupPolicy - attribute: - <ul> - <li>If the vaule is "optional", guestOS boots up by dropping the file.</li> - <li>If other values are specified, guestOS fails to boot up. (the default)</li> - </ul> + <span class="since">Since 1.0.6</span>, it is possible to define + policy on what happens if the file is not accessible when + booting or migrating. This is done by + a <code>startupPolicy</code> attribute: </p> + <ul> + <li>If the value is "mandatory" (the default), the guest fails + to boot or migrate if the file is not found.</li> + <li>If the value is "optional", a missing file is at boot or + migration is substituted with /dev/null, so the guest still sees + the device but the host no longer tracks guest data on the device.</li> + <li>If the value is "requisite", the file is required for + booting, but optional on migration.</li> + </ul> <pre> ... diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c index 1bc3305..154445d 100644 --- i/src/qemu/qemu_process.c +++ w/src/qemu/qemu_process.c @@ -2439,38 +2439,24 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; if ((fd = open(dev->source.data.file.path, - O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { - virReportSystemError(errno, - _("Unable to pre-create chardev file '%s'"), - dev->source.data.file.path); + O_CREAT | O_APPEND | O_RDONLY, S_IRUSR|S_IWUSR)) < 0) { if (dev->source.data.file.startupPolicy != VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { - return -1; - } - VIR_FREE(dev->source.data.file.path); - /* - * Change a destination to /dev/null to boot guest OS up - * even if a log disk is broken. - */ - VIR_WARN("Switch the destination to /dev/null"); - dev->source.data.file.path = strdup("/dev/null"); - - if (!(dev->source.data.file.path)) { - virReportOOMError(); - return -1; - } - - if ((fd = open(dev->source.data.file.path, - O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { virReportSystemError(errno, _("Unable to pre-create chardev file '%s'"), dev->source.data.file.path); return -1; } + /* Change destination to /dev/null to work around missing file. */ + VIR_WARN("chardev file '%s' not found, switching to /dev/null", + dev->source.data.file.path); + VIR_FREE(dev->source.data.file.path); + if (VIR_STRDUP(dev->source.data.file.path, "/dev/null") < 0) + return -1; + } else { + VIR_FORCE_CLOSE(fd); } - VIR_FORCE_CLOSE(fd); - return 0; } -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Seiji Aguchi <seiji.aguchi@hds.com> [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=<log file's path> -device isa-serial,chardev=chardevserial0,id=serial0 When a hardware failure happens in the disk, qemu-kvm can't create the chardevs. In this case, guest OS doesn't boot up. Actually, there are users who don't desire that guest OS goes down due to a hardware failure of a log disk only. Therefore, qemu should offer some way to boot guest OS up even if the log disk is broken. [Solution] This patch supports startupPolicy for chardev. The starupPolicy is introduced just in cases where chardev is "file" because this patch aims for making guest OS boot up when a hardware failure happens. In other cases (pty, dev, pipe and unix) it is not introduced because they don't access to hardware. The policy works as follows. - If the value is "optional", guestOS boots up by dropping the chardev. - If other values are specified, guestOS fails to boot up. (the default) Description about original startupPolicy attribute: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=e5a84d74a278 Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com> Signed-off-by: Eric Blake <eblake@redhat.com> --- 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). docs/formatdomain.html.in | 15 ++++++++++++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 23 ++++++++++++++++------- tests/virt-aa-helper-test | 3 +++ 6 files changed, 45 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 755d084..299fa49 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4212,13 +4212,26 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. + <span class="since">Since 1.0.6</span>, it is possible to define + policy on what happens if the file is not accessible when + booting or migrating. This is done by + a <code>startupPolicy</code> attribute: </p> + <ul> + <li>If the value is "mandatory" (the default), the guest fails + to boot or migrate if the file is not found.</li> + <li>If the value is "optional", a missing file is at boot or + migration is substituted with /dev/null, so the guest still sees + the device but the host no longer tracks guest data on the device.</li> + <li>If the value is "requisite", the file is required for + booting, but optional on migration.</li> + </ul> <pre> ... <devices> <serial type="file"> - <source path="/var/log/vm/vm-serial.log"/> + <source path="/var/log/vm/vm-serial.log" startupPolicy="optional"/> <target port="1"/> </serial> </devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3cace35..e8eec6c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2780,6 +2780,9 @@ </optional> <optional> <attribute name="path"/> + <optional> + <ref name='startupPolicy'/> + </optional> </optional> <optional> <attribute name="host"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..c24a9f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6626,6 +6626,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *path = NULL; char *mode = NULL; char *protocol = NULL; + char *startupPolicy = NULL; int remaining = 0; while (cur != NULL) { @@ -6646,6 +6647,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, !(flags & VIR_DOMAIN_XML_INACTIVE))) path = virXMLPropString(cur, "path"); + if (startupPolicy == NULL && + def->type == VIR_DOMAIN_CHR_TYPE_FILE) + startupPolicy = virXMLPropString(cur, "startupPolicy"); break; case VIR_DOMAIN_CHR_TYPE_UDP: @@ -6718,6 +6722,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, def->data.file.path = path; path = NULL; + + def->data.file.startupPolicy = + virDomainStartupPolicyTypeFromString(startupPolicy); + startupPolicy = NULL; break; case VIR_DOMAIN_CHR_TYPE_STDIO: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a71d6c..4d49dd5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1074,6 +1074,7 @@ struct _virDomainChrSourceDef { /* no <source> for null, vc, stdio */ struct { char *path; + int startupPolicy; /* enum virDomainStartupPolicy */ } file; /* pty, file, pipe, or device */ struct { char *host; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4fd4fb..154445d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2439,15 +2439,24 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; if ((fd = open(dev->source.data.file.path, - O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { - virReportSystemError(errno, - _("Unable to pre-create chardev file '%s'"), - dev->source.data.file.path); - return -1; + O_CREAT | O_APPEND | O_RDONLY, S_IRUSR|S_IWUSR)) < 0) { + if (dev->source.data.file.startupPolicy != + VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + virReportSystemError(errno, + _("Unable to pre-create chardev file '%s'"), + dev->source.data.file.path); + return -1; + } + /* Change destination to /dev/null to work around missing file. */ + VIR_WARN("chardev file '%s' not found, switching to /dev/null", + dev->source.data.file.path); + VIR_FREE(dev->source.data.file.path); + if (VIR_STRDUP(dev->source.data.file.path, "/dev/null") < 0) + return -1; + } else { + VIR_FORCE_CLOSE(fd); } - VIR_FORCE_CLOSE(fd); - return 0; } diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index af91c61..7172fd6 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -255,6 +255,9 @@ testme "0" "disk (empty cdrom)" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log' startupPolicy='optional'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" +testme "0" "serial" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='pty'><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial (pty)" "-r -u $valid_uuid" "$test_xml" -- 1.8.1.4
On 05/24/2013 04:46 PM, Eric Blake wrote:
From: Seiji Aguchi <seiji.aguchi@hds.com>
[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=<log file's 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 [/> <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
Eric,
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Eric Blake Sent: Tuesday, May 28, 2013 7:08 PM Cc: libvir-list@redhat.com; dle-develop@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 <seiji.aguchi@hds.com>
[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=<log file's 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 [/> <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?
I need to think carefully about it before deciding it in a hurry.
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.
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, " <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
participants (2)
-
Eric Blake -
Seiji Aguchi