Re: [PATCH 1/1] storage: Fix a vol-clone bug for ppc64

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

 



On 11/08/2013 06:25 AM, Li Zhang wrote:
> On 2013年11月07日 18:41, Ján Tomko wrote:
>> On 11/07/2013 09:35 AM, Li Zhang wrote:
>>> Currently, wbytes is defined as size_t type which is 8 bytes on ppc64,
>>> but args's value in ioctl(fd, args..) in kernel is unsigned int.
>>> So it will read more 4 bytes by ioctl() with size_t from memory which
>>> gets a large number and causes out of memory error.
>> On x86_64 size_t is 8 bytes and int is 4 bytes, I guess this is an
>> endianness issue.
> 
> It doesn't look like the endianness issue.
> It appends more 32 0s to the value.
> I need to trace kernel to see how this happens.
> 
> Here is what I get on PPC64.
> blksz = 0x1000000000000, pbsz = 0x20000000000
> 
> Actually, they should be :
> blksz = 0x10000, pbsz = 0x200
> 
> On x86_64, they can get right value.
> 

That looks like endianness to me :)
0x200 on little endian:
4-byte int:    0x00 0x02 0x00 0x00
8-byte size_t: 0x00 0x02 0x00 0x00  0x00 0x00 0x00 0x00

0x200 on big endian:
4-byte int:    0x00 0x00 0x02 0x00
8-byte size_t: 0x00 0x00 0x00 0x00  0x00 0x00 0x02 0x00
8-byte size_t with the int written in the first four bytes:
               0x00 0x00 0x02 0x00  0x00 0x00 0x00 0x00
Which is what you get in pbsz on PPC64.

>>
>>> This patch changes size_t to unsigned int to synchronize with kernel.
>> The type in the kernel seems to be int:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/block/ioctl.c#n363
>>
>> https://lkml.org/lkml/2013/11/1/620
> 
> Ah, you are right, should be int. :)
> 
>>
>>> Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
>>> ---
>>>   src/storage/storage_backend.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>>> index 4b1b00d..f510080 100644
>>> --- a/src/storage/storage_backend.c
>>> +++ b/src/storage/storage_backend.c
>>> @@ -135,7 +135,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
>>>       int amtread = -1;
>>>       int ret = 0;
>>>       size_t rbytes = READ_BLOCK_SIZE_DEFAULT;
>>> -    size_t wbytes = 0;
>>> +    unsigned int wbytes = 0;
>>>       int interval;
>>>       char *zerobuf = NULL;
>>>       char *buf = NULL;
>>>
>> I'll push this with s/unsigned // later, unless someone has a different
>> opinion.
> 
> Got it, thanks.
> 

Now pushed.

Jan


Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]