Re: [Qemu-devel] [PATCH 1/2] vmdk: Fix error for JSON descriptor file names

[Max Reitz <mreitz@redhat.com>]

On 2014-12-03 at 13:44, Kevin Wolf wrote:
> Am 03.12.2014 um 10:23 hat Max Reitz geschrieben:
>> If vmdk blindly tries to use path_combine() using bs->file->filename as
>> the base file name, this will result in a bad error message for JSON
>> file names when calling bdrv_open(). It is better to only try
>> bs->file->exact_filename; if that is empty, bs->file->filename will be
>> useless for path_combine() and an error should be emitted (containing
>> bs->file->filename because bs->file->exact_filename is empty).
>>
>> Note that s->create_type does not need to be freed on error because it
>> will be freed by the caller (which ultimately is vmdk_open()).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/vmdk.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 2cbfd3e..fe549c2 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -894,7 +894,14 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
>>       }
>>       s->create_type = g_strdup(ct);
>>       s->desc_offset = 0;
>> -    ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp);
>> +
>> +    if (!bs->file->exact_filename[0]) {
>> +        error_setg(errp, "Cannot use extents with VMDK descriptor file '%s'",
>> +                   bs->file->filename);
>> +        ret = -EINVAL;
>> +        goto exit;
>> +    }
> Isn't this overly restrictive? If the extent paths are all absolute,
> there is no reason not to open them. Or does the VMDK spec say that they
> are always relative?

Yes, you're right. Will respin.

Max

>> +    ret = vmdk_parse_extents(buf, bs, bs->file->exact_filename, errp);
>>   exit:
>>       return ret;
>>   }
> Kevin