Re: [libvirt] [PATCH 1/3] qemu: pass stop reason from qemuProcessStopCPUs to stop handler

On 16.10.2018 21:40, John Ferlan wrote: > > $SUBJ: > > s/pass/Pass > > On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote: >> We send duplicate suspended lifecycle events on qemu process stop in several >> places. The reason is stop event handler always send suspended event and >> we addidionally send same event but with more presise reason after call > > *additionally > *precise > >> to qemuProcessStopCPUs. Domain state change is also duplicated. >> >> Let's change domain state and send event only in handler. For this >> purpuse first let's pass state change reason to event handler (event > > *purpose > >> reason is deducible from it). >> >> Inspired by similar patch for resume: 5dab984ed >> "qemu: Pass running reason to RESUME event handler". >> > > In any case, I think the above was a bit more appropriate for the cover > since it details a few things being altered in the 3rd patch of the > series. So, maybe this could change to: > > Similar to commit 5dab984ed which saves and passes the running reason to > the RESUME event handler, during qemuProcessStopCPUs let's save and pass > the pause reason in the domain private data so that the STOP event > handler can use it. > >> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; >> --- >> src/qemu/qemu_domain.h | 4 ++++ >> src/qemu/qemu_process.c | 15 ++++++++++++--- >> 2 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h >> index 80bd4bd..380ea14 100644 >> --- a/src/qemu/qemu_domain.h >> +++ b/src/qemu/qemu_domain.h >> @@ -370,6 +370,10 @@ struct _qemuDomainObjPrivate { >> /* qemuProcessStartCPUs stores the reason for starting vCPUs here for the >> * RESUME event handler to use it */ >> virDomainRunningReason runningReason; >> + >> + /* qemuProcessStopCPUs stores the reason for starting vCPUs here for the > > s/starting/pausing/ > > too much copy-pasta > > FWIW: Similar to the comment I made to Jirka for his series, I assume > this STOP processing would have the similar issue with the event going > to the old libvirtd and thus not needing to save/restore via XML state > processing. For context see: > > https://www.redhat.com/archives/libvir-list/2018-September/msg01145.html > >> + * STOP event handler to use it */ >> + virDomainPausedReason pausedReason; >> }; >> > > With a couple of adjustments, > > Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; > > John > > I can make the adjustments so that you don't need to send another series > - just need your acknowledgment for that. > I'm ok with you changes Nikolay -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list