1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/3] Clean/fix test driver usage of FindBy{UUID|ID}
  4. View patch

[libvirt] [PATCH v2 3/3] test: Need to relock afer virDomainObjListRemove

John Ferlan posted 3 patches 7 years, 1 month ago
[libvirt] [PATCH v2 3/3] test: Need to relock afer virDomainObjListRemove
Posted by John Ferlan 7 years, 1 month ago 
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@redhat.com>
---
 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] test: Need to relock afer virDomainObjListRemove
Posted by Marc Hartmayer 7 years, 1 month ago 
On Mon, Apr 02, 2018 at 02:27 PM +0200, John Ferlan <jferlan@redhat.com> 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@redhat.com>
> ---
>  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@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>

Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>

I would change the order of this patch series as the last patch is a
fix, no?

--
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] test: Need to relock afer virDomainObjListRemove
Posted by John Ferlan 7 years, 1 month ago 

On 04/04/2018 02:57 AM, Marc Hartmayer wrote:
> On Mon, Apr 02, 2018 at 02:27 PM +0200, John Ferlan <jferlan@redhat.com> 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@redhat.com>
>> ---
>>  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@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
> 
> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
> 
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.