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(a)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