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

Re: [libvirt] [PATCH v2 2/2] rpc: Size up RPC limits



On 17.05.2012 00:27, Eric Blake wrote:
> On 05/15/2012 09:04 AM, Michal Privoznik wrote:
>> Since we are allocating RPC buffer dynamically, we can increase limits
>> for max. size of RPC message and RPC string. This is needed to cover
>> some corner cases where libvirt is run on such huge machines that their
>> capabilities XML is 4 times bigger than our current limit. This leaves
>> users with inability to even connect.
>> ---
>>  src/remote/remote_protocol.x |   20 ++++++++++----------
>>  src/rpc/virnetprotocol.x     |    6 +++---
>>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> I think we've already analyzed the compatibility concerns:
> 
> small command or response:
> no change in behavior, regardless of the mix of old or new server and client
> 
> large command (for example, a 'virDomainCreate' with a huge XML):
> old client: fails, server never contacted
> new client, old server: server rejects the message, but I'm not sure
> what happens next - does the server still keep the connection alive?  At
> any rate, we can't change this behavior, except perhaps to teach the
> client to not send large commands without first validating what the
> server is willing to receive
> new client, new server: message can now get through
> 
> large response (for example, a huge capabilities XML):
> old server: response never sent (hence the commit message mentioning
> failure to even connect)
> new server, old client: client rejects the response, but I'm not sure
> what happens next - does the client drop the connection?  At any rate,
> we can't change this behavior, except perhaps to teach the server to not
> send large responses without first validating what the client is willing
> to recieve
> new server, new client: message can now get through
> 
> In both cases where I'm not sure what happens, I can't help but wonder
> if we could make the code smarter by adding a flag to the handshaking of
> an initial connection:
> 
> When connecting, the client sets a flag stating that it is new and
> understands larger limits.  If the server rejects the flag, then the
> client retries the connection without the flag, but also records that it
> must not send large commands, and gracefully fails up front without ever
> passing too much information over the RPC wire.
> 
> When receiving a connection, the server checks for the new flag.  If the
> client didn't pass the flag, then the server records that it must not
> send large responses, and gracefully sends error messages instead of
> aborting connections when encountering a situation where the response
> has to be huge.
> 
> However, I don't know if we need this much handshaking, or if we are
> okay with just larger limits and declaring the mismatch to be a problem
> only if you are in a situation where you want large messages in the
> first place.  And I think we can add any handshaking as a followup
> patch.  Therefore:
> 
>>  /* Upper limit on lists of domain names. */
>> -const REMOTE_DOMAIN_NAME_LIST_MAX = 1024;
>> +const REMOTE_DOMAIN_NAME_LIST_MAX = 16384;
>>  
>>  /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */
>>  const REMOTE_CPUMAP_MAX = 256;
>> @@ -94,25 +94,25 @@ const REMOTE_CPUMAPS_MAX = 16384;
>>  const REMOTE_MIGRATE_COOKIE_MAX = 16384;
>>  
>>  /* Upper limit on lists of network names. */
>> -const REMOTE_NETWORK_NAME_LIST_MAX = 256;
>> +const REMOTE_NETWORK_NAME_LIST_MAX = 4096;
>>  
>>  /* Upper limit on lists of interface names. */
>> -const REMOTE_INTERFACE_NAME_LIST_MAX = 256;
>> +const REMOTE_INTERFACE_NAME_LIST_MAX = 4096;
> 
> I still wonder if these should be larger to match
> REMOTE_DOMAIN_NAME_LIST_MAX, since with SRIOV devices, the notion of one
> virtual interface per VM makes sense.
> 
> 
>> @@ -160,13 +160,13 @@ const REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX = 1024;
>>   * Note applications need to be aware of this limit and issue multiple
>>   * requests for large amounts of data.
>>   */
>> -const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 65536;
>> +const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 1048576;
> 
> If I recall correctly, this limit was documented in src/libvirt.c; we
> should update that documentation to reflect the difference in limits and
> which version introduced the difference.
> 
> At this point, I'm comfortable giving ACK to this patch, modulo the
> libvirt.c doc tweak, to maximize our testing time, and give us a chance
> to decide if we also need to add handshaking support when establishing a
> connection.
> 

Okay, I'll squash this in:

diff --git a/src/libvirt.c b/src/libvirt.c
index 22fc863..ec1a61a 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -7578,6 +7578,7 @@ error:
  * NB. The remote driver imposes a 64K byte limit on 'size'.
  * For your program to be able to work reliably over a remote
  * connection you should split large requests to <= 65536 bytes.
+ * However, with 0.9.13 this RPC limit has been raised to 1M byte.
  *
  * Returns: 0 in case of success or -1 in case of failure.
  */
@@ -7738,6 +7739,7 @@ error:
  * NB. The remote driver imposes a 64K byte limit on 'size'.
  * For your program to be able to work reliably over a remote
  * connection you should split large requests to <= 65536 bytes.
+ * However, with 0.9.13 this RPC limit has been raised to 1M byte.
  *
  * Returns: 0 in case of success or -1 in case of failure.
  */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 79d47fb..d8a6576 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -94,13 +94,13 @@ const REMOTE_CPUMAPS_MAX = 16384;
 const REMOTE_MIGRATE_COOKIE_MAX = 16384;

 /* Upper limit on lists of network names. */
-const REMOTE_NETWORK_NAME_LIST_MAX = 4096;
+const REMOTE_NETWORK_NAME_LIST_MAX = 16384;

 /* Upper limit on lists of interface names. */
-const REMOTE_INTERFACE_NAME_LIST_MAX = 4096;
+const REMOTE_INTERFACE_NAME_LIST_MAX = 16384;

 /* Upper limit on lists of defined interface names. */
-const REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX = 4096;
+const REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX = 16384;

 /* Upper limit on lists of storage pool names. */
 const REMOTE_STORAGE_POOL_NAME_LIST_MAX = 4096;


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