On Fri, 11/07 16:34, Max Reitz wrote: > On 2014-11-07 at 15:48, Jun Li wrote: > >When bs->filename and bs->backing_file are relative pathname and not under > >the > >same directory, path_combine() can not give the correct path for > >bs->backing_file. So add get_localfile_absolute_path to get absolute path for > >local file. > > Well, for me it is the correct path. I'm using > > $ mkdir -p foo > $ qemu-img create -f qcow2 foo/backing.qcow2 64M > $ qemu-img create -f qcow2 -b backing.qcow2 foo/backed.qcow2 > > I guess this patch will break that? > > I know this isn't ideal, but that's just how it works, so we would break > existing usage. > > Furthermore, compiling with this patch fails for me because on my system > readlink() and realpath() have the attribute warn_unused_result. Also, I'm > not sure, but readlink() may only resolve one link (which may point to > another link in turn). > > And finally, with this patch applied and the return value issue fixed: > > $ mkdir -p foo > $ qemu-img create -f qcow2 foo/backing.qcow2 64M > $ qemu-img create -f qcow2 -b foo/backing.qcow2 foo/backed.qcow2 64M > $ cd foo > $ qemu-img info backed.qcow2 > lstat: No such file or directory > > Because the backing field in the qcow2 file points to "foo/backing.qcow2" > which does not exist. > > If you are giving a relative filename to the "-b" option during qemu-img > create, this filename is not (contrary to intuition) relative to the current > working directory of qemu-img, but it is the value which is put directly > into the backing field of the file to be created; and that value is relative > to that file's directory. > > See also > http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg00632.html and > http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg00633.html >
Hi Max, Do above two patches can resolve this bz https://bugzilla.redhat.com/show_bug.cgi?id=1161582#c2 ? If it can solve this bz, so please ignore my patch. If not, why not using absolute filename to instead relative filename to the "-b" option during qemu-img create? Using absolute filename just need more spaces for backing_file of structure BlockDriverState. BTW, the above two patches are very necessary, I think. Regards, Jun Li > > >e.g: > >$ pwd > >/tmp > >$ /opt/qemu-git-arm/bin/qemu-img create -f qcow2 ./test 5M > >Formatting './test', fmt=qcow2 size=5242880 encryption=off cluster_size=65536 > >lazy_refcounts=off > >$ /opt/qemu-git-arm/bin/qemu-img create -f qcow2 ./tmp/test1 -b ./test > >Formatting './tmp/test1', fmt=qcow2 size=5242880 backing_file='./test' > >encryption=off cluster_size=65536 lazy_refcounts=off > >$ /opt/qemu-git-arm/bin/qemu-img create -f qcow2 ./tmp/test2 -b ./tmp/test1 > >qemu-img: ./tmp/test2: Could not open './tmp/test1': Could not open backing > >file: Could not open './tmp/./test': No such file or directory: No such file > >or directory > > > >This patch also fixes the following bug: > >https://bugzilla.redhat.com/show_bug.cgi?id=1161582#c2 > > > >Signed-off-by: Jun Li <junm...@gmail.com> > >--- > > block.c | 28 +++++++++++++++++++++++++++- > > include/block/block.h | 2 ++ > > 2 files changed, 29 insertions(+), 1 deletion(-) > > > >diff --git a/block.c b/block.c > >index dacd881..5e2f669 100644 > >--- a/block.c > >+++ b/block.c > >@@ -259,6 +259,32 @@ int path_is_absolute(const char *path) > > #endif > > } > >+void get_localfile_absolute_path(char *dest, int dest_size, > >+ const char *filename) > >+{ > >+ struct stat sb; > >+ char *linkname; > >+ > >+ if (path_is_absolute(filename)) { > >+ pstrcpy(dest, dest_size, filename); > >+ } else { > >+ if (lstat(filename, &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(filename, linkname, sb.st_size + 1); > >+ linkname[sb.st_size] = '\0'; > >+ realpath(linkname, dest); > >+ } else { > >+ realpath(filename, dest); > >+ } > >+ } > >+} > >+ > > /* if filename is absolute, just copy it to dest. Otherwise, build a > > path to it by considering it is relative to base_path. URL are > > supported. */ > >@@ -308,7 +334,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); > >+ get_localfile_absolute_path(dest, sz, bs->backing_file); > > } > > } > >diff --git a/include/block/block.h b/include/block/block.h > >index 13e4537..6ddb150 100644 > >--- a/include/block/block.h > >+++ b/include/block/block.h > >@@ -398,6 +398,8 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, > > int bdrv_is_snapshot(BlockDriverState *bs); > > int path_is_absolute(const char *path); > >+void get_localfile_absolute_path(char *dest, int dest_size, > >+ const char *filename); > > void path_combine(char *dest, int dest_size, > > const char *base_path, > > const char *filename); >