[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filena
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename |
Date: |
Mon, 12 May 2014 09:15:00 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 05/10/2014 10:35 AM, Jun Li wrote:
> From: Jun Li <address@hidden>
I see three different "[PATCH v3]" mails with this subject. To make
sure we are reviewing the right version, it might help to bump to version 4.
>
> 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.
I'm having a hard time parsing that. We usually represent backing
chains with the base image on the left:
base <- sn1
Can you show the output of 'qemu-img info --backing-chain
/home/woodpecker/tmp/sn2' to make it a bit more obvious what your setup
is? What command are you issuing that triggers a path_combine that is
getting the wrong result?
>
> In this patch, will check the backing_filename is a symlink or not firstly,
> then return the full(absolute) path via realpath.
This feels fishy to me, and liable to do the wrong thing. I need more
context on how to reproduce the issue you are attempting to fix before I
can even decide if your fix is the right approach.
>
> Signed-off-by: Jun Li <address@hidden>
> ---
> block.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index b749d31..6674719 100644
> --- a/block.c
> +++ b/block.c
> @@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
>
> void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t
> sz)
> {
> + struct stat sb;
> + char *linkname;
> +
> 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);
> + if (lstat(bs->backing_file, &sb) == -1) {
> + perror("lstat");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* Check linkname is a link or not */
> + if (S_ISLNK(sb.st_mode)) {
> + linkname = malloc(sb.st_size + 1);
> + readlink(bs->backing_file, linkname, sb.st_size + 1);
> + linkname[sb.st_size] = '\0';
> + realpath(linkname, dest);
> + } else {
> + realpath(bs->backing_file, dest);
> + }
Why are you tweaking just this caller, instead of path_combine() to
affect all other callers?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename, Jun Li, 2014/05/10