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

[Max Reitz]

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 <address@hidden>
> > ---
> >   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