[libvirt] [PATCHv5 0/9] 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. What is changed in v5: * minor code fixes * code was moved between patches * patch sequence changed Dmitry Andreev (9): conf: refactor code for checking ABI stability of panic device conf: add 'model' attribute for panic device with values isa, pseries, hyperv tests: add tests for the new panic device attribute - 'model' qemu: add support for hv_crash feature as a panic device tests: rework tests for panic devices tests: add tests for the new 'hyperv' panic device model Allow multiple panic devices tests: add tests for multiple panic devices conf: reject multiple panic devices of same model docs/formatdomain.html.in | 19 +++- docs/schemas/domaincommon.rng | 13 ++- src/conf/domain_conf.c | 123 +++++++++++++-------- src/conf/domain_conf.h | 15 ++- src/qemu/qemu_command.c | 80 ++++++++++++-- src/qemu/qemu_domain.c | 30 ++++- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-hyperv-panic.args | 21 ++++ .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 +++++ .../qemuxml2argv-panic-double.args | 21 ++++ .../qemuxml2argvdata/qemuxml2argv-panic-double.xml | 28 +++++ tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml | 31 ++++++ .../qemuxml2argv-panic-no-address.xml | 2 +- .../qemuxml2argv-panic-pseries.xml | 30 +++++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml | 31 ++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 30 +++++ tests/qemuxml2xmltest.c | 8 +- 21 files changed, 448 insertions(+), 69 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml -- 1.8.3.1

--- v5: this code was moved from another patch src/conf/domain_conf.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0ac7dbf..a14dd77 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17619,7 +17619,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } static bool -virDomainPanicCheckABIStability(virDomainPanicDefPtr src, +virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, virDomainPanicDefPtr dst) { if (!src && !dst) @@ -17695,13 +17695,6 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr src, } static bool -virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, - virDomainPanicDefPtr dst) -{ - return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); -} - -static bool virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, virDomainMemoryDefPtr dst) { @@ -18118,7 +18111,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainRNGDefCheckABIStability(src->rngs[i], dst->rngs[i])) goto error; - if (!virDomainPanicCheckABIStability(src->panic, dst->panic)) + if (!virDomainPanicDefCheckABIStability(src->panic, dst->panic)) goto error; if (src->nshmems != dst->nshmems) { @@ -18143,16 +18136,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } - if (src->panic && dst->panic) { - if (!virDomainPanicDefCheckABIStability(src->panic, dst->panic)) - goto error; - } else if (src->panic || dst->panic) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Either both target and source domains or none of " - "them must have PANIC device present")); - goto error; - } - if (src->nmems != dst->nmems) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain memory device count %zu " -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:30 +0300, Dmitry Andreev wrote:
--- v5: this code was moved from another patch src/conf/domain_conf.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0ac7dbf..a14dd77 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17619,7 +17619,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, }
static bool -virDomainPanicCheckABIStability(virDomainPanicDefPtr src, +virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, virDomainPanicDefPtr dst) { if (!src && !dst) ...
ACK with the following patch squashed in. Jirka diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 4bbf941..7f4c643 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -17614,7 +17614,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, static bool virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, - virDomainPanicDefPtr dst) + virDomainPanicDefPtr dst) { if (!src && !dst) return true;

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 and docs are updated for the new attribute. --- v5: minor fixes docs/formatdomain.html.in | 18 ++++++++++++++++-- docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 11 +++++++++++ 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e5e0167..32b196d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6152,19 +6152,33 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <panic> + <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> </devices> ... </pre> <dl> + <dt><code>model</code></dt> + <dd> + <p> + The optional <code>model</code> attribute specifies what type + of panic device is provided. 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 1.3.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 and hyperv models. </p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 994face..9d21650 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5361,6 +5361,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 a14dd77..b8738af 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -525,6 +525,12 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, VIR_DOMAIN_WATCHDOG_ACTION_LAST, "none", "inject-nmi") +VIR_ENUM_IMPL(virDomainPanicModel, VIR_DOMAIN_PANIC_MODEL_LAST, + "default", + "isa", + "pseries", + "hyperv") + VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vga", "cirrus", @@ -10197,6 +10203,7 @@ static virDomainPanicDefPtr virDomainPanicDefParseXML(xmlNodePtr node) { virDomainPanicDefPtr panic; + char *model = NULL; if (VIR_ALLOC(panic) < 0) return NULL; @@ -10204,10 +10211,22 @@ virDomainPanicDefParseXML(xmlNodePtr node) if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, 0) < 0) goto error; + model = virXMLPropString(node, "model"); + if (model != NULL && + (panic->model = virDomainPanicModelTypeFromString(model)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown panic model '%s'"), model); + 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 */ @@ -17633,6 +17652,14 @@ virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, return false; } + if (src->model != dst->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target panic model '%s' does not match source '%s'"), + virDomainPanicModelTypeToString(dst->model), + virDomainPanicModelTypeToString(src->model)); + return false; + } + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); } @@ -20619,6 +20646,11 @@ static int virDomainPanicDefFormat(virBufferPtr buf, int indent = virBufferGetIndent(buf, false); virBufferAddLit(buf, "<panic"); + + if (def->model) + virBufferAsprintf(buf, " model='%s'", + virDomainPanicModelTypeToString(def->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 8d43ee6..11d891f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2045,7 +2045,17 @@ struct _virDomainIdMapDef { }; +typedef enum { + VIR_DOMAIN_PANIC_MODEL_DEFAULT, + 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; /* virDomainPanicModel */ virDomainDeviceInfo info; }; @@ -3060,6 +3070,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) -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:31 +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 and docs are updated for the new attribute. --- v5: minor fixes
docs/formatdomain.html.in | 18 ++++++++++++++++-- docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 11 +++++++++++ 4 files changed, 69 insertions(+), 3 deletions(-)
ACK Jirka

--- v5: tests was moved from another patch .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 +++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml | 31 ++++++++++++++++++++++ .../qemuxml2argv-panic-pseries.xml | 30 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 3 +++ 4 files changed, 89 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml 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/qemuxml2argvdata/qemuxml2argv-panic-isa.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml new file mode 100644 index 0000000..b9595a8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-isa.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 model='isa'> + <address type='isa' iobase='0x505'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml new file mode 100644 index 0000000..8fcd644 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-system-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio'/> + </console> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index cbd4d0d..fbb46d6 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"); @@ -591,6 +592,8 @@ mymain(void) DO_TEST("pcihole64-q35"); DO_TEST("panic"); + DO_TEST("panic-isa"); + DO_TEST("panic-pseries"); DO_TEST("panic-no-address"); DO_TEST_DIFFERENT("disk-backing-chains"); -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:32 +0300, Dmitry Andreev wrote:
--- v5: tests was moved from another patch
.../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 +++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml | 31 ++++++++++++++++++++++ .../qemuxml2argv-panic-pseries.xml | 30 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 3 +++ 4 files changed, 89 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml
ACK Jirka

Panic device type used depends on 'model' attribute. If no model is specified then device type depends on hypervisor and guest arch. 'pseries' model is used for pSeries guest and 'isa' model is used in other cases. XML: <devices> <panic model='hyperv'/> </devices> QEMU command line: qemu -cpu <cpu_model>,hv_crash --- v5: * autoselection of panic device model in post parse was moved from another patch * ARCH_IS_X86 check for 'hyperv' panic device was added src/qemu/qemu_command.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_domain.c | 9 ++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4d00fd9..bb6d5fe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7577,6 +7577,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); @@ -11050,17 +11060,45 @@ 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 + switch ((virDomainPanicModel) def->panic->model) { + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + /* Panic with model 'hyperv' is not a device, it should + * be configured in cpu commandline. The address * cannot be configured by the user */ + if (!ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only i686 and x86_64 architectures are support " + "panic device of model 'hyperv'")); + goto error; + } 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; + + 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 " + "of model 'pseries'")); + goto error; + } + break; + + case VIR_DOMAIN_PANIC_MODEL_ISA: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the " @@ -11085,6 +11123,11 @@ qemuBuildCommandLine(virConnectPtr conn, "with ISA address type")); goto error; } + + /* default model value was changed before in post parse */ + case VIR_DOMAIN_PANIC_MODEL_DEFAULT: + case VIR_DOMAIN_PANIC_MODEL_LAST: + break; } } @@ -12436,6 +12479,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; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 18513f9..689abc2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1350,6 +1350,15 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } + if (dev->type == VIR_DOMAIN_DEVICE_PANIC && + dev->data.panic->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) { + if (ARCH_IS_PPC64(def->os.arch) && + STRPREFIX(def->os.machine, "pseries")) + dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + else + dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_ISA; + } + ret = 0; cleanup: -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:33 +0300, Dmitry Andreev wrote:
Panic device type used depends on 'model' attribute.
If no model is specified then device type depends on hypervisor and guest arch. 'pseries' model is used for pSeries guest and 'isa' model is used in other cases.
XML: <devices> <panic model='hyperv'/> </devices>
QEMU command line: qemu -cpu <cpu_model>,hv_crash --- v5: * autoselection of panic device model in post parse was moved from another patch * ARCH_IS_X86 check for 'hyperv' panic device was added
src/qemu/qemu_command.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_domain.c | 9 ++++++++ 2 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4d00fd9..bb6d5fe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c ... @@ -11050,17 +11060,45 @@ 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 + switch ((virDomainPanicModel) def->panic->model) { + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + /* Panic with model 'hyperv' is not a device, it should + * be configured in cpu commandline. The address * cannot be configured by the user */ + if (!ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only i686 and x86_64 architectures are support " + "panic device of model 'hyperv'")); + goto error; + } 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; + + 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 " + "of model 'pseries'")); + goto error; + } + break;
This could be reorganized a bit so that the flow is similar to heprv panic model.
+ + case VIR_DOMAIN_PANIC_MODEL_ISA: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the "
Some tests fail after this patch, the next patch (5/9) needs to be squashed in this one to make them succeed again. The following patch (6/9) is not necessary, but it is small enough and it logically fits here, which makes it a good candidate for squashing. ACK with patches 5/9 and 6/9, and the following diff squashed in. Jirka diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index bb6d5fe..e83be58 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -11067,7 +11067,7 @@ qemuBuildCommandLine(virConnectPtr conn, * cannot be configured by the user */ if (!ARCH_IS_X86(def->os.arch)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only i686 and x86_64 architectures are support " + _("only i686 and x86_64 guests support " "panic device of model 'hyperv'")); goto error; } @@ -11080,22 +11080,22 @@ qemuBuildCommandLine(virConnectPtr conn, break; 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 { + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (!ARCH_IS_PPC64(def->os.arch) || + !STRPREFIX(def->os.machine, "pseries")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only pSeries guests support panic device " "of model 'pseries'")); goto error; } + 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; + } break; case VIR_DOMAIN_PANIC_MODEL_ISA:

Panic device model will be selected automatically if it is not chosen by the user. Tests should cover this case. --- .../qemuxml2argv-panic-no-address.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml | 31 ++++++++++++++++++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 30 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +-- 7 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml 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-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/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml new file mode 100644 index 0000000..b9595a8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-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 model='isa'> + <address type='isa' iobase='0x505'/> + </panic> + </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> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml new file mode 100644 index 0000000..8fcd644 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-system-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio'/> + </console> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index fbb46d6..0e43ee9 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -535,7 +535,7 @@ mymain(void) DO_TEST("pseries-nvram"); DO_TEST_DIFFERENT("pseries-panic-missing"); - DO_TEST("pseries-panic-no-address"); + DO_TEST_DIFFERENT("pseries-panic-no-address"); /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); @@ -591,7 +591,7 @@ mymain(void) DO_TEST("pcihole64-none"); DO_TEST("pcihole64-q35"); - DO_TEST("panic"); + DO_TEST_DIFFERENT("panic"); DO_TEST("panic-isa"); DO_TEST("panic-pseries"); DO_TEST("panic-no-address"); -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:34 +0300, Dmitry Andreev wrote:
Panic device model will be selected automatically if it is not chosen by the user. Tests should cover this case. --- .../qemuxml2argv-panic-no-address.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2argv-pseries-nvram.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml | 31 ++++++++++++++++++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 30 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +-- 7 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml
ACK if squashed in 4/9. Jirka

--- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.args | 21 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 23 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args 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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index dc8654e..a15305d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -692,6 +692,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); -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:35 +0300, Dmitry Andreev wrote:
--- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.args | 21 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 23 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args
ACK if squashed in 4/9. Jirka

'model' attribute was added to a panic device but only one panic device is allowed. This patch changes panic device presence from 'optional' to 'zeroOrMore'. --- v5: part of the code was moved out from this commit docs/formatdomain.html.in | 1 + docs/schemas/domaincommon.rng | 4 ++-- src/conf/domain_conf.c | 53 ++++++++++++++++++++----------------------- src/conf/domain_conf.h | 4 +++- src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++---------------- src/qemu/qemu_domain.c | 21 +++++++++++++---- 6 files changed, 77 insertions(+), 56 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 32b196d..06aec4b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6152,6 +6152,7 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> + <panic model='hyperv'/> <panic model='isa'> <address type='isa' iobase='0x505'/> </panic> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9d21650..7e7fd58 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4044,9 +4044,9 @@ <optional> <ref name="nvram"/> </optional> - <optional> + <zeroOrMore> <ref name="panic"/> - </optional> + </zeroOrMore> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8738af..ef322f5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2538,7 +2538,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainTPMDefFree(def->tpm); - virDomainPanicDefFree(def->panic); + for (i = 0; i < def->npanics; i++) + virDomainPanicDefFree(def->panics[i]); + VIR_FREE(def->panics); VIR_FREE(def->idmap.uidmap); VIR_FREE(def->idmap.gidmap); @@ -3617,10 +3619,10 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->tpm->info, opaque) < 0) return -1; } - if (def->panic) { - device.type = VIR_DOMAIN_DEVICE_PANIC; - device.data.panic = def->panic; - if (cb(def, &device, &def->panic->info, opaque) < 0) + device.type = VIR_DOMAIN_DEVICE_PANIC; + for (i = 0; i < def->npanics; i++) { + device.data.panic = def->panics[i]; + if (cb(def, &device, &def->panics[i]->info, opaque) < 0) return -1; } @@ -16412,23 +16414,19 @@ 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")); + if (n && VIR_ALLOC_N(def->panics, n) < 0) goto error; - } - if (n > 0) { + for (i = 0; i < n; i++) { virDomainPanicDefPtr panic = - virDomainPanicDefParseXML(nodes[0]); + virDomainPanicDefParseXML(nodes[i]); if (!panic) goto error; - def->panic = panic; - VIR_FREE(nodes); + def->panics[def->npanics++] = panic; } + VIR_FREE(nodes); /* analysis of the shmem devices */ if ((n = virXPathNodeSet("./devices/shmem", ctxt, &nodes)) < 0) @@ -17641,17 +17639,6 @@ static bool virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, virDomainPanicDefPtr dst) { - if (!src && !dst) - return true; - - if (!src || !dst) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain panic device count '%d' " - "does not match source count '%d'"), - src ? 1 : 0, dst ? 1 : 0); - return false; - } - if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target panic model '%s' does not match source '%s'"), @@ -18138,8 +18125,16 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainRNGDefCheckABIStability(src->rngs[i], dst->rngs[i])) goto error; - if (!virDomainPanicDefCheckABIStability(src->panic, dst->panic)) + if (src->npanics != dst->npanics) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain panic device count %zu " + "does not match source %zu"), dst->npanics, src->npanics); goto error; + } + + for (i = 0; i < src->npanics; i++) + if (!virDomainPanicDefCheckABIStability(src->panics[i], dst->panics[i])) + goto error; if (src->nshmems != dst->nshmems) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -22457,9 +22452,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->nvram) virDomainNVRAMDefFormat(buf, def->nvram, flags); - if (def->panic && - virDomainPanicDefFormat(buf, def->panic) < 0) - goto error; + for (n = 0; n < def->npanics; n++) + if (virDomainPanicDefFormat(buf, def->panics[n]) < 0) + goto error; for (n = 0; n < def->nshmems; n++) if (virDomainShmemDefFormat(buf, def->shmems[n], flags) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 11d891f..038d65b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2316,6 +2316,9 @@ struct _virDomainDef { size_t nmems; virDomainMemoryDefPtr *mems; + size_t npanics; + virDomainPanicDefPtr *panics; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2324,7 +2327,6 @@ struct _virDomainDef { virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; - virDomainPanicDefPtr panic; void *namespaceData; virDomainXMLNamespace ns; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bb6d5fe..5eb8ac0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7577,14 +7577,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; - } + for (i = 0; i < def->npanics; i++) { + if (def->panics[i]->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) { + if (!have_cpu) { + virBufferAdd(&buf, default_model, -1); + have_cpu = true; + } - virBufferAddLit(&buf, ",hv_crash"); + virBufferAddLit(&buf, ",hv_crash"); + break; + } } if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { @@ -11059,8 +11061,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (def->panic) { - switch ((virDomainPanicModel) def->panic->model) { + for (i = 0; i < def->npanics; i++) { + switch ((virDomainPanicModel) def->panics[i]->model) { case VIR_DOMAIN_PANIC_MODEL_HYPERV: /* Panic with model 'hyperv' is not a device, it should * be configured in cpu commandline. The address @@ -11071,7 +11073,7 @@ qemuBuildCommandLine(virConnectPtr conn, "panic device of model 'hyperv'")); goto error; } - if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("setting the panic device address is not " "supported for model 'hyperv'")); @@ -11084,7 +11086,7 @@ qemuBuildCommandLine(virConnectPtr conn, /* 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) { + if (def->panics[i]->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'")); @@ -11106,11 +11108,11 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - switch (def->panic->info.type) { + switch (def->panics[i]->info.type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: virCommandAddArg(cmd, "-device"); virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", - def->panic->info.addr.isa.iobase); + def->panics[i]->info.addr.isa.iobase); break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: @@ -12480,12 +12482,22 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, 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; + size_t j; + for (j = 0; j < dom->npanics; j++) { + if (dom->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) + break; + } + + if (j == dom->npanics) { + virDomainPanicDefPtr panic; + if (VIR_ALLOC(panic) < 0 || + VIR_APPEND_ELEMENT_COPY(dom->panics, + dom->npanics, panic) < 0) { + VIR_FREE(panic); + goto cleanup; + } + panic->model = VIR_DOMAIN_PANIC_MODEL_HYPERV; + } } else if (STRPREFIX(tokens[i], "hv_")) { const char *token = tokens[i] + 3; /* "hv_" */ const char *feature, *value; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 689abc2..8e6c038 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1178,12 +1178,23 @@ qemuDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_INPUT_BUS_USB) < 0) goto cleanup; - if (addPanicDevice && !def->panic) { - virDomainPanicDefPtr panic; - if (VIR_ALLOC(panic) < 0) - goto cleanup; + if (addPanicDevice) { + size_t j; + for (j = 0; j < def->npanics; j++) { + if (def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT || + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) + break; + } - def->panic = panic; + if (j == def->npanics) { + virDomainPanicDefPtr panic; + if (VIR_ALLOC(panic) < 0 || + VIR_APPEND_ELEMENT_COPY(def->panics, + def->npanics, panic) < 0) { + VIR_FREE(panic); + goto cleanup; + } + } } ret = 0; -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:36 +0300, Dmitry Andreev wrote:
'model' attribute was added to a panic device but only one panic device is allowed. This patch changes panic device presence from 'optional' to 'zeroOrMore'. --- v5: part of the code was moved out from this commit
docs/formatdomain.html.in | 1 + docs/schemas/domaincommon.rng | 4 ++-- src/conf/domain_conf.c | 53 ++++++++++++++++++++----------------------- src/conf/domain_conf.h | 4 +++- src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++---------------- src/qemu/qemu_domain.c | 21 +++++++++++++---- 6 files changed, 77 insertions(+), 56 deletions(-)
ACK with the following patch squashed in. And I think the next patch which tests this new functionality is small enough that it would be nice to have it squashed in this one. Jirka diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index d6977f2..2bddb67 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -18126,9 +18126,10 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } - for (i = 0; i < src->npanics; i++) + for (i = 0; i < src->npanics; i++) { if (!virDomainPanicDefCheckABIStability(src->panics[i], dst->panics[i])) goto error; + } if (src->nshmems != dst->nshmems) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

--- .../qemuxml2argv-panic-double.args | 21 ++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-panic-double.xml | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 1 + 4 files changed, 52 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-double.args b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.args new file mode 100644 index 0000000..0d3a385 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.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 \ +-nodefconfig \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-boot n \ +-usb \ +-device pvpanic,ioport=1285 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml new file mode 100644 index 0000000..aadb758 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml @@ -0,0 +1,28 @@ +<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'/> + <panic model='isa'> + <address type='isa' iobase='0x505'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a15305d..ea16c98 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1633,6 +1633,8 @@ mymain(void) DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("panic-double", QEMU_CAPS_DEVICE_PANIC, + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("panic-no-address", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0e43ee9..fd58331 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -594,6 +594,7 @@ mymain(void) DO_TEST_DIFFERENT("panic"); DO_TEST("panic-isa"); DO_TEST("panic-pseries"); + DO_TEST("panic-double"); DO_TEST("panic-no-address"); DO_TEST_DIFFERENT("disk-backing-chains"); -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:37 +0300, Dmitry Andreev wrote:
--- .../qemuxml2argv-panic-double.args | 21 ++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-panic-double.xml | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 1 + 4 files changed, 52 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml
ACK if squashed in 7/9. Jirka

Only one panic device per model is allowed. --- v5: minor code fixes src/conf/domain_conf.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef322f5..c30f420 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3747,6 +3747,28 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr def) return ret; } +static int +virDomainDefRejectDuplicatePanics(virDomainDefPtr def) +{ + bool exists[VIR_DOMAIN_PANIC_MODEL_LAST]; + size_t i; + + for (i = 0; i < VIR_DOMAIN_PANIC_MODEL_LAST; i++) + exists[i] = false; + + for (i = 0; i < def->npanics; i++) { + virDomainPanicModel model = def->panics[i]->model; + if (exists[model]) { + virReportError(VIR_ERR_XML_ERROR, + _("Multiple panic devices with model '%s'"), + virDomainPanicModelTypeToString(model)); + return -1; + } + exists[model] = true; + } + + return 0; +} /** * virDomainDefMetadataSanitize: @@ -3976,6 +3998,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefRejectDuplicateControllers(def) < 0) return -1; + if (virDomainDefRejectDuplicatePanics(def) < 0) + return -1; + /* verify settings of guest timers */ for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDefPtr timer = def->clock.timers[i]; -- 1.8.3.1

On Tue, Nov 24, 2015 at 15:26:38 +0300, Dmitry Andreev wrote:
Only one panic device per model is allowed. --- v5: minor code fixes
src/conf/domain_conf.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
ACK Jirka

On Tue, Nov 24, 2015 at 15:26:29 +0300, Dmitry Andreev wrote:
A new Hyper-V cpu feature 'hv_crash' was added to QEMU. The feature will become available in v2.5.0.
What is changed in v5: * minor code fixes * code was moved between patches * patch sequence changed
Dmitry Andreev (9): conf: refactor code for checking ABI stability of panic device conf: add 'model' attribute for panic device with values isa, pseries, hyperv tests: add tests for the new panic device attribute - 'model' qemu: add support for hv_crash feature as a panic device tests: rework tests for panic devices tests: add tests for the new 'hyperv' panic device model Allow multiple panic devices tests: add tests for multiple panic devices conf: reject multiple panic devices of same model
Thanks for the good work and patience. I think the series is pretty good now. I found a few nits which are not really worth a respin so I fixed them (see my replies to individual patches) and pushed the series. Jirka
participants (2)
-
Dmitry Andreev
-
Jiri Denemark