When block streaming an image, if a base name is passed in that is a relative name, but not accessible from the top-level snapshot, then the relative name is stored incorrectly in the image file.
For instance, given a snapshot case of: /tmp/a/base.raw /tmp/a/snap1.qcow2 /tmp/b/snap2.qcow2 if they are all chained with relative filenames, like so: base(bak:"") <- snap1(bak:"base.raw") <- snap2(bak:"../a/snap1.qcow2") Then the merged top-layer will point to an inaccessible path for the base file: base(bak:"") <- snap2(bak:"base.raw") This patch checks for a relative path for a basename, and fixes it so that it is stored in the top-layer image relative to the top-layer image: base(bak:"") <- snap2(bak:"../a/base.raw") Signed-off-by: Jeff Cody <jc...@redhat.com> I submitted this as an RFC, because I had made a few assumptions that I would like to get vetted. Assumptions: 1.) bs->backing_hd->filename is always the same file as bs->backing_file 2.) realpath() and dirname() in QEMU behave properly across OS platforms --- block/stream.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 78 insertions(+), 1 deletions(-) diff --git a/block/stream.c b/block/stream.c index 5c939c7..b82555b 100644 --- a/block/stream.c +++ b/block/stream.c @@ -13,6 +13,9 @@ #include "trace.h" #include "block_int.h" +#include <libgen.h> +#include <string.h> +#include <limits.h> enum { /* @@ -163,6 +166,69 @@ static int coroutine_fn is_allocated_base(BlockDriverState *top, return 1; } +/* Fixes the filename for the proposed backing file, so that + * A) if it is relative, it points to the relative path accessible + * from the current bs->filename, OR + * B) returns 'backing' if 'backing' is already an absolute path + * + * Returns NULL if no relative or absolute path can be found. + */ +static char *path_find_relative(char *current, char *backing) +{ + char *src; + char *dest; + char *src_tmp; + char *src_dir; + char *rel_backing = NULL; + char relpath[PATH_MAX] = {0}; + int offset = 0; + + + src = realpath(current, NULL); + if (src == NULL) { + goto exit; + } + + dest = realpath(backing, NULL); + if (dest == NULL) { + free(src); + goto exit; + } + + src_tmp = g_strdup(src); + + if (!strcmp(backing, dest)) { + rel_backing = g_strdup(backing); + goto free_and_exit; + } + + src_dir = dirname(src_tmp); + g_strlcpy(src_tmp, src_dir, strlen(src)); + + for (;;) { + if (!strncmp(src_tmp, dest, strlen(src_tmp))) { + offset = strlen(src_tmp); + offset = strlen(src_tmp) > 1 ? offset+1 : offset; + if (offset < strlen(dest)) { + rel_backing = g_strconcat(relpath, &dest[offset], NULL); + } + break; + } else if (strlen(src_tmp) <= 1) { + break; + } + src_dir = dirname(src_tmp); + g_strlcpy(src_tmp, src_dir, strlen(src)); + g_strlcat(relpath, "../", sizeof(relpath)); + } + +free_and_exit: + g_free(src_tmp); + free(src); + free(dest); +exit: + return rel_backing; +} + static void coroutine_fn stream_run(void *opaque) { StreamBlockJob *s = opaque; @@ -172,6 +238,7 @@ static void coroutine_fn stream_run(void *opaque) int ret = 0; int n; void *buf; + char *base_filename = NULL; s->common.len = bdrv_getlength(bs); if (s->common.len < 0) { @@ -240,9 +307,19 @@ retry: if (sector_num == end && ret == 0) { const char *base_id = NULL; if (base) { - base_id = s->backing_file_id; + /* fix up relative paths, if any */ + if (!path_is_absolute(s->backing_file_id)) { + base_filename = path_find_relative(bs->filename, + s->base->filename); + base_id = base_filename; + } else { + base_id = s->backing_file_id; + } } ret = bdrv_change_backing_file(bs, base_id, NULL); + if (base_filename) { + g_free(base_filename); + } close_unused_images(bs, base, base_id); } -- 1.7.9.rc2.1.g69204