[Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary

yuchenlin--- via Qemu-devel posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180913082952.3675-1-yuchenlin@synology.com
Test docker-clang@ubuntu failed
Test checkpatch passed
block/vmdk.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
[Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
Posted by yuchenlin--- via Qemu-devel 7 years, 1 month ago
From: yuchenlin <yuchenlin@synology.com>

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.

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
'syno-vm-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.

Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
v2 -> v3:
* Update commit message

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


Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
Posted by Fam Zheng 7 years, 1 month ago
On Thu, 09/13 16:29, yuchenlin@synology.com wrote:
> From: yuchenlin <yuchenlin@synology.com>
> 
> 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.
> 
> 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
> 'syno-vm-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.
> 
> Signed-off-by: yuchenlin <yuchenlin@synology.com>

Reviewed-by: Fam Zheng <famz@redhat.com>


Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
Posted by yuchenlin via Qemu-devel 7 years, 1 month ago
Ping?

On 2018-09-13 16:34, Fam Zheng wrote:
> On Thu, 09/13 16:29, yuchenlin@synology.com wrote:
>> From: yuchenlin <yuchenlin@synology.com>
>> 
>> 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.
>> 
>> 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
>> 'syno-vm-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.
>> 
>> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>


Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
Posted by Fam Zheng 7 years ago
On Fri, 10/05 10:00, yuchenlin wrote:
> Ping?

Hi,

This was merged as 51b3c6b73acae1e3fd3c7d441fc86dd17356695f.

Fam

> 
> On 2018-09-13 16:34, Fam Zheng wrote:
> > On Thu, 09/13 16:29, yuchenlin@synology.com wrote:
> > > From: yuchenlin <yuchenlin@synology.com>
> > > 
> > > 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.
> > > 
> > > 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
> > > 'syno-vm-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.
> > > 
> > > Signed-off-by: yuchenlin <yuchenlin@synology.com>
> > 
> > Reviewed-by: Fam Zheng <famz@redhat.com>
> 

Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
Posted by yuchenlin via Qemu-devel 7 years ago
On 2018-10-08 10:38, Fam Zheng wrote:
> On Fri, 10/05 10:00, yuchenlin wrote:
>> Ping?
> 
> Hi,
> 
> This was merged as 51b3c6b73acae1e3fd3c7d441fc86dd17356695f.
> 
> Fam
> 

Hi,

Thank you for your information.

yuchenlin

>> 
>> On 2018-09-13 16:34, Fam Zheng wrote:
>> > On Thu, 09/13 16:29, yuchenlin@synology.com wrote:
>> > > From: yuchenlin <yuchenlin@synology.com>
>> > >
>> > > 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.
>> > >
>> > > 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
>> > > 'syno-vm-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.
>> > >
>> > > Signed-off-by: yuchenlin <yuchenlin@synology.com>
>> >
>> > Reviewed-by: Fam Zheng <famz@redhat.com>
>>