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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
