Change in vdsm[master]: Remove '/var/log/core' for fedora

danken at redhat.com danken at redhat.com
Sat May 12 18:32:58 UTC 2012


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@/*.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 at linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list