[libvirt] [PATCHv3 0/4] 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. The previous version of this patch is https://www.redhat.com/archives/libvir-list/2015-November/msg00400.html What is changed: * 'model' is an optional attribute and will not be added if it wasn't specified by the user [1/4] * docs text fixed [1/4] [3/4] * schema and code allows multiple panic devices [3/4] * more tests is added and all changes in old tests are removed [4/4] What isn't changed: I didn't add <code> tag around model values in docs because there are many examples of "<ul><li> 'value' —" without a <code> tag. Dmitry Andreev (4): conf: add 'model' attribute for panic device with values isa, pseries, hyperv qemu: add support for hv_crash feature as a panic device schema: allow multiple panic devices tests: add tests for panic models and multiple panic devices docs/formatdomain.html.in | 19 +++- docs/schemas/domaincommon.rng | 13 ++- src/conf/domain_conf.c | 101 ++++++++++++--------- src/conf/domain_conf.h | 15 ++- src/qemu/qemu_command.c | 84 +++++++++++++++-- src/qemu/qemu_domain.c | 21 ++++- 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-pseries.xml | 30 ++++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 4 + 15 files changed, 353 insertions(+), 64 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 -- 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 and docs are updated for the new attribute. --- docs/formatdomain.html.in | 18 ++++++++++++++++-- docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++-- src/conf/domain_conf.h | 11 +++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c88b032..ef30624 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 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 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..8339280 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 && + (int)(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 */ @@ -20629,8 +20648,13 @@ static int virDomainPanicDefFormat(virBufferPtr buf, { virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; int indent = virBufferGetIndent(buf, false); + const char *model = virDomainPanicModelTypeToString(def->model); + + if (def->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) + virBufferAddLit(buf, "<panic"); + else + virBufferAsprintf(buf, "<panic model='%s'", model); - virBufferAddLit(buf, "<panic"); 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..be18994 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; 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

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 --- src/qemu/qemu_command.c | 64 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..2d0cec0 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,49 @@ 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 + virDomainPanicModel model = def->panic->model; + + if (model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) { + if (ARCH_IS_PPC64(def->os.arch) && + STRPREFIX(def->os.machine, "pseries")) + model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + else + model = VIR_DOMAIN_PANIC_MODEL_ISA; + } + + switch (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 (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 " + "with 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 " @@ -11185,6 +11227,11 @@ qemuBuildCommandLine(virConnectPtr conn, "with ISA address type")); goto error; } + + /* default model value was changed before switch */ + case VIR_DOMAIN_PANIC_MODEL_DEFAULT: + case VIR_DOMAIN_PANIC_MODEL_LAST: + break; } } @@ -12534,6 +12581,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

'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'. --- docs/formatdomain.html.in | 1 + docs/schemas/domaincommon.rng | 4 +-- src/conf/domain_conf.c | 73 ++++++++++++++++++------------------------- src/conf/domain_conf.h | 4 ++- src/qemu/qemu_command.c | 50 ++++++++++++++++++----------- src/qemu/qemu_domain.c | 21 ++++++++++--- 6 files changed, 83 insertions(+), 70 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ef30624..7a14473 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 d67b1bf..8965976 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 8339280..2f17675 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; } @@ -16407,23 +16409,20 @@ 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) @@ -17633,17 +17632,14 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } static bool -virDomainPanicCheckABIStability(virDomainPanicDefPtr src, +virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, virDomainPanicDefPtr dst) { - if (!src && !dst) - return true; - - if (!src || !dst) { + if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain panic device count '%d' " - "does not match source count '%d'"), - src ? 1 : 0, dst ? 1 : 0); + _("Target panic model '%s' does not match source '%s'"), + virDomainPanicModelTypeToString(dst->model), + virDomainPanicModelTypeToString(src->model)); return false; } @@ -17709,13 +17705,6 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr src, } static bool -virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, - virDomainPanicDefPtr dst) -{ - return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); -} - -static bool virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, virDomainMemoryDefPtr dst) { @@ -18132,8 +18121,16 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainRNGDefCheckABIStability(src->rngs[i], dst->rngs[i])) goto error; - if (!virDomainPanicCheckABIStability(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, @@ -18157,16 +18154,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 " @@ -22460,9 +22447,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 be18994..43f99ac 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 2d0cec0..965b68e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7623,14 +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; - } + 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) { @@ -11159,8 +11161,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (def->panic) { - virDomainPanicModel model = def->panic->model; + for (i = 0; i < def->npanics; i++) { + virDomainPanicModel model = def->panics[i]->model; if (model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) { if (ARCH_IS_PPC64(def->os.arch) && @@ -11175,7 +11177,7 @@ qemuBuildCommandLine(virConnectPtr conn, /* Panic with model 'hyperv' is not a device, it should * be configured in cpu commandline. 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 'hyperv'")); @@ -11188,7 +11190,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'")); @@ -11210,11 +11212,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: @@ -12582,12 +12584,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 3d2b7a0..19ae5f7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1175,12 +1175,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

--- 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-pseries.xml | 30 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c | 4 +++ 9 files changed, 164 insertions(+) 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 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/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..63009d9 --- /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> + <address type='isa' iobase='0x505'/> + </panic> + </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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1c52828..b82ba95 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); @@ -1626,6 +1627,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 cbd4d0d..ea75aff 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,9 @@ mymain(void) DO_TEST("pcihole64-q35"); DO_TEST("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 Fri, Nov 13, 2015 at 11:30:30 +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.
This patch adds support for this feature.
The previous version of this patch is https://www.redhat.com/archives/libvir-list/2015-November/msg00400.html
What is changed: * 'model' is an optional attribute and will not be added if it wasn't specified by the user [1/4]
Ouch, there seems to be a misunderstanding here. I didn't want you to stop updating the model if it wasn't specified. The reason for adding a "default" model is: - XML parser takes the XML and leaves model as "default" in case it wasn't specified in the XML - post parse callback in qemu driver changes the default panic model to the right one (isa or pseries); we'd also need to check we don't have multiple panic devices with the same model - XML formatter only prints model if it is not default (won't happen with qemu driver, of course)
* docs text fixed [1/4] [3/4] * schema and code allows multiple panic devices [3/4] * more tests is added and all changes in old tests are removed [4/4]
What isn't changed:
I didn't add <code> tag around model values in docs because there are many examples of "<ul><li> 'value' —" without a <code> tag.
Fair enough. Jirka

On Fri, Nov 13, 2015 at 11:30:30 +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.
This patch adds support for this feature. n & Addressing -> Compose messages in HTML format for each ac The previous version of this patch is https://www.redhat.com/archives/libvir-list/2015-November/msg00400.html
What is changed: * 'model' is an optional attribute and will not be added if it wasn't specified by the user [1/4]
Ouch, there seems to be a misunderstanding here. I didn't want you to stop updating the model if it wasn't specified. The reason for adding a "default" model is:
- XML parser takes the XML and leaves model as "default" in case it wasn't specified in the XML - post parse callback in qemu driver changes the default panic model to User may change guest arch or hv but he still want to have default
the right one (isa or pseries); we'd also need to check we don't have multiple panic devices with the same model One per type? Should I use virDomainPanicDefPtr panics[VIR_DOMAIN_PANIC_MODEL_LAST]; or just send error from qemu_command if find multiple panic devices with
On 13.11.2015 12:12, Jiri Denemark wrote: panic device. Are you sure we need to set panic model? the same model in array?
- XML formatter only prints model if it is not default (won't happen with qemu driver, of course)
* docs text fixed [1/4] [3/4] * schema and code allows multiple panic devices [3/4] * more tests is added and all changes in old tests are removed [4/4]
What isn't changed:
I didn't add <code> tag around model values in docs because there are many examples of "<ul><li> 'value' —" without a <code> tag.
Fair enough.
Jirka
participants (2)
-
Dmitry Andreev
-
Jiri Denemark