On 04/27/2012 07:22 AM, Michal Privoznik wrote:
Currently, we are allocating buffer for RPC messages statically. This is not such pain when RPC limits are small. However, if we want ever to increase those limits, we need to allocate buffer dynamically, based on RPC message len (= the first 4 bytes). Therefore we will decrease our mem usage in most cases and still be flexible enough in corner cases. --- src/rpc/virnetclient.c | 16 ++- src/rpc/virnetmessage.c | 12 ++- src/rpc/virnetmessage.h | 6 +- src/rpc/virnetserverclient.c | 20 ++- tests/virnetmessagetest.c | 393 +++++++++++++++++++++++-------------------
Wow, the biggest impact was to the testsuite.
5 files changed, 264 insertions(+), 183 deletions(-)
+++ b/src/rpc/virnetmessage.h @@ -31,13 +31,11 @@ typedef virNetMessage *virNetMessagePtr;
typedef void (*virNetMessageFreeCallback)(virNetMessagePtr msg, void *opaque);
-/* Never allocate this (huge) buffer on the stack. Always - * use virNetMessageNew() to allocate on the heap - */ struct _virNetMessage { bool tracked;
- char buffer[VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX]; + //char buffer[VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX]; + char *buffer;
Drop the C++ comment (starting with //), and instead write: char *buffer; /* Typically VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX */
@@ -373,6 +378,9 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, if (!(client->rx = virNetMessageNew(true))) goto error; client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX; + if (VIR_ALLOC_N(client->rx->buffer, client->rx->bufferLength) < 0) { + virReportOOMError(); + }
Shouldn't this 'goto error'?
@@ -922,6 +930,11 @@ readmore: client->wantClose = true; } else { client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX; + if (VIR_ALLOC_N(client->rx->buffer, + client->rx->bufferLength) < 0) { + virReportOOMError(); + client->wantClose = true; + } client->nrequests++;
Do you still want nrequests incremented on failure to allocate? Everything else looked okay. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org