Re: [libvirt] [PATCH] Allow to dynamically set the size of the debug buffer
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)