On 01/18/2017 06:34 PM, John Ferlan wrote: > > > On 01/10/2017 01:23 AM, Wang King wrote: >> Avoid return with the closeCallbacks locked when get callbacks list for connect fail. >> >> Signed-off-by: Wang King <king.wang@xxxxxxxxxx> >> --- >> src/util/virclosecallbacks.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> > > ACK, > > John > I ended up pushing this change (a few days ago) and then realizing almost immediately that the !list option was to Lock not Unlock - so I fixed that... I guess the power of my original suggestion to just have the Unlock on the !list outweighed the reality of what was written the patch and I just assumed. In any case, that part is resolved. As for patches 2 & 3, I've proposed something slightly different with attribution to you, see: http://www.redhat.com/archives/libvir-list/2017-January/msg01025.html I decided that using your patch 3 concept of taking the extra ref would be wise, although perhaps unnecessary since the callers each would essentially return NULL when the vm object was removed from the list, so as long as we removed the reference before the call the result would be the same. Still for a better look and feel with respect to consistency, using EndAPI is the better solution IMO. John >> diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c >> index 891a92b..1fa9596 100644 >> --- a/src/util/virclosecallbacks.c >> +++ b/src/util/virclosecallbacks.c >> @@ -331,8 +331,10 @@ virCloseCallbacksRun(virCloseCallbacksPtr closeCallbacks, >> >> virObjectLock(closeCallbacks); >> list = virCloseCallbacksGetForConn(closeCallbacks, conn); >> - if (!list) >> + if (!list) { >> + virObjectLock(closeCallbacks); >> return; >> + } >> >> for (i = 0; i < list->nentries; i++) { >> char uuidstr[VIR_UUID_STRING_BUFLEN]; >> > > -- > 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
