[libvirt] [PATCH] qemu: Emit event if save fails

[Michal Privoznik]

On 06.11.2012 18:07, Eric Blake wrote:
> On 11/05/2012 08:05 AM, Michal Privoznik wrote:
>> When we are doing a (managed-) save of a domain, we stop its processors
>> firstly. And if the process of saving fails for some reason we try to
>> wake them up again. However, if this fails, an event should be emitted
>> so mgmt application can decide what to do.
>> ---
>>
>> I am not completely sure about combination of event and
>> event detail, maybe we need to invent a new combination.
>> Suggestions?  VIR_DOMAIN_EVENT_STOPPED_FAILED means an
>> hypervisor error which may fit when 'cont' fails.
>>
>>  src/qemu/qemu_driver.c |    6 +++++-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 978af57..e1c6067 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -2990,8 +2990,12 @@ endjob:
>>                  rc = qemuProcessStartCPUs(driver, vm, dom->conn,
>>                                            VIR_DOMAIN_RUNNING_SAVE_CANCELED,
>>                                            QEMU_ASYNC_JOB_SAVE);
>> -                if (rc < 0)
>> +                if (rc < 0) {
>>                      VIR_WARN("Unable to resume guest CPUs after save failure");
>> +                    event = virDomainEventNewFromObj(vm,
> 
> Memory leak - event might already be non-NULL, but you are discarding it
> by using your new event.

I don't think so. It is not clear from this 3 lines of context, but we
create an event iff 'ret' is zero. In which case this code is never run.

> 
>> +                                                     VIR_DOMAIN_EVENT_STOPPED,
> 
> Hmm, VIR_DOMAIN_EVENT_STOPPED seems wrong - if we failed, then the qemu
> process still exists, and we are paused (VIR_DOMAIN_EVENT_SUSPENDED),
> not stopped.
> 
>> +                                                     VIR_DOMAIN_EVENT_STOPPED_FAILED);
> 
> I think I would add a new category in libvirt.h.in; maybe
> VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR /* suspended after failure during
> libvirt API call */.  None of the existing VIR_DOMAIN_EVENT_SUSPENDED_*
> reasons seem to fit.  We may have other places in qemu_driver.c that
> should also use this new error (that is, we have several commands that
> temporarily pause the guest to perform an action, including Peter's
> recent work on external snapshots, and where we might leave the guest
> suspended on error).
> 

Yeah, I thought that too. Okay, I'll add a new one then and repost.

Michal