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

[Kevin Wolf <kwolf@redhat.com>]

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?

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

Kevin