Hi John,
I have reworked this patch on the other new thread[1], please review that
patch.
Thank you :-)
[1] https://www.redhat.com/archives/libvir-list/2017-July/msg00921.html
>On 07/12/2017 07:34 AM, wang.yi59@xxxxxxxxxx wrote:
>> Hi John,
>>
>> Thanks for your review!
>>
>
>Somehow your response is out of synch with the rest of the series -
>things like this get lost very quickly.
>
>>> This seems to be a strange sequence of operations, but the claim is that
>>
>>> by adding this logic to CreateWithFlags, then the problem you're facing
>>
>>> is resolved. However, is adding this to the Create logic the right thing
>>
>>> to do?
>>
>>> IIUC:
>>
>> This condition is rare, so I designed some operations which can help us
>> to understand what happened:
>>
>> # virsh define win7 -> successful
>>
>> # virsh start win7 &; sleep 0.2; virsh undefine win7 -> start failed and
>> undefine successful
>>
>>
>>
>> and we may see libvirtd output such logs like this:
>>
>> qemuDomainDefineXMLFlags:7386 : Creating domain win7
>>
>> qemuProcessStart:6086 : conn=....
>>
>> qemuDomainUndefineFlags:7501 : Undefining domain win7
>>
>> error : qemuMonitorOpenUnix:379 : failed to connect to monitor socket
>>
>>
>>
>> Libvirtd unlocked vm during qemuProcessStart, so qemuDomainUndefineFlags
>> can get the lock and undefine vm successfully.
>
>Can you be a bit more specific. Do you mean during qemuConnectMonitor
>processing during ProcessLaunch? Or perhaps Agent processing?
>
>In any case, perhaps then the better fix is to prohibit undefine whilst
>unlocked for Monitor or Agent start. The perhaps interesting question is
>where to set flag. Setting/clearing just around Monitor/Agent startup
>would be a seemingly short window and perhaps where you're seeing the
>virObjectUnlock; however, I wonder if perhaps all of qemuProcessLaunch
>should be "protected" as there's some really critical things happening
>in there. Thus at the top one sets a boolean "obj->starting = true" and
>at cleanup "obj->starting = false".
>
>In qemuDomainUndefineFlags, there'd need to be a similar check to the if
>(!persistent) along the lines of "if (starting)" then an error message
>indicating that the domain is starting up and cannot be undefined.
>Perhaps similarly for Destroy just to be safe.
>
>I didn't think all that long about it, but hopefully it's enough to
>perhaps have to generate more patches... I don't believe with that it'd
>be possible to have the need to qemuDomainRemoveInactive in the
>CreateWithFlags, but I could be wrong.
---
Best wishes
Yi Wang
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
