[Top][All Lists]

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

Re: [Qemu-devel] [PATCH for-2.8 v1 1/1] block/vmdk: Fix the endian probl

From: Hao QingFeng
Subject: Re: [Qemu-devel] [PATCH for-2.8 v1 1/1] block/vmdk: Fix the endian problem of buf_len
Date: Fri, 25 Nov 2016 22:00:32 +0800
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.0

在 2016-11-25 20:05, Kevin Wolf 写道:
Am 25.11.2016 um 11:48 hat Hao QingFeng geschrieben:
在 2016-11-25 18:21, Kevin Wolf 写道:
[ Cc: Fam, qemu-stable ]

Am 25.11.2016 um 11:06 hat QingFeng Hao geschrieben:
The problem was triggered by qemu-iotests case 055. It failed when it
was comparing the compressed vmdk image with original test.img.

The cause is that buf_len in vmdk_write_extent wasn't converted to
little-endian before it was stored to disk. But later vmdk_read_extent
read it and converted it from little-endian to cpu endian.
If the cpu is big-endian like s390, the problem will happen and
the data length read by vmdk_read_extent will become invalid!
The fix is to add the conversion in vmdk_write_extent.

Signed-off-by: QingFeng Hao <address@hidden>
Signed-off-by: Jing Liu <address@hidden>
Sounds like something that should still be fixed for 2.8 and in the
stable branches.
I didn't find the stable branch on upstream, the latest is 2.6 maybe
it's a private one? :-)
I think maybe it's only created once work for the 2.7.1 release begins.

Anyway, this is not your problem, just make sure to CC the qemu-stable
mailing list for bug fixes that affect previous versions as well. It's
also helpful if you put a line like this immediately before the

Cc: address@hidden

This makes sure that the stable release maintainers see the patch,
and everything else will be taken care of by them.
Thanks and I'll add it in the next patch.

diff --git a/block/vmdk.c b/block/vmdk.c
index a11c27a..bf6667f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1355,7 +1355,7 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
          data->lba = offset >> BDRV_SECTOR_BITS;
-        data->size = buf_len;
+        data->size = cpu_to_le32(buf_len);
At least data->lba needs to be fixed, too, both here and in
vmdk_read_extent(). Host endianness in an image file is always wrong.
Good detection!
Are you going to send a v2 that adds this fix?
Yes, I think so.  :-)

Maybe we should audit the whole driver for endianness problems.
Good sight! maybe we can encapsulate a suite of endianness functions
for the structures to avoid missing some elements of them like lba?
Also will be better for reuse. When I am checking the endianness calls
in vmdk_create_extent, I am just afraid of missing one. :-)
Sounds like a good move in general, but let's make sure to keep it
separate from the fixes. The reason is that the fixes can still be
included in the 2.8 release, but code refactoring would only be for 2.9.

You can work on both at the same time, of course, but organise things so
that we have a patch series where all the bug fixes come first (for 2.8)
and only then the type-safe refactoring (for 2.9).
Thanks for your advice! I'll ensure the bug fixes come first and prepare other stuff meanwhile.

QingFeng Hao(Robin)

reply via email to

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