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

[David Scott <dave@recoil.org>]

> On 9 Jun 2016, at 16:51, Ian Jackson <Ian.Jackson@eu.citrix.com> 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@citrix.com>
>> ---
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: David Scott <dave@recoil.org>
> 
> You should mention that autogen.sh should be rerun.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
>> 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 :-)

Cheers,
Dave
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel