[libvirt] [PATCH v4 2/2] qemu: Implement VIR_DUMP_RESET

[Eric Blake]

On 09/26/2011 10:19 AM, Michal Privoznik wrote:
> This patch extends qemudDomainCoreDump so it supports new VIR_DUMP_RESET
> flag. If this flag is set, domain is reset on successful dump. However,
> this is needed to be done after we start CPUs.
> ---
>   src/qemu/qemu_driver.c |    9 ++++++++-
>   1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0d0bea2..6199db5 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2866,11 +2866,13 @@ static int qemudDomainCoreDump(virDomainPtr dom,
>   {
>       struct qemud_driver *driver = dom->conn->privateData;
>       virDomainObjPtr vm;
> +    qemuDomainObjPrivatePtr priv;
>       int resume = 0, paused = 0;
>       int ret = -1;
>       virDomainEventPtr event = NULL;
>
> -    virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH | VIR_DUMP_BYPASS_CACHE, -1);
> +    virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH |
> +                  VIR_DUMP_BYPASS_CACHE | VIR_DUMP_RESET, -1);

This part is okay, but...

>
>       qemuDriverLock(driver);
>       vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> @@ -2938,6 +2940,11 @@ endjob:
>               if (virGetLastError() == NULL)
>                   qemuReportError(VIR_ERR_OPERATION_FAILED,
>                                   "%s", _("resuming after dump failed"));
> +        } else if ((ret == 0)&&  (flags&  VIR_DUMP_RESET)) {
> +            priv =  vm->privateData;
> +            qemuDomainObjEnterMonitorWithDriver(driver, vm);
> +            ret = qemuMonitorSystemReset(priv->mon);
> +            qemuDomainObjExitMonitorWithDriver(driver, vm);
>           }
>       }

Hmm.  This is inside a hunk that says if (resume && paused && active). 
But we want reset to work even if we don't need to resume the guest 
(that is, the guest was already paused before we triggered the core dump 
api).  Also, I don't see why qemuMonitorSystemReset has to wait for CPUs 
to be running.  Shouldn't the logic instead be:

else if (((resume && paused) || (flags & VIR_DUMP_RESET)) &&
          virDomainObjIsActive(vm)) {
     if (flags & VIR_DUMP_RESET)
         ... qemuMonitorSystemReset
     if (resume)
         ... qemuProcessStartCPUs
}

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org