[libvirt] [PATCH v3 0/4] add support for panic device
panic device is a device that enables libvirt to receive notification of guest panic event. qemu implements it by pvpanic. This series adds support for panic device. It is implemented in qemu driver only. changes in v3: - introduce generic ISA address - rename pvpanic to panic. - add RNG schemas for new elements - add tests for panic device - error out if panic device is requested but qemu is too old Hu Tao (4): conf: introduce generic ISA address conf: add support for panic device qemu: add support for -device pvpanic test: add test for panic device docs/formatdomain.html.in | 33 ++++++ docs/schemas/basictypes.rng | 17 ++++ docs/schemas/domaincommon.rng | 16 +++ src/conf/domain_conf.c | 135 ++++++++++++++++++++++++- src/conf/domain_conf.h | 18 ++++ src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 16 +++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemuxml2argvdata/qemuxml2argv-panic.args | 6 ++ tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 31 ++++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 2 + 15 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.xml -- 1.7.11.7
--- docs/formatdomain.html.in | 5 ++++ docs/schemas/basictypes.rng | 17 ++++++++++++ docs/schemas/domaincommon.rng | 6 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 +++++++ 5 files changed, 99 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1850a2b..054ebc6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2425,6 +2425,11 @@ operating system. <span class="since">Since 1.0.4</span> </dd> + <dt><code>type='isa'</code></dt> + <dd>ISA addresses have the following additional + attributes: <code>iobase</code> and <code>irq</code>. + <span class="since">Since 1.2.1</span> + </dd> </dl> <h4><a name="elementsControllers">Controllers</a></h4> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 268bc5a..34ef613 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -380,4 +380,21 @@ </element> </define> + <define name="isaaddress"> + <optional> + <attribute name="iobase"> + <data type="string"> + <param name="pattern">0x[a-fA-F0-9]{1,4}</param> + </data> + </attribute> + </optional> + <optional> + <attribute name="irq"> + <data type="string"> + <param name="pattern">0x[a-fA-F0-9]</param> + </data> + </attribute> + </optional> + </define> + </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 80848d2..3e98af9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3921,6 +3921,12 @@ </attribute> <ref name="ccwaddress"/> </group> + <group> + <attribute name="type"> + <value>isa</value> + </attribute> + <ref name="isaaddress"/> + </group> </choice> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 140eb80..7d5617e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -212,7 +212,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "spapr-vio", "virtio-s390", "ccw", - "virtio-mmio") + "virtio-mmio", + "isa") VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", @@ -3053,6 +3054,13 @@ virDomainDeviceInfoFormat(virBufferPtr buf, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: + if (info->addr.isa.iobase > 0) + virBufferAsprintf(buf, " iobase='0x%x'", info->addr.isa.iobase); + if (info->addr.isa.irq> 0) + virBufferAsprintf(buf, " irq='0x%x'", info->addr.isa.irq); + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown address type '%d'"), info->type); @@ -3389,6 +3397,40 @@ cleanup: return ret; } +static int +virDomainDeviceISAAddressParseXML(xmlNodePtr node, + virDomainDeviceISAAddressPtr addr) +{ + int ret = -1; + char *iobase; + char *irq; + + memset(addr, 0, sizeof(*addr)); + + iobase = virXMLPropString(node, "iobase"); + irq = virXMLPropString(node, "irq"); + + if (iobase && + virStrToLong_ui(iobase, NULL, 16, &addr->iobase) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Cannot parse <address> 'iobase' attribute")); + goto cleanup; + } + + if (irq && + virStrToLong_ui(irq, NULL, 16, &addr->irq) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Cannot parse <address> 'irq' attribute")); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(iobase); + VIR_FREE(irq); + return ret; +} + /* Parse the XML definition for a device address * @param node XML nodeset to parse for device address definition */ @@ -3520,6 +3562,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: + if (virDomainDeviceISAAddressParseXML(address, &info->addr.isa) < 0) + goto cleanup; + break; + default: /* Should not happen */ virReportError(VIR_ERR_INTERNAL_ERROR, @@ -12916,6 +12963,20 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, return false; } break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: + if (src->addr.isa.iobase != dst->addr.isa.iobase || + src->addr.isa.irq != dst->addr.isa.irq) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device isa address %d:%d " + "does not match source %d:%d"), + dst->addr.isa.iobase, + dst->addr.isa.irq, + src->addr.isa.iobase, + src->addr.isa.irq); + return false; + } + break; } return true; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4561ccc..25bd362 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -208,6 +208,7 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; @@ -284,6 +285,13 @@ struct _virDomainDeviceUSBMaster { unsigned int startport; }; +typedef struct _virDomainDeviceISAAddress virDomainDeviceISAAddress; +typedef virDomainDeviceISAAddress *virDomainDeviceISAAddressPtr; +struct _virDomainDeviceISAAddress { + unsigned int iobase; + unsigned int irq; +}; + typedef struct _virDomainDeviceInfo virDomainDeviceInfo; typedef virDomainDeviceInfo *virDomainDeviceInfoPtr; struct _virDomainDeviceInfo { @@ -301,6 +309,7 @@ struct _virDomainDeviceInfo { virDomainDeviceUSBAddress usb; virDomainDeviceSpaprVioAddress spaprvio; virDomainDeviceCCWAddress ccw; + virDomainDeviceISAAddress isa; } addr; int mastertype; union { -- 1.7.11.7
On Mon, Dec 09, 2013 at 05:11:13PM +0800, Hu Tao wrote:
--- docs/formatdomain.html.in | 5 ++++ docs/schemas/basictypes.rng | 17 ++++++++++++ docs/schemas/domaincommon.rng | 6 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 +++++++ 5 files changed, 99 insertions(+), 1 deletion(-)
ACK
@@ -3053,6 +3054,13 @@ virDomainDeviceInfoFormat(virBufferPtr buf, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: + if (info->addr.isa.iobase > 0) + virBufferAsprintf(buf, " iobase='0x%x'", info->addr.isa.iobase); + if (info->addr.isa.irq> 0)
Minor whitespace bug.
+ virBufferAsprintf(buf, " irq='0x%x'", info->addr.isa.irq); + break; +
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/12/2013 05:07 AM, Daniel P. Berrange wrote:
On Mon, Dec 09, 2013 at 05:11:13PM +0800, Hu Tao wrote:
Commit message should mention the new XML.
--- docs/formatdomain.html.in | 5 ++++ docs/schemas/basictypes.rng | 17 ++++++++++++ docs/schemas/domaincommon.rng | 6 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 +++++++ 5 files changed, 99 insertions(+), 1 deletion(-)
ACK
@@ -3053,6 +3054,13 @@ virDomainDeviceInfoFormat(virBufferPtr buf, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: + if (info->addr.isa.iobase > 0) + virBufferAsprintf(buf, " iobase='0x%x'", info->addr.isa.iobase); + if (info->addr.isa.irq> 0)
Minor whitespace bug.
Fixed and will push when I get through the rest of the series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
panic device is a device that enables libvirt to receive notification of guest panic event. --- docs/formatdomain.html.in | 28 +++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 72 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 9 ++++++ 4 files changed, 119 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 054ebc6..c8f213e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5085,6 +5085,34 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsPanic">panic device</a></h4> + <p> + panic device enables libvirt to receive panic notification from a QEMU + guest. + <span class="since">Since 1.2.1, QEMU and KVM only</span> + </p> + <p> + Example: usage of panic configuration + </p> +<pre> + ... + <devices> + <panic> + <address type='isa' iobase='0x505'/> + </panic> + </devices> + ... +</pre> + <dl> + <dt><code>address</code></dt> + <dd> + <p> + address of panic. The default ioport is 0x505. Most users + don't need to specify an address. + </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3e98af9..8ae96f2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3556,6 +3556,9 @@ <optional> <ref name="nvram"/> </optional> + <optional> + <ref name="panic"/> + </optional> </interleave> </element> </define> @@ -4409,4 +4412,11 @@ </data> </choice> </define> + <define name="panic"> + <element name="panic"> + <optional> + <ref name="address"/> + </optional> + </element> + </define> </grammar> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7d5617e..233b848 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1877,6 +1877,15 @@ virDomainResourceDefFree(virDomainResourceDefPtr resource) VIR_FREE(resource); } +void +virDomainPanicDefFree(virDomainPanicDefPtr panic) +{ + if (!panic) + return; + + virDomainDeviceInfoClear(&panic->info); + VIR_FREE(panic); +} void virDomainDefFree(virDomainDefPtr def) { @@ -1965,6 +1974,8 @@ void virDomainDefFree(virDomainDefPtr def) virDomainTPMDefFree(def->tpm); + virDomainPanicDefFree(def->panic); + VIR_FREE(def->idmap.uidmap); VIR_FREE(def->idmap.gidmap); @@ -10673,6 +10684,22 @@ cleanup: return idmap; } +static virDomainPanicDefPtr +virDomainPanicDefParseXML(xmlNodePtr node) +{ + virDomainPanicDefPtr panic; + + if (VIR_ALLOC(panic) < 0) + return NULL; + + if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, 0) < 0) + goto error; + + return panic; +error: + virDomainPanicDefFree(panic); + return NULL; +} /* Parse the XML definition for a vcpupin or emulatorpin. * @@ -12500,6 +12527,27 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + /* analysis of the panic devices */ + def->panic = NULL; + if ((n = virXPathNodeSet("./devices/panic", ctxt, &nodes)) < 0) { + goto error; + } + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single panic device is supported")); + goto error; + } + if (n > 0) { + virDomainPanicDefPtr panic = + virDomainPanicDefParseXML(nodes[0]); + if (!panic) + goto error; + + def->panic = panic; + VIR_FREE(nodes); + } + + /* analysis of the user namespace mapping */ if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) goto error; @@ -13589,6 +13637,13 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, return true; } +static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, + virDomainPanicDefPtr dst) +{ + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); +} + /* This compares two configurations and looks for any differences * which will affect the guest ABI. This is primarily to allow @@ -13930,6 +13985,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainRNGDefCheckABIStability(src->rng, dst->rng)) return false; + if (!virDomainPanicCheckABIStability(src->panic, dst->panic)) + return false; + return true; } @@ -15776,6 +15834,16 @@ virDomainWatchdogDefFormat(virBufferPtr buf, return 0; } +static int virDomainPanicDefFormat(virBufferPtr buf, + virDomainPanicDefPtr def) +{ + virBufferAddLit(buf, " <panic>\n"); + if (virDomainDeviceInfoFormat(buf, &def->info, 0) < 0) + return -1; + virBufferAddLit(buf, " </panic>\n"); + + return 0; +} static int virDomainRNGDefFormat(virBufferPtr buf, @@ -17199,6 +17267,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->nvram) virDomainNVRAMDefFormat(buf, def->nvram, flags); + if (def->panic && + virDomainPanicDefFormat(buf, def->panic) < 0) + goto error; + virBufferAddLit(buf, " </devices>\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 25bd362..7e68aa2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -126,6 +126,9 @@ typedef virDomainIdMapEntry *virDomainIdMapEntryPtr; typedef struct _virDomainIdMapDef virDomainIdMapDef; typedef virDomainIdMapDef *virDomainIdMapDefPtr; +typedef struct _virDomainPanicDef virDomainPanicDef; +typedef virDomainPanicDef *virDomainPanicDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1919,6 +1922,10 @@ struct _virDomainIdMapDef { }; +struct _virDomainPanicDef { + virDomainDeviceInfo info; +}; + void virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, int ndevices); @@ -2070,6 +2077,7 @@ struct _virDomainDef { virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; virDomainRNGDefPtr rng; + virDomainPanicDefPtr panic; void *namespaceData; virDomainXMLNamespace ns; @@ -2213,6 +2221,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, bool virDomainObjTaint(virDomainObjPtr obj, enum virDomainTaintFlags taint); +void virDomainPanicDefFree(virDomainPanicDefPtr panic); void virDomainResourceDefFree(virDomainResourceDefPtr resource); void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); -- 1.7.11.7
On Mon, Dec 09, 2013 at 05:11:14PM +0800, Hu Tao wrote:
panic device is a device that enables libvirt to receive notification of guest panic event. --- docs/formatdomain.html.in | 28 +++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 72 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 9 ++++++ 4 files changed, 119 insertions(+)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/12/2013 05:08 AM, Daniel P. Berrange wrote:
On Mon, Dec 09, 2013 at 05:11:14PM +0800, Hu Tao wrote:
panic device is a device that enables libvirt to receive notification of guest panic event. --- docs/formatdomain.html.in | 28 +++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 72 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 9 ++++++
domain_conf.h gave me a conflict to based on tree activity in the meantime, but I got that fixed.
4 files changed, 119 insertions(+)
ACK
Will push with the rest of the series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/09/2013 04:11 AM, Hu Tao wrote: <...snip...>
+static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, + virDomainPanicDefPtr dst) +{ + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); +} +
These changes have resulted in a libvirtd crash during the virt-test 'snapshot_edit' checks: Thread 1 (Thread 0x7f5332837700 (LWP 10964)): #0 virDomainDeviceInfoCheckABIStability (src=<optimized out>, dst=<optimized out>) at conf/domain_conf.c:13007 #1 0x00007f534211bbc7 in virDomainPanicCheckABIStability (dst=<optimized out>, src=<optimized out>) at conf/domain_conf.c:13712 #2 virDomainDefCheckABIStability (src=<optimized out>, dst=<optimized out>) at conf/domain_conf.c:14056 #3 0x00007f53421340cb in virDomainSnapshotRedefinePrep (domain=domain@entry=0x7f53000057c0, vm=vm@entry=0x7f53000036d0, defptr=defptr@entry=0x7f5332836978, snap=snap@entry=0x7f5332836970, update_current=update_current@entry=0x7f5332836962, flags=flags@entry=1) at conf/snapshot_conf.c:1230 #4 0x00007f5329e4356c in qemuDomainSnapshotCreateXML (domain=0x7f53000057c0, xmlDesc=<optimized out>, flags=1) at qemu/qemu_driver.c:12719 #5 0x00007f53421a0738 in virDomainSnapshotCreateXML (domain=domain@entry=0x7f53000057c0, xmlDesc=0x7f53000081d0 "<domainsnapshot>\n <name>snap2</name>\n <description>new-desc</description>\n <state>running</state>\n <parent>\n <name>snap1</name>\n </parent>\n <creationTime>1387487268</creationTime>\n <memory s"..., flags=1) at libvirt.c:19695 #6 0x00007f5342bc85b7 in remoteDispatchDomainSnapshotCreateXML (server=<optimized out>, msg=<optimized out>, ret=0x7f5300003c80, args=0x7f5300003c40, rerr=0x7f5332836c80, client=<optimized out>) at remote_dispatch.h:8223 #7 remoteDispatchDomainSnapshotCreateXMLHelper (server=<optimized out>, client=<optimized out>, msg=<optimized out>, rerr=0x7f5332836c80, args=0x7f5300003c40, ret=0x7f5300003c80) at remote_dispatch.h:8199 #8 0x00007f53421ede3a in virNetServerProgramDispatchCall (msg=0x7f5344c1acc0, client=0x7f5344c1aa80, server=0x7f5344c01570, prog=0x7f5344c15940) at rpc/virnetserverprogram.c:435 #9 virNetServerProgramDispatch (prog=0x7f5344c15940, server=server@entry=0x7f5344c01570, client=0x7f5344c1aa80, msg=0x7f5344c1acc0) at rpc/virnetserverprogram.c:305 #10 0x00007f53421e7c98 in virNetServerProcessMsg (msg=<optimized out>, prog=<optimized out>, client=<optimized out>, srv=0x7f5344c01570) at rpc/virnetserver.c:165 ... (gdb) up 3 (gdb) print *other->def->dom $2 = {virtType = 2, id = -1, uuid = "\210Γ;\254e@\212\245\034\032h$\230\030\264", name = 0x7f53201bdb60 "virt-tests-vm1", title = 0x0, ... seclabels = 0x7f53201fe650, watchdog = 0x0, memballoon = 0x7f5320201d10, nvram = 0x0, tpm = 0x0, cpu = 0x0, sysinfo = 0x0, redirfilter = 0x0, rng = 0x0, panic = 0x0, namespaceData = 0x0, ns = {parse = 0x7f5329dd2770 <qemuDomainDefNamespaceParse>, ... (gdb) print *def->dom $3 = {virtType = 2, id = -1, uuid = "\210Γ;\254e@\212\245\034\032h$\230\030\264", name = 0x7f530000fdf0 "virt-tests-vm1", title = 0x0, ... rng = 0x0, panic = 0x0, namespaceData = 0x0, ns = {parse = 0x7f5329dd2770 <qemuDomainDefNamespaceParse>, Unlike the virDomainRNGDefCheckABIStability() API, the new virDomainPanicCheckABIStability() has no checks for !src && !dst I'll put together a patch for this, but figured I'd ask now if there were checks that should also be made in the PanicCheck API... John
/* This compares two configurations and looks for any differences * which will affect the guest ABI. This is primarily to allow @@ -13930,6 +13985,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainRNGDefCheckABIStability(src->rng, dst->rng)) return false;
+ if (!virDomainPanicCheckABIStability(src->panic, dst->panic)) + return false; + return true; }
@@ -15776,6 +15834,16 @@ virDomainWatchdogDefFormat(virBufferPtr buf, return 0; }
On 12/20/2013 06:59 AM, John Ferlan wrote:
On 12/09/2013 04:11 AM, Hu Tao wrote:
<...snip...>
+static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, + virDomainPanicDefPtr dst) +{ + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
Unlike the virDomainRNGDefCheckABIStability() API, the new virDomainPanicCheckABIStability() has no checks for !src && !dst
Yay for automated regression testing catching something! And sorry that I missed this in my initial review. Yes, adding or removing the panic device is an ABI incompatibility.
I'll put together a patch for this, but figured I'd ask now if there were checks that should also be made in the PanicCheck API...
Not really - a panic device has only two properties: existence, and address (the existence check is missing, which caused the core; but the address check is already there). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Dec 20, 2013 at 07:11:00AM -0700, Eric Blake wrote:
On 12/20/2013 06:59 AM, John Ferlan wrote:
On 12/09/2013 04:11 AM, Hu Tao wrote:
<...snip...>
+static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, + virDomainPanicDefPtr dst) +{ + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
Unlike the virDomainRNGDefCheckABIStability() API, the new virDomainPanicCheckABIStability() has no checks for !src && !dst
Yay for automated regression testing catching something! And sorry that I missed this in my initial review. Yes, adding or removing the panic device is an ABI incompatibility.
Indeed. The ABI stability checks are something we really ought to be able to have our unit tests do in fact. At the very least we could feed in every single XML file we have in the test suite as both the src & dst params, to serve as an "identity" test. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 16 ++++++++++++++++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + 6 files changed, 24 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 548b988..b532dbb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -243,6 +243,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio-mmio", "ich9-intel-hda", "kvm-pit-lost-tick-policy", + + "pvpanic", /* 160 */ ); struct _virQEMUCaps { @@ -1394,6 +1396,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-storage", QEMU_CAPS_DEVICE_USB_STORAGE }, { "virtio-mmio", QEMU_CAPS_DEVICE_VIRTIO_MMIO }, { "ich9-intel-hda", QEMU_CAPS_DEVICE_ICH9_INTEL_HDA }, + { "pvpanic", QEMU_CAPS_DEVICE_PANIC }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 02d47c6..1aedbf9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -199,6 +199,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */ + QEMU_CAPS_DEVICE_PANIC = 160, /* -device pvpanic */ + QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..8487356 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9588,6 +9588,22 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (def->panic) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { + if (def->panic->info.addr.isa.iobase > 0) { + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", + def->panic->info.addr.isa.iobase); + } else { + virCommandAddArgList(cmd, "-device", "pvpanic", NULL); + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("your QEMU is too old to support pvpanic")); + goto error; + } + } + if (mlock) { unsigned long long memKB; diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 09cf657..1c84ce2 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -131,4 +131,5 @@ <flag name='usb-storage.removable'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='pvpanic'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 33ee73b..3b1b456 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -135,4 +135,5 @@ <flag name='virtio-mmio'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='pvpanic'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index a66034a..fc9e034 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -134,4 +134,5 @@ <flag name='virtio-mmio'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='pvpanic'/> </qemuCaps> -- 1.7.11.7
On Mon, Dec 09, 2013 at 05:11:15PM +0800, Hu Tao wrote:
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 16 ++++++++++++++++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + 6 files changed, 24 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 548b988..b532dbb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -243,6 +243,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio-mmio", "ich9-intel-hda", "kvm-pit-lost-tick-policy", + + "pvpanic", /* 160 */ );
struct _virQEMUCaps { @@ -1394,6 +1396,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-storage", QEMU_CAPS_DEVICE_USB_STORAGE }, { "virtio-mmio", QEMU_CAPS_DEVICE_VIRTIO_MMIO }, { "ich9-intel-hda", QEMU_CAPS_DEVICE_ICH9_INTEL_HDA }, + { "pvpanic", QEMU_CAPS_DEVICE_PANIC }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 02d47c6..1aedbf9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -199,6 +199,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */
+ QEMU_CAPS_DEVICE_PANIC = 160, /* -device pvpanic */ + QEMU_CAPS_LAST, /* this must always be the last item */ };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..8487356 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9588,6 +9588,22 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
+ if (def->panic) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { + if (def->panic->info.addr.isa.iobase > 0) { + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", + def->panic->info.addr.isa.iobase); + } else { + virCommandAddArgList(cmd, "-device", "pvpanic", NULL); + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("your QEMU is too old to support pvpanic")); + goto error; + } + } + if (mlock) { unsigned long long memKB;
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 09cf657..1c84ce2 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -131,4 +131,5 @@ <flag name='usb-storage.removable'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='pvpanic'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 33ee73b..3b1b456 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -135,4 +135,5 @@ <flag name='virtio-mmio'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='pvpanic'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index a66034a..fc9e034 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -134,4 +134,5 @@ <flag name='virtio-mmio'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='pvpanic'/> </qemuCaps>
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/09/2013 02:11 AM, Hu Tao wrote:
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 16 ++++++++++++++++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + 6 files changed, 24 insertions(+)
More merge conflicts, and it's my bedtime; I'll resolve and push in the morning, as well as squash 3 and 4 into one patch. Thanks again for doing this work. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Dec 12, 2013 at 09:20:43PM -0700, Eric Blake wrote:
On 12/09/2013 02:11 AM, Hu Tao wrote:
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 16 ++++++++++++++++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + 6 files changed, 24 insertions(+)
More merge conflicts, and it's my bedtime; I'll resolve and push in the morning, as well as squash 3 and 4 into one patch.
Thanks!
--- tests/qemuxml2argvdata/qemuxml2argv-panic.args | 6 +++++ tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 31 ++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c | 2 ++ 4 files changed, 42 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic.args b/tests/qemuxml2argvdata/qemuxml2argv-panic.args new file mode 100644 index 0000000..8e07cba --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-hda /dev/HostVG/QEMUGuest1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-device pvpanic,ioport=1285 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic.xml new file mode 100644 index 0000000..e354511 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic.xml @@ -0,0 +1,31 @@ +<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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + <panic> + <address type='isa' iobase='0x505'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index dad5973..537b3ce 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1115,6 +1115,9 @@ mymain(void) DO_TEST("kvm-pit-device", QEMU_CAPS_NO_KVM_PIT, QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ceaaf6a..0253f4e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -322,6 +322,8 @@ mymain(void) DO_TEST("pcihole64-none"); DO_TEST("pcihole64-q35"); + DO_TEST("panic"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.7.11.7
On Mon, Dec 09, 2013 at 05:11:16PM +0800, Hu Tao wrote:
--- tests/qemuxml2argvdata/qemuxml2argv-panic.args | 6 +++++ tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 31 ++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c | 2 ++ 4 files changed, 42 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.xml
ACK, but these should be part of the previous patch Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/12/2013 05:10 AM, Daniel P. Berrange wrote:
On Mon, Dec 09, 2013 at 05:11:16PM +0800, Hu Tao wrote:
--- tests/qemuxml2argvdata/qemuxml2argv-panic.args | 6 +++++ tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 31 ++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c | 2 ++ 4 files changed, 42 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic.xml
ACK, but these should be part of the previous patch
Series now pushed as 3 patches. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Hu Tao -
John Ferlan