How about using the sync_file_range to sync the metadata? -----Original Message----- From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On Behalf Of Lan, Tianyu Sent: Saturday, November 19, 2011 10:09 AM To: Kevin Wolf Cc: penberg@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; asias.hejun@xxxxxxxxx; levinsasha928@xxxxxxxxx; prasadjoshi124@xxxxxxxxx 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 majordomo@xxxxxxxxxxxxxxx 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
