From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51562) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLqol-0005ht-E0 for qemu-devel@nongnu.org; Wed, 10 Oct 2012 03:35:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLqoh-0008Se-56 for qemu-devel@nongnu.org; Wed, 10 Oct 2012 03:34:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13212) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLqog-0008SX-Sr for qemu-devel@nongnu.org; Wed, 10 Oct 2012 03:34:55 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9A7YsQA027376 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 10 Oct 2012 03:34:54 -0400 Message-ID: <5075251A.1010509@redhat.com> Date: Wed, 10 Oct 2012 09:34:50 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <37ca1a150224c144f9329fc8408f78c388f50a02.1349848348.git.jcody@redhat.com> In-Reply-To: <37ca1a150224c144f9329fc8408f78c388f50a02.1349848348.git.jcody@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] block: make bdrv_find_backing_image compare canonical filenames List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, qemu-devel@nongnu.org Il 10/10/2012 07:56, Jeff Cody ha scritto: > Currently, bdrv_find_backing_image compares bs->backing_file with > what is passed in as a backing_file name. Mismatches may occur, > however, when bs->backing_file and backing_file are both not > absolute or relative. > > Use path_combine() to make sure any relative backing filenames are > relative to the current image filename being searched, and then use > realpath() to make all comparisons based on absolute filenames. > > This also changes bdrv_find_backing_image to no longer be recursive, > but iterative. > > Signed-off-by: Jeff Cody > --- > block.c | 38 ++++++++++++++++++++++++++++++++------ > 1 file changed, 32 insertions(+), 6 deletions(-) > > diff --git a/block.c b/block.c > index e95f613..641b8fa 100644 > --- a/block.c > +++ b/block.c > @@ -3123,18 +3123,44 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs, > return -ENOTSUP; > } > > +/* backing_file can either be relative, or absolute. If it is > + * relative, it must be relative to the chain. So, passing in > + * bs->filename from a BDS as backing_file should not be done, > + * as that may be relative to the CWD rather than the chain. */ > BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, > const char *backing_file) > { > - if (!bs->drv) { > + char filename_full[PATH_MAX]; > + char backing_file_full[PATH_MAX]; > + char filename_tmp[PATH_MAX]; > + BlockDriverState *curr_bs = NULL; > + > + if (!bs || !bs->drv) { > return NULL; > } > > - if (bs->backing_hd) { > - if (strcmp(bs->backing_file, backing_file) == 0) { > - return bs->backing_hd; > - } else { > - return bdrv_find_backing_image(bs->backing_hd, backing_file); > + for (curr_bs = bs; curr_bs->backing_hd; curr_bs = curr_bs->backing_hd) { > + /* If not an absolute filename path, make it relative to the current > + * image's filename path */ > + path_combine(filename_tmp, sizeof(filename_tmp), > + curr_bs->filename, backing_file); > + > + /* We are going to compare absolute pathnames */ > + if (!realpath(filename_tmp, filename_full)) { > + continue; > + } > + > + /* We need to make sure the backing filename we are comparing against > + * is relative to the current image filename (or absolute) */ > + path_combine(filename_tmp, sizeof(filename_tmp), > + curr_bs->filename, curr_bs->backing_file); > + > + if (!realpath(filename_tmp, backing_file_full)) { Unfortunately realpath does not exist on Win32. But http://lists.gnu.org/archive/html/grub-devel/2011-09/msg00034.html suggests that there is a _fullpath and in fact already has all you need to write a qemu_realpath function. Paolo > + continue; > + } > + > + if (strcmp(backing_file_full, filename_full) == 0) { > + return curr_bs->backing_hd; > } > } > >