[libvirt] [PATCH v2 2/3] qemu: hot-plug of watchdog

[Peter Krempa]

On Wed, Sep 27, 2017 at 09:33:16 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1447169
> 
> Once again, since domain can have at most one watchdog it

again? You are mentioning it the first time in this series.

> simplifies things a bit. However, since we must be able to set
> the watchdog action as well, new monitor command needs to be
> used.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_alias.c                              | 13 ++++-
>  src/qemu/qemu_alias.h                              |  2 +
>  src/qemu/qemu_command.c                            |  2 +-
>  src/qemu/qemu_command.h                            |  4 +-
>  src/qemu/qemu_driver.c                             | 10 +++-
>  src/qemu/qemu_hotplug.c                            | 68 ++++++++++++++++++++++
>  src/qemu/qemu_hotplug.h                            |  3 +
>  src/qemu/qemu_monitor.c                            | 12 ++++
>  src/qemu/qemu_monitor.h                            |  2 +
>  src/qemu/qemu_monitor_json.c                       | 27 +++++++++
>  src/qemu/qemu_monitor_json.h                       |  3 +
>  tests/qemuhotplugtest.c                            |  9 ++-
>  .../qemuhotplug-watchdog.xml                       |  1 +
>  .../qemuhotplug-base-live+watchdog.xml             | 55 +++++++++++++++++
>  14 files changed, 206 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
>  create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml

[...]

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4913e18e6..885483c0f 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2836,6 +2836,74 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
>  }
>  
>  
> +int
> +qemuDomainAttachWatchdog(virQEMUDriverPtr driver,
> +                         virDomainObjPtr vm,
> +                         virDomainWatchdogDefPtr watchdog)
> +{
> +    int ret = -1;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_WATCHDOG, { .watchdog = watchdog } };
> +    virDomainWatchdogAction actualAction = watchdog->action;
> +    const char *actionStr = NULL;
> +    char *watchdogstr = NULL;
> +    bool releaseAddress = false;
> +    int rv;
> +
> +    if (vm->def->watchdog) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("domain already has a watchdog"));
> +        return -1;
> +    }
> +
> +    if (qemuAssignDeviceWatchdogAlias(watchdog) < 0)
> +        return -1;
> +
> +    if (!(watchdogstr = qemuBuildWatchdogDevStr(vm->def, watchdog, priv->qemuCaps)))
> +        return -1;
> +
> +    if (watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {

If you are not going to allow auto-generation of PCI address you should
explicitly reject ADDRESS_TYPE_NONE.

Otherwise the address won't be reserved. If the address type is based on
the model you'll need to be able to figure it out.

Also I presume that the ISA based ones won't support hotplug so you
could deal with that too.

> +        if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
> +            goto cleanup;
> +        releaseAddress = true;
> +    }
> +
> +    /* QEMU doesn't have a 'dump' action; we tell qemu to 'pause', then
> +       libvirt listens for the watchdog event, and we perform the dump
> +       ourselves. so convert 'dump' to 'pause' for the qemu cli */
> +    if (actualAction == VIR_DOMAIN_WATCHDOG_ACTION_DUMP)
> +        actualAction = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE;
> +
> +    actionStr = virDomainWatchdogActionTypeToString(actualAction);
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +
> +    rv = qemuMonitorSetWatchdogAction(priv->mon, actionStr);
> +
> +    if (rv >= 0)
> +        rv = qemuMonitorAddDevice(priv->mon, watchdogstr);
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> +        releaseAddress = false;
> +        virDomainAuditWatchdog(vm, watchdog, "attach", rv >= 0);

Reporting success here (in case of rv >= 0) is bogus since the VM died.

> +        goto cleanup;
> +    }
> +
> +    if (rv < 0)
> +        goto cleanup;
> +
> +    releaseAddress = false;
> +    vm->def->watchdog = watchdog;
> +    ret = 0;
> +
> + cleanup:
> +    if (releaseAddress)
> +        qemuDomainReleaseDeviceAddress(vm, &watchdog->info, NULL);
> +    VIR_FREE(watchdogstr);
> +    return ret;
> +}
> +
> +
>  static int
>  qemuDomainChangeNetBridge(virDomainObjPtr vm,
>                            virDomainNetDefPtr olddev,

[...]

>  #endif /* QEMU_MONITOR_H */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 63b855920..9876939ca 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -7692,3 +7692,30 @@ qemuMonitorJSONQueryNamedBlockNodes(qemuMonitorPtr mon)
>  
>      return ret;
>  }
> +
> +
> +int
> +qemuMonitorJSONSetWatchdogAction(qemuMonitorPtr mon,
> +                                 const char *action)
> +{
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +    int ret = -1;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("watchdog-set-action",
> +                                           "s:action", action,
> +                                           NULL)))
> +        return -1;
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)

Inconsistent spacing.

> +        goto cleanup;
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170927/60e6219f/attachment-0001.sig>