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 -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list