Re: [PATCH 4/7] storage: Modify stateInitialize to support storage state XML

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

 




On 03/24/2015 06:06 AM, Erik Skultety wrote:
> Storage state driver directories initialization needs to be modified
> to become more generic.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
> ---
>  src/storage/storage_driver.c | 52 ++++++++++++++++++++++++--------------------
>  1 file changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index e088ffa..9bd93d2 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c

Could we fix the comment above this too!

s/QEmu daemon/Storage Driver/

> @@ -154,54 +154,60 @@ storageStateInitialize(bool privileged,
>                         virStateInhibitCallback callback ATTRIBUTE_UNUSED,
>                         void *opaque ATTRIBUTE_UNUSED)
>  {
> -    char *base = NULL;
> +    int ret = -1;
> +    char *configdir = NULL;
> +    char *rundir = NULL;
>  
>      if (VIR_ALLOC(driver) < 0)
> -        return -1;
> +        return ret;
>  
>      if (virMutexInit(&driver->lock) < 0) {
>          VIR_FREE(driver);
> -        return -1;
> +        return ret;
>      }
>      storageDriverLock();
>  
>      if (privileged) {
> -        if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0)
> +        if (VIR_STRDUP(driver->configDir,
> +                       SYSCONFDIR "/libvirt/storage") < 0 ||
> +            VIR_STRDUP(driver->autostartDir,
> +                       SYSCONFDIR "/libvirt/storage/autostart") < 0 ||
> +            VIR_STRDUP(driver->stateDir,
> +                       LOCALSTATEDIR "/run/libvirt/storage") < 0)
>              goto error;
>      } else {
> -        base = virGetUserConfigDirectory();
> -        if (!base)
> +        configdir = virGetUserConfigDirectory();
> +        rundir = virGetUserRuntimeDirectory();
> +        if (!(configdir && rundir))
> +            goto error;
> +
> +        if ((virAsprintf(&driver->configDir,
> +                        "%s/storage", configdir) < 0) ||
> +            (virAsprintf(&driver->autostartDir,
> +                        "%s/storage", configdir) < 0) ||
> +            (virAsprintf(&driver->stateDir,
> +                         "%s/storage/run", rundir) < 0))
>              goto error;
>      }
>      driver->privileged = privileged;
>  
> -    /*
> -     * Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/...
> -     * (session) or /etc/libvirt/storage/... (system).
> -     */
> -    if (virAsprintf(&driver->configDir,
> -                    "%s/storage", base) == -1)
> -        goto error;
> -
> -    if (virAsprintf(&driver->autostartDir,
> -                    "%s/storage/autostart", base) == -1)
> -        goto error;
> -
> -    VIR_FREE(base);
> -

Braver than I was - although I prefer your method because it's clearer.

I think if this is merged into patch 2 - it'll be easier to follow

John

>      if (virStoragePoolLoadAllConfigs(&driver->pools,
>                                       driver->configDir,
>                                       driver->autostartDir) < 0)
>          goto error;
>  
>      storageDriverUnlock();
> -    return 0;
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(configdir);
> +    VIR_FREE(rundir);
> +    return ret;
>  
>   error:
> -    VIR_FREE(base);
>      storageDriverUnlock();
>      storageStateCleanup();
> -    return -1;
> +    goto cleanup;
>  }
>  
>  /**
> 

--
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]