[libvirt] [PATCH 4/4] storage: don't follow backing chain symlinks too eagerly

[Daniel P. Berrange]

On Wed, Feb 06, 2013 at 10:10:20PM -0700, Eric Blake wrote:
> If you have a qcow2 file /path1/to/file pointed to by symlink
> /path2/symlink, and pass qemu /path2/symlink, then qemu treats
> a relative backing file in the qcow2 metadata as being relative
> to /path2, not /path1/to.  Yes, this means that it is possible
> to create a qcow2 file where the choice of WHICH directory and
> symlink you access its contents from will then determine WHICH
> backing file (if any) you actually find; the results can be
> rather screwy, but we have to match what qemu does.
> 
> Libvirt and qemu default to creating absolute backing file
> names, so most users don't hit this.  But at least VDSM uses
> symlinks and relative backing names alongside the
> --reuse-external flags to libvirt snapshot operations, with the
> result that libvirt was failing to follow the intended chain of
> backing files, and then backing files were not granted the
> necessary sVirt permissions to be opened by qemu.
> 
> See https://bugzilla.redhat.com/show_bug.cgi?id=903248 for
> more gory details.  This fixes a regression introduced in
> commit 8250783.
> 
> I tested this patch by creating the following chain:
> 
> ls /home/eblake/Downloads/Fedora.iso # raw file for base
> cd /var/lib/libvirt/images
> qemu-img create -f qcow2 \
>   -obacking_file=/home/eblake/Downloads/Fedora.iso,backing_fmt=raw one
> mkdir sub
> cd sub
> ln -s ../one onelink
> qemu-img create -f qcow2 \
>   -obacking_file=../sub/onelink,backing_fmt=qcow2 two
> mv two ..
> ln -s ../two twolink
> qemu-img create -f qcow2 \
>   -obacking_file=../sub/twolink,backing_fmt=qcow2 three
> mv three ..
> ln -s ../three threelink
> 
> then pointing my domain at /var/lib/libvirt/images/sub/threelink.
> Prior to this patch, I got complaints about missing backing
> files; afterwards, I was able to verify that the backing chain
> (and hence DAC and SELinux relabels) of the entire chain worked.
> 
> * src/util/virstoragefile.h (_virStorageFileMetadata): Add
> directory member.
> * src/util/virstoragefile.c (absolutePathFromBaseFile): Drop,
> replaced by...
> (virFindBackingFile): ...better function.
> (virStorageFileGetMetadataInternal): Add an argument.
> (virStorageFileGetMetadataFromFD, virStorageFileChainLookup)
> (virStorageFileGetMetadata): Update callers.
> ---
>  src/util/virstoragefile.c | 88 ++++++++++++++++++++++++++++++-----------------
>  src/util/virstoragefile.h |  1 +
>  2 files changed, 57 insertions(+), 32 deletions(-)

ACK only on the basis that you tested it, since I find it hard to follow
the code well enough to verify it does the right thing with relative
symlinks, without actually testing it.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|