From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, Jeff Cody <codyprime@gmail.com>,
qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PATCH 8/9] vhdx: Rework truncation logic
Date: Tue, 28 Apr 2020 15:29:04 -0500 [thread overview]
Message-ID: <20200428202905.770727-9-eblake@redhat.com> (raw)
In-Reply-To: <20200428202905.770727-1-eblake@redhat.com>
The vhdx driver uses truncation for image growth, with a special case
for blocks that already read as zero but which are only being
partially written. But with a bit of rearranging, it's just as easy
to defer the decision on whether truncation resulted in zeroes to the
actual allocation attempt, reducing the number of places that still
use bdrv_has_zero_init_truncate.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/vhdx.c | 89 ++++++++++++++++++++++++++++++----------------------
1 file changed, 51 insertions(+), 38 deletions(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 21497f731878..fe544abaf52a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1241,12 +1241,16 @@ exit:
/*
* Allocate a new payload block at the end of the file.
*
- * Allocation will happen at 1MB alignment inside the file
+ * Allocation will happen at 1MB alignment inside the file.
+ *
+ * If @need_zero is set on entry but not cleared on return, then truncation
+ * could not guarantee that the new portion reads as zero, and the caller
+ * will take care of it instead.
*
* Returns the file offset start of the new payload block
*/
static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
- uint64_t *new_offset)
+ uint64_t *new_offset, bool *need_zero)
{
int64_t current_len;
@@ -1263,6 +1267,17 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
return -EINVAL;
}
+ if (*need_zero) {
+ int ret;
+
+ ret = bdrv_truncate(bs->file, *new_offset + s->block_size, false,
+ PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE, NULL);
+ if (ret != -ENOTSUP) {
+ *need_zero = false;
+ return ret;
+ }
+ }
+
return bdrv_truncate(bs->file, *new_offset + s->block_size, false,
PREALLOC_MODE_OFF, 0, NULL);
}
@@ -1356,18 +1371,38 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
/* in this case, we need to preserve zero writes for
* data that is not part of this write, so we must pad
* the rest of the buffer to zeroes */
-
- /* if we are on a posix system with ftruncate() that extends
- * a file, then it is zero-filled for us. On Win32, the raw
- * layer uses SetFilePointer and SetFileEnd, which does not
- * zero fill AFAIK */
-
- /* Queue another write of zero buffers if the underlying file
- * does not zero-fill on file extension */
-
- if (bdrv_has_zero_init_truncate(bs->file->bs) == 0) {
- use_zero_buffers = true;
-
+ use_zero_buffers = true;
+ /* fall through */
+ case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
+ case PAYLOAD_BLOCK_UNMAPPED:
+ case PAYLOAD_BLOCK_UNMAPPED_v095:
+ case PAYLOAD_BLOCK_UNDEFINED:
+ bat_prior_offset = sinfo.file_offset;
+ ret = vhdx_allocate_block(bs, s, &sinfo.file_offset,
+ &use_zero_buffers);
+ if (ret < 0) {
+ goto exit;
+ }
+ /*
+ * once we support differencing files, this may also be
+ * partially present
+ */
+ /* update block state to the newly specified state */
+ vhdx_update_bat_table_entry(bs, s, &sinfo, &bat_entry,
+ &bat_entry_offset,
+ PAYLOAD_BLOCK_FULLY_PRESENT);
+ bat_update = true;
+ /*
+ * Since we just allocated a block, file_offset is the
+ * beginning of the payload block. It needs to be the
+ * write address, which includes the offset into the
+ * block, unless the entire block needs to read as
+ * zeroes but truncation was not able to provide them,
+ * in which case we need to fill in the rest.
+ */
+ if (!use_zero_buffers) {
+ sinfo.file_offset += sinfo.block_offset;
+ } else {
/* zero fill the front, if any */
if (sinfo.block_offset) {
iov1.iov_len = sinfo.block_offset;
@@ -1379,7 +1414,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
}
/* our actual data */
- qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
+ qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
sinfo.bytes_avail);
/* zero fill the back, if any */
@@ -1394,29 +1429,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
sectors_to_write += iov2.iov_len >> BDRV_SECTOR_BITS;
}
}
- /* fall through */
- case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
- case PAYLOAD_BLOCK_UNMAPPED:
- case PAYLOAD_BLOCK_UNMAPPED_v095:
- case PAYLOAD_BLOCK_UNDEFINED:
- bat_prior_offset = sinfo.file_offset;
- ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
- if (ret < 0) {
- goto exit;
- }
- /* once we support differencing files, this may also be
- * partially present */
- /* update block state to the newly specified state */
- vhdx_update_bat_table_entry(bs, s, &sinfo, &bat_entry,
- &bat_entry_offset,
- PAYLOAD_BLOCK_FULLY_PRESENT);
- bat_update = true;
- /* since we just allocated a block, file_offset is the
- * beginning of the payload block. It needs to be the
- * write address, which includes the offset into the block */
- if (!use_zero_buffers) {
- sinfo.file_offset += sinfo.block_offset;
- }
+
/* fall through */
case PAYLOAD_BLOCK_FULLY_PRESENT:
/* if the file offset address is in the header zone,
--
2.26.2
next prev parent reply other threads:[~2020-04-28 20:34 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 20:28 [PATCH 0/9] More truncate improvements Eric Blake
2020-04-28 20:28 ` [PATCH 1/9] gluster: Drop useless has_zero_init callback Eric Blake
2020-04-28 20:28 ` [PATCH 2/9] file-win32: Support BDRV_REQ_ZERO_WRITE for truncate Eric Blake
2020-04-28 20:28 ` [PATCH 3/9] nfs: " Eric Blake
2020-04-28 20:29 ` [PATCH 4/9] rbd: " Eric Blake
2020-04-28 20:29 ` [PATCH 5/9] sheepdog: " Eric Blake
2020-04-28 20:29 ` [PATCH 6/9] ssh: " Eric Blake
2020-04-29 9:01 ` Richard W.M. Jones
2020-04-28 20:29 ` [PATCH 7/9] parallels: Rework truncation logic Eric Blake
2020-05-07 11:14 ` Denis V. Lunev
2020-04-28 20:29 ` Eric Blake [this message]
2020-04-28 20:29 ` [PATCH 9/9] block: Drop unused .bdrv_has_zero_init_truncate Eric Blake
2020-04-29 9:02 ` Richard W.M. Jones
2020-04-29 2:24 ` [PATCH 0/9] More truncate improvements no-reply
2020-04-29 13:14 ` Eric Blake
2020-04-29 2:24 ` no-reply
2020-05-06 13:03 ` Eric Blake
2020-05-07 10:15 ` Kevin Wolf
2020-05-07 14:29 ` Eric Blake
2020-05-07 14:32 ` Kevin Wolf
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=20200428202905.770727-9-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=codyprime@gmail.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--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).