[libvirt] [PATCHv2 2/3] qemu: add support for hv_crash feature as a panic device

[Jiri Denemark]

On Thu, Nov 12, 2015 at 14:07:39 +0300, Dmitry Andreev wrote:
> XML:
> <devices>
>   <panic model='hyperv'/>
> </devices>
> 
> QEMU command line:
> qemu -cpu <cpu_model>,hv_crash
> ---
>  src/qemu/qemu_command.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 792ada3..6ad4240 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7623,6 +7623,16 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver,
>          }
>      }
>  
> +    if (def->panic &&
> +        def->panic->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) {
> +        if (!have_cpu) {
> +            virBufferAdd(&buf, default_model, -1);
> +            have_cpu = true;
> +        }
> +
> +        virBufferAddLit(&buf, ",hv_crash");
> +    }
> +
>      if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
>          if (!have_cpu) {
>              virBufferAdd(&buf, default_model, -1);
> @@ -11150,17 +11160,36 @@ qemuBuildCommandLine(virConnectPtr conn,
>      }
>  
>      if (def->panic) {
> -        if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) {
> -            /* For pSeries guests, the firmware provides the same
> -             * functionality as the pvpanic device. The address
> -             * cannot be configured by the user */
> +        switch (def->panic->model) {

Typecast the model to let compiler check we didn't forget any:

           switch ((virDomainPanicModel) def->panic->model) {

> +        case VIR_DOMAIN_PANIC_MODEL_PSERIES:
> +            if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) {
> +                /* For pSeries guests, the firmware provides the same
> +                 * functionality as the pvpanic device. The address
> +                 * cannot be configured by the user */
> +                if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                   _("setting the panic device address is not "
> +                                     "supported for model 'pseries'"));
> +                    goto error;
> +                }
> +            } else {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("Only pSeries guests support panic device "
> +                                 "with model 'pseries'"));
> +                goto error;
> +            }
> +            break;

An empty line would be nice here.

> +        case VIR_DOMAIN_PANIC_MODEL_HYPERV:
> +            /* Panic with model 'hyperv' is not a device, it should
> +             * be configured in cpu commandline.*/
>              if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("setting the panic device address is not "
> -                                 "supported for pSeries guests"));
> +                                 "supported for model 'hyperv'"));
>                  goto error;
>              }
> -        } else {
> +            break;
> +        default:

Avoid using default: in switch statements. Explicitly handle all
possible values instead.

>              if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("the QEMU binary does not support the "
> @@ -12534,6 +12563,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
>                  if (virCPUDefAddFeature(cpu, feature, policy) < 0)
>                      goto cleanup;
>              }
> +        } else if (STREQ(tokens[i], "hv_crash")) {
> +            virDomainPanicDefPtr panic;
> +            if (VIR_ALLOC(panic) < 0)
> +                goto cleanup;
> +
> +            panic->model = VIR_DOMAIN_PANIC_MODEL_HYPERV;
> +            dom->panic = panic;
>          } else if (STRPREFIX(tokens[i], "hv_")) {
>              const char *token = tokens[i] + 3; /* "hv_" */
>              const char *feature, *value;

Jirka