Re: [Qemu-devel] [PATCH] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]

On 04/05/2014 10:43 AM, Jun Li wrote:
> This patch fixed the following bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1084302 .
> path_combine can not calculate the correct full path name for backing_file.
> Such as:
> create a snapshot chain:
> sn2->sn1->$BASE_IMG
> backing file is : /home/wookpecker/img.qcow2
> sn1 : /home/woodpecker/tmp/sn1
> sn2 : /home/woodpecker/tmp/sn2
> when create sn2, path_combine can not got a correct path for $BASE_IMG.
> 
> Signed-off-by: Jun Li <junmuzi@gmail.com>
> ---
>  block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 990a754..2519f68 100644
> --- a/block.c
> +++ b/block.c
> @@ -307,7 +307,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)
>      if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
>          pstrcpy(dest, sz, bs->backing_file);
>      } else {
> -        path_combine(dest, sz, bs->filename, bs->backing_file);
> +        realpath(bs->backing_file, dest);

Ouch - I think this does the wrong thing in the presence of symlinks.

VDSM is an existing client that likes to create symlinks to qcow2 files,
where the backing chain uses relative names that work ONLY if you
resolve them in relation to the directory containing the symlink.  As an
example, if I have the following tree:

/path/to/base         - qcow2 file
/path/to/mid          - qcow2 file, backing of '../dir/baselink'
/path/to/top          - qcow2 file, backing of '../dir/midlink'
/path/to/dir/baselink - symlink to '../base'
/path/to/dir/midlink  - symlink to '../mid'
/path/to/dir/toplink  - symlink to '../top'

then changing directories to '/path/to/dir' and opening './toplink'
should correctly find ../dir/midlink and ../dir/../dir/baselink (aka
../mid and ../top) as its backing chain.  But if you use realpath() on
the results, you will get '/path/to/mid' as the backing file of
'toplink' (that is, the symlink is canonicalized), and when you then go
to find the backing file, '/path/to/../dir/baselink' does NOT exist (by
using realpath, you are starting the search for the relative backing
file from the wrong directory).


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]