[libvirt] [PATCH 1/5] storage: Create helper to generate FS pool source value

[Michal Privoznik]

On 07.12.2015 21:47, John Ferlan wrote:
> Refactor the code that builds the pool source string during the FS
> storage pool mount to be a separate helper.
> 
> A future patch will use the helper in order to validate the mounted
> FS matches the pool's expectation during poolCheck processing
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_fs.c | 51 +++++++++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 99ea394..ef1a7d0 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -375,6 +375,39 @@ virStorageBackendFileSystemIsValid(virStoragePoolObjPtr pool)
>      return 0;
>  }
>  
> +
> +/**
> + * virStorageBackendFileSystemGetPoolSource
> + * @pool: storage pool object pointer
> + *
> + * Allocate/return a string representing the FS storage pool source.
> + * It is up to the caller to VIR_FREE the allocated string
> + */
> +static char *
> +virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool)
> +{
> +    char *src = NULL;
> +
> +    if (pool->def->type == VIR_STORAGE_POOL_NETFS) {
> +        if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
> +            if (virAsprintf(&src, "//%s/%s",
> +                            pool->def->source.hosts[0].name,
> +                            pool->def->source.dir) == -1)
> +                return NULL;

When touching this, I'd s/== -1/< 0/. Then, these if()-s seem a bit
useless - if virAsprintf() fails, @src is going to be NULL anyway.
Your call if you want to do ignore_value(virAsprintf(...));

> +        } else {
> +            if (virAsprintf(&src, "%s:%s",
> +                            pool->def->source.hosts[0].name,
> +                            pool->def->source.dir) == -1)
> +                return NULL;
> +        }
> +    } else {
> +        if (VIR_STRDUP(src, pool->def->source.devices[0].path) < 0)
> +            return NULL;
> +    }
> +    return src;
> +}
> +
> +
>  /**
>   * @pool storage pool to check for status
>   *
> @@ -445,22 +478,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
>          return -1;
>      }
>  
> -    if (pool->def->type == VIR_STORAGE_POOL_NETFS) {
> -        if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
> -            if (virAsprintf(&src, "//%s/%s",
> -                            pool->def->source.hosts[0].name,
> -                            pool->def->source.dir) == -1)
> -                return -1;
> -        } else {
> -            if (virAsprintf(&src, "%s:%s",
> -                            pool->def->source.hosts[0].name,
> -                            pool->def->source.dir) == -1)
> -                return -1;
> -        }
> -    } else {
> -        if (VIR_STRDUP(src, pool->def->source.devices[0].path) < 0)
> -            return -1;
> -    }
> +    if (!(src = virStorageBackendFileSystemGetPoolSource(pool)))
> +        return -1;
>  
>      if (netauto)
>          cmd = virCommandNewArgList(MOUNT,
> 

ACK

Michal