[libvirt] [PATCHv2] Eliminate large stack buffer in doTunnelSendAll

[Daniel Veillard]

On Fri, Mar 05, 2010 at 11:20:38PM -0500, Laine Stump wrote:
> doTunnelSendAll function (used by QEMU migration) uses a 64k buffer on
> the stack, which could be problematic. This patch replaces that with a
> buffer from the heap.
> 
> While in the neighborhood, this patch also improves error reporting in
> the case that saferead fails - previously, virStreamAbort() was called
> (resetting errno) before reporting the error. It's been changed to
> report the error first.
> ---
> 
> Eric noticed that I was inconsistent in the ordering of cleanup
> vs. error reporting, so I fixed that in this version.
> 
>  src/qemu/qemu_driver.c |   18 +++++++++++++++---
>  1 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index bb3edde..5a18938 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8399,19 +8399,28 @@ cleanup:
>  }
>  
>  
> +#define TUNNEL_SEND_BUF_SIZE 65536
> +
>  static int doTunnelSendAll(virStreamPtr st,
>                             int sock)
>  {
> -    char buffer[65536];
> -    int nbytes = sizeof(buffer);
> +    char *buffer;
> +    int nbytes = TUNNEL_SEND_BUF_SIZE;
> +
> +    if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) {
> +        virReportOOMError();
> +        virStreamAbort(st);
> +        return -1;
> +    }
>  
>      /* XXX should honour the 'resource' parameter here */
>      for (;;) {
>          nbytes = saferead(sock, buffer, nbytes);
>          if (nbytes < 0) {
> -            virStreamAbort(st);
>              virReportSystemError(errno, "%s",
>                                   _("tunnelled migration failed to read from qemu"));
> +            virStreamAbort(st);
> +            VIR_FREE(buffer);
>              return -1;
>          }
>          else if (nbytes == 0)
> @@ -8421,10 +8430,13 @@ static int doTunnelSendAll(virStreamPtr st,
>          if (virStreamSend(st, buffer, nbytes) < 0) {
>              qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
>                              _("Failed to write migration data to remote libvirtd"));
> +            VIR_FREE(buffer);
>              return -1;
>          }
>      }
>  
> +    VIR_FREE(buffer);
> +
>      if (virStreamFinish(st) < 0)
>          /* virStreamFinish set the error for us */
>          return -1;

  ACK, this needed fixing ! Pushed,

   thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/