Re: [Qemu-devel] [PATCH 2/5] vmdk: add support for “zeroed‐grain” GTE

[Fam Zheng]

On Fri, 04/19 10:59, Stefan Hajnoczi wrote:
> On Fri, Apr 19, 2013 at 11:48:42AM +0800, Fam Zheng wrote:
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 450a721..5e60940 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -33,10 +33,13 @@
> >  #define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
> >  #define VMDK4_COMPRESSION_DEFLATE 1
> >  #define VMDK4_FLAG_RGD (1 << 1)
> > +/* Zeroed-grain enable bit */
> > +#define VMDK4_FLAG_ZG   (1 << 2)
> 
> Please use a clear name like VMDK4_FLAG_ZERO_GRAIN.
> 
> > @@ -81,6 +84,8 @@ typedef struct VmdkExtent {
> >      bool flat;
> >      bool compressed;
> >      bool has_marker;
> > +    bool has_zero_grain;
> > +    int version;
> 
> uint32_t according to the spec.  Please use fixed-size integer types
> instead of int, long, etc which can change depending on the host
> architecture.

This is an internal structure holding information for extent, used the
same way as BDRVVmdkState, there is no direct correspondence to file
header fields, so I think it should be OK, as `int qcow_version' is also
found in block/qcow2.h.

> 
> > @@ -1181,7 +1193,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
> > sector_num,
> >                                  sector_num << 9, !extent->compressed,
> >                                  &cluster_offset);
> >          if (extent->compressed) {
> > -            if (ret == 0) {
> > +            if (ret == VMDK_OK) {
> 
> Should this be squashed into the previous patch?

Yes.

-- 
Fam