On 04/04/2018 02:57 AM, Marc Hartmayer wrote: > On Mon, Apr 02, 2018 at 02:27 PM +0200, John Ferlan <jferlan@xxxxxxxxxx> wrote: >> For all @dom's fetched from a testDomObjFromDomain because >> virDomainObjListRemove will return an unlocked domain object >> we should relock it prior to the cleanup label which will use >> virDomainObjEndAPI which would Unlock and Unref the passed >> object (and we should avoid unlocking an unlocked object). >> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> src/test/test_driver.c | 26 +++++++++++++++++++------- >> 1 file changed, 19 insertions(+), 7 deletions(-) >> >> diff --git a/src/test/test_driver.c b/src/test/test_driver.c >> index 3c01aa50e1..5e39ae574c 100644 >> --- a/src/test/test_driver.c >> +++ b/src/test/test_driver.c >> @@ -1802,8 +1802,10 @@ static int testDomainDestroyFlags(virDomainPtr domain, >> VIR_DOMAIN_EVENT_STOPPED, >> VIR_DOMAIN_EVENT_STOPPED_DESTROYED); >> >> - if (!privdom->persistent) >> + if (!privdom->persistent) { >> virDomainObjListRemove(privconn->domains, privdom); >> + virObjectLock(privdom); >> + } >> >> ret = 0; >> cleanup: >> @@ -1901,8 +1903,10 @@ static int testDomainShutdownFlags(virDomainPtr domain, >> VIR_DOMAIN_EVENT_STOPPED, >> VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); >> >> - if (!privdom->persistent) >> + if (!privdom->persistent) { >> virDomainObjListRemove(privconn->domains, privdom); >> + virObjectLock(privdom); >> + } >> >> ret = 0; >> cleanup: >> @@ -1971,8 +1975,10 @@ static int testDomainReboot(virDomainPtr domain, >> VIR_DOMAIN_EVENT_STOPPED, >> VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); >> >> - if (!privdom->persistent) >> + if (!privdom->persistent) { >> virDomainObjListRemove(privconn->domains, privdom); >> + virObjectLock(privdom); >> + } >> } >> >> ret = 0; >> @@ -2110,8 +2116,10 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, >> VIR_DOMAIN_EVENT_STOPPED, >> VIR_DOMAIN_EVENT_STOPPED_SAVED); >> >> - if (!privdom->persistent) >> + if (!privdom->persistent) { >> virDomainObjListRemove(privconn->domains, privdom); >> + virObjectLock(privdom); >> + } >> >> ret = 0; >> cleanup: >> @@ -2296,8 +2304,10 @@ static int testDomainCoreDumpWithFormat(virDomainPtr domain, >> event = virDomainEventLifecycleNewFromObj(privdom, >> VIR_DOMAIN_EVENT_STOPPED, >> VIR_DOMAIN_EVENT_STOPPED_CRASHED); >> - if (!privdom->persistent) >> + if (!privdom->persistent) { >> virDomainObjListRemove(privconn->domains, privdom); >> + virObjectLock(privdom); >> + } >> } >> >> ret = 0; >> @@ -3076,10 +3086,12 @@ static int testDomainUndefineFlags(virDomainPtr domain, >> VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); >> privdom->hasManagedSave = false; >> >> - if (virDomainObjIsActive(privdom)) >> + if (virDomainObjIsActive(privdom)) { >> privdom->persistent = 0; >> - else >> + } else { >> virDomainObjListRemove(privconn->domains, privdom); >> + virObjectLock(privdom); >> + } >> >> ret = 0; >> >> -- >> 2.13.6 >> >> -- >> libvir-list mailing list >> libvir-list@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/libvir-list >> > > Reviewed-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx> > > I would change the order of this patch series as the last patch is a > fix, no? I can move it to be first as it's not related to the other two. Thanks for the reviews! John > > -- > Beste Grüße / Kind regards > Marc Hartmayer > > IBM Deutschland Research & Development GmbH > Vorsitzende des Aufsichtsrats: Martina Koederitz > Geschäftsführung: Dirk Wittkopp > Sitz der Gesellschaft: Böblingen > Registergericht: Amtsgericht Stuttgart, HRB 243294 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list