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

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

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 redhat com>
> ---
>  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


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

Reviewed-by: John Ferlan <jferlan redhat com>


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