RE: [RFC v2 PATCH] kvm tools, qcow: Add the support for copy-on-write clusters

["Lan, Tianyu" <tianyu.lan@intel.com>]

Hi Kevin:
	Thanks for your review. The following means that there should be a fsync after updating 
metadata(refcunt block, l1 table and l2 table).

Thanks 
Tianyu Lan

-----Original Message-----
> +		/*write l2 table*/
> +		l2t->dirty = 1;
> +		if (qcow_l2_cache_write(q, l2t) < 0)
>  			goto free_cache;

You need to make sure that the refcount update is written first (e.g.
with fsync), otherwise you risk corruption when the host crashes in the middle.

>  
> -		if (cache_table(q, l2t) < 0) {
> -			if (ftruncate(q->fd, f_sz) < 0)
> -				goto free_cache;
> +		/* Update the l1 talble */
> +		l1t->l1_table[l1t_idx] = cpu_to_be64(l2t_new_offset
> +			| QCOW2_OFLAG_COPIED);
>  
> -			goto free_cache;
> -		}
> +		if (pwrite_in_full(q->fd, l1t->l1_table,
> +			l1t->table_size * sizeof(u64),
> +			header->l1_table_offset) < 0)
> +			goto error;

Likewise, the L1 table write must be ordered against the L2 write.

goto error is using the wrong label.

>  
> -		/* Update the in-core entry */
> -		l1t->l1_table[l1t_idx] = cpu_to_be64(l2t_offset);
> +		/*cache l2 table*/
> +		cache_table(q, l2t);

After so many explicit comments, you can probably guess what's wrong here...