<div class="zcontentRow"> Hi John,I have reworked this patch on the other new thread[1], please review thatpatch.Thank you :-) [1] https://www.redhat.com/archives/libvir-list/2017-July/msg00921.html >On 07/12/2017 07:34 AM, wang.yi59@zte.com.cn 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. <div class="zMailSign"><div><div>---Best wishesYi Wang</div></div></div> </div>