qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PULL 13/17] qcow2: Use bs->bl.request_alignment when updating an L1 entry
Date: Thu,  6 Feb 2020 13:51:28 +0100	[thread overview]
Message-ID: <20200206125132.594625-14-mreitz@redhat.com> (raw)
In-Reply-To: <20200206125132.594625-1-mreitz@redhat.com>

From: Alberto Garcia <berto@igalia.com>

When updating an L1 entry the qcow2 driver writes a (512-byte) sector
worth of data to avoid a read-modify-write cycle. Instead of always
writing 512 bytes we should follow the alignment requirements of the
storage backend.

(the only exception is when the alignment is larger than the cluster
size because then we could be overwriting data after the L1 table)

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0384fb2339..1947f13a2d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -216,26 +216,31 @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
 }
 
 /*
- * Writes one sector of the L1 table to the disk (can't update single entries
- * and we really don't want bdrv_pread to perform a read-modify-write)
+ * Writes an L1 entry to disk (note that depending on the alignment
+ * requirements this function may write more that just one entry in
+ * order to prevent bdrv_pwrite from performing a read-modify-write)
  */
-#define L1_ENTRIES_PER_SECTOR (512 / 8)
 int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
 {
     BDRVQcow2State *s = bs->opaque;
-    uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 };
     int l1_start_index;
     int i, ret;
+    int bufsize = MAX(sizeof(uint64_t),
+                      MIN(bs->file->bs->bl.request_alignment, s->cluster_size));
+    int nentries = bufsize / sizeof(uint64_t);
+    g_autofree uint64_t *buf = g_try_new0(uint64_t, nentries);
 
-    l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1);
-    for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size;
-         i++)
-    {
+    if (buf == NULL) {
+        return -ENOMEM;
+    }
+
+    l1_start_index = QEMU_ALIGN_DOWN(l1_index, nentries);
+    for (i = 0; i < MIN(nentries, s->l1_size - l1_start_index); i++) {
         buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);
     }
 
     ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,
-            s->l1_table_offset + 8 * l1_start_index, sizeof(buf), false);
+            s->l1_table_offset + 8 * l1_start_index, bufsize, false);
     if (ret < 0) {
         return ret;
     }
@@ -243,7 +248,7 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
     BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
     ret = bdrv_pwrite_sync(bs->file,
                            s->l1_table_offset + 8 * l1_start_index,
-                           buf, sizeof(buf));
+                           buf, bufsize);
     if (ret < 0) {
         return ret;
     }
-- 
2.24.1



  parent reply	other threads:[~2020-02-06 13:11 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 12:51 [PULL 00/17] Block patches Max Reitz
2020-02-06 12:51 ` [PULL 01/17] qcow2: Assert that host cluster offsets fit in L2 table entries Max Reitz
2020-02-06 12:51 ` [PULL 02/17] block: Use a GString in bdrv_perm_names() Max Reitz
2020-02-06 12:51 ` [PULL 03/17] block: fix memleaks in bdrv_refresh_filename Max Reitz
2020-02-06 12:51 ` [PULL 04/17] qcow2: Use a GString in report_unsupported_feature() Max Reitz
2020-02-06 12:51 ` [PULL 05/17] iotests: remove 'linux' from default supported platforms Max Reitz
2020-02-06 12:51 ` [PULL 06/17] iotests: Test 041 only works on certain systems Max Reitz
2020-02-06 12:51 ` [PULL 07/17] iotests: Test 183 does not work on macOS and OpenBSD Max Reitz
2020-02-06 12:51 ` [PULL 08/17] iotests: Check for the availability of the required devices in 267 and 127 Max Reitz
2020-02-06 12:51 ` [PULL 09/17] iotests: Skip Python-based tests if QEMU does not support virtio-blk Max Reitz
2020-02-06 12:51 ` [PULL 10/17] iotests: Enable more tests in the 'auto' group to improve test coverage Max Reitz
2020-02-06 12:51 ` [PULL 11/17] qcow2: Don't round the L1 table allocation up to the sector size Max Reitz
2020-02-06 12:51 ` [PULL 12/17] qcow2: Tighten cluster_offset alignment assertions Max Reitz
2020-02-06 12:51 ` Max Reitz [this message]
2020-02-06 12:51 ` [PULL 14/17] qcow2: Don't require aligned offsets in qcow2_co_copy_range_from() Max Reitz
2020-02-06 12:51 ` [PULL 15/17] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value Max Reitz
2020-02-06 12:51 ` [PULL 16/17] block/backup-top: fix failure path Max Reitz
2020-02-06 12:51 ` [PULL 17/17] iotests: add test for backup-top failure on permission activation Max Reitz
2020-02-06 18:58 ` [PULL 00/17] Block patches Peter Maydell

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=20200206125132.594625-14-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).