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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list