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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list