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

[Jesse J Cook]

On Wed, 2011-03-09 at 09:23 -0700, Eric Blake wrote:
> 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.

Backing store paths are relative to the directory that owns the original
qcow2 image. This could be a problem if storage pools support
subdirectories in the future because the storage pool path is being used
for the directory path.
 
> 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.

Correct. I should be checking for leading '/'.

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

Correct. I should compute the path prior to checking.

I can make the updates and submit a new patch.

-- 
Jesse Cook
Research Scientist
EADS NA Defense Security & Systems Solutions, Inc. (DS3)
Email: jesse.cook at eads-na-security.com