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