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

Hi, John! Looks like this series is stucked somehow even though there is almost 100% agreement.

On 17.10.2018 11:41, Nikolay Shirokovskiy wrote: > > > 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 >