Am 27.08.2013 13:32, schrieb Kevin Wolf:
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
Yes, but this wouldn't correspond with the write call if (cluster_offset
& ((1 << 9) - 1)) != 0. ;-)
Basically, I just wanted it to match exactly the write command.
+ 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.
Seems reasonable. I didn't want to prevent the caller from receiving
information about the exact overlap, but that could be achieved through
an optional result pointer as well, I think.
+ 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.
Yes, you're right.
Max
