[libvirt] [PATCHv5 4/9] qemu: add support for hv_crash feature as a panic device

[Jiri Denemark]

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: