How about using the sync_file_range to sync the metadata?

-----Original Message-----
From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf Of 
Lan, Tianyu
Sent: Saturday, November 19, 2011 10:09 AM
To: Kevin Wolf
Cc: penb...@kernel.org; kvm@vger.kernel.org; asias.he...@gmail.com; 
levinsasha...@gmail.com; prasadjoshi...@gmail.com
Subject: RE: [RFC v2 PATCH] kvm tools, qcow: Add the support for copy-on-write 
clusters

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...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to