[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] block: make bdrv_find_backing_image compare
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] block: make bdrv_find_backing_image compare canonical filenames |
Date: |
Wed, 10 Oct 2012 03:59:18 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
On 10/10/2012 03:34 AM, Paolo Bonzini wrote:
> 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 <address@hidden>
>> ---
>> 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
>
Doesn't block.c already rely on realpath(), in bdrv_open()? Why would
this be any different using it here?
>> + continue;
>> + }
>> +
>> + if (strcmp(backing_file_full, filename_full) == 0) {
>> + return curr_bs->backing_hd;
>> }
>> }
>>
>>
>
Re: [Qemu-devel] [PATCH 1/2] block: make bdrv_find_backing_image compare canonical filenames, Eric Blake, 2012/10/12
[Qemu-devel] [PATCH 2/2] block: in commit, determine base image from the top image, Jeff Cody, 2012/10/10