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@xxxxxxxxxxxxxxxxxxxx
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list