[libvirt] [PATCHv2 10/16] storage: remember relative names in backing chain

[Laine Stump]

On 10/13/2012 06:00 PM, Eric Blake wrote:
> In order to search for a backing file name as literally present
> in a chain, we need to remember if the chain had relative names.
> Also, searching for absolute names is easier if we only have
> to canonicalize once, rather than on every iteration.
>
> * src/util/storage_file.h (_virStorageFileMetadata): Add field.
> * src/util/storage_file.c (virStorageFileGetMetadataFromBuf):
> (virStorageFileFreeMetadata): Manage it
> (absolutePathFromBaseFile): Store absolute names in canonical form.
> ---
>  src/util/storage_file.c | 28 ++++++++++++++++++++--------
>  src/util/storage_file.h |  3 ++-
>  2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/src/util/storage_file.c b/src/util/storage_file.c

So I just noticed that this file is in util rather than store. I guess
because its functions are called both by the storage driver and directly
from qemu, right?


> index 5387f1e..6f299c0 100644
> --- a/src/util/storage_file.c
> +++ b/src/util/storage_file.c
> @@ -28,6 +28,7 @@
>  #include <sys/stat.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <stdlib.h>
>  #ifdef __linux__
>  # if HAVE_LINUX_MAGIC_H
>  #  include <linux/magic.h>
> @@ -531,18 +532,22 @@ static char *
>  absolutePathFromBaseFile(const char *base_file, const char *path)
>  {
>      char *res;
> +    char *tmp;
>      size_t d_len = dir_len (base_file);
>
>      /* If path is already absolute, or if dirname(base_file) is ".",
>         just return a copy of path.  */
>      if (*path == '/' || d_len == 0)
> -        return strdup(path);
> +        return canonicalize_file_name(path);
>
>      /* Ensure that the following cast-to-int is valid.  */
>      if (d_len > INT_MAX)
>          return NULL;
>
> -    ignore_value(virAsprintf(&res, "%.*s/%s", (int) d_len, base_file, path));
> +    if (virAsprintf(&tmp, "%.*s/%s", (int) d_len, base_file, path) < 0)
> +        return NULL;
> +    res = canonicalize_file_name(tmp);
> +    VIR_FREE(tmp);
>      return res;
>  }
>
> @@ -698,17 +703,23 @@ virStorageFileGetMetadataFromBuf(int format,
>
>          meta->backingStoreIsFile = false;
>          if (backing != NULL) {
> +            meta->backingStore = strdup(backing);
> +            if (meta->backingStore == NULL) {
> +                virReportOOMError();
> +                VIR_FREE(backing);
> +                return -1;
> +            }
>              if (virBackingStoreIsFile(backing)) {
>                  meta->backingStoreIsFile = true;
> +                meta->backingStoreRaw = meta->backingStore;
>                  meta->backingStore = absolutePathFromBaseFile(path, backing);
> -            } else {
> -                meta->backingStore = strdup(backing);
> +                if (meta->backingStore == NULL) {
> +                    virReportOOMError();
> +                    VIR_FREE(backing);
> +                    return -1;
> +                }
>              }
>              VIR_FREE(backing);
> -            if (meta->backingStore == NULL) {
> -                virReportOOMError();
> -                return -1;
> -            }
>              meta->backingStoreFormat = backingFormat;
>          } else {
>              meta->backingStore = NULL;
> @@ -1012,6 +1023,7 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta)
>
>      virStorageFileFreeMetadata(meta->backingMeta);
>      VIR_FREE(meta->backingStore);
> +    VIR_FREE(meta->backingStoreRaw);
>      VIR_FREE(meta);
>  }
>
> diff --git a/src/util/storage_file.h b/src/util/storage_file.h
> index 9a60451..685fcb8 100644
> --- a/src/util/storage_file.h
> +++ b/src/util/storage_file.h
> @@ -53,7 +53,8 @@ VIR_ENUM_DECL(virStorageFileFormat);
>  typedef struct _virStorageFileMetadata virStorageFileMetadata;
>  typedef virStorageFileMetadata *virStorageFileMetadataPtr;
>  struct _virStorageFileMetadata {
> -    char *backingStore;
> +    char *backingStore; /* Canonical name (absolute file, or protocol) */
> +    char *backingStoreRaw; /* If file, original name, possibly relative */
>      int backingStoreFormat; /* enum virStorageFileFormat */
>      bool backingStoreIsFile;
>      virStorageFileMetadataPtr backingMeta;

ACK.