[libvirt] [PATCH 1/1] storage: Fix a vol-clone bug for ppc64
From: Li Zhang <zhlcindy@linux.vnet.ibm.com> vol-clone reports out of memory error with disk type on ppc64. 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. This patch changes size_t to unsigned int to synchronize with kernel. Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- 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; -- 1.8.2.1
On 11/07/2013 09:35 AM, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
vol-clone reports out of memory error with disk type on ppc64.
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.
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/i... https://lkml.org/lkml/2013/11/1/620
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- 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. Jan
On 2013年11月07日 18:41, Ján Tomko wrote:
On 11/07/2013 09:35 AM, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
vol-clone reports out of memory error with disk type on ppc64.
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.
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/i... https://lkml.org/lkml/2013/11/1/620
Ah, you are right, should be int. :)
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- 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.
Jan
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/i...
Ah, you are right, should be int. :)
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- 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
On 2013年11月08日 15:10, Ján Tomko wrote:
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.
Ah, I converted the big endian incorrectly. Thanks for your explanation.
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/i...
https://lkml.org/lkml/2013/11/1/620 Ah, you are right, should be int. :)
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- 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
participants (2)
-
Ján Tomko -
Li Zhang