|
From: | Jun Li |
Subject: | Re: [Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename |
Date: | Sun, 18 May 2014 18:34:15 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 05/12/2014 11:15 PM, Eric Blake
wrote:
Yes, I have send v4 of this patch and changed the commit description.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. Sorry, this is the wrong commit description, please see it on v4. In version 4, I describe it just as followings: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? create a snapshot chain: $BASE_IMG<-sn1<-sn2 backing file is : ./img.qcow2 sn1 : ./tmp/sn1 sn2 : ./tmp/sn2 So, # /opt/qemu-git-arm/bin/qemu-img info --backing-chain ./tmp/sn2 image: ./tmp/sn2 file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 196K cluster_size: 65536 backing file: ./tmp/sn1 (actual path: /home/lijun/Work/tmp/sn1) Format specific information: compat: 1.1 lazy refcounts: false image: /home/lijun/Work/tmp/sn1 file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 196K cluster_size: 65536 backing file: ./img.qcow2 (actual path: /home/lijun/Work/img.qcow2) Format specific information: compat: 1.1 lazy refcounts: false image: /home/lijun/Work/img.qcow2 file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 196K cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: false ------------------ Before this patch, # qemu-img create -f qcow2 -b ./img.qcow2 ./tmp/sn1 # qemu-img create -f qcow2 -b ./tmp/sn1 ./tmp/sn2 qemu-img: Could not open './tmp/sn1': No such file or directory This patch fixed the following bug(Bug 1084302 - Should improve error info when can't find backing file for snapshot):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. https://bugzilla.redhat.com/show_bug.cgi?id=1084302 I will check. Thx.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? Best Regards, Jun Li |
[Prev in Thread] | Current Thread | [Next in Thread] |