[libvirt] [PATCHv2 0/3] Hyper-v crash feature support

A new Hyper-V cpu feature 'hv_crash' was added to QEMU. The feature will become available in v2.5.0. This patch adds support for this feature. Dmitry Andreev (3): conf: add 'model' attribute for panic device with values isa, pseries, hyperv qemu: add support for hv_crash feature as a panic device tests: add tests for panic device with model 'hyperv' docs/formatdomain.html.in | 29 ++++++++++++- docs/schemas/domaincommon.rng | 9 ++++ src/conf/domain_conf.c | 33 ++++++++++++--- src/conf/domain_conf.h | 10 +++++ src/qemu/qemu_command.c | 48 +++++++++++++++++++--- src/qemu/qemu_domain.c | 4 ++ tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-hyperv-panic.args | 21 ++++++++++ .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 +++++++++++ .../qemuxml2argv-panic-no-address.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- .../qemuxml2argv-pseries-panic-address.xml | 2 +- .../qemuxml2argv-pseries-panic-no-address.xml | 2 +- tests/qemuxml2argvtest.c | 1 + .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- tests/qemuxml2xmltest.c | 1 + 18 files changed, 176 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml -- 1.8.3.1

Libvirt already has two types of panic devices - pvpanic and pSeries firmware. This patch introduces the 'model' attribute and a new type of panic device. 'isa' model is for ISA pvpanic device. 'pseries' model is a default value for pSeries guests. 'hyperv' model is the new type. It's used for Hyper-V crash. Schema, docs and tests are updated for the new attribute. --- docs/formatdomain.html.in | 29 +++++++++++++++++-- docs/schemas/domaincommon.rng | 9 ++++++ src/conf/domain_conf.c | 33 ++++++++++++++++++---- src/conf/domain_conf.h | 10 +++++++ src/qemu/qemu_domain.c | 4 +++ .../qemuxml2argv-panic-no-address.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- .../qemuxml2argv-pseries-panic-address.xml | 2 +- .../qemuxml2argv-pseries-panic-no-address.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- 12 files changed, 85 insertions(+), 14 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c88b032..93b9813 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6152,19 +6152,44 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> ... </pre> + <p> + Example: usage of panic configuration for Hyper-V guests + </p> +<pre> + ... + <devices> + <panic model='hyperv'/> + </devices> + ... +</pre> <dl> + <dt><code>model</code></dt> + <dd> + <p> + The required <code>model</code> attribute specifies what type + of panic device is provided. + </p> + <ul> + <li>'isa' — for ISA pvpanic device</li> + <li>'pseries' — default and valid only for pSeries guests.</li> + <li>'hyperv' — for Hyper-V crash CPU feature. + <span class="since">Since 2.5.0, QEMU and KVM only</span> + </li> + </ul> + </dd> + <dt><code>address</code></dt> <dd> <p> address of panic. The default ioport is 0x505. Most users don't need to specify an address, and doing so is forbidden - altogether for pSeries guests. + altogether for pSeries guests and for Hyper-V crash. </p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..d67b1bf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5359,6 +5359,15 @@ <define name="panic"> <element name="panic"> <optional> + <attribute name="model"> + <choice> + <value>isa</value> + <value>pseries</value> + <value>hyperv</value> + </choice> + </attribute> + </optional> + <optional> <ref name="address"/> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb00444..935d429 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -525,6 +525,11 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, VIR_DOMAIN_WATCHDOG_ACTION_LAST, "none", "inject-nmi") +VIR_ENUM_IMPL(virDomainPanicModel, VIR_DOMAIN_PANIC_MODEL_LAST, + "isa", + "pseries", + "hyperv") + VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vga", "cirrus", @@ -10194,20 +10199,36 @@ virDomainTPMDefParseXML(xmlNodePtr node, } static virDomainPanicDefPtr -virDomainPanicDefParseXML(xmlNodePtr node) +virDomainPanicDefParseXML(const virDomainDef *dom, xmlNodePtr node) { virDomainPanicDefPtr panic; + char *model = NULL; if (VIR_ALLOC(panic) < 0) return NULL; + if (!(model = virXMLPropString(node, "model"))) { + if (ARCH_IS_PPC64(dom->os.arch) && STRPREFIX(dom->os.machine, "pseries")) + panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + else + panic->model = VIR_DOMAIN_PANIC_MODEL_ISA; + } else if ((panic->model = virDomainPanicModelTypeFromString(model)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown panic model '%s'"), model); + goto error; + } + if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, 0) < 0) goto error; + cleanup: + VIR_FREE(model); return panic; + error: virDomainPanicDefFree(panic); - return NULL; + panic = NULL; + goto cleanup; } /* Parse the XML definition for an input device */ @@ -12751,7 +12772,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_PANIC: - if (!(dev->data.panic = virDomainPanicDefParseXML(node))) + if (!(dev->data.panic = virDomainPanicDefParseXML(def, node))) goto error; break; case VIR_DOMAIN_DEVICE_MEMORY: @@ -16398,7 +16419,7 @@ virDomainDefParseXML(xmlDocPtr xml, } if (n > 0) { virDomainPanicDefPtr panic = - virDomainPanicDefParseXML(nodes[0]); + virDomainPanicDefParseXML(def, nodes[0]); if (!panic) goto error; @@ -20627,10 +20648,12 @@ virDomainWatchdogDefFormat(virBufferPtr buf, static int virDomainPanicDefFormat(virBufferPtr buf, virDomainPanicDefPtr def) { + const char *model = virDomainPanicModelTypeToString(def->model); + virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; int indent = virBufferGetIndent(buf, false); - virBufferAddLit(buf, "<panic"); + virBufferAsprintf(buf, "<panic model='%s'", model); virBufferAdjustIndent(&childrenBuf, indent + 2); if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f10b534..00f3679 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2045,7 +2045,16 @@ struct _virDomainIdMapDef { }; +typedef enum { + VIR_DOMAIN_PANIC_MODEL_ISA, + VIR_DOMAIN_PANIC_MODEL_PSERIES, + VIR_DOMAIN_PANIC_MODEL_HYPERV, + + VIR_DOMAIN_PANIC_MODEL_LAST +} virDomainPanicModel; + struct _virDomainPanicDef { + int model; virDomainDeviceInfo info; }; @@ -3060,6 +3069,7 @@ VIR_ENUM_DECL(virDomainMemballoonModel) VIR_ENUM_DECL(virDomainSmbiosMode) VIR_ENUM_DECL(virDomainWatchdogModel) VIR_ENUM_DECL(virDomainWatchdogAction) +VIR_ENUM_DECL(virDomainPanicModel) VIR_ENUM_DECL(virDomainVideo) VIR_ENUM_DECL(virDomainHostdevMode) VIR_ENUM_DECL(virDomainHostdevSubsys) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3d2b7a0..8ec3e8e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1180,6 +1180,10 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (VIR_ALLOC(panic) < 0) goto cleanup; + if (ARCH_IS_PPC64(def->os.arch) && + STRPREFIX(def->os.machine, "pseries")) + panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + def->panic = panic; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml index 79f8a1e..f3f3fbb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml @@ -24,6 +24,6 @@ <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> <memballoon model='virtio'/> - <panic/> + <panic model='isa'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic.xml index e354511..b9595a8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-panic.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic.xml @@ -24,7 +24,7 @@ <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> <memballoon model='virtio'/> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml index 3a96209..39f4a1f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml @@ -37,6 +37,6 @@ <model type='cirrus' vram='16384' heads='1'/> </video> <memballoon model='none'/> - <panic/> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml index 619186a..2da2832 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml @@ -20,6 +20,6 @@ <nvram> <address type='spapr-vio' reg='0x4000'/> </nvram> - <panic/> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml index e62ead8..be5b862 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml @@ -25,7 +25,7 @@ <address type='spapr-vio'/> </console> <memballoon model='none'/> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml index 9312975..8fcd644 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml @@ -25,6 +25,6 @@ <address type='spapr-vio'/> </console> <memballoon model='none'/> - <panic/> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml index 9312975..8fcd644 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -25,6 +25,6 @@ <address type='spapr-vio'/> </console> <memballoon model='none'/> - <panic/> + <panic model='pseries'/> </devices> </domain> -- 1.8.3.1

On Thu, Nov 12, 2015 at 14:07:38 +0300, Dmitry Andreev wrote:
Libvirt already has two types of panic devices - pvpanic and pSeries firmware. This patch introduces the 'model' attribute and a new type of panic device.
'isa' model is for ISA pvpanic device. 'pseries' model is a default value for pSeries guests. 'hyperv' model is the new type. It's used for Hyper-V crash.
Schema, docs and tests are updated for the new attribute. --- docs/formatdomain.html.in | 29 +++++++++++++++++-- docs/schemas/domaincommon.rng | 9 ++++++ src/conf/domain_conf.c | 33 ++++++++++++++++++---- src/conf/domain_conf.h | 10 +++++++ src/qemu/qemu_domain.c | 4 +++ .../qemuxml2argv-panic-no-address.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- .../qemuxml2argv-pseries-panic-address.xml | 2 +- .../qemuxml2argv-pseries-panic-no-address.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- 12 files changed, 85 insertions(+), 14 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c88b032..93b9813 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6152,19 +6152,44 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> ... </pre> + <p> + Example: usage of panic configuration for Hyper-V guests + </p> +<pre> + ... + <devices> + <panic model='hyperv'/> + </devices> + ... +</pre>
I think it would be enough to add <panic model='hyperv'/> as another device in the previous example. That is: <pre> ... <devices> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> + <panic model='hyperv'/> </devices> ... </pre>
<dl> + <dt><code>model</code></dt> + <dd> + <p> + The required <code>model</code> attribute specifies what type + of panic device is provided.
The attribute is optional. The panic model used when this attribute is missing depends on the hypervisor and guest arch.
+ </p> + <ul> + <li>'isa' — for ISA pvpanic device</li> + <li>'pseries' — default and valid only for pSeries guests.</li> + <li>'hyperv' — for Hyper-V crash CPU feature. + <span class="since">Since 2.5.0, QEMU and KVM only</span> + </li> + </ul>
We use nested <dl> with values enclosed in <code/>.
+ </dd> + <dt><code>address</code></dt> <dd> <p> address of panic. The default ioport is 0x505. Most users don't need to specify an address, and doing so is forbidden - altogether for pSeries guests. + altogether for pSeries guests and for Hyper-V crash.
+ altogether for pseries and hyperv models.
</p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..d67b1bf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5359,6 +5359,15 @@ <define name="panic"> <element name="panic"> <optional> + <attribute name="model"> + <choice> + <value>isa</value> + <value>pseries</value> + <value>hyperv</value> + </choice> + </attribute> + </optional> + <optional> <ref name="address"/> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb00444..935d429 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -525,6 +525,11 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, VIR_DOMAIN_WATCHDOG_ACTION_LAST, "none", "inject-nmi")
+VIR_ENUM_IMPL(virDomainPanicModel, VIR_DOMAIN_PANIC_MODEL_LAST,
You need to add a "default" model.
+ "isa", + "pseries", + "hyperv") + VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vga", "cirrus", @@ -10194,20 +10199,36 @@ virDomainTPMDefParseXML(xmlNodePtr node, }
static virDomainPanicDefPtr -virDomainPanicDefParseXML(xmlNodePtr node) +virDomainPanicDefParseXML(const virDomainDef *dom, xmlNodePtr node)
No need to change the prototype.
{ virDomainPanicDefPtr panic; + char *model = NULL;
if (VIR_ALLOC(panic) < 0) return NULL;
+ if (!(model = virXMLPropString(node, "model"))) { + if (ARCH_IS_PPC64(dom->os.arch) && STRPREFIX(dom->os.machine, "pseries")) + panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + else + panic->model = VIR_DOMAIN_PANIC_MODEL_ISA; + } else if ((panic->model = virDomainPanicModelTypeFromString(model)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown panic model '%s'"), model); + goto error; + } +
The only thing which should be done here is panic->model = virDomainPanicModelTypeFromString(model); if (panic->model < 0) { virReportError(...); goto error; } The rest belongs to QEMU specific postparse callback [1].
if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, 0) < 0) goto error;
+ cleanup: + VIR_FREE(model); return panic; + error: virDomainPanicDefFree(panic); - return NULL; + panic = NULL; + goto cleanup; }
/* Parse the XML definition for an input device */ @@ -12751,7 +12772,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_PANIC: - if (!(dev->data.panic = virDomainPanicDefParseXML(node))) + if (!(dev->data.panic = virDomainPanicDefParseXML(def, node))) goto error; break; case VIR_DOMAIN_DEVICE_MEMORY: @@ -16398,7 +16419,7 @@ virDomainDefParseXML(xmlDocPtr xml, } if (n > 0) { virDomainPanicDefPtr panic = - virDomainPanicDefParseXML(nodes[0]); + virDomainPanicDefParseXML(def, nodes[0]); if (!panic) goto error;
@@ -20627,10 +20648,12 @@ virDomainWatchdogDefFormat(virBufferPtr buf, static int virDomainPanicDefFormat(virBufferPtr buf, virDomainPanicDefPtr def) { + const char *model = virDomainPanicModelTypeToString(def->model); + virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; int indent = virBufferGetIndent(buf, false);
- virBufferAddLit(buf, "<panic"); + virBufferAsprintf(buf, "<panic model='%s'", model);
Only print model attribute if panic->model is set.
virBufferAdjustIndent(&childrenBuf, indent + 2); if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f10b534..00f3679 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2045,7 +2045,16 @@ struct _virDomainIdMapDef { };
+typedef enum {
Add VIR_DOMAIN_PANIC_MODEL_DEFAULT here so that you can distinguish whether the model was specified or not.
+ VIR_DOMAIN_PANIC_MODEL_ISA, + VIR_DOMAIN_PANIC_MODEL_PSERIES, + VIR_DOMAIN_PANIC_MODEL_HYPERV, + + VIR_DOMAIN_PANIC_MODEL_LAST +} virDomainPanicModel; + struct _virDomainPanicDef { + int model; virDomainDeviceInfo info; };
@@ -3060,6 +3069,7 @@ VIR_ENUM_DECL(virDomainMemballoonModel) VIR_ENUM_DECL(virDomainSmbiosMode) VIR_ENUM_DECL(virDomainWatchdogModel) VIR_ENUM_DECL(virDomainWatchdogAction) +VIR_ENUM_DECL(virDomainPanicModel) VIR_ENUM_DECL(virDomainVideo) VIR_ENUM_DECL(virDomainHostdevMode) VIR_ENUM_DECL(virDomainHostdevSubsys) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3d2b7a0..8ec3e8e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1180,6 +1180,10 @@ qemuDomainDefPostParse(virDomainDefPtr def,
This is the callback referenced by [1].
if (VIR_ALLOC(panic) < 0) goto cleanup;
+ if (ARCH_IS_PPC64(def->os.arch) && + STRPREFIX(def->os.machine, "pseries")) + panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + def->panic = panic; }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml index 79f8a1e..f3f3fbb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml @@ -24,6 +24,6 @@ <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> <memballoon model='virtio'/> - <panic/> + <panic model='isa'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic.xml index e354511..b9595a8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-panic.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic.xml @@ -24,7 +24,7 @@ <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> <memballoon model='virtio'/> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml index 3a96209..39f4a1f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml @@ -37,6 +37,6 @@ <model type='cirrus' vram='16384' heads='1'/> </video> <memballoon model='none'/> - <panic/> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml index 619186a..2da2832 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml @@ -20,6 +20,6 @@ <nvram> <address type='spapr-vio' reg='0x4000'/> </nvram> - <panic/> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml index e62ead8..be5b862 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml @@ -25,7 +25,7 @@ <address type='spapr-vio'/> </console> <memballoon model='none'/> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml index 9312975..8fcd644 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml @@ -25,6 +25,6 @@ <address type='spapr-vio'/> </console> <memballoon model='none'/> - <panic/> + <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml index 9312975..8fcd644 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -25,6 +25,6 @@ <address type='spapr-vio'/> </console> <memballoon model='none'/> - <panic/> + <panic model='pseries'/> </devices> </domain>
You should add tests with <panic/> and a corresponding <panic model='...'/> in xml2xmloutdata for both isa and pseries models to make sure we autogenerate the right model. Jirka

On Thu, Nov 12, 2015 at 12:56:27 +0100, Jiri Denemark wrote:
On Thu, Nov 12, 2015 at 14:07:38 +0300, Dmitry Andreev wrote:
Libvirt already has two types of panic devices - pvpanic and pSeries firmware. This patch introduces the 'model' attribute and a new type of panic device.
'isa' model is for ISA pvpanic device. 'pseries' model is a default value for pSeries guests. 'hyperv' model is the new type. It's used for Hyper-V crash.
Schema, docs and tests are updated for the new attribute. --- docs/formatdomain.html.in | 29 +++++++++++++++++-- docs/schemas/domaincommon.rng | 9 ++++++ src/conf/domain_conf.c | 33 ++++++++++++++++++---- src/conf/domain_conf.h | 10 +++++++ src/qemu/qemu_domain.c | 4 +++ .../qemuxml2argv-panic-no-address.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-panic.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- .../qemuxml2argv-pseries-panic-address.xml | 2 +- .../qemuxml2argv-pseries-panic-no-address.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- 12 files changed, 85 insertions(+), 14 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c88b032..93b9813 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6152,19 +6152,44 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> ... </pre> + <p> + Example: usage of panic configuration for Hyper-V guests + </p> +<pre> + ... + <devices> + <panic model='hyperv'/> + </devices> + ... +</pre>
I think it would be enough to add <panic model='hyperv'/> as another device in the previous example. That is:
<pre> ... <devices> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> + <panic model='hyperv'/> </devices> ... </pre>
And one more important thing. We will now support specifying more panic elements for a single domain so both the code and the schema have to be changed to reflect this. Jirka

XML: <devices> <panic model='hyperv'/> </devices> QEMU command line: qemu -cpu <cpu_model>,hv_crash --- src/qemu/qemu_command.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..6ad4240 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7623,6 +7623,16 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, } } + if (def->panic && + def->panic->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) { + if (!have_cpu) { + virBufferAdd(&buf, default_model, -1); + have_cpu = true; + } + + virBufferAddLit(&buf, ",hv_crash"); + } + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { if (!have_cpu) { virBufferAdd(&buf, default_model, -1); @@ -11150,17 +11160,36 @@ qemuBuildCommandLine(virConnectPtr conn, } if (def->panic) { - if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { - /* For pSeries guests, the firmware provides the same - * functionality as the pvpanic device. The address - * cannot be configured by the user */ + switch (def->panic->model) { + case VIR_DOMAIN_PANIC_MODEL_PSERIES: + if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 'pseries'")); + goto error; + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only pSeries guests support panic device " + "with model 'pseries'")); + goto error; + } + break; + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + /* Panic with model 'hyperv' is not a device, it should + * be configured in cpu commandline.*/ if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("setting the panic device address is not " - "supported for pSeries guests")); + "supported for model 'hyperv'")); goto error; } - } else { + break; + default: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the " @@ -12534,6 +12563,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (virCPUDefAddFeature(cpu, feature, policy) < 0) goto cleanup; } + } else if (STREQ(tokens[i], "hv_crash")) { + virDomainPanicDefPtr panic; + if (VIR_ALLOC(panic) < 0) + goto cleanup; + + panic->model = VIR_DOMAIN_PANIC_MODEL_HYPERV; + dom->panic = panic; } else if (STRPREFIX(tokens[i], "hv_")) { const char *token = tokens[i] + 3; /* "hv_" */ const char *feature, *value; -- 1.8.3.1

On Thu, Nov 12, 2015 at 14:07:39 +0300, Dmitry Andreev wrote:
XML: <devices> <panic model='hyperv'/> </devices>
QEMU command line: qemu -cpu <cpu_model>,hv_crash --- src/qemu/qemu_command.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..6ad4240 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7623,6 +7623,16 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, } }
+ if (def->panic && + def->panic->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) { + if (!have_cpu) { + virBufferAdd(&buf, default_model, -1); + have_cpu = true; + } + + virBufferAddLit(&buf, ",hv_crash"); + } + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { if (!have_cpu) { virBufferAdd(&buf, default_model, -1); @@ -11150,17 +11160,36 @@ qemuBuildCommandLine(virConnectPtr conn, }
if (def->panic) { - if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { - /* For pSeries guests, the firmware provides the same - * functionality as the pvpanic device. The address - * cannot be configured by the user */ + switch (def->panic->model) {
Typecast the model to let compiler check we didn't forget any: switch ((virDomainPanicModel) def->panic->model) {
+ case VIR_DOMAIN_PANIC_MODEL_PSERIES: + if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 'pseries'")); + goto error; + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only pSeries guests support panic device " + "with model 'pseries'")); + goto error; + } + break;
An empty line would be nice here.
+ case VIR_DOMAIN_PANIC_MODEL_HYPERV: + /* Panic with model 'hyperv' is not a device, it should + * be configured in cpu commandline.*/ if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("setting the panic device address is not " - "supported for pSeries guests")); + "supported for model 'hyperv'")); goto error; } - } else { + break; + default:
Avoid using default: in switch statements. Explicitly handle all possible values instead.
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the " @@ -12534,6 +12563,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (virCPUDefAddFeature(cpu, feature, policy) < 0) goto cleanup; } + } else if (STREQ(tokens[i], "hv_crash")) { + virDomainPanicDefPtr panic; + if (VIR_ALLOC(panic) < 0) + goto cleanup; + + panic->model = VIR_DOMAIN_PANIC_MODEL_HYPERV; + dom->panic = panic; } else if (STRPREFIX(tokens[i], "hv_")) { const char *token = tokens[i] + 3; /* "hv_" */ const char *feature, *value;
Jirka

--- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-hyperv-panic.args | 21 ++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 5 files changed, 49 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 34f3e5c..7759a09 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -261,6 +261,7 @@ mymain(void) DO_TEST("smp"); DO_TEST("hyperv"); + DO_TEST("hyperv-panic"); DO_TEST("kvm-features"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args new file mode 100644 index 0000000..a9f13e0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-cpu qemu32,hv_crash \ +-m 214 \ +-smp 6 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-boot n \ +-usb \ +-net none \ +-serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml new file mode 100644 index 0000000..9f0edbb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + <panic model='hyperv'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1c52828..700cad3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -686,6 +686,7 @@ mymain(void) DO_TEST("hyperv", NONE); DO_TEST("hyperv-off", NONE); + DO_TEST("hyperv-panic", NONE); DO_TEST("kvm-features", NONE); DO_TEST("kvm-features-off", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index cbd4d0d..9afe049 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -375,6 +375,7 @@ mymain(void) DO_TEST("hyperv"); DO_TEST("hyperv-off"); + DO_TEST("hyperv-panic"); DO_TEST("kvm-features"); DO_TEST("kvm-features-off"); -- 1.8.3.1

On Thu, Nov 12, 2015 at 14:07:40 +0300, Dmitry Andreev wrote:
--- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-hyperv-panic.args | 21 ++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 5 files changed, 49 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml
Another test using both at once <panic model='hyperv'/> <panic model='isa'/> would be nice. Jirka
participants (2)
-
Dmitry Andreev
-
Jiri Denemark