12:35 p.m.
The lxc controller can't see libvirtd's log level setting so it
needs to re-query it from the environment. The parsing code has
a few users now, so I added a new function to the internal API,
virLogParseDefaultPriority() along the lines of the other parse
functions.
Signed-off-by: Amy Griffis <amy.griffis(a)hp.com>
diff --git a/qemud/qemud.c b/qemud/qemud.c
index a58a767..bd17ccb 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -2465,16 +2465,8 @@ qemudSetLogging(virConfPtr conf, const char *filename) {
*/
GET_CONF_INT (conf, filename, log_level);
debugEnv = getenv("LIBVIRT_DEBUG");
- if (debugEnv && *debugEnv && *debugEnv != '0') {
- if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info"))
- log_level = VIR_LOG_INFO;
- else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning"))
- log_level = VIR_LOG_WARN;
- else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error"))
- log_level = VIR_LOG_ERROR;
- else
- log_level = VIR_LOG_DEBUG;
- }
+ if (debugEnv && *debugEnv && *debugEnv != '0')
+ log_level = virLogParseDefaultPriority(debugEnv);
if ((verbose) && (log_level >= VIR_LOG_WARN))
log_level = VIR_LOG_INFO;
virLogSetDefaultPriority(log_level);
diff --git a/src/libvirt.c b/src/libvirt.c
index bf49018..a3f36da 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -258,6 +258,7 @@ virInitialize(void)
{
#ifdef ENABLE_DEBUG
char *debugEnv;
+ int logPrio;
#endif
if (initialized)
return(0);
@@ -272,14 +273,8 @@ virInitialize(void)
#ifdef ENABLE_DEBUG
debugEnv = getenv("LIBVIRT_DEBUG");
if (debugEnv && *debugEnv && *debugEnv != '0') {
- if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info"))
- virLogSetDefaultPriority(VIR_LOG_INFO);
- else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning"))
- virLogSetDefaultPriority(VIR_LOG_WARN);
- else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error"))
- virLogSetDefaultPriority(VIR_LOG_ERROR);
- else
- virLogSetDefaultPriority(VIR_LOG_DEBUG);
+ logPrio = virLogParseDefaultPriority(debugEnv);
+ virLogSetDefaultPriority(logPrio);
}
debugEnv = getenv("LIBVIRT_LOG_FILTERS");
if (debugEnv)
diff --git a/src/libvirt_debug.syms b/src/libvirt_debug.syms
index 1742a0b..539d879 100644
--- a/src/libvirt_debug.syms
+++ b/src/libvirt_debug.syms
@@ -12,6 +12,7 @@ virLogMessage;
virLogSetDefaultPriority;
virLogDefineFilter;
virLogDefineOutput;
+virLogParseDefaultPriority;
virLogParseFilters;
virLogParseOutputs;
virLogStartup;
diff --git a/src/logging.c b/src/logging.c
index 83e07cc..c72d52e 100644
--- a/src/logging.c
+++ b/src/logging.c
@@ -793,5 +793,30 @@ int virLogParseFilters(const char *filters) {
}
return(ret);
}
+
+/**
+ * virLogParseDefaultPriority:
+ * @priority: string defining the desired logging behavior
+ *
+ * It can take a string or number corresponding to the following log
+ * levels:
+ * 1: DEBUG
+ * 2: INFO
+ * 3: WARNING
+ * 4: ERROR
+ *
+ * DEBUG is returned if @priority doesn't match any of the other values.
+ */
+int virLogParseDefaultPriority(const char *priority) {
+
+ if (STREQ(priority, "2") || STREQ(priority, "info"))
+ return VIR_LOG_INFO;
+ else if (STREQ(priority, "3") || STREQ(priority, "warning"))
+ return VIR_LOG_WARN;
+ else if (STREQ(priority, "4") || STREQ(priority, "error"))
+ return VIR_LOG_ERROR;
+ else
+ return VIR_LOG_DEBUG;
+}
#endif /* ENABLE_DEBUG */
diff --git a/src/logging.h b/src/logging.h
index 7ea8935..72b3119 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -110,6 +110,7 @@ typedef void (*virLogCloseFunc) (void *data);
#ifdef ENABLE_DEBUG
+extern int virLogParseDefaultPriority(const char *priority);
extern int virLogSetDefaultPriority(int priority);
extern int virLogDefineFilter(const char *match, int priority, int flags);
extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c,
@@ -130,6 +131,7 @@ extern void virLogMessage(const char *category, int priority,
#else /* ENABLE_DEBUG */
+#define virLogParseDefaultPriority(p)
#define virLogSetDefaultPriority(p)
#define virLogDefineFilter(m, p, f)
#define virLogDefineOutput(func, c, d, p, f)
diff --git a/src/lxc_controller.c b/src/lxc_controller.c
index 356ecb8..9355d39 100644
--- a/src/lxc_controller.c
+++ b/src/lxc_controller.c
@@ -594,6 +594,8 @@ int main(int argc, char *argv[])
int monitor = -1;
int appPty = -1;
int bg = 0;
+ char *debugEnv;
+ int logPrio;
virCapsPtr caps = NULL;
virDomainDefPtr def = NULL;
char *configFile = NULL;
@@ -735,6 +737,13 @@ int main(int argc, char *argv[])
}
}
+ /* New programs need to re-query and set the libvirt log level. */
+ debugEnv = getenv("LIBVIRT_DEBUG");
+ if (debugEnv && *debugEnv && *debugEnv != '0') {
+ logPrio = virLogParseDefaultPriority(debugEnv);
+ virLogSetDefaultPriority(logPrio);
+ }
+
/* Accept initial client which is the libvirtd daemon */
if ((client = accept(monitor, NULL, 0)) < 0) {
virReportSystemError(NULL, errno, "%s",
1:44 p.m.
On 06/16/2009 01:35 PM, Amy Griffis wrote:
The lxc controller can't see libvirtd's log level setting so
it
needs to re-query it from the environment. The parsing code has
a few users now, so I added a new function to the internal API,
virLogParseDefaultPriority() along the lines of the other parse
functions.
I'd say go the extra step and add something like virLogSetFromEnv, which
encapsulates the duplicate getenv calls as well.
There is also another duplicate of this code in tests/eventtest.c.
Thanks,
Cole
5:12 p.m.
Cole Robinson wrote: [Tue Jun 16 2009, 02:44:28PM EDT]
On 06/16/2009 01:35 PM, Amy Griffis wrote:
> The lxc controller can't see libvirtd's log level setting so it
> needs to re-query it from the environment. The parsing code has
> a few users now, so I added a new function to the internal API,
> virLogParseDefaultPriority() along the lines of the other parse
> functions.
I'd say go the extra step and add something like virLogSetFromEnv, which
encapsulates the duplicate getenv calls as well.
I thought about that, but wanted to keep consistent behavior with
the other two parsing routines. I think we could go ahead and
change all of them to include the getenv(). Only minor gotcha is
qemud wants to call virLogParseOutputs() with it's own string in
one case. So we'd need to make the getenv() conditional on not
getting an input string. I think this would be cleaner. What do
you think?
There is also another duplicate of this code in tests/eventtest.c.
Oops, missed that one. Thanks.
Amy
7:14 p.m.
On 06/16/2009 06:12 PM, Amy Griffis wrote:
Cole Robinson wrote: [Tue Jun 16 2009, 02:44:28PM EDT]
> On 06/16/2009 01:35 PM, Amy Griffis wrote:
>> The lxc controller can't see libvirtd's log level setting so it
>> needs to re-query it from the environment. The parsing code has
>> a few users now, so I added a new function to the internal API,
>> virLogParseDefaultPriority() along the lines of the other parse
>> functions.
> I'd say go the extra step and add something like virLogSetFromEnv, which
> encapsulates the duplicate getenv calls as well.
I thought about that, but wanted to keep consistent behavior with
the other two parsing routines. I think we could go ahead and
change all of them to include the getenv(). Only minor gotcha is
qemud wants to call virLogParseOutputs() with it's own string in
one case. So we'd need to make the getenv() conditional on not
getting an input string. I think this would be cleaner. What do
you think?
My preference would be to make both virLogParseDefaultPriority and
virLogSetFromEnv, rather than have one function with 2 different results.
- Cole
5:44 a.m.
On Tue, Jun 16, 2009 at 06:12:04PM -0400, Amy Griffis wrote:
Cole Robinson wrote: [Tue Jun 16 2009, 02:44:28PM EDT]
> On 06/16/2009 01:35 PM, Amy Griffis wrote:
> > The lxc controller can't see libvirtd's log level setting so it
> > needs to re-query it from the environment. The parsing code has
> > a few users now, so I added a new function to the internal API,
> > virLogParseDefaultPriority() along the lines of the other parse
> > functions.
>
> I'd say go the extra step and add something like virLogSetFromEnv, which
> encapsulates the duplicate getenv calls as well.
I thought about that, but wanted to keep consistent behavior with
the other two parsing routines. I think we could go ahead and
change all of them to include the getenv(). Only minor gotcha is
qemud wants to call virLogParseOutputs() with it's own string in
one case. So we'd need to make the getenv() conditional on not
getting an input string. I think this would be cleaner. What do
you think?
I think there's valid reasons for both suggested APIs.
I'd suggest a slight variation on your original API, instead of:
+ logPrio = virLogParseDefaultPriority(debugEnv);
+ virLogSetDefaultPriority(logPrio);
Just have a 'virLogSetDefaultPriorityFromString(char *str)',
eg so it'd parse the string and then call virLogSetDefaultPriority
for you.
Also then having a virLogSetFromEnv() method which uses getenv to
fetch the vars and then calls the virLogSet... methods
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
1:37 p.m.
Daniel P. Berrange wrote: [Wed Jun 17 2009, 06:44:31AM EDT]
On Tue, Jun 16, 2009 at 06:12:04PM -0400, Amy Griffis wrote:
> Cole Robinson wrote: [Tue Jun 16 2009, 02:44:28PM EDT]
> > On 06/16/2009 01:35 PM, Amy Griffis wrote:
> > > The lxc controller can't see libvirtd's log level setting so it
> > > needs to re-query it from the environment. The parsing code has
> > > a few users now, so I added a new function to the internal API,
> > > virLogParseDefaultPriority() along the lines of the other parse
> > > functions.
> >
> > I'd say go the extra step and add something like virLogSetFromEnv, which
> > encapsulates the duplicate getenv calls as well.
>
> I thought about that, but wanted to keep consistent behavior with
> the other two parsing routines. I think we could go ahead and
> change all of them to include the getenv(). Only minor gotcha is
> qemud wants to call virLogParseOutputs() with it's own string in
> one case. So we'd need to make the getenv() conditional on not
> getting an input string. I think this would be cleaner. What do
> you think?
I think there's valid reasons for both suggested APIs.
I'd suggest a slight variation on your original API, instead of:
+ logPrio = virLogParseDefaultPriority(debugEnv);
+ virLogSetDefaultPriority(logPrio);
Just have a 'virLogSetDefaultPriorityFromString(char *str)',
eg so it'd parse the string and then call virLogSetDefaultPriority
for you.
Since the config file parser in libvirtd is returning an integer,
it actually needs one function to get the environment setting and
convert it to integer, and then a second function to set the
value in the log module from an integer.
Also then having a virLogSetFromEnv() method which uses getenv to
fetch the vars and then calls the virLogSet... methods
But I see what you guys are saying. I'll make these changes and
send a new patch.
Thanks,
Amy
12:25 p.m.
Taking a second look at the log level code and documentation,
there seems to be some confusion around the value of '0'. It's
not one of the defined log priorities, but it is mentioned in the
documentation on the website both as meaning "log everything" and
"no logging at all".
In the code, there are times when it's accepted (for outputs),
rejected (for filters) and ignored (for the loglevel). For the
outputs, specifying level 0 is effectively the same as specifying
level 1.
I'm wondering if there is any historical reason for the '0'
value, or if we could just do away with it completely?
Thanks,
Amy
8:58 a.m.
On Thu, Jun 18, 2009 at 01:25:46PM -0400, Amy Griffis wrote:
Taking a second look at the log level code and documentation,
there seems to be some confusion around the value of '0'. It's
not one of the defined log priorities, but it is mentioned in the
documentation on the website both as meaning "log everything" and
"no logging at all".
In the code, there are times when it's accepted (for outputs),
rejected (for filters) and ignored (for the loglevel). For the
outputs, specifying level 0 is effectively the same as specifying
level 1.
I'm wondering if there is any historical reason for the '0'
value, or if we could just do away with it completely?
Well I think we should clean this up, yes it's a bit of a mess
and I think that's the kind of things were we can clean things
without really breaking compatibility.
I would map it uniformly as same as 1, and clean up the documentation
to remove any reference. I guess that's the best considering the
current state. If you feel you can work out a patch, that would be
great :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5635
days inactive
5638
days old
7 comments
4 participants
participants (4)
-
Amy Griffis -
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard