Re: [Qemu-devel] [PATCH v2 2/2] vmdk: implment bdrv_get_specific_info

[Fam Zheng]

On Fri, 10/11 05:52, Eric Blake wrote:
> On 10/11/2013 02:31 AM, Fam Zheng wrote:
> > Implement .bdrv_get_specific_info to return the extent information.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> 
> > +
> > +    *spec_info->vmdk = (ImageInfoSpecificVmdk) {
> > +        .create_type = g_strdup(s->create_type),
> > +        .cid = s->cid,
> > +    };
> > +
> 
> >  
> >  ##
> > +# @ImageInfoSpecificVmdk:
> > +#
> > +# @create_type: The create type of VMDK image
> 
> Is it worth making this an enum type rather than an open-coded string?
> But that's not a show-stopper to me.

For now I think a string is good enough, it's only used to display with
qemu-img info, string type saves converting in opening code, as well as
repeating type names in multiple places.

We can convert them in the future if there comes more uses of the types.

Fam

> 
> > +#
> > +# @cid: Content id of image
> > +#
> > +# @parent-cid: Parent VMDK image's cid
> > +#
> > +# @extents: List of extent files
> > +#
> > +# Since: 1.7
> > +##
> > +{ 'type': 'ImageInfoSpecificVmdk',
> > +  'data': {
> > +      'create_type': 'str',
> > +      'cid': 'int',
> > +      'parent-cid': 'int',
> > +      'extents': ['ImageInfo']
> > +  } }
> 
> Both patches look fine from the QMP point of view; I didn't closely
> review the matching C code for accuracy though.
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>