[libvirt] [PATCH] Access qemu backing_file with relative pool path

[Eric Blake]

On 03/03/2011 08:06 PM, Jesse Cook wrote:
> This patch enables the relative backing file path support provided by
> qemu-img create.
> 
> If the storage pool is not found with the specified path, check if the
> file exists relative to the pool where the new image will be created by
> prepending the storage pool path.
> ---
>  src/storage/storage_backend.c |   32 ++++++++++++++++++++++++++++----
>  1 files changed, 28 insertions(+), 4 deletions(-)
> 
> @@ -687,10 +687,34 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>              return -1;
>          }
>          if (access(vol->backingStore.path, R_OK) != 0) {
> -            virReportSystemError(errno,
> -                                 _("inaccessible backing store volume %s"),
> -                                 vol->backingStore.path);
> -            return -1;
> +            /* If the backing store image is not found with the specified path,
> +             * check for the file relative to the pool path. */
> +            int accessRetCode = -1;
> +
> +            char *absolutePath = NULL;
> +
> +            virBuffer absPathBuf = VIR_BUFFER_INITIALIZER;
> +
> +            virBufferVSprintf(&absPathBuf,
> +                              "%s/%s",
> +                              pool->def->target.path,
> +                              vol->backingStore.path);

I agree that we need to handle relative paths, but are they relative to
the directory that owns the original qcow2 image, or are they relative
to the cwd of the qemu process (not necessarily the same directory)?  We
need to make sure that we have the same interpretation of relative paths
as qemu.

Meanwhile, shouldn't you only be computing an alternate name if
vol->backingStore.path is relative?  If it's absolute, then you would be
generating /path/to/pool//path/to/backing, which is the wrong file
compared to what qemu would be using.

For that matter, isn't access(vol->backingStore.path, R_OK) wrong for a
relative path, unless you are executing in the same current working
directory as qemu will be when it opens the relative path?

In other words, I think you need to refactor the logic into:

if (relative) compute absolute
if (access fails) return -1

instead of your:

if (access fails) {
  blindly compute absolute
  if (access fails) return -1
}

and only do access() once.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110309/6e4eff3b/attachment-0001.sig>