[libvirt] [PATCH v4 RESEND 3/4] Add a watchdog action `dump'

[Daniel P. Berrange]

On Thu, Dec 02, 2010 at 03:30:10PM +0800, Hu Tao wrote:
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index f4f965e..ba41f80 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -191,6 +191,11 @@
>  # save_image_format = "raw"
>  # dump_image_format = "raw"
>  
> +# When a domain is configured to be auto-dumped when libvirtd receives a
> +# watchdog event from qemu guest, libvirtd will save dump files in directory
> +# specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump
> +#
> +# auto_dump_path = "/var/lib/libvirt/qemu/dump"
>  
>  # If provided by the host and a hugetlbfs mount point is configured,
>  # a guest may request huge page backing.  When this mount point is

Also need to list this new setting in qemu.aug and test_qemu.aug

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e534195..bd25d90 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6263,6 +6300,64 @@ cleanup:
>      return ret;
>  }
>  
> +static void processWatchdogEvent(void *data, void *opaque ATTRIBUTE_UNUSED)
> +{
> +    int ret;
> +    struct watchdogEvent *wdEvent = data;
> +
> +    switch (wdEvent->action) {
> +    case VIR_DOMAIN_WATCHDOG_ACTION_DUMP:
> +        {
> +            char *dumpfile;
> +            int i;
> +
> +            qemuDomainObjPrivatePtr priv = wdEvent->vm->privateData;
> +
> +            i = virAsprintf(&dumpfile, "%s/%s-%u",
> +                            qemu_driver->autoDumpPath,
> +                            wdEvent->vm->def->name,
> +                            (unsigned int)time(NULL));
> +
> +            qemuDriverLock(qemu_driver);
> +            virDomainObjLock(wdEvent->vm);
> +
> +            if (qemuDomainObjBeginJobWithDriver(qemu_driver, wdEvent->vm) < 0)
> +                break;
> +
> +            if (!virDomainObjIsActive(wdEvent->vm)) {
> +                qemuReportError(VIR_ERR_OPERATION_INVALID,
> +                                "%s", _("domain is not running"));
> +                break;
> +            }
> +
> +            ret = doCoreDump(qemu_driver,
> +                             wdEvent->vm,
> +                             dumpfile,
> +                             getCompressionType(qemu_driver));
> +            if (ret < 0)
> +                qemuReportError(VIR_ERR_OPERATION_FAILED,
> +                                "%s", _("Dump failed"));
> +
> +            qemuDomainObjEnterMonitorWithDriver(qemu_driver, wdEvent->vm);
> +            ret = qemuMonitorStartCPUs(priv->mon, NULL);
> +            qemuDomainObjExitMonitorWithDriver(qemu_driver, wdEvent->vm);
> +
> +            if (ret < 0)
> +                qemuReportError(VIR_ERR_OPERATION_FAILED,
> +                                "%s", _("Resuming after dump failed"));
> +
> +            ignore_value(qemuDomainObjEndJob(wdEvent->vm));
> +
> +            virDomainObjUnlock(wdEvent->vm);

It isn't safe to ignore the qemuDomainObjEndJob return value,
because if the return value is 0, then the VM object has been
free()d. So you need todo

    if (qemuDomainObjEndJob(wdEvent->vm) > 0)
           virDomainObjUnlock(wdEvent->vm);

> +            qemuDriverUnlock(qemu_driver);
> +
> +            VIR_FREE(dumpfile);
> +        }
> +        break;
> +    }
> +
> +    VIR_FREE(wdEvent);
> +}

I'd prefer it if the 'qemu_driver' was passed in via the 'void *opaque'
parameter. Although it is available via the global variable, we aim to
avoid using that in the driver code, except for the global startup/shutdown
methods.

Regards,
Daniel