Am 26.08.2013 um 15:04 hat Max Reitz geschrieben:
> The pre-write overlap check function is now called before most of the
> qcow2 writes (aborting it on collision or other error).
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/qcow2-cache.c | 17 +++++++++++++++++
> block/qcow2-cluster.c | 23 +++++++++++++++++++++++
> block/qcow2-snapshot.c | 24 ++++++++++++++++++++++++
> block/qcow2.c | 38 +++++++++++++++++++++++++++++++++++++-
> 4 files changed, 101 insertions(+), 1 deletion(-)
> @@ -368,6 +384,13 @@ static int coroutine_fn copy_sectors(BlockDriverState
> *bs,
> &s->aes_encrypt_key);
> }
>
> + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
> + ((cluster_offset >> 9) + n_start) << 9, n * BDRV_SECTOR_SIZE);
Looks a bit overcomplicated, I'd like something like this better:
cluster_offset + n_start * BDRV_SECTOR_SIZE
> + if (ret) {
> + ret = (ret < 0) ? ret : -EIO;
I wonder whether the -EIO logic should be moved into
qcow2_pre_write_overlap_check(). Currently each single caller seems to
have this check.
> + goto out;
> + }
> +
> BLKDBG_EVENT(bs->file, BLKDBG_COW_WRITE);
> ret = bdrv_co_writev(bs->file, (cluster_offset >> 9) + n_start, n,
> &qiov);
> if (ret < 0) {
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 0caac90..6f69ecc 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -189,6 +189,15 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
> return ret;
> }
>
> + /* The snapshot list position has not yet been updated, so these clusters
> + * must indeed be completely free */
> + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
> + offset, s->nb_snapshots * sizeof(h));
> + if (ret) {
> + return (ret < 0) ? ret : -EIO;
> + }
This doesn't check the full size. snapshots_size should have the right
value.
> +
> +
> /* Write all snapshots to the new list */
> for(i = 0; i < s->nb_snapshots; i++) {
> sn = s->snapshots + i;
> @@ -363,6 +372,13 @@ int qcow2_snapshot_create(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info)
> l1_table[i] = cpu_to_be64(s->l1_table[i]);
> }
>
> + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
> + sn->l1_table_offset, s->l1_size * sizeof(uint64_t));
> + if (ret) {
> + ret = (ret < 0) ? ret : -EIO;
> + goto fail;
> + }
> +
> ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
> s->l1_size * sizeof(uint64_t));
> if (ret < 0) {
> @@ -475,6 +491,14 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char
> *snapshot_id)
> goto fail;
> }
>
> + ret = qcow2_pre_write_overlap_check(bs,
> + QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1,
> + s->l1_table_offset, cur_l1_bytes);
> + if (ret) {
> + ret = (ret < 0) ? ret : -EIO;
> + goto fail;
> + }
> +
> ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset, sn_l1_table,
> cur_l1_bytes);
> if (ret < 0) {
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 1d0d7ca..95497c6 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -621,6 +621,8 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags)
> qcow2_free_snapshots(bs);
> qcow2_refcount_close(bs);
> g_free(s->l1_table);
> + /* else pre-write overlap checks in cache_destroy may crash */
> + s->l1_table = NULL;
> if (s->l2_table_cache) {
> qcow2_cache_destroy(bs, s->l2_table_cache);
> }
> @@ -920,6 +922,14 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState
> *bs,
> cur_nr_sectors * 512);
> }
>
> + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
> + ((cluster_offset >> 9) + index_in_cluster) << 9,
Same thing as above.
> + cur_nr_sectors << 9);
> + if (ret) {
> + ret = (ret < 0) ? ret : -EIO;
> + goto fail;
> + }
> +
> qemu_co_mutex_unlock(&s->lock);
> BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
> trace_qcow2_writev_data(qemu_coroutine_self(),
Kevin
