Re: [libvirt] [PATCH] Fix logging in libvirt_lxc controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]