On 03/05/2010 02:30 PM, 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.
There's a lot more large stacks noted by Coverity, but this is a good start.
>
> 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.
Yep.
> - 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) {
> + virStreamAbort(st);
> + virReportOOMError();
Is this backwards?
> + 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);
Especially given that you just reordered the virStreamAbort to come last
here? If the above was not a mistake, then ACK to the patch.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list