Re: [Xen-devel] [PATCH 11/11] oxenstored: honour XEN_{LOG, RUN}_DIR in oxenstored.conf

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jun 09, 2016 at 09:38:55PM +0100, David Scott wrote:
> 
> > On 9 Jun 2016, at 16:51, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> > 
> > Wei Liu writes ("[PATCH 11/11] oxenstored: honour XEN_{LOG,RUN}_DIR in 
> > oxenstored.conf"):
> >> Generate oxenstored.conf with configure. This involves modifying
> >> tools/configure.ac and rerun autogen.sh.
> >> 
> >> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> ---
> >> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> >> Cc: David Scott <dave@xxxxxxxxxx>
> > 
> > You should mention that autogen.sh should be rerun.
> > 
> > Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > 
> >> There are two hard-coded paths in logging.ml, but I'm not sure if
> >> generate an ocaml _Path module is the right thing to do.
> > 
> > I would be interested to hear Dave's opinion.
> 
> For reference the paths are:
> 
>   let xenstored_log_destination = ref (File "/var/log/xenstored.log")
>   let access_log_destination = ref (File "/var/log/xenstored-access.log”)
> 
> These correspond to the command line arguments:
> 
>   ("access-log-file", Config.String Logging.set_access_log_destination);
>   ("xenstored-log-file", Config.String Logging.set_xenstored_log_destination);
> 
> I think if you want to remove these paths completely from the binary then 
> generating a simple module would be fine. I guess other options would be
> 
> - make the paths into optional values, default to None, and interpret None as 
> “don’t bother logging”. Might not be a good idea to encourage people to turn 
> off logging though.
> - make it mandatory to set the paths via the config file?
> 
> I don’t have a strong opinion though :-)
> 

I think this patch also requires an ack from you because this is
under the ocaml directory.

I've hold off pushing this one.

Wei.

> Cheers,
> Dave

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel