[libvirt] [PATCH v2] qemu: Honour <on_reboot/>

[Michal Privoznik]

On 08/29/2017 12:57 PM, Martin Kletzander wrote:
> On Tue, Aug 29, 2017 at 12:46:43PM +0200, Michal Privoznik wrote:
>> On 08/29/2017 11:08 AM, Martin Kletzander wrote:
>>> On Wed, Aug 16, 2017 at 04:38:09PM +0200, Michal Privoznik wrote:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1476866
>>>>
>>>> For some reason, we completely ignore <on_reboot/> setting for
>>>> domains. The implementation is simply not there. It never was.
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>>> ---
>>>>
>>>> diff to v1:
>>>> - dropped the spoofed logic
>>>> - Switch from qemuProcessShutdownOrReboot() to qemuPrcoessStop()
>>>> because that's
>>>>  what <on_crash/> impl does too.
>>>>
>>>> src/qemu/qemu_process.c | 27 ++++++++++++++++++++++++---
>>>> 1 file changed, 24 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>>> index fed2bc588..3df6c320e 100644
>>>> --- a/src/qemu/qemu_process.c
>>>> +++ b/src/qemu/qemu_process.c
>>>> @@ -484,6 +484,7 @@ qemuProcessHandleReset(qemuMonitorPtr mon
>>>> ATTRIBUTE_UNUSED,
>>>>     virObjectEventPtr event;
>>>>     qemuDomainObjPrivatePtr priv;
>>>>     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>>>> +    int ret = -1;
>>>>
>>>>     virObjectLock(vm);
>>>>
>>>> @@ -495,12 +496,32 @@ qemuProcessHandleReset(qemuMonitorPtr mon
>>>> ATTRIBUTE_UNUSED,
>>>>     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
>>>> driver->caps) < 0)
>>>>         VIR_WARN("Failed to save status on vm %s", vm->def->name);
>>>>
>>>> +    if (vm->def->onReboot == VIR_DOMAIN_LIFECYCLE_DESTROY ||
>>>> +        vm->def->onReboot == VIR_DOMAIN_LIFECYCLE_PRESERVE) {
>>>> +
>>>> +        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>>>> +            goto cleanup;
>>>> +
>>>> +        if (!virDomainObjIsActive(vm)) {
>>>> +            VIR_DEBUG("Ignoring RESET event from inactive domain %s",
>>>> +                      vm->def->name);
>>>> +            goto endjob;
>>>> +        }
>>>> +
>>>> +        qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED,
>>>> +                        QEMU_ASYNC_JOB_NONE, 0);
>>>> +        virDomainAuditStop(vm, "destroyed");
>>>
>>> Queuing another event here that the domain is being destroyed seems both
>>> appropriate and weird to me.  So I'll leave it up to you.  It's not like
>>> anyone ever used this functionality... ever.  ACK either way.
>>
>> I think we want to queue the event here. Imagine that there's a mgmt app
>> that acts like HA daemon. Whenever a domain is destroyed it has to start
>> it up again. Well, with the current code it has to listen to RESET event
>> and depending on libvirt version it has to either ignore the event or
>> start it up again. However, if we queue the event all that the app has
>> to do is to listen to DESTROY event. Therefore I'm inclined to leave it
>> here.
>>
> 
> Leave?  I was talking about adding it.  I don't see it here.

Oh, I though that qemuProcessStop does queue an event. But it doesn't. I
shouldn't reply to e-mails right after having lunch. I'm going to post a
patch that does enqueue the event. Stay tuned.

Michal