[libvirt] [PATCH] Allow to dynamically set the size of the debug buffer

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110308/1458f566/attachment-0001.sig>