Re: [PATCH v2 6/8] virstorageobj: Check for source duplicates from virStoragePoolObjAssignDef

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> Just like a few commits earlier, checking for pool source
> duplicates and unlocking pools list afterwards is a buggy
> pattern. The check must go into virStoragePoolObjAssignDef.

Probably should be noted that this also swaps the order of checking from
pool name/uuid duplication, then pool source duplication to pool source,
then pool name/uuid. In the long run it doesn't matter and this new
order is fine. I assume the current order was chosen because it's
quicker to HashLookup as opposed to HashSearch (although lately making
assumptions hasn't necessarily worked out for me).

Another change is now the duplication checks are held under a write lock
instead of a concurrent read lock.

> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/conf/virstorageobj.c     | 7 ++++---
>  src/conf/virstorageobj.h     | 4 ----
>  src/libvirt_private.syms     | 1 -
>  src/storage/storage_driver.c | 6 ------
>  4 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index dce45ce870..c717176133 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -1486,17 +1486,15 @@ virStoragePoolObjSourceFindDuplicateCb(const void *payload,
>  }
>  
>  
> -int
> +static int
>  virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools,
>                                       virStoragePoolDefPtr def)

Let's take the opportunity to rename this too

    virStoragePoolObjAssignIsDuplicatePoolSource

Return true/false too... All can be done in the same patch.

Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>

John

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux