Dan Kenigsberg has posted comments on this change.
Change subject: Remove '/var/log/core' for fedora
......................................................................
Patch Set 1: (6 inline comments)
Thanks for taking this cleanup upon yourself. I'm have several concerns regarding the
implementation: I would like that EL6 run a vdsm-tool verb called --configure-core-dump
(it can also add coredump dir to MONITORED_DIR), and that nothing else in vdsm would
care.
....................................................
File configure.ac
Line 58: with_rhel=yes
this should better be a --with-conf-core-dump, that should be set by the spec file
according to the distribution. Maybe vdsm for Debian would want to configure core dump.
....................................................
File vdsm/utils.py
Line 203: MONITORED_PATHS = ['/tmp', '/var/log',
constants.P_VDSM_CORE_DUMP, constants.P_VDSM_RUN]
Arghh, too bad we need to be aware of VDSM_CORE_DUMP during runtime - I'd like to
avoid adding it to constants.
having VDSM_CORE_DUMP=='' to signify "no core dump dir" seems too hacky
to me. Please try, as a first patch, define MONITORED_PATHS in config.py. There, it can be
edited to be lacking VDSM_CORE_DUMP altogether.
....................................................
File vdsm/vdsmd.init.in
Line 476: echo $CORE_DUMP_PATTERN > $SYS_CORE_PATTERN
this is a great candidate for a vdsm-tool verb: "configure-core-dump" that is
not to be called by Fedora by default.
....................................................
File vdsm/vdsm-logrotate.conf.rhel.in
Line 1: @VDSMCOREDUMPDIR(a)/*.log {
this should have been VDSMLOGDIR, I suppose.
Line 10: }
oh, file name should not mention rhel (that's a trademark we should not use upstream),
use el6 instead. but i think it would be best to have a separate logrotate file about
coredump, which only the el6 spec would install.
Line 19: su @VDSMUSER@ @VDSMGROUP@
actually, el6's logrotate does not have su, so it shouldn't be here.
--
To view, visit
http://gerrit.ovirt.org/4348
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I594b5aa2ecaad1b1a268fa231a5769ef437ec75e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>