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