qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Alberto Garcia <berto@igalia.com>,
	qemu-block@nongnu.org, Derek Su <dereksu@qnap.com>,
	Max Reitz <mreitz@redhat.com>
Subject: [PATCH v9 01/34] qcow2: Make Qcow2AioTask store the full host offset
Date: Sun, 28 Jun 2020 13:02:10 +0200	[thread overview]
Message-ID: <02c9a7133a220ab2279a8cb360bf3488e444be1e.1593342067.git.berto@igalia.com> (raw)
In-Reply-To: <cover.1593342067.git.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>
---
 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 e20590c3b7..d792137af6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -74,7 +74,7 @@ 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,
@@ -2103,7 +2103,7 @@ 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,
@@ -2130,16 +2130,12 @@ 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;
@@ -2157,7 +2153,7 @@ 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;
@@ -2170,7 +2166,7 @@ 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,
@@ -2185,7 +2181,7 @@ 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,
@@ -2194,7 +2190,7 @@ 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) {
@@ -2208,13 +2204,12 @@ 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:
@@ -2230,19 +2225,17 @@ 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:
@@ -2258,7 +2251,7 @@ 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);
 }
 
@@ -2294,11 +2287,20 @@ 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;
@@ -2449,7 +2451,7 @@ 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,
@@ -2458,7 +2460,6 @@ 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) {
@@ -2471,9 +2472,7 @@ 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;
         }
@@ -2497,10 +2496,8 @@ 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;
@@ -2530,7 +2527,7 @@ 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);
 }
@@ -2585,8 +2582,8 @@ 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;
@@ -4569,7 +4566,7 @@ 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,
@@ -4581,8 +4578,8 @@ 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 dbe76a7613..e21b95f3e6 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -77,7 +77,7 @@ 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.20.1



  reply	other threads:[~2020-06-28 11:14 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-28 11:02 [PATCH v9 00/34] Add subcluster allocation to qcow2 Alberto Garcia
2020-06-28 11:02 ` Alberto Garcia [this message]
2020-06-28 11:02 ` [PATCH v9 02/34] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset() Alberto Garcia
2020-06-30 10:19   ` Max Reitz
2020-06-30 10:27     ` Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 03/34] qcow2: Add calculate_l2_meta() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 04/34] qcow2: Split cluster_needs_cow() out of count_cow_clusters() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 05/34] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied() Alberto Garcia
2020-06-30 10:38   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 06/34] qcow2: Add get_l2_entry() and set_l2_entry() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 07/34] qcow2: Document the Extended L2 Entries feature Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 08/34] qcow2: Add dummy has_subclusters() function Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 09/34] qcow2: Add subcluster-related fields to BDRVQcow2State Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 10/34] qcow2: Add offset_to_sc_index() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 11/34] qcow2: Add offset_into_subcluster() and size_to_subclusters() Alberto Garcia
2020-07-01 12:23   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 12/34] qcow2: Add l2_entry_size() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 13/34] qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap() Alberto Garcia
2020-07-01 12:28   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 14/34] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type() Alberto Garcia
2020-07-01 12:52   ` Max Reitz
2020-07-01 16:26     ` Alberto Garcia
2020-07-02  9:57       ` Max Reitz
2020-07-02 22:00         ` Alberto Garcia
2020-07-03  7:17           ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 15/34] qcow2: Add qcow2_get_subcluster_range_type() Alberto Garcia
2020-07-01 13:37   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 16/34] qcow2: Add qcow2_cluster_is_allocated() Alberto Garcia
2020-07-01 13:55   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 17/34] qcow2: Add cluster type parameter to qcow2_get_host_offset() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 18/34] qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_* Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 19/34] qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 20/34] qcow2: Add subcluster support to calculate_l2_meta() Alberto Garcia
2020-07-02 11:30   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 21/34] qcow2: Add subcluster support to qcow2_get_host_offset() Alberto Garcia
2020-07-02 12:46   ` Max Reitz
2020-07-02 22:04     ` Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 22/34] qcow2: Add subcluster support to zero_in_l2_slice() Alberto Garcia
2020-07-02 12:56   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 23/34] qcow2: Add subcluster support to discard_in_l2_slice() Alberto Garcia
2020-07-02 13:24   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 24/34] qcow2: Add subcluster support to check_refcounts_l2() Alberto Garcia
2020-07-02 13:32   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 25/34] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2() Alberto Garcia
2020-07-02 14:01   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 26/34] qcow2: Clear the L2 bitmap when allocating a compressed cluster Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 27/34] qcow2: Add subcluster support to handle_alloc_space() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes() Alberto Garcia
2020-07-02 14:28   ` Max Reitz
2020-07-02 22:40     ` Alberto Garcia
2020-07-03  7:18       ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 29/34] qcow2: Add subcluster support to qcow2_measure() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 30/34] qcow2: Add prealloc field to QCowL2Meta Alberto Garcia
2020-07-02 14:50   ` Max Reitz
2020-07-02 14:58     ` Eric Blake
2020-07-02 15:09       ` Max Reitz
2020-07-02 23:05         ` Alberto Garcia
2020-07-03  7:22           ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 31/34] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit Alberto Garcia
2020-07-02 15:13   ` Max Reitz
2020-07-03 12:43     ` Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 32/34] qcow2: Allow preallocation and backing files if extended_l2 is set Alberto Garcia
2020-07-03  7:45   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 33/34] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters Alberto Garcia
2020-07-03  7:46   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 34/34] iotests: Add tests for qcow2 images with extended L2 entries Alberto Garcia
2020-07-03  9:49   ` Max Reitz
2020-07-03 13:06     ` Alberto Garcia
2020-07-03 13:47       ` Max Reitz
2020-07-03 15:20         ` Alberto Garcia
2020-07-06 13:57       ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=02c9a7133a220ab2279a8cb360bf3488e444be1e.1593342067.git.berto@igalia.com \
    --to=berto@igalia.com \
    --cc=dereksu@qnap.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).