On 03/08/2011 03:44 AM, Daniel Veillard wrote:
> Allow to dynamically set the size of the debug buffer
>
> This is the part allowing to dynamically resize the debug log
> buffer from it's default 64kB size. The buffer is now dynamically
> allocated.
> It adds a new API virLogSetBufferSize() which resizes the buffer
> If passed a zero size, the buffer is deallocated and we do the small
> optimization of not formatting messages which are not output anymore.
Can you convince git to also include the diffstat of your patches?
> @@ -192,6 +195,15 @@ int virLogStartup(void) {
>
> virLogInitialized = 1;
> virLogLock();
> + if (VIR_ALLOC_N(virLogBuffer, virLogSize) < 0) {
> + /*
> + * The debug buffer is not a critical component, allow startup
> + * even in case of failure to allocate it in case of a
> + * configuration mistake.
> + */
> + virReportOOMError();
> + virLogSize = 0;
If there was a configuration mistake, such as requesting way too much
memory, should we first try a fallback of 64k before giving up completely?
Is virReportOOMError() the right thing to do here, if we are proceeding
on with execution, or do we need to issue a manual VIR_WARN instead?
> /**
> + * virLogSetBufferSize:
> + * @size: size of the buffer in kilobytes or 0 to deactivate
> + *
> + * Dynamically set the size or desactivate the logging buffer used to keep
s/desactivate/deactivate/
> +extern int
> +virLogSetBufferSize(int size) {
Why not s/int/size_t/, so that 64-bit systems with beefy RAM can request
gigabytes of log? Then again, that seems like a resource hog, so int is
probably okay.
> + int ret = 0;
> + int oldsize;
> + char *oldLogBuffer;
> +
> + if (size < 0)
> + size = 0;
> +
> + if ((virLogInitialized == 0) || (size * 1024 == virLogSize))
Ouch - you didn't check for integer overflow before doing this multiply.
You need to add something like:
if (INT_MAX / 1024 < size) {
VIR_WARN("requested log size too large: %d", size);
return -1;
}
> + return ret;
> +
> + virLogLock();
> +
> + oldsize = virLogSize;
> + oldLogBuffer = virLogBuffer;
> +
> + virLogSize = size * 1024;
> + if (VIR_ALLOC_N(virLogBuffer, virLogSize) < 0) {
> + virReportOOMError();
> + virLogBuffer = oldLogBuffer;
> + virLogSize = oldsize;
> + ret =-1;
Spacing looks awkward, s/=-1/= -1/
> @@ -350,23 +408,28 @@ virLogEmergencyDumpAll(int signum) {
> virLogDumpAllFD( "Caught unexpected signal", -1);
> break;
> }
> + if ((virLogBuffer == NULL) || (virLogSize <= 0)) {
> + virLogDumpAllFD(" internal log buffer desactivated\n", -1);
s/desactivated/deactivated/
--
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