[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 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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