From: Alberto Garcia <berto@igalia.com>

The file_cluster_offset field of Qcow2AioTask stores a cluster-aligned

host offset. In practice this is not very useful because all users(*)

of this structure need the final host offset into the cluster, which

they calculate using

host_offset = file_cluster_offset + offset_into_cluster(s, offset)

There is no reason why Qcow2AioTask cannot store host_offset directly

and that is what this patch does.

(*) compressed clusters are the exception: in this case what

file_cluster_offset was storing was the full compressed cluster

descriptor (offset + size). This does not change with this patch

but it is documented now.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-Id: <07c4b15c644dcf06c9459f98846ac1c4ea96e26f.1594396418.git.berto@igalia.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2.c | 69 ++++++++++++++++++++++------------------------

block/trace-events | 2 +-

2 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ typedef struct {

static int coroutine_fn

qcow2_co_preadv_compressed(BlockDriverState *bs,

- uint64_t file_cluster_offset,

+ uint64_t cluster_descriptor,

uint64_t offset,

uint64_t bytes,

QEMUIOVector *qiov,

@@ -XXX,XX +XXX,XX @@ out:

static coroutine_fn int

qcow2_co_preadv_encrypted(BlockDriverState *bs,

- uint64_t file_cluster_offset,

+ uint64_t host_offset,

uint64_t offset,

uint64_t bytes,

QEMUIOVector *qiov,

@@ -XXX,XX +XXX,XX @@ qcow2_co_preadv_encrypted(BlockDriverState *bs,

}

BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);

- ret = bdrv_co_pread(s->data_file,

- file_cluster_offset + offset_into_cluster(s, offset),

- bytes, buf, 0);

+ ret = bdrv_co_pread(s->data_file, host_offset, bytes, buf, 0);

if (ret < 0) {

goto fail;

}

- if (qcow2_co_decrypt(bs,

- file_cluster_offset + offset_into_cluster(s, offset),

- offset, buf, bytes) < 0)

+ if (qcow2_co_decrypt(bs, host_offset, offset, buf, bytes) < 0)

{

ret = -EIO;

goto fail;

@@ -XXX,XX +XXX,XX @@ typedef struct Qcow2AioTask {

BlockDriverState *bs;

QCow2ClusterType cluster_type; /* only for read */

- uint64_t file_cluster_offset;

+ uint64_t host_offset; /* or full descriptor in compressed clusters */

uint64_t offset;

uint64_t bytes;

QEMUIOVector *qiov;

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_add_task(BlockDriverState *bs,

AioTaskPool *pool,

AioTaskFunc func,

QCow2ClusterType cluster_type,

- uint64_t file_cluster_offset,

+ uint64_t host_offset,

uint64_t offset,

uint64_t bytes,

QEMUIOVector *qiov,

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_add_task(BlockDriverState *bs,

.bs = bs,

.cluster_type = cluster_type,

.qiov = qiov,

- .file_cluster_offset = file_cluster_offset,

+ .host_offset = host_offset,

.offset = offset,

.bytes = bytes,

.qiov_offset = qiov_offset,

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_add_task(BlockDriverState *bs,

trace_qcow2_add_task(qemu_coroutine_self(), bs, pool,

func == qcow2_co_preadv_task_entry ? "read" : "write",

- cluster_type, file_cluster_offset, offset, bytes,

+ cluster_type, host_offset, offset, bytes,

qiov, qiov_offset);

if (!pool) {

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_add_task(BlockDriverState *bs,

static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,

QCow2ClusterType cluster_type,

- uint64_t file_cluster_offset,

+ uint64_t host_offset,

uint64_t offset, uint64_t bytes,

QEMUIOVector *qiov,

size_t qiov_offset)

{

BDRVQcow2State *s = bs->opaque;

- int offset_in_cluster = offset_into_cluster(s, offset);

switch (cluster_type) {

case QCOW2_CLUSTER_ZERO_PLAIN:

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,

qiov, qiov_offset, 0);

case QCOW2_CLUSTER_COMPRESSED:

- return qcow2_co_preadv_compressed(bs, file_cluster_offset,

+ return qcow2_co_preadv_compressed(bs, host_offset,

offset, bytes, qiov, qiov_offset);

case QCOW2_CLUSTER_NORMAL:

- assert(offset_into_cluster(s, file_cluster_offset) == 0);

if (bs->encrypted) {

- return qcow2_co_preadv_encrypted(bs, file_cluster_offset,

+ return qcow2_co_preadv_encrypted(bs, host_offset,

offset, bytes, qiov, qiov_offset);

}

BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);

- return bdrv_co_preadv_part(s->data_file,

- file_cluster_offset + offset_in_cluster,

+ return bdrv_co_preadv_part(s->data_file, host_offset,

bytes, qiov, qiov_offset, 0);

default:

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)

assert(!t->l2meta);

- return qcow2_co_preadv_task(t->bs, t->cluster_type, t->file_cluster_offset,

+ return qcow2_co_preadv_task(t->bs, t->cluster_type, t->host_offset,

t->offset, t->bytes, t->qiov, t->qiov_offset);

}

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,

{

qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);

} else {

+ /*

+ * For compressed clusters the variable cluster_offset

+ * does not actually store the offset but the full

+ * descriptor. We need to leave it unchanged because

+ * that's what qcow2_co_preadv_compressed() expects.

+ */

+ uint64_t host_offset = (ret == QCOW2_CLUSTER_COMPRESSED) ?

+ cluster_offset :

+ cluster_offset + offset_into_cluster(s, offset);

if (!aio && cur_bytes != bytes) {

aio = aio_task_pool_new(QCOW2_MAX_WORKERS);

}

ret = qcow2_add_task(bs, aio, qcow2_co_preadv_task_entry, ret,

- cluster_offset, offset, cur_bytes,

+ host_offset, offset, cur_bytes,

qiov, qiov_offset, NULL);

if (ret < 0) {

goto out;

@@ -XXX,XX +XXX,XX @@ static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)

* not use it somehow after qcow2_co_pwritev_task() call

*/

static coroutine_fn int qcow2_co_pwritev_task(BlockDriverState *bs,

- uint64_t file_cluster_offset,

+ uint64_t host_offset,

uint64_t offset, uint64_t bytes,

QEMUIOVector *qiov,

uint64_t qiov_offset,

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_task(BlockDriverState *bs,

int ret;

BDRVQcow2State *s = bs->opaque;

void *crypt_buf = NULL;

- int offset_in_cluster = offset_into_cluster(s, offset);

QEMUIOVector encrypted_qiov;

if (bs->encrypted) {

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_task(BlockDriverState *bs,

}

qemu_iovec_to_buf(qiov, qiov_offset, crypt_buf, bytes);

- if (qcow2_co_encrypt(bs, file_cluster_offset + offset_in_cluster,

- offset, crypt_buf, bytes) < 0)

- {

+ if (qcow2_co_encrypt(bs, host_offset, offset, crypt_buf, bytes) < 0) {

ret = -EIO;

goto out_unlocked;

}

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_task(BlockDriverState *bs,

*/

if (!merge_cow(offset, bytes, qiov, qiov_offset, l2meta)) {

BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);

- trace_qcow2_writev_data(qemu_coroutine_self(),

- file_cluster_offset + offset_in_cluster);

- ret = bdrv_co_pwritev_part(s->data_file,

- file_cluster_offset + offset_in_cluster,

+ trace_qcow2_writev_data(qemu_coroutine_self(), host_offset);

+ ret = bdrv_co_pwritev_part(s->data_file, host_offset,

bytes, qiov, qiov_offset, 0);

if (ret < 0) {

goto out_unlocked;

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_task_entry(AioTask *task)

assert(!t->cluster_type);

- return qcow2_co_pwritev_task(t->bs, t->file_cluster_offset,

+ return qcow2_co_pwritev_task(t->bs, t->host_offset,

t->offset, t->bytes, t->qiov, t->qiov_offset,

t->l2meta);

}

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_part(

aio = aio_task_pool_new(QCOW2_MAX_WORKERS);

}

ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_task_entry, 0,

- cluster_offset, offset, cur_bytes,

- qiov, qiov_offset, l2meta);

+ cluster_offset + offset_in_cluster, offset,

+ cur_bytes, qiov, qiov_offset, l2meta);

l2meta = NULL; /* l2meta is consumed by qcow2_co_pwritev_task() */

if (ret < 0) {

goto fail_nometa;

@@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,

static int coroutine_fn

qcow2_co_preadv_compressed(BlockDriverState *bs,

- uint64_t file_cluster_offset,

+ uint64_t cluster_descriptor,

uint64_t offset,

uint64_t bytes,

QEMUIOVector *qiov,

@@ -XXX,XX +XXX,XX @@ qcow2_co_preadv_compressed(BlockDriverState *bs,

uint8_t *buf, *out_buf;

int offset_in_cluster = offset_into_cluster(s, offset);

- coffset = file_cluster_offset & s->cluster_offset_mask;

- nb_csectors = ((file_cluster_offset >> s->csize_shift) & s->csize_mask) + 1;

+ coffset = cluster_descriptor & s->cluster_offset_mask;

+ nb_csectors = ((cluster_descriptor >> s->csize_shift) & s->csize_mask) + 1;

csize = nb_csectors * QCOW2_COMPRESSED_SECTOR_SIZE -

(coffset & ~QCOW2_COMPRESSED_SECTOR_MASK);

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ luring_io_uring_submit(void *s, int ret) "LuringState %p ret %d"

luring_resubmit_short_read(void *s, void *luringcb, int nread) "LuringState %p luringcb %p nread %d"

# qcow2.c

-qcow2_add_task(void *co, void *bs, void *pool, const char *action, int cluster_type, uint64_t file_cluster_offset, uint64_t offset, uint64_t bytes, void *qiov, size_t qiov_offset) "co %p bs %p pool %p: %s: cluster_type %d file_cluster_offset %" PRIu64 " offset %" PRIu64 " bytes %" PRIu64 " qiov %p qiov_offset %zu"

+qcow2_add_task(void *co, void *bs, void *pool, const char *action, int cluster_type, uint64_t host_offset, uint64_t offset, uint64_t bytes, void *qiov, size_t qiov_offset) "co %p bs %p pool %p: %s: cluster_type %d file_cluster_offset %" PRIu64 " offset %" PRIu64 " bytes %" PRIu64 " qiov %p qiov_offset %zu"

qcow2_writev_start_req(void *co, int64_t offset, int bytes) "co %p offset 0x%" PRIx64 " bytes %d"

qcow2_writev_done_req(void *co, int ret) "co %p ret %d"

qcow2_writev_start_part(void *co) "co %p"

--

2.26.2