qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2] vmdk: align end of file to a sector boundary


From: yuchenlin
Subject: Re: [Qemu-block] [PATCH v2] vmdk: align end of file to a sector boundary
Date: Thu, 13 Sep 2018 15:47:21 +0800
User-agent: Roundcube Webmail/1.1.2

On 2018-09-13 10:54, Fam Zheng wrote:
On Thu, 09/13 10:31, address@hidden wrote:
From: yuchenlin <address@hidden>

There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.

The code looks good to me. Can you also explain why it is important to align
file size to sector boundary in the comment?

Fam


In my opinion, there are three reasons to do it. First, if vmdk doesn't align at the sector boundary, there may be many undefined behaviors, such as in vbox it will show VMDK: Compressed image is corrupted '88-disk1.vmdk' (VERR_ZIP_CORRUPTED) when we try to import an ova with unaligned vmdk. Second, all the cluster_sector is aligned to sector, the last one should be like this, too. Third, it ease reading with sector based I/Os.

What do you think?

yuchenlin


Signed-off-by: yuchenlin <address@hidden>
---
v1 -> v2:
* Add more detail comment.
* Add QEMU_ALIGN_UP to show the intention more clearly.
* Add newline in the end of bdrv_truncate.

thanks

 block/vmdk.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index a9d0084e36..2c9e86d98f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1698,6 +1698,27 @@ static int coroutine_fn
 vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
                            uint64_t bytes, QEMUIOVector *qiov)
 {
+    if (bytes == 0) {
+        /* The caller will write bytes 0 to signal EOF.
+         * When receive it, we align EOF to a sector boundary. */
+        BDRVVmdkState *s = bs->opaque;
+        int i, ret;
+        int64_t length;
+
+        for (i = 0; i < s->num_extents; i++) {
+            length = bdrv_getlength(s->extents[i].file->bs);
+            if (length < 0) {
+                return length;
+            }
+            length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);
+            ret = bdrv_truncate(s->extents[i].file, length,
+                                PREALLOC_MODE_OFF, NULL);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+        return 0;
+    }
     return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
 }

--
2.18.0





reply via email to

[Prev in Thread] Current Thread [Next in Thread]