Re: [libvirt] [PATCH v1 07/11] virStoragePoolObjListAdd: Separate out definition assignment

[Michal Privoznik]

On 8/19/19 6:08 PM, John Ferlan wrote:


Was it intentional to swap the order?



That's a good question. Honestly, I don't recall. I'll fix it in my 
local branch. It looks like we should set the new pool definition with 
the pool list locked so that virStoragePoolObjIsDuplicate() and
virStoragePoolObjSourceFindDuplicate() can return expected value if two 
threads were fighting.


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v1 07/11] virStoragePoolObjListAdd: Separate out definition assignment

[John Ferlan]

On 5/24/19 10:35 AM, Michal Privoznik wrote:
> Separate storage pool definition assignment into a function.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/virstorageobj.c | 23 ---
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index 7515b5d107..2b9ad6fc98 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -1505,6 +1505,21 @@ 
> virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools,
>  }
>  
>  
> +static void
> +virStoragePoolObjAssignDef(virStoragePoolObjPtr obj,
> +   virStoragePoolDefPtr def,
> +   unsigned int flgs ATTRIBUTE_UNUSED)
> +{
> +if (!virStoragePoolObjIsActive(obj)) {
> +virStoragePoolDefFree(obj->def);
> +obj->def = def;
> +} else {
> +virStoragePoolDefFree(obj->newDef);
> +obj->newDef = def;
> +}
> +}
> +
> +
>  /**
>   * virStoragePoolObjListAdd:
>   * @pools: Storage Pool object list pointer
> @@ -1540,14 +1555,8 @@ virStoragePoolObjListAdd(virStoragePoolObjListPtr 
> pools,
>  if (rc < 0)
>  goto error;
>  if (rc > 0) {
> -if (!virStoragePoolObjIsActive(obj)) {
> -virStoragePoolDefFree(obj->def);
> -obj->def = def;
> -} else {
> -virStoragePoolDefFree(obj->newDef);
> -obj->newDef = def;
> -}
>  virObjectRWUnlock(pools);
> +virStoragePoolObjAssignDef(obj, def, flags);

Was it intentional to swap the order?

John

>  return obj;
>  }
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v1 07/11] virStoragePoolObjListAdd: Separate out definition assignment

[Ján Tomko]

On Fri, May 24, 2019 at 04:35:43PM +0200, Michal Privoznik wrote:

Separate storage pool definition assignment into a function.

Signed-off-by: Michal Privoznik 
---
src/conf/virstorageobj.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v1 07/11] virStoragePoolObjListAdd: Separate out definition assignment

[Michal Privoznik]

Separate storage pool definition assignment into a function.

Signed-off-by: Michal Privoznik 
---
 src/conf/virstorageobj.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 7515b5d107..2b9ad6fc98 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1505,6 +1505,21 @@ 
virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools,
 }
 
 
+static void
+virStoragePoolObjAssignDef(virStoragePoolObjPtr obj,
+   virStoragePoolDefPtr def,
+   unsigned int flgs ATTRIBUTE_UNUSED)
+{
+if (!virStoragePoolObjIsActive(obj)) {
+virStoragePoolDefFree(obj->def);
+obj->def = def;
+} else {
+virStoragePoolDefFree(obj->newDef);
+obj->newDef = def;
+}
+}
+
+
 /**
  * virStoragePoolObjListAdd:
  * @pools: Storage Pool object list pointer
@@ -1540,14 +1555,8 @@ virStoragePoolObjListAdd(virStoragePoolObjListPtr pools,
 if (rc < 0)
 goto error;
 if (rc > 0) {
-if (!virStoragePoolObjIsActive(obj)) {
-virStoragePoolDefFree(obj->def);
-obj->def = def;
-} else {
-virStoragePoolDefFree(obj->newDef);
-obj->newDef = def;
-}
 virObjectRWUnlock(pools);
+virStoragePoolObjAssignDef(obj, def, flags);
 return obj;
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list