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