[libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Chen Hanxiao
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

When adding a large number of virtio storage devices to
virtual machine by using 'virsh edit' command, there is
a problem:
When we added more than 190 virtio disks by 'virsh edit'
command, we got a feedback as 'error: Unable to encode
message payload'. In virt-mananger, the same defect occurs
with about 180 virtio disks added.

PCI devices are limited by the virtualized system
architecture. Out of the 32 available PCI devices for a
guest, 4 are not removable. This means there are up to
28 free PCI slots available for additional devices per
guest. Each PCI device in a guest can have up to 8 functions.
One slot kept for network interface, we could add at most
216 (27*8) virtio disks.
The length of xml description with multifuction for one virtio
disk is about 290 characters; In virt-manger, an extra tag
'alias name' will come to the xml description, which would add
about 40 more characters.
So for one xml description, 330 characters would be enough.
A brand new virtual machine with one IDE bus disk needs about
1900 characters for xml description.

In src/remote/remote_protocol.x, there is a limitation for XDR
enoding length as REMOTE_STRING_MAX.
According to the analysis above, at least 73180(1900+216*330)
characters are needed for 216 virtio disks. In the view of
variable length in tag 'source file', so I think it would be
better to extend this limitation from 65536 to 8.
---
 src/remote/remote_protocol.x |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2d57247..2c8dcbb 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -65,7 +65,7 @@
  * This is an arbitrary limit designed to stop the decoder from trying
  * to allocate unbounded amounts of memory when fed with a bad message.
  */
-const REMOTE_STRING_MAX = 65536;
+const REMOTE_STRING_MAX = 8;
 
 /* A long string, which may NOT be NULL. */
 typedef string remote_nonnull_stringREMOTE_STRING_MAX;
-- 
1.7.1

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Michal Privoznik
On 19.04.2012 08:25, Chen Hanxiao wrote:
 From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
 When adding a large number of virtio storage devices to
 virtual machine by using 'virsh edit' command, there is
 a problem:
 When we added more than 190 virtio disks by 'virsh edit'
 command, we got a feedback as 'error: Unable to encode
 message payload'. In virt-mananger, the same defect occurs
 with about 180 virtio disks added.
 
 PCI devices are limited by the virtualized system
 architecture. Out of the 32 available PCI devices for a
 guest, 4 are not removable. This means there are up to
 28 free PCI slots available for additional devices per
 guest. Each PCI device in a guest can have up to 8 functions.
 One slot kept for network interface, we could add at most
 216 (27*8) virtio disks.
 The length of xml description with multifuction for one virtio
 disk is about 290 characters; In virt-manger, an extra tag
 'alias name' will come to the xml description, which would add
 about 40 more characters.
 So for one xml description, 330 characters would be enough.
 A brand new virtual machine with one IDE bus disk needs about
 1900 characters for xml description.
 
 In src/remote/remote_protocol.x, there is a limitation for XDR
 enoding length as REMOTE_STRING_MAX.
 According to the analysis above, at least 73180(1900+216*330)
 characters are needed for 216 virtio disks. In the view of
 variable length in tag 'source file', so I think it would be
 better to extend this limitation from 65536 to 8.
 ---
  src/remote/remote_protocol.x |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
 index 2d57247..2c8dcbb 100644
 --- a/src/remote/remote_protocol.x
 +++ b/src/remote/remote_protocol.x
 @@ -65,7 +65,7 @@
   * This is an arbitrary limit designed to stop the decoder from trying
   * to allocate unbounded amounts of memory when fed with a bad message.
   */
 -const REMOTE_STRING_MAX = 65536;
 +const REMOTE_STRING_MAX = 8;
  
  /* A long string, which may NOT be NULL. */
  typedef string remote_nonnull_stringREMOTE_STRING_MAX;

While this limit is pretty harmless, the much bigger issue is - a per
message buffer of VIR_NET_MESSAGE_MAX bytes which is hold during whole
API. I am writing a code to allocate this buffer dynamically based on
message length.

btw: don't you want size up VIR_NET_MESSAGE_STRING_MAX in
virnetprotocol.x as well?

Michal

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Richard W.M. Jones
On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
   * This is an arbitrary limit designed to stop the decoder from trying
   * to allocate unbounded amounts of memory when fed with a bad message.
   */
 -const REMOTE_STRING_MAX = 65536;
 +const REMOTE_STRING_MAX = 8;

Can this limit be changed?  I thought it would break existing clients
or servers.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Michal Privoznik
On 19.04.2012 14:45, Richard W.M. Jones wrote:
 On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
   * This is an arbitrary limit designed to stop the decoder from trying
   * to allocate unbounded amounts of memory when fed with a bad message.
   */
 -const REMOTE_STRING_MAX = 65536;
 +const REMOTE_STRING_MAX = 8;
 
 Can this limit be changed?  I thought it would break existing clients
 or servers.
 
 Rich.
 

Yes  no. There are 4 possibilities:

- old both server  client: long string get dropped at server side and
unable to encode string/payload error is thrown.

- old server  new client: if server wants to send long string it's the
previous case; client will successfully retransmit message to the daemon
where it gets later dropped because of limit violation.

- new server  old client: see previous case

- new server  new client: nothing gets broken, everything works.

The only problem is with these gimme-list-of-* APIs. Because here you'll
get only partial result (first N entries - based on RPC limit).

Thus, changing this limit is okay.

Michal

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Richard W.M. Jones
On Thu, Apr 19, 2012 at 03:04:09PM +0200, Michal Privoznik wrote:
 On 19.04.2012 14:45, Richard W.M. Jones wrote:
  On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
* This is an arbitrary limit designed to stop the decoder from trying
* to allocate unbounded amounts of memory when fed with a bad message.
*/
  -const REMOTE_STRING_MAX = 65536;
  +const REMOTE_STRING_MAX = 8;
  
  Can this limit be changed?  I thought it would break existing clients
  or servers.
  
  Rich.
  
 
 Yes  no. There are 4 possibilities:
 
 - old both server  client: long string get dropped at server side and
 unable to encode string/payload error is thrown.
 
 - old server  new client: if server wants to send long string it's the
 previous case; client will successfully retransmit message to the daemon
 where it gets later dropped because of limit violation.
 
 - new server  old client: see previous case
 
 - new server  new client: nothing gets broken, everything works.
 
 The only problem is with these gimme-list-of-* APIs. Because here you'll
 get only partial result (first N entries - based on RPC limit).
 
 Thus, changing this limit is okay.

If we're going to change it, I say change it to something much bigger.

The original limit was very conservative, and was based on some
assumptions about wanting to keep buffers on the stack which is no
longer true in modern libvirt code.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Chen HanXiao
On Thu, Apr 19, 2012 at 03:04:09PM +0200, Michal Privoznik wrote:
 On 19.04.2012 14:45, Richard W.M. Jones wrote:
  On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
* This is an arbitrary limit designed to stop the decoder from trying
* to allocate unbounded amounts of memory when fed with a bad message.
*/
  -const REMOTE_STRING_MAX = 65536;
  +const REMOTE_STRING_MAX = 8;
  
  Can this limit be changed?  I thought it would break existing clients
  or servers.
  
  Rich.
  
 
If we're going to change it, I say change it to something much bigger.

The original limit was very conservative, and was based on some
assumptions about wanting to keep buffers on the stack which is no
longer true in modern libvirt code.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/


It would be beter to change current REMOTE_STRING_MAX to a bigger number.
But VIR_NET_MESSAGE_MAX limited the buffer for XDR stream as 
4*REMOTE_STRING_MAX in
current libvirt.
So could the REMOTE_STRING_MAX change as half as the VIR_NET_MESSAGE_MAX?

-const REMOTE_STRING_MAX = 65536;
+const REMOTE_STRING_MAX = 131072;  //twice as many as before


It's accuracy to allocate the buffer size 4 times as much as message size.
According to the real message, most of them are strings. So I think 
REMOTE_STRING_MAX with half size of VIR_NET_MESSAGE_MAX is not 
a bad choice.
I test this change on my machine, it seems to work fine.

If Michal's dynamically allocating buffer function finished, we could change 
this 
limit to a bigger one.

--
Regards

Chen HanXiao

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