From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38446) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cAH3W-0004vl-Ke for qemu-devel@nongnu.org; Fri, 25 Nov 2016 09:00:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cAH3Q-0001KD-Ti for qemu-devel@nongnu.org; Fri, 25 Nov 2016 09:00:46 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:45226 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cAH3Q-0001Je-Ii for qemu-devel@nongnu.org; Fri, 25 Nov 2016 09:00:40 -0500 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uAPDx0Ot121682 for ; Fri, 25 Nov 2016 09:00:39 -0500 Received: from e32.co.us.ibm.com (e32.co.us.ibm.com [32.97.110.150]) by mx0b-001b2d01.pphosted.com with ESMTP id 26xj5gj2m3-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 25 Nov 2016 09:00:39 -0500 Received: from localhost by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 25 Nov 2016 07:00:38 -0700 References: <20161125100629.88589-1-haoqf@linux.vnet.ibm.com> <20161125100629.88589-2-haoqf@linux.vnet.ibm.com> <20161125102121.GB4584@noname.redhat.com> <36ff325c-19f7-80d4-bd5f-d8bf4b1b364e@linux.vnet.ibm.com> <20161125120507.GC4584@noname.redhat.com> From: Hao QingFeng Date: Fri, 25 Nov 2016 22:00:32 +0800 MIME-Version: 1.0 In-Reply-To: <20161125120507.GC4584@noname.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Message-Id: <47ea3d7c-883d-e642-6c24-b3e040122fae@linux.vnet.ibm.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-2.8 v1 1/1] block/vmdk: Fix the endian problem of buf_len List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, cornelia.huck@de.ibm.com, borntraeger@de.ibm.com, famz@redhat.com, qemu-stable@nongnu.org =E5=9C=A8 2016-11-25 20:05, Kevin Wolf =E5=86=99=E9=81=93: > Am 25.11.2016 um 11:48 hat Hao QingFeng geschrieben: >> =E5=9C=A8 2016-11-25 18:21, Kevin Wolf =E5=86=99=E9=81=93: >>> [ 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 i= t >>>> 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_exte= nt >>>> 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 >>>> Signed-off-by: Jing Liu >>> 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 > signoffs: > > Cc: qemu-stable@nongnu.org > > 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 *exten= t, int64_t cluster_offset, >>>> } >>>> data->lba =3D offset >> BDRV_SECTOR_BITS; >>>> - data->size =3D buf_len; >>>> + data->size =3D 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 s= o > 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=20 other stuff meanwhile. > Kevin > --=20 QingFeng Hao(Robin)