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; -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list