[libvirt] [PATCH v3] xml: introduce startupPolicy for chardev device
Seiji Aguchi
seiji.aguchi at hds.com
Fri Aug 2 14:16:48 UTC 2013
Any comment?
> -----Original Message-----
> From: libvir-list-bounces at redhat.com [mailto:libvir-list-bounces at redhat.com] On Behalf Of Seiji Aguchi
> Sent: Wednesday, July 24, 2013 4:06 PM
> To: libvir-list at redhat.com
> Subject: [libvirt] [PATCH v3] 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
>
> 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 at hds.com>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> Change from v2
> - To pass "make check", add followings.
> - Add serial source optional testing.
> - check if startupPolicy is NULL in virDomainChrSourceDefParseXML().
> - Add xml format of startupPolicy in virDomainChrSourceDefFormat().
>
> Patch v2 and comment from Eric Blake
> - https://www.redhat.com/archives/libvir-list/2013-May/msg01814.html
> - https://www.redhat.com/archives/libvir-list/2013-May/msg01943.html
> ---
> docs/formatdomain.html.in | 16 ++++++++-
> docs/schemas/domaincommon.rng | 3 ++
> src/conf/domain_conf.c | 22 +++++++++++-
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_process.c | 25 +++++++++++++-
> .../qemuxml2argv-serial-source-optional.args | 9 +++++
> .../qemuxml2argv-serial-source-optional.xml | 35 ++++++++++++++++++++
> tests/qemuxml2argvtest.c | 2 +
> tests/qemuxml2xmltest.c | 1 +
> tests/virt-aa-helper-test | 3 ++
> 10 files changed, 113 insertions(+), 4 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 7601aaa..5c9d4fb 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4281,13 +4281,27 @@ 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 745b959..10b3365 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2817,6 +2817,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 10cb7f6..279ff9e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6819,6 +6819,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> char *path = NULL;
> char *mode = NULL;
> char *protocol = NULL;
> + char *startupPolicy = NULL;
> int remaining = 0;
>
> while (cur != NULL) {
> @@ -6839,6 +6840,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:
> @@ -6911,6 +6915,13 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>
> def->data.file.path = path;
> path = NULL;
> +
> + if (startupPolicy) {
> + def->data.file.startupPolicy =
> + virDomainStartupPolicyTypeFromString(startupPolicy);
> + startupPolicy = NULL;
> + }
> +
> break;
>
> case VIR_DOMAIN_CHR_TYPE_STDIO:
> @@ -15014,8 +15025,15 @@ 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",
> - def->data.file.path);
> + 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 a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index f265966..0899556 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1102,6 +1102,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 a46d944..35d63d5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2511,7 +2511,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/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-
> source-optional.args
> new file mode 100644
> index 0000000..9ffe8de
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args
> @@ -0,0 +1,9 @@
> +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 a/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-
> source-optional.xml
> new file mode 100644
> index 0000000..1aeb82a
> --- /dev/null
> +++ b/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' startupPolicy='optional'/>
> + <target type='serial' port='0'/>
> + </console>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 0f96eef..588c922 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -728,6 +728,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 a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 77cac3f..9faca1f 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -234,6 +234,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");
> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list